From cd44a2c7c09cfd337207497dc706185a2e83733d Mon Sep 17 00:00:00 2001 From: soyuka Date: Fri, 21 Oct 2022 15:57:45 +0200 Subject: [PATCH 1/4] fix(serializer): empty object as array with supports cache reverts #4999 --- .../Resource/ResourceMetadataCollection.php | 4 +++- src/Serializer/AbstractItemNormalizer.php | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/Metadata/Resource/ResourceMetadataCollection.php b/src/Metadata/Resource/ResourceMetadataCollection.php index 2921635706b..eb6be193c11 100644 --- a/src/Metadata/Resource/ResourceMetadataCollection.php +++ b/src/Metadata/Resource/ResourceMetadataCollection.php @@ -51,7 +51,9 @@ public function getOperation(?string $operationName = null, bool $forceCollectio foreach ($metadata->getOperations() ?? [] as $name => $operation) { $isCollection = $operation instanceof CollectionOperationInterface; - if ('' === $operationName && \in_array($operation->getMethod() ?? HttpOperation::METHOD_GET, [HttpOperation::METHOD_GET, HttpOperation::METHOD_OPTIONS, HttpOperation::METHOD_HEAD], true) && ($forceCollection ? $isCollection : !$isCollection)) { + $method = $operation->getMethod() ?? HttpOperation::METHOD_GET; + $isGetOperation = $method === HttpOperation::METHOD_GET || $method === HttpOperation::METHOD_OPTIONS || $method === HttpOperation::METHOD_HEAD; + if ('' === $operationName && $isGetOperation && ($forceCollection ? $isCollection : !$isCollection)) { return $this->operationCache[$operationName] = $operation; } diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index d953fdadf48..c35317fcbc2 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -54,6 +54,7 @@ abstract class AbstractItemNormalizer extends AbstractObjectNormalizer protected PropertyAccessorInterface $propertyAccessor; protected array $localCache = []; + protected array $localOperationCache = []; public function __construct(protected PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, protected PropertyMetadataFactoryInterface $propertyMetadataFactory, protected IriConverterInterface $iriConverter, protected ResourceClassResolverInterface $resourceClassResolver, PropertyAccessorInterface $propertyAccessor = null, NameConverterInterface $nameConverter = null, ClassMetadataFactoryInterface $classMetadataFactory = null, array $defaultContext = [], ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory = null, protected ?ResourceAccessCheckerInterface $resourceAccessChecker = null) { @@ -511,10 +512,17 @@ protected function getFactoryOptions(array $context): array $options['serializer_groups'] = (array) $context[self::GROUPS]; } - if (isset($context['resource_class']) && $this->resourceClassResolver->isResourceClass($context['resource_class']) && $this->resourceMetadataCollectionFactory) { - $resourceClass = $this->resourceClassResolver->getResourceClass(null, $context['resource_class']); // fix for abstract classes and interfaces - // This is a hot spot, we should avoid calling this here but in many cases we can't - $operation = $context['operation'] ?? $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation($context['operation_name'] ?? null); + // This is a hot spot + if (isset($context['resource_class'])) { + $operationCacheKey = $context['resource_class'] . ($context['operation_name'] ?? ''); + $operation = $context['operation'] ?? $this->localOperationCache[$operationCacheKey] ?? null; + + if (!isset($this->localOperationCache[$operationCacheKey]) && !$operation && $this->resourceClassResolver->isResourceClass($context['resource_class']) && $this->resourceMetadataCollectionFactory) { + $resourceClass = $this->resourceClassResolver->getResourceClass(null, $context['resource_class']); // fix for abstract classes and interfaces + $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation($context['operation_name'] ?? null); + $this->localOperationCache[$operationCacheKey] = $operation; + } + $options['normalization_groups'] = $operation->getNormalizationContext()['groups'] ?? null; $options['denormalization_groups'] = $operation->getDenormalizationContext()['groups'] ?? null; $options['operation_name'] = $operation->getName(); @@ -592,7 +600,7 @@ protected function getAttributeValue(object $object, string $attribute, string $ if ($type && $type->getClassName()) { $childContext = $this->createChildContext($context, $attribute, $format); unset($childContext['iri'], $childContext['uri_variables']); - $childContext['output']['gen_id'] = $propertyMetadata->getGenId() ?? true; + $childContext['output']['gen_id'] = $propertyMetadata->getGenId() ?? true; return $this->serializer->normalize($attributeValue, $format, $childContext); } From 077f1f2e47464d1592db721b3a6433327554163d Mon Sep 17 00:00:00 2001 From: soyuka Date: Mon, 24 Oct 2022 17:38:00 +0200 Subject: [PATCH 2/4] fix issue when empty operation --- src/Serializer/AbstractItemNormalizer.php | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index c35317fcbc2..91ca2b7d183 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -40,6 +40,7 @@ use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer; use Symfony\Component\Serializer\Normalizer\DenormalizerInterface; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; +use Symfony\Component\Serializer\Serializer; /** * Base item normalizer. @@ -514,7 +515,7 @@ protected function getFactoryOptions(array $context): array // This is a hot spot if (isset($context['resource_class'])) { - $operationCacheKey = $context['resource_class'] . ($context['operation_name'] ?? ''); + $operationCacheKey = $context['resource_class'].($context['operation_name'] ?? ''); $operation = $context['operation'] ?? $this->localOperationCache[$operationCacheKey] ?? null; if (!isset($this->localOperationCache[$operationCacheKey]) && !$operation && $this->resourceClassResolver->isResourceClass($context['resource_class']) && $this->resourceMetadataCollectionFactory) { @@ -522,10 +523,12 @@ protected function getFactoryOptions(array $context): array $operation = $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation($context['operation_name'] ?? null); $this->localOperationCache[$operationCacheKey] = $operation; } - - $options['normalization_groups'] = $operation->getNormalizationContext()['groups'] ?? null; - $options['denormalization_groups'] = $operation->getDenormalizationContext()['groups'] ?? null; - $options['operation_name'] = $operation->getName(); + + if ($operation) { + $options['normalization_groups'] = $operation->getNormalizationContext()['groups'] ?? null; + $options['denormalization_groups'] = $operation->getDenormalizationContext()['groups'] ?? null; + $options['operation_name'] = $operation->getName(); + } } return $options; From 5157d959e495295fe5707babe3cea388b1e8fa24 Mon Sep 17 00:00:00 2001 From: soyuka Date: Thu, 27 Oct 2022 14:26:57 +0200 Subject: [PATCH 3/4] revert 08450c2684f82fa83c3d099de74ff225e82c1d3e --- .../CollectionFiltersNormalizer.php | 14 ++++++++-- src/Hydra/Serializer/CollectionNormalizer.php | 28 +++++++++++++++++-- .../PartialCollectionViewNormalizer.php | 7 +++-- .../Resource/ResourceMetadataCollection.php | 2 +- .../AbstractCollectionNormalizer.php | 24 +++++++++++++++- 5 files changed, 64 insertions(+), 11 deletions(-) diff --git a/src/Hydra/Serializer/CollectionFiltersNormalizer.php b/src/Hydra/Serializer/CollectionFiltersNormalizer.php index 684f1343470..b68c7c31564 100644 --- a/src/Hydra/Serializer/CollectionFiltersNormalizer.php +++ b/src/Hydra/Serializer/CollectionFiltersNormalizer.php @@ -19,9 +19,11 @@ use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; use Psr\Container\ContainerInterface; use Symfony\Component\Serializer\Exception\UnexpectedValueException; +use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer; use Symfony\Component\Serializer\Normalizer\CacheableSupportsMethodInterface; use Symfony\Component\Serializer\Normalizer\NormalizerAwareInterface; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; +use Symfony\Component\Serializer\Serializer; /** * Enhances the result of collection by adding the filters applied on collection. @@ -58,13 +60,19 @@ public function hasCacheableSupportsMethod(): bool */ public function normalize(mixed $object, string $format = null, array $context = []): array|string|int|float|bool|\ArrayObject|null { - $data = $this->collectionNormalizer->normalize($object, $format, $context); - if (!\is_array($data)) { - throw new UnexpectedValueException('Expected data to be an array'); + $preserveEmpty = ($context[Serializer::EMPTY_ARRAY_AS_OBJECT] ?? false) || ($context[AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS] ?? false); + if ($preserveEmpty && $object instanceof \ArrayObject && !\count($object)) { + return $object; } + + $data = $this->collectionNormalizer->normalize($object, $format, $context); if (!isset($context['resource_class']) || isset($context['api_sub_level'])) { return $data; } + + if (!\is_array($data)) { + throw new UnexpectedValueException('Expected data to be an array'); + } $resourceClass = $this->resourceClassResolver->getResourceClass($object, $context['resource_class']); $operation = $context['operation'] ?? $this->resourceMetadataCollectionFactory->create($resourceClass)->getOperation($context['operation_name'] ?? null); $resourceFilters = $operation->getFilters(); diff --git a/src/Hydra/Serializer/CollectionNormalizer.php b/src/Hydra/Serializer/CollectionNormalizer.php index a6e2c515d06..3546962d8ca 100644 --- a/src/Hydra/Serializer/CollectionNormalizer.php +++ b/src/Hydra/Serializer/CollectionNormalizer.php @@ -27,6 +27,7 @@ use Symfony\Component\Serializer\Normalizer\NormalizerAwareInterface; use Symfony\Component\Serializer\Normalizer\NormalizerAwareTrait; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; +use Symfony\Component\Serializer\Serializer; /** * This normalizer handles collections. @@ -56,7 +57,7 @@ public function __construct(private readonly ContextBuilderInterface $contextBui */ public function supportsNormalization(mixed $data, string $format = null, array $context = []): bool { - return self::FORMAT === $format && is_iterable($data) && isset($context['resource_class']) && !isset($context['api_sub_level']); + return self::FORMAT === $format && is_iterable($data); } /** @@ -64,8 +65,12 @@ public function supportsNormalization(mixed $data, string $format = null, array * * @param iterable $object */ - public function normalize(mixed $object, string $format = null, array $context = []): array + public function normalize(mixed $object, string $format = null, array $context = []) { + if (!isset($context['resource_class']) || isset($context['api_sub_level'])) { + return $this->normalizeRawCollection($object, $format, $context); + } + $resourceClass = $this->resourceClassResolver->getResourceClass($object, $context['resource_class']); $context = $this->initContext($resourceClass, $context); $context['api_collection_sub_level'] = true; @@ -103,6 +108,23 @@ public function normalize(mixed $object, string $format = null, array $context = public function hasCacheableSupportsMethod(): bool { - return false; + return true; + } + + /** + * Normalizes a raw collection (not API resources). + */ + protected function normalizeRawCollection(iterable $object, string $format = null, array $context = []) + { + if (\is_array($object) && !$object && ($context[Serializer::EMPTY_ARRAY_AS_OBJECT] ?? false)) { + return new \ArrayObject(); + } + + $data = []; + foreach ($object as $index => $obj) { + $data[$index] = $this->normalizer->normalize($obj, $format, $context); + } + + return $data; } } diff --git a/src/Hydra/Serializer/PartialCollectionViewNormalizer.php b/src/Hydra/Serializer/PartialCollectionViewNormalizer.php index 33c6ff65947..b7de04c1a01 100644 --- a/src/Hydra/Serializer/PartialCollectionViewNormalizer.php +++ b/src/Hydra/Serializer/PartialCollectionViewNormalizer.php @@ -45,14 +45,15 @@ public function __construct(private readonly NormalizerInterface $collectionNorm public function normalize(mixed $object, string $format = null, array $context = []): array|string|int|float|bool|\ArrayObject|null { $data = $this->collectionNormalizer->normalize($object, $format, $context); - if (!\is_array($data)) { - throw new UnexpectedValueException('Expected data to be an array'); - } if (isset($context['api_sub_level'])) { return $data; } + if (!\is_array($data)) { + throw new UnexpectedValueException('Expected data to be an array'); + } + $currentPage = $lastPage = $itemsPerPage = $pageTotalItems = null; if ($paginated = ($object instanceof PartialPaginatorInterface)) { if ($object instanceof PaginatorInterface) { diff --git a/src/Metadata/Resource/ResourceMetadataCollection.php b/src/Metadata/Resource/ResourceMetadataCollection.php index eb6be193c11..e0f904a4f90 100644 --- a/src/Metadata/Resource/ResourceMetadataCollection.php +++ b/src/Metadata/Resource/ResourceMetadataCollection.php @@ -52,7 +52,7 @@ public function getOperation(?string $operationName = null, bool $forceCollectio foreach ($metadata->getOperations() ?? [] as $name => $operation) { $isCollection = $operation instanceof CollectionOperationInterface; $method = $operation->getMethod() ?? HttpOperation::METHOD_GET; - $isGetOperation = $method === HttpOperation::METHOD_GET || $method === HttpOperation::METHOD_OPTIONS || $method === HttpOperation::METHOD_HEAD; + $isGetOperation = HttpOperation::METHOD_GET === $method || HttpOperation::METHOD_OPTIONS === $method || HttpOperation::METHOD_HEAD === $method; if ('' === $operationName && $isGetOperation && ($forceCollection ? $isCollection : !$isCollection)) { return $this->operationCache[$operationName] = $operation; } diff --git a/src/Serializer/AbstractCollectionNormalizer.php b/src/Serializer/AbstractCollectionNormalizer.php index c2f8c63d879..075ec207f62 100644 --- a/src/Serializer/AbstractCollectionNormalizer.php +++ b/src/Serializer/AbstractCollectionNormalizer.php @@ -22,6 +22,7 @@ use Symfony\Component\Serializer\Normalizer\NormalizerAwareInterface; use Symfony\Component\Serializer\Normalizer\NormalizerAwareTrait; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; +use Symfony\Component\Serializer\Serializer; /** * Base collection normalizer. @@ -50,7 +51,7 @@ public function __construct(protected ResourceClassResolverInterface $resourceCl */ public function supportsNormalization(mixed $data, string $format = null, array $context = []): bool { - return static::FORMAT === $format && is_iterable($data) && isset($context['resource_class']) && !isset($context['api_sub_level']); + return static::FORMAT === $format && is_iterable($data); } public function hasCacheableSupportsMethod(): bool @@ -65,6 +66,10 @@ public function hasCacheableSupportsMethod(): bool */ public function normalize(mixed $object, string $format = null, array $context = []): array|string|int|float|bool|\ArrayObject|null { + if (!isset($context['resource_class']) || isset($context['api_sub_level'])) { + return $this->normalizeRawCollection($object, $format, $context); + } + $resourceClass = $this->resourceClassResolver->getResourceClass($object, $context['resource_class']); $context = $this->initContext($resourceClass, $context); $data = []; @@ -82,6 +87,23 @@ public function normalize(mixed $object, string $format = null, array $context = return array_merge_recursive($data, $paginationData, $itemsData); } + /** + * Normalizes a raw collection (not API resources). + */ + protected function normalizeRawCollection(iterable $object, string $format = null, array $context = []): array + { + if (\is_array($object) && !$object && ($context[Serializer::EMPTY_ARRAY_AS_OBJECT] ?? false)) { + return new \ArrayObject(); + } + + $data = []; + foreach ($object as $index => $obj) { + $data[$index] = $this->normalizer->normalize($obj, $format, $context); + } + + return $data; + } + /** * Gets the pagination configuration. */ From 96678236a5c055858fb4d5811e107ecf7e5a61cd Mon Sep 17 00:00:00 2001 From: soyuka Date: Thu, 27 Oct 2022 14:36:32 +0200 Subject: [PATCH 4/4] review --- src/Api/IdentifiersExtractor.php | 14 +++++-- .../CollectionFiltersNormalizer.php | 4 +- src/Hydra/Serializer/CollectionNormalizer.php | 4 +- .../Resource/ResourceMetadataCollection.php | 20 +++++----- .../AbstractCollectionNormalizer.php | 6 +-- src/Serializer/AbstractItemNormalizer.php | 17 +++++---- src/Symfony/Routing/IriConverter.php | 38 +++++++++++++------ tests/Api/IdentifiersExtractorTest.php | 29 ++++++++++++++ .../Serializer/CollectionNormalizerTest.php | 6 +-- .../Serializer/CollectionNormalizerTest.php | 6 +-- .../Serializer/CollectionNormalizerTest.php | 6 +-- 11 files changed, 102 insertions(+), 48 deletions(-) diff --git a/src/Api/IdentifiersExtractor.php b/src/Api/IdentifiersExtractor.php index f77095de9aa..6c288b5777f 100644 --- a/src/Api/IdentifiersExtractor.php +++ b/src/Api/IdentifiersExtractor.php @@ -49,26 +49,34 @@ public function __construct(ResourceMetadataCollectionFactoryInterface $resource */ public function getIdentifiersFromItem(object $item, Operation $operation = null, array $context = []): array { - $identifiers = []; - if (!$this->isResourceClass($this->getObjectClass($item))) { return ['id' => $this->propertyAccessor->getValue($item, 'id')]; } + if ($operation && $operation->getClass()) { + return $this->getIdentifiersFromOperation($item, $operation, $context); + } + $resourceClass = $this->getResourceClass($item, true); $operation ??= $this->resourceMetadataFactory->create($resourceClass)->getOperation(null, false, true); + return $this->getIdentifiersFromOperation($item, $operation, $context); + } + + private function getIdentifiersFromOperation(object $item, Operation $operation, array $context = []): array + { if ($operation instanceof HttpOperation) { $links = $operation->getUriVariables(); } elseif ($operation instanceof GraphQlOperation) { $links = $operation->getLinks(); } + $identifiers = []; foreach ($links ?? [] as $link) { if (1 < (is_countable($link->getIdentifiers()) ? \count($link->getIdentifiers()) : 0)) { $compositeIdentifiers = []; foreach ($link->getIdentifiers() as $identifier) { - $compositeIdentifiers[$identifier] = $this->getIdentifierValue($item, $link->getFromClass() ?? $resourceClass, $identifier, $link->getParameterName()); + $compositeIdentifiers[$identifier] = $this->getIdentifierValue($item, $link->getFromClass() ?? $operation->getClass(), $identifier, $link->getParameterName()); } $identifiers[$link->getParameterName()] = CompositeIdentifierParser::stringify($compositeIdentifiers); diff --git a/src/Hydra/Serializer/CollectionFiltersNormalizer.php b/src/Hydra/Serializer/CollectionFiltersNormalizer.php index b68c7c31564..2cc505863b6 100644 --- a/src/Hydra/Serializer/CollectionFiltersNormalizer.php +++ b/src/Hydra/Serializer/CollectionFiltersNormalizer.php @@ -23,7 +23,6 @@ use Symfony\Component\Serializer\Normalizer\CacheableSupportsMethodInterface; use Symfony\Component\Serializer\Normalizer\NormalizerAwareInterface; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; -use Symfony\Component\Serializer\Serializer; /** * Enhances the result of collection by adding the filters applied on collection. @@ -60,8 +59,7 @@ public function hasCacheableSupportsMethod(): bool */ public function normalize(mixed $object, string $format = null, array $context = []): array|string|int|float|bool|\ArrayObject|null { - $preserveEmpty = ($context[Serializer::EMPTY_ARRAY_AS_OBJECT] ?? false) || ($context[AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS] ?? false); - if ($preserveEmpty && $object instanceof \ArrayObject && !\count($object)) { + if (($context[AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS] ?? false) && $object instanceof \ArrayObject && !\count($object)) { return $object; } diff --git a/src/Hydra/Serializer/CollectionNormalizer.php b/src/Hydra/Serializer/CollectionNormalizer.php index 3546962d8ca..019e40fe1cf 100644 --- a/src/Hydra/Serializer/CollectionNormalizer.php +++ b/src/Hydra/Serializer/CollectionNormalizer.php @@ -65,7 +65,7 @@ public function supportsNormalization(mixed $data, string $format = null, array * * @param iterable $object */ - public function normalize(mixed $object, string $format = null, array $context = []) + public function normalize(mixed $object, string $format = null, array $context = []): array|string|int|float|bool|\ArrayObject|null { if (!isset($context['resource_class']) || isset($context['api_sub_level'])) { return $this->normalizeRawCollection($object, $format, $context); @@ -114,7 +114,7 @@ public function hasCacheableSupportsMethod(): bool /** * Normalizes a raw collection (not API resources). */ - protected function normalizeRawCollection(iterable $object, string $format = null, array $context = []) + protected function normalizeRawCollection(iterable $object, string $format = null, array $context = []): array|\ArrayObject { if (\is_array($object) && !$object && ($context[Serializer::EMPTY_ARRAY_AS_OBJECT] ?? false)) { return new \ArrayObject(); diff --git a/src/Metadata/Resource/ResourceMetadataCollection.php b/src/Metadata/Resource/ResourceMetadataCollection.php index e0f904a4f90..63cf0252102 100644 --- a/src/Metadata/Resource/ResourceMetadataCollection.php +++ b/src/Metadata/Resource/ResourceMetadataCollection.php @@ -24,6 +24,8 @@ */ final class ResourceMetadataCollection extends \ArrayObject { + private const GRAPHQL_PREFIX = 'g_'; + private const HTTP_PREFIX = 'h_'; private array $operationCache = []; public function __construct(private readonly string $resourceClass, array $input = []) @@ -34,12 +36,12 @@ public function __construct(private readonly string $resourceClass, array $input public function getOperation(?string $operationName = null, bool $forceCollection = false, bool $httpOperation = false): Operation { $operationName ??= ''; - if (isset($this->operationCache[$operationName])) { - return $this->operationCache[$operationName]; + if (isset($this->operationCache[self::HTTP_PREFIX.$operationName])) { + return $this->operationCache[self::HTTP_PREFIX.$operationName]; } - if (isset($this->operationCache['graphql_'.$operationName])) { - return $this->operationCache['graphql_'.$operationName]; + if (isset($this->operationCache[self::GRAPHQL_PREFIX.$operationName])) { + return $this->operationCache[self::GRAPHQL_PREFIX.$operationName]; } $it = $this->getIterator(); @@ -54,26 +56,26 @@ public function getOperation(?string $operationName = null, bool $forceCollectio $method = $operation->getMethod() ?? HttpOperation::METHOD_GET; $isGetOperation = HttpOperation::METHOD_GET === $method || HttpOperation::METHOD_OPTIONS === $method || HttpOperation::METHOD_HEAD === $method; if ('' === $operationName && $isGetOperation && ($forceCollection ? $isCollection : !$isCollection)) { - return $this->operationCache[$operationName] = $operation; + return $this->operationCache[self::HTTP_PREFIX.$operationName] = $operation; } if ($name === $operationName) { - return $this->operationCache[$operationName] = $operation; + return $this->operationCache[self::HTTP_PREFIX.$operationName] = $operation; } if ($operation->getUriTemplate() === $operationName) { - return $this->operationCache[$operationName] = $operation; + return $this->operationCache[self::HTTP_PREFIX.$operationName] = $operation; } } foreach ($metadata->getGraphQlOperations() ?? [] as $name => $operation) { $isCollection = $operation instanceof CollectionOperationInterface; if ('' === $operationName && ($forceCollection ? $isCollection : !$isCollection) && false === $httpOperation) { - return $this->operationCache['graphql_'.$operationName] = $operation; + return $this->operationCache[self::GRAPHQL_PREFIX.$operationName] = $operation; } if ($name === $operationName) { - return $this->operationCache['graphql_'.$operationName] = $operation; + return $this->operationCache[self::GRAPHQL_PREFIX.$operationName] = $operation; } } diff --git a/src/Serializer/AbstractCollectionNormalizer.php b/src/Serializer/AbstractCollectionNormalizer.php index 075ec207f62..d84e2c87363 100644 --- a/src/Serializer/AbstractCollectionNormalizer.php +++ b/src/Serializer/AbstractCollectionNormalizer.php @@ -56,7 +56,7 @@ public function supportsNormalization(mixed $data, string $format = null, array public function hasCacheableSupportsMethod(): bool { - return false; + return true; } /** @@ -90,9 +90,9 @@ public function normalize(mixed $object, string $format = null, array $context = /** * Normalizes a raw collection (not API resources). */ - protected function normalizeRawCollection(iterable $object, string $format = null, array $context = []): array + protected function normalizeRawCollection(iterable $object, string $format = null, array $context = []): array|\ArrayObject { - if (\is_array($object) && !$object && ($context[Serializer::EMPTY_ARRAY_AS_OBJECT] ?? false)) { + if (!$object && ($context[Serializer::EMPTY_ARRAY_AS_OBJECT] ?? false) && \is_array($object)) { return new \ArrayObject(); } diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index 91ca2b7d183..47f0cc9fe1f 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -55,7 +55,7 @@ abstract class AbstractItemNormalizer extends AbstractObjectNormalizer protected PropertyAccessorInterface $propertyAccessor; protected array $localCache = []; - protected array $localOperationCache = []; + protected array $localFactoryOptionsCache = []; public function __construct(protected PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, protected PropertyMetadataFactoryInterface $propertyMetadataFactory, protected IriConverterInterface $iriConverter, protected ResourceClassResolverInterface $resourceClassResolver, PropertyAccessorInterface $propertyAccessor = null, NameConverterInterface $nameConverter = null, ClassMetadataFactoryInterface $classMetadataFactory = null, array $defaultContext = [], ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory = null, protected ?ResourceAccessCheckerInterface $resourceAccessChecker = null) { @@ -506,6 +506,11 @@ protected function denormalizeRelation(string $attributeName, ApiProperty $prope */ protected function getFactoryOptions(array $context): array { + $operationCacheKey = ($context['resource_class'] ?? '').($context['operation_name'] ?? '').($context['api_normalize'] ?? ''); + if ($operationCacheKey && isset($this->localFactoryOptionsCache[$operationCacheKey])) { + return $this->localFactoryOptionsCache[$operationCacheKey]; + } + $options = []; if (isset($context[self::GROUPS])) { @@ -515,13 +520,11 @@ protected function getFactoryOptions(array $context): array // This is a hot spot if (isset($context['resource_class'])) { - $operationCacheKey = $context['resource_class'].($context['operation_name'] ?? ''); - $operation = $context['operation'] ?? $this->localOperationCache[$operationCacheKey] ?? null; + $operation = $context['operation'] ?? null; - if (!isset($this->localOperationCache[$operationCacheKey]) && !$operation && $this->resourceClassResolver->isResourceClass($context['resource_class']) && $this->resourceMetadataCollectionFactory) { + 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['operation_name'] ?? null); - $this->localOperationCache[$operationCacheKey] = $operation; } if ($operation) { @@ -531,7 +534,7 @@ protected function getFactoryOptions(array $context): array } } - return $options; + return $this->localFactoryOptionsCache[$operationCacheKey] = $options; } /** @@ -603,7 +606,7 @@ protected function getAttributeValue(object $object, string $attribute, string $ if ($type && $type->getClassName()) { $childContext = $this->createChildContext($context, $attribute, $format); unset($childContext['iri'], $childContext['uri_variables']); - $childContext['output']['gen_id'] = $propertyMetadata->getGenId() ?? true; + $childContext['output']['gen_id'] = $propertyMetadata->getGenId() ?? true; return $this->serializer->normalize($attributeValue, $format, $childContext); } diff --git a/src/Symfony/Routing/IriConverter.php b/src/Symfony/Routing/IriConverter.php index 947153d95cc..0d004f41b1b 100644 --- a/src/Symfony/Routing/IriConverter.php +++ b/src/Symfony/Routing/IriConverter.php @@ -48,6 +48,8 @@ final class IriConverter implements IriConverterInterface use ResourceClassInfoTrait; use UriVariablesResolverTrait; + private $localOperationCache = []; + 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) { $this->resourceClassResolver = $resourceClassResolver; @@ -102,6 +104,11 @@ public function getIriFromResource(object|string $resource, int $referenceType = { $resourceClass = \is_string($resource) ? $resource : $this->getObjectClass($resource); + $localOperationCacheKey = ($operation?->getName() ?? '').$resourceClass.(\is_string($resource) ? '_c' : '_i'); + if ($operation && isset($this->localOperationCache[$localOperationCacheKey])) { + return $this->generateSymfonyRoute($resource, $referenceType, $this->localOperationCache[$localOperationCacheKey], $context); + } + if (!$this->resourceClassResolver->isResourceClass($resourceClass)) { return $this->generateSkolemIri($resource, $referenceType, $operation, $context, $resourceClass); } @@ -140,6 +147,23 @@ public function getIriFromResource(object|string $resource, int $referenceType = return $this->generateSkolemIri($resource, $referenceType, $operation, $context, $resourceClass); } + $this->localOperationCache[$localOperationCacheKey] = $operation; + + return $this->generateSymfonyRoute($resource, $referenceType, $operation, $context); + } + + private function generateSkolemIri(object|string $resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, Operation $operation = null, array $context = [], string $resourceClass = null): string + { + if (!$this->decorated) { + throw new InvalidArgumentException(sprintf('Unable to generate an IRI for the item of type "%s"', $resourceClass)); + } + + // Use a skolem iri, the route is defined in genid.xml + 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 + { $identifiers = $context['uri_variables'] ?? []; if (\is_object($resource)) { @@ -148,7 +172,7 @@ public function getIriFromResource(object|string $resource, int $referenceType = } catch (InvalidArgumentException|RuntimeException $e) { // We can try using context uri variables if any if (!$identifiers) { - throw new InvalidArgumentException(sprintf('Unable to generate an IRI for the item of type "%s"', $resourceClass), $e->getCode(), $e); + throw new InvalidArgumentException(sprintf('Unable to generate an IRI for the item of type "%s"', $operation->getClass()), $e->getCode(), $e); } } } @@ -156,17 +180,7 @@ public function getIriFromResource(object|string $resource, int $referenceType = try { return $this->router->generate($operation->getName(), $identifiers, $operation->getUrlGenerationStrategy() ?? $referenceType); } catch (RoutingExceptionInterface $e) { - throw new InvalidArgumentException(sprintf('Unable to generate an IRI for the item of type "%s"', $resourceClass), $e->getCode(), $e); + throw new InvalidArgumentException(sprintf('Unable to generate an IRI for the item of type "%s"', $operation->getClass()), $e->getCode(), $e); } } - - private function generateSkolemIri(object|string $resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, Operation $operation = null, array $context = [], string $resourceClass = null): string - { - if (!$this->decorated) { - throw new InvalidArgumentException(sprintf('Unable to generate an IRI for the item of type "%s"', $resourceClass)); - } - - // Use a skolem iri, the route is defined in genid.xml - return $this->decorated->getIriFromResource($resource, $referenceType, $operation, $context); - } } diff --git a/tests/Api/IdentifiersExtractorTest.php b/tests/Api/IdentifiersExtractorTest.php index 228c179decd..b44b19d2b49 100644 --- a/tests/Api/IdentifiersExtractorTest.php +++ b/tests/Api/IdentifiersExtractorTest.php @@ -52,6 +52,7 @@ public function testGetIdentifiersFromItem(): void $operation = $this->prophesize(HttpOperation::class); $item = new Dummy(); $resourceClass = Dummy::class; + $operation->getClass()->willReturn(null); $resourceClassResolverProphecy->isResourceClass(Argument::any())->willReturn(true); $resourceClassResolverProphecy->getResourceClass($item)->willReturn($resourceClass); @@ -60,6 +61,34 @@ public function testGetIdentifiersFromItem(): void $this->assertEquals([], $identifiersExtractor->getIdentifiersFromItem($item, $operation->reveal())); } + public function testGetIdentifiersFromItemWithOperation(): void + { + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + + $resourceClassResolver = $resourceClassResolverProphecy->reveal(); + + $identifiersExtractor = new IdentifiersExtractor( + $resourceMetadataFactoryProphecy->reveal(), + $resourceClassResolver, + $propertyNameCollectionFactoryProphecy->reveal(), + $propertyMetadataFactoryProphecy->reveal() + ); + + $operation = $this->prophesize(HttpOperation::class); + $item = new Dummy(); + $resourceClass = Dummy::class; + $operation->getClass()->willReturn($resourceClass); + $operation->getUriVariables()->willReturn([]); + + $resourceClassResolverProphecy->isResourceClass(Argument::any())->willReturn(true); + $resourceClassResolverProphecy->getResourceClass($item)->shouldNotBeCalled(); + + $this->assertEquals([], $identifiersExtractor->getIdentifiersFromItem($item, $operation->reveal())); + } + public function testGetIdentifiersFromItemWithId(): void { $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataCollectionFactoryInterface::class); diff --git a/tests/Hal/Serializer/CollectionNormalizerTest.php b/tests/Hal/Serializer/CollectionNormalizerTest.php index 6626a8a57a5..a766cdfa7c9 100644 --- a/tests/Hal/Serializer/CollectionNormalizerTest.php +++ b/tests/Hal/Serializer/CollectionNormalizerTest.php @@ -40,12 +40,12 @@ public function testSupportsNormalize(): void $normalizer = new CollectionNormalizer($resourceClassResolverProphecy->reveal(), 'page', $resourceMetadataFactoryProphecy->reveal()); $this->assertTrue($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, ['resource_class' => 'Foo'])); - $this->assertFalse($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, ['resource_class' => 'Foo', 'api_sub_level' => true])); - $this->assertFalse($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, [])); + $this->assertTrue($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, ['resource_class' => 'Foo', 'api_sub_level' => true])); + $this->assertTrue($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, [])); $this->assertTrue($normalizer->supportsNormalization(new \ArrayObject(), CollectionNormalizer::FORMAT, ['resource_class' => 'Foo'])); $this->assertFalse($normalizer->supportsNormalization([], 'xml', ['resource_class' => 'Foo'])); $this->assertFalse($normalizer->supportsNormalization(new \ArrayObject(), 'xml', ['resource_class' => 'Foo'])); - $this->assertFalse($normalizer->hasCacheableSupportsMethod()); + $this->assertTrue($normalizer->hasCacheableSupportsMethod()); } public function testNormalizePaginator(): void diff --git a/tests/Hydra/Serializer/CollectionNormalizerTest.php b/tests/Hydra/Serializer/CollectionNormalizerTest.php index b8029027c2d..c880ab4bb38 100644 --- a/tests/Hydra/Serializer/CollectionNormalizerTest.php +++ b/tests/Hydra/Serializer/CollectionNormalizerTest.php @@ -47,12 +47,12 @@ public function testSupportsNormalize(): void $normalizer = new CollectionNormalizer($contextBuilder->reveal(), $resourceClassResolverProphecy->reveal(), $iriConvert->reveal()); $this->assertTrue($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, ['resource_class' => 'Foo'])); - $this->assertFalse($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, ['resource_class' => 'Foo', 'api_sub_level' => true])); - $this->assertFalse($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, [])); + $this->assertTrue($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, ['resource_class' => 'Foo', 'api_sub_level' => true])); + $this->assertTrue($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, [])); $this->assertTrue($normalizer->supportsNormalization(new \ArrayObject(), CollectionNormalizer::FORMAT, ['resource_class' => 'Foo'])); $this->assertFalse($normalizer->supportsNormalization([], 'xml', ['resource_class' => 'Foo'])); $this->assertFalse($normalizer->supportsNormalization(new \ArrayObject(), 'xml', ['resource_class' => 'Foo'])); - $this->assertFalse($normalizer->hasCacheableSupportsMethod()); + $this->assertTrue($normalizer->hasCacheableSupportsMethod()); } public function testNormalizeResourceCollection(): void diff --git a/tests/JsonApi/Serializer/CollectionNormalizerTest.php b/tests/JsonApi/Serializer/CollectionNormalizerTest.php index dfd7ed01e5c..95b167f46d5 100644 --- a/tests/JsonApi/Serializer/CollectionNormalizerTest.php +++ b/tests/JsonApi/Serializer/CollectionNormalizerTest.php @@ -42,12 +42,12 @@ public function testSupportsNormalize(): void $normalizer = new CollectionNormalizer($resourceClassResolverProphecy->reveal(), 'page', $resourceMetadataFactoryProphecy->reveal()); $this->assertTrue($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, ['resource_class' => 'Foo'])); - $this->assertFalse($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, ['resource_class' => 'Foo', 'api_sub_level' => true])); - $this->assertFalse($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, [])); + $this->assertTrue($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, ['resource_class' => 'Foo', 'api_sub_level' => true])); + $this->assertTrue($normalizer->supportsNormalization([], CollectionNormalizer::FORMAT, [])); $this->assertTrue($normalizer->supportsNormalization(new \ArrayObject(), CollectionNormalizer::FORMAT, ['resource_class' => 'Foo'])); $this->assertFalse($normalizer->supportsNormalization([], 'xml', ['resource_class' => 'Foo'])); $this->assertFalse($normalizer->supportsNormalization(new \ArrayObject(), 'xml', ['resource_class' => 'Foo'])); - $this->assertFalse($normalizer->hasCacheableSupportsMethod()); + $this->assertTrue($normalizer->hasCacheableSupportsMethod()); } public function testNormalizePaginator(): void