From 54977b5d6f3b68ec1f381d3a9264a45f6f5c942b Mon Sep 17 00:00:00 2001 From: Manuel Rossard Date: Thu, 15 Jun 2023 14:07:10 +0200 Subject: [PATCH] fix: search on nested sub-entity that doesn't use "id" as its ORM identifier --- features/doctrine/search_filter.feature | 9 +++ .../Common/Filter/SearchFilterTrait.php | 8 ++- src/Doctrine/Orm/Filter/SearchFilter.php | 5 +- .../Resources/config/doctrine_mongodb_odm.xml | 2 +- .../Bundle/Resources/config/doctrine_orm.xml | 2 +- tests/Behat/DoctrineContext.php | 16 +++++ .../ApiResource/Issue5605/MainResource.php | 41 +++++++++++++ .../ApiResource/Issue5605/SubResource.php | 39 ++++++++++++ .../TestBundle/Entity/DummySubEntity.php | 61 +++++++++++++++++++ .../TestBundle/Entity/DummyWithSubEntity.php | 60 ++++++++++++++++++ .../State/Issue5605/MainResourceProvider.php | 59 ++++++++++++++++++ .../State/Issue5605/SubResourceProvider.php | 39 ++++++++++++ tests/Fixtures/app/config/config_doctrine.yml | 15 +++++ 13 files changed, 352 insertions(+), 4 deletions(-) create mode 100644 tests/Fixtures/TestBundle/ApiResource/Issue5605/MainResource.php create mode 100644 tests/Fixtures/TestBundle/ApiResource/Issue5605/SubResource.php create mode 100644 tests/Fixtures/TestBundle/Entity/DummySubEntity.php create mode 100644 tests/Fixtures/TestBundle/Entity/DummyWithSubEntity.php create mode 100644 tests/Fixtures/TestBundle/State/Issue5605/MainResourceProvider.php create mode 100644 tests/Fixtures/TestBundle/State/Issue5605/SubResourceProvider.php diff --git a/features/doctrine/search_filter.feature b/features/doctrine/search_filter.feature index 13128f882f8..2b8502fa493 100644 --- 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 index 01af4e5c88a..e07a201046c 100644 --- a/src/Doctrine/Common/Filter/SearchFilterTrait.php +++ b/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 } diff --git a/src/Doctrine/Orm/Filter/SearchFilter.php b/src/Doctrine/Orm/Filter/SearchFilter.php index b4bfb6e09d2..3ae351fec19 100644 --- a/src/Doctrine/Orm/Filter/SearchFilter.php +++ b/src/Doctrine/Orm/Filter/SearchFilter.php @@ -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_mongodb_odm.xml b/src/Symfony/Bundle/Resources/config/doctrine_mongodb_odm.xml index 126995326d5..8abdb8f9745 100644 --- a/src/Symfony/Bundle/Resources/config/doctrine_mongodb_odm.xml +++ b/src/Symfony/Bundle/Resources/config/doctrine_mongodb_odm.xml @@ -116,7 +116,7 @@ - + diff --git a/src/Symfony/Bundle/Resources/config/doctrine_orm.xml b/src/Symfony/Bundle/Resources/config/doctrine_orm.xml index 112c4f6d252..6318863fd24 100644 --- a/src/Symfony/Bundle/Resources/config/doctrine_orm.xml +++ b/src/Symfony/Bundle/Resources/config/doctrine_orm.xml @@ -152,7 +152,7 @@ - + diff --git a/tests/Behat/DoctrineContext.php b/tests/Behat/DoctrineContext.php index f1b3d14b0a6..a271b8d895b 100644 --- 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 new file mode 100644 index 00000000000..d92d86e961e --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/Issue5605/MainResource.php @@ -0,0 +1,41 @@ + + * + * 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 new file mode 100644 index 00000000000..9ad526cbb5b --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/Issue5605/SubResource.php @@ -0,0 +1,39 @@ + + * + * 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 new file mode 100644 index 00000000000..1743843c299 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/DummySubEntity.php @@ -0,0 +1,61 @@ + + * + * 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 new file mode 100644 index 00000000000..ebd9344c783 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/DummyWithSubEntity.php @@ -0,0 +1,60 @@ + + * + * 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 new file mode 100644 index 00000000000..bfea5439774 --- /dev/null +++ b/tests/Fixtures/TestBundle/State/Issue5605/MainResourceProvider.php @@ -0,0 +1,59 @@ + + * + * 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 new file mode 100644 index 00000000000..0054635f2f8 --- /dev/null +++ b/tests/Fixtures/TestBundle/State/Issue5605/SubResourceProvider.php @@ -0,0 +1,39 @@ + + * + * 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; + } +} diff --git a/tests/Fixtures/app/config/config_doctrine.yml b/tests/Fixtures/app/config/config_doctrine.yml index 8045ddaae50..7a00a296db0 100644 --- a/tests/Fixtures/app/config/config_doctrine.yml +++ b/tests/Fixtures/app/config/config_doctrine.yml @@ -120,3 +120,18 @@ services: arguments: $itemProvider: '@ApiPlatform\Doctrine\Orm\State\ItemProvider' $collectionProvider: '@ApiPlatform\Doctrine\Orm\State\CollectionProvider' + + ApiPlatform\Tests\Fixtures\TestBundle\State\Issue5605\MainResourceProvider: + class: 'ApiPlatform\Tests\Fixtures\TestBundle\State\Issue5605\MainResourceProvider' + tags: + - name: 'api_platform.state_provider' + arguments: + $itemProvider: '@ApiPlatform\Doctrine\Orm\State\ItemProvider' + $collectionProvider: '@ApiPlatform\Doctrine\Orm\State\CollectionProvider' + + ApiPlatform\Tests\Fixtures\TestBundle\State\Issue5605\SubResourceProvider: + class: 'ApiPlatform\Tests\Fixtures\TestBundle\State\Issue5605\SubResourceProvider' + tags: + - name: 'api_platform.state_provider' + arguments: + $itemProvider: '@ApiPlatform\Doctrine\Orm\State\ItemProvider'