search

doctrine / DoctrineModule

Doctrine Module for Laminas
http://www.doctrine-project.org/projects/doctrine-module.html
MIT License
398 stars 270 forks source link

Fix cache configuration for better Doctrine compatibility. #806

Closed demiankatz closed 1 year ago

demiankatz commented 1 year ago

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 https://github.com/doctrine/DoctrineORMModule/pull/734 for related discussion.

demiankatz commented 1 year ago

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 commented 1 year ago

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

demiankatz commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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!