-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
Metadata caching isn't compatible with prefixed cache keys #338
base: 3.3.x
Are you sure you want to change the base?
Metadata caching isn't compatible with prefixed cache keys #338
Conversation
…ith prefixed cache keys
This would require tests to prevent regressions. |
You mean unit tests with prefixed cache reader? |
You should write a unit test that fails without your changes. |
I wrote this test |
$cacheItems = iterator_to_array($cacheItems); | ||
} | ||
|
||
$loadedMetadataItems = array_combine($loadedMetadata, $cacheItems); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this logic assumes that $cacheItems is in the same order than
$loadedMetadataItems`. However, I don't think PSR-6 makes any guarantee about the order of items in the returned result.
The CacheItemPoolInterface phpdoc only says that the returned iterable is keyed by cache keys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. This led me to an even simpler solution for this code part. Check my newer commit. It iterates over loaded metadata and get item for each, but I think it's not too frequent that $loadedMetadata's count is more than one, so it's maybe not a performance issue.
The metadata cache generation, and cache saving didn't work if prefixed cache system is used.
The old functionality checked if the cache item's key is in
$classNames
array. But this array generated only with thegetCacheKey
function, and because of this the items of this array doesn't contain the global prefix of the cache keys.With the fixed solution we're using only the prefixed keys, and cache item will be saved.