Use a request attribute instead of a stateful service

symfony · dunglas · Apr 4, 2017 · Apr 4, 2017 · Apr 5, 2017 · Apr 6, 2017
commit aba1dbb11848aca4e7c9fd815b4919d8996b263d
diff --git a/src/Symfony/Bridge/Twig/Extension/WebLinkExtension.php b/src/Symfony/Bridge/Twig/Extension/WebLinkExtension.php
@@ -11,8 +11,9 @@

 namespace Symfony\Bridge\Twig\Extension;

+use Fig\Link\GenericLinkProvider;
 use Fig\Link\Link;
-use Symfony\Component\WebLink\WebLinkManagerInterface;
+use Symfony\Component\HttpFoundation\RequestStack;

 /**
 * Twig extension for the Symfony WebLink component.
@@ -21,11 +22,11 @@
 */
 class WebLinkExtension extends \Twig_Extension
 {
-    private $manager;
+    private $requestStack;

-    public function __construct(WebLinkManagerInterface $manager)
+    public function __construct(RequestStack $requestStack)
    {
-        $this->manager = $manager;
+        $this->requestStack = $requestStack;
    }

    /**
@@ -54,12 +55,17 @@ public function getFunctions()
     */
    public function link($uri, $rel, array $attributes = array())
    {
+        if (!$request = $this->requestStack->getMasterRequest()) {
+            return $uri;
+        }
+
        $link = new Link($rel, $uri);
        foreach ($attributes as $key => $value) {
            $link = $link->withAttribute($key, $value);
        }

-        $this->manager->add($link);
+        $linkProvider = $request->attributes->get('_links', new GenericLinkProvider());
+        $request->attributes->set('_links', $linkProvider->withLink($link));

        return $uri;
    }
@@ -74,7 +80,7 @@ public function link($uri, $rel, array $attributes = array())
     */
    public function preload($uri, array $attributes = array())
    {
-        return $this->link($uri, WebLinkManagerInterface::REL_PRELOAD, $attributes);
+        return $this->link($uri, 'preload', $attributes);
    }

    /**
@@ -87,7 +93,7 @@ public function preload($uri, array $attributes = array())
     */
    public function dnsPrefetch($uri, array $attributes = array())
    {
-        return $this->link($uri, WebLinkManagerInterface::REL_DNS_PREFETCH, $attributes);
+        return $this->link($uri, 'dns-prefetch', $attributes);
    }

    /**
@@ -100,7 +106,7 @@ public function dnsPrefetch($uri, array $attributes = array())
     */
    public function preconnect($uri, array $attributes = array())
    {
-        return $this->link($uri, WebLinkManagerInterface::REL_PRECONNECT, $attributes);
+        return $this->link($uri, 'preconnect', $attributes);
    }

    /**
@@ -113,7 +119,7 @@ public function preconnect($uri, array $attributes = array())
     */
    public function prefetch($uri, array $attributes = array())
    {
-        return $this->link($uri, WebLinkManagerInterface::REL_PREFETCH, $attributes);
+        return $this->link($uri, 'prefetch', $attributes);
    }

    /**
@@ -126,7 +132,7 @@ public function prefetch($uri, array $attributes = array())
     */
    public function prerender($uri, array $attributes = array())
    {
-        return $this->link($uri, WebLinkManagerInterface::REL_PRERENDER, $attributes);
+        return $this->link($uri, 'prerender', $attributes);
    }

    /**

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/WebLinkExtensionTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/WebLinkExtensionTest.php
@@ -14,17 +14,18 @@
 use Fig\Link\Link;
 use PHPUnit\Framework\TestCase;
 use Symfony\Bridge\Twig\Extension\WebLinkExtension;
-use Symfony\Component\WebLink\WebLinkManager;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestStack;

 /**
 * @author Kévin Dunglas <dunglas@gmail.com>
 */
 class WebLinkExtensionTest extends TestCase
 {
    /**
-     * @var WebLinkManager
+     * @var Request
     */
-    private $manager;
+    private $request;

    /**
     * @var WebLinkExtension
@@ -33,60 +34,64 @@ class WebLinkExtensionTest extends TestCase

    protected function setUp()
    {
-        $this->manager = new WebLinkManager();
-        $this->extension = new WebLinkExtension($this->manager);
+        $this->request = new Request();
+
+        $requestStack = new RequestStack();
+        $requestStack->push($this->request);
+
+        $this->extension = new WebLinkExtension($requestStack);
    }

    public function testLink()
    {
        $this->assertEquals('/foo.css', $this->extension->link('/foo.css', 'preload', array('as' => 'style', 'nopush' => true)));

        $link = (new Link('preload', '/foo.css'))->withAttribute('as', 'style')->withAttribute('nopush', true);
-        $this->assertEquals(array($link), array_values($this->manager->getLinkProvider()->getLinks()));
+        $this->assertEquals(array($link), array_values($this->request->attributes->get('_links')->getLinks()));
    }

    public function testPreload()
    {
        $this->assertEquals('/foo.css', $this->extension->preload('/foo.css', array('as' => 'style', 'crossorigin' => true)));

        $link = (new Link('preload', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true);
-        $this->assertEquals(array($link), array_values($this->manager->getLinkProvider()->getLinks()));
+        $this->assertEquals(array($link), array_values($this->request->attributes->get('_links')->getLinks()));
    }

    public function testDnsPrefetch()
    {
        $this->assertEquals('/foo.css', $this->extension->dnsPrefetch('/foo.css', array('as' => 'style', 'crossorigin' => true)));

        $link = (new Link('dns-prefetch', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true);
-        $this->assertEquals(array($link), array_values($this->manager->getLinkProvider()->getLinks()));
+        $this->assertEquals(array($link), array_values($this->request->attributes->get('_links')->getLinks()));
    }

    public function testPreconnect()
    {
        $this->assertEquals('/foo.css', $this->extension->preconnect('/foo.css', array('as' => 'style', 'crossorigin' => true)));

        $link = (new Link('preconnect', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true);
-        $this->assertEquals(array($link), array_values($this->manager->getLinkProvider()->getLinks()));
+        $this->assertEquals(array($link), array_values($this->request->attributes->get('_links')->getLinks()));
    }

    public function testPrefetch()
    {
        $this->assertEquals('/foo.css', $this->extension->prefetch('/foo.css', array('as' => 'style', 'crossorigin' => true)));

        $link = (new Link('prefetch', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true);
-        $this->assertEquals(array($link), array_values($this->manager->getLinkProvider()->getLinks()));
+        $this->assertEquals(array($link), array_values($this->request->attributes->get('_links')->getLinks()));
    }

    public function testPrerender()
    {
        $this->assertEquals('/foo.css', $this->extension->prerender('/foo.css', array('as' => 'style', 'crossorigin' => true)));

        $link = (new Link('prerender', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true);
-        $this->assertEquals(array($link), array_values($this->manager->getLinkProvider()->getLinks()));
+        $this->assertEquals(array($link), array_values($this->request->attributes->get('_links')->getLinks()));
    }

    public function testGetName()
    {
-        $this->assertEquals('web_link', (new WebLinkExtension(new WebLinkManager()))->getName());
+        $this->assertEquals('web_link', $this->extension->getName());
    }
 }
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/web_link.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/web_link.xml
@@ -6,16 +6,7 @@

    <services>

-        <service id="web_link.manager" class="Symfony\Component\WebLink\WebLinkManager" public="false">
-            <argument type="service">
-                <service class="Fig\Link\GenericLinkProvider" />
-            </argument>
-        </service>
-        <service id="Symfony\Component\WebLink\WebLinkManagerInterface" alias="web_link.manager" public="false" />
-
        <service id="web_link.add_link_header_listener" class="Symfony\Component\Link\WebLink\AddLinkHeaderListener" public="false">
-            <argument type="service" id="web_link.manager" />
-
            <tag name="kernel.event_subscriber" />
        </service>


diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
@@ -411,7 +411,7 @@ public function testAssetsDefaultVersionStrategyAsService()
    public function testWebLink()
    {
        $container = $this->createContainerFromFile('web_link');
-        $this->assertTrue($container->hasDefinition('web_link.manager'));
+        $this->assertTrue($container->hasDefinition('web_link.add_link_header_listener'));
    }

    public function testTranslator()

diff --git a/src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php b/src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php
@@ -53,7 +53,7 @@ public function load(array $configs, ContainerBuilder $container)
        if (interface_exists(WebLinkManagerInterface::class)) {
            $definition = $container->register('twig.extension.weblink', WebLinkExtension::class);
            $definition->setPublic(false);
-            $definition->addArgument(new Reference('web_link.manager'));
+            $definition->addArgument(new Reference('request_stack'));
        }

        foreach ($configs as $key => $config) {

diff --git a/src/Symfony/Component/WebLink/EventListener/AddLinkHeaderListener.php b/src/Symfony/Component/WebLink/EventListener/AddLinkHeaderListener.php
@@ -11,11 +11,11 @@

 namespace Symfony\Component\WebLink\EventListener;

+use Psr\Link\LinkProviderInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\WebLink\HttpHeaderSerializer;
-use Symfony\Component\WebLink\WebLinkManagerInterface;

 /**
 * Adds the Link HTTP header to the response.
@@ -26,12 +26,10 @@
 */
 class AddLinkHeaderListener implements EventSubscriberInterface
 {
-    private $manager;
    private $serializer;

-    public function __construct(WebLinkManagerInterface $manager)
+    public function __construct()
    {
-        $this->manager = $manager;
        $this->serializer = new HttpHeaderSerializer();
    }

@@ -41,12 +39,12 @@ public function onKernelResponse(FilterResponseEvent $event)
            return;
        }

-        if ($value = $this->serializer->serialize($this->manager->getLinkProvider())) {
-            $event->getResponse()->headers->set('Link', $value, false);
-
-            // Free memory
-            $this->manager->clear();
+        $linkProvider = $event->getRequest()->attributes->get('_links');
+        if (!$linkProvider instanceof LinkProviderInterface || !($links = $linkProvider->getLinks())) {
+            return;
        }
+
+        $event->getResponse()->headers->set('Link', $this->serializer->serialize($links), false);
    }

    /**

diff --git a/src/Symfony/Component/WebLink/HttpHeaderSerializer.php b/src/Symfony/Component/WebLink/HttpHeaderSerializer.php
@@ -11,7 +11,7 @@

 namespace Symfony\Component\WebLink;

-use Psr\Link\LinkProviderInterface;
+use Psr\Link\LinkInterface;

 /**
 * Serializes a list of Link instances to a HTTP Link header.
@@ -25,14 +25,14 @@ final class HttpHeaderSerializer
    /**
     * Builds the value of the "Link" HTTP header.
     *
-     * @param LinkProviderInterface $linkProvider
+     * @param LinkInterface[]|\Traversable $links
     *
     * @return string|null
     */
-    public function serialize(LinkProviderInterface $linkProvider)
+    public function serialize($links)
    {
        $elements = array();
-        foreach ($linkProvider->getLinks() as $link) {
+        foreach ($links as $link) {
            if ($link->isTemplated()) {
                continue;
            }

diff --git a/src/Symfony/Component/WebLink/Tests/EventListener/AddLinkHeaderListenerTest.php b/src/Symfony/Component/WebLink/Tests/EventListener/AddLinkHeaderListenerTest.php
@@ -14,8 +14,8 @@
 use Fig\Link\GenericLinkProvider;
 use Fig\Link\Link;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\WebLink\EventListener\AddLinkHeaderListener;
-use Symfony\Component\WebLink\WebLinkManager;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
@@ -28,14 +28,14 @@ class AddLinkHeaderListenerTest extends TestCase
 {
    public function testOnKernelResponse()
    {
-        $manager = new WebLinkManager(new GenericLinkProvider());
-        $manager->add(new Link('preload', '/foo'));
-
-        $subscriber = new AddLinkHeaderListener($manager);
+        $request = new Request(array(), array(), array('_links' => new GenericLinkProvider(array(new Link('preload', '/foo')))));
        $response = new Response('', 200, array('Link' => '<https://demo.api-platform.com/docs.jsonld>; rel="http://www.w3.org/ns/hydra/core#apiDocumentation"'));

+        $subscriber = new AddLinkHeaderListener();
+
        $event = $this->getMockBuilder(FilterResponseEvent::class)->disableOriginalConstructor()->getMock();
        $event->method('isMasterRequest')->willReturn(true);
+        $event->method('getRequest')->willReturn($request);
        $event->method('getResponse')->willReturn($response);

        $subscriber->onKernelResponse($event);
@@ -48,7 +48,6 @@ public function testOnKernelResponse()
        );

        $this->assertEquals($expected, $response->headers->get('Link', null, false));
-        $this->assertEmpty($manager->getLinkProvider()->getLinks());
    }

    public function testSubscribedEvents()

diff --git a/src/Symfony/Component/WebLink/Tests/HttpHeaderSerializerTest.php b/src/Symfony/Component/WebLink/Tests/HttpHeaderSerializerTest.php
@@ -30,19 +30,19 @@ protected function setUp()

    public function testSerialize()
    {
-        $linkProvider = new GenericLinkProvider(array(
+        $links = array(
            new Link('prerender', '/1'),
            (new Link('dns-prefetch', '/2'))->withAttribute('pr', 0.7),
            (new Link('preload', '/3'))->withAttribute('as', 'script')->withAttribute('nopush', false),
            (new Link('preload', '/4'))->withAttribute('as', 'image')->withAttribute('nopush', true),
            (new Link('alternate', '/5'))->withRel('next')->withAttribute('hreflang', array('fr', 'de'))->withAttribute('title', 'Hello'),
-        ));
+        );

-        $this->assertEquals('</1>; rel="prerender",</2>; rel="dns-prefetch"; pr="0.7",</3>; rel="preload"; as="script",</4>; rel="preload"; as="image"; nopush,</5>; rel="alternate next"; hreflang="fr"; hreflang="de"; title="Hello"', $this->serializer->serialize($linkProvider));
+        $this->assertEquals('</1>; rel="prerender",</2>; rel="dns-prefetch"; pr="0.7",</3>; rel="preload"; as="script",</4>; rel="preload"; as="image"; nopush,</5>; rel="alternate next"; hreflang="fr"; hreflang="de"; title="Hello"', $this->serializer->serialize($links));
    }

    public function testSerializeEmpty()
    {
-        $this->assertNull($this->serializer->serialize(new GenericLinkProvider()));
+        $this->assertNull($this->serializer->serialize(array()));
    }
 }
-Original file line number
+Diff line change
@@ -6,16 +6,7 @@
         <services>
-            <service id="web_link.manager" class="Symfony\Component\WebLink\WebLinkManager" public="false">
-                <argument type="service">
-                    <service class="Fig\Link\GenericLinkProvider" />
-                </argument>
-            </service>
-            <service id="Symfony\Component\WebLink\WebLinkManagerInterface" alias="web_link.manager" public="false" />
             <service id="web_link.add_link_header_listener" class="Symfony\Component\Link\WebLink\AddLinkHeaderListener" public="false">
             Copy link

  
      
    
  

  
      

  
    Contributor


      

  

  
    
      

      
            eko
  

      

      

      


        Apr 10, 2017


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      It looks like there is a typo in class name here, should be: Symfony\Component\WebLink\AddLinkHeaderListener
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
      
  
    
    
      
        
            
    All reactions
-                <argument type="service" id="web_link.manager" />
                 <tag name="kernel.event_subscriber" />
             </service>
-          Expand Down