Fix filter Eager Loading #944

abluchet · abluchet · commit f220063ff651 · 2017-02-21T17:30:02.000+01:00
diff --git a/features/bootstrap/FeatureContext.php b/features/bootstrap/FeatureContext.php
@@ -9,11 +9,13 @@
  * file that was distributed with this source code.
  */
 
+use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Bar;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeItem;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeLabel;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeRelation;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\FileConfigDummy;
+use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Foo;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedDummy;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelationEmbedder;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\UuidIdentifierDummy;
@@ -135,18 +137,21 @@ public function thereIsDummyObjectsWithRelatedDummy($nb)
     }
 
     /**
-     * @Given there is :nb dummy objects with relatedDummies
+     * @Given there is :nb dummy objects having each :nbrelated relatedDummies
      */
-    public function thereIsDummyObjectsWithRelatedDummies($nb)
+    public function thereIsDummyObjectsWithRelatedDummies($nb, $nbrelated)
     {
         for ($i = 1; $i <= $nb; ++$i) {
-            $relatedDummy = new RelatedDummy();
-            $relatedDummy->setName('RelatedDummy #'.$i);
-
             $dummy = new Dummy();
             $dummy->setName('Dummy #'.$i);
             $dummy->setAlias('Alias #'.($nb - $i));
-            $dummy->addRelatedDummy($relatedDummy);
+
+            for ($j = 1; $j <= $nbrelated; ++$j) {
+                $relatedDummy = new RelatedDummy();
+                $relatedDummy->setName('RelatedDummy'.$j.$i);
+                $this->manager->persist($relatedDummy);
+                $dummy->addRelatedDummy($relatedDummy);
+            }
 
             $this->manager->persist($relatedDummy);
             $this->manager->persist($dummy);
@@ -311,4 +316,28 @@ public function thereIsAFileConfigDummyObject()
         $this->manager->persist($fileConfigDummy);
         $this->manager->flush();
     }
+
+    /**
+     * @Given there is a Foo entity with related bars
+     */
+    public function thereIsAFooEntityWithRelatedBars()
+    {
+        $foo = new Foo();
+        $this->manager->persist($foo);
+
+        $bar1 = new Bar();
+        $bar1->setProp('toto');
+        $bar1->setFoo($foo);
+        $this->manager->persist($bar1);
+
+        $bar2 = new Bar();
+        $bar2->setProp('tata');
+        $bar2->setFoo($foo);
+        $this->manager->persist($bar2);
+
+        $foo->setBars([$bar1, $bar2]);
+        $this->manager->persist($foo);
+
+        $this->manager->flush();
+    }
 }
