fix: search on nested sub-entity that doesn't use "id" as its ORM ide…

…ntifier
api-platform · Jun 10, 2023 · f876199 · f876199
1 parent 5f8e4a8
commit f876199
Show file tree

Hide file tree

Showing 12 changed files with 344 additions and 4 deletions.
diff --git a/features/doctrine/search_filter.feature b/features/doctrine/search_filter.feature
@@ -1024,3 +1024,12 @@ Feature: Search filter on collections
     Then the response status code should be 200
     And the response should be in JSON
     And the JSON node "hydra:totalItems" should be equal to 1
+
+  @!mongodb
+  @createSchema
+  Scenario: Search on nested sub-entity that doesn't use "id" as its ORM identifier
+    Given there is a dummy entity with a sub entity with id "stringId" and name "someName"
+    When I send a "GET" request to "/dummy_with_subresource?subEntity=/dummy_subresource/stringId"
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the JSON node "hydra:totalItems" should be equal to 1
diff --git a/src/Doctrine/Common/Filter/SearchFilterTrait.php b/src/Doctrine/Common/Filter/SearchFilterTrait.php
@@ -124,7 +124,8 @@ protected function getIdFromValue(string $value): mixed
             $iriConverter = $this->getIriConverter();
             $item = $iriConverter->getResourceFromIri($value, ['fetch_data' => false]);
 
-            return $this->getPropertyAccessor()->getValue($item, 'id');
+            $identifiers = $this->identifiersExtractor->getIdentifiersFromItem($item);
+            return  1 === \count($identifiers) ? array_pop($identifiers) : $identifiers;
         } catch (InvalidArgumentException) {
             // Do nothing, return the raw value
         }

diff --git a/src/Doctrine/Orm/Filter/SearchFilter.php b/src/Doctrine/Orm/Filter/SearchFilter.php
@@ -41,7 +41,7 @@ final class SearchFilter extends AbstractFilter implements SearchFilterInterface
 
     public const DOCTRINE_INTEGER_TYPE = Types::INTEGER;
 
