Merge pull request #1689 from deguif/use-service-locator-for-pager-persisters

Use service locator for pager persisters registry

Author: XWB

src/DependencyInjection/Compiler/RegisterPagerPersistersPass.php

@@ -13,7 +13,6 @@
 
 use FOS\ElasticaBundle\DependencyInjection\Compiler\ConfigSourcePass;
 use FOS\ElasticaBundle\DependencyInjection\Compiler\IndexPass;
-use FOS\ElasticaBundle\DependencyInjection\Compiler\RegisterPagerPersistersPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\HttpKernel\Bundle\Bundle;
 
@@ -28,6 +27,5 @@ public function build(ContainerBuilder $container)
 
         $container->addCompilerPass(new ConfigSourcePass());
         $container->addCompilerPass(new IndexPass());
-        $container->addCompilerPass(new RegisterPagerPersistersPass());
     }
 }

src/FOSElasticaBundle.php

@@ -11,7 +11,6 @@
 
 namespace FOS\ElasticaBundle\Finder;
 
-use Elastica\Query;
 use FOS\ElasticaBundle\Paginator\PaginatorAdapterInterface;
 use Pagerfanta\Pagerfanta;
 

src/Finder/PaginatedFinderInterface.php

@@ -11,47 +11,27 @@
 
 namespace FOS\ElasticaBundle\Persister;
 
-use Symfony\Component\DependencyInjection\ContainerAwareInterface;
-use Symfony\Component\DependencyInjection\ContainerAwareTrait;
+use Symfony\Component\DependencyInjection\ServiceLocator;
 
