Merge pull request #504 from soyuka/fix-composite-identifier-collection

Fix and test composite identifier collection retrieval

features/bootstrap/FeatureContext.php

@@ -9,6 +9,9 @@
  * file that was distributed with this source code.
  */
 
+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\RelatedDummy;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelationEmbedder;
@@ -226,4 +229,29 @@ public function thereIsARelationEmbedderObject()
         $this->manager->persist($relationEmbedder);
         $this->manager->flush();
     }
+
+    /**
+     * @Given there are Composite identifier objects
+     */
+    public function thereIsACompositeIdentifierObject()
+    {
+        $item = new CompositeItem();
+        $item->setField1('foobar');
+        $this->manager->persist($item);
+
+        for ($i = 0; $i < 4; $i++) {
+            $label = new CompositeLabel();
+            $label->setValue('foo-'.$i);
+
+            $rel = new CompositeRelation();
+            $rel->setCompositeLabel($label);
+            $rel->setCompositeItem($item);
+            $rel->setValue('somefoobardummy');
+
+            $this->manager->persist($label);
+            $this->manager->persist($rel);
+        }
+
+        $this->manager->flush();
+    }
 }

features/composite.feature

@@ -0,0 +1,80 @@
+Feature: Retrieve data with Composite identifiers
+  In order to retrieve relations with composite identifiers
+  As a client software developer
+  I need to retrieve all collections 
+
+  @createSchema
+  @dropSchema
+  Scenario: Get collection with composite identifiers
+    Given there are Composite identifier objects
+    When I send a "GET" request to "/composite_items"
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/ld+json"
+    And the JSON should be equal to:
+    """
+    {
+         "@context": "/contexts/CompositeItem",
+         "@id": "/composite_items",
+         "@type": "hydra:Collection",
+         "hydra:member": [
+             {
+                 "@id": "/composite_items/1",
+                 "@type": "CompositeItem",
+                 "field1": "foobar",
+                 "compositeValues": [
+                     "/composite_relations/1-1",
+                     "/composite_relations/1-2",
+                     "/composite_relations/1-3",
+                     "/composite_relations/1-4"
+                 ]
+             }
+         ],
+         "hydra:totalItems": 1
+    }
+    """
+
+  @createSchema
+  @dropSchema
+  Scenario: Get collection with composite identifiers
+    Given there are Composite identifier objects
+    When I send a "GET" request to "/composite_relations"
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/ld+json"
+    And the JSON should be equal to:
+    """
+    {
+        "@context": "\/contexts\/CompositeRelation",
+        "@id": "\/composite_relations",
+        "@type": "hydra:Collection",
+        "hydra:member": [
+            {
+                "@id": "\/composite_relations\/1-1",
+                "@type": "CompositeRelation",
+                "id": "1-1",
+                "value": "somefoobardummy"
+            },
+            {
+                "@id": "\/composite_relations\/1-2",
+                "@type": "CompositeRelation",
+                "id": "1-2",
+                "value": "somefoobardummy"
+            },
+            {
+                "@id": "\/composite_relations\/1-3",
+                "@type": "CompositeRelation",
+                "id": "1-3",
+                "value": "somefoobardummy"
+            }
+        ],
+        "hydra:totalItems": 4,
+        "hydra:view": {
+            "@id": "\/composite_relations?page=1",
+            "@type": "hydra:PartialCollectionView",
+            "hydra:first": "\/composite_relations?page=1",
+            "hydra:last": "\/composite_relations?page=2",
+            "hydra:next": "\/composite_relations?page=2"
+        }
+    }
+    """

features/json-ld/context.feature

@@ -24,7 +24,10 @@ Feature: JSON-LD contexts generation
       "relatedDummy": "/related_dummies",
       "relationEmbedder": "/relation_embedders",
       "thirdLevel": "/third_levels",