diff --git a/features/doctrine/date_filter.feature b/features/doctrine/date_filter.feature
@@ -389,18 +389,18 @@ Feature: Date filter on collections
     And the JSON should be equal to:
     """
     {
-          "@context": "\/contexts\/Dummy",
-          "@id": "\/dummies",
+          "@context": "/contexts/Dummy",
+          "@id": "/dummies",
           "@type": "hydra:Collection",
           "hydra:member": [],
           "hydra:totalItems": 0,
           "hydra:view": {
-              "@id": "\/dummies?relatedDummy.dummyDate%5Bafter%5D=2015-04-28",
+              "@id": "/dummies?relatedDummy.dummyDate%5Bafter%5D=2015-04-28",
               "@type": "hydra:PartialCollectionView"
           },
           "hydra:search": {
               "@type": "hydra:IriTemplate",
-              "hydra:template": "\/dummies{?id,id[],name,alias,description,relatedDummy.name,relatedDummy.name[],relatedDummies,relatedDummies[],dummy,order[id],order[name],order[relatedDummy.symfony],dummyDate[before],dummyDate[after],relatedDummy.dummyDate[before],relatedDummy.dummyDate[after],dummyFloat[between],dummyFloat[gt],dummyFloat[gte],dummyFloat[lt],dummyFloat[lte],dummyPrice[between],dummyPrice[gt],dummyPrice[gte],dummyPrice[lt],dummyPrice[lte],dummyBoolean,dummyFloat,dummyPrice}",
+              "hydra:template": "/dummies{?id,id[],name,alias,description,relatedDummy.name,relatedDummy.name[],relatedDummies,relatedDummies[],dummy,relatedDummies.name,order[id],order[name],order[relatedDummy.symfony],dummyDate[before],dummyDate[after],relatedDummy.dummyDate[before],relatedDummy.dummyDate[after],dummyFloat[between],dummyFloat[gt],dummyFloat[gte],dummyFloat[lt],dummyFloat[lte],dummyPrice[between],dummyPrice[gt],dummyPrice[gte],dummyPrice[lt],dummyPrice[lte],dummyBoolean,dummyFloat,dummyPrice}",
               "hydra:variableRepresentation": "BasicRepresentation",
               "hydra:mapping": [
                   {
@@ -463,6 +463,12 @@ Feature: Date filter on collections
                       "property": "dummy",
                       "required": false
                   },
+                  {
+                      "@type": "IriTemplateMapping",
+                      "variable": "relatedDummies.name",
+                      "property": "relatedDummies.name",
+                      "required": false
+                  },
                   {
                       "@type": "IriTemplateMapping",
                       "variable": "order[id]",
diff --git a/features/doctrine/search_filter.feature b/features/doctrine/search_filter.feature
@@ -4,6 +4,56 @@ Feature: Search filter on collections
   I need to search for collections properties
 
   @createSchema
+  Scenario: Test #944
+    Given there is a Foo entity with related bars
+    When I send a "GET" request to "/foos?bars.prop=toto"
+    Then the response status code should be 200
+    And print last JSON response
+    And the JSON should be equal to:
+		"""
+    {
+        "@context": "/contexts/Foo",
+        "@id": "/foos",
+        "@type": "hydra:Collection",
+        "hydra:member": [
+            {
+                "@id": "/foos/1",
+                "@type": "Foo",
+                "bars": [
+                    {
+                        "@id": "/bars/1",
+                        "@type": "Bar",
+                        "prop": "toto"
+                    },
+                    {
+                        "@id": "/bars/2",
+                        "@type": "Bar",
+                        "prop": "tata"
+                    }
+                ]
+            }
+        ],
+        "hydra:totalItems": 1,
+        "hydra:view": {
+            "@id": "/foos?bars.prop=toto",
+            "@type": "hydra:PartialCollectionView"
+        },
+        "hydra:search": {
+            "@type": "hydra:IriTemplate",
+            "hydra:template": "/foos{?bars.prop}",
+            "hydra:variableRepresentation": "BasicRepresentation",
+            "hydra:mapping": [
+                {
+                    "@type": "IriTemplateMapping",
+                    "variable": "bars.prop",
+                    "property": "bars.prop",
+                    "required": false
+                }
+            ]
+        }
+    }
+    """
+
   Scenario: Search collection by name (partial)
     Given there is "30" dummy objects
     When I send a "GET" request to "/dummies?name=my"
@@ -42,7 +92,6 @@ Feature: Search filter on collections
     """
 
   Scenario: Search collection by name (partial case insensitive)
-    Given there is "30" dummy objects
     When I send a "GET" request to "/dummies?dummy=somedummytest1"
     Then the response status code should be 200
     And the response should be in JSON
