2.0 - Cache for 'all-modules' doesn't actually cache

[ric2016]

When initializing custom modules via all() in ModuleService.php, the modules are supposed to get cached via

return app('cache.array')->remember('all-modules', function (): Collection {
etc

This doesn't work, leading to the initialization code getting executed repeatedly. This apparently leads to further errors (the instance of the custom module that is ultimately used is not the one on which boot() has been called, for one thing).

You can verify this via adjusting the remember method in Cache.php:

public function remember(string $key, Closure $closure, int $ttl = null)
    {
        $debug = ("all-modules" == $key);
        if ($debug) {
          error_log("remember?".$key);
        }      
        $ret = $this->cache->get(md5($key), static function (ItemInterface $item) use ($closure, $ttl) {
            $item->expiresAfter($ttl);

            return $closure();
        });
        if ($debug) {
          if ($this->cache->getItem(md5($key))->isHit()) {
            error_log("hit");
          } else {
            error_log("miss");
          }
        }
        return $ret;
    }

The error log shows cache misses directly after the item is supposed to be set via cache->get. This is caused by ArrayAdapter.php returning false in

if ($this->storeSerialized && null === $value = $this->freeze($value, $key)) {

So it's apparently caused by freeze not working as expected in this case. Consequently, the issue is resolved by using a non-serializing adapter in UseCache.php:

$this->array_adapter = new ArrayAdapter($defaultLifetime = 0, $storeSerialized = false);

I'm not sure about the implications of this change though. As this is an in-memory cache, it may be preferable anyway not to serialize? Otherwise, we'd have to drill deeper in order to find the reason why the freeze doesn't work as expected in this case.

[fisharebest]

As this is an in-memory cache, it may be preferable anyway not to serialize?

This is probably correct.

[fisharebest]

Can you tell me how I could reproduce this error?

Ignore - I found it just after I posted.

[ric2016]

While this fixes the error, I just realized it may not be a good idea to rely on the cache returning a specific instance that is further initialized elsewhere (as the bug demonstrated).

It would be more cleaner to fully initialize the custom modules within the cache value initialization function (not sure if this is easily possible). I'll take a look at the code, this isn't urgent.

[fisharebest]

It would be more cleaner to fully initialize the custom modules within the cache value initialization

In #2860, the hit-count module has two roles.

• middleware - calculate the data and store it
• footer - display the data that we stored earlier

The logic relies on the exact-same object being available each time.

I guess it is an unusual case, because the module provides two roles.
But perhaps it is a bad design to allow modules to store local data?
Our business logic ("services") are just functions and do not have "state".

Should modules be the same? i.e. the output of functions depends only on the input, and not on any previous function calls?

It would be more cleaner to fully initialize the custom modules within the cache value initialization function (not sure if this is easily possible)

I agree that it is cleaner (and more maintainable) if modules are "stateless".

It is easy to find these cases - they have local variables that are modified after the constuctor is called.

One difficulty is that we do not boot all modules. We don't know which ones to boot until after we have created them, checked permissions and settings, etc.