From d1e01ce6536cba2bc852b0f65fd1c04e2eb40ea7 Mon Sep 17 00:00:00 2001 From: Antoine Bluchet Date: Tue, 12 Apr 2016 15:40:52 +0200 Subject: [PATCH] Fix and test composite identifier collection retrieval --- features/bootstrap/FeatureContext.php | 28 +++++ features/composite.feature | 80 ++++++++++++ features/json-ld/context.feature | 5 +- .../Orm/Extension/PaginationExtension.php | 23 +++- src/Bridge/Doctrine/Orm/Util/QueryChecker.php | 23 ++++ .../TestBundle/Entity/CompositeItem.php | 87 +++++++++++++ .../TestBundle/Entity/CompositeLabel.php | 71 +++++++++++ .../TestBundle/Entity/CompositeRelation.php | 115 ++++++++++++++++++ 8 files changed, 430 insertions(+), 2 deletions(-) create mode 100644 features/composite.feature create mode 100644 tests/Fixtures/TestBundle/Entity/CompositeItem.php create mode 100644 tests/Fixtures/TestBundle/Entity/CompositeLabel.php create mode 100644 tests/Fixtures/TestBundle/Entity/CompositeRelation.php diff --git a/features/bootstrap/FeatureContext.php b/features/bootstrap/FeatureContext.php index c43295e8578..4d51a546b86 100644 --- a/features/bootstrap/FeatureContext.php +++ b/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(); + } } diff --git a/features/composite.feature b/features/composite.feature new file mode 100644 index 00000000000..fc2d5632fbe --- /dev/null +++ b/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" + } + } + """ diff --git a/features/json-ld/context.feature b/features/json-ld/context.feature index 8c121ad708d..791d6c641cd 100644 --- a/features/json-ld/context.feature +++ b/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" } """ diff --git a/src/Bridge/Doctrine/Orm/Extension/PaginationExtension.php b/src/Bridge/Doctrine/Orm/Extension/PaginationExtension.php index 415c3d5968f..4350c6f0e44 100644 --- a/src/Bridge/Doctrine/Orm/Extension/PaginationExtension.php +++ b/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; } diff --git a/src/Bridge/Doctrine/Orm/Util/QueryChecker.php b/src/Bridge/Doctrine/Orm/Util/QueryChecker.php index c294ff6e025..dc2f33eb923 100644 --- a/src/Bridge/Doctrine/Orm/Util/QueryChecker.php +++ b/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. * diff --git a/tests/Fixtures/TestBundle/Entity/CompositeItem.php b/tests/Fixtures/TestBundle/Entity/CompositeItem.php new file mode 100644 index 00000000000..50698475945 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/CompositeItem.php @@ -0,0 +1,87 @@ + + * + * 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; + } +} diff --git a/tests/Fixtures/TestBundle/Entity/CompositeLabel.php b/tests/Fixtures/TestBundle/Entity/CompositeLabel.php new file mode 100644 index 00000000000..290b92bd34a --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/CompositeLabel.php @@ -0,0 +1,71 @@ + + * + * 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; + } +} diff --git a/tests/Fixtures/TestBundle/Entity/CompositeRelation.php b/tests/Fixtures/TestBundle/Entity/CompositeRelation.php new file mode 100644 index 00000000000..9be1303b093 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/CompositeRelation.php @@ -0,0 +1,115 @@ + + * + * 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 CompositeRelation +{ + /** + * @ORM\Column(type="string", nullable=true) + * @Groups({"default"}) + */ + private $value; + + /** + * @ORM\Id + * @ORM\ManyToOne(targetEntity="CompositeItem", inversedBy="compositeValues") + * @ORM\JoinColumn(name="composite_item_id", referencedColumnName="id", nullable=false) + * @Groups({"default"}) + */ + private $compositeItem; + + /** + * @ORM\Id + * @ORM\ManyToOne(targetEntity="CompositeLabel") + * @ORM\JoinColumn(name="composite_label_id", referencedColumnName="id", nullable=false) + * @Groups({"default"}) + */ + private $compositeLabel; + + /** + * Get composite id. + * + * @return string + */ + public function getId() + { + return sprintf('%s-%s', $this->compositeItem->getId(), $this->compositeLabel->getId()); + } + + /** + * 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; + } + + /** + * Get compositeItem. + * + * @return compositeItem. + */ + public function getCompositeItem() + { + return $this->compositeItem; + } + + /** + * Set compositeItem. + * + * @param compositeItem the value to set. + */ + public function setCompositeItem($compositeItem) + { + $this->compositeItem = $compositeItem; + } + + /** + * Get compositeLabel. + * + * @return compositeLabel. + */ + public function getCompositeLabel() + { + return $this->compositeLabel; + } + + /** + * Set compositeLabel. + * + * @param compositeLabel the value to set. + */ + public function setCompositeLabel($compositeLabel) + { + $this->compositeLabel = $compositeLabel; + } +}