-
-
Notifications
You must be signed in to change notification settings - Fork 55
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
ObjectCache fails to create consistent cache keys in V4 #355
Comments
Please check the documentation https://docs.laminas.dev/laminas-cache/v4/migration/to-version-4/ |
Granted, the docs you mention state that "ObjectCache does not inherit from CallbackCache", and the fact that all classes are now final forced to not inherit them anymore, but inject and forward calls - still it is impossible to use ObjectCache the same way it used to be, which isn't mentioned, or is it? |
It is a new major version, so there may be significant and not backward compatible changes here.
Unfortunately I can't say anything about the old or new behaviour, so it would be great if you could provide a test as a pull request. |
I created a repo with Github workflow to illustrate the issue: https://github.com/SvenRtbg/laminas-cache-testcase/actions/runs/12708532107/job/35425679989 The first run tests using Laminas-Cache 3.12.2, then an upgrade is performed to Laminas 4.1.0. The first test run successfully completes, and emits the output "Cache Miss" twice for the two test classes, and does not complain about the Closure property. With the new version, Closure properties cannot be used anymore, and the caching in the other cases is ineffective due to the changing property values - this is illustrated by the new random values that are returned (there is a 0.1% chance the second random value is identical - I didn't want to put call counters into the classes to not interfere with the issue). Why is the new behaviour a problem?
There is a workaround possible: The main object to be cached can implement the "Serializable" interface and return a static value. However that might conflict with the existing need to have this already in place - and in general it feels a bit awkward to surprisingly implement it just to make caching work again. One alternative that looks like it might work, but relevant code is removed in v4: The documentation states there is a Sorry for the wall of text. I believe we should focus first on "What is |
To illustrate where we are coming from, here's a link to the old Zend documentation: https://docs.zendframework.com/zend-cache/pattern/object-cache/#caching-a-filter This example suggests that It's still worth noting that the pattern configuration options are identical to what Laminas states as the current options at https://docs.laminas.dev/laminas-cache/v4/pattern/object-cache/#configuration-options And if we go back to the roots, I'd like to point to the matching code of Zend Framework 1 (maintained at https://github.com/Shardj/zf1-future/blob/master/library/Zend/Cache/Frontend/Class.php) which has a So all in all, I am proposing to restore the behavior that has been present in the best caching library I have found so far, i.e. not serializing the object tree as the input for a cache id, but only use the class name, or alternatively use a single fixed string passed via |
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
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
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 <[email protected]>
Side remark: There seems to be no CHANGELOG.md that I could consult prior to reporting the BC break
BC Break Report
The
\Laminas\Cache\Pattern\ObjectCache
class changed the way it creates the cache key between V3 and v4.0.0.Summary
V4.0.0 and up broke apart the inheritance chain of
ObjectCache
andCallbackCache
. This introduced a significant behaviour change when usingObjectCache
which may make it impossible to create cache hits anymore.Previous behavior
At the end of executing
ObjectCache::call
, the call is passed toCallbackCache
asparent::call()
, and there the line$key = $this->generateCallbackKey($callback, $args);
does jump back toObjectCache::generateCallbackKey()
, which returns the name of the cached object class and the method as a string.Current behavior
Starting with V4.0.0, there is no inheritance chain, and above mentioned line jumps to
CallbackCache::generateCallbackKey()
, which does quite some logic.The most obvious difference is that now the object that is to be cached is passed to
\serialize($object)
to create the cache key. This is bad for multiple reasons, as the object tree that is actually to be cached might be quite large.But the obvious problem we encountered is that inside this object tree that is supposed to execute a SOAP call, we have a profiler class that stores the current time. Said time stamp now is part of the cache key, and will change, effectively disabling the cache.
As a side effect, I discovered that the logic creating the callback key explicitly mentions
Closure
as a case in the comment, but does not prevent thatClosure
to be serialized later - which PHP still cannot do. Again, closures embedded in the object tree will prevent caching as they cannot be serialized withCallbackCache
, compared to the previous code inObjectCache
.How to reproduce
I'll try to create a test case with details.
In summary, any object stack that contains dynamic data in properties will yield changing cache keys.
Injecting this class to be cached may work when executed in the same runtime context, but will fail when used in a web server context.
If a closure is present in any property, serialization will fail.
The text was updated successfully, but these errors were encountered: