diff --git a/lib/private/Memcache/Redis.php b/lib/private/Memcache/Redis.php index edf9a80aad497..e29bbcf141283 100644 --- a/lib/private/Memcache/Redis.php +++ b/lib/private/Memcache/Redis.php @@ -32,8 +32,8 @@ class Redis extends Cache implements IMemcacheTTL { '75526f8048b13ce94a41b58eee59c664b4990ab2', ], 'caSetTtl' => [ - 'if redis.call("get", KEYS[1]) == ARGV[1] then redis.call("expire", KEYS[1], ARGV[2]) return 1 else return 0 end', - 'fa4acbc946d23ef41d7d3910880b60e6e4972d72', + 'if redis.call("get", KEYS[1]) == ARGV[1] then return redis.call("expire", KEYS[1], ARGV[2]) else return 0 end', + '94eac401502554c02b811e3199baddde62d976d4', ], ]; @@ -65,14 +65,23 @@ public function get($key) { return self::decodeValue($result); } - #[\Override] - public function set($key, $value, $ttl = 0) { - $value = self::encodeValue($value); + private static function normalizeTtl(int $ttl): int { if ($ttl === 0) { // having infinite TTL can lead to leaked keys as the prefix changes with version upgrades - $ttl = self::DEFAULT_TTL; + return self::DEFAULT_TTL; } - $ttl = min($ttl, self::MAX_TTL); + + if ($ttl > 0) { + return min($ttl, self::MAX_TTL); + } + + return $ttl; + } + + #[\Override] + public function set($key, $value, $ttl = 0) { + $value = self::encodeValue($value); + $ttl = self::normalizeTtl((int)$ttl); return $this->getCache()->setex($this->getPrefix() . $key, $ttl, $value); } @@ -111,11 +120,7 @@ public function clear($prefix = '') { #[\Override] public function add($key, $value, $ttl = 0) { $value = self::encodeValue($value); - if ($ttl === 0) { - // having infinite TTL can lead to leaked keys as the prefix changes with version upgrades - $ttl = self::DEFAULT_TTL; - } - $ttl = min($ttl, self::MAX_TTL); + $ttl = self::normalizeTtl((int)$ttl); $args = ['nx']; $args['ex'] = $ttl; @@ -187,11 +192,7 @@ public function ncad(string $key, mixed $old): bool { #[\Override] public function setTTL($key, $ttl) { - if ($ttl === 0) { - // having infinite TTL can lead to leaked keys as the prefix changes with version upgrades - $ttl = self::DEFAULT_TTL; - } - $ttl = min($ttl, self::MAX_TTL); + $ttl = self::normalizeTtl((int)$ttl); $this->getCache()->expire($this->getPrefix() . $key, $ttl); } @@ -204,6 +205,7 @@ public function getTTL(string $key): int|false { #[\Override] public function compareSetTTL(string $key, mixed $value, int $ttl): bool { $value = self::encodeValue($value); + $ttl = self::normalizeTtl($ttl); return $this->evalLua('caSetTtl', [$key], [$value, $ttl]) > 0; } diff --git a/lib/public/IMemcacheTTL.php b/lib/public/IMemcacheTTL.php index 5f70bc3b8ebb2..737d618291462 100644 --- a/lib/public/IMemcacheTTL.php +++ b/lib/public/IMemcacheTTL.php @@ -9,34 +9,45 @@ namespace OCP; /** - * Interface for memcache backends that support setting ttl after the value is set + * Interface for memcache backends that can update and retrieve the TTL of an + * existing cache entry. * * @since 8.2.2 */ interface IMemcacheTTL extends IMemcache { /** - * Set the ttl for an existing value + * Update the TTL of an existing cache entry. * - * @param string $key - * @param int $ttl time to live in seconds + * Implementations may treat a non-positive TTL as immediate expiry. + * + * @param string $key Cache key + * @param int $ttl TTL in seconds. 0 uses the backend default TTL, negative + * values expire the entry immediately. * @since 8.2.2 */ public function setTTL(string $key, int $ttl); /** - * Get the ttl for an existing value, in seconds till expiry + * Get the remaining TTL of an existing cache entry. * - * @return int|false - * @since 27 + * @param string $key Cache key + * @return int Remaining TTL in whole seconds, or false if the key has no + * positive remaining TTL or does not exist + * @since 27.0.0 */ public function getTTL(string $key): int|false; + /** - * Set the ttl for an existing value if the value matches + * Atomically update the TTL if the existing value matches. + * + * Implementations may treat a non-positive TTL as immediate expiry. * - * @param string $key - * @param mixed $value - * @param int $ttl time to live in seconds - * @since 27 + * @param string $key Cache key + * @param mixed $value Expected current value + * @param int $ttl TTL in seconds. uses the backend default TTL, negative + * values expire the entry immediately. + * @return bool True if the value matched and the TTL was updated, false otherwise + * @since 27.0.0 */ - public function compareSetTTL(string $key, $value, int $ttl): bool; + public function compareSetTTL(string $key, mixed $value, int $ttl): bool; } diff --git a/tests/lib/Memcache/RedisTest.php b/tests/lib/Memcache/RedisTest.php index d410dac6686ae..f355af1ad95c0 100644 --- a/tests/lib/Memcache/RedisTest.php +++ b/tests/lib/Memcache/RedisTest.php @@ -65,23 +65,63 @@ protected function setUp(): void { $this->instance = new Redis($this->getUniqueID()); } + private function assertTtlInRange(int $expected, int|false $actual): void { + $this->assertNotFalse($actual); + // allow for 1s of inaccuracy due to time moving forward + $this->assertGreaterThanOrEqual($expected - 1, $actual); + $this->assertLessThanOrEqual($expected, $actual); + } + public function testScriptHashes(): void { foreach (Redis::LUA_SCRIPTS as $script) { $this->assertEquals(sha1($script[0]), $script[1]); } } - public function testCasTtlNotChanged(): void { + public function testCompareSetTtlUpdatesTtlWhenValueMatches(): void { $this->instance->set('foo', 'bar', 50); + $this->assertTrue($this->instance->compareSetTTL('foo', 'bar', 100)); - // allow for 1s of inaccuracy due to time moving forward - $this->assertLessThan(1, 100 - $this->instance->getTTL('foo')); + + $ttl = $this->instance->getTTL('foo'); + $this->assertTtlInRange(100, $ttl); } - public function testCasTtlChanged(): void { + public function testCompareSetTtlDoesNotUpdateTtlWhenValueDiffers(): void { $this->instance->set('foo', 'bar1', 50); + $this->assertFalse($this->instance->compareSetTTL('foo', 'bar', 100)); - // allow for 1s of inaccuracy due to time moving forward - $this->assertLessThan(1, 50 - $this->instance->getTTL('foo')); + + $ttl = $this->instance->getTTL('foo'); + $this->assertTtlInRange(50, $ttl); + } + + public function testCompareSetTtlZeroUsesDefaultTtl(): void { + $this->instance->set('foo', 'bar', 50); + + $this->assertTrue($this->instance->compareSetTTL('foo', 'bar', 0)); + + $ttl = $this->instance->getTTL('foo'); + $this->assertTtlInRange(Redis::DEFAULT_TTL, $ttl); + } + + public function testCompareSetTtlIsClampedToMaxTtl(): void { + $expectedMaxTtl = 30 * 24 * 60 * 60; + + $this->instance->set('foo', 'bar', 50); + + $this->assertTrue($this->instance->compareSetTTL('foo', 'bar', $expectedMaxTtl + 1000)); + + $ttl = $this->instance->getTTL('foo'); + $this->assertTtlInRange($expectedMaxTtl, $ttl); + } + + public function testCompareSetTtlNegativeExpiresKeyImmediately(): void { + $this->instance->set('foo', 'bar', 50); + + $this->assertTrue($this->instance->compareSetTTL('foo', 'bar', -1)); + $this->assertFalse($this->instance->hasKey('foo')); + $this->assertNull($this->instance->get('foo')); + $this->assertFalse($this->instance->getTTL('foo')); } }