Use a tagged iterator to register menu providers

The compiler pass is now only responsible for detecting whether we need
to use the ChainProvider or no, but does not handle the configuration of
the ChainProvider anymore.

Author: stof

config/menu.xml

@@ -31,7 +31,7 @@
         <service id="Knp\Menu\Provider\MenuProviderInterface" alias="knp_menu.menu_provider" public="false"/>
 
         <service id="knp_menu.menu_provider.chain" class="Knp\Menu\Provider\ChainProvider" public="false">
-            <argument type="collection" />
+            <argument type="tagged_iterator" tag="knp_menu.provider" />
         </service>
 
         <service id="knp_menu.menu_provider.lazy" class="Knp\Menu\Provider\LazyProvider" public="false">

src/DependencyInjection/Compiler/AddProvidersPass.php

@@ -2,13 +2,11 @@
 
 namespace Knp\Bundle\MenuBundle\DependencyInjection\Compiler;
 
-use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
-use Symfony\Component\DependencyInjection\Reference;
 
 /**
- * This compiler pass registers the providers in the ChainProvider.
+ * This compiler pass registers the ChainProvider or the only provider as `knp_menu.menu_provider`.
  *
  * @author Christophe Coevoet <[email protected]>
  *
@@ -18,26 +16,16 @@ final class AddProvidersPass implements CompilerPassInterface
 {
     public function process(ContainerBuilder $container): void
     {
-        if (!$container->hasDefinition('knp_menu.menu_provider.chain')) {
-            return;
-        }
-
         $providers = [];
         foreach ($container->findTaggedServiceIds('knp_menu.provider') as $id => $tags) {
-            $providers[] = new Reference($id);
+            $providers[] = $id;
         }
 
         if (1 === \count($providers)) {
             // Use an alias instead of wrapping it in the ChainProvider for performances
             // when using only one (the default case as the bundle defines one provider)
-            $container->setAlias('knp_menu.menu_provider', (string) \reset($providers));
+            $container->setAlias('knp_menu.menu_provider', $providers[0]);
         } else {
-            if (\class_exists(IteratorArgument::class)) {
-                $providers = new IteratorArgument($providers);
-            }
-
-            $definition = $container->getDefinition('knp_menu.menu_provider.chain');
-            $definition->replaceArgument(0, $providers);
             $container->setAlias('knp_menu.menu_provider', 'knp_menu.menu_provider.chain');
         }
     }

tests/DependencyInjection/Compiler/AddProvidersPassTest.php

@@ -5,20 +5,10 @@
 use Knp\Bundle\MenuBundle\DependencyInjection\Compiler\AddProvidersPass;
 use Knp\Menu\Provider\ChainProvider;
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
-use Symfony\Component\DependencyInjection\Reference;
 
 class AddProvidersPassTest extends TestCase
 {
-    public function testProcessWithoutProviderDefinition(): void
-    {
-        $containerBuilder = new ContainerBuilder();
-        (new AddProvidersPass())->process($containerBuilder);
-
-        self::assertFalse($containerBuilder->hasAlias('knp_menu.menu_provider'));
-    }
-
     public function testProcessForOneProvider(): void
     {
         $containerBuilder = new ContainerBuilder();
@@ -33,12 +23,6 @@ public function testProcessForOneProvider(): void
 
     public function testProcessForManyProviders(): void
     {
-        $expectedProviders = [new Reference('id'), new Reference('id2')];
-
-        if (\class_exists(IteratorArgument::class)) {
-            $expectedProviders = new IteratorArgument($expectedProviders);
-        }
-
         $containerBuilder = new ContainerBuilder();
         $containerBuilder->register('knp_menu.menu_provider.chain', ChainProvider::class)->setArgument(0, []);
         $containerBuilder->register('id')->addTag('knp_menu.provider');
@@ -48,9 +32,5 @@ public function testProcessForManyProviders(): void
 
         self::assertTrue($containerBuilder->hasAlias('knp_menu.menu_provider'));
         self::assertSame('knp_menu.menu_provider.chain', (string) $containerBuilder->getAlias('knp_menu.menu_provider'));
-
-        self::assertEquals(
-            [$expectedProviders],
-            $containerBuilder->getDefinition('knp_menu.menu_provider.chain')->getArguments());
     }
 }