Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Serializer] Fix configuration of the cache key #36340

Merged
merged 1 commit into from Aug 17, 2020

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Apr 4, 2020

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #35574 doctrine/orm#8030 (partially)
License MIT
Doc PR n/a

Currently, a bug prevents to configure the context keys to exclude from the cache key computation. The value is always replaced in the constructor. This PR fixes the problem.

@dunglas dunglas changed the title [Serializer] It must possible to configure to keys to exclude for the cache key computation [Serializer] It must be possible to configure the keys to exclude from the cache key computation Apr 4, 2020
@dunglas dunglas changed the title [Serializer] It must be possible to configure the keys to exclude from the cache key computation [Serializer] Fix configuration of the cache key Apr 4, 2020
@alanpoulain
Copy link
Contributor

It's only an issue if the default context for the cache key is changed by a child class and if the constructor is called afterwards, doesn't it?

@nicolas-grekas nicolas-grekas added this to the 4.4 milestone Apr 4, 2020
@nicolas-grekas
Copy link
Member

A test case would be nice if you can.

@dunglas
Copy link
Member Author

dunglas commented Apr 5, 2020

@alanpoulain it's even the case if you just pass the keys to exclude as parameter of the constructor.
@nicolas-grekas test added and exception message fixed

@fabpot
Copy link
Member

fabpot commented Aug 11, 2020

Is this one still relevant? I see that the original issue has been closed as the problem has been fixed in PHP itself IIUC? /cc @dunglas @nicolas-grekas

@dunglas
Copy link
Member Author

dunglas commented Aug 16, 2020

AFAIK it's still an issue.

@fabpot
Copy link
Member

fabpot commented Aug 16, 2020

@dunglas Ok, can you rebase so that we can merge?

@fabpot
Copy link
Member

fabpot commented Aug 17, 2020

Thank you @dunglas.

@fabpot fabpot merged commit 66b9fef into symfony:4.4 Aug 17, 2020
This was referenced Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants