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

Fix cache configuration for better Doctrine compatibility. #806

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

demiankatz
Copy link
Contributor

@demiankatz demiankatz commented Jun 8, 2023

The default Laminas cache key pattern is incompatible with Doctrine's basic assumptions. This PR loosens things up a bit. It might not be perfect -- I'm open to suggestions for cleaner regexes -- but it at least works with the current DoctrineORMModule test suite.

Additionally, the old Doctrine file system cache adapter included automatic serialization, but the Laminas file system cache adapter does not. This adjusts the configuration to fix that inconsistency and ensure compatible behavior.

See doctrine/DoctrineORMModule#734 for related discussion.

@demiankatz demiankatz changed the title Make cache key pattern more compatible with Doctrine. Fix cache configuration for better Doctrine compatibility. Jun 8, 2023
@demiankatz
Copy link
Contributor Author

I see there are some style issues that need addressing; I have to run for a few minutes, but I'll fix them as soon as I get back!

@demiankatz
Copy link
Contributor Author

Okay, style issues fixed -- this should be ready for review, if @greg0ire or @TomHAnderson has a moment.

@TomHAnderson TomHAnderson added this to the 6.0.2 milestone Jun 8, 2023
@TomHAnderson TomHAnderson merged commit 07b4e0f into doctrine:6.0.x Jun 8, 2023
@demiankatz demiankatz deleted the flexible-key-pattern branch June 8, 2023 15:43
@demiankatz
Copy link
Contributor Author

Thanks, @TomHAnderson! I don't expect to need to do any more work here in the immediate future, so if you don't mind releasing 6.0.2, that would be fantastic. :-)

@Novynn
Copy link

Novynn commented Jun 9, 2023

This change is breaking things. An example is: annotation methods are cached by Doctrine using a hash symbol (ie. #) which isn't included in the regex.

@demiankatz
Copy link
Contributor Author

Feel free to open a new PR to further open up the regex. I'm not familiar enough with doctrine to know all the possibilities, but it was even more broken before my changes. I'd appreciate any help you can offer to fix it the rest of the way!

@TomHAnderson
Copy link
Member

TomHAnderson commented Jun 9, 2023

Reconfiguring the caching is a big change, hence the major release. If you can improve the regex or give examples of the problem you're seeing it would be most helpful.

@demiankatz
Copy link
Contributor Author

It would probably be a good idea to have a unit test that exercises common cache keys to prevent regressions like this. I'll see if I can help with that in the near future!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants