fix(doctrine): searchfilter with nested custom identifiers (#5760)

features/doctrine/search_filter.feature

@@ -1033,3 +1033,22 @@ 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
+
+  @!mongodb
+  @createSchema
+  Scenario: Filters can use UUIDs
+    Given there is a group object with uuid "61817181-0ecc-42fb-a6e7-d97f2ddcb344" and 2 users
+    And there is a group object with uuid "32510d53-f737-4e70-8d9d-58e292c871f8" and 1 users
+    When I send a "GET" request to "/issue5735/issue5735_users?groups[]=/issue5735/groups/61817181-0ecc-42fb-a6e7-d97f2ddcb344&groups[]=/issue5735/groups/32510d53-f737-4e70-8d9d-58e292c871f8"
+    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 3

src/Doctrine/Common/Filter/SearchFilterTrait.php

@@ -124,7 +124,13 @@ protected function getIdFromValue(string $value): mixed
             $iriConverter = $this->getIriConverter();
             $item = $iriConverter->getResourceFromIri($value, ['fetch_data' => false]);
 
-            return $this->getPropertyAccessor()->getValue($item, 'id');
+            if (null === $this->identifiersExtractor) {
+                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
         }

src/Doctrine/Orm/Filter/SearchFilter.php

@@ -196,6 +196,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))) {
@@ -216,13 +217,44 @@ protected function filterProperty(string $property, $value, QueryBuilder $queryB
             return;
         }
 
-        $values = array_map($this->getIdFromValue(...), $values);
-
+        // association, let's fetch the entity (or reference to it) if we can so we can make sure we get its orm id
         $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)) {
+        $values = array_map(function ($value) use ($associationFieldIdentifier, $doctrineTypeField) {
+            if (is_numeric($value)) {
+                return $value;
+            }
+            try {
+                $item = $this->getIriConverter()->getResourceFromIri($value, ['fetch_data' => false]);
+
+                return $this->propertyAccessor->getValue($item, $associationFieldIdentifier);
+            } catch (InvalidArgumentException) {
+                /*
+                 * Can we do better? This is not the ApiResource the call was made on,
+                 * so we don't get any kind of api metadata for it without (a lot of?) work elsewhere...
+                 * Let's just pretend it's always the ORM id for now.
+                 */
+                if (!$this->hasValidValues([$value], $doctrineTypeField)) {
+                    $this->logger->notice('Invalid filter ignored', [
+                        'exception' => new InvalidArgumentException(sprintf('Values for field "%s" are not valid according to the doctrine type.', $associationFieldIdentifier)),
+                    ]);
+
+                    return null;
+                }
+
+                return $value;
+            }
+        }, $values);
+
+        $expected = \count($values);
+        $values = array_filter($values, static fn ($value) => null !== $value);
+        if ($expected > \count($values)) {
+            /*
+             * Shouldn't this actually fail harder?
+             */
             $this->logger->notice('Invalid filter ignored', [
                 'exception' => new InvalidArgumentException(sprintf('Values for field "%s" are not valid according to the doctrine type.', $field)),
             ]);

src/Doctrine/Orm/State/ItemProvider.php

@@ -52,7 +52,8 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
         $manager = $this->managerRegistry->getManagerForClass($entityClass);
 
         $fetchData = $context['fetch_data'] ?? true;
-        if (!$fetchData) {
+        if (!$fetchData && \array_key_exists('id', $uriVariables)) {
+            // todo : if uriVariables don't contain the id, this fails. This should behave like it does in the following code
             return $manager->getReference($entityClass, $uriVariables);
         }
 

src/Symfony/Bundle/Resources/config/doctrine_mongodb_odm.xml

@@ -116,7 +116,7 @@
         <service id="api_platform.doctrine_mongodb.odm.search_filter" class="ApiPlatform\Doctrine\Odm\Filter\SearchFilter" public="false" abstract="true">
             <argument type="service" id="doctrine_mongodb" />
             <argument type="service" id="api_platform.iri_converter" />
-            <argument type="service" id="api_platform.identifiers_extractor.cached" on-invalid="ignore" />
+            <argument type="service" id="api_platform.identifiers_extractor" on-invalid="ignore" />
             <argument type="service" id="api_platform.property_accessor" />
             <argument type="service" id="logger" on-invalid="ignore" />
             <argument key="$nameConverter" type="service" id="api_platform.name_converter" on-invalid="ignore"/>

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" on-invalid="ignore" />
             <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" />

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;
@@ -144,6 +146,7 @@
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\IriOnlyDummy;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue5722\Event;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue5722\ItemLog;
+use ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue5735\Group;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\MaxDepthDummy;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\MultiRelationsDummy;
 use ApiPlatform\Tests\Fixtures\TestBundle\Entity\MultiRelationsRelatedDummy;
@@ -2149,6 +2152,43 @@ 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();
+    }
+
+    /**
+     * @Given there is a group object with uuid :uuid and :nbUsers users
+     */
+    public function thereIsAGroupWithUuidAndNUsers(string $uuid, int $nbUsers): void
+    {
+        $group = new Group();
+        $group->setUuid(\Symfony\Component\Uid\Uuid::fromString($uuid));
+
+        $this->manager->persist($group);
+
+        for ($i = 0; $i < $nbUsers; ++$i) {
+            $user = new \ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue5735\Issue5735User();
+            $user->addGroup($group);
+            $this->manager->persist($user);
+        }
+
+        // add another user not in this group
+        $user = new \ApiPlatform\Tests\Fixtures\TestBundle\Entity\Issue5735\Issue5735User();
+        $this->manager->persist($user);
+
+        $this->manager->flush();
+    }
+
     /**
      * @Given there are logs on an event
      */

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 <[email protected]>
+ *
+ * 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;
+}

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 <[email protected]>
+ *
+ * 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;
+}

tests/Fixtures/TestBundle/Entity/DummySubEntity.php

@@ -0,0 +1,61 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * 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;
+    }
+}

tests/Fixtures/TestBundle/Entity/DummyWithSubEntity.php

@@ -0,0 +1,60 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * 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;
+    }
+}