-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Asset] Add support for preloading with links and HTTP/2 push #21478
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
64bd530
f920ca4
1de19cc
c5ca4bc
639d7d1
9c11a0f
d39b4f1
581c700
693107c
2d8d750
0b7fe14
dffb7b6
18412e5
78d7b6a
05134b6
a7f77b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
<?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\Bridge\Twig\Tests\Extension; | ||
|
||
use Symfony\Bridge\Twig\Extension\AssetExtension; | ||
use Symfony\Component\Asset\Packages; | ||
use Symfony\Component\Asset\Preload\PreloadManager; | ||
|
||
/** | ||
* @author Kévin Dunglas <dunglas@gmail.com> | ||
*/ | ||
class AssetExtensionTest extends \PHPUnit_Framework_TestCase | ||
{ | ||
public function testGetAndPreloadAssetUrl() | ||
{ | ||
if (!class_exists(PreloadManager::class)) { | ||
$this->markTestSkipped('Requires Asset 3.3+.'); | ||
} | ||
|
||
$preloadManager = new PreloadManager(); | ||
$extension = new AssetExtension(new Packages(), $preloadManager); | ||
|
||
$this->assertEquals('/foo.css', $extension->preload('/foo.css', 'style', true)); | ||
$this->assertEquals('</foo.css>; rel=preload; as=style; nopush', $preloadManager->buildLinkValue()); | ||
} | ||
|
||
/** | ||
* @expectedException \RuntimeException | ||
*/ | ||
public function testNoConfiguredPreloadManager() | ||
{ | ||
$extension = new AssetExtension(new Packages()); | ||
$extension->preload('/foo.css'); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
<?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\Asset\EventListener; | ||
|
||
use Symfony\Component\Asset\Preload\PreloadManager; | ||
use Symfony\Component\Asset\Preload\PreloadManagerInterface; | ||
use Symfony\Component\EventDispatcher\EventSubscriberInterface; | ||
use Symfony\Component\HttpKernel\Event\FilterResponseEvent; | ||
use Symfony\Component\HttpKernel\KernelEvents; | ||
|
||
/** | ||
* Adds the preload Link HTTP header to the response. | ||
* | ||
* @author Kévin Dunglas <dunglas@gmail.com> | ||
*/ | ||
class PreloadListener implements EventSubscriberInterface | ||
{ | ||
private $preloadManager; | ||
|
||
public function __construct(PreloadManagerInterface $preloadManager) | ||
{ | ||
$this->preloadManager = $preloadManager; | ||
} | ||
|
||
public function onKernelResponse(FilterResponseEvent $event) | ||
{ | ||
if (!$event->isMasterRequest()) { | ||
return; | ||
} | ||
|
||
if ($value = $this->preloadManager->buildLinkValue()) { | ||
$event->getResponse()->headers->set('Link', $value, false); | ||
|
||
// Free memory | ||
$this->preloadManager->clear(); | ||
} | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public static function getSubscribedEvents() | ||
{ | ||
return array(KernelEvents::RESPONSE => 'onKernelResponse'); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
<?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\Asset\Preload; | ||
|
||
/** | ||
* Manages preload HTTP headers. | ||
* | ||
* @author Kévin Dunglas <dunglas@gmail.com> | ||
*/ | ||
class PreloadManager implements PreloadManagerInterface | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would forbid decoration for no reason imho - see comments on the interface: building a profiler panel could require decorating this class There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I meant decoration by inheritance of course. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There are 2 benefits:
I don't see connection between contract enforcing and "inheritance should not be prevented". I'd say the opposite. See similar opinion, for example: http://ocramius.github.io/blog/when-to-declare-classes-final/ /cc @Ocramius |
||
{ | ||
private $resources = array(); | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function addResource($uri, $as = '', $nopush = false) | ||
{ | ||
$this->resources[$uri] = array('as' => $as, 'nopush' => $nopush); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function clear() | ||
{ | ||
$this->resources = array(); | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function buildLinkValue() | ||
{ | ||
if (!$this->resources) { | ||
return; | ||
} | ||
|
||
$parts = array(); | ||
foreach ($this->resources as $uri => $options) { | ||
$as = '' === $options['as'] ? '' : sprintf('; as=%s', $options['as']); | ||
$nopush = $options['nopush'] ? '; nopush' : ''; | ||
|
||
$parts[] = sprintf('<%s>; rel=preload%s%s', $uri, $as, $nopush); | ||
} | ||
|
||
return implode(',', $parts); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
<?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\Asset\Preload; | ||
|
||
/** | ||
* Manages resources to preload according to the W3C "Preload" specification. | ||
* | ||
* @see https://www.w3.org/TR/preload/ | ||
* | ||
* @author Kévin Dunglas <dunglas@gmail.com> | ||
*/ | ||
interface PreloadManagerInterface | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need this interface for? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This extension point allows to replace the manager by a smarter one (for instance if you want to create one giving access to the list of resources added or to change them programmatically). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it could also allow something to add a profiler panel reporting the preloaded resources for instance, by using a decorator implementation |
||
{ | ||
/** | ||
* Adds an element to the list of resources to preload. | ||
* | ||
* @param string $uri The resource URI | ||
* @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 | ||
*/ | ||
public function addResource($uri, $as = '', $nopush = false); | ||
|
||
/** | ||
* Clears the list of resources. | ||
*/ | ||
public function clear(); | ||
|
||
/** | ||
* Builds the value of the preload Link HTTP header. | ||
* | ||
* @return string|null | ||
*/ | ||
public function buildLinkValue(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
<?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\Asset\Tests\EventListener; | ||
|
||
use Symfony\Component\Asset\EventListener\PreloadListener; | ||
use Symfony\Component\Asset\Preload\PreloadManager; | ||
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> | ||
*/ | ||
class PreloadListenerTest extends \PHPUnit_Framework_TestCase | ||
{ | ||
public function testOnKernelResponse() | ||
{ | ||
$manager = new PreloadManager(); | ||
$manager->addResource('/foo'); | ||
|
||
$subscriber = new PreloadListener($manager); | ||
$response = new Response('', 200, array('Link' => '<https://demo.api-platform.com/docs.jsonld>; rel="http://www.w3.org/ns/hydra/core#apiDocumentation"')); | ||
|
||
$event = $this->getMockBuilder(FilterResponseEvent::class)->disableOriginalConstructor()->getMock(); | ||
$event->method('isMasterRequest')->willReturn(true); | ||
$event->method('getResponse')->willReturn($response); | ||
|
||
$subscriber->onKernelResponse($event); | ||
|
||
$this->assertInstanceOf(EventSubscriberInterface::class, $subscriber); | ||
|
||
$expected = array( | ||
'<https://demo.api-platform.com/docs.jsonld>; rel="http://www.w3.org/ns/hydra/core#apiDocumentation"', | ||
'</foo>; rel=preload', | ||
); | ||
|
||
$this->assertEquals($expected, $response->headers->get('Link', null, false)); | ||
$this->assertNull($manager->buildLinkValue()); | ||
} | ||
|
||
public function testSubscribedEvents() | ||
{ | ||
$this->assertEquals(array(KernelEvents::RESPONSE => 'onKernelResponse'), PreloadListener::getSubscribedEvents()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
<?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\Asset\Preload; | ||
|
||
/** | ||
* @author Kévin Dunglas <dunglas@gmail.com> | ||
*/ | ||
class PreloadManagerTest extends \PHPUnit_Framework_TestCase | ||
{ | ||
public function testManageResources() | ||
{ | ||
$manager = new PreloadManager(); | ||
$this->assertInstanceOf(PreloadManagerInterface::class, $manager); | ||
|
||
$manager->addResource('/foo/bar.js', 'script', false); | ||
$manager->addResource('/foo/baz.css'); | ||
$manager->addResource('/foo/bat.png', 'image', true); | ||
|
||
$this->assertEquals('</foo/bar.js>; rel=preload; as=script,</foo/baz.css>; rel=preload,</foo/bat.png>; rel=preload; as=image; nopush', $manager->buildLinkValue()); | ||
} | ||
} |
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.
The default
PreloadManagerInterface
implementation is simple and does not have dependencies, so I suppose it does not make sense to have a proper runtime.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.
Sorry I don't get what you mean.
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.
Maybe we're just missing some doc about twig runtimes for extensions? :)