From 0585dce57846b8525bbec7148cc3d33d8226b1b7 Mon Sep 17 00:00:00 2001 From: soyuka Date: Wed, 23 Nov 2022 11:22:40 +0100 Subject: [PATCH] fix(metadata): keep configured uri variables fixes #5184 --- features/main/attribute_resource.feature | 31 +++++++---------- ...iResourceToLegacyResourceMetadataTrait.php | 4 +++ .../Serializer/DocumentationNormalizer.php | 4 +++ src/Doctrine/Odm/State/LinksHandlerTrait.php | 6 ++++ src/Doctrine/Orm/State/LinksHandlerTrait.php | 2 +- ...plateResourceMetadataCollectionFactory.php | 6 ++++ src/OpenApi/Factory/OpenApiFactory.php | 4 +++ tests/Fixtures/TestBundle/Entity/Company.php | 7 ---- .../IncompleteUriVariableConfigured.php | 34 +++++++++++++++++++ 9 files changed, 72 insertions(+), 26 deletions(-) create mode 100644 tests/Fixtures/TestBundle/Entity/IncompleteUriVariableConfigured.php diff --git a/features/main/attribute_resource.feature b/features/main/attribute_resource.feature index dc258101820..83df0d96c75 100644 --- a/features/main/attribute_resource.feature +++ b/features/main/attribute_resource.feature @@ -1,12 +1,12 @@ +@php8 +@v3 +@!mysql +@!mongodb Feature: Resource attributes In order to use the Resource attribute As a developer I should be able to fetch data from a state provider - - @php8 - @v3 - @!mysql - @!mongodb + Scenario: Retrieve a Resource collection When I add "Content-Type" header equal to "application/ld+json" And I send a "GET" request to "/attribute_resources" @@ -36,11 +36,7 @@ Feature: Resource attributes } """ - @php8 - @v3 - @!mysql - @!mongodb - Scenario: Retrieve the first resource + Scenario: Retrieve the first resource When I add "Content-Type" header equal to "application/ld+json" And I send a "GET" request to "/attribute_resources/1" Then the response status code should be 200 @@ -57,10 +53,6 @@ Feature: Resource attributes } """ - @php8 - @v3 - @!mysql - @!mongodb Scenario: Retrieve the aliased resource When I add "Content-Type" header equal to "application/ld+json" And I send a "GET" request to "/dummy/1/attribute_resources/2" @@ -80,10 +72,6 @@ Feature: Resource attributes } """ - @php8 - @v3 - @!mysql - @!mongodb Scenario: Patch the aliased resource When I add "Content-Type" header equal to "application/merge-patch+json" And I send a "PATCH" request to "/dummy/1/attribute_resources/2" with body: @@ -105,3 +93,10 @@ Feature: Resource attributes "name": "Patched" } """ + + Scenario: Uri variables should be configured properly + When I send a "GET" request to "/photos/1/resize/300/100" + Then the response status code should be 400 + And the response should be in JSON + And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" + And the JSON node "hydra:description" should be equal to 'Unable to generate an IRI for the item of type "ApiPlatform\Tests\Fixtures\TestBundle\Entity\IncompleteUriVariableConfigured"' diff --git a/src/Core/Metadata/Resource/ApiResourceToLegacyResourceMetadataTrait.php b/src/Core/Metadata/Resource/ApiResourceToLegacyResourceMetadataTrait.php index 482a4b2995e..7623276dbd2 100644 --- a/src/Core/Metadata/Resource/ApiResourceToLegacyResourceMetadataTrait.php +++ b/src/Core/Metadata/Resource/ApiResourceToLegacyResourceMetadataTrait.php @@ -100,6 +100,10 @@ private function transformUriVariablesToIdentifiers(array $arrayOperation): arra $arrayOperation['identifiers'] = []; foreach ($arrayOperation['uri_variables'] as $parameterName => $identifiedBy) { + if ($identifiedBy->getExpandedValue() ?? false) { + continue; + } + if (1 === \count($identifiedBy->getIdentifiers() ?? ['id'])) { $arrayOperation['identifiers'][$parameterName] = [$identifiedBy->getFromClass(), $identifiedBy->getIdentifiers()[0] ?? ['id']]; continue; diff --git a/src/Core/Swagger/Serializer/DocumentationNormalizer.php b/src/Core/Swagger/Serializer/DocumentationNormalizer.php index 8e7a153511e..3fe7e1e208a 100644 --- a/src/Core/Swagger/Serializer/DocumentationNormalizer.php +++ b/src/Core/Swagger/Serializer/DocumentationNormalizer.php @@ -274,6 +274,10 @@ private function addPaths(bool $v3, \ArrayObject $paths, \ArrayObject $definitio } foreach ($operations as $operationName => $operation) { + if (false === ($operation['openapi'] ?? null)) { + continue; + } + // Skolem IRI if ('api_genid' === ($operation['route_name'] ?? null)) { continue; diff --git a/src/Doctrine/Odm/State/LinksHandlerTrait.php b/src/Doctrine/Odm/State/LinksHandlerTrait.php index dcaac93c0cc..287ba13dac8 100644 --- a/src/Doctrine/Odm/State/LinksHandlerTrait.php +++ b/src/Doctrine/Odm/State/LinksHandlerTrait.php @@ -37,6 +37,12 @@ private function handleLinks(Builder $aggregationBuilder, array $identifiers, ar return; } + foreach ($links as $i => $link) { + if (null !== $link->getExpandedValue()) { + unset($links[$i]); + } + } + $executeOptions = $operation->getExtraProperties()['doctrine_mongodb']['execute_options'] ?? []; $this->buildAggregation($resourceClass, array_reverse($links), array_reverse($identifiers), $context, $executeOptions, $resourceClass, $aggregationBuilder); diff --git a/src/Doctrine/Orm/State/LinksHandlerTrait.php b/src/Doctrine/Orm/State/LinksHandlerTrait.php index db9c87991e1..c27c2f122d9 100644 --- a/src/Doctrine/Orm/State/LinksHandlerTrait.php +++ b/src/Doctrine/Orm/State/LinksHandlerTrait.php @@ -45,7 +45,7 @@ private function handleLinks(QueryBuilder $queryBuilder, array $identifiers, Que $identifiers = array_reverse($identifiers); foreach (array_reverse($links) as $link) { - if ($link->getExpandedValue() || !$link->getFromClass()) { + if (null !== $link->getExpandedValue() || !$link->getFromClass()) { continue; } diff --git a/src/Metadata/Resource/Factory/UriTemplateResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/UriTemplateResourceMetadataCollectionFactory.php index 1e385b55d8f..65bc62d349e 100644 --- a/src/Metadata/Resource/Factory/UriTemplateResourceMetadataCollectionFactory.php +++ b/src/Metadata/Resource/Factory/UriTemplateResourceMetadataCollectionFactory.php @@ -132,7 +132,9 @@ private function configureUriVariables($operation) return $this->normalizeUriVariables($operation); } + $hasUserConfiguredUriVariables = !($operation->getExtraProperties['is_legacy_resource_metadata'] ?? false); if (!$operation->getUriVariables()) { + $hasUserConfiguredUriVariables = false; $operation = $operation->withUriVariables($this->transformLinksToUriVariables($this->linkFactory->createLinksFromIdentifiers($operation))); } @@ -157,6 +159,10 @@ private function configureUriVariables($operation) }); if (\count($variables) !== \count($uriVariables)) { + if ($hasUserConfiguredUriVariables) { + return $operation; + } + $newUriVariables = []; foreach ($variables as $variable) { if (isset($uriVariables[$variable])) { diff --git a/src/OpenApi/Factory/OpenApiFactory.php b/src/OpenApi/Factory/OpenApiFactory.php index b435d991b34..b8e0c93560c 100644 --- a/src/OpenApi/Factory/OpenApiFactory.php +++ b/src/OpenApi/Factory/OpenApiFactory.php @@ -194,6 +194,10 @@ private function collectPaths(ApiResource $resource, ResourceMetadataCollection // Set up parameters foreach ($uriVariables ?? [] as $parameterName => $uriVariable) { + if ($uriVariable->getExpandedValue() ?? false) { + continue; + } + $parameter = new Model\Parameter($parameterName, 'path', (new \ReflectionClass($uriVariable->getFromClass()))->getShortName().' identifier', true, false, false, ['type' => 'string']); if ($this->hasParameter($parameter, $parameters)) { continue; diff --git a/tests/Fixtures/TestBundle/Entity/Company.php b/tests/Fixtures/TestBundle/Entity/Company.php index f514c3dd578..4569740b591 100644 --- a/tests/Fixtures/TestBundle/Entity/Company.php +++ b/tests/Fixtures/TestBundle/Entity/Company.php @@ -28,13 +28,6 @@ #[GetCollection] #[Get] #[Post] -#[ApiResource( - uriTemplate: '/employees/{employeeId}/rooms/{roomId}/company/{companyId}', - uriVariables: [ - 'employeeId' => ['from_class' => Employee::class, 'from_property' => 'company'], - ], -)] -#[Get] #[ApiResource( uriTemplate: '/employees/{employeeId}/company', uriVariables: [ diff --git a/tests/Fixtures/TestBundle/Entity/IncompleteUriVariableConfigured.php b/tests/Fixtures/TestBundle/Entity/IncompleteUriVariableConfigured.php new file mode 100644 index 00000000000..346f1df20f3 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/IncompleteUriVariableConfigured.php @@ -0,0 +1,34 @@ + + * + * 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 ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\Operation; + +#[Get(uriTemplate: '/photos/{id}/resize/{width}/{height}', uriVariables: ['id'], provider: [IncompleteUriVariableConfigured::class, 'provide'], openapi: false)] +final class IncompleteUriVariableConfigured +{ + public function __construct(public string $id) + { + } + + public static function provide(Operation $operation, array $uriVariables = [], array $context = []): self + { + if (isset($uriVariables['width'])) { + throw new \LogicException('URI variable "width" should not exist'); + } + + return new self($uriVariables['id']); + } +}