Deprecate constructing AdminPoolLoader with a third argument

The container was added to the router cache in symfony/framework-bundle in 4.4.6. See symfony/symfony#36143
franmomu · Mar 29, 2021 · 4e8bae3 · 4e8bae3
1 parent dfce4c4
commit 4e8bae3
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 15 deletions.
diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md
@@ -4,6 +4,10 @@ UPGRADE 3.x
 UPGRADE FROM 3.xx to 3.xx
 =========================
 
+### `Sonata\AdminBundle\Route\AdminPoolLoader`
+
+Deprecated constructing it passing a third argument.
+
 ### `Sonata\AdminBundle\Filter\FilterFactory`
 
 Deprecated passing a value which is not registered as a service as argument 2 for `create()` method.

diff --git a/composer.json b/composer.json
@@ -46,7 +46,7 @@
         "symfony/event-dispatcher-contracts": "^1.1 || ^2.0",
         "symfony/expression-language": "^4.4 || ^5.1",
         "symfony/form": "^4.4.12",
-        "symfony/framework-bundle": "^4.4",
+        "symfony/framework-bundle": "^4.4.",
         "symfony/http-foundation": "^4.4 || ^5.1",
         "symfony/http-kernel": "^4.4",
         "symfony/options-resolver": "^4.4 || ^5.1",

diff --git a/src/DependencyInjection/Compiler/AddDependencyCallsCompilerPass.php b/src/DependencyInjection/Compiler/AddDependencyCallsCompilerPass.php
@@ -238,9 +238,6 @@ static function (array $a, array $b): int {
         $pool->replaceArgument(1, $admins);
         $pool->replaceArgument(2, $groups);
         $pool->replaceArgument(3, $classes);
-
-        $routeLoader = $container->getDefinition('sonata.admin.route_loader');
-        $routeLoader->replaceArgument(1, $admins);
     }
 
     /**

diff --git a/src/Resources/config/core.php b/src/Resources/config/core.php
@@ -78,11 +78,13 @@
             ->tag('routing.loader')
             ->args([
                 new ReferenceConfigurator('sonata.admin.pool'),
-                [],
-                new ReferenceConfigurator('service_container'),
             ])
 
         ->alias(AdminPoolLoader::class, 'sonata.admin.route_loader')
+            ->deprecate(...BCDeprecationParameters::forConfig(
+                'The "%alias_id%" alias is deprecated since sonata-project/admin-bundle 3.x and will be removed in 4.0.',
+                '3.x'
+            ))
 
         ->set('sonata.admin.helper', AdminHelper::class)
             ->public()

diff --git a/src/Route/AdminPoolLoader.php b/src/Route/AdminPoolLoader.php
@@ -34,18 +34,33 @@ class AdminPoolLoader extends Loader
     protected $pool;
 
     /**
+     * NEXT_MAJOR: Remove this property.
+     *
      * @var array
      */
     protected $adminServiceIds = [];
 
     /**
-     * @var ContainerInterface
+     * NEXT_MAJOR: Remove this property.
+     *
+     * @var ContainerInterface|null
      */
     protected $container;
 
-    public function __construct(Pool $pool, array $adminServiceIds, ContainerInterface $container)
+    // NEXT_MAJOR: Remove $adminServiceIds and $container parameters.
+    public function __construct(Pool $pool, array $adminServiceIds = [], ?ContainerInterface $container = null)
     {
         $this->pool = $pool;
+        // NEXT_MAJOR: Remove next line.
+        if (\func_num_args() > 1) {
+            @trigger_error(sprintf(
+                'Passing more than one argument to "%s()" is deprecated since'
+                .' sonata-project/admin-bundle 3.x.',
+                __METHOD__
+            ), \E_USER_DEPRECATED);
+        }
+
+        // NEXT_MAJOR: Remove the following two lines.
         $this->adminServiceIds = $adminServiceIds;
         $this->container = $container;
     }
@@ -58,7 +73,8 @@ public function supports($resource, $type = null)
     public function load($resource, $type = null)
     {
         $collection = new SymfonyRouteCollection();
-        foreach ($this->adminServiceIds as $id) {
+        // NEXT_MAJOR: Replace $this->getAdminServiceIds() with $this->pool->getAdminServiceIds()
+        foreach ($this->getAdminServiceIds() as $id) {
             $admin = $this->pool->getInstance($id);
 
             foreach ($admin->getRoutes()->getElements() as $code => $route) {
@@ -71,11 +87,26 @@ public function load($resource, $type = null)
             }
         }
 
-        $reflection = new \ReflectionObject($this->container);
-        if (file_exists($reflection->getFileName())) {
-            $collection->addResource(new FileResource($reflection->getFileName()));
+        // NEXT_MAJOR: Remove this block.
+        if (null !== $this->container) {
+            $reflection = new \ReflectionObject($this->container);
+            if (file_exists($reflection->getFileName())) {
+                $collection->addResource(new FileResource($reflection->getFileName()));
+            }
         }
 
         return $collection;
     }
+
+    /**
+     * @return string[]
+     */
+    private function getAdminServiceIds(): array
+    {
+        if ([] !== $this->adminServiceIds) {
+            return $this->adminServiceIds;
+        }
+
+        return $this->pool->getAdminServiceIds();
+    }
 }
diff --git a/tests/Route/AdminPoolLoaderTest.php b/tests/Route/AdminPoolLoaderTest.php
@@ -18,6 +18,7 @@
 use Sonata\AdminBundle\Admin\Pool;
 use Sonata\AdminBundle\Route\AdminPoolLoader;
 use Sonata\AdminBundle\Route\RouteCollection;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 use Symfony\Component\DependencyInjection\Container;
 use Symfony\Component\Routing\Route as SymfonyRoute;
 use Symfony\Component\Routing\RouteCollection as SymfonyRouteCollection;
@@ -27,12 +28,15 @@
  */
 class AdminPoolLoaderTest extends TestCase
 {
+    // NEXT_MAJOR: Remove next line.
+    use ExpectDeprecationTrait;
+
     public function testSupports(): void
     {
         $container = new Container();
-        $pool = new Pool($container);
+        $pool = new Pool($container, ['foo_admin', 'bar_admin']);
 
-        $adminPoolLoader = new AdminPoolLoader($pool, ['foo_admin', 'bar_admin'], $container);
+        $adminPoolLoader = new AdminPoolLoader($pool);
 
         $this->assertTrue($adminPoolLoader->supports('foo', 'sonata_admin'));
         $this->assertFalse($adminPoolLoader->supports('foo', 'bar'));
@@ -43,7 +47,7 @@ public function testLoad(): void
         $container = new Container();
         $pool = new Pool($container, ['foo_admin', 'bar_admin']);
 
-        $adminPoolLoader = new AdminPoolLoader($pool, ['foo_admin', 'bar_admin'], $container);
+        $adminPoolLoader = new AdminPoolLoader($pool);
 
         $routeCollection1 = new RouteCollection('base.Code.Route.foo', 'baseRouteNameFoo', 'baseRoutePatternFoo', 'baseControllerNameFoo');
         $routeCollection2 = new RouteCollection('base.Code.Route.bar', 'baseRouteNameBar', 'baseRoutePatternBar', 'baseControllerNameBar');
@@ -73,4 +77,32 @@ public function testLoad(): void
         $this->assertInstanceOf(SymfonyRoute::class, $collection->get('baseRouteNameBar_bar'));
         $this->assertInstanceOf(SymfonyRoute::class, $collection->get('baseRouteNameBar_bar'));
     }
+
+    /**
+     * NEXT_MAJOR: Remove this method.
+     *
+     * @group legacy
+     */
+    public function testThrowsADeprecationConstructingWithContainer(): void
+    {
+        $container = new Container();
+        $pool = new Pool($container);
+
+        $this->expectDeprecation('Passing more than one argument to "Sonata\AdminBundle\Route\AdminPoolLoader::__construct()" is deprecated since sonata-project/admin-bundle 3.x.');
+        new AdminPoolLoader($pool, ['foo_admin', 'bar_admin'], $container);
+    }
+
+    /**
+     * NEXT_MAJOR: Remove this method.
+     *
+     * @group legacy
+     */
+    public function testThrowsADeprecationConstructingWithAdminServicesIds(): void
+    {
+        $container = new Container();
+        $pool = new Pool($container);
+
+        $this->expectDeprecation('Passing more than one argument to "Sonata\AdminBundle\Route\AdminPoolLoader::__construct()" is deprecated since sonata-project/admin-bundle 3.x.');
+        new AdminPoolLoader($pool, ['foo_admin', 'bar_admin']);
+    }
 }