Don't rely on HDEL ignoring keys for .pop

Previously we would try to .delete keys that did not exist if a default
was provided when calling .pop. This is okay to do (because HDEL will
just ignore any attempts to delete non-existing calls), but it does add
an additional pointless API call to Redis, so I've added some validation
as a small optimization.

This also adds a few additional lines of documentation as requested by
@SebastiaanZ in their review.

1 files changed, 21 insertions, 2 deletions

diff --git a/bot/utils/redis_cache.py b/bot/utils/redis_cache.py
index f9d9e571f..6831be157 100644
--- a/bot/utils/redis_cache.py
+++ b/bot/utils/redis_cache.py
@@ -35,6 +35,14 @@ class RedisCache:
         cache = RedisCache()
 
         async def my_method(self):
+
+            # Now we're ready to use the RedisCache.
+            # One thing to note here is that this will not work unless
+            # we access self.cache through an _instance_ of this class.
+            #
+            # For example, attempting to use SomeCog.cache will _not_ work,
+            # you _must_ instantiate the class first and use that instance.
+            #
             # Now we can store some stuff in the cache just by doing this.
             # This data will persist through restarts!
             await self.cache.set("key", "value")
@@ -167,7 +175,13 @@ class RedisCache:
             return value
 
     async def delete(self, key: ValidRedisType) -> None:
-        """Delete an item from the Redis cache."""
+        """
+        Delete an item from the Redis cache.
+
+        If we try to delete a key that does not exist, it will simply be ignored.
+
+        See https://redis.io/commands/hdel for more info on how this works.
+        """
         await self._validate_cache()
         key = self._to_typestring(key)
         return await self._redis.hdel(self._namespace, key)
@@ -206,7 +220,12 @@ class RedisCache:
     async def pop(self, key: ValidRedisType, default: Optional[ValidRedisType] = None) -> ValidRedisType:
         """Get the item, remove it from the cache, and provide a default if not found."""
         value = await self.get(key, default)
-        await self.delete(key)
+
+        # No need to try to delete something that doesn't exist,
+        # that's just a superfluous API call.
+        if value != default:
+            await self.delete(key)
+
         return value
 
     async def update(self, items: Dict) -> None: