Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Serializer] Do not cache attributes ifattributes in context#25185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[Serializer] Do not cache attributes ifattributes in context#25185
Uh oh!
There was an error while loading.Please reload this page.
Conversation
98561d4 to303280dCompare| } | ||
| if (isset($this->attributesCache[$class])) { | ||
| if (isset($this->attributesCache[$class]) && !isset($context['attributes'])) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If$context['attributes'] is defined, we will fill the $this->attributesCache[$class] array with useless values.
IMHO, the correct patch is:
if (!isset($context['attributes']}return$this->extractAttributes($object,$format,$context);}
before the cache layer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Good point, updated.
303280d to6e87382Comparefabpot commentedNov 29, 2017
Thank you@sroze. |
…ntext (sroze)This PR was merged into the 3.3 branch.Discussion----------[Serializer] Do not cache attributes if `attributes` in context| Q | A| ------------- | ---| Branch? | 3.3| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#25108| License | MIT| Doc PR | øCaching attributes based on the class works only when these attributes are not overwritten. This disables the cache when they are.To me, this `extractAttributes` method should actually be a `AttributeResolver` dependency that can be decorated using different caching strategies I'd say but... that's a much bigger refactoring that needs more reflection with@dunglas.Commits-------6e87382 Do not cache cache attributes if `attributes` is in the context
Caching attributes based on the class works only when these attributes are not overwritten. This disables the cache when they are.
To me, this
extractAttributesmethod should actually be aAttributeResolverdependency that can be decorated using different caching strategies I'd say but... that's a much bigger refactoring that needs more reflection with@dunglas.