adjust redis compareSetTTL to use a lua script

Signed-off-by: Robin Appelman <robin@icewind.nl>
nextcloud · Dec 5, 2023 · bba6fd7 · bba6fd7
1 parent 07282d1
commit bba6fd7
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 9 deletions.
diff --git a/lib/private/Memcache/Redis.php b/lib/private/Memcache/Redis.php
@@ -46,6 +46,10 @@ class Redis extends Cache implements IMemcacheTTL {
 			'if redis.call("get", KEYS[1]) == ARGV[1] then return redis.call("del", KEYS[1]) else return 0 end',
 			'cf0e94b2e9ffc7e04395cf88f7583fc309985910',
 		],
+		'caSetTtl' => [
+			'if redis.call("get", KEYS[1]) == ARGV[1] then redis.call("expire", KEYS[1], ARGV[2]) return 1 else return 0 end',
+			'fa4acbc946d23ef41d7d3910880b60e6e4972d72',
+		],
 	];
 
 	/**
@@ -187,15 +191,9 @@ public function getTTL(string $key): int|false {
 	}
 
 	public function compareSetTTL(string $key, mixed $value, int $ttl): bool {
-		$fullKey = $this->getPrefix() . $key;
-		$redis = $this->getCache();
-		if ($this->get($key) === $value) {
-			$result = $redis->multi()
-				->expire($fullKey, $ttl)
-				->exec();
-			return $result !== false;
-		}
-		return false;
+		$value = self::encodeValue($value);
+
+		return $this->evalLua('caSetTtl', [$key], [$value, $ttl]) > 0;
 	}
 
 	public static function isAvailable(): bool {

diff --git a/tests/lib/Memcache/RedisTest.php b/tests/lib/Memcache/RedisTest.php
@@ -9,11 +9,18 @@
 
 namespace Test\Memcache;
 
+use OC\Memcache\Redis;
+
 /**
  * @group Memcache
  * @group Redis
  */
 class RedisTest extends Cache {
+	/**
+	 * @var Redis cache;
+	 */
+	protected $instance;
+
 	public static function setUpBeforeClass(): void {
 		parent::setUpBeforeClass();
 
@@ -62,4 +69,18 @@ public function testScriptHashes() {
 			$this->assertEquals(sha1($script[0]), $script[1]);
 		}
 	}
+
+	public function testCasTtlNotChanged() {
+		$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'));
+	}
+
+	public function testCasTtlChanged() {
+		$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'));
+	}
 }