Use PSR-13

symfony · dunglas · Apr 4, 2017 · Apr 4, 2017 · Apr 5, 2017 · Apr 6, 2017
commit 66c4ed6a57df0f171a654ca8386b593b367a3a28
diff --git a/composer.json b/composer.json
@@ -18,9 +18,11 @@
    "require": {
        "php": ">=5.5.9",
        "doctrine/common": "~2.4",
+        "fig/link-util": "^1.0",
        "twig/twig": "~1.32|~2.2",
        "psr/cache": "~1.0",
        "psr/container": "^1.0",
+        "psr/link": "^1.0",
        "psr/log": "~1.0",
        "psr/simple-cache": "^1.0",
        "symfony/polyfill-intl-icu": "~1.0",

diff --git a/src/Symfony/Bridge/Twig/Extension/WebLinkExtension.php b/src/Symfony/Bridge/Twig/Extension/WebLinkExtension.php
@@ -11,6 +11,7 @@

 namespace Symfony\Bridge\Twig\Extension;

+use Fig\Link\Link;
 use Symfony\Component\WebLink\WebLinkManagerInterface;

 /**
@@ -20,11 +21,11 @@
 */
 class WebLinkExtension extends \Twig_Extension
 {
-    private $linkManager;
+    private $manager;

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

    /**
@@ -53,7 +54,12 @@ public function getFunctions()
     */
    public function link($uri, $rel, array $attributes = array())
    {
-        $this->linkManager->add($uri, $rel, $attributes);
+        $link = new Link($rel, $uri);
+        foreach ($attributes as $key => $value) {
+            $link = $link->withAttribute($key, $value);
+        }
+
+        $this->manager->add($link);

        return $uri;
    }

diff --git a/src/Symfony/Bridge/Twig/Tests/Extension/WebLinkExtensionTest.php b/src/Symfony/Bridge/Twig/Tests/Extension/WebLinkExtensionTest.php
@@ -11,6 +11,7 @@

 namespace Symfony\Bridge\Twig\Tests\Extension;

+use Fig\Link\Link;
 use PHPUnit\Framework\TestCase;
 use Symfony\Bridge\Twig\Extension\WebLinkExtension;
 use Symfony\Component\WebLink\WebLinkManager;
@@ -20,58 +21,68 @@
 */
 class WebLinkExtensionTest extends TestCase
 {
+    /**
+     * @var WebLinkManager
+     */
+    private $manager;
+
+    /**
+     * @var WebLinkExtension
+     */
+    private $extension;
+
+    protected function setUp()
+    {
+        $this->manager = new WebLinkManager();
+        $this->extension = new WebLinkExtension($this->manager);
+    }
+
    public function testLink()
    {
-        $linkManager = new WebLinkManager();
-        $extension = new WebLinkExtension($linkManager);
+        $this->assertEquals('/foo.css', $this->extension->link('/foo.css', 'preload', array('as' => 'style', 'nopush' => true)));

-        $this->assertEquals('/foo.css', $extension->link('/foo.css', 'preload', array('as' => 'style', 'nopush' => true)));
-        $this->assertEquals('</foo.css>; rel=preload; as=style; nopush', $linkManager->buildHeaderValue());
+        $link = (new Link('preload', '/foo.css'))->withAttribute('as', 'style')->withAttribute('nopush', true);
+        $this->assertEquals(array($link), array_values($this->manager->getLinkProvider()->getLinks()));
    }

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

-        $this->assertEquals('/foo.css', $extension->preload('/foo.css', array('as' => 'style', 'crossorigin' => true)));
-        $this->assertEquals('</foo.css>; rel=preload; as=style; crossorigin', $linkManager->buildHeaderValue());
+        $link = (new Link('preload', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true);
+        $this->assertEquals(array($link), array_values($this->manager->getLinkProvider()->getLinks()));
    }

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

-        $this->assertEquals('/foo.css', $extension->dnsPrefetch('/foo.css', array('as' => 'style', 'crossorigin' => true)));
-        $this->assertEquals('</foo.css>; rel=dns-prefetch; as=style; crossorigin', $linkManager->buildHeaderValue());
+        $link = (new Link('dns-prefetch', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true);
+        $this->assertEquals(array($link), array_values($this->manager->getLinkProvider()->getLinks()));
    }

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

-        $this->assertEquals('/foo.css', $extension->preconnect('/foo.css', array('as' => 'style', 'crossorigin' => true)));
-        $this->assertEquals('</foo.css>; rel=preconnect; as=style; crossorigin', $linkManager->buildHeaderValue());
+        $link = (new Link('preconnect', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true);
+        $this->assertEquals(array($link), array_values($this->manager->getLinkProvider()->getLinks()));
    }

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

-        $this->assertEquals('/foo.css', $extension->prefetch('/foo.css', array('as' => 'style', 'crossorigin' => true)));
-        $this->assertEquals('</foo.css>; rel=prefetch; as=style; crossorigin', $linkManager->buildHeaderValue());
+        $link = (new Link('prefetch', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true);
+        $this->assertEquals(array($link), array_values($this->manager->getLinkProvider()->getLinks()));
    }

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

-        $this->assertEquals('/foo.css', $extension->prerender('/foo.css', array('as' => 'style', 'crossorigin' => true)));
-        $this->assertEquals('</foo.css>; rel=prerender; as=style; crossorigin', $linkManager->buildHeaderValue());
+        $link = (new Link('prerender', '/foo.css'))->withAttribute('as', 'style')->withAttribute('crossorigin', true);
+        $this->assertEquals(array($link), array_values($this->manager->getLinkProvider()->getLinks()));
    }

    public function testGetName()

diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/web_link.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/web_link.xml
@@ -9,7 +9,7 @@
        <service id="web_link.manager" class="Symfony\Component\WebLink\WebLinkManager" public="false" />
        <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\HeaderListener" 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" />

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

 namespace Symfony\Component\WebLink\EventListener;

+use Symfony\Component\WebLink\HttpHeaderSerializer;
 use Symfony\Component\WebLink\WebLinkManagerInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
@@ -25,11 +26,13 @@
 */
 class AddLinkHeaderListener implements EventSubscriberInterface
 {
-    private $linkManager;
+    private $manager;
+    private $serializer;

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

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

-        if ($value = $this->linkManager->buildHeaderValue()) {
+        if ($value = $this->serializer->serialize($this->manager->getLinkProvider())) {
            $event->getResponse()->headers->set('Link', $value, false);

            // Free memory
-            $this->linkManager->clear();
+            $this->manager->clear();
        }
    }


diff --git a/src/Symfony/Component/WebLink/HttpHeaderSerializer.php b/src/Symfony/Component/WebLink/HttpHeaderSerializer.php
@@ -0,0 +1,66 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\WebLink;
+
+use Psr\Link\LinkProviderInterface;
+
+/**
+ * Serializes a list of Link instances to a HTTP Link header.
+ *
+ * @see https://tools.ietf.org/html/rfc5988
+ *
+ * @author Kévin Dunglas <dunglas@gmail.com>
+ */
+final class HttpHeaderSerializer
+{
+    /**
+     * Builds the value of the "Link" HTTP header.
+     *
+     * @param LinkProviderInterface $linkProvider
+     *
+     * @return string|null
+     */
+    public function serialize(LinkProviderInterface $linkProvider)
+    {
+        $elements = array();
+        foreach ($linkProvider->getLinks() as $link) {
+            if ($link->isTemplated()) {
+                continue;
+            }
+
+            $attributesParts = array('', sprintf('rel="%s"', implode(' ', $link->getRels())));
+            foreach ($link->getAttributes() as $key => $value) {
+                if (is_array($value)) {
+                    foreach ($value as $v) {
+                        $attributesParts[] = sprintf('%s="%s"', $key, $v);
+                    }
+
+                    continue;
+                }
+
+                if (!is_bool($value)) {
+                    $attributesParts[] = sprintf('%s="%s"', $key, $value);
+
+                    continue;
+                }
+
+                if (true === $value) {
+                    $attributesParts[] = $key;
+                }
+            }
+
+            $elements[] = sprintf('<%s>%s', $link->getHref(), implode('; ', $attributesParts));
+        }
+
+        return $elements ? implode(',', $elements) : null;
+    }
+}
diff --git a/src/Symfony/Component/WebLink/Tests/EventListener/AddLinkHeaderListenerTest.php b/src/Symfony/Component/WebLink/Tests/EventListener/AddLinkHeaderListenerTest.php
@@ -11,6 +11,7 @@

 namespace Symfony\Component\WebLink\Tests\EventListener;

+use Fig\Link\Link;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\WebLink\EventListener\AddLinkHeaderListener;
 use Symfony\Component\WebLink\WebLinkManager;
@@ -27,7 +28,7 @@ class AddLinkHeaderListenerTest extends TestCase
    public function testOnKernelResponse()
    {
        $manager = new WebLinkManager();
-        $manager->add('/foo', 'preload');
+        $manager->add(new Link('preload', '/foo'));

        $subscriber = new AddLinkHeaderListener($manager);
        $response = new Response('', 200, array('Link' => '<https://demo.api-platform.com/docs.jsonld>; rel="http://www.w3.org/ns/hydra/core#apiDocumentation"'));
@@ -42,11 +43,11 @@ public function testOnKernelResponse()

        $expected = array(
            '<https://demo.api-platform.com/docs.jsonld>; rel="http://www.w3.org/ns/hydra/core#apiDocumentation"',
-            '</foo>; rel=preload',
+            '</foo>; rel="preload"',
        );

        $this->assertEquals($expected, $response->headers->get('Link', null, false));
-        $this->assertNull($manager->buildHeaderValue());
+        $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
@@ -0,0 +1,45 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\WebLink\Tests;
+
+use Fig\Link\GenericLinkProvider;
+use Fig\Link\Link;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\WebLink\HttpHeaderSerializer;
+
+class HttpHeaderSerializerTest extends TestCase
+{
+    /**
+     * @var HttpHeaderSerializer
+     */
+    private $serializer;
+
+    protected function setUp() {
+        $this->serializer = new HttpHeaderSerializer();
+    }
+
+    public function testSerialize() {
+        $linkProvider = new GenericLinkProvider(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));
+    }
+
+    public function testSerializeEmpty() {
+        $this->assertNull($this->serializer->serialize(new GenericLinkProvider()));
+    }
+}
diff --git a/src/Symfony/Component/WebLink/Tests/WebLinkManagerTest.php b/src/Symfony/Component/WebLink/Tests/WebLinkManagerTest.php
@@ -11,6 +11,7 @@

 namespace Symfony\Component\WebLink\Tests;

+use Fig\Link\Link;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\WebLink\WebLinkManager;
 use Symfony\Component\WebLink\WebLinkManagerInterface;
@@ -20,17 +21,32 @@
 */
 class WebLinkManagerTest extends TestCase
 {
-    public function testManageResources()
+    /**
+     * @var WebLinkManager
+     */
+    private $manager;
+
+    protected function setUp()
+    {
+        $this->manager = new WebLinkManager();
+    }
+
+    public function testAdd()
    {
-        $manager = new WebLinkManager();
-        $this->assertInstanceOf(WebLinkManagerInterface::class, $manager);
+        $this->assertInstanceOf(WebLinkManagerInterface::class, $this->manager);
+
+        $this->manager->add($link1 = new Link());
+        $this->manager->add($link2 = new Link());
+
+        $this->assertSame(array($link1, $link2), array_values($this->manager->getLinkProvider()->getLinks()));
+    }

-        $manager->add('/hello.html', 'prerender', array('pr' => 0.7));
+    public function testClear() {
+        $this->manager->add($link1 = new Link());
+        $this->manager->add($link2 = new Link());

-        $manager->add('/foo/bar.js', 'preload', array('as' => 'script', 'nopush' => false));
-        $manager->add('/foo/baz.css', 'preload');
-        $manager->add('/foo/bat.png', 'preload', array('as' => 'image', 'nopush' => true));
+        $this->manager->clear();

-        $this->assertEquals('</hello.html>; rel=prerender; pr=0.7,</foo/bar.js>; rel=preload; as=script,</foo/baz.css>; rel=preload,</foo/bat.png>; rel=preload; as=image; nopush', $manager->buildHeaderValue());
+        $this->assertEmpty($this->manager->getLinkProvider()->getLinks());
    }
 }
-Original file line number
+Diff line change
@@ -18,9 +18,11 @@
         "require": {
             "php": ">=5.5.9",
             "doctrine/common": "~2.4",
+            "fig/link-util": "^1.0",
             Copy link

  
      
    
  

  
      

  
  Member


      

  

  
    
      

      
            nicolas-grekas
  

      

      

      


        Apr 6, 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.
    

    
      
      


  


  
    
      this dep is in require-dev in all components, but in require here, looks suspicious
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

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


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



    
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
  Member


      

  Author


  

  
    
      

      
            dunglas
  

      

      

      


        Apr 7, 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.
    

    
      
      


  


  
    
      We need this package in the Twig extension and the FrameworkBundle  (even at runtime). But the those packages have only a soft dependency to WebLink. In the context of the full stack, I think it makes sense to add it to have something working out of the box.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

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


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



    
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
    Contributor


      

  

  
    
      

      
            teohhanhui
  

      

      

      


        Apr 7, 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's not required by any of the packages in replace, so it should indeed remain in require-dev.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

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


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



    
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
  Member


      

  Author


  

  
    
      

      
            dunglas
  

      

      

      


        Apr 7, 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 is required if you use WebLink && (Twig Bridge || Framework Bundle). We cannot express such conditions with Composer, but the full stack framework install WebLink + Twig Bridge + Framework Bundle so it needs to have this package as dependency.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

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


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    



    
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
  Member


      

  

  
    
      

      
            nicolas-grekas
  

      

      

      


        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.
    

    
      
      


  


  
    
      Still looks inconsistent to me. symfony/symfony should not behave differently than using standalone components...
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

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


  
  


          
      
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    

    





    
        
  
    
        
    
  


      
          
  
      
            Copy link

  
      
    
  

  
      

  
  Member


      

  Author


  

  
    
      

      
            dunglas
  

      

      

      


        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.
    

    
      
      


  


  
    
      fig/link-util added back as a dependency of the WebLink component. It fixes the inconsistency, makes the component easier to use for end users and... it's the only one implementation of PSR-13 for now.
    
  
  


    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

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


  
  


          
      
  
    
    
      
        
            
    All reactions
             "twig/twig": "~1.32|~2.2",
             "psr/cache": "~1.0",
             "psr/container": "^1.0",
+            "psr/link": "^1.0",
             "psr/log": "~1.0",
             "psr/simple-cache": "^1.0",
             "symfony/polyfill-intl-icu": "~1.0",
-          Expand Down