[Routing] Deprecate ServiceRouterLoader and ObjectRouteLoader in favor of ContainerLoader and ObjectLoader

UPGRADE-4.4.md

@@ -83,6 +83,7 @@ FrameworkBundle
    has been deprecated.
  * The `ControllerResolver` and `DelegatingLoader` classes have been marked as `final`.
  * The `controller_name_converter` and `resolve_controller_name_subscriber` services have been deprecated.
+ * Deprecated `routing.loader.service`, use `routing.loader.container` instead.
 
 HttpClient
 ----------
@@ -129,6 +130,12 @@ PropertyAccess
 
  * Deprecated passing `null` as 2nd argument of `PropertyAccessor::createCache()` method (`$defaultLifetime`), pass `0` instead.
 
+Routing
+-------
+
+ * Deprecated `ServiceRouterLoader` in favor of `ContainerLoader`.
+ * Deprecated `ObjectRouteLoader` in favor of `ObjectLoader`.
+
 Security
 --------
 

src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md

@@ -10,6 +10,7 @@ CHANGELOG
  * The `ControllerResolver` and `DelegatingLoader` classes have been marked as `final`
  * Added support for configuring chained cache pools
  * Deprecated booting the kernel before running `WebTestCase::createClient()`
+ * Deprecated `routing.loader.service`, use `routing.loader.container` instead.
 
 4.3.0
 -----

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

@@ -41,6 +41,11 @@
         </service>
 
         <service id="routing.loader.service" class="Symfony\Component\Routing\Loader\DependencyInjection\ServiceRouterLoader">
+            <argument type="service" id="service_container" />
+            <deprecated>The "%service_id%" service is deprecated since Symfony 4.4, use "routing.loader.container" instead.</deprecated>
+        </service>
+
+        <service id="routing.loader.container" class="Symfony\Component\Routing\Loader\ContainerLoader">
             <tag name="routing.loader" />
             <argument type="service" id="service_container" />
         </service>

src/Symfony/Bundle/FrameworkBundle/composer.json

@@ -27,7 +27,7 @@
         "symfony/polyfill-mbstring": "~1.0",
         "symfony/filesystem": "^3.4|^4.0|^5.0",
         "symfony/finder": "^3.4|^4.0|^5.0",