@@ -167,3 +216,17 @@ Feature: Search filter on collections
       }
     }
     """
+
+  @createSchema
+  @dropSchema
+  Scenario: Search related collection by name
+    Given there is 3 dummy objects having each 3 relatedDummies
+    When I add "Accept" header equal to "application/hal+json"
+    And I send a "GET" request to "/dummies?relatedDummies.name=RelatedDummy1"
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the JSON node "_embedded.item" should have 3 elements
+    And the JSON node "_embedded.item[0]._links.relatedDummies" should have 3 elements
+    And the JSON node "_embedded.item[1]._links.relatedDummies" should have 3 elements
+    And the JSON node "_embedded.item[2]._links.relatedDummies" should have 3 elements
+
diff --git a/features/main/crud.feature b/features/main/crud.feature
@@ -123,7 +123,7 @@ Feature: Create-Retrieve-Update-Delete
       "hydra:totalItems": 1,
       "hydra:search": {
         "@type": "hydra:IriTemplate",
-        "hydra:template": "/dummies{?id,id[],name,alias,description,relatedDummy.name,relatedDummy.name[],relatedDummies,relatedDummies[],dummy,order[id],order[name],order[relatedDummy.symfony],dummyDate[before],dummyDate[after],relatedDummy.dummyDate[before],relatedDummy.dummyDate[after],dummyFloat[between],dummyFloat[gt],dummyFloat[gte],dummyFloat[lt],dummyFloat[lte],dummyPrice[between],dummyPrice[gt],dummyPrice[gte],dummyPrice[lt],dummyPrice[lte],dummyBoolean,dummyFloat,dummyPrice}",
+        "hydra:template": "/dummies{?id,id[],name,alias,description,relatedDummy.name,relatedDummy.name[],relatedDummies,relatedDummies[],dummy,relatedDummies.name,order[id],order[name],order[relatedDummy.symfony],dummyDate[before],dummyDate[after],relatedDummy.dummyDate[before],relatedDummy.dummyDate[after],dummyFloat[between],dummyFloat[gt],dummyFloat[gte],dummyFloat[lt],dummyFloat[lte],dummyPrice[between],dummyPrice[gt],dummyPrice[gte],dummyPrice[lt],dummyPrice[lte],dummyBoolean,dummyFloat,dummyPrice}",
         "hydra:variableRepresentation": "BasicRepresentation",
         "hydra:mapping": [
           {
@@ -186,6 +186,12 @@ Feature: Create-Retrieve-Update-Delete
             "property": "dummy",
             "required": false
           },
+          {
+            "@type": "IriTemplateMapping",
+            "variable": "relatedDummies.name",
+            "property": "relatedDummies.name",
+            "required": false
+          },
           {
             "@type": "IriTemplateMapping",
             "variable": "order[id]",
diff --git a/src/Bridge/Doctrine/Orm/Extension/FilterEagerLoadingExtension.php b/src/Bridge/Doctrine/Orm/Extension/FilterEagerLoadingExtension.php
@@ -0,0 +1,141 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace ApiPlatform\Core\Bridge\Doctrine\Orm\Extension;
+
+use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
+use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
+use Doctrine\ORM\Mapping\ClassMetadataInfo;
+use Doctrine\ORM\Query\Expr\Join;
+use Doctrine\ORM\QueryBuilder;
+
+/**
+ * Fixes filters on OneToMany associations
+ * https://github.com/api-platform/core/issues/944.
+ */
+final class FilterEagerLoadingExtension implements QueryCollectionExtensionInterface
+{
+    private $resourceMetadataFactory;
+    private $forceEager;
+
+    public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFactory, $forceEager = true)
+    {
+        $this->resourceMetadataFactory = $resourceMetadataFactory;
+        $this->forceEager = $forceEager;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function applyToCollection(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, string $operationName = null)
+    {
+        if (false === $this->isForceEager($resourceClass, ['collection_operation_name' => $operationName])) {
+            return;
+        }
+
+        $entityManager = $queryBuilder->getEntityManager();
+        $classMetadata = $entityManager->getClassMetadata($resourceClass);
+
+        //If no where part, nothing to do
+        $wherePart = $queryBuilder->getDQLPart('where');
+
+        if (!$wherePart) {
+            return;
+        }
+
+        $whereParts = $wherePart->getParts();
+        $joinParts = $queryBuilder->getDQLPart('join');
+
+        //Keep a clone of the current queryBuilder to avoid alteration if it's not necessary
+        $queryBuilderClone = clone $queryBuilder;
+        $queryBuilderClone->resetDQLPart('where');
+
+        $repository = $entityManager->getRepository($resourceClass);
+
+        $altered = 0;
+
+        foreach ($joinParts as $i => $joins) {
+            foreach ($joins as $j => $join) {
+                $fieldName = explode('.', $join->getJoin())[1];
+                $joinAlias = $join->getAlias();
+
+                $mapping = $classMetadata->getAssociationMapping($fieldName);
+
+                //Not a OneToMany join
+                if ($mapping['type'] !== ClassMetadataInfo::ONE_TO_MANY) {
+                    break;
+                }
+
+                //Create a query that starts from the target entity
+                $qb = $entityManager->getRepository($mapping['targetEntity'])->createQueryBuilder($joinAlias);
+
+                //Find where parts that belong to the sub query
+                $qbWhereParts = [];
+                foreach ($whereParts as $key => $where) {
+                    if (false === strpos($where, $joinAlias)) {
+                        continue;
+                    }
+
+                    $qbWhereParts[] = $where;
+                    unset($whereParts[$key]);
+                }
+
+                if (0 === count($qbWhereParts)) {
+                    break;
+                }
+
+                //We did alter the query
+                ++$altered;
+
+                $qb->select("IDENTITY($joinAlias.{$mapping['mappedBy']})");
+
+                foreach ($qbWhereParts as $where) {
+                    $qb->add('where', $where);
+                }
+
+                $queryBuilderClone->andWhere($queryBuilderClone->expr()->in('o', str_replace($joinAlias, $queryNameGenerator->generateJoinAlias($joinAlias), $qb->getDQL())));
+            }
+        }
+
+        if (0 === $altered) {
+            return;
+        }
+
+        //It should be altered, create the WHERE clause again
+        $queryBuilder->resetDQLPart('where');
+        foreach ($queryBuilderClone->getDQLPart('where')->getParts() as $wherePart) {
+            $queryBuilder->add('where', $wherePart);
+        }
+    }
+
+    /**
+     * Does an operation force eager?
+     *
+     * @param string $resourceClass
+     * @param array  $options
+     *
+     * @return bool
+     */
+    private function isForceEager(string $resourceClass, array $options): bool
+    {
+        $resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
+
+        if (isset($options['collection_operation_name'])) {
+            $forceEager = $resourceMetadata->getCollectionOperationAttribute($options['collection_operation_name'], 'force_eager', null, true);
+        } elseif (isset($options['item_operation_name'])) {
+            $forceEager = $resourceMetadata->getItemOperationAttribute($options['item_operation_name'], 'force_eager', null, true);
+        } else {
+            $forceEager = $resourceMetadata->getAttribute('force_eager');
+        }
+
+        return is_bool($forceEager) ? $forceEager : $this->forceEager;
+    }
+}
diff --git a/src/Bridge/Symfony/Bundle/Resources/config/doctrine_orm.xml b/src/Bridge/Symfony/Bundle/Resources/config/doctrine_orm.xml
@@ -105,6 +105,15 @@
             <tag name="api_platform.doctrine.orm.query_extension.collection" priority="32" />
         </service>
 
+        <!-- This needs to be executed right after the filter extension -->
+
+        <service id="api_platform.doctrine.orm.query_extension.filter_eager_loading" class="ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\FilterEagerLoadingExtension" public="false">
+            <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
+            <argument>%api_platform.eager_loading.force_eager%</argument>
+
+            <tag name="api_platform.doctrine.orm.query_extension.collection" priority="31" />
+        </service>
+
         <service id="api_platform.doctrine.orm.query_extension.pagination" class="ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\PaginationExtension" public="false">
             <argument type="service" id="doctrine" />
             <argument type="service" id="request_stack" />
diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php
@@ -248,6 +248,7 @@ private function getContainerBuilderProphecy()
             'api_platform.doctrine.orm.order_filter',
             'api_platform.doctrine.orm.query_extension.eager_loading',
             'api_platform.doctrine.orm.query_extension.filter',
+            'api_platform.doctrine.orm.query_extension.filter_eager_loading',
             'api_platform.doctrine.orm.query_extension.order',
             'api_platform.doctrine.orm.query_extension.pagination',
             'api_platform.doctrine.orm.range_filter',
diff --git a/tests/Fixtures/TestBundle/Entity/Bar.php b/tests/Fixtures/TestBundle/Entity/Bar.php
diff --git a/tests/Fixtures/TestBundle/Entity/Foo.php b/tests/Fixtures/TestBundle/Entity/Foo.php
diff --git a/tests/Fixtures/app/config/config.yml b/tests/Fixtures/app/config/config.yml
