Preload is now a standalone Twig function

symfony · dunglas · Jan 31, 2017 · Jan 31, 2017 · Jan 31, 2017 · Jan 31, 2017
commit 639d7d10f8d743c693394f6e1e0853b2de0f0507
diff --git a/src/Symfony/Bridge/Twig/Extension/AssetExtension.php b/src/Symfony/Bridge/Twig/Extension/AssetExtension.php
@@ -12,6 +12,7 @@
 namespace Symfony\Bridge\Twig\Extension;
 
 use Symfony\Component\Asset\Packages;
+use Symfony\Component\Asset\Preload\PreloadManagerInterface;
 
 /**
  * Twig extension for the Symfony Asset component.
@@ -21,10 +22,12 @@
 class AssetExtension extends \Twig_Extension
 {
     private $packages;
+    private $preloadManager;
 
-    public function __construct(Packages $packages)
+    public function __construct(Packages $packages, PreloadManagerInterface $preloadManager = null)
     {
         $this->packages = $packages;
+        $this->preloadManager = $preloadManager;
     }
 
     /**
@@ -34,8 +37,8 @@ public function getFunctions()
     {
         return array(
             new \Twig_SimpleFunction('asset', array($this, 'getAssetUrl')),
-            new \Twig_SimpleFunction('preloaded_asset', array($this, 'getAndPreloadAssetUrl')),
             new \Twig_SimpleFunction('asset_version', array($this, 'getAssetVersion')),
+            new \Twig_SimpleFunction('preload', array($this, 'preload')),
         );
     }
 
@@ -56,34 +59,36 @@ public function getAssetUrl($path, $packageName = null)
     }
 
     /**
-     * Returns the public url/path of an asset and preloads it.
-     *
-     * If the package used to generate the path is an instance of
-     * UrlPackage, you will always get a URL and not a path.
+     * Returns the version of an asset.
      *
-     * @param string      $path        A public path
-     * @param string      $as          A valid destination according to https://fetch.spec.whatwg.org/#concept-request-destination
-     * @param bool        $nopush      If this asset should not be pushed over HTTP/2
-     * @param string|null $packageName The name of the asset package to use
+     * @param string $path        A public path
+     * @param string $packageName The name of the asset package to use
      *
-     * @return string The public path of the asset
+     * @return string The asset version
      */
-    public function getAndPreloadAssetUrl($path, $as = '', $nopush = false, $packageName = null)
+    public function getAssetVersion($path, $packageName = null)
     {
-        return $this->packages->getAndPreloadUrl($path, $as, $nopush, $packageName);
+        return $this->packages->getVersion($path, $packageName);
     }
 
     /**
-     * Returns the version of an asset.
+     * Preloads an asset.
      *
-     * @param string $path        A public path
-     * @param string $packageName The name of the asset package to use
+     * @param string $path   A public path
+     * @param string $as     A valid destination according to https://fetch.spec.whatwg.org/#concept-request-destination
+     * @param bool   $nopush If this asset should not be pushed over HTTP/2
      *
-     * @return string The asset version
+     * @return string The path of the asset
      */
-    public function getAssetVersion($path, $packageName = null)
+    public function preload($path, $as = '', $nopush = false)
     {
-        return $this->packages->getVersion($path, $packageName);
+        if (null === $this->preloadManager) {
+            throw new \RuntimeException('A preload manager must be configured to use the "preload" function.');
+        }
+
+        $this->preloadManager->addResource($path, $as, $nopush);
+
+        return $path;
     }
 
     /**

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/AssetExtensionTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/AssetExtensionTest.php
@@ -12,10 +12,8 @@
 namespace Symfony\Bridge\Twig\Tests\Extension;
 
 use Symfony\Bridge\Twig\Extension\AssetExtension;
-use Symfony\Component\Asset\Package;
 use Symfony\Component\Asset\Packages;
 use Symfony\Component\Asset\Preload\HttpFoundationPreloadManager;
-use Symfony\Component\Asset\VersionStrategy\EmptyVersionStrategy;
 
 /**
  * @author Kévin Dunglas <dunglas@gmail.com>
@@ -25,9 +23,18 @@ class AssetExtensionTest extends \PHPUnit_Framework_TestCase
     public function testGetAndPreloadAssetUrl()
     {
         $preloadManager = new HttpFoundationPreloadManager();
-        $extension = new AssetExtension(new Packages(new Package(new EmptyVersionStrategy(), null, $preloadManager)));
+        $extension = new AssetExtension(new Packages(), $preloadManager);
 
-        $this->assertEquals('/foo.css', $extension->getAndPreloadAssetUrl('/foo.css', 'style', true));
+        $this->assertEquals('/foo.css', $extension->preload('/foo.css', 'style', true));
         $this->assertEquals(array('/foo.css' => array('as' => 'style', 'nopush' => true)), $preloadManager->getResources());
     }
+
+    /**
+     * @expectedException \RuntimeException
+     */
+    public function testNoConfiguredPreloadManager()
+    {
+        $extension = new AssetExtension(new Packages());
+        $extension->preload('/foo.css');
+    }
 }
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -772,10 +772,7 @@ private function registerAssetsConfiguration(array $config, ContainerBuilder $co
 
             $preloadListener = $container->register('asset.preload_listener', PreloadListener::class);
             $preloadListener->addArgument(new Reference('assets.preload_manager'));
-            $preloadListener->addTag('kernel.event_listener', array(
-                'event' => 'kernel.response',
-                'method' => 'onKernelResponse',
-            ));
+            $preloadListener->addTag('kernel.event_subscriber');
         }
 
         $defaultPackage = $this->createPackageDefinition($config['base_path'], $config['base_urls'], $defaultVersion);
