Skip to content

Commit

Permalink
bug #30907 [Serializer] Respect ignored attributes in cache key of no…
Browse files Browse the repository at this point in the history
…rmalizer (dbu)

This PR was squashed before being merged into the 3.4 branch (closes #30907).

Discussion
----------

[Serializer] Respect ignored attributes in cache key of normalizer

EUFOSSA

| Q             | A
| ------------- | ---
| Branch       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Do not share the attributes cache in object normalizer when using a different setting for the ignoredAttributes setting.

In Symfony 4.2, the setter is deprecated in favor of the ignored_attibutes option in the $context. When merging this up, we will however still need to respect the field as well for BC, the cache key does not look at the default context (apart from the deprecated modifiers, the default context is immutable)

There might be performance regression for some use cases, but also could be a performance improvement when using 'attributes' in the context with lists of objects of the same class.

Commits
-------

926d228 [Serializer] Respect ignored attributes in cache key of normalizer
  • Loading branch information
fabpot committed Apr 8, 2019
2 parents d3eae71 + 926d228 commit 7a3060a
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 14 deletions.
Expand Up @@ -397,7 +397,7 @@ protected function denormalizeParameter(\ReflectionClass $class, \ReflectionPara
}
$parameterClass = $parameter->getClass()->getName();

return $this->serializer->denormalize($parameterData, $parameterClass, $format, $this->createChildContext($context, $parameterName));
return $this->serializer->denormalize($parameterData, $parameterClass, $format, $this->createChildContext($context, $parameterName, $format));
}

return $parameterData;
Expand All @@ -407,14 +407,15 @@ protected function denormalizeParameter(\ReflectionClass $class, \ReflectionPara
}

/**
* @param array $parentContext
* @param string $attribute
* @param array $parentContext
* @param string $attribute Attribute name
* @param string|null $format
*
* @return array
*
* @internal
*/
protected function createChildContext(array $parentContext, $attribute)
protected function createChildContext(array $parentContext, $attribute/*, string $format = null */)
{
if (isset($parentContext[self::ATTRIBUTES][$attribute])) {
$parentContext[self::ATTRIBUTES] = $parentContext[self::ATTRIBUTES][$attribute];
Expand Down
Expand Up @@ -94,7 +94,7 @@ public function normalize($object, $format = null, array $context = [])
throw new LogicException(sprintf('Cannot normalize attribute "%s" because the injected serializer is not a normalizer', $attribute));
}

$data = $this->updateData($data, $attribute, $this->serializer->normalize($attributeValue, $format, $this->createChildContext($context, $attribute)));
$data = $this->updateData($data, $attribute, $this->serializer->normalize($attributeValue, $format, $this->createChildContext($context, $attribute, $format)));
}

return $data;
Expand Down Expand Up @@ -128,15 +128,13 @@ protected function getAttributes($object, $format = null, array $context)
return $allowedAttributes;
}

if (isset($context['attributes'])) {
return $this->extractAttributes($object, $format, $context);
}
$attributes = $this->extractAttributes($object, $format, $context);

if (isset($this->attributesCache[$class])) {
return $this->attributesCache[$class];
if ($context['cache_key']) {
$this->attributesCache[$key] = $attributes;
}

return $this->attributesCache[$class] = $this->extractAttributes($object, $format, $context);
return $attributes;
}

/**
Expand Down Expand Up @@ -276,7 +274,7 @@ private function validateAndDenormalize($currentClass, $attribute, $data, $forma
throw new LogicException(sprintf('Cannot denormalize attribute "%s" for class "%s" because injected serializer is not a denormalizer', $attribute, $class));
}

$childContext = $this->createChildContext($context, $attribute);
$childContext = $this->createChildContext($context, $attribute, $format);
if ($this->serializer->supportsDenormalization($data, $class, $format, $childContext)) {
return $this->serializer->denormalize($data, $class, $format, $childContext);
}
Expand Down Expand Up @@ -373,7 +371,32 @@ private function isMaxDepthReached(array $attributesMetadata, $class, $attribute
}

/**
* Gets the cache key to use.
* Overwritten to update the cache key for the child.
*
* We must not mix up the attribute cache between parent and children.
*
* {@inheritdoc}
*/
protected function createChildContext(array $parentContext, $attribute/*, string $format = null */)
{
if (\func_num_args() >= 3) {
$format = \func_get_arg(2);
} else {
// will be deprecated in version 4
$format = null;
}

$context = parent::createChildContext($parentContext, $attribute, $format);
// format is already included in the cache_key of the parent.
$context['cache_key'] = $this->getCacheKey($format, $context);

return $context;
}

/**
* Builds the cache key for the attributes cache.
*
* The key must be different for every option in the context that could change which attributes should be handled.
*
* @param string|null $format
* @param array $context
Expand All @@ -382,8 +405,13 @@ private function isMaxDepthReached(array $attributesMetadata, $class, $attribute
*/
private function getCacheKey($format, array $context)
{
unset($context['cache_key']); // avoid artificially different keys
try {
return md5($format.serialize($context));
return md5($format.serialize([
'context' => $context,
'ignored' => $this->ignoredAttributes,
'camelized' => $this->camelizedAttributes,
]));
} catch (\Exception $exception) {
// The context cannot be serialized, skip the cache
return false;
Expand Down
Expand Up @@ -380,6 +380,16 @@ public function testIgnoredAttributes()
['fooBar' => 'foobar'],
$this->normalizer->normalize($obj, 'any')
);

$this->normalizer->setIgnoredAttributes(['foo', 'baz', 'camelCase', 'object']);

$this->assertEquals(
[
'fooBar' => 'foobar',
'bar' => 'bar',
],
$this->normalizer->normalize($obj, 'any')
);
}

public function testIgnoredAttributesDenormalize()
Expand Down

0 comments on commit 7a3060a

Please sign in to comment.