feature #21767 [DI][Router][DX] Invalidate routing cache when container parameters changed (ogizanagi)

This PR was merged into the 3.3-dev branch.

Discussion
----------

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

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21426
| License       | MIT
| Doc PR        | N/A

Supersedes #21443 but only for master.

Indeed, this implementation uses a new feature: a `ContainerParametersResource` which compares cached containers parameters (collected at some point, here by the `Router`) with current ones in the container.

On the contrary of the previous PR targeting 2.7, this will only invalidate routing cache when parameters actually used in the routes changed and will avoid always rebuilding the routing cache when the container is rebuilt, just to catch the edge case of someone modifying a parameter.

Commits
-------

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

src/Symfony/Bundle/FrameworkBundle/Resources/config/services.xml

@@ -60,6 +60,11 @@
             <argument /> <!-- resource checkers -->
         </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>

src/Symfony/Bundle/FrameworkBundle/Routing/Router.php

@@ -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;
@@ -27,6 +28,7 @@ use Symfony\Component\DependencyInjection\Exception\RuntimeException;
 class Router extends BaseRouter implements WarmableInterface
 {
     private $container;
+    private $collectedParameters = array();
 
     /**
      * Constructor.
@@ -53,6 +55,7 @@ class Router extends BaseRouter implements WarmableInterface
         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;
@@ -153,6 +156,8 @@ class Router extends BaseRouter implements WarmableInterface
             $resolved = $container->getParameter($match[1]);
 
             if (is_string($resolved) || is_numeric($resolved)) {
+                $this->collectedParameters[$match[1]] = $resolved;
+
                 return (string) $resolved;
             }
 

src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RouterTest.php

@@ -13,6 +13,7 @@ namespace Symfony\Bundle\FrameworkBundle\Tests\Routing;
 
 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;
 
@@ -217,6 +218,20 @@ class RouterTest extends TestCase
         $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())));

src/Symfony/Component/DependencyInjection/Config/ContainerParametersResource.php

@@ -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;
+    }
+}

src/Symfony/Component/DependencyInjection/Config/ContainerParametersResourceChecker.php

@@ -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;
+    }
+}

src/Symfony/Component/DependencyInjection/Tests/Config/ContainerParametersResourceCheckerTest.php

@@ -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);
+    }
+}

src/Symfony/Component/DependencyInjection/Tests/Config/ContainerParametersResourceTest.php

@@ -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());
+    }
+}