fix: APCu cache condition

[SystemKeeper]

I looked really hard, but to me the condition is wrong. There are 2 cases:

1. If there's no distributed cache, we ensure that the local cache is not set to APCu

OR

2. If the distributed cache is set, it should not be set to APCu

So having:

  'memcache.local' => '\\OC\\Memcache\\APCu',
  'memcache.distributed' => '\\OC\\Memcache\\Redis',

should be perfectly fine. But from the current code, it is invalid, since $distributedCacheClass === '' is already false and everything is combined with &&.

After changing the code to || I hit the createDistributed line in my local environment correctly.

[bigcat88]

Thanks for finding this!
What do you think about such change:

if (
    ($distributedCacheClass === '' && $localCacheClass !== \OC\Memcache\APCu::class)
    || ($distributedCacheClass !== '' && $distributedCacheClass !== \OC\Memcache\APCu::class)
) {
    $this->cache = $cacheFactory->createDistributed(Application::APP_ID . '/service');
}

For the case when memcache.distributed=`` and memcache.local=`APCu` ?

[SystemKeeper]

What do you think about such change

Yes, makes sense. Otherwise we would end up again with APCu as the cache. I updated the PR accordingly.

[SystemKeeper]

Now I am wondering, would we need to check $localCacheClass !== '' as well, or is it fine for your use-case?

Edit: Seems to be fine from a quick glance. In that case the NullCache is assigned, which would return null on get anyway and we fallback to querying the db.

[oleksandr-nc]

yep, with the adjusted condition I guess we are fine

[oleksandr-nc]

/backport to stable32

[oleksandr-nc]

/backport to stable31

[oleksandr-nc]

@SystemKeeper can you please initiate backports? looks like bot ignores me 🤔

[SystemKeeper]

It's not you, the bot is down :-(

[SystemKeeper]

/backport to stable32

[SystemKeeper]

/backport to stable31