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

Compatibility with PSR-16/PSR-6 #7

Open
eweso opened this issue Feb 5, 2021 · 3 comments
Open

Compatibility with PSR-16/PSR-6 #7

eweso opened this issue Feb 5, 2021 · 3 comments

Comments

@eweso
Copy link

eweso commented Feb 5, 2021

Feature Request

Summary

PSR-16 allows "A-Z, a-z, 0-9, , and ." as valid key characters for the cache item name. But the key name pattern of the Filesystem storage is "/^[a-z0-9+-]*$/Di" and therefore doesn't match PSR-16 since "." isn't allowed here. This leads to errors when using the simple cache decorator on libraries such as "browscap/browscap-php" that has a cache item named "browscap.version".

Current behavior

Using the simple cache decorator with a laminas cache storage such as Filesystem throws an Exception when a dot is used inside the cache item name.

Expected behavior

The cache storage should accept key names with a dot since it's specified by PSR-16 and breaks standard compatibility with the simple cache decorator. A valid key pattern for the Filesystem storage could be this one "^[a-z0-9_+-.]*$/Di".

@boesing boesing transferred this issue from laminas/laminas-cache Feb 6, 2021
@boesing
Copy link
Member

boesing commented Feb 6, 2021

Filesystem Storage does not fully support PSR-16.

$this->skippedTests['testSetTtl'] = $ttlMessage;
$this->skippedTests['testSetMultipleTtl'] = $ttlMessage;
$this->skippedTests['testSetValidKeys'] = $keyMessage;
$this->skippedTests['testSetMultipleValidKeys'] = $keyMessage;

The problem is, that you cannot specify dynamic TTL per cache key and thus, you have exactly one possible TTL which is not useful when you are using the PSR-16.

So I would not recommend using Filesystem in combination with PSR-6 or PSR-16.
Filesystem cache items are not being "deleted" when they expire as they do not hold their own TTL.
That could be an improvement but its not implemented like this for now.

Feel free to provide a PR with proper implementation. I am happy to merge this for the next major as it would be a BC break anyways.

I've implemented this in some of my projects like this:

  • Write another file which contains the TTL
  • On read, read the TTL file first and then verify the filemtime of the file containing the cached data
  • delete the file if TTL exceeded

But this will require pretty much I/O so today, I would probably change the internal cache file format to pass the TTL with the cached data. So in case filemtime did not exceed TTL, returning the cached data would be fine. If its exceeded, treat it as the cache isnt available and delete the file accordingly.


So, regarding this, I would change the Issue type from Bug to Feature Request as the Filesystem adapter never stated that it is compatible with PSR-16.

@boesing boesing changed the title Cache key pattern doesn't match PSR-16 Compatibility with PSR-16 Feb 6, 2021
@boesing boesing changed the title Compatibility with PSR-16 Compatibility with PSR-16/PSR-6 Dec 1, 2021
@boesing boesing pinned this issue Mar 29, 2022
@montypanday
Copy link

Hi team,

I encountered the same issue with using PhpSpreadsheet.

The key 'phpspreadsheet.65e162e5970b33.58521221.A1' doesn't match against pattern '/^[a-z0-9_+-]*$/Di'

Using Laminas filesystem cache adapter with the SimpleCacheDecorator

Kind regards

@boesing
Copy link
Member

boesing commented Mar 1, 2024

The key 'phpspreadsheet.65e162e5970b33.58521221.A1' doesn't match against pattern '/^[a-z0-9_+-]*$/Di'

That is due to the default key pattern. You can change that and allow . by passing the appropriate option.
The cache decorator just receives a configured Adapter, so it would also fail without simple cache and thus not a PSR-16 issue but configuration issue.

But we can adapt key pattern in the future to allow dots. I do not see a big deal with that.

Would you mind creating an RFC in a new issue? I do not have a need for this and thus having first hand infos such as your library ur using, etc. would provide a good base to discuss widening the key pattern.

Thanks in advance.

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

No branches or pull requests

2 participants