Alternative fix to #2643: allow autowiring of filter classes

api-platform · Mar 29, 2019 · 7608858 · 7608858
1 parent 66ee1ba
commit 7608858
Show file tree

Hide file tree

Showing 9 changed files with 92 additions and 35 deletions.
diff --git a/src/Bridge/Doctrine/Orm/Filter/AbstractFilter.php b/src/Bridge/Doctrine/Orm/Filter/AbstractFilter.php
@@ -42,10 +42,7 @@ abstract class AbstractFilter implements FilterInterface
     protected $logger;
     protected $properties;
 
-    /**
-     * @param RequestStack|null $requestStack No prefix to prevent autowiring of this deprecated property
-     */
-    public function __construct(ManagerRegistry $managerRegistry, $requestStack = null, LoggerInterface $logger = null, array $properties = null)
+    public function __construct(ManagerRegistry $managerRegistry, ?RequestStack $requestStack = null, LoggerInterface $logger = null, array $properties = null)
     {
         if (null !== $requestStack) {
             @trigger_error(sprintf('Passing an instance of "%s" is deprecated since 2.2. Use "filters" context key instead.', RequestStack::class), E_USER_DEPRECATED);

diff --git a/src/Bridge/Doctrine/Orm/Filter/OrderFilter.php b/src/Bridge/Doctrine/Orm/Filter/OrderFilter.php
@@ -40,10 +40,7 @@ class OrderFilter extends AbstractContextAwareFilter implements OrderFilterInter
 {
     use OrderFilterTrait;
 
-    /**
-     * @param RequestStack|null $requestStack No prefix to prevent autowiring of this deprecated property
-     */
-    public function __construct(ManagerRegistry $managerRegistry, $requestStack = null, string $orderParameterName = 'order', LoggerInterface $logger = null, array $properties = null)
+    public function __construct(ManagerRegistry $managerRegistry, ?RequestStack $requestStack = null, string $orderParameterName = 'order', LoggerInterface $logger = null, array $properties = null)
     {
         if (null !== $properties) {
             $properties = array_map(function ($propertyOptions) {

diff --git a/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php b/src/Bridge/Doctrine/Orm/Filter/SearchFilter.php
@@ -38,10 +38,7 @@ class SearchFilter extends AbstractContextAwareFilter implements SearchFilterInt
 
     public const DOCTRINE_INTEGER_TYPE = DBALType::INTEGER;
 
-    /**
-     * @param RequestStack|null $requestStack No prefix to prevent autowiring of this deprecated property
-     */
-    public function __construct(ManagerRegistry $managerRegistry, $requestStack = null, IriConverterInterface $iriConverter, PropertyAccessorInterface $propertyAccessor = null, LoggerInterface $logger = null, array $properties = null)
+    public function __construct(ManagerRegistry $managerRegistry, ?RequestStack $requestStack, IriConverterInterface $iriConverter, PropertyAccessorInterface $propertyAccessor = null, LoggerInterface $logger = null, array $properties = null)
     {
         parent::__construct($managerRegistry, $requestStack, $logger, $properties);
 

diff --git a/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php b/src/Bridge/Symfony/Bundle/ApiPlatformBundle.php
@@ -18,6 +18,8 @@
 use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\ElasticsearchClientPass;
 use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\FilterPass;
 use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\MetadataAwareNameConverterPass;
+use Symfony\Component\DependencyInjection\Compiler\PassConfig;
+use Symfony\Component\DependencyInjection\Compiler\ResolveInstanceofConditionalsPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\HttpKernel\Bundle\Bundle;
 
@@ -36,7 +38,8 @@ public function build(ContainerBuilder $container)
         parent::build($container);
 
         $container->addCompilerPass(new DataProviderPass());
-        $container->addCompilerPass(new AnnotationFilterPass());
+        // Run the compiler pass before the {@see ResolveInstanceofConditionalsPass} to allow autoconfiguration of generated filter definitions.
+        $container->addCompilerPass(new AnnotationFilterPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 101);
         $container->addCompilerPass(new FilterPass());
         $container->addCompilerPass(new ElasticsearchClientPass());
         $container->addCompilerPass(new MetadataAwareNameConverterPass());

diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php
@@ -16,10 +16,12 @@
 use ApiPlatform\Core\Api\FilterInterface;
 use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Extension\AggregationCollectionExtensionInterface;
 use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Extension\AggregationItemExtensionInterface;
+use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Filter\AbstractFilter as DoctrineMongoDbOdmAbstractFilter;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\EagerLoadingExtension;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\FilterEagerLoadingExtension;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryCollectionExtensionInterface as DoctrineQueryCollectionExtensionInterface;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryItemExtensionInterface;
+use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractContextAwareFilter as DoctrineOrmAbstractContextAwareFilter;
 use ApiPlatform\Core\Bridge\Elasticsearch\DataProvider\Extension\RequestBodySearchCollectionExtensionInterface;
 use ApiPlatform\Core\DataPersister\DataPersisterInterface;
 use ApiPlatform\Core\DataProvider\CollectionDataProviderInterface;
@@ -39,6 +41,7 @@
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
+use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
 use Symfony\Component\Finder\Finder;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
@@ -475,6 +478,8 @@ private function registerDoctrineConfiguration(ContainerBuilder $container, arra
             ->addTag('api_platform.doctrine.orm.query_extension.item');
         $container->registerForAutoconfiguration(DoctrineQueryCollectionExtensionInterface::class)
             ->addTag('api_platform.doctrine.orm.query_extension.collection');
+        $container->registerForAutoconfiguration(DoctrineOrmAbstractContextAwareFilter::class)
+            ->setBindings(['$requestStack' => null]);
 
         $loader->load('doctrine_orm.xml');
 
@@ -501,6 +506,8 @@ private function registerDoctrineMongoDbOdmConfiguration(ContainerBuilder $conta
             ->addTag('api_platform.doctrine.mongodb.aggregation_extension.item');
         $container->registerForAutoconfiguration(AggregationCollectionExtensionInterface::class)
             ->addTag('api_platform.doctrine.mongodb.aggregation_extension.collection');
+        $container->registerForAutoconfiguration(DoctrineMongoDbOdmAbstractFilter::class)
+            ->setBindings(['$managerRegistry' => new Reference('doctrine_mongodb')]);
 
         $loader->load('doctrine_mongodb_odm.xml');
     }

diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/AnnotationFilterPass.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Compiler/AnnotationFilterPass.php
@@ -13,15 +13,14 @@
 
 namespace ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler;
 
-use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Filter\FilterInterface as MongoDbOdmFilterInterface;
 use ApiPlatform\Core\Util\AnnotationFilterExtractorTrait;
 use ApiPlatform\Core\Util\ReflectionClassRecursiveIterator;
 use Doctrine\Common\Annotations\Reader;
 use Symfony\Component\DependencyInjection\ChildDefinition;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
-use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
 
 /**
  * Injects filters.
@@ -34,43 +33,48 @@ final class AnnotationFilterPass implements CompilerPassInterface
 {
     use AnnotationFilterExtractorTrait;
 
-    public const TAG_FILTER_NAME = 'api_platform.filter';
+    private const TAG_FILTER_NAME = 'api_platform.filter';
+
+    /**
+     * @var Reader|null
+     */
+    private $reader;
 
     /**
      * {@inheritdoc}
      */
     public function process(ContainerBuilder $container)
     {
         $resourceClassDirectories = $container->getParameter('api_platform.resource_class_directories');
-        /**
-         * @var Reader
-         */
-        $reader = $container->get('annotation_reader');
 
         foreach (ReflectionClassRecursiveIterator::getReflectionClassesFromDirectories($resourceClassDirectories) as $className => $reflectionClass) {
-            $this->createFilterDefinitions($reflectionClass, $reader, $container);
+            $this->createFilterDefinitions($reflectionClass, $container);
         }
     }
 
-    private function createFilterDefinitions(\ReflectionClass $reflectionClass, Reader $reader, ContainerBuilder $container): void
+    /**
+     * @throws InvalidArgumentException
+     */
+    private function createFilterDefinitions(\ReflectionClass $reflectionClass, ContainerBuilder $container): void
     {
+        $reader = $this->reader ?? $this->reader = $container->get('annotation_reader');
+
         foreach ($this->readFilterAnnotations($reflectionClass, $reader) as $id => [$arguments, $filterClass]) {
-            if ($container->hasDefinition($id)) {
+            if ($container->has($id)) {
                 continue;
             }
 
-            if ($container->has($filterClass) && $container->findDefinition($filterClass)->isAbstract()) {
-                $definition = new ChildDefinition($filterClass);
+            if ($container->has($filterClass) && ($definition = $container->findDefinition($filterClass))->isAbstract()) {
+                $definition = new ChildDefinition($definition->getClass());
+            } elseif ($reflectionClass = $container->getReflectionClass($filterClass, false)) {
+                $definition = new Definition($reflectionClass->getName());
+                $definition->setAutoconfigured(true);
             } else {
-                $definition = new Definition();
-                $definition->setClass($filterClass);
+                throw new InvalidArgumentException(sprintf('Class "%s" used for service "%s" cannot be found.', $filterClass, $id));
             }
 
             $definition->addTag(self::TAG_FILTER_NAME);
             $definition->setAutowired(true);
-            if (is_a($filterClass, MongoDbOdmFilterInterface::class, true)) {
-                $definition->setArgument('$managerRegistry', new Reference('doctrine_mongodb'));
-            }
 
             foreach ($arguments as $key => $value) {
                 $definition->setArgument("$$key", $value);

diff --git a/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php b/tests/Bridge/Symfony/Bundle/ApiPlatformBundleTest.php
@@ -21,6 +21,7 @@
 use ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Compiler\MetadataAwareNameConverterPass;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
+use Symfony\Component\DependencyInjection\Compiler\PassConfig;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 
 /**
@@ -32,7 +33,7 @@ public function testBuild()
     {
         $containerProphecy = $this->prophesize(ContainerBuilder::class);
         $containerProphecy->addCompilerPass(Argument::type(DataProviderPass::class))->shouldBeCalled();
-        $containerProphecy->addCompilerPass(Argument::type(AnnotationFilterPass::class))->shouldBeCalled();
+        $containerProphecy->addCompilerPass(Argument::type(AnnotationFilterPass::class), PassConfig::TYPE_BEFORE_OPTIMIZATION, 101)->shouldBeCalled();
         $containerProphecy->addCompilerPass(Argument::type(FilterPass::class))->shouldBeCalled();
         $containerProphecy->addCompilerPass(Argument::type(ElasticsearchClientPass::class))->shouldBeCalled();
         $containerProphecy->addCompilerPass(Argument::type(MetadataAwareNameConverterPass::class))->shouldBeCalled();

diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php
@@ -22,6 +22,7 @@
 use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Extension\FilterExtension as MongoDbOdmFilterExtension;
 use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Extension\OrderExtension as MongoDbOdmOrderExtension;
 use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Extension\PaginationExtension as MongoDbOdmPaginationExtension;
+use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Filter\AbstractFilter as DoctrineMongoDbOdmAbstractFilter;
 use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Filter\BooleanFilter as MongoDbOdmBooleanFilter;
 use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Filter\DateFilter as MongoDbOdmDateFilter;
 use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Filter\ExistsFilter as MongoDbOdmExistsFilter;
@@ -36,6 +37,7 @@
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\PaginationExtension;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryCollectionExtensionInterface;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryItemExtensionInterface;
+use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractContextAwareFilter as DoctrineOrmAbstractContextAwareFilter;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\BooleanFilter;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\DateFilter;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\ExistsFilter;
@@ -87,6 +89,7 @@
 use Symfony\Component\DependencyInjection\Extension\ExtensionInterface;
 use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
+use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Serializer\Exception\ExceptionInterface;
 
@@ -480,6 +483,8 @@ public function testDisableDoctrine()
         $this->childDefinitionProphecy->addTag('api_platform.doctrine.orm.query_extension.item')->shouldNotBeCalled();
         $containerBuilderProphecy->registerForAutoconfiguration(QueryCollectionExtensionInterface::class)->shouldNotBeCalled();
         $this->childDefinitionProphecy->addTag('api_platform.doctrine.orm.query_extension.collection')->shouldNotBeCalled();
+        $containerBuilderProphecy->registerForAutoconfiguration(DoctrineOrmAbstractContextAwareFilter::class)->shouldNotBeCalled();
+        $this->childDefinitionProphecy->setBindings(['$requestStack' => null])->shouldNotBeCalled();
         $containerBuilderProphecy->setDefinition('api_platform.doctrine.listener.http_cache.purge', Argument::type(Definition::class))->shouldNotBeCalled();
         $containerBuilderProphecy->setDefinition('api_platform.doctrine.orm.boolean_filter', Argument::type(Definition::class))->shouldNotBeCalled();
         $containerBuilderProphecy->setDefinition('api_platform.doctrine.orm.collection_data_provider', Argument::type(Definition::class))->shouldNotBeCalled();
@@ -526,6 +531,8 @@ public function testDisableDoctrineMongoDbOdm()
         $this->childDefinitionProphecy->addTag('api_platform.doctrine.mongodb.aggregation_extension.item')->shouldNotBeCalled();
         $containerBuilderProphecy->registerForAutoconfiguration(AggregationCollectionExtensionInterface::class)->shouldNotBeCalled();
         $this->childDefinitionProphecy->addTag('api_platform.doctrine.mongodb.aggregation_extension.collection')->shouldNotBeCalled();
+        $containerBuilderProphecy->registerForAutoconfiguration(DoctrineMongoDbOdmAbstractFilter::class)->shouldNotBeCalled();
+        $this->childDefinitionProphecy->setBindings(Argument::allOf(Argument::withEntry('$managerRegistry', Argument::type(Reference::class))))->shouldNotBeCalled();
         $containerBuilderProphecy->setDefinition('api_platform.doctrine_mongodb.odm.aggregation_extension.filter', Argument::type(Definition::class))->shouldNotBeCalled();
         $containerBuilderProphecy->setDefinition('api_platform.doctrine_mongodb.odm.aggregation_extension.order', Argument::type(Definition::class))->shouldNotBeCalled();
         $containerBuilderProphecy->setDefinition('api_platform.doctrine_mongodb.odm.aggregation_extension.pagination', Argument::type(Definition::class))->shouldNotBeCalled();
@@ -907,6 +914,10 @@ private function getBaseContainerBuilderProphecy()
             ->willReturn($this->childDefinitionProphecy)->shouldBeCalledTimes(1);
         $this->childDefinitionProphecy->addTag('api_platform.doctrine.orm.query_extension.collection')->shouldBeCalledTimes(1);
 
+        $containerBuilderProphecy->registerForAutoconfiguration(DoctrineOrmAbstractContextAwareFilter::class)
+            ->willReturn($this->childDefinitionProphecy)->shouldBeCalledTimes(1);
+        $this->childDefinitionProphecy->setBindings(['$requestStack' => null])->shouldBeCalledTimes(1);
+
         $containerBuilderProphecy->registerForAutoconfiguration(AggregationItemExtensionInterface::class)
             ->willReturn($this->childDefinitionProphecy)->shouldBeCalledTimes(1);
         $this->childDefinitionProphecy->addTag('api_platform.doctrine.mongodb.aggregation_extension.item')->shouldBeCalledTimes(1);
@@ -915,6 +926,10 @@ private function getBaseContainerBuilderProphecy()
             ->willReturn($this->childDefinitionProphecy)->shouldBeCalledTimes(1);
         $this->childDefinitionProphecy->addTag('api_platform.doctrine.mongodb.aggregation_extension.collection')->shouldBeCalledTimes(1);
 
+        $containerBuilderProphecy->registerForAutoconfiguration(DoctrineMongoDbOdmAbstractFilter::class)
+            ->willReturn($this->childDefinitionProphecy)->shouldBeCalledTimes(1);
+        $this->childDefinitionProphecy->setBindings(Argument::allOf(Argument::withEntry('$managerRegistry', Argument::type(Reference::class))))->shouldBeCalledTimes(1);
+
         $containerBuilderProphecy->registerForAutoconfiguration(DataTransformerInterface::class)
             ->willReturn($this->childDefinitionProphecy)->shouldBeCalledTimes(1);
         $this->childDefinitionProphecy->addTag('api_platform.data_transformer')->shouldBeCalledTimes(1);