From d3fb375c66aa591c982756ac5325475a9608f8ad Mon Sep 17 00:00:00 2001 From: pshafer-als <76011594+pshafer-als@users.noreply.github.com> Date: Sat, 4 Mar 2023 17:26:07 -0800 Subject: [PATCH] Make BaseCache an ABC (#683) --- CHANGES.rst | 2 +- aiocache/backends/memcached.py | 8 ++--- aiocache/backends/memory.py | 6 ++-- aiocache/backends/redis.py | 6 ++-- aiocache/base.py | 17 ++++++++-- tests/ut/conftest.py | 13 +++---- tests/ut/test_base.py | 24 +++++-------- tests/ut/test_decorators.py | 7 ++-- tests/utils.py | 62 +++++++++++++++++++++++++++++++++- 9 files changed, 104 insertions(+), 41 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index c9cb9579..31ba5b7d 100644 --- a/CHANGES.rst +++ b/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). diff --git a/aiocache/backends/memcached.py b/aiocache/backends/memcached.py index 45e1971a..06b33b42 100644 --- a/aiocache/backends/memcached.py +++ b/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) diff --git a/aiocache/backends/memory.py b/aiocache/backends/memory.py index 43534094..e3662707 100644 --- a/aiocache/backends/memory.py +++ b/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) diff --git a/aiocache/backends/redis.py b/aiocache/backends/redis.py index 08b07558..d30202ff 100644 --- a/aiocache/backends/redis.py +++ b/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) diff --git a/aiocache/base.py b/aiocache/base.py index 57e11e00..22ac58b6 100644 --- a/aiocache/base.py +++ b/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() diff --git a/tests/ut/conftest.py b/tests/ut/conftest.py index 385294c9..0e240097 100644 --- a/tests/ut/conftest.py +++ b/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 diff --git a/tests/ut/test_base.py b/tests/ut/test_base.py index 3449dabf..cfda509e 100644 --- a/tests/ut/test_base.py +++ b/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) diff --git a/tests/ut/test_decorators.py b/tests/ut/test_decorators.py index 4b0ebdab..cfa81e1b 100644 --- a/tests/ut/test_decorators.py +++ b/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] diff --git a/tests/utils.py b/tests/utils.py index a6f622fe..12194f83 100644 --- a/tests/utils.py +++ b/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)