TTL handling broken, always permanent

This module now has tests on merge requests ( doesn't show up here), and they are failing, which is expected based on what was discussed in the related github issue.

This patch has been required for Pantheon customers hitting maxmemory limit and is critical to evicting keys as expected using LRU

I can't tell whether the automated test needs to change or if the behavior itself has a bug. I've merged in the most recent release to the 8.1 branch, and the error that I'm seeing from PHPUnit is:

1) Drupal\Tests\redis\Kernel\RedisCacheTest::testSetGet
Failed asserting that false is of type "object".
/builds/issue/redis-3179757/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:122

I believe that the automated test itself needs to be updated, but I'm not certain. Anyone have a firm answer on that?

See the duplicate issue #2877893: Cache Expiration doesn't follow Drupal conventions → where I've explained this about 7 years ago :)

The test is from core. Core expects that the API allows to return expired cache items if you ask for that. Setting a TTL in redis is incompatible with the expectation that core has for a cache backend implementation.

The only way to get tests to pass is to make this behavior configurable. Either a boolean enable/disable, or an idea I just had, is that we add a ttl offset setting. So that can be set to e.g. 300, then any ttl we receive is extended by 300s, which means that for this timeframe, we can still return expired items, allowing certain use cases like returning expired items while one process recalculates it. Then we just need to configure that specific test to have enough ttl offset for the tests to still work.

Any updates?

Rerolled patch for 1.8 version of the module without the change to the line:

$this->permTtl = ($iv->y * 31536000 + $iv->m * 2592000 + $iv->days * 86400 + $iv->h * 3600 + $iv->i * 60 + $iv->s);

This line is conflicting with the issue/patch https://www.drupal.org/node/2944938 → , since we are using both patches on our projects.

Again, per #21, this is incompatible with test expectations that core has for cache backends and it will never pass tests, don't bother debugging that, that's by design. Core expects expired items to be returned when asked for. This changes it so that it doesn't.

🐛 Allow to remove support for invalidateAll() and treat it as deleteAll() Active is an example of what I'm asking for. Add a setting that allows to make this a configurable opt-in behavior, it absolutely makes sense, but I want to have redis by default correspond to core cache backend expecatations as much as possible.

I closed one issue as a duplicate, I think I'd prefer to also close ✨ Default lifetime for permanent items for Drupal 8 Needs review as a duplicate and merge them together.

Also, if this is a behavior that is important for your site, feel free to reach out and sponsor a few hours of my time through MD Systems to finish this up with tests and documentation and I'm happy to commit it.