Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Psr16Adapter fails to cache any item if a namespace is used#32019

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@moufmouf
Copy link
Contributor

QA
Branch?3.4
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
LicenseMIT

The Psr16Adapter 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.

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

@moufmoufmoufmoufforce-pushed thebroken_psr16adapter_with_namespace branch from635206e to500fba8CompareJune 13, 2019 08:12
@moufmoufmoufmouf changed the titlePsr16Adapter fails to cache any item if a namespace is used[WIP] Psr16Adapter fails to cache any item if a namespace is usedJun 13, 2019
@moufmoufmoufmoufforce-pushed thebroken_psr16adapter_with_namespace branch fromb8158ea to02fd552CompareJune 13, 2019 09:23
@moufmoufmoufmouf changed the title[WIP] Psr16Adapter fails to cache any item if a namespace is usedPsr16Adapter fails to cache any item if a namespace is usedJun 13, 2019
@nicolas-grekas
Copy link
Member

Wow, thanks for spotting. The bug exists on 3.4, inSimpleCacheAdapter.
The fix is wrong: the currently attached patch changesAbstractAdapter which is used by all adapters.
OnlyPsr16Adapter/SimpleCacheAdapter is affected, so that's where the fix should go.
It also doesn't work when versioning is enabled since versioning adds another:.
Instead, we could maybe make the: an@internalNS_SEPARATOR const (because PHP5.5 on 3.4 doesn't allow protected consts) that we reference in AbstractAdapter usingstatic::NS_SEPARATOR?

@stof
Copy link
Member

Why would we need to use different separators per implementation, rather than using a separator accepted by all ?

@nicolas-grekas
Copy link
Member

Two reasons, on a very different scale:

  • changing the separator means invalidating all existing caches, better not when it's not needed
  • using a separator that is forbidden makes it 100% sure that no collision is possible, that's desired.

@moufmouf
Copy link
ContributorAuthor

Ok, opened a new PR#32025 that targets Symfony 3.4.

Shall I close this one or continue working on it too?

@nicolas-grekas
Copy link
Member

Nope, no need, thanks.

fabpot added a commit that referenced this pull requestJun 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       | MITThis is a backport of#32019The 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#L37The 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
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@moufmouf@nicolas-grekas@stof@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp