From d4c908c19426207357382f0fba85e821d018876f Mon Sep 17 00:00:00 2001 From: Diogo Peralta Cordeiro Date: Sun, 27 Feb 2022 00:29:26 +0000 Subject: [PATCH] [CORE][Cache] Implement listPushRight --- components/Tag/Tag.php | 2 +- plugins/ActivityPub/Util/Model/Note.php | 2 +- src/Core/Cache.php | 33 +++++++++++++++++++++++-- tests/Core/CacheTest.php | 16 +++++++++--- 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/components/Tag/Tag.php b/components/Tag/Tag.php index 2e44221a65..68152fd084 100644 --- a/components/Tag/Tag.php +++ b/components/Tag/Tag.php @@ -93,7 +93,7 @@ class Tag extends Component 'use_canonical' => $extra_args['tag_use_canonical'] ?? false, 'language_id' => $lang_id, ])); - Cache::pushList("tag-{$canonical_tag}", $note); + Cache::listPushLeft("tag-{$canonical_tag}", $note); foreach (self::cacheKeys($canonical_tag) as $key) { Cache::delete($key); } diff --git a/plugins/ActivityPub/Util/Model/Note.php b/plugins/ActivityPub/Util/Model/Note.php index 8867ac383b..a5b4d6730d 100644 --- a/plugins/ActivityPub/Util/Model/Note.php +++ b/plugins/ActivityPub/Util/Model/Note.php @@ -290,7 +290,7 @@ class Note extends Model 'use_canonical' => $ap_tag->get('canonical') ?? false, 'language_id' => $lang_id ?? null, ])); - Cache::pushList("tag-{$canonical_tag}", $obj); + Cache::listPushLeft("tag-{$canonical_tag}", $obj); foreach (Tag::cacheKeys($canonical_tag) as $key) { Cache::delete($key); } diff --git a/src/Core/Cache.php b/src/Core/Cache.php index 2e5a47fbdf..c17cb92907 100644 --- a/src/Core/Cache.php +++ b/src/Core/Cache.php @@ -225,7 +225,7 @@ abstract class Cache public static function exists(string $key, string $pool = 'default'): bool { if (isset(self::$redis[$pool])) { - return self::$redis[$pool]->exists($key); + return self::$redis[$pool]->exists($key) === 1; } else { // there's no set method, must be done this way return self::$pools[$pool]->hasItem($key); @@ -237,7 +237,13 @@ abstract class Cache * for redis and others, trimming to $max_count if given * TODO(hugo): $calculate = [] is the same as false miss * + * @param string $key Cache key * @param callable(?CacheItem $item, bool &$save): (string|object|array) $calculate + * @param string $pool Cache pool being used (between different cache managers (of the same or different systems)) + * @param null|int $max_count When setting cache, it trims to max count + * @param null|int $left Offset on Get + * @param null|int $right Limit on Get + * @param float $beta Likelihood of recomputing value (1 to infinity) */ public static function getList(string $key, callable $calculate, string $pool = 'default', ?int $max_count = null, ?int $left = null, ?int $right = null, float $beta = 1.0): array { @@ -314,7 +320,7 @@ abstract class Cache /** * Push a value to the list */ - public static function pushList(string $key, mixed $value, string $pool = 'default', ?int $max_count = null, float $beta = 1.0): void + public static function listPushLeft(string $key, mixed $value, string $pool = 'default', ?int $max_count = null, float $beta = 1.0): void { if (isset(self::$redis[$pool])) { self::$redis[$pool] @@ -334,6 +340,29 @@ abstract class Cache } } + /** + * Push a value to the list + */ + public static function listPushRight(string $key, mixed $value, string $pool = 'default', ?int $max_count = null, float $beta = 1.0): void + { + if (isset(self::$redis[$pool])) { + self::$redis[$pool] + // doesn't need to be atomic, adding at one end, deleting at the other + ->multi(Redis::PIPELINE) + ->rPush($key, $value) + // trim to $max_count, if given + ->lTrim($key, -($max_count ?? 0), -1) + ->exec(); + } else { + $res = self::get($key, fn () => [], $pool, $beta); + $res[] = $value; + if (!\is_null($max_count)) { + $res = \array_slice($res, -$max_count, null); // Trim away the older values + } + self::set($key, $res, $pool); + } + } + /** * Delete a whole list at $key */ diff --git a/tests/Core/CacheTest.php b/tests/Core/CacheTest.php index 5345aeb89d..b51bce4e68 100644 --- a/tests/Core/CacheTest.php +++ b/tests/Core/CacheTest.php @@ -106,10 +106,14 @@ class CacheTest extends KernelTestCase static::assertSame(['foo'], Cache::getList($key . '1', fn ($i) => ['foo'])); static::assertSame(['foo', 'bar'], Cache::getList($key, fn ($i) => ['foo', 'bar'])); static::assertSame(['foo', 'bar'], Cache::getList($key, function () { $this->assertFalse('should not be called'); })); // Repeat to test no recompute lrange - Cache::pushList($key, 'quux'); + Cache::listPushLeft($key, 'quux'); static::assertSame(['quux', 'foo', 'bar'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); })); - Cache::pushList($key, 'foobar', max_count: 2); + Cache::listPushLeft($key, 'foobar', max_count: 2); static::assertSame(['foobar', 'quux'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); })); + Cache::listPushRight($key, 'foo'); + static::assertSame(['foobar', 'quux', 'foo'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); })); + Cache::listPushRight($key, 'bar', max_count: 2); + static::assertSame(['foo', 'bar'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); })); static::assertTrue(Cache::deleteList($key)); } @@ -132,10 +136,14 @@ class CacheTest extends KernelTestCase static::assertSame(['foo'], Cache::getList($key . '1', fn ($i) => ['foo'], pool: 'file')); static::assertSame(['foo', 'bar'], Cache::getList($key, fn ($i) => ['foo', 'bar'], pool: 'file')); static::assertSame(['foo', 'bar'], Cache::getList($key, function () { $this->assertFalse('should not be called'); }, pool: 'file')); // Repeat to test no recompute lrange - Cache::pushList($key, 'quux', pool: 'file'); + Cache::listPushLeft($key, 'quux', pool: 'file'); static::assertSame(['quux', 'foo', 'bar'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); }, pool: 'file')); - Cache::pushList($key, 'foobar', max_count: 2, pool: 'file'); + Cache::listPushLeft($key, 'foobar', max_count: 2, pool: 'file'); static::assertSame(['foobar', 'quux'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); }, pool: 'file')); + Cache::listPushRight($key, 'foo'); + static::assertSame(['foobar', 'quux', 'foo'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); })); + Cache::listPushRight($key, 'bar', max_count: 2); + static::assertSame(['foo', 'bar'], Cache::getList($key, function ($i) { $this->assertFalse('should not be called'); })); static::assertTrue(Cache::deleteList($key, pool: 'file')); } }