fix: RedisCache flush with prefix

[distantnative]

Changelog

🐛 Bug fixes

• Redis cache driver: Fixed flushing all databases even when using a prefix. Now only the Reds database for the cache with its prefix gets flushed.

For review team

• Add changes & docs to release notes draft in Notion

[afbora]

I noticed that RedisCache::flush() temporarily changes OPT_SCAN to SCAN_RETRY, but it does not restore the previous value afterwards.

This probably won’t cause any real issue in normal Kirby usage, but if the same Redis connection is later used for scan(), its behavior would stay changed. To avoid that side effect, would it make sense to store the old OPT_SCAN value before the scan and restore it in finally?

Something like:

$scan = $this->connection->getOption(Redis::OPT_SCAN);


try {
    ...
} finally {
    $this->connection->setOption(Redis::OPT_PREFIX, $prefix);
    $this->connection->setOption(Redis::OPT_SCAN, Redis::SCAN_NORETRY);
    $this->connection->setOption(Redis::OPT_SCAN, $scan);
}

[distantnative]

@afbora
Why setting first once more to Redis::SCAN_NORETRY before setting back to $scan?

    $this->connection->setOption(Redis::OPT_SCAN, Redis::SCAN_NORETRY);
    $this->connection->setOption(Redis::OPT_SCAN, $scan);

[afbora]

@distantnative I tested my first idea again and I think it was not fully correct. First I thought this would be enough:

$this->connection->setOption(Redis::OPT_SCAN, Redis::SCAN_NORETRY);
$this->connection->setOption(Redis::OPT_SCAN, $scan);

But OPT_SCAN does not seem to work like a normal single value. For example, the old value can be 3. That can mean that both SCAN_RETRY and SCAN_PREFIX were enabled. But if we just do this:

$this->connection->setOption(Redis::OPT_SCAN, 3);

PhpRedis can understand that as SCAN_NOPREFIX, so it does not restore the old state correctly. So I think we should first turn both options off, then turn on again only the ones that were enabled before:

$this->connection->setOption(Redis::OPT_SCAN, Redis::SCAN_NORETRY);
$this->connection->setOption(Redis::OPT_SCAN, Redis::SCAN_NOPREFIX);

if (($scan & Redis::SCAN_RETRY) !== 0) {
    $this->connection->setOption(Redis::OPT_SCAN, Redis::SCAN_RETRY);
}

if (($scan & Redis::SCAN_PREFIX) !== 0) {
    $this->connection->setOption(Redis::OPT_SCAN, Redis::SCAN_PREFIX);
}

So basically:

1. reset scan retry
2. reset scan prefix
3. if retry was enabled before, enable it again
4. if prefix was enabled before, enable it again

But I think it needs more tests.
My first test only covered the simple case, so I missed the combined value case.
Now make sense?

[distantnative]

@afbora Maybe I do not understand the Redis behavior here:

$this->connection->setOption(Redis::OPT_SCAN, Redis::SCAN_NORETRY);
$this->connection->setOption(Redis::OPT_SCAN, Redis::SCAN_NOPREFIX);

But it reads like we first set the OPT_SCAN option to the value SCAN_NORETRY. Just to overwrite it in the next line with SCAN_NOPREFIX. Then why do we write the first line at all if we overwrite it right away?

[afbora]

@distantnative I tested this with a small script and it seems OPT_SCAN does not behave like a single overwritten value.

Output:

$ php scripts/redis-scan-option-debug.php
Constants:
SCAN_NORETRY  = 0
SCAN_RETRY    = 1
SCAN_PREFIX   = 2
SCAN_NOPREFIX = 3

Initial                        value=0 retry=off prefix=off
After SCAN_RETRY               value=1 retry=on prefix=off
After SCAN_PREFIX              value=3 retry=on prefix=on

Now disable only retry:
After SCAN_NORETRY             value=2 retry=off prefix=on

Now disable only prefix:
After SCAN_NOPREFIX            value=0 retry=off prefix=off

Reset and restore a combined old value:
After full reset               value=0 retry=off prefix=off
After restoring old value      value=3 retry=on prefix=on

So SCAN_NORETRY only disables the retry flag, but keeps the prefix flag unchanged. Then SCAN_NOPREFIX disables the prefix flag. That’s why both lines are needed if we want to fully reset OPT_SCAN before restoring the previous flags. Now make sense?

Here the script: redis-scan-option-debug.php