-      "user": "/users"
+      "user": "/users",
+      "compositeItem": "/composite_items",
+      "compositeLabel": "/composite_labels",
+      "compositeRelation": "/composite_relations"
     }
     """
 

src/Bridge/Doctrine/Orm/Extension/PaginationExtension.php

@@ -99,7 +99,7 @@ public function supportsResult(string $resourceClass, string $operationName = nu
      */
     public function getResult(QueryBuilder $queryBuilder)
     {
-        $doctrineOrmPaginator = new DoctrineOrmPaginator($queryBuilder);
+        $doctrineOrmPaginator = new DoctrineOrmPaginator($queryBuilder, $this->useFetchJoinCollection($queryBuilder));
         $doctrineOrmPaginator->setUseOutputWalkers($this->useOutputWalkers($queryBuilder));
 
         return new Paginator($doctrineOrmPaginator);
@@ -117,6 +117,20 @@ private function isPaginationEnabled(Request $request, ResourceMetadata $resourc
         return $enabled;
     }
 
+    /**
+     * Determines whether the Paginator should fetch join collections, if the root entity uses composite identifiers it should not.
+     *
+     * @see https://github.com/doctrine/doctrine2/issues/2910
+     *
+     * @param QueryBuilder $queryBuilder
+     *
+     * @return bool
+     */
+    private function useFetchJoinCollection(QueryBuilder $queryBuilder): bool
+    {
+        return !QueryChecker::hasRootEntityWithCompositeIdentifier($queryBuilder, $this->managerRegistry);
+    }
+
     /**
      * Determines whether output walkers should be used.
      *
@@ -156,6 +170,13 @@ private function useOutputWalkers(QueryBuilder $queryBuilder) : bool
             return true;
         }
 
+        /*
+         * When using composite identifiers pagination will need Output walkers
+         */
+        if (QueryChecker::hasRootEntityWithCompositeIdentifier($queryBuilder, $this->managerRegistry)) {
+            return true;
+        }
+
         // Disable output walkers by default (performance)
         return false;
     }

src/Bridge/Doctrine/Orm/Util/QueryChecker.php

@@ -57,6 +57,29 @@ public static function hasRootEntityWithForeignKeyIdentifier(QueryBuilder $query
         return false;
     }
 
+    /**
+     * Determines whether the query builder has any composite identifier.
+     *
+     * @param QueryBuilder    $queryBuilder
+     * @param ManagerRegistry $managerRegistry
+     *
+     * @return bool
+     */
+    public static function hasRootEntityWithCompositeIdentifier(QueryBuilder $queryBuilder, ManagerRegistry $managerRegistry) : bool
+    {
+        foreach ($queryBuilder->getRootEntities() as $rootEntity) {
+            $rootMetadata = $managerRegistry
+                ->getManagerForClass($rootEntity)
+                ->getClassMetadata($rootEntity);
+
+            if ($rootMetadata->isIdentifierComposite) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     /**
      * Determines whether the query builder has the maximum number of results specified.
      *

tests/Fixtures/TestBundle/Entity/CompositeItem.php

@@ -0,0 +1,87 @@
+<?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\Tests\Fixtures\TestBundle\Entity;
+
+use ApiPlatform\Core\Annotation\Resource;
+use Doctrine\ORM\Mapping as ORM;
+use Symfony\Component\Serializer\Annotation\Groups;
+
+/**
+ * @Resource
+ * @ORM\Entity
+ */
+class CompositeItem
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column(type="integer")
+     * @ORM\GeneratedValue(strategy="AUTO")
+     */
+    private $id;
+
+    /**
+     * @ORM\Column(type="string", nullable=true)
+     * @Groups({"default"})
+     */
+    private $field1;
+
+    /**
+     * @ORM\OneToMany(targetEntity="CompositeRelation", mappedBy="compositeItem", fetch="EAGER")
+     * @Groups({"default"})
+     */
+    private $compositeValues;
+
+    /**
+     * Get id.
+     *
+     * @return id.
+     */
+    public function getId()
+    {
+        return $this->id;
+    }
+
+    /**
+     * Get field1.
+     *
+     * @return field1.
+     */
+    public function getField1()
+    {
+        return $this->field1;
+    }
+
+    /**
+     * Set field1.
+     *
+     * @param field1 the value to set.
+     */
+    public function setField1($field1)
+    {
+        $this->field1 = $field1;
+    }
+
+    /**
+     * Get compositeValues.
+     *
+     * @return compositeValues.
+     */
+    public function getCompositeValues()
+    {
+        return $this->compositeValues;
+    }
+
+    public function __toString()
+    {
+        return (string) $this->id;
+    }
+}

tests/Fixtures/TestBundle/Entity/CompositeLabel.php

@@ -0,0 +1,71 @@
+<?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\Tests\Fixtures\TestBundle\Entity;
+
+use ApiPlatform\Core\Annotation\Resource;
+use Doctrine\ORM\Mapping as ORM;
+use Symfony\Component\Serializer\Annotation\Groups;
+
+/**
+ * @ORM\Entity
+ * @Resource
+ */
+class CompositeLabel
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column(type="integer")
+     * @ORM\GeneratedValue(strategy="AUTO")
+     */
+    private $id;
+
+    /**
+     * @ORM\Column(type="string", nullable=true)
+     * @Groups({"default"})
+     */
+    private $value;
+
+    /**
+     * Get id.
+     *
+     * @return id.
+     */
+    public function getId()
+    {
+        return $this->id;
+    }
+
+    /**
+     * Get value.
+     *
+     * @return value.
+     */
+    public function getValue()
+    {
+        return $this->value;
+    }
+
+    /**
+     * Set value.
+     *
+     * @param value the value to set.
+     */
+    public function setValue($value)
+    {
+        $this->value = $value;
+    }
+
+    public function __toString()
+    {
+        return (string) $this->id;
+    }
+}