Merge pull request #3248 from api-platform/fix/normalize-relation-return-type

Ensure correct return type from AbstractItemNormalizer::normalizeRelation

Author: teohhanhui

phpstan.neon.dist

@@ -25,6 +25,9 @@ parameters:
 			path: %currentWorkingDirectory%/src/Identifier/Normalizer/IntegerDenormalizer.php
 
 		# False positives
+		-
+			message: '#Variable \$iri might not be defined\.#'
+			path: %currentWorkingDirectory%/src/JsonApi/Serializer/ItemNormalizer.php
 		-
 			message: '#Variable \$positionPm might not be defined\.#'
 			path: %currentWorkingDirectory%/src/Util/ClassInfoTrait.php
@@ -60,7 +63,6 @@ parameters:
 		-
 			message: '#Property ApiPlatform\\Core\\Test\\DoctrineMongoDbOdmFilterTestCase::\$repository \(Doctrine\\ODM\\MongoDB\\Repository\\DocumentRepository\) does not accept Doctrine\\ORM\\EntityRepository<ApiPlatform\\Core\\Tests\\Fixtures\\TestBundle\\Document\\Dummy>\.#'
 			path: %currentWorkingDirectory%/src/Test/DoctrineMongoDbOdmFilterTestCase.php
-		- '#Method ApiPlatform\\Core\\(Serializer\\Abstract|JsonApi\\Serializer\\)ItemNormalizer::normalizeRelation\(\) should return array\|string but returns array\|bool\|float\|int\|string\|null\.#'
 		- '#Method ApiPlatform\\Core\\Util\\RequestParser::parseRequestParams\(\) should return array but returns array\|false\.#'
 		-
 			message: '#Method ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Util\\QueryBuilderHelper::mapJoinAliases() should return array<string, array<string>\|string> but returns array<int|string, mixed>\.#'
@@ -116,3 +118,8 @@ parameters:
 			path: %currentWorkingDirectory%/src/Action/ExceptionAction.php
 		- '#Call to method get(Class|Headers|StatusCode)\(\) on an unknown class Symfony\\Component\\ErrorHandler\\Exception\\FlattenException\.#'
 		- '#Class Symfony\\Component\\ErrorHandler\\Exception\\FlattenException not found\.#'
+		-
+			message: '#Instanceof between bool\|float\|int|null and ArrayObject will always evaluate to false\.#'
+			paths:
+				- %currentWorkingDirectory%/src/JsonApi/Serializer/ItemNormalizer.php
+				- %currentWorkingDirectory%/src/Serializer/AbstractItemNormalizer.php

src/JsonApi/Serializer/ItemNormalizer.php

@@ -14,7 +14,6 @@
 namespace ApiPlatform\Core\JsonApi\Serializer;
 
 use ApiPlatform\Core\Api\IriConverterInterface;
-use ApiPlatform\Core\Api\OperationType;
 use ApiPlatform\Core\Api\ResourceClassResolverInterface;
 use ApiPlatform\Core\Exception\ItemNotFoundException;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
@@ -199,34 +198,29 @@ protected function denormalizeRelation(string $attributeName, PropertyMetadata $
      * {@inheritdoc}
      *
      * @see http://jsonapi.org/format/#document-resource-object-linkage
-     *
-     * @throws LogicException
      */
     protected function normalizeRelation(PropertyMetadata $propertyMetadata, $relatedObject, string $resourceClass, ?string $format, array $context)
     {
-        if (null === $relatedObject) {
-            if (isset($context['operation_type'], $context['subresource_resources'][$resourceClass]) && OperationType::SUBRESOURCE === $context['operation_type']) {
-                $iri = $this->iriConverter->getItemIriFromResourceClass($resourceClass, $context['subresource_resources'][$resourceClass]);
-            } else {
-                if ($this->serializer instanceof NormalizerInterface) {
-                    return $this->serializer->normalize($relatedObject, $format, $context);
-                }
-                throw new LogicException(sprintf('The injected serializer must be an instance of "%s".', NormalizerInterface::class));
-            }
-        } else {
+        if (null !== $relatedObject) {
             $iri = $this->iriConverter->getIriFromItem($relatedObject);
             $context['iri'] = $iri;
 
             if (isset($context['resources'])) {
                 $context['resources'][$iri] = $iri;
             }
-            if (isset($context['api_included'])) {
-                if (!$this->serializer instanceof NormalizerInterface) {
-                    throw new LogicException(sprintf('The injected serializer must be an instance of "%s".', NormalizerInterface::class));
-                }
+        }
 
-                return $this->serializer->normalize($relatedObject, $format, $context);
+        if (null === $relatedObject || isset($context['api_included'])) {
+            if (!$this->serializer instanceof NormalizerInterface) {
+                throw new LogicException(sprintf('The injected serializer must be an instance of "%s".', NormalizerInterface::class));
             }
+
+            $normalizedRelatedObject = $this->serializer->normalize($relatedObject, $format, $context);
+            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');
+            }
+
+            return $normalizedRelatedObject;
         }
 
         return [

src/Serializer/AbstractItemNormalizer.php

@@ -495,6 +495,7 @@ protected function createRelationSerializationContext(string $resourceClass, arr
      * {@inheritdoc}
      *
      * @throws NoSuchPropertyException
+     * @throws UnexpectedValueException
      * @throws LogicException
      */
     protected function getAttributeValue($object, $attribute, $format = null, array $context = [])
@@ -515,13 +516,16 @@ protected function getAttributeValue($object, $attribute, $format = null, array
         $type = $propertyMetadata->getType();
 
         if (
-            is_iterable($attributeValue) &&
             $type &&
             $type->isCollection() &&
             ($collectionValueType = $type->getCollectionValueType()) &&
             ($className = $collectionValueType->getClassName()) &&
             $this->resourceClassResolver->isResourceClass($className)
         ) {
+            if (!is_iterable($attributeValue)) {
+                throw new UnexpectedValueException('Unexpected non-iterable value for to-many relation.');
+            }
+
             $resourceClass = $this->resourceClassResolver->getResourceClass($attributeValue, $className);
             $childContext = $this->createChildContext($context, $attribute, $format);
             $childContext['resource_class'] = $resourceClass;
@@ -535,6 +539,10 @@ protected function getAttributeValue($object, $attribute, $format = null, array
             ($className = $type->getClassName()) &&
             $this->resourceClassResolver->isResourceClass($className)
         ) {
+            if (!\is_object($attributeValue) && null !== $attributeValue) {
+                throw new UnexpectedValueException('Unexpected non-object value for to-one relation.');
+            }
+
             $resourceClass = $this->resourceClassResolver->getResourceClass($attributeValue, $className);
             $childContext = $this->createChildContext($context, $attribute, $format);
             $childContext['resource_class'] = $resourceClass;
@@ -556,23 +564,32 @@ protected function getAttributeValue($object, $attribute, $format = null, array
      * Normalizes a collection of relations (to-many).
      *
      * @param iterable $attributeValue
+     *
+     * @throws UnexpectedValueException
      */
     protected function normalizeCollectionOfRelations(PropertyMetadata $propertyMetadata, $attributeValue, string $resourceClass, ?string $format, array $context): array
     {
         $value = [];
         foreach ($attributeValue as $index => $obj) {
+            if (!\is_object($obj) && null !== $obj) {
+                throw new UnexpectedValueException('Unexpected non-object element in to-many relation.');
+            }
+
             $value[$index] = $this->normalizeRelation($propertyMetadata, $obj, $resourceClass, $format, $context);
         }
 
         return $value;
     }
 
     /**
-     * Normalizes a relation as an object if is a Link or as an URI.
+     * Normalizes a relation.
+     *
+     * @param object|null $relatedObject
      *
      * @throws LogicException
+     * @throws UnexpectedValueException
      *
-     * @return string|array
+     * @return string|array|\ArrayObject|null IRI or normalized object data
      */
     protected function normalizeRelation(PropertyMetadata $propertyMetadata, $relatedObject, string $resourceClass, ?string $format, array $context)
     {
@@ -581,7 +598,12 @@ protected function normalizeRelation(PropertyMetadata $propertyMetadata, $relate
                 throw new LogicException(sprintf('The injected serializer must be an instance of "%s".', NormalizerInterface::class));
             }
 
-            return $this->serializer->normalize($relatedObject, $format, $context);
+            $normalizedRelatedObject = $this->serializer->normalize($relatedObject, $format, $context);
+            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');
+            }
+
+            return $normalizedRelatedObject;
         }
 
         $iri = $this->iriConverter->getIriFromItem($relatedObject);