ObjectCache fails to create consistent cache keys in V4

[SvenRtbg]

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.

Q	A
Version	4.0.0 up to 4.1.0

Summary

V4.0.0 and up broke apart the inheritance chain of ObjectCache and CallbackCache. This introduced a significant behaviour change when using ObjectCache which may make it impossible to create cache hits anymore.

Previous behavior

At the end of executing ObjectCache::call, the call is passed to CallbackCache as parent::call(), and there the line $key = $this->generateCallbackKey($callback, $args); does jump back to ObjectCache::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 that Closure to be serialized later - which PHP still cannot do. Again, closures embedded in the object tree will prevent caching as they cannot be serialized with CallbackCache, compared to the previous code in ObjectCache.

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.

class Foo { 
    private int $time; 
    public function __construct() 
    { 
        $this->time = time();
    }
    public function cachedMethod(): string
    {
        return "some_cacheable_value";
    }
}

If a closure is present in any property, serialization will fail.

[froschdesign]

Please check the documentation https://docs.laminas.dev/laminas-cache/v4/migration/to-version-4/
Or the release notes.

[SvenRtbg]

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?

[froschdesign]

It is a new major version, so there may be significant and not backward compatible changes here.

I'll try to create a test case with details.

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.
Thanks in advance! 👍🏻

[SvenRtbg]

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?

• I believe injecting mocks instead of real object into the cache layer is a valid use case - and mocks do change internally to track call states, i.e. even if the original code has no dynamic properties, mocks will.
• Anyways, even the real object tree may have some internal properties that may change. Imagine having a wrapper around Guzzle that for some reason needs to track cookies. Previously the cookies would not affect caching - now they are. You could argue that cookies may have an effect on the cache result, and I agree it's possible - but then again the caching itself should happen on a different level. If developers decided that wrapping on a level is correct, it should not be questioned by the library.
• The last argument: Even if no CHANGING properties are used in the object tree, there might be traces of variance just because calls hitting a server are routed to different machines, and these machines, when building the object tree for some reason include some property that is specific to the machine or the individual call - for example, there might be a trace id created per call that is supposed to be forwarded, but only of that call isn't cached. Dealing with forwarding the trace id into the cache infrastructure is a different task I want to exclude here. If the object tree is treated as a different object each time, no cache hits can happen.

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 object_key pattern option that can be used to label different variants of the same class if needed. By default, it returns the class name of the object. So even if the inheritance between ObjectCache and CallbackCache isn't coming back, this option could be set explicitly and be used to determine a cache identifier that is independent of dynamic data or closures in the object tree. Both classes share the same PatternOptions object - but in it's current state, code in CallbackCache would need to determine the use case (standalone or inside ObjectCache), which doesn't happen right now.

Sorry for the wall of text. I believe we should focus first on "What is ObjectCache supposed to do?" It used to be a very good wrapper for object method calls and did caching simply based on class name and method, with any difference of a use case to be covered in either the storage namespace or the object_key.

[SvenRtbg]

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 ObjectCache is supposed to simply wrap around the original class, and you'd call the original method name to trigger the cache evaluation. The class used in the example isn't too fancy, though, so I wouldn't be able to extrapolate any assumptions about the intended design of dealing with object variants from this simple example.

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 makeId() method at the end of the class that puts together the class name, method, and serialized arguments - i.e. it does NOT care about properties in the original object tree, if that would require different treatments, it was part of the implementation task.

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 object_key, when caching an object.

[SvenRtbg]

I would like to raise awareness again, as there was no feedback until now after I provided a test illustrating the different behavior and a pull request proposing a fix.