Open
Conversation
With the removal of inheritance between ObjectCache and CallbackCache, the way cache keys are created changed for ObjectCache. Previously CallbackCache was calling code in the ObjectCache class to create a cache key based on the class name and method called. This fix restores this by detecting if there is an object saved in the PatternOptions, which is taken as an indicator we are dealing with the ObjectCache use case. As a side effect, this fix restores the ability to user-define the configuration "object_key" in case multiple variants of the same class need to be treated differently, as documented for ObjectCache pattern options. resolves laminas#355 Signed-off-by: Sven Rautenberg <sven@rtbg.de>
The closure prevents the object from getting serialized, which should not happen during tests, as the ObjectCache code path must not serialize the object. Signed-off-by: Sven Rautenberg <sven@rtbg.de>
SvenRtbg
force-pushed
the
fix-proposal-355
branch
5 times, most recently
from
ccf2b46 to
1263af4
Compare
Signed-off-by: Sven Rautenberg <sven@rtbg.de>
SvenRtbg
force-pushed
the
fix-proposal-355
branch
from
1263af4 to
eb54767
Compare
Member
|
@boesing |
Author
|
I'd like to draw attention towards this PR again, as I would still like it to be merged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
I discovered that
\Laminas\Cache\Pattern\ObjectCache, after being untangled and not inheriting fromCallbackCacheanymore, has changed the way the cache key computation is done and now is sensitive to all properties being present, and cannot deal with Closures present in them.This is described in issue #355.
Expected behaviour (identical with V3.x):
ObjectCacheallows to cache object trees that contain Closures, as they are not used to create a cache key.ObjectCacheis consistently creating the same cache key independent from any property values that are contained in the object tree.Observed behavior with V4.0 up to v4.1.0:
ObjectCachethrows an exception when encountering a Closure in a property.ObjectCachecreates differing cache keys if properties change inside the object tree.This proposal adds a test case indicator by simply forcing a useless Closure into the
\LaminasTest\Cache\Pattern\TestAsset\TestObjectCache, which prevents serializing this object.Then a code path is added to
CallbackCachethat detects if an object is present in thePatternOptions, which is taken as an indicator we are dealing withObjectCacheforwarding the call intoCallbackCache, and apply the previously existing cache key logic that was present inObjectCache::generateCallbackKey()from version 3.x. This will restore the previous behavior.This PR targets the 4.2.x branch, as there is currently only a minor lock file commit after the 4.1.x branch, and merging this fix may be considered serious enough to warrant a new minor release, as it reverts a major version change I consider to be accidental.