[Cache] Hash cache key on save #23763
Conversation
lstrojny
changed the title
|
you can add a test case in |
|
@nicolas-grekas added a case to the MemcachedAdapterTest |
|
can we move it to the FilesystemAdapterTest ? I think it may have the issue also, and this would allow more people to actually run the test |
| $byLifetime = array(); | ||
| $now = time(); | ||
| $expiredIds = array(); | ||
|
|
||
| foreach ($deferred as $key => $item) { | ||
| $id = $getId(is_int($key) ? (string) $key : $key); |
There was a problem hiding this comment.
I would call this inline (no need for the $id intermediary variable)
and I would move the cast to string inside the closure.
also, now need of the is_int check, we should unconditionally cast to string instead
|
wait, it really needs to be in AdapterTestCase, all of them need to run that test |
|
it should also be in Simple\CacheTestCase, just for parity |
|
Could even be part of https://github.com/php-cache/integration-tests, with even longer keys. |
|
Thank you @lstrojny. |
This PR was merged into the 3.3 branch. Discussion ---------- [Cache] Hash cache key on save | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | ye | Fixed tickets | n.A. | License | MIT | Doc PR | n.A. Cache keys are not hashed right now in adapters extending from `AbstractAdapter`. This PR fixes this. I am not familiar enough with the cache test suite so I don't know where to add an regression test. Commits ------- 94b1b12 Hash cache keys on save
Cache keys are not hashed right now in adapters extending from
AbstractAdapter. This PR fixes this. I am not familiar enough with the cache test suite so I don't know where to add an regression test.