-        "symfony/routing": "^4.3|^5.0"
+        "symfony/routing": "^4.4|^5.0"
     },
     "require-dev": {
         "doctrine/cache": "~1.0",

src/Symfony/Component/Routing/CHANGELOG.md

@@ -1,6 +1,12 @@
 CHANGELOG
 =========
 
+4.4.0
+-----
+
+ * Deprecated `ServiceRouterLoader` in favor of `ContainerLoader`.
+ * Deprecated `ObjectRouteLoader` in favor of `ObjectLoader`.
+
 4.3.0
 -----
 

src/Symfony/Component/Routing/Loader/ContainerLoader.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\Routing\Loader;
+
+use Psr\Container\ContainerInterface;
+
+/**
+ * A route loader that executes a service from a PSR-11 container to load the routes.
+ *
+ * @author Ryan Weaver <ryan@knpuniversity.com>
+ */
+class ContainerLoader extends ObjectLoader
+{
+    private $container;
+
+    public function __construct(ContainerInterface $container)
+    {
+        $this->container = $container;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function supports($resource, $type = null)
+    {
+        return 'service' === $type;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function getObject(string $id)
+    {
+        return $this->container->get($id);
+    }
+}

src/Symfony/Component/Routing/Loader/DependencyInjection/ServiceRouterLoader.php

@@ -12,12 +12,17 @@
 namespace Symfony\Component\Routing\Loader\DependencyInjection;
 
 use Psr\Container\ContainerInterface;
+use Symfony\Component\Routing\Loader\ContainerLoader;
 use Symfony\Component\Routing\Loader\ObjectRouteLoader;
 
+@trigger_error(sprintf('The "%s" class is deprecated since Symfony 4.4, use "%s" instead.', ServiceRouterLoader::class, ContainerLoader::class), E_USER_DEPRECATED);
+
 /**
  * A route loader that executes a service to load the routes.
  *
  * @author Ryan Weaver <ryan@knpuniversity.com>
+ *
+ * @deprecated since Symfony 4.4, use Symfony\Component\Routing\Loader\ContainerLoader instead.
  */
 class ServiceRouterLoader extends ObjectRouteLoader
 {

src/Symfony/Component/Routing/Loader/ObjectLoader.php

@@ -0,0 +1,84 @@
+<?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\Routing\Loader;
+
+use Symfony\Component\Config\Loader\Loader;
+use Symfony\Component\Config\Resource\FileResource;
+use Symfony\Component\Routing\RouteCollection;
+
+/**
+ * A route loader that calls a method on an object to load the routes.
+ *
+ * @author Ryan Weaver <ryan@knpuniversity.com>
+ */
+abstract class ObjectLoader extends Loader
+{
+    /**
+     * Returns the object that the method will be called on to load routes.
+     *
+     * For example, if your application uses a service container,
+     * the $id may be a service id.
+     *
+     * @return object
+     */
+    abstract protected function getObject(string $id);
+
+    /**
+     * Calls the object method that will load the routes.
+     *
+     * @param string      $resource object_id::method
+     * @param string|null $type     The resource type
+     *
+     * @return RouteCollection
+     */
+    public function load($resource, $type = null)
+    {
+        if (!preg_match('/^[^\:]+(?:::(?:[^\:]+))?$/', $resource)) {
+            throw new \InvalidArgumentException(sprintf('Invalid resource "%s" passed to the %s route loader: use the format "object_id::method" or "object_id" if your object class has an "__invoke" method.', $resource, \is_string($type) ? '"'.$type.'"' : 'object'));
+        }
+
+        $parts = explode('::', $resource);
+        $method = $parts[1] ?? '__invoke';
+
+        $loaderObject = $this->getObject($parts[0]);
+
+        if (!\is_object($loaderObject)) {
+            throw new \LogicException(sprintf('%s:getObject() must return an object: %s returned', \get_class($this), \gettype($loaderObject)));
+        }
+
+        if (!\is_callable([$loaderObject, $method])) {
+            throw new \BadMethodCallException(sprintf('Method "%s" not found on "%s" when importing routing resource "%s"', $method, \get_class($loaderObject), $resource));
+        }
+
+        $routeCollection = $loaderObject->$method($this);
+
+        if (!$routeCollection instanceof RouteCollection) {
+            $type = \is_object($routeCollection) ? \get_class($routeCollection) : \gettype($routeCollection);
+
+            throw new \LogicException(sprintf('The %s::%s method must return a RouteCollection: %s returned', \get_class($loaderObject), $method, $type));
+        }
+
+        // make the object file tracked so that if it changes, the cache rebuilds
+        $this->addClassResource(new \ReflectionClass($loaderObject), $routeCollection);
+
+        return $routeCollection;
+    }
+
+    private function addClassResource(\ReflectionClass $class, RouteCollection $collection)
+    {
+        do {
+            if (is_file($class->getFileName())) {
+                $collection->addResource(new FileResource($class->getFileName()));
+            }
+        } while ($class = $class->getParentClass());
+    }
+}

src/Symfony/Component/Routing/Loader/ObjectRouteLoader.php

@@ -11,16 +11,18 @@
 
 namespace Symfony\Component\Routing\Loader;
 
-use Symfony\Component\Config\Loader\Loader;
-use Symfony\Component\Config\Resource\FileResource;
 use Symfony\Component\Routing\RouteCollection;
 
+@trigger_error(sprintf('The "%s" class is deprecated since Symfony 4.4, use "%s" instead.', ObjectRouteLoader::class, ObjectLoader::class), E_USER_DEPRECATED);
+
 /**
  * A route loader that calls a method on an object to load the routes.
  *
  * @author Ryan Weaver <ryan@knpuniversity.com>
+ *
+ * @deprecated since Symfony 4.4, use ObjectLoader instead.
  */
-abstract class ObjectRouteLoader extends Loader
+abstract class ObjectRouteLoader extends ObjectLoader
 {
     /**
      * Returns the object that the method will be called on to load routes.
@@ -53,32 +55,7 @@ abstract class ObjectRouteLoader extends Loader
             @trigger_error(sprintf('Referencing service route loaders with a single colon is deprecated since Symfony 4.1. Use %s instead.', $resource), E_USER_DEPRECATED);
         }
 
-        $parts = explode('::', $resource);
+        return parent::load($resource, $type);
-        $serviceString = $parts[0];
-        $method = $parts[1] ?? '__invoke';
-
-        $loaderObject = $this->getServiceObject($serviceString);
-
-        if (!\is_object($loaderObject)) {
-            throw new \LogicException(sprintf('%s:getServiceObject() must return an object: %s returned', \get_class($this), \gettype($loaderObject)));
-        }
-
-        if (!\is_callable([$loaderObject, $method])) {
-            throw new \BadMethodCallException(sprintf('Method "%s" not found on "%s" when importing routing resource "%s"', $method, \get_class($loaderObject), $resource));
-        }
-
-        $routeCollection = $loaderObject->$method($this);
-
-        if (!$routeCollection instanceof RouteCollection) {
-            $type = \is_object($routeCollection) ? \get_class($routeCollection) : \gettype($routeCollection);
-
-            throw new \LogicException(sprintf('The %s::%s method must return a RouteCollection: %s returned', \get_class($loaderObject), $method, $type));
-        }
-
-        // make the service file tracked so that if it changes, the cache rebuilds
-        $this->addClassResource(new \ReflectionClass($loaderObject), $routeCollection);
-
-        return $routeCollection;
     }
 
     /**
@@ -89,12 +66,11 @@ abstract class ObjectRouteLoader extends Loader
         return 'service' === $type;
     }
 
-    private function addClassResource(\ReflectionClass $class, RouteCollection $collection)
+    /**
+     * {@inheritdoc}
+     */
+    protected function getObject(string $id)
     {
-        do {
+        return $this->getServiceObject($id);
-            if (is_file($class->getFileName())) {
-                $collection->addResource(new FileResource($class->getFileName()));
-            }
-        } while ($class = $class->getParentClass());
     }
 }

src/Symfony/Component/Routing/Tests/Fixtures/TestObjectRouteLoader.php

@@ -0,0 +1,24 @@
+<?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\Routing\Tests\Fixtures;
+
+use Symfony\Component\Routing\Loader\ObjectRouteLoader;
+
+class TestObjectRouteLoader extends ObjectRouteLoader
+{
+    public $loaderMap = [];
+
+    protected function getServiceObject($id)
+    {
+        return $this->loaderMap[$id] ?? null;
+    }
+}

src/Symfony/Component/Routing/Tests/Loader/ContainerLoaderTest.php

@@ -0,0 +1,36 @@
+<?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\Routing\Tests\Loader;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\DependencyInjection\Container;
+use Symfony\Component\Routing\Loader\ContainerLoader;
+
+class ContainerLoaderTest extends TestCase
+{
+    /**
+     * @dataProvider supportsProvider
+     */
+    public function testSupports(bool $expected, string $type = null)
+    {
+        $this->assertSame($expected, (new ContainerLoader(new Container()))->supports('foo', $type));
+    }
+
+    public function supportsProvider()
+    {
+        return [
+            [true, 'service'],
+            [false, 'bar'],
+            [false, null],
+        ];
+    }
+}

src/Symfony/Component/Routing/Tests/Loader/DependencyInjection/ServiceRouterLoaderTest.php

@@ -0,0 +1,29 @@
+<?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\Routing\Tests\Loader;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\DependencyInjection\Container;
+use Symfony\Component\Routing\Loader\DependencyInjection\ServiceRouterLoader;
+
+class ServiceRouterLoaderTest extends TestCase
+{
+    /**
+     * @group legacy
+     * @expectedDeprecation The "Symfony\Component\Routing\Loader\DependencyInjection\ServiceRouterLoader" class is deprecated since Symfony 4.4, use "Symfony\Component\Routing\Loader\ContainerLoader" instead.
+     * @expectedDeprecation The "Symfony\Component\Routing\Loader\ObjectRouteLoader" class is deprecated since Symfony 4.4, use "Symfony\Component\Routing\Loader\ObjectLoader" instead.
+     */
+    public function testDeprecationWarning()
+    {
+        new ServiceRouterLoader(new Container());
+    }
+}

src/Symfony/Component/Routing/Tests/Loader/ObjectLoaderTest.php

@@ -0,0 +1,131 @@
+<?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\Routing\Tests\Loader;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Routing\Loader\ObjectLoader;
+use Symfony\Component\Routing\Route;
+use Symfony\Component\Routing\RouteCollection;
+
+class ObjectLoaderTest extends TestCase
+{
+    public function testLoadCallsServiceAndReturnsCollection()
+    {
+        $loader = new TestObjectLoader();
+
+        // create a basic collection that will be returned
+        $collection = new RouteCollection();
+        $collection->add('foo', new Route('/foo'));
+
+        $loader->loaderMap = [
+            'my_route_provider_service' => new TestObjectLoaderRouteService($collection),
+        ];
+
+        $actualRoutes = $loader->load(
+            'my_route_provider_service::loadRoutes',
+            'service'
+        );
+
+        $this->assertSame($collection, $actualRoutes);
+        // the service file should be listed as a resource
+        $this->assertNotEmpty($actualRoutes->getResources());
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     * @dataProvider getBadResourceStrings
+     */
+    public function testExceptionWithoutSyntax(string $resourceString): void
+    {
+        $loader = new TestObjectLoader();
+        $loader->load($resourceString);
+    }
+
+    public function getBadResourceStrings()
+    {
+        return [
+            ['Foo:Bar:baz'],
+            ['Foo::Bar::baz'],
+            ['Foo:'],
+            ['Foo::'],
+            [':Foo'],
+            ['::Foo'],
+        ];
+    }
+
+    /**
+     * @expectedException \LogicException
+     */
+    public function testExceptionOnNoObjectReturned()
+    {
+        $loader = new TestObjectLoader();
+        $loader->loaderMap = ['my_service' => 'NOT_AN_OBJECT'];
+        $loader->load('my_service::method');
+    }
+
+    /**
+     * @expectedException \BadMethodCallException
+     */
+    public function testExceptionOnBadMethod()
+    {
+        $loader = new TestObjectLoader();
+        $loader->loaderMap = ['my_service' => new \stdClass()];
+        $loader->load('my_service::method');
+    }
+
+    /**
+     * @expectedException \LogicException
+     */
+    public function testExceptionOnMethodNotReturningCollection()
+    {
+        $service = $this->getMockBuilder('stdClass')
+            ->setMethods(['loadRoutes'])
+            ->getMock();
+        $service->expects($this->once())
+            ->method('loadRoutes')
+            ->willReturn('NOT_A_COLLECTION');
+
+        $loader = new TestObjectLoader();
+        $loader->loaderMap = ['my_service' => $service];
+        $loader->load('my_service::loadRoutes');
+    }
+}
+
+class TestObjectLoader extends ObjectLoader
+{
+    public $loaderMap = [];
+
+    public function supports($resource, $type = null)
+    {
+        return 'service';
+    }
+
+    protected function getObject(string $id)
+    {
+        return $this->loaderMap[$id] ?? null;
+    }
+}
+
+class TestObjectLoaderRouteService
+{
+    private $collection;
+
+    public function __construct($collection)
+    {
+        $this->collection = $collection;
+    }
+
+    public function loadRoutes()
+    {
+        return $this->collection;
+    }
+}

src/Symfony/Component/Routing/Tests/Loader/ObjectRouteLoaderTest.php

@@ -12,26 +12,28 @@
 namespace Symfony\Component\Routing\Tests\Loader;
 
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\Routing\Loader\ObjectRouteLoader;
 use Symfony\Component\Routing\Route;
 use Symfony\Component\Routing\RouteCollection;
+use Symfony\Component\Routing\Tests\Fixtures\TestObjectRouteLoader;
 
+/**
+ * @group legacy
+ */
 class ObjectRouteLoaderTest extends TestCase
 {
     /**
-     * @group legacy
      * @expectedDeprecation Referencing service route loaders with a single colon is deprecated since Symfony 4.1. Use my_route_provider_service::loadRoutes instead.
      */
     public function testLoadCallsServiceAndReturnsCollectionWithLegacyNotation()
     {
-        $loader = new ObjectRouteLoaderForTest();
+        $loader = new TestObjectRouteLoader();
 
         // create a basic collection that will be returned
         $collection = new RouteCollection();
         $collection->add('foo', new Route('/foo'));
 
         $loader->loaderMap = [
-            'my_route_provider_service' => new RouteService($collection),
+            'my_route_provider_service' => new TestObjectRouteLoaderRouteService($collection),
         ];
 
         $actualRoutes = $loader->load(
@@ -46,14 +48,14 @@ class ObjectRouteLoaderTest extends TestCase
 
     public function testLoadCallsServiceAndReturnsCollection()
     {
-        $loader = new ObjectRouteLoaderForTest();
+        $loader = new TestObjectRouteLoader();
 
         // create a basic collection that will be returned
         $collection = new RouteCollection();
         $collection->add('foo', new Route('/foo'));
 
         $loader->loaderMap = [
-            'my_route_provider_service' => new RouteService($collection),
+            'my_route_provider_service' => new TestObjectRouteLoaderRouteService($collection),
         ];
 
         $actualRoutes = $loader->load(
@@ -72,7 +74,7 @@ class ObjectRouteLoaderTest extends TestCase
      */
     public function testExceptionWithoutSyntax(string $resourceString): void
     {
-        $loader = new ObjectRouteLoaderForTest();
+        $loader = new TestObjectRouteLoader();
         $loader->load($resourceString);
     }
 
@@ -93,7 +95,7 @@ class ObjectRouteLoaderTest extends TestCase
      */
     public function testExceptionOnNoObjectReturned()
     {
-        $loader = new ObjectRouteLoaderForTest();
+        $loader = new TestObjectRouteLoader();
         $loader->loaderMap = ['my_service' => 'NOT_AN_OBJECT'];
         $loader->load('my_service::method');
     }
@@ -103,7 +105,7 @@ class ObjectRouteLoaderTest extends TestCase
      */
     public function testExceptionOnBadMethod()
     {
-        $loader = new ObjectRouteLoaderForTest();
+        $loader = new TestObjectRouteLoader();
         $loader->loaderMap = ['my_service' => new \stdClass()];
         $loader->load('my_service::method');
     }
@@ -120,23 +122,13 @@ class ObjectRouteLoaderTest extends TestCase
             ->method('loadRoutes')
             ->willReturn('NOT_A_COLLECTION');
 
-        $loader = new ObjectRouteLoaderForTest();
+        $loader = new TestObjectRouteLoader();
         $loader->loaderMap = ['my_service' => $service];
         $loader->load('my_service::loadRoutes');
     }
 }
 
-class ObjectRouteLoaderForTest extends ObjectRouteLoader
+class TestObjectRouteLoaderRouteService
-{
-    public $loaderMap = [];
-
-    protected function getServiceObject($id)
-    {
-        return isset($this->loaderMap[$id]) ? $this->loaderMap[$id] : null;
-    }
-}
-
-class RouteService
 {
     private $collection;