From 4cc4d06b11bcda7b592dc09457a9bceba4116fc0 Mon Sep 17 00:00:00 2001
From: Hugo Sales <hugo@hsal.es>
Date: Tue, 1 Mar 2022 11:07:21 +0000
Subject: [PATCH] [CORE][Cache] Fix bug where empty lists must be stored as a
 string in Redis (not supported natively), so we can't directly push to it,
 but the key still exists

---
 src/Core/Cache.php | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/Core/Cache.php b/src/Core/Cache.php
index d9d2be7cb7..63a19e7ac9 100644
--- a/src/Core/Cache.php
+++ b/src/Core/Cache.php
@@ -248,7 +248,7 @@ abstract class Cache
     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
     {
         if (isset(self::$redis[$pool])) {
-            $result = self::redisMaybeRecompute(
+            return self::redisMaybeRecompute(
                 $key,
                 recompute: /**
                  * Caculate and trim the list to the correct size
@@ -270,11 +270,19 @@ abstract class Cache
                 no_recompute: /**
                  * Fetch (a portion of) the list from the cache
                  */
-                fn () => self::$redis[$pool]->lRange($key, $left ?? 0, ($right ?? $max_count ?? 0) - 1),
+                function () use ($pool, $key, $left, $right, $max_count) {
+                    $res = self::$redis[$pool]->lRange($key, $left ?? 0, ($right ?? $max_count ?? 0) - 1);
+                    if ($res === false) {
+                        // Empty lists are not supported by redis, but MSGPACK stores them as a
+                        // string, so check if they exist
+                        return self::$redis[$pool]->get($key);
+                    } else {
+                        return $res;
+                    }
+                },
                 pool: $pool,
                 beta: $beta,
             );
-            return empty($result) ? [] : $result; // TODO may be wrong? not sure
         } else {
             return self::get(
                 $key,
@@ -359,13 +367,18 @@ abstract class Cache
     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]
+            $res = self::$redis[$pool]
                 // doesn't need to be atomic, adding at one end, deleting at the other
                 ->multi(Redis::PIPELINE)
                 ->lPush($key, $value)
                 // trim to $max_count, if given
                 ->lTrim($key, 0, ($max_count ?? 0) - 1)
                 ->exec();
+            // $res has a bool per operation in the pipeline. False means it failed
+            if (\in_array(false, $res)) {
+                // Empty lists are stored as strings, so you can't push to it. Delete it and then push
+                self::$redis[$pool]->multi(Redis::PIPELINE)->del($key)->lPush($key, $value)->exec();
+            }
         } else {
             $res = self::get($key, fn () => [], $pool, $beta);
             array_unshift($res, $value);
@@ -382,13 +395,18 @@ abstract class Cache
     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]
+            $res = 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();
+            // $res has a bool per operation in the pipeline. False means it failed
+            if (\in_array(false, $res)) {
+                // Empty lists are stored as strings, so you can't push to it. Delete it and then push
+                self::$redis[$pool]->multi(Redis::PIPELINE)->del($key)->rPush($key, $value)->exec();
+            }
         } else {
             $res   = self::get($key, fn () => [], $pool, $beta);
             $res[] = $value;