From 87e38355f4087c14f44465a59c8ef203394fbc7d Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Sun, 15 Dec 2024 18:51:43 +0100 Subject: [PATCH 1/5] performance improvments RelatedCollectionLinkNormalizer --- .../RelatedCollectionLinkNormalizer.php | 56 +++++++++++++------ 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php b/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php index 326197a67a..ae67ae06f4 100644 --- a/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php +++ b/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php @@ -8,6 +8,7 @@ use ApiPlatform\Exception\ResourceClassNotFoundException; use ApiPlatform\Metadata\GetCollection; use ApiPlatform\Metadata\IriConverterInterface; +use ApiPlatform\Metadata\Operation; use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface; use ApiPlatform\Metadata\UrlGeneratorInterface; use App\Entity\BaseEntity; @@ -81,6 +82,8 @@ class RelatedCollectionLinkNormalizer implements NormalizerInterface, Serializer use PropertyHelperTrait; use ClassInfoTrait; + private $exactSearchFilterExistsCache = []; + public function __construct( private NormalizerInterface $decorated, private ServiceLocator $filterLocator, @@ -110,13 +113,15 @@ public function normalize($data, $format = null, array $context = []): null|arra if (isset($link['href'])) { continue; } - - try { - $normalized_data['_links'][$rel] = ['href' => $this->getRelatedCollectionHref($data, $rel, $context)]; - } catch (UnsupportedRelationException $e) { - // The relation is not supported, or there is no matching filter defined on the related entity + // Only consider relations with non-null value (object or collection) + if (property_exists($data, $rel) && !isset($data->$rel)) { continue; } + + list ($ok, $href) = $this->getRelatedCollectionHref($data, $rel, $context); + if ($ok) { + $normalized_data['_links'][$rel] = ['href' => $href]; + } } return $normalized_data; @@ -136,7 +141,7 @@ public function setSerializer(SerializerInterface $serializer): void { } } - public function getRelatedCollectionHref($object, $rel, array $context = []): string { + public function getRelatedCollectionHref($object, $rel, array $context = []): array { $resourceClass = $this->getObjectClass($object); if ($this->nameConverter instanceof NameConverterInterface) { @@ -149,7 +154,7 @@ public function getRelatedCollectionHref($object, $rel, array $context = []): st $params = $this->extractUriParams($object, $annotation->getParams()); [$uriTemplate] = $this->uriTemplateFactory->createFromResourceClass($annotation->getRelatedEntity()); - return $this->uriTemplate->expand($uriTemplate, $params); + return [true, $this->uriTemplate->expand($uriTemplate, $params)]; } try { @@ -161,7 +166,7 @@ public function getRelatedCollectionHref($object, $rel, array $context = []): st $relationMetadata = $classMetadata->getAssociationMapping($rel); } catch (MappingException) { - throw new UnsupportedRelationException($resourceClass.'#'.$rel.' is not a Doctrine association. Embedding non-Doctrine collections is currently not implemented.'); + return [false, $resourceClass.'#'.$rel.' is not a Doctrine association. Embedding non-Doctrine collections is currently not implemented.']; } $relatedResourceClass = $relationMetadata['targetEntity']; @@ -170,21 +175,36 @@ public function getRelatedCollectionHref($object, $rel, array $context = []): st $relatedFilterName ??= $relationMetadata['inversedBy']; if (empty($relatedResourceClass) || empty($relatedFilterName)) { - throw new UnsupportedRelationException('The '.$resourceClass.'#'.$rel.' relation does not have both a targetEntity and a mappedBy or inversedBy property'); + return [false, 'The '.$resourceClass.'#'.$rel.' relation does not have both a targetEntity and a mappedBy or inversedBy property']; } - $resourceMetadataCollection = $this->resourceMetadataCollectionFactory->create($relatedResourceClass); - $operation = OperationHelper::findOneByType($resourceMetadataCollection, GetCollection::class); - - if (!$operation) { - throw new UnsupportedRelationException('The resource '.$relatedResourceClass.' does not implement GetCollection() operation.'); + $lookupKey = $relatedResourceClass.':'.$relatedFilterName; + if (isset($this->exactSearchFilterExistsCache[$lookupKey])) { + $result = $this->exactSearchFilterExistsCache[$lookupKey]; + + } else { + $result = [null, '']; + $resourceMetadataCollection = $this->resourceMetadataCollectionFactory->create($relatedResourceClass); + $operation = OperationHelper::findOneByType($resourceMetadataCollection, GetCollection::class); + + if (!$operation) { + $result = [null, 'The resource '.$relatedResourceClass.' does not implement GetCollection() operation.']; + } else { + $filterExists = $this->exactSearchFilterExists($relatedResourceClass, $relatedFilterName); + if (!$filterExists) { + $result = [null, 'The resource '.$relatedResourceClass.' does not have a search filter for the relation '.$relatedFilterName.'.']; + } else { + $result = [$operation, '']; + } + } + $this->exactSearchFilterExistsCache[$lookupKey] = $result; } - if (!$this->exactSearchFilterExists($relatedResourceClass, $relatedFilterName)) { - throw new UnsupportedRelationException('The resource '.$relatedResourceClass.' does not have a search filter for the relation '.$relatedFilterName.'.'); + if ($result[0] instanceof Operation) { + return [true, $this->router->generate($result[0]->getName(), [$relatedFilterName => urlencode($this->iriConverter->getIriFromResource($object))], UrlGeneratorInterface::ABS_PATH)]; + } else { + return [false, $result[1]]; } - - return $this->router->generate($operation->getName(), [$relatedFilterName => urlencode($this->iriConverter->getIriFromResource($object))], UrlGeneratorInterface::ABS_PATH); } protected function getRelatedCollectionLinkAnnotation(string $className, string $propertyName): ?RelatedCollectionLink { From 5ef06ec00e46091f8378624d1686a504eba2a70f Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Sun, 15 Dec 2024 19:40:07 +0100 Subject: [PATCH 2/5] remove property_exists check (does not work yet) --- .../Normalizer/RelatedCollectionLinkNormalizer.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php b/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php index ae67ae06f4..441c5047fd 100644 --- a/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php +++ b/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php @@ -114,9 +114,10 @@ public function normalize($data, $format = null, array $context = []): null|arra continue; } // Only consider relations with non-null value (object or collection) - if (property_exists($data, $rel) && !isset($data->$rel)) { - continue; - } + // (does not yet work, some properties are private and cannot be read out) + // if (property_exists($data, $rel) && !isset($data->$rel)) { + // continue; + // } list ($ok, $href) = $this->getRelatedCollectionHref($data, $rel, $context); if ($ok) { From 256bef7f10d66ad8b4e99faa43da4238833eb532 Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Sun, 15 Dec 2024 19:58:02 +0100 Subject: [PATCH 3/5] cs-fix --- .../Normalizer/RelatedCollectionLinkNormalizer.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php b/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php index 441c5047fd..993ab89270 100644 --- a/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php +++ b/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php @@ -119,7 +119,7 @@ public function normalize($data, $format = null, array $context = []): null|arra // continue; // } - list ($ok, $href) = $this->getRelatedCollectionHref($data, $rel, $context); + list($ok, $href) = $this->getRelatedCollectionHref($data, $rel, $context); if ($ok) { $normalized_data['_links'][$rel] = ['href' => $href]; } @@ -182,12 +182,11 @@ public function getRelatedCollectionHref($object, $rel, array $context = []): ar $lookupKey = $relatedResourceClass.':'.$relatedFilterName; if (isset($this->exactSearchFilterExistsCache[$lookupKey])) { $result = $this->exactSearchFilterExistsCache[$lookupKey]; - } else { $result = [null, '']; $resourceMetadataCollection = $this->resourceMetadataCollectionFactory->create($relatedResourceClass); $operation = OperationHelper::findOneByType($resourceMetadataCollection, GetCollection::class); - + if (!$operation) { $result = [null, 'The resource '.$relatedResourceClass.' does not implement GetCollection() operation.']; } else { @@ -203,9 +202,9 @@ public function getRelatedCollectionHref($object, $rel, array $context = []): ar if ($result[0] instanceof Operation) { return [true, $this->router->generate($result[0]->getName(), [$relatedFilterName => urlencode($this->iriConverter->getIriFromResource($object))], UrlGeneratorInterface::ABS_PATH)]; - } else { - return [false, $result[1]]; } + + return [false, $result[1]]; } protected function getRelatedCollectionLinkAnnotation(string $className, string $propertyName): ?RelatedCollectionLink { From 17c9264c0ae11692e2af02125d94c46d9a4e30d1 Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Mon, 16 Dec 2024 18:42:11 +0100 Subject: [PATCH 4/5] ignore public property with value NULL --- .../Normalizer/RelatedCollectionLinkNormalizer.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php b/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php index 993ab89270..4501ae3b7a 100644 --- a/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php +++ b/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php @@ -113,11 +113,13 @@ public function normalize($data, $format = null, array $context = []): null|arra if (isset($link['href'])) { continue; } - // Only consider relations with non-null value (object or collection) - // (does not yet work, some properties are private and cannot be read out) - // if (property_exists($data, $rel) && !isset($data->$rel)) { - // continue; - // } + + // If relation is a public property, this property can be checked to be a non-null value + $values = get_object_vars($data); + if (array_key_exists($rel, $values) && null == $values[$rel]) { + // target-value is NULL + continue; + } list($ok, $href) = $this->getRelatedCollectionHref($data, $rel, $context); if ($ok) { From 70a7155e779576c6723a15eb34ccff92bfafb192 Mon Sep 17 00:00:00 2001 From: Pirmin Mattmann Date: Mon, 23 Dec 2024 20:15:21 +0100 Subject: [PATCH 5/5] cleanup --- .../RelatedCollectionLinkNormalizer.php | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php b/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php index 4501ae3b7a..fe57060406 100644 --- a/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php +++ b/api/src/Serializer/Normalizer/RelatedCollectionLinkNormalizer.php @@ -82,7 +82,10 @@ class RelatedCollectionLinkNormalizer implements NormalizerInterface, Serializer use PropertyHelperTrait; use ClassInfoTrait; - private $exactSearchFilterExistsCache = []; + /** + * @var (Operation|string)[] + */ + private array $exactSearchFilterExistsOperationCache = []; public function __construct( private NormalizerInterface $decorated, @@ -121,10 +124,10 @@ public function normalize($data, $format = null, array $context = []): null|arra continue; } - list($ok, $href) = $this->getRelatedCollectionHref($data, $rel, $context); - if ($ok) { - $normalized_data['_links'][$rel] = ['href' => $href]; + if (!$this->getRelatedCollectionHref($data, $rel, $context, $result)) { + continue; } + $normalized_data['_links'][$rel] = ['href' => $result]; } return $normalized_data; @@ -144,7 +147,7 @@ public function setSerializer(SerializerInterface $serializer): void { } } - public function getRelatedCollectionHref($object, $rel, array $context = []): array { + protected function getRelatedCollectionHref($object, $rel, array $context, &$href): bool { $resourceClass = $this->getObjectClass($object); if ($this->nameConverter instanceof NameConverterInterface) { @@ -157,7 +160,9 @@ public function getRelatedCollectionHref($object, $rel, array $context = []): ar $params = $this->extractUriParams($object, $annotation->getParams()); [$uriTemplate] = $this->uriTemplateFactory->createFromResourceClass($annotation->getRelatedEntity()); - return [true, $this->uriTemplate->expand($uriTemplate, $params)]; + $href = $this->uriTemplate->expand($uriTemplate, $params); + + return true; } try { @@ -169,7 +174,8 @@ public function getRelatedCollectionHref($object, $rel, array $context = []): ar $relationMetadata = $classMetadata->getAssociationMapping($rel); } catch (MappingException) { - return [false, $resourceClass.'#'.$rel.' is not a Doctrine association. Embedding non-Doctrine collections is currently not implemented.']; + // $resourceClass # $rel is not a Doctrine association. Embedding non-Doctrine collections is currently not implemented + return false; } $relatedResourceClass = $relationMetadata['targetEntity']; @@ -178,35 +184,38 @@ public function getRelatedCollectionHref($object, $rel, array $context = []): ar $relatedFilterName ??= $relationMetadata['inversedBy']; if (empty($relatedResourceClass) || empty($relatedFilterName)) { - return [false, 'The '.$resourceClass.'#'.$rel.' relation does not have both a targetEntity and a mappedBy or inversedBy property']; + // The $resourceClass # $rel relation does not have both a targetEntity and a mappedBy or inversedBy property + return false; } $lookupKey = $relatedResourceClass.':'.$relatedFilterName; - if (isset($this->exactSearchFilterExistsCache[$lookupKey])) { - $result = $this->exactSearchFilterExistsCache[$lookupKey]; + if (isset($this->exactSearchFilterExistsOperationCache[$lookupKey])) { + $result = $this->exactSearchFilterExistsOperationCache[$lookupKey]; } else { - $result = [null, '']; + $result = 'No Operation'; $resourceMetadataCollection = $this->resourceMetadataCollectionFactory->create($relatedResourceClass); $operation = OperationHelper::findOneByType($resourceMetadataCollection, GetCollection::class); if (!$operation) { - $result = [null, 'The resource '.$relatedResourceClass.' does not implement GetCollection() operation.']; + // The resource $relatedResourceClass does not implement GetCollection() operation } else { $filterExists = $this->exactSearchFilterExists($relatedResourceClass, $relatedFilterName); if (!$filterExists) { - $result = [null, 'The resource '.$relatedResourceClass.' does not have a search filter for the relation '.$relatedFilterName.'.']; + // The resource $relatedResourceClass does not have a search filter for the relation $relatedFilterName } else { - $result = [$operation, '']; + $result = $operation; } } - $this->exactSearchFilterExistsCache[$lookupKey] = $result; + $this->exactSearchFilterExistsOperationCache[$lookupKey] = $result; } - if ($result[0] instanceof Operation) { - return [true, $this->router->generate($result[0]->getName(), [$relatedFilterName => urlencode($this->iriConverter->getIriFromResource($object))], UrlGeneratorInterface::ABS_PATH)]; + if ($result instanceof Operation) { + $href = $this->router->generate($result->getName(), [$relatedFilterName => urlencode($this->iriConverter->getIriFromResource($object))], UrlGeneratorInterface::ABS_PATH); + + return true; } - return [false, $result[1]]; + return false; } protected function getRelatedCollectionLinkAnnotation(string $className, string $propertyName): ?RelatedCollectionLink {