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

SimpleCacheAdapter fails to cache any item if a namespace is used #32025

Merged

Conversation

moufmouf
Copy link
Contributor

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

This is a backport of #32019

The SimpleCacheAdapter extends AdapterTestCase.
When adding a namespace, the AdapterTestCase adds ":" after the namespace:

https://github.com/symfony/symfony/blob/v4.3.1/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php#L37

The namespace is prepended to the cache key.
But in PSR-16, the ":" is a forbidden character.

As a result, the cache key is invalid and cache is not persisted. If you use Psr16Adapter + a namespace, the cache simply does not work.

As per @nicolas-grekas advices, a NS_SEPARATOR const is added to change the namespace separator for the SimpleCacheAdapter to "_" (that is compatible with PSR-16).

The first commit of this PR starts with an additional test and no fix (to showcase the problem).

@moufmouf moufmouf force-pushed the broken_simplecacheadapter_with_namespace branch 4 times, most recently from ca39d00 to b50221c Compare June 13, 2019 14:17
@moufmouf
Copy link
Contributor Author

@nicolas-grekas Ready for review. I think I took into account all your comments.

What do you want me to do with #32019? Shall I close it (if you want to backport the bug from this version?) or shall I continue working on it to make it in line with this PR?

@fabpot
Copy link
Member

fabpot commented Jun 14, 2019

Thank you @moufmouf.

@fabpot fabpot force-pushed the broken_simplecacheadapter_with_namespace branch from 9f8be54 to ffd3469 Compare June 14, 2019 11:16
@fabpot fabpot merged commit ffd3469 into symfony:3.4 Jun 14, 2019
fabpot added a commit that referenced this pull request Jun 14, 2019
…is used (moufmouf)

This PR was squashed before being merged into the 3.4 branch (closes #32025).

Discussion
----------

SimpleCacheAdapter fails to cache any item if a namespace is used

| Q             | A
| ------------- | ---
| Branch?       |3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

This is a backport of #32019

The SimpleCacheAdapter extends AdapterTestCase.
When adding a namespace, the AdapterTestCase adds ":" after the namespace:

https://github.com/symfony/symfony/blob/v4.3.1/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php#L37

The namespace is prepended to the cache key.
But in PSR-16, the ":" is a forbidden character.

As a result, the cache key is invalid and cache is not persisted. If you use Psr16Adapter + a namespace, the cache simply does not work.

As per @nicolas-grekas advices, a NS_SEPARATOR const is added to change the namespace separator for the `SimpleCacheAdapter` to "_" (that is compatible with PSR-16).

The first commit of this PR starts with an additional test and no fix (to showcase the problem).

Commits
-------

ffd3469 SimpleCacheAdapter fails to cache any item if a namespace is used
This was referenced Jun 26, 2019
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

4 participants