tribe-common
tribe-common copied to clipboard
Handle model caching correctly
Ticket: TEC-4379
Artifacts: tests and screencast
This commit updates the logic used in the Post decoration process to make sure it plays nice with different caching solutions.
Depending on the specific caching solution or plugin, the failure will
come down to PHP trying to build, during an unserialize
call, objects
of not yet defined classes.
This might happen for 2 main reasons:
- When the object was cached plugins A, B and C were active; if only A is active when the object is unserialized, then this will lead to a cascading failure when PHP tries to build objects of classes defined by the plugins B and C.
- The object is unserialized before the plugins defininig the classes are loaded, the result will be, as is the case for 1, a cascading failure during unserialization.
Unserialization failures will trigger, by default, a warning
(E_WARNING
) that will not stop the code execution and will
propagate incoherent of partially built values down the execution line.
The main cause of this kind of issues would be posts cached in the
posts
group. Functions like tribe_get_event
will decorate these post
objects adding dynamic properties to them (e.g., is_now
or
start_date
or venues
). Any one of these properties that is not a
scalar or an instance of an object defined by Core PHP (e.g. stdClass
or DateTime
) will likely cause this failure.
The posts
cache group, in particular, is consistenly pre-fetched by
many caching solutions when trying to push performance, it's beyond TEC
control.
Given TEC code, and other developer codes, depends on the dynamic
properties mentioned above, we cannot remove them.
Also, we cannot control the serialization of a WP_Post
instance as
it is unfiltered internal code.
With these premises the solution is two-fold:
- Make sure no property dynamically added to
WP_Post
instances is an object of a class not defined by PHP Core. - Alleviate the cost of re-decorating each
WP_Post
instance by caching the proerties in a way under our control (a_tec_pre_serialized
property is added to theWP_Post
) - Memoize the decorated
WP_Post
instance, do not cache it.
PHP 8.2 will deprecate dynamic properties, i.e., all the ones we
decorate the WP_Post
instance with. The WP_Post
class will look for
undefined properties in the meta (see WP_Post::__get
), but meta lookup
is really inefficient since the value will be filtered each time it's
fetched. I've thought about using this refactoring to mitigate that
problem too, but the Core team is steering toward the wide-spread use of
the AllowDynamicProperties
attribute; I've hence decided not to move
dynamic properties to meta.