Fixed simpleCache serious bug?

[Siropu]

Affected version

DP10

So I've used this code:

$simpleCache = \XF::app()->simpleCache();
$simpleCache['Siropu\Chat']['key'] = $myFinderResultObject;

And now, every XF page is a blank page with no errors.

 

[Siropu]

I've done a reinstall and tried the same thing without \ for the add-on id and the result is the same, a blank page on any page. Seems to be an issues with storing objects.

 

[Mike]

The short answer is really that we aren't expecting you to try to serialize finders, entities or collections (though most specifically entities). The strong likelihood is that we'll just block that.

However, that's definitely not the sort of thing you should be storing in simple cache.

 

[Siropu]

I didn't even serialized the object. I was assuming that it handles everything but I guess I was wrong.
Are there any other built in alternatives? I want to cache the finder result object so I can apply its methods when needed for each user.

 

[Xon]

The stuff you are putting into simpleCache should be very static and simple, and used without fail on every pageload. If it is constantly changing, the either suck up the SQL queries or use the XF cache infrastructure which can be hitting redis or memcache.

Are there any other built in alternatives? I want to cache the finder result object so I can apply its methods when needed for each user.

Yes, you serialize it to an array explicitly and then later use "instantiateEntity". This instantiateEntity is the same dehydrate method used to turn a key/value set into an entity.

This is what I'm doing in my as yet unreleased XF2 Content Rating add-on.

    public function getRatingTypeCacheData()
    {
        $ratingTypes = $this->finder('SV\ContentRatings:RatingType')
            ->order(['display_order', 'title'])
            ->whereOr(['visible' => 1], ['usable' => 1])
            ->fetch();

        $cache = [];

        foreach ($ratingTypes as $ratingTypeId => $ratingType)
        {
            // cache the entire object, as we re-hydrate into an entity later and don't want to accidentally wipe disabled settings
            $cache[$ratingTypeId] = $ratingType->toArray();
        }

        return $cache;
    }

    public function getRatingTypesAsEntities()
    {
        // this can be called multiple times per rateable content being displayed, so keep a local cache
        if ($this->_ratingEntities === null)
        {
            $ratingEntities = [];
            $ratingTypes = $this->getRatingTypes();
            foreach($ratingTypes as $ratingTypeId => $ratingType)
            {
                $ratingEntities[$ratingTypeId] = $this->em->instantiateEntity('SV\ContentRatings:RatingType', $ratingType);
            }
            $this->_ratingEntities = $ratingEntities;
        }

        return $this->_ratingEntities;
    }

I then use \XF::registry()->set() & \XF::registry()->set() at some stage to populate the cache across pageloads. Remember to call \XF::registry()->delete() in your installer

 

[Mike]

There is a potentially dangerous thing with instantiating an entity from cache like that. You can do it if you're careful and understand the risks, but just to be clear...

If you instantiate an entity like that, those values will be put into the entity cache for that page view. A subsequent attempt to load that entity will pull it from the cache. So if there's ever any chance that your cache is outdated, then that could lead to potentially confusing behavior. As an example, if you load the cache at the beginning of every page load, if there was a change in the DB and then you went to edit that entity in the ACP, you'd probably see the value in the cache rather than the DB; saving would then actually push that cache value into the DB. Extending that, rebuilding the cache would rebuild it from what already exists in the cache. (You can detach entities from the cache so subsequent fetches give new values, though there are also theoretical risks with that, depending on behaviors you rely on.)

Conversely, if we allowed serialization but actually pulled from the DB on unserialize, then we risk getting different values than when saved, which might not be clear with the serialization concept. (It would be clear if you understood what was happening but if you think you're storing values and they suddenly change, that could certainly be confusing.)

So while doable, I'd say here be dragons...

 

[Xon]

@Mike I reviewed the code path and yeah. "here be dragons" is right.

In my case it is OK that the admincp is seeing the cached values, editing them and then persisting those to the DB. Since that is what everyone else is interacting with anyway. In the general case I wouldn't recommend it.

Also; instantiateEntity will allocate the Entity object even if it doesn't use it due to an existing cached entity. Do you want me to report that as a bug?

 

[Mike]

Also; instantiateEntity will allocate the Entity object even if it doesn't use it due to an existing cached entity. Do you want me to report that as a bug

I assume you mean when it hits the true path here?

if (isset($this->entities[$class][$primary]))

If so, then no, that would be expected. There should only ever be one instance of a given entity at a time (at least that's attached). This means that if changes happen (or are pending), they are reflected in all places that reference that entity (given PHP's passing object by reference), including any new instantiation requests.

 

[Xon]

If so, then no, that would be expected. There should only ever be one instance of a given entity at a time (at least that's attached). This means that if changes happen (or are pending), they are reflected in all places that reference that entity (given PHP's passing object by reference), including any new instantiation requests.

XF2 unconditionally create the entity, if values are passed in then check to see if it exists in the cache.

My suggestion is to only create the entity object if it is needed in the first place (ie blank or a new entity to be populated with data, which is not in the cache)

 

[Siropu]

What about storing multi-dimensional array in simpleCache? Would that be a problem?

 

[Snog]

What about storing multi-dimensional array in simpleCache? Would that be a problem?

I hope not because that's exactly what I have stored in simplecache for a couple of add-ons. And the array is serialized, which shouldn't be a problem either. I think the problem being discussed is about an entity being stored, which would be unrelated to an array.

I use the simplecache to store a set of permissions in an array that are unique to my add-ons, but are required on just about every page. Those permissions are set in a special admin page, and nowhere else. So once set, typically they would never (or rarely) change.

 

[Mike]

I've now blocked serialization of a number of problematic objects would should prevent bad data from getting into caches like this in the first place. Most notable example includes the entities, but it also includes things like finders, the entity manager, and the DB adapter.