From 96af30f1faf41d323a822eaee7a2ca9c9fcc8fc7 Mon Sep 17 00:00:00 2001 From: Cornel Cruceru Date: Fri, 12 Jan 2024 15:31:40 +0200 Subject: [PATCH 1/4] [Serializer] Rewrite `AbstractObjectNormalizer::createChildContext()` to use the provided cache_key from original context when creating child contexts If the normalization starts with an initial 'cache_key' it is currently discarded when creating child contexts. This change fixes that. The main reason behind it is that if the client code sends a unique identifier (ApiPlatform injects the IRI) and we have a use case that iterates a big resultset we end up with a big private cache that quickly runs out of allowed memory. The solution should be to send a 'cache_key' which ignores the entire cache key calculation that hashes the context. --- .../Normalizer/AbstractObjectNormalizer.php | 25 +++++++------ .../AbstractObjectNormalizerTest.php | 37 +++++++++++++++++++ 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php index a2eb65ca5bc6f..58dcf55bb6096 100644 --- a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php +++ b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php @@ -153,9 +153,7 @@ public function supportsNormalization(mixed $data, string $format = null /* , ar */ public function normalize(mixed $object, string $format = null, array $context = []) { - if (!isset($context['cache_key'])) { - $context['cache_key'] = $this->getCacheKey($format, $context); - } + $context['cache_key'] = $this->getCacheKey($format, $context); $this->validateCallbackContext($context); @@ -287,8 +285,6 @@ abstract protected function extractAttributes(object $object, string $format = n /** * Gets the attribute value. - * - * @return mixed */ abstract protected function getAttributeValue(object $object, string $attribute, string $format = null, array $context = []); @@ -302,14 +298,9 @@ public function supportsDenormalization(mixed $data, string $type, string $forma return class_exists($type) || (interface_exists($type, false) && null !== $this->classDiscriminatorResolver?->getMappingForClass($type)); } - /** - * @return mixed - */ public function denormalize(mixed $data, string $type, string $format = null, array $context = []) { - if (!isset($context['cache_key'])) { - $context['cache_key'] = $this->getCacheKey($format, $context); - } + $context['cache_key'] = $this->getCacheKey($format, $context); $this->validateCallbackContext($context); @@ -734,7 +725,13 @@ private function isMaxDepthReached(array $attributesMetadata, string $class, str protected function createChildContext(array $parentContext, string $attribute, ?string $format): array { $context = parent::createChildContext($parentContext, $attribute, $format); - $context['cache_key'] = $this->getCacheKey($format, $context); + if (isset($context['cache_key'])) { + if (false !== $context['cache_key']) { + $context['cache_key'] .= '-'.$attribute; + } + } else { + $context['cache_key'] = $this->getCacheKey($format, $context); + } return $context; } @@ -746,6 +743,10 @@ protected function createChildContext(array $parentContext, string $attribute, ? */ private function getCacheKey(?string $format, array $context): bool|string { + if (isset($context['cache_key'])) { + return $context['cache_key']; + } + foreach ($context[self::EXCLUDE_FROM_CACHE_KEY] ?? $this->defaultContext[self::EXCLUDE_FROM_CACHE_KEY] as $key) { unset($context[$key]); } diff --git a/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php index 13a34fa233457..a87009a3e557f 100644 --- a/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php +++ b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php @@ -788,6 +788,43 @@ public function supportsNormalization(mixed $data, string $format = null, array $this->assertSame('called', $object->bar); } + public function testChildContextKeepsOriginalContextCacheKey() + { + $foobar = new Dummy(); + $foobar->foo = new EmptyDummy(); + $foobar->bar = 'bar'; + $foobar->baz = 'baz'; + $data = [ + 'foo' => [], + 'bar' => 'bar', + 'baz' => 'baz', + ]; + + $normalizer = new class() extends AbstractObjectNormalizerDummy { + protected function extractAttributes(object $object, string $format = null, array $context = []): array + { + return array_keys((array) $object); + } + + protected function getAttributeValue(object $object, string $attribute, string $format = null, array $context = []): mixed + { + return $object->{$attribute}; + } + + protected function createChildContext(array $parentContext, string $attribute, ?string $format): array + { + $childContext = parent::createChildContext($parentContext, $attribute, $format); + AbstractObjectNormalizerTest::assertSame('hardcoded-'.$attribute, $childContext['cache_key']); + + return $childContext; + } + }; + + $serializer = new Serializer([$normalizer]); + $normalizedObject = $serializer->normalize($foobar, null, ['cache_key' => 'hardcoded']); + $this->assertSame($data, $normalizedObject); + } + public function testDenormalizeUnionOfEnums() { $serializer = new Serializer([ From 72aac246a201030a4afec28300a5a96d7fe4f66e Mon Sep 17 00:00:00 2001 From: Cornel Cruceru Date: Fri, 12 Jan 2024 18:16:06 +0200 Subject: [PATCH 2/4] changed assert approach for childContextCacheKey --- .../Tests/Normalizer/AbstractObjectNormalizerTest.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php index a87009a3e557f..5a5cc282fdbd8 100644 --- a/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php +++ b/src/Symfony/Component/Serializer/Tests/Normalizer/AbstractObjectNormalizerTest.php @@ -801,6 +801,8 @@ public function testChildContextKeepsOriginalContextCacheKey() ]; $normalizer = new class() extends AbstractObjectNormalizerDummy { + public ?string $childContextCacheKey = null; + protected function extractAttributes(object $object, string $format = null, array $context = []): array { return array_keys((array) $object); @@ -814,7 +816,7 @@ protected function getAttributeValue(object $object, string $attribute, string $ protected function createChildContext(array $parentContext, string $attribute, ?string $format): array { $childContext = parent::createChildContext($parentContext, $attribute, $format); - AbstractObjectNormalizerTest::assertSame('hardcoded-'.$attribute, $childContext['cache_key']); + $this->childContextCacheKey = $childContext['cache_key']; return $childContext; } @@ -822,7 +824,8 @@ protected function createChildContext(array $parentContext, string $attribute, ? $serializer = new Serializer([$normalizer]); $normalizedObject = $serializer->normalize($foobar, null, ['cache_key' => 'hardcoded']); - $this->assertSame($data, $normalizedObject); + // $this->assertSame($data, $normalizedObject); + $this->assertSame('hardcoded-foo', $normalizer->childContextCacheKey); } public function testDenormalizeUnionOfEnums() From 7a28a2971f14ffcd850bdd33c8295bc0e60bed74 Mon Sep 17 00:00:00 2001 From: Cornel Cruceru Date: Fri, 12 Jan 2024 18:19:32 +0200 Subject: [PATCH 3/4] reverted to inline ifs before calling getCacheKey --- .../Normalizer/AbstractObjectNormalizer.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php index 58dcf55bb6096..b73230e4eb382 100644 --- a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php +++ b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php @@ -153,7 +153,9 @@ public function supportsNormalization(mixed $data, string $format = null /* , ar */ public function normalize(mixed $object, string $format = null, array $context = []) { - $context['cache_key'] = $this->getCacheKey($format, $context); + if (!isset($context['cache_key'])) { + $context['cache_key'] = $this->getCacheKey($format, $context); + } $this->validateCallbackContext($context); @@ -300,7 +302,9 @@ public function supportsDenormalization(mixed $data, string $type, string $forma public function denormalize(mixed $data, string $type, string $format = null, array $context = []) { - $context['cache_key'] = $this->getCacheKey($format, $context); + if (!isset($context['cache_key'])) { + $context['cache_key'] = $this->getCacheKey($format, $context); + } $this->validateCallbackContext($context); @@ -743,10 +747,6 @@ protected function createChildContext(array $parentContext, string $attribute, ? */ private function getCacheKey(?string $format, array $context): bool|string { - if (isset($context['cache_key'])) { - return $context['cache_key']; - } - foreach ($context[self::EXCLUDE_FROM_CACHE_KEY] ?? $this->defaultContext[self::EXCLUDE_FROM_CACHE_KEY] as $key) { unset($context[$key]); } From 765d27636a5a58b7f4b6dcaf5dcd293bf896d3eb Mon Sep 17 00:00:00 2001 From: Cornel Cruceru Date: Fri, 12 Jan 2024 18:19:49 +0200 Subject: [PATCH 4/4] reverted unintended code style change --- .../Serializer/Normalizer/AbstractObjectNormalizer.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php index b73230e4eb382..5a8ac29ebe36d 100644 --- a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php +++ b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php @@ -287,6 +287,8 @@ abstract protected function extractAttributes(object $object, string $format = n /** * Gets the attribute value. + * + * @return mixed */ abstract protected function getAttributeValue(object $object, string $attribute, string $format = null, array $context = []);