Skip to content

[DI][Router][DX] Invalidate routing cache when container parameters changed #21767

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

Merged
merged 1 commit into from
Mar 5, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@
<argument type="collection"></argument>
</service>

<service class="Symfony\Component\DependencyInjection\Config\ContainerParametersResourceChecker" public="false">
<argument type="service" id="service_container" />
<tag name="config_cache.resource_checker" priority="-980" />
</service>

<service class="Symfony\Component\Config\Resource\SelfCheckingResourceChecker" public="false">
<tag name="config_cache.resource_checker" priority="-990" />
</service>
Expand Down
5 changes: 5 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Routing/Router.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Bundle\FrameworkBundle\Routing;

use Symfony\Component\DependencyInjection\Config\ContainerParametersResource;
use Symfony\Component\Routing\Router as BaseRouter;
use Symfony\Component\Routing\RequestContext;
use Symfony\Component\DependencyInjection\ContainerInterface;
Expand All @@ -27,6 +28,7 @@
class Router extends BaseRouter implements WarmableInterface
{
private $container;
private $collectedParameters = array();

/**
* Constructor.
Expand All @@ -53,6 +55,7 @@ public function getRouteCollection()
if (null === $this->collection) {
$this->collection = $this->container->get('routing.loader')->load($this->resource, $this->options['resource_type']);
$this->resolveParameters($this->collection);
$this->collection->addResource(new ContainerParametersResource($this->collectedParameters));
}

return $this->collection;
Expand Down Expand Up @@ -153,6 +156,8 @@ private function resolve($value)
$resolved = $container->getParameter($match[1]);

if (is_string($resolved) || is_numeric($resolved)) {
$this->collectedParameters[$match[1]] = $resolved;

return (string) $resolved;
}

Expand Down
15 changes: 15 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RouterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use PHPUnit\Framework\TestCase;
use Symfony\Bundle\FrameworkBundle\Routing\Router;
use Symfony\Component\DependencyInjection\Config\ContainerParametersResource;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;

Expand Down Expand Up @@ -217,6 +218,20 @@ public function testDefaultValuesAsNonStrings($value)
$this->assertSame($value, $route->getDefault('foo'));
}

public function testGetRouteCollectionAddsContainerParametersResource()
{
$routeCollection = $this->getMockBuilder(RouteCollection::class)->getMock();
$routeCollection->method('getIterator')->willReturn(new \ArrayIterator(array(new Route('/%locale%'))));
$routeCollection->expects($this->once())->method('addResource')->with(new ContainerParametersResource(array('locale' => 'en')));

$sc = $this->getServiceContainer($routeCollection);
$sc->setParameter('locale', 'en');

$router = new Router($sc, 'foo');

$router->getRouteCollection();
}

public function getNonStringValues()
{
return array(array(null), array(false), array(true), array(new \stdClass()), array(array('foo', 'bar')), array(array(array())));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
<?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\DependencyInjection\Config;

use Symfony\Component\Config\Resource\ResourceInterface;

/**
* Tracks container parameters.
*
* @author Maxime Steinhausser <maxime.steinhausser@gmail.com>
*/
class ContainerParametersResource implements ResourceInterface, \Serializable
{
private $parameters;

/**
* @param array $parameters The container parameters to track
*/
public function __construct(array $parameters)
{
$this->parameters = $parameters;
}

/**
* {@inheritdoc}
*/
public function __toString()
{
return 'container_parameters_'.md5(serialize($this->parameters));
}

/**
* {@inheritdoc}
*/
public function serialize()
{
return serialize($this->parameters);
}

/**
* {@inheritdoc}
*/
public function unserialize($serialized)
{
$this->parameters = unserialize($serialized);
}

/**
* @return array Tracked parameters
*/
public function getParameters()
{
return $this->parameters;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?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\DependencyInjection\Config;

use Symfony\Component\Config\Resource\ResourceInterface;
use Symfony\Component\Config\ResourceCheckerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;

/**
* @author Maxime Steinhausser <maxime.steinhausser@gmail.com>
*/
class ContainerParametersResourceChecker implements ResourceCheckerInterface
{
/** @var ContainerInterface */
private $container;

public function __construct(ContainerInterface $container)
{
$this->container = $container;
}

/**
* {@inheritdoc}
*/
public function supports(ResourceInterface $metadata)
{
return $metadata instanceof ContainerParametersResource;
}

/**
* {@inheritdoc}
*/
public function isFresh(ResourceInterface $resource, $timestamp)
{
foreach ($resource->getParameters() as $key => $value) {
if (!$this->container->hasParameter($key) || $this->container->getParameter($key) !== $value) {
return false;
}
}

return true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
<?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\DependencyInjection\Tests\Config;

use PHPUnit\Framework\TestCase;
use Symfony\Component\Config\ResourceCheckerInterface;
use Symfony\Component\DependencyInjection\Config\ContainerParametersResource;
use Symfony\Component\DependencyInjection\Config\ContainerParametersResourceChecker;
use Symfony\Component\DependencyInjection\ContainerInterface;

class ContainerParametersResourceCheckerTest extends TestCase
{
/** @var ContainerParametersResource */
private $resource;

/** @var ResourceCheckerInterface */
private $resourceChecker;

/** @var ContainerInterface */
private $container;

protected function setUp()
{
$this->resource = new ContainerParametersResource(array('locales' => array('fr', 'en'), 'default_locale' => 'fr'));
$this->container = $this->getMockBuilder(ContainerInterface::class)->getMock();
$this->resourceChecker = new ContainerParametersResourceChecker($this->container);
}

public function testSupports()
{
$this->assertTrue($this->resourceChecker->supports($this->resource));
}

/**
* @dataProvider isFreshProvider
*/
public function testIsFresh(callable $mockContainer, $expected)
{
$mockContainer($this->container);

$this->assertSame($expected, $this->resourceChecker->isFresh($this->resource, time()));
}

public function isFreshProvider()
{
yield 'not fresh on missing parameter' => array(function (\PHPUnit_Framework_MockObject_MockObject $container) {
$container->method('hasParameter')->with('locales')->willReturn(false);
}, false);

yield 'not fresh on different value' => array(function (\PHPUnit_Framework_MockObject_MockObject $container) {
$container->method('getParameter')->with('locales')->willReturn(array('nl', 'es'));
}, false);

yield 'fresh on every identical parameters' => array(function (\PHPUnit_Framework_MockObject_MockObject $container) {
$container->expects($this->exactly(2))->method('hasParameter')->willReturn(true);
$container->expects($this->exactly(2))->method('getParameter')
->withConsecutive(
array($this->equalTo('locales')),
array($this->equalTo('default_locale'))
)
->will($this->returnValueMap(array(
array('locales', array('fr', 'en')),
array('default_locale', 'fr'),
)))
;
}, true);
}
}
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\DependencyInjection\Tests\Config;

use PHPUnit\Framework\TestCase;
use Symfony\Component\DependencyInjection\Config\ContainerParametersResource;

class ContainerParametersResourceTest extends TestCase
{
/** @var ContainerParametersResource */
private $resource;

protected function setUp()
{
$this->resource = new ContainerParametersResource(array('locales' => array('fr', 'en'), 'default_locale' => 'fr'));
}

public function testToString()
{
$this->assertSame('container_parameters_9893d3133814ab03cac3490f36dece77', (string) $this->resource);
}

public function testSerializeUnserialize()
{
$unserialized = unserialize(serialize($this->resource));

$this->assertEquals($this->resource, $unserialized);
}

public function testGetParameters()
{
$this->assertSame(array('locales' => array('fr', 'en'), 'default_locale' => 'fr'), $this->resource->getParameters());
}
}