Make BaseCache an ABC (aio-libs#683)

CHANGES.rst

@@ -15,7 +15,7 @@ There are a number of backwards-incompatible changes. These points should help w
 * The ``key_builder`` parameter for caches now expects a callback which accepts 2 strings and returns a string in all cache implementations, making the builders simpler and interchangeable.
 * The ``key`` parameter has been removed from the ``cached`` decorator. The behaviour can be easily reimplemented with ``key_builder=lambda *a, **kw: "foo"``
 * When using the ``key_builder`` parameter in ``@multicached``, the function will now return the original, unmodified keys, only using the transformed keys in the cache (this has always been the documented behaviour, but not the implemented behaviour).
-* ``BaseSerializer`` is now an ``ABC``, so cannot be instantiated directly.
+* ``BaseCache`` and ``BaseSerializer`` are now ``ABC``s, so cannot be instantiated directly.
 * If subclassing ``BaseCache`` to implement a custom backend:
 
   * The cache key type used by the backend must now be specified when inheriting (e.g. ``BaseCache[str]`` typically).

aiocache/backends/memcached.py

@@ -119,6 +119,10 @@ async def _redlock_release(self, key, _):
     async def _close(self, *args, _conn=None, **kwargs):
         await self.client.close()
 
+    def build_key(self, key: str, namespace: Optional[str] = None) -> bytes:
+        ns_key = self._str_build_key(key, namespace).replace(" ", "_")
+        return str.encode(ns_key)
+
 
 class MemcachedCache(MemcachedBackend):
     """
@@ -148,9 +152,5 @@ def __init__(self, serializer=None, **kwargs):
     def parse_uri_path(cls, path):
         return {}
 
-    def build_key(self, key: str, namespace: Optional[str] = None) -> bytes:
-        ns_key = self._str_build_key(key, namespace).replace(" ", "_")
-        return str.encode(ns_key)
-
     def __repr__(self):  # pragma: no cover
         return "MemcachedCache ({}:{})".format(self.endpoint, self.port)

aiocache/backends/memory.py

@@ -107,6 +107,9 @@ def __delete(self, key):
 
         return 0
 
+    def build_key(self, key: str, namespace: Optional[str] = None) -> str:
+        return self._str_build_key(key, namespace)
+
 
 class SimpleMemoryCache(SimpleMemoryBackend):
     """
@@ -132,6 +135,3 @@ def __init__(self, serializer=None, **kwargs):
     @classmethod
     def parse_uri_path(cls, path):
         return {}
-
-    def build_key(self, key: str, namespace: Optional[str] = None) -> str:
-        return self._str_build_key(key, namespace)

aiocache/backends/redis.py

@@ -178,6 +178,9 @@ async def _redlock_release(self, key, value):
     async def _close(self, *args, _conn=None, **kwargs):
         await self.client.close()
 
+    def build_key(self, key: str, namespace: Optional[str] = None) -> str:
+        return self._str_build_key(key, namespace)
+
 
 class RedisCache(RedisBackend):
     """
@@ -235,6 +238,3 @@ def parse_uri_path(cls, path):
 
     def __repr__(self):  # pragma: no cover
         return "RedisCache ({}:{})".format(self.endpoint, self.port)
-
-    def build_key(self, key: str, namespace: Optional[str] = None) -> str:
-        return self._str_build_key(key, namespace)

aiocache/base.py

@@ -3,7 +3,7 @@
 import logging
 import os
 import time
-from abc import abstractmethod
+from abc import ABC, abstractmethod
 from enum import Enum
 from types import TracebackType
 from typing import Callable, Generic, List, Optional, Set, TYPE_CHECKING, Type, TypeVar
@@ -93,7 +93,7 @@ async def _plugins(self, *args, **kwargs):
         return _plugins
 
 
-class BaseCache(Generic[CacheKeyType]):
+class BaseCache(Generic[CacheKeyType], ABC):
     """
     Base class that agregates the common logic for the different caches that may exist. Cache
     related available options are:
@@ -180,6 +180,7 @@ async def add(self, key, value, ttl=SENTINEL, dumps_fn=None, namespace=None, _co
         logger.debug("ADD %s %s (%.4f)s", ns_key, True, time.monotonic() - start)
         return True
 
+    @abstractmethod
     async def _add(self, key, value, ttl, _conn=None):
         raise NotImplementedError()
 
@@ -209,9 +210,11 @@ async def get(self, key, default=None, loads_fn=None, namespace=None, _conn=None
         logger.debug("GET %s %s (%.4f)s", ns_key, value is not None, time.monotonic() - start)
         return value if value is not None else default
 
+    @abstractmethod
     async def _get(self, key, encoding, _conn=None):
         raise NotImplementedError()
 
+    @abstractmethod
     async def _gets(self, key, encoding="utf-8", _conn=None):
         raise NotImplementedError()
 
@@ -250,6 +253,7 @@ async def multi_get(self, keys, loads_fn=None, namespace=None, _conn=None):
         )
         return values
 
+    @abstractmethod
     async def _multi_get(self, keys, encoding, _conn=None):
         raise NotImplementedError()
 
@@ -286,6 +290,7 @@ async def set(
         logger.debug("SET %s %d (%.4f)s", ns_key, True, time.monotonic() - start)
         return res
 
+    @abstractmethod
     async def _set(self, key, value, ttl, _cas_token=None, _conn=None):
         raise NotImplementedError()
 
@@ -325,6 +330,7 @@ async def multi_set(self, pairs, ttl=SENTINEL, dumps_fn=None, namespace=None, _c
         )
         return True
 
+    @abstractmethod
     async def _multi_set(self, pairs, ttl, _conn=None):
         raise NotImplementedError()
 
@@ -349,6 +355,7 @@ async def delete(self, key, namespace=None, _conn=None):
         logger.debug("DELETE %s %d (%.4f)s", ns_key, ret, time.monotonic() - start)
         return ret
 
+    @abstractmethod
     async def _delete(self, key, _conn=None):
         raise NotImplementedError()
 
@@ -373,6 +380,7 @@ async def exists(self, key, namespace=None, _conn=None):
         logger.debug("EXISTS %s %d (%.4f)s", ns_key, ret, time.monotonic() - start)
         return ret
 
+    @abstractmethod
     async def _exists(self, key, _conn=None):
         raise NotImplementedError()
 
@@ -400,6 +408,7 @@ async def increment(self, key, delta=1, namespace=None, _conn=None):
         logger.debug("INCREMENT %s %d (%.4f)s", ns_key, ret, time.monotonic() - start)
         return ret
 
+    @abstractmethod
     async def _increment(self, key, delta, _conn=None):
         raise NotImplementedError()
 
@@ -425,6 +434,7 @@ async def expire(self, key, ttl, namespace=None, _conn=None):
         logger.debug("EXPIRE %s %d (%.4f)s", ns_key, ret, time.monotonic() - start)
         return ret
 
+    @abstractmethod
     async def _expire(self, key, ttl, _conn=None):
         raise NotImplementedError()
 
@@ -448,6 +458,7 @@ async def clear(self, namespace=None, _conn=None):
         logger.debug("CLEAR %s %d (%.4f)s", namespace, ret, time.monotonic() - start)
         return ret
 
+    @abstractmethod
     async def _clear(self, namespace, _conn=None):
         raise NotImplementedError()
 
@@ -476,9 +487,11 @@ async def raw(self, command, *args, _conn=None, **kwargs):
         logger.debug("%s (%.4f)s", command, time.monotonic() - start)
         return ret
 
+    @abstractmethod
     async def _raw(self, command, *args, **kwargs):
         raise NotImplementedError()
 
+    @abstractmethod
     async def _redlock_release(self, key, value):
         raise NotImplementedError()
 

tests/ut/conftest.py

@@ -7,8 +7,8 @@
 from aiocache import caches
 from aiocache.backends.memcached import MemcachedCache
 from aiocache.backends.redis import RedisCache
-from aiocache.base import BaseCache
 from aiocache.plugins import BasePlugin
+from ..utils import AbstractBaseCache, ConcreteBaseCache
 
 if sys.version_info < (3, 8):
     # Missing AsyncMock on 3.7
@@ -29,14 +29,14 @@ def reset_caches():
 
 @pytest.fixture
 def mock_cache(mocker):
-    return create_autospec(BaseCache[str]())
+    return create_autospec(ConcreteBaseCache())
 
 
 @pytest.fixture
 def mock_base_cache():
     """Return BaseCache instance with unimplemented methods mocked out."""
     plugin = create_autospec(BasePlugin, instance=True)
-    cache = BaseCache[str](timeout=0.002, plugins=(plugin,))
+    cache = ConcreteBaseCache(timeout=0.002, plugins=(plugin,))
     methods = ("_add", "_get", "_gets", "_set", "_multi_get", "_multi_set", "_delete",
                "_exists", "_increment", "_expire", "_clear", "_raw", "_close",
                "_redlock_release", "acquire_conn", "release_conn")
@@ -50,15 +50,12 @@ def mock_base_cache():
 
 @pytest.fixture
 def abstract_base_cache():
-    # TODO: Is there need for a separate BaseCache[bytes] fixture?
-    return BaseCache[str]()
+    return AbstractBaseCache()
 
 
 @pytest.fixture
 def base_cache():
-    # TODO: Is there need for a separate BaseCache[bytes] fixture?
-    cache = BaseCache[str]()
-    cache.build_key = cache._str_build_key
+    cache = ConcreteBaseCache()
     return cache
 
 

tests/ut/test_base.py

@@ -4,8 +4,8 @@
 
 import pytest
 
-from aiocache.base import API, BaseCache, _Conn
-from ..utils import Keys, ensure_key
+from aiocache.base import API, _Conn
+from ..utils import AbstractBaseCache, ConcreteBaseCache, Keys, ensure_key
 
 
 class TestAPI:
@@ -137,11 +137,11 @@ async def dummy(self, *args, **kwargs):
 
 class TestBaseCache:
     def test_str_ttl(self):
-        cache = BaseCache[str](ttl="1.5")
+        cache = AbstractBaseCache(ttl="1.5")
         assert cache.ttl == 1.5
 
     def test_str_timeout(self):
-        cache = BaseCache[str](timeout="1.5")
+        cache = AbstractBaseCache(timeout="1.5")
         assert cache.timeout == 1.5
 
     async def test_add(self, base_cache):
@@ -213,7 +213,7 @@ def set_test_namespace(self, base_cache):
     )
     def test_str_build_key(self, set_test_namespace, namespace, expected):
         # TODO: Runtime check for namespace=None: Raise ValueError or replace with ""?
-        cache = BaseCache[str](namespace=namespace)
+        cache = AbstractBaseCache(namespace=namespace)
         assert cache._str_build_key(Keys.KEY) == expected
 
     @pytest.mark.parametrize(
@@ -223,14 +223,8 @@ def test_str_build_key(self, set_test_namespace, namespace, expected):
     def test_build_key(self, set_test_namespace, base_cache, namespace, expected):
         assert base_cache.build_key(Keys.KEY, namespace) == expected
 
-    def patch_str_build_key(self, cache: BaseCache[str]) -> None:
-        """Implement build_key() on BaseCache[str] as if it were subclassed"""
-        cache.build_key = cache._str_build_key  # type: ignore[assignment]
-        return
-
     def test_alt_build_key(self):
-        cache = BaseCache[str](key_builder=lambda key, namespace: "x")
-        self.patch_str_build_key(cache)
+        cache = ConcreteBaseCache(key_builder=lambda key, namespace: "x")
         assert cache.build_key(Keys.KEY, "namespace") == "x"
 
     def alt_build_key(self, key, namespace):
@@ -244,8 +238,7 @@ def alt_build_key(self, key, namespace):
     )
     def test_alt_build_key_override_namespace(self, namespace, expected):
         """Custom key_builder overrides namespace of cache"""
-        cache = BaseCache[str](key_builder=self.alt_build_key, namespace="test")
-        self.patch_str_build_key(cache)
+        cache = ConcreteBaseCache(key_builder=self.alt_build_key, namespace="test")
         assert cache.build_key(Keys.KEY, namespace) == expected
 
     @pytest.mark.parametrize(
@@ -264,8 +257,7 @@ async def test_alt_build_key_default_namespace(self, namespace, expected):
         even when that cache is supplied to a lock or to a decorator
         using the ``alias`` argument.
         """
-        cache = BaseCache[str](key_builder=self.alt_build_key, namespace=namespace)
-        self.patch_str_build_key(cache)
+        cache = ConcreteBaseCache(key_builder=self.alt_build_key, namespace=namespace)
 
         # Verify that private members are called with the correct ns_key
         await self._assert_add__alt_build_key_default_namespace(cache, expected)

tests/ut/test_decorators.py

@@ -8,9 +8,10 @@
 
 from aiocache import cached, cached_stampede, multi_cached
 from aiocache.backends.memory import SimpleMemoryCache
-from aiocache.base import BaseCache, SENTINEL
+from aiocache.base import SENTINEL
 from aiocache.decorators import _get_args_dict
 from aiocache.lock import RedLock
+from ..utils import AbstractBaseCache
 
 
 async def stub(*args, value=None, seconds=0, **kwargs):
@@ -208,7 +209,7 @@ async def what(self, a, b):
 
     async def test_reuses_cache_instance(self):
         with patch("aiocache.decorators._get_cache", autospec=True) as get_c:
-            cache = create_autospec(BaseCache, instance=True)
+            cache = create_autospec(AbstractBaseCache, instance=True)
             get_c.side_effect = [cache, None]
 
             @cached()
@@ -561,7 +562,7 @@ async def what(self, keys=None, what=1):
 
     async def test_reuses_cache_instance(self):
         with patch("aiocache.decorators._get_cache", autospec=True) as get_c:
-            cache = create_autospec(BaseCache, instance=True)
+            cache = create_autospec(AbstractBaseCache, instance=True)
             cache.multi_get.return_value = [None]
             get_c.side_effect = [cache, None]
 

tests/utils.py

@@ -1,4 +1,7 @@
 from enum import Enum
+from typing import Optional, Union
+
+from aiocache.base import BaseCache
 
 
 class Keys(str, Enum):
@@ -9,8 +12,65 @@ class Keys(str, Enum):
 KEY_LOCK = Keys.KEY + "-lock"
 
 
-def ensure_key(key):
+def ensure_key(key: Union[str, Enum]) -> str:
     if isinstance(key, Enum):
         return key.value
     else:
         return key
+
+
+class AbstractBaseCache(BaseCache[str]):
+    """BaseCache that can be mocked for NotImplementedError tests"""
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+    def build_key(self, key: str, namespace: Optional[str] = None) -> str:
+        return super().build_key(key, namespace)
+
+    async def _add(self, key, value, ttl, _conn=None):
+        return await super()._add(key, value, ttl, _conn)
+
+    async def _get(self, key, encoding, _conn=None):
+        return await super()._get(key, encoding, _conn)
+
+    async def _gets(self, key, encoding="utf-8", _conn=None):
+        return await super()._gets(key, encoding, _conn)
+
+    async def _multi_get(self, keys, encoding, _conn=None):
+        return await super()._multi_get(keys, encoding, _conn)
+
+    async def _set(self, key, value, ttl, _cas_token=None, _conn=None):
+        return await super()._set(key, value, ttl, _cas_token, _conn)
+
+    async def _multi_set(self, pairs, ttl, _conn=None):
+        return await super()._multi_set(pairs, ttl, _conn)
+
+    async def _delete(self, key, _conn=None):
+        return await super()._delete(key, _conn)
+
+    async def _exists(self, key, _conn=None):
+        return await super()._exists(key, _conn)
+
+    async def _increment(self, key, delta, _conn=None):
+        return await super()._increment(key, delta, _conn)
+
+    async def _expire(self, key, ttl, _conn=None):
+        return await super()._expire(key, ttl, _conn)
+
+    async def _clear(self, namespace, _conn=None):
+        return await super()._clear(namespace, _conn)
+
+    async def _raw(self, command, *args, **kwargs):
+        return await super()._raw(command, *args, **kwargs)
+
+    async def _redlock_release(self, key, value):
+        return await super()._redlock_release(key, value)
+
+
+class ConcreteBaseCache(AbstractBaseCache):
+    """BaseCache that can be mocked for tests"""
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+    def build_key(self, key: str, namespace: Optional[str] = None) -> str:
+        return self._str_build_key(key, namespace)