-final class PagerPersisterRegistry implements ContainerAwareInterface
+final class PagerPersisterRegistry
 {
-    use ContainerAwareTrait;
+    /** @var ServiceLocator */
+    private $persisters;
 
-    /**
-     * @var string[]
-     */
-    private $nameToServiceIdMap = [];
-
-    /**
-     * @param string[] $nameToServiceIdMap
-     */
-    public function __construct(array $nameToServiceIdMap)
+    public function __construct(ServiceLocator $persisters)
     {
-        $this->nameToServiceIdMap = $nameToServiceIdMap;
+        $this->persisters = $persisters;
     }
 
     /**
-     * @param string $name
-     *
      * @throws \InvalidArgumentException if no pager persister was registered for the given name
-     *
-     * @return PagerPersisterInterface
      */
-    public function getPagerPersister($name)
+    public function getPagerPersister(string $name): PagerPersisterInterface
     {
-        if (!isset($this->nameToServiceIdMap[$name])) {
+        if (!$this->persisters->has($name)) {
             throw new \InvalidArgumentException(sprintf('No pager persister was registered for the give name "%s".', $name));
         }
 
-        $serviceId = $this->nameToServiceIdMap[$name];
-
-        $pagerPersister = $this->container->get($serviceId);
-
-        if (!$pagerPersister instanceof PagerPersisterInterface) {
-            throw new \LogicException(sprintf('The pager provider service "%s" must implement "%s" interface but it is an instance of "%s" class.', $serviceId, PagerPersisterInterface::class, get_class($pagerPersister)));
-        }
-
-        return $pagerPersister;
+        return $this->persisters->get($name);
     }
 }

src/Persister/PagerPersisterRegistry.php

@@ -15,10 +15,8 @@
 
 class PersisterRegistry
 {
-    /**
-     * @var ServiceLocator
-     */
-    private $persisters = [];
+    /** @var ServiceLocator */
+    private $persisters;
 
     public function __construct(ServiceLocator $persisters)
     {

src/Persister/PersisterRegistry.php

@@ -19,7 +19,7 @@
 class PagerProviderRegistry
 {
     /** @var ServiceLocator */
-    private $providers = [];
+    private $providers;
 
     public function __construct(ServiceLocator $providers)
     {

src/Provider/PagerProviderRegistry.php

@@ -21,11 +21,7 @@
         </service>
 
         <service id="fos_elastica.pager_persister_registry" class="FOS\ElasticaBundle\Persister\PagerPersisterRegistry">
-            <argument type="collection" /> <!-- nameToServiceIdMap -->
-
-            <call method="setContainer">
-                <argument type="service" id="service_container" />
-            </call>
+            <argument type="tagged_locator" tag="fos_elastica.pager_persister" index-by="persisterName" />
         </service>
 
         <service id="fos_elastica.persister_registry" class="FOS\ElasticaBundle\Persister\PersisterRegistry">

src/Resources/config/persister.xml

@@ -390,9 +390,9 @@ public function testShouldRegisterPagerPersisterRegisterService()
 
         $this->assertTrue($container->hasDefinition('fos_elastica.pager_persister_registry'));
 
-        $listener = $container->getDefinition('fos_elastica.pager_persister_registry');
-        $this->assertSame(PagerPersisterRegistry::class, $listener->getClass());
-        $this->assertSame([], $listener->getArgument(0));
+        $registry = $container->getDefinition('fos_elastica.pager_persister_registry');
+        $this->assertSame(PagerPersisterRegistry::class, $registry->getClass());
+        $this->assertCount(0, $registry->getArgument(0)->getTaggedIteratorArgument()->getValues());
     }
 
     public function testShouldRegisterDoctrineORMListener()

tests/Unit/DependencyInjection/FOSElasticaExtensionTest.php

@@ -22,7 +22,6 @@
 use FOS\ElasticaBundle\Provider\PagerProviderInterface;
 use FOS\ElasticaBundle\Provider\PagerProviderRegistry;
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\ServiceLocator;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\Messenger\Envelope;
@@ -38,7 +37,7 @@ public function testShouldImplementPagerPersisterInterface()
 
     public function testInsertDispatchAsyncPersistPageObject()
     {
-        $pagerPersisterRegistry = new PagerPersisterRegistry([]);
+        $pagerPersisterRegistry = new PagerPersisterRegistry($this->createMock(ServiceLocator::class));
         $pagerProviderRegistry = $this->createMock(PagerProviderRegistry::class);
         $messageBus = $this->createMock(MessageBusInterface::class);
         $sut = new AsyncPagerPersister($pagerPersisterRegistry, $pagerProviderRegistry, $messageBus);
@@ -57,24 +56,20 @@ function ($message) {
 
     public function testInsertPageReturnObjectCount()
     {
-        $pagerPersisterRegistry = new PagerPersisterRegistry(
-            [
-                InPlacePagerPersister::NAME => 'my_service',
-            ]
-        );
-
-        $serviceLocator = $this->createMock(ServiceLocator::class);
-        $serviceLocator->expects($this->once())->method('has')->with('foo')->willReturn(true);
-        $serviceLocator->expects($this->once())->method('get')->with('foo')->willReturn($this->createMock(ObjectPersisterInterface::class));
+        $persistersLocator = $this->createMock(ServiceLocator::class);
+        $persistersLocator->expects($this->once())->method('has')->with('foo')->willReturn(true);
+        $persistersLocator->expects($this->once())->method('get')->with('foo')->willReturn($this->createMock(ObjectPersisterInterface::class));
 
-        $container = $this->createMock(ContainerInterface::class);
-        $container->expects($this->once())->method('get')->with('my_service')->willReturn(
+        $pagerPersistersLocator = $this->createMock(ServiceLocator::class);
+        $pagerPersistersLocator->expects($this->once())->method('has')->with('in_place')->willReturn(true);
+        $pagerPersistersLocator->expects($this->once())->method('get')->with('in_place')->willReturn(
             new InPlacePagerPersister(
-                new PersisterRegistry($serviceLocator),
+                new PersisterRegistry($persistersLocator),
                 $this->createMock(EventDispatcherInterface::class)
             )
         );
-        $pagerPersisterRegistry->setContainer($container);
+
+        $pagerPersisterRegistry = new PagerPersisterRegistry($pagerPersistersLocator);
 
         $pagerMock = $this->createMock(PagerInterface::class);
         $pagerMock->expects($this->exactly(2))->method('setMaxPerPage')->with(10);

tests/Unit/Persister/AsyncPagerPersisterTest.php

@@ -12,84 +12,49 @@
 namespace FOS\ElasticaBundle\Persister;
 
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\DependencyInjection\Container;
-use Symfony\Component\DependencyInjection\ContainerAwareInterface;
+use Symfony\Component\DependencyInjection\ServiceLocator;
 
 class PagerPersisterRegistryTest extends TestCase
 {
-    public function testShouldImplementContainerAwareInterface()
-    {
-        $rc = new \ReflectionClass(PagerPersisterRegistry::class);
-
-        $this->assertTrue($rc->implementsInterface(ContainerAwareInterface::class));
-    }
-
     public function testShouldBeFinal()
     {
         $rc = new \ReflectionClass(PagerPersisterRegistry::class);
 
         $this->assertTrue($rc->isFinal());
     }
 
-    public function testCouldBeConstructedWithNameToServiceIdMap()
-    {
-        new PagerPersisterRegistry([]);
-    }
-
     public function testThrowsIfThereIsNoSuchEntryInNameToServiceIdMap()
     {
-        $container = new Container();
-
-        $registry = new PagerPersisterRegistry([
-            'the_name' => 'the_service_id',
-        ]);
-        $registry->setContainer($container);
+        $serviceLocator = $this->createMock(ServiceLocator::class);
+        $serviceLocator->expects($this->once())->method('has')->with('the_name')->willReturn(false);
 
         $this->expectException(\InvalidArgumentException::class);
-        $this->expectExceptionMessage('No pager persister was registered for the give name "the_other_name".');
-        $registry->getPagerPersister('the_other_name');
-    }
+        $this->expectExceptionMessage('No pager persister was registered for the give name "the_name".');
 
-    public function testThrowsIfRelatedServiceDoesNotImplementPagerPersisterInterface()
-    {
-        $container = new Container();
-        $container->set('the_service_id', new \stdClass());
-
-        $registry = new PagerPersisterRegistry([
-            'the_name' => 'the_service_id',
-        ]);
-        $registry->setContainer($container);
-
-        $this->expectException(\LogicException::class);
-        $this->expectExceptionMessage('The pager provider service "the_service_id" must implement "FOS\ElasticaBundle\Persister\PagerPersisterInterface" interface but it is an instance of "stdClass" class.');
-        $registry->getPagerPersister('the_name');
+        (new PagerPersisterRegistry($serviceLocator))->getPagerPersister('the_name');
     }
 
-    public function testThrowsIfThereIsServiceWithSuchId()
+    public function testThrowsIfRelatedServiceDoesNotImplementPagerPersisterInterface()
     {
-        $container = new Container();
+        $serviceLocator = $this->createMock(ServiceLocator::class);
+        $serviceLocator->expects($this->once())->method('has')->with('the_name')->willReturn(true);
+        $serviceLocator->expects($this->once())->method('get')->with('the_name')->willReturn(new \stdClass());
 
-        $registry = new PagerPersisterRegistry([
-            'the_name' => 'the_service_id',
-        ]);
-        $registry->setContainer($container);
+        $this->expectException(\TypeError::class);
+        $this->expectExceptionMessage('Return value of FOS\ElasticaBundle\Persister\PagerPersisterRegistry::getPagerPersister() must implement interface FOS\ElasticaBundle\Persister\PagerPersisterInterface, instance of stdClass returned');
 
-        $this->expectException(\LogicException::class);
-        $this->expectExceptionMessage('You have requested a non-existent service "the_service_id".');
-        $registry->getPagerPersister('the_name');
+        (new PagerPersisterRegistry($serviceLocator))->getPagerPersister('the_name');
     }
 
     public function testShouldReturnPagerPersisterByGivenName()
     {
         $pagerPersisterMock = $this->createPagerPersisterMock();
 
-        $container = new Container();
-        $container->set('the_service_id', $pagerPersisterMock);
+        $serviceLocator = $this->createMock(ServiceLocator::class);
+        $serviceLocator->expects($this->once())->method('has')->with('the_name')->willReturn(true);
+        $serviceLocator->expects($this->once())->method('get')->with('the_name')->willReturn($pagerPersisterMock);
 
-        $registry = new PagerPersisterRegistry([
-            'the_name' => 'the_service_id',
-        ]);
-        $registry->setContainer($container);
+        $registry = new PagerPersisterRegistry($serviceLocator);
 
         $actualPagerPersister = $registry->getPagerPersister('the_name');