-    public function __construct(ManagerRegistry $managerRegistry, IriConverterInterface $iriConverter, PropertyAccessorInterface $propertyAccessor = null, LoggerInterface $logger = null, array $properties = null, IdentifiersExtractorInterface $identifiersExtractor = null, NameConverterInterface $nameConverter = null)
+    public function __construct(ManagerRegistry $managerRegistry, IriConverterInterface $iriConverter, PropertyAccessorInterface $propertyAccessor = null, LoggerInterface $logger = null, array $properties = null, IdentifiersExtractorInterface $identifiersExtractor, NameConverterInterface $nameConverter = null)
     {
         parent::__construct($managerRegistry, $logger, $properties, $nameConverter);
 
@@ -100,6 +100,7 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
         if ($metadata->hasField($field)) {
             if ('id' === $field) {
                 $values = array_map($this->getIdFromValue(...), $values);
+                // todo: handle composite IDs
             }
 
             if (!$this->hasValidValues($values, $this->getDoctrineFieldType($property, $resourceClass))) {
@@ -121,9 +122,11 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
         }
 
         $values = array_map($this->getIdFromValue(...), $values);
+        // todo: handle composite IDs
 
         $associationResourceClass = $metadata->getAssociationTargetClass($field);
-        $associationFieldIdentifier = $metadata->getIdentifierFieldNames()[0];
+        $associationMetadata = $this->getClassMetadata($associationResourceClass);
+        $associationFieldIdentifier = $associationMetadata->getIdentifierFieldNames()[0];
         $doctrineTypeField = $this->getDoctrineFieldType($associationFieldIdentifier, $associationResourceClass);
 
         if (!$this->hasValidValues($values, $doctrineTypeField)) {

diff --git a/src/Symfony/Bundle/Resources/config/doctrine_orm.xml b/src/Symfony/Bundle/Resources/config/doctrine_orm.xml
@@ -152,7 +152,7 @@
             <argument type="service" id="api_platform.iri_converter" />
             <argument type="service" id="api_platform.property_accessor" />
             <argument type="service" id="logger" on-invalid="ignore" />
-            <argument key="$identifiersExtractor" type="service" id="api_platform.identifiers_extractor.cached" on-invalid="ignore" />
+            <argument key="$identifiersExtractor" type="service" id="api_platform.identifiers_extractor" />
             <argument key="$nameConverter" type="service" id="api_platform.name_converter" on-invalid="ignore" />
         </service>
         <service id="ApiPlatform\Doctrine\Orm\Filter\SearchFilter" alias="api_platform.doctrine.orm.search_filter" />

diff --git a/tests/Behat/DoctrineContext.php b/tests/Behat/DoctrineContext.php
@@ -128,8 +128,10 @@
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyPassenger;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyProduct;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyProperty;
+use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummySubEntity;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyTableInheritanceNotApiResourceChild;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyTravel;
+use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyWithSubEntity;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\EmbeddableDummy;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\EmbeddedDummy;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\EntityClassWithDateTime;
@@ -2139,6 +2141,20 @@ public function thereIsAResourceUsingEntityClassAndDateTime(): void
         $this->manager->flush();
     }
 
+    /**
+     * @Given there is a dummy entity with a sub entity with id :strId and name :name
+     */
+    public function thereIsADummyWithSubEntity(string $strId, string $name): void
+    {
+        $subEntity = new DummySubEntity($strId, $name);
+        $mainEntity = new DummyWithSubEntity();
+        $mainEntity->setSubEntity($subEntity);
+        $mainEntity->setName('main');
+        $this->manager->persist($subEntity);
+        $this->manager->persist($mainEntity);
+        $this->manager->flush();
+    }
+
     private function isOrm(): bool
     {
         return null !== $this->schemaTool;

diff --git a/tests/Fixtures/TestBundle/ApiResource/Issue5605/MainResource.php b/tests/Fixtures/TestBundle/ApiResource/Issue5605/MainResource.php
@@ -0,0 +1,41 @@
+<?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.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue5605;
+
+use ApiPlatform\Doctrine\Orm\Filter\SearchFilter;
+use ApiPlatform\Doctrine\Orm\State\Options;
+use ApiPlatform\Metadata\ApiFilter;
+use ApiPlatform\Metadata\ApiProperty;
+use ApiPlatform\Metadata\ApiResource;
+use ApiPlatform\Metadata\Get;
+use ApiPlatform\Metadata\GetCollection;
+use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyWithSubEntity;
+use ApiPlatform\Tests\Fixtures\TestBundle\State\Issue5605\MainResourceProvider;
+
+#[ApiResource(
+    operations  : [
+        new Get(uriTemplate: '/dummy_with_subresource/{id}', uriVariables: ['id']),
+        new GetCollection(uriTemplate: '/dummy_with_subresource')
+    ],
+    provider    : MainResourceProvider::class,
+    stateOptions: new Options(entityClass: DummyWithSubEntity::class)
+)]
+#[ApiFilter(SearchFilter::class, properties: ['subEntity'])]
+class MainResource
+{
+    #[ApiProperty(identifier: true)]
+    public int $id;
+    public string $name;
+    public SubResource $subResource;
+}
diff --git a/tests/Fixtures/TestBundle/ApiResource/Issue5605/SubResource.php b/tests/Fixtures/TestBundle/ApiResource/Issue5605/SubResource.php
@@ -0,0 +1,39 @@
+<?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.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue5605;
+
+use ApiPlatform\Doctrine\Orm\State\Options;
+use ApiPlatform\Metadata\ApiProperty;
+use ApiPlatform\Metadata\ApiResource;
+use ApiPlatform\Metadata\Get;
+use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummySubEntity;
+use ApiPlatform\Tests\Fixtures\TestBundle\State\Issue5605\SubResourceProvider;
+
+#[ApiResource(
+    operations  : [
+        new Get(
+            uriTemplate: '/dummy_subresource/{strId}',
+            uriVariables: ['strId']
+        )
+    ],
+    provider: SubResourceProvider::class,
+    stateOptions: new Options(entityClass: DummySubEntity::class)
+)]
+class SubResource
+{
+    #[ApiProperty(identifier: true)]
+    public string $strId;
+
+    public string $name;
+}
diff --git a/tests/Fixtures/TestBundle/Entity/DummySubEntity.php b/tests/Fixtures/TestBundle/Entity/DummySubEntity.php
@@ -0,0 +1,61 @@
+<?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.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity;
+
+use Doctrine\ORM\Mapping as ORM;
+
+#[ORM\Entity]
+class DummySubEntity
+{
+    #[ORM\Id]
+    #[ORM\Column(type: 'string')]
+    private string $strId;
+
+    #[ORM\Column]
+    private string $name;
+
+    #[ORM\OneToOne(inversedBy: 'subEntity', cascade: ['persist'])]
+    private ?DummyWithSubEntity $mainEntity = null;
+
+    public function __construct($strId, $name)
+    {
+        $this->strId = $strId;
+        $this->name = $name;
+    }
+
+    public function getStrId(): string
+    {
+        return $this->strId;
+    }
+
+    public function getMainEntity(): ?DummyWithSubEntity
+    {
+        return $this->mainEntity;
+    }
+
+    public function setMainEntity(DummyWithSubEntity $mainEntity): void
+    {
+        $this->mainEntity = $mainEntity;
+    }
+
+    public function getName(): string
+    {
+        return $this->name;
+    }
+
+    public function setName(string $name): void
+    {
+        $this->name = $name;
+    }
+}
diff --git a/tests/Fixtures/TestBundle/Entity/DummyWithSubEntity.php b/tests/Fixtures/TestBundle/Entity/DummyWithSubEntity.php
@@ -0,0 +1,60 @@
+<?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.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity;
+
+use Doctrine\ORM\Mapping as ORM;
+
+#[ORM\Entity]
+class DummyWithSubEntity
+{
+    #[ORM\Id]
+    #[ORM\Column(type: 'integer')]
+    #[ORM\GeneratedValue(strategy: 'AUTO')]
+    private int $id;
+
+    #[ORM\Column]
+    private string $name;
+
+    #[ORM\OneToOne(mappedBy: 'mainEntity', cascade: ['persist'], fetch: 'EAGER')]
+    private ?DummySubEntity $subEntity = null;
+
+    public function getId(): int
+    {
+        return $this->id;
+    }
+
+    public function getName(): string
+    {
+        return $this->name;
+    }
+
+    public function setName(string $name): void
+    {
+        $this->name = $name;
+    }
+
+    public function getSubEntity(): ?DummySubEntity
+    {
+        return $this->subEntity;
+    }
+
+    public function setSubEntity(?DummySubEntity $subEntity): void
+    {
+        if (null !== $subEntity && $subEntity->getMainEntity() !== $this) {
+            $subEntity->setMainEntity($this);
+        }
+
+        $this->subEntity = $subEntity;
+    }
+}
diff --git a/tests/Fixtures/TestBundle/State/Issue5605/MainResourceProvider.php b/tests/Fixtures/TestBundle/State/Issue5605/MainResourceProvider.php
@@ -0,0 +1,56 @@
+<?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.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Tests\Fixtures\TestBundle\State\Issue5605;
+
+use ApiPlatform\Metadata\Get;
+use ApiPlatform\Metadata\Operation;
+use ApiPlatform\State\ProviderInterface;
+use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue5605\MainResource;
+use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue5605\SubResource;
+use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyWithSubEntity;
+
+class MainResourceProvider implements ProviderInterface
+{
+    public function __construct(private readonly ProviderInterface $itemProvider, private readonly ProviderInterface $collectionProvider)
+    {
+    }
+
+    public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null
+    {
+        if ($operation instanceof Get) {
+            /**
+             * @var DummyWithSubEntity $entity
+             */
+            $entity = $this->itemProvider->provide($operation, $uriVariables, $context);
+            return $this->getResource($entity);
+        }
+        $resources = [];
+        $entities = $this->collectionProvider->provide($operation, $uriVariables, $context);
+        foreach($entities as $entity) {
+            $resources[] =  $this->getResource($entity);
+        }
+        return $resources;
+    }
+
+    protected function getResource(DummyWithSubEntity $entity): MainResource
+    {
+        $resource = new MainResource();
+        $resource->name = $entity->getName();
+        $resource->id = $entity->getId();
+        $resource->subResource = new SubResource();
+        $resource->subResource->name = $entity->getSubEntity()->getName();
+        $resource->subResource->strId = $entity->getSubEntity()->getStrId();
+        return $resource;
+    }
+}
diff --git a/tests/Fixtures/TestBundle/State/Issue5605/SubResourceProvider.php b/tests/Fixtures/TestBundle/State/Issue5605/SubResourceProvider.php
@@ -0,0 +1,39 @@
+<?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.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\Tests\Fixtures\TestBundle\State\Issue5605;
+
+use ApiPlatform\Metadata\Operation;
+use ApiPlatform\State\ProviderInterface;
+use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue5605\SubResource;
+use ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummySubEntity;
+
+class SubResourceProvider implements ProviderInterface
+{
+
+    public function __construct(private readonly ProviderInterface $itemProvider)
+    {
+    }
+
+    public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null
+    {
+        /**
+         * @var DummySubEntity $entity
+         */
+        $entity = $this->itemProvider->provide($operation, $uriVariables, $context);
+        $resource = new SubResource();
+        $resource->strId = $entity->getStrId();
+        $resource->name = $entity->getName();
+        return $resource;
+    }
+}