@@ -819,10 +816,6 @@ private function createPackageDefinition($basePath, array $baseUrls, Reference $
             ->replaceArgument(1, $version)
         ;
 
-        if (class_exists(HttpFoundationPreloadManager::class)) {
-            $package->addArgument(new Reference('assets.preload_manager'));
-        }
-
         return $package;
     }
 

diff --git a/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml b/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml
@@ -70,6 +70,7 @@
 
         <service id="twig.extension.assets" class="Symfony\Bridge\Twig\Extension\AssetExtension" public="false">
             <argument type="service" id="assets.packages" />
+            <argument type="service" id="assets.preload_manager" on-invalid="ignore" />
         </service>
 
         <service id="twig.extension.code" class="Symfony\Bridge\Twig\Extension\CodeExtension" public="false">

diff --git a/src/Symfony/Component/Asset/EventListener/PreloadListener.php b/src/Symfony/Component/Asset/EventListener/PreloadListener.php
@@ -12,14 +12,16 @@
 namespace Symfony\Component\Asset\EventListener;
 
 use Symfony\Component\Asset\Preload\HttpFoundationPreloadManager;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
+use Symfony\Component\HttpKernel\KernelEvents;
 
 /**
  * Adds preload's Link HTTP headers to the response.
  *
  * @author Kévin Dunglas <dunglas@gmail.com>
  */
-class PreloadListener
+class PreloadListener implements EventSubscriberInterface
 {
     private $preloadManager;
 
@@ -31,5 +33,16 @@ public function __construct(HttpFoundationPreloadManager $preloadManager)
     public function onKernelResponse(FilterResponseEvent $event)
     {
         $this->preloadManager->setLinkHeader($event->getResponse());
+
+        // Free memory
+        $this->preloadManager->setResources(array());
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public static function getSubscribedEvents()
+    {
+        return array(KernelEvents::RESPONSE => 'onKernelResponse');
     }
 }
diff --git a/src/Symfony/Component/Asset/Package.php b/src/Symfony/Component/Asset/Package.php
@@ -22,19 +22,16 @@
  *
  * @author Kris Wallsmith <kris@symfony.com>
  * @author Fabien Potencier <fabien@symfony.com>
- * @author Kévin Dunglas <dunglas@gmail.com>
  */
-class Package implements PreloadedPackageInterface
+class Package implements PackageInterface
 {
     private $versionStrategy;
     private $context;
-    private $preloadManager;
 
-    public function __construct(VersionStrategyInterface $versionStrategy, ContextInterface $context = null, PreloadManagerInterface $preloadManager = null)
+    public function __construct(VersionStrategyInterface $versionStrategy, ContextInterface $context = null)
     {
         $this->versionStrategy = $versionStrategy;
         $this->context = $context ?: new NullContext();
-        $this->preloadManager = $preloadManager;
     }
 
     /**
@@ -57,21 +54,6 @@ public function getUrl($path)
         return $this->versionStrategy->applyVersion($path);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function getAndPreloadUrl($path, $as = '', $nopush = false)
-    {
-        if (null === $this->preloadManager) {
-            throw new LogicException('There is no preload manager, configure one first.');
-        }
-
-        $url = $this->getUrl($path);
-        $this->preloadManager->addResource($url, $as, $nopush);
-
-        return $url;
-    }
-
     /**
      * @return ContextInterface
      */

diff --git a/src/Symfony/Component/Asset/Packages.php b/src/Symfony/Component/Asset/Packages.php
@@ -113,28 +113,4 @@ public function getUrl($path, $packageName = null)
     {
         return $this->getPackage($packageName)->getUrl($path);
     }
-
-    /**
-     * Returns the public path and preloads it.
-     *
-     * Absolute paths (i.e. http://...) are returned unmodified.
-     *
-     * @param string $path        A public path
-     * @param string $as          A valid destination according to https://fetch.spec.whatwg.org/#concept-request-destination
-     * @param bool   $nopush      If this asset should not be pushed over HTTP/2
-     * @param string $packageName The name of the asset package to use
-     *
-     * @return string A public path which takes into account the base path and URL path
-     *
-     * @throws InvalidArgumentException If the requested package doesn't support preloading
-     */
-    public function getAndPreloadUrl($path, $as = '', $nopush = false, $packageName = null)
-    {
-        $package = $this->getPackage($packageName);
-        if (!$package instanceof PreloadedPackageInterface) {
-            throw new InvalidArgumentException(sprintf('The "%s" package doesn\'t support preloading.', $packageName ?: 'default'));
-        }
-
-        return $package->getAndPreloadUrl($path, $as, $nopush);
-    }
 }
diff --git a/src/Symfony/Component/Asset/PreloadedPackageInterface.php b/src/Symfony/Component/Asset/PreloadedPackageInterface.php
diff --git a/src/Symfony/Component/Asset/Tests/EventListener/PreloadListenerTest.php b/src/Symfony/Component/Asset/Tests/EventListener/PreloadListenerTest.php
@@ -13,8 +13,10 @@
 
 use Symfony\Component\Asset\EventListener\PreloadListener;
 use Symfony\Component\Asset\Preload\HttpFoundationPreloadManager;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
+use Symfony\Component\HttpKernel\KernelEvents;
 
 /**
  * @author Kévin Dunglas <dunglas@gmail.com>
@@ -26,13 +28,21 @@ public function testOnKernelResponse()
         $manager = new HttpFoundationPreloadManager();
         $manager->addResource('/foo');
 
-        $listener = new PreloadListener($manager);
+        $subscriber = new PreloadListener($manager);
         $response = new Response();
 
         $event = $this->getMockBuilder(FilterResponseEvent::class)->disableOriginalConstructor()->getMock();
         $event->method('getResponse')->willReturn($response);
 
-        $listener->onKernelResponse($event);
+        $subscriber->onKernelResponse($event);
+
+        $this->assertInstanceOf(EventSubscriberInterface::class, $subscriber);
         $this->assertEquals('</foo>; rel=preload', $response->headers->get('Link'));
+        $this->assertEmpty($manager->getResources());
+    }
+
+    public function testSubscribedEvents()
+    {
+        $this->assertEquals(array(KernelEvents::RESPONSE => 'onKernelResponse'), PreloadListener::getSubscribedEvents());
     }
 }
diff --git a/src/Symfony/Component/Asset/Tests/PackageTest.php b/src/Symfony/Component/Asset/Tests/PackageTest.php
@@ -52,21 +52,4 @@ public function testGetVersion()
         $package = new Package(new StaticVersionStrategy('v1'));
         $this->assertEquals('v1', $package->getVersion('/foo'));
     }
-
-    public function testGetAndPreloadUrl()
-    {
-        $preloadManager = $this->getMockBuilder(PreloadManagerInterface::class)->getMock();
-        $preloadManager
-            ->expects($this->exactly(2))
-            ->method('addResource')
-            ->withConsecutive(
- 
179B
               array($this->equalTo('/foo'), $this->equalTo(''), $this->equalTo(false)),
-                array($this->equalTo('/bar'), $this->equalTo('script'), $this->equalTo(true))
-            )
-        ;
-
-        $package = new Package(new EmptyVersionStrategy(), null, $preloadManager);
-        $this->assertEquals('/foo', $package->getAndPreloadUrl('/foo'));
-        $this->assertEquals('/bar', $package->getAndPreloadUrl('/bar', 'script', true));
-    }
 }
diff --git a/src/Symfony/Component/Asset/Tests/PackagesTest.php b/src/Symfony/Component/Asset/Tests/PackagesTest.php
@@ -56,27 +56,6 @@ public function testGetUrl()
         $this->assertEquals('/foo?a', $packages->getUrl('/foo', 'a'));
     }
 
-    public function testGetAndPreloadUrl()
-    {
-        $preloadManager = $this->getMockBuilder(PreloadManagerInterface::class)->getMock();
-        $preloadManager
-            ->expects($this->exactly(2))
-            ->method('addResource')
-            ->withConsecutive(
-                array($this->equalTo('/foo?default'), $this->equalTo(''), $this->equalTo(false)),
-                array($this->equalTo('/foo?a'), $this->equalTo('script'), $this->equalTo(true))
-            )
-        ;
-
-        $packages = new Packages(
-            new Package(new StaticVersionStrategy('default'), null, $preloadManager),
-            array('a' => new Package(new StaticVersionStrategy('a'), null, $preloadManager))
-        );
-
-        $this->assertEquals('/foo?default', $packages->getAndPreloadUrl('/foo'));
-        $this->assertEquals('/foo?a', $packages->getAndPreloadUrl('/foo', 'script', true, 'a'));
-    }
-
     /**
      * @expectedException \Symfony\Component\Asset\Exception\LogicException
      */
@@ -94,13 +73,4 @@ public function testUndefinedPackage()
         $packages = new Packages();
         $packages->getPackage('a');
     }
-
-    /**
-     * @expectedException \Symfony\Component\Asset\Exception\InvalidArgumentException
-     */
-    public function testDoesNotSupportPreloading()
-    {
-        $packages = new Packages($this->getMockBuilder(PackageInterface::class)->getMock());
-        $packages->getAndPreloadUrl('/foo');
-    }
 }