go further on that fix

api-platform · Aug 22, 2023 · 8f9fedb · 8f9fedb
1 parent ba27035
commit 8f9fedb
Show file tree

Hide file tree

Showing 12 changed files with 114 additions and 109 deletions.
diff --git a/features/hydra/item_uri_template.feature b/features/hydra/item_uri_template.feature
@@ -192,11 +192,31 @@ Feature: Exposing a collection of objects should use the specified operation to
 
   Scenario: Get a collection referencing an invalid itemUriTemplate
     When I send a "GET" request to "/issue5662/admin/reviews"
-    Then the response status code should be 400
+    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; charset=utf-8"
-    And the JSON node "@context" should be equal to "/contexts/Error"
-    And the JSON node "@type" should be equal to "hydra:Error"
-    And the JSON node "hydra:title" should be equal to "An error occurred"
-    And the JSON node "hydra:description" should be equal to 'Unable to find operation "/issue5662/reviews/{id}{._format}"'
-    And the JSON node "trace" should exist
+    And the JSON should be equal to:
+    """
+    {
+        "@context": "/contexts/Review",
+        "@id": "/issue5662/admin/reviews",
+        "@type": "hydra:Collection",
+        "hydra:totalItems": 2,
+        "hydra:member": [
+            {
+                "@id": "/issue5662/admin/reviews/1",
+                "@type": "Review",
+                "book": "/issue5662/books/a",
+                "id": 1,
+                "body": "Best book ever!"
+            },
+            {
+                "@id": "/issue5662/admin/reviews/2",
+                "@type": "Review",
+                "book": "/issue5662/books/b",
+                "id": 2,
+                "body": "Worst book ever!"
+            }
+        ]
+    }
+    """
diff --git a/src/Hydra/Serializer/CollectionNormalizer.php b/src/Hydra/Serializer/CollectionNormalizer.php
@@ -39,7 +39,7 @@ final class CollectionNormalizer extends AbstractCollectionNormalizer
         self::IRI_ONLY => false,
     ];
 
-    public function __construct(private readonly ContextBuilderInterface $contextBuilder, ResourceClassResolverInterface $resourceClassResolver, private readonly IriConverterInterface $iriConverter, private readonly ?ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory = null, array $defaultContext = [])
+    public function __construct(private readonly ContextBuilderInterface $contextBuilder, ResourceClassResolverInterface $resourceClassResolver, private readonly IriConverterInterface $iriConverter, readonly ?ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory = null, array $defaultContext = [])
     {
         $this->defaultContext = array_merge($this->defaultContext, $defaultContext);
 
@@ -80,26 +80,8 @@ protected function getItemsData(iterable $object, string $format = null, array $
     {
         $data = [];
         $data['hydra:member'] = [];
-
         $iriOnly = $context[self::IRI_ONLY] ?? $this->defaultContext[self::IRI_ONLY];
 
-        if (($operation = $context['operation'] ?? null) && method_exists($operation, 'getItemUriTemplate')) {
-            $context['item_uri_template'] = $operation->getItemUriTemplate();
-        }
-
-        // We need to keep this operation for serialization groups for later
-        if (isset($context['operation'])) {
-            $context['root_operation'] = $context['operation'];
-        }
-
-        if (isset($context['operation_name'])) {
-            $context['root_operation_name'] = $context['operation_name'];
-        }
-
-        // We need to unset the operation to ensure a proper IRI generation inside items
-        unset($context['operation']);
-        unset($context['operation_name'], $context['uri_variables']);
-
         foreach ($object as $obj) {
             if ($iriOnly) {
                 $data['hydra:member'][] = $this->iriConverter->getIriFromResource($obj);

diff --git a/src/JsonLd/Serializer/ItemNormalizer.php b/src/JsonLd/Serializer/ItemNormalizer.php
@@ -99,20 +99,13 @@ public function normalize(mixed $object, string $format = null, array $context =
             unset($context['operation'], $context['operation_name']);
         }
 
-        if (($operation = $context['operation'] ?? null) && method_exists($operation, 'getItemUriTemplate') && ($itemUriTemplate = $operation->getItemUriTemplate())) {
-            $context['item_uri_template'] = $itemUriTemplate;
-        }
-
         if ($iri = $this->iriConverter->getIriFromResource($object, UrlGeneratorInterface::ABS_PATH, $context['operation'] ?? null, $context)) {
             $context['iri'] = $iri;
             $metadata['@id'] = $iri;
         }
 
         $context['api_normalize'] = true;
 
-        /* @see https://github.com/api-platform/core/pull/5663 */
-        unset($context['item_uri_template']);
-
         $data = parent::normalize($object, $format, $context);
         if (!\is_array($data)) {
             return $data;

diff --git a/src/Serializer/AbstractCollectionNormalizer.php b/src/Serializer/AbstractCollectionNormalizer.php
@@ -34,6 +34,7 @@ abstract class AbstractCollectionNormalizer implements NormalizerInterface, Norm
         initContext as protected;
     }
     use NormalizerAwareTrait;
+    use OperationContextTrait;
 
     /**
      * This constant must be overridden in the child class.
@@ -96,27 +97,12 @@ public function normalize(mixed $object, string $format = null, array $context =
         }
 
         $resourceClass = $this->resourceClassResolver->getResourceClass($object, $context['resource_class']);
-        $context = $this->initContext($resourceClass, $context);
+        $collectionContext = $this->initContext($resourceClass, $context);
         $data = [];
-        $paginationData = $this->getPaginationData($object, $context);
+        $paginationData = $this->getPaginationData($object, $collectionContext);
 
-        if (($operation = $context['operation'] ?? null) && method_exists($operation, 'getItemUriTemplate')) {
-            $context['item_uri_template'] = $operation->getItemUriTemplate();
-        }
-
-        // We need to keep this operation for serialization groups for later
-        if (isset($context['operation'])) {
-            $context['root_operation'] = $context['operation'];
-        }
-
-        if (isset($context['operation_name'])) {
-            $context['root_operation_name'] = $context['operation_name'];
-        }
-
-        unset($context['operation']);
-        unset($context['operation_type'], $context['operation_name']);
-
-        $itemsData = $this->getItemsData($object, $format, $context);
+        $childContext = $this->createOperationContext($collectionContext, $resourceClass);
+        $itemsData = $this->getItemsData($object, $format, $childContext);
 
         return array_merge_recursive($data, $paginationData, $itemsData);
     }

diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php
@@ -19,8 +19,6 @@
 use ApiPlatform\Exception\InvalidArgumentException;
 use ApiPlatform\Exception\ItemNotFoundException;
 use ApiPlatform\Metadata\ApiProperty;
-use ApiPlatform\Metadata\CollectionOperationInterface;
-use ApiPlatform\Metadata\Exception\OperationNotFoundException;
 use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
 use ApiPlatform\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
 use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
@@ -56,6 +54,7 @@ abstract class AbstractItemNormalizer extends AbstractObjectNormalizer
     use CloneTrait;
     use ContextTrait;
     use InputOutputMetadataTrait;
+    use OperationContextTrait;
 
     protected PropertyAccessorInterface $propertyAccessor;
     protected array $localCache = [];
@@ -134,12 +133,6 @@ public function normalize(mixed $object, string $format = null, array $context =
             return $this->serializer->normalize($object, $format, $context);
         }
 
-        if (isset($context['operation']) && $context['operation'] instanceof CollectionOperationInterface) {
-            unset($context['operation_name']);
-            unset($context['operation']);
-            unset($context['iri']);
-        }
-
         if ($this->resourceClassResolver->isResourceClass($resourceClass)) {
             $context = $this->initContext($resourceClass, $context);
         }
@@ -590,7 +583,7 @@ protected function getFactoryOptions(array $context): array
 
             if (!$operation && $this->resourceMetadataCollectionFactory && $this->resourceClassResolver->isResourceClass($context['resource_class'])) {
                 $resourceClass = $this->resourceClassResolver->getResourceClass(null, $context['resource_class']); // fix for abstract classes and interfaces
-                $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation($context['root_operation_name'] ?? $context['operation_name'] ?? null);
+                $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null);
             }
 
             if ($operation) {
@@ -716,8 +709,7 @@ protected function normalizeRelation(ApiProperty $propertyMetadata, ?object $rel
                 throw new LogicException(sprintf('The injected serializer must be an instance of "%s".', NormalizerInterface::class));
             }
 
-            $relatedContext = $context;
-            unset($relatedContext['force_resource_class']);
+            $relatedContext = $this->createOperationContext($context, $resourceClass);
             $normalizedRelatedObject = $this->serializer->normalize($relatedObject, $format, $relatedContext);
             if (!\is_string($normalizedRelatedObject) && !\is_array($normalizedRelatedObject) && !$normalizedRelatedObject instanceof \ArrayObject && null !== $normalizedRelatedObject) {
                 throw new UnexpectedValueException('Expected normalized relation to be an IRI, array, \ArrayObject or null');
@@ -883,29 +875,4 @@ private function setValue(object $object, string $attributeName, mixed $value):
             // Properties not found are ignored
         }
     }
-
-    private function createOperationContext(array $context, string $resourceClass = null): array
-    {
-        if (isset($context['operation']) && !isset($context['root_operation'])) {
-            $context['root_operation'] = $context['operation'];
-            $context['root_operation_name'] = $context['operation_name'];
-        }
-
-        unset($context['iri'], $context['uri_variables']);
-        if (!$resourceClass) {
-            return $context;
-        }
-
-        unset($context['operation'], $context['operation_name']);
-        $context['resource_class'] = $resourceClass;
-        if ($this->resourceMetadataCollectionFactory) {
-            try {
-                $context['operation'] = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation();
-                $context['operation_name'] = $context['operation']->getName();
-            } catch (OperationNotFoundException) {
-            }
-        }
-
-        return $context;
-    }
 }
diff --git a/src/Serializer/OperationContextTrait.php b/src/Serializer/OperationContextTrait.php
@@ -0,0 +1,50 @@
+<?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\Serializer;
+
+/**
+ * @internal
+ */
+trait OperationContextTrait
+{
+    /**
+     * This context is created when working on a relation context or items of a collection. It cleans the previously given
+     * context as the operation changes.
+     */
+    protected function createOperationContext(array $context, string $resourceClass = null): array
+    {
+        if (isset($context['operation']) && !isset($context['root_operation'])) {
+            $context['root_operation'] = $context['operation'];
+        }
+
+        if (isset($context['operation_name']) || isset($context['graphql_operation_name'])) {
+            $context['root_operation_name'] = $context['operation_name'] ?? $context['graphql_operation_name'];
+        }
+
+        unset($context['iri'], $context['uri_variables'], $context['item_uri_template'], $context['force_resource_class']);
+
+        if (!$resourceClass) {
+            return $context;
+        }
+
+        if (($operation = $context['operation'] ?? null) && method_exists($operation, 'getItemUriTemplate')) {
+            $context['item_uri_template'] = $operation->getItemUriTemplate();
+        }
+
+        unset($context['operation'], $context['operation_name']);
+        $context['resource_class'] = $resourceClass;
+
+        return $context;
+    }
+}
diff --git a/src/Serializer/SerializerContextBuilder.php b/src/Serializer/SerializerContextBuilder.php
@@ -14,6 +14,7 @@
 namespace ApiPlatform\Serializer;
 
 use ApiPlatform\Exception\RuntimeException;
+use ApiPlatform\Metadata\CollectionOperationInterface;
 use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
 use ApiPlatform\Util\RequestAttributesExtractor;
 use Symfony\Component\HttpFoundation\Request;
@@ -53,6 +54,11 @@ public function createFromRequest(Request $request, bool $normalization, array $
         $context['input'] = $operation->getInput();
         $context['output'] = $operation->getOutput();
 
+        // Special case as this is usually handled by our OperationContextTrait, here we want to force the IRI in the response
+        if (!$operation instanceof CollectionOperationInterface && method_exists($operation, 'getItemUriTemplate') && $operation->getItemUriTemplate()) {
+            $context['item_uri_template'] = $operation->getItemUriTemplate();
+        }
+
         if ($operation->getTypes()) {
             $context['types'] = $operation->getTypes();
         }

diff --git a/src/Symfony/Routing/IriConverter.php b/src/Symfony/Routing/IriConverter.php
@@ -50,6 +50,7 @@ final class IriConverter implements IriConverterInterface
     use UriVariablesResolverTrait;
 
     private $localOperationCache = [];
+    private $localIdentifiersExtractorOperationCache = [];
 
     public function __construct(private readonly ProviderInterface $provider, private readonly RouterInterface $router, private readonly IdentifiersExtractorInterface $identifiersExtractor, ResourceClassResolverInterface $resourceClassResolver, private readonly ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory, UriVariablesConverterInterface $uriVariablesConverter = null, private readonly ?IriConverterInterface $decorated = null, private readonly ?OperationMetadataFactoryInterface $operationMetadataFactory = null)
     {
@@ -105,9 +106,13 @@ public function getIriFromResource(object|string $resource, int $referenceType =
     {
         $resourceClass = $context['force_resource_class'] ?? (\is_string($resource) ? $resource : $this->getObjectClass($resource));
 
+        if ($this->operationMetadataFactory && isset($context['item_uri_template'])) {
+            $operation = $this->operationMetadataFactory->create($context['item_uri_template']);
+        }
+
         $localOperationCacheKey = ($operation?->getName() ?? '').$resourceClass.(\is_string($resource) ? '_c' : '_i');
         if ($operation && isset($this->localOperationCache[$localOperationCacheKey])) {
-            return $this->generateSymfonyRoute($resource, $referenceType, $this->localOperationCache[$localOperationCacheKey], $context);
+            return $this->generateSymfonyRoute($resource, $referenceType, $this->localOperationCache[$localOperationCacheKey], $context, $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] ?? null);
         }
 
         if (!$this->resourceClassResolver->isResourceClass($resourceClass)) {
@@ -128,13 +133,7 @@ public function getIriFromResource(object|string $resource, int $referenceType =
             unset($context['uri_variables']);
         }
 
-        if ($this->operationMetadataFactory && isset($context['item_uri_template'])) {
-            $operation = $this->operationMetadataFactory->create($context['item_uri_template']);
-            if (!$operation) {
-                throw new InvalidArgumentException(sprintf('Unable to find operation "%s"', $context['item_uri_template']));
-            }
-        }
-
+        $identifiersExtractorOperation = $operation;
         // In symfony the operation name is the route name, try to find one if none provided
         if (
             !$operation->getName()
@@ -143,6 +142,7 @@ public function getIriFromResource(object|string $resource, int $referenceType =
             $forceCollection = $operation instanceof CollectionOperationInterface;
             try {
                 $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation(null, $forceCollection, true);
+                $identifiersExtractorOperation = $operation;
             } catch (OperationNotFoundException) {
             }
         }
@@ -152,8 +152,9 @@ public function getIriFromResource(object|string $resource, int $referenceType =
         }
 
         $this->localOperationCache[$localOperationCacheKey] = $operation;
+        $this->localIdentifiersExtractorOperationCache[$localOperationCacheKey] = $identifiersExtractorOperation;
 
-        return $this->generateSymfonyRoute($resource, $referenceType, $operation, $context);
+        return $this->generateSymfonyRoute($resource, $referenceType, $operation, $context, $identifiersExtractorOperation);
     }
 
     private function generateSkolemIri(object|string $resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, Operation $operation = null, array $context = [], string $resourceClass = null): string
@@ -166,22 +167,17 @@ private function generateSkolemIri(object|string $resource, int $referenceType =
         return $this->decorated->getIriFromResource($resource, $referenceType, $operation, $context);
     }
 
-    private function generateSymfonyRoute(object|string $resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, Operation $operation = null, array $context = []): string
+    private function generateSymfonyRoute(object|string $resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, Operation $operation = null, array $context = [], Operation $identifiersExtractorOperation = null): string
     {
         $identifiers = $context['uri_variables'] ?? [];
 
         if (\is_object($resource)) {
             try {
-                $identifiers = $this->identifiersExtractor->getIdentifiersFromItem($resource, $operation, $context);
+                $identifiers = $this->identifiersExtractor->getIdentifiersFromItem($resource, $identifiersExtractorOperation, $context);
             } catch (InvalidArgumentException|RuntimeException $e) {
                 // We can try using context uri variables if any
                 if (!$identifiers) {
-                    // Try with the first operation found
-                    try {
-                        $identifiers = $this->identifiersExtractor->getIdentifiersFromItem($resource, null, $context);
-                    } catch (InvalidArgumentException|RuntimeException $e) {
-                        throw new InvalidArgumentException(sprintf('Unable to generate an IRI for the item of type "%s"', $operation->getClass()), $e->getCode(), $e);
-                    }
+                    throw new InvalidArgumentException(sprintf('Unable to generate an IRI for the item of type "%s"', $operation->getClass()), $e->getCode(), $e);
                 }
             }
         }

diff --git a/tests/Fixtures/TestBundle/ApiResource/Issue5396/CompositeKeyWithDifferentType.php b/tests/Fixtures/TestBundle/ApiResource/Issue5396/CompositeKeyWithDifferentType.php
@@ -26,12 +26,16 @@ class CompositeKeyWithDifferentType
     #[ApiProperty(identifier: true)]
     public ?string $verificationKey;
 
-    public static function provide(Operation $operation, array $uriVariables = [], array $context = []): array
+    public static function provide(Operation $operation, array $uriVariables = [], array $context = []): self
     {
         if (!\is_string($uriVariables['verificationKey'])) {
             throw new \RuntimeException('verificationKey should be a string.');
         }
 
-        return $context;
+        $t = new self();
+        $t->id = $uriVariables['id'];
+        $t->verificationKey = $uriVariables['verificationKey'];
+
+        return $t;
     }
 }