Skip to content

[Cache] Added reserved characters constant for CacheItem #33516

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 11, 2019
Merged

[Cache] Added reserved characters constant for CacheItem #33516

merged 1 commit into from
Sep 11, 2019

Conversation

andyexeter
Copy link
Contributor

Q A
Branch? 4.4
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR N/A

This PR introduces a RESERVED_CHARACTERS constant to the CacheItem class to keep things DRY.

The rationale for making the constant public is so that developers can access the list of reserved characters so they could - for example - sanitise keys before passing them to the Cache component.

@nicolas-grekas nicolas-grekas added this to the next milestone Sep 9, 2019
derrabus
derrabus approved these changes Sep 9, 2019
View reviewed changes
@derrabus
Copy link
Member

derrabus commented Sep 9, 2019

On second thought: Those reserved characters are not implementation-specific, right? Maybe we could add them to the contracts' CacheItemInterface instead?

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Sep 10, 2019 •
edited
Loading

On ItemInterface in contracts, good idea.
Don't miss bumping the minimum version of the cache-contracts to ^1.1.7 for the component.

@andyexeter
Copy link
Contributor Author

Don't miss bumping the minimum version of the cache-contracts to ^1.1.7 for the component.

@nicolas-grekas Could you confirm you are saying I should change the following:

symfony/src/Symfony/Component/Cache/composer.json

Line 27 in f0aff8c

"symfony/cache-contracts": "^1.1|^2",

To this:

"symfony/cache-contracts": "^1.1.7|^2",

@andyexeter
Copy link
Contributor Author

The constant has been moved to ItemInterface and composer.json has been updated to bump the cache-contracts minimum version.

@fabpot
Copy link
Member

fabpot commented Sep 11, 2019

Thank you @andyexeter.

@fabpot fabpot mentioned this pull request Sep 11, 2019
Closed
@fabpot fabpot closed this in 9c3345f Sep 11, 2019
@fabpot fabpot merged commit d853306 into symfony:4.4 Sep 11, 2019
@yellow1912
Copy link

yellow1912 commented Sep 14, 2019 •
edited
Loading

I started getting this error today after updating. I wonder if this is related to the new change:

Undefined class constant 'RESERVED_CHARACTERS'

image

@yellow1912 yellow1912 mentioned this pull request Sep 16, 2019
Closed
@xabbuh xabbuh mentioned this pull request Sep 16, 2019
Merged
xabbuh added a commit that referenced this pull request Sep 16, 2019
@xabbuh
bug #33597 [Cache] bump required symfony/contracts version (xabbuh)
24faadc 
This PR was merged into the 4.4 branch.

Discussion
----------

[Cache] bump required symfony/contracts version

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #33593
| License       | MIT
| Doc PR        |

make the changes from #33516 work when `symfony/symfony` is required instead of `symfony/cache`

Commits
-------

343d01f bump required symfony/contracts version
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
This was referenced Nov 12, 2019
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@derrabus derrabus derrabus approved these changes

Assignees
No one assigned
Labels
Cache Status: Reviewed
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.
6 participants
@andyexeter @derrabus @nicolas-grekas @fabpot @yellow1912 @carsonbot