search

doctrine / DoctrineModule

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

6.0.2 regex change is too restrictive #807

Closed Novynn closed 1 year ago

Novynn commented 1 year ago

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

The initial regex that was breaking the tests in https://github.com/doctrine/DoctrineORMModule/pull/734 is only relevant to the "Filesystem" storage adapter.

https://github.com/laminas/laminas-cache-storage-adapter-filesystem/blob/d78283f47a7d0fd3968c63836ee2805b53bff06f/src/FilesystemOptions.php#L27

TomHAnderson commented 1 year ago

Can you give a full key that uses the hash?

Novynn commented 1 year ago

The hash is used here:

https://github.com/doctrine/annotations/blob/4b68cf86b766ec429f4f68af648817cdfb360582/lib/Doctrine/Common/Annotations/PsrCachedReader.php#L113

TomHAnderson commented 1 year ago

As @Novynn has pointed out, /^[a-z0-9_\+\-\[\]\\\\$]*$/Di is the regex for the Filesystem cache. Because that's the case does it make sense to include a regex for each cache?

demiankatz commented 1 year ago

Good point, I'll fix it.

demiankatz commented 1 year ago

See #809. This should be a more focused fix now, and it also has test coverage. Thanks, @Novynn!

Novynn commented 1 year ago

Thank you to @TomHAnderson and @demiankatz for sorting this out. Moving the filter to the filesystem has solved any concerns I had as we don't use the adapter. I'm sure using the filesystem to cache has all kinds of restrictions.