From aa9684174655d961866e369ad3de4231778999ea Mon Sep 17 00:00:00 2001 From: allen0099 Date: Sun, 27 Sep 2026 14:08:25 +0000 Subject: [PATCH] fix(cache-manager): match key_prefix literally in clear_pattern clear_pattern passed key_prefix + pattern to the backend as one glob, so glob characters in the manager's own prefix were live: cache[1]: missed its own keys and a?: also cleared ab:'s. A prefix free of glob metacharacters keeps the backend's native clear_pattern; any other prefix lists every key, matches the prefix literally and the remainder with fnmatchcase, and deletes via delete_many. The constructor warns about such a prefix. --- changelog.d/140.fixed.md | 10 ++ docs/APP_CACHE.md | 18 +- fastapi_cachex/manager.py | 64 ++++++- i18n/zh-TW/docs/APP_CACHE.md | 6 +- tests/test_cache_manager_clear_pattern.py | 204 ++++++++++++++++++++++ 5 files changed, 291 insertions(+), 11 deletions(-) create mode 100644 changelog.d/140.fixed.md create mode 100644 tests/test_cache_manager_clear_pattern.py diff --git a/changelog.d/140.fixed.md b/changelog.d/140.fixed.md new file mode 100644 index 0000000..2f23cc9 --- /dev/null +++ b/changelog.d/140.fixed.md @@ -0,0 +1,10 @@ +**`CacheManager.clear_pattern()` matches the manager's `key_prefix` literally.** +The prefix used to be passed to the backend as part of the glob, so glob +characters in it were live: `key_prefix="cache[1]:"` missed its own keys, and +`key_prefix="a?:"` also cleared the keys of a manager with prefix `ab:`. A +prefix without `*`, `?`, `[`, `]` or `\` still goes to the backend's native +`clear_pattern()` as before. A prefix with one makes `clear_pattern()` list +every key and match the rest of the key against `pattern` with +`fnmatch.fnmatchcase`, which is slower on Redis and uses fnmatch rather than +Redis glob syntax for `pattern`. Constructing a `CacheManager` with such a +prefix emits a `UserWarning`. diff --git a/docs/APP_CACHE.md b/docs/APP_CACHE.md index e756d94..e7004a7 100644 --- a/docs/APP_CACHE.md +++ b/docs/APP_CACHE.md @@ -32,8 +32,9 @@ profile = await manager.get_or_set("user:42", lambda: load_user(42), ttl=300) if await manager.add(f"webhook:{event_id}", True, ttl=86400): await deliver_webhook(event_id) -# Glob over this manager's namespace, using the backend's native pattern -# support (Redis SCAN) rather than enumerating every key. +# Glob over this manager's namespace. Only the pattern is a glob; the prefix +# is literal. With a prefix free of *?[]\ this uses the backend's native +# pattern support (Redis SCAN) rather than enumerating every key. await manager.clear_pattern("user:*") # matches "myapp:user:*" ``` @@ -59,6 +60,19 @@ Complete runnable example: [`examples/app_cache.py`](https://github.com/allen009 `key_prefix="cache:users:"`, and an empty `key_prefix` makes `clear()` remove everything in the backend, including HTTP responses, locks, OAuth states and sessions. Give each manager a prefix that does not start with another's. +- `clear_pattern(pattern)` treats only `pattern` as a glob; `key_prefix` is + always matched literally. With a prefix free of glob metacharacters + (`*`, `?`, `[`, `]`, `\`) it hands `key_prefix + pattern` to the backend's + `clear_pattern()` (Redis `SCAN MATCH`), and `pattern` uses the backend's + glob syntax. A prefix that contains one, such as `cache[1]:`, cannot be + passed on as a glob, so `clear_pattern()` lists every key with + `get_all_keys()`, keeps those that start with the prefix and whose remainder + matches `pattern` under `fnmatch.fnmatchcase`, and deletes them with + `delete_many()`. That is slower on Redis, and `pattern` is then fnmatch + syntax rather than Redis glob: case-sensitive, no backslash escapes, and + `[!a]` rather than `[^a]` for negation. Constructing a `CacheManager` with + such a prefix emits a `UserWarning`; pick a prefix without `*?[]\` to keep + the fast path. - The `AppCache` dependency creates and registers a default `CacheManager` the first time it is used; `CacheManagerProxy.set()` registers your own instead. diff --git a/fastapi_cachex/manager.py b/fastapi_cachex/manager.py index 3a86010..dc8fe08 100644 --- a/fastapi_cachex/manager.py +++ b/fastapi_cachex/manager.py @@ -1,9 +1,11 @@ """Generic application-level cache manager for FastAPI-CacheX.""" +import fnmatch import hashlib import inspect import json import logging +import warnings from collections.abc import Awaitable from collections.abc import Callable from typing import Any @@ -17,6 +19,11 @@ _DECODE_ERRORS = (AttributeError, UnicodeDecodeError, json.JSONDecodeError) +# Characters that are live in a glob pattern: the Redis set, mirrored from +# ``backends/redis.py`` (fnmatch treats the backslash literally, but a prefix +# holding one still cannot be passed through to Redis unescaped). +_GLOB_SPECIAL = frozenset("*?[]\\") + class CacheManager: """Provides convenient get/set/delete access to the configured cache backend. @@ -33,7 +40,7 @@ def __init__( key_prefix: str = "cache:", default_ttl: int | None = None, ) -> None: - """Initialize CacheManager. + r"""Initialize CacheManager. Args: backend: Cache backend instance. If None, uses BackendProxy.get(). @@ -45,10 +52,30 @@ def __init__( BackendNotFoundError: If ``backend`` is None and no backend has been set with ``BackendProxy.set()``. ValueError: If ``default_ttl`` is zero or negative. + + Warns: + UserWarning: If ``key_prefix`` contains a glob metacharacter + (``*``, ``?``, ``[``, ``]`` or ``\``). ``clear_pattern()`` then + lists every key and filters in Python instead of handing the + pattern to the backend. """ self.backend = backend if backend is not None else BackendProxy.get() self.key_prefix = key_prefix self.default_ttl = validate_ttl(default_ttl) + if self._prefix_has_glob: + warnings.warn( + f"CacheManager key_prefix {key_prefix!r} contains a glob " + "metacharacter, so clear_pattern() will list every key in the " + "backend and filter them in Python, which is slower on Redis " + "than a server-side SCAN MATCH. Use a prefix without any of " + "*?[]\\ to keep the fast path.", + UserWarning, + stacklevel=2, + ) + + @property + def _prefix_has_glob(self) -> bool: + return not _GLOB_SPECIAL.isdisjoint(self.key_prefix) def _cache_key(self, key: str) -> str: return f"{self.key_prefix}{key}" @@ -204,12 +231,25 @@ async def get_or_set( return value async def clear_pattern(self, pattern: str) -> int: - """Clear all keys under this manager's namespace matching a glob pattern. + r"""Clear all keys under this manager's namespace matching a glob pattern. + + Only ``pattern`` is a glob; ``self.key_prefix`` is always matched + literally. + + When ``key_prefix`` holds no glob metacharacter (``*?[]\``), this + delegates to the backend's native ``clear_pattern`` (e.g. Redis + ``SCAN MATCH``), and ``pattern`` uses the backend's glob syntax. + + Otherwise it cannot pass the prefix to the backend as a glob, so it + lists every key with ``get_all_keys()``, keeps those that start with + ``key_prefix`` and whose remainder matches ``pattern`` under + ``fnmatch.fnmatchcase``, and removes them with ``delete_many()``. That + is slower on Redis, and ``pattern`` is then fnmatch syntax rather than + Redis glob: no backslash escapes, and ``[!a]`` rather than ``[^a]`` + for negation. The constructor warns about such a prefix. - Delegates to the backend's native ``clear_pattern`` (e.g. Redis ``SCAN``), - which can be more efficient than ``clear_prefix``'s full key-space scan. - Note that backends without key-enumeration support (e.g. Memcached) - cannot honor this and will return 0 with a ``RuntimeWarning``. + Backends without key enumeration (e.g. Memcached) cannot honor either + path and return 0 with a ``RuntimeWarning``. Args: pattern: Glob pattern (relative to ``self.key_prefix``) to match @@ -219,7 +259,17 @@ async def clear_pattern(self, pattern: str) -> int: Number of cache entries cleared. """ match_pattern = self._cache_key(pattern) - cleared = await self.backend.clear_pattern(match_pattern) + if self._prefix_has_glob: + prefix = self.key_prefix + keys = await self.backend.get_all_keys() + cleared = await self.backend.delete_many( + key + for key in keys + if key.startswith(prefix) + and fnmatch.fnmatchcase(key.removeprefix(prefix), pattern) + ) + else: + cleared = await self.backend.clear_pattern(match_pattern) logger.debug( "Cache CLEAR_PATTERN; pattern=%s removed=%s", match_pattern, cleared ) diff --git a/i18n/zh-TW/docs/APP_CACHE.md b/i18n/zh-TW/docs/APP_CACHE.md index b3f6c23..1696ee5 100644 --- a/i18n/zh-TW/docs/APP_CACHE.md +++ b/i18n/zh-TW/docs/APP_CACHE.md @@ -30,8 +30,9 @@ profile = await manager.get_or_set("user:42", lambda: load_user(42), ttl=300) if await manager.add(f"webhook:{event_id}", True, ttl=86400): await deliver_webhook(event_id) -# 在此 manager 的命名空間內做萬用字元（glob）比對，使用後端原生的模式比對 -# 支援（Redis SCAN），而不是列舉所有鍵。 +# 在此 manager 的命名空間內做萬用字元（glob）比對。只有 pattern 是 glob， +# 前綴一律照字面比對。前綴不含 *?[]\ 時，會使用後端原生的模式比對支援 +# （Redis SCAN），而不是列舉所有鍵。 await manager.clear_pattern("user:*") # 比對 "myapp:user:*" ``` @@ -45,6 +46,7 @@ await manager.clear_pattern("user:*") # 比對 "myapp:user:*" - `add()` 只在鍵尚未被占用時寫入值，並回傳是否有寫入。檢查與寫入是同一個後端原子操作（`set_if_absent`），因此適合「每個鍵只做一次」的工作，例如 webhook 或電子郵件的去重。已過期的鍵視為未被占用；存放無法解碼之值的鍵則不算，即使 `get()` 會把它當成未命中。 - 鍵預設位於獨立、以 `cache:` 為前綴的命名空間，與 HTTP 路由快取及 OAuth state 分開，因此 `clear()`／`clear_prefix()` 絕不會動到無關的快取項目。 - 前綴是以單純的字串前綴比對。因此 `key_prefix="cache:"` 的 manager 也會清除 `key_prefix="cache:users:"` 的 manager 的項目；而空的 `key_prefix` 會讓 `clear()` 移除後端中的所有內容，包括 HTTP 回應、鎖、OAuth state 與 Session。請讓每個 manager 的前綴都不以另一個 manager 的前綴開頭。 +- `clear_pattern(pattern)` 只把 `pattern` 當成 glob；`key_prefix` 一律照字面比對。前綴不含 glob 特殊字元（`*`、`?`、`[`、`]`、`\`）時，會把 `key_prefix + pattern` 交給後端的 `clear_pattern()`（Redis `SCAN MATCH`），`pattern` 採用後端的 glob 語法。前綴含有這些字元時（例如 `cache[1]:`），無法把它當成 glob 傳給後端，因此 `clear_pattern()` 會以 `get_all_keys()` 列出所有鍵，保留以該前綴開頭、且其餘部分以 `fnmatch.fnmatchcase` 符合 `pattern` 的鍵，再以 `delete_many()` 刪除。這在 Redis 上較慢，而且此時 `pattern` 採用 fnmatch 語法而非 Redis glob：區分大小寫、不支援反斜線跳脫，否定用 `[!a]` 而非 `[^a]`。以這種前綴建立 `CacheManager` 時會發出 `UserWarning`；請改用不含 `*?[]\` 的前綴以維持快速路徑。 - `AppCache` 依賴項在第一次使用時會建立並註冊一個預設的 `CacheManager`；`CacheManagerProxy.set()` 則可改為註冊你自己的實例。 > [!NOTE] diff --git a/tests/test_cache_manager_clear_pattern.py b/tests/test_cache_manager_clear_pattern.py new file mode 100644 index 0000000..2436ead --- /dev/null +++ b/tests/test_cache_manager_clear_pattern.py @@ -0,0 +1,204 @@ +"""CacheManager.clear_pattern() with glob metacharacters in the key prefix.""" + +import warnings +from collections.abc import AsyncGenerator +from typing import TYPE_CHECKING +from typing import Any + +import pytest +import pytest_asyncio + +from fastapi_cachex.backends.memory import MemoryBackend +from fastapi_cachex.manager import CacheManager +from fastapi_cachex.types import CacheEntry +from tests.live_servers import MEMCACHED_SERVER +from tests.live_servers import REDIS_HOST +from tests.live_servers import REDIS_PORT +from tests.live_servers import requires_memcached +from tests.live_servers import requires_redis +from tests.live_servers import requires_redis_package + +if TYPE_CHECKING: + from fastapi_cachex.backends.base import BaseCacheBackend + +_SLOW_PATH_WARNING = r"clear_pattern\(\) will list every key" +_ENTRY = CacheEntry(fingerprint="x", content=b"1") + + +def _globby_manager(backend: "BaseCacheBackend", key_prefix: str) -> CacheManager: + """Build a manager whose prefix holds glob characters, expecting the warning.""" + with pytest.warns(UserWarning, match=_SLOW_PATH_WARNING): + return CacheManager(backend=backend, key_prefix=key_prefix) + + +@pytest_asyncio.fixture( + params=[ + pytest.param("memory", id="MemoryBackend"), + pytest.param( + "redis", + id="RedisBackend", + marks=[requires_redis, requires_redis_package], + ), + ] +) +async def backend(request: Any) -> AsyncGenerator["BaseCacheBackend", Any]: + """A memory backend, or a live Redis one when it is available.""" + if request.param == "memory": + mem_backend = MemoryBackend() + mem_backend.start_cleanup() + yield mem_backend + await mem_backend.clear() + mem_backend.stop_cleanup() + return + + from fastapi_cachex.backends import AsyncRedisCacheBackend + + redis_backend = AsyncRedisCacheBackend( + host=REDIS_HOST, + port=REDIS_PORT, + socket_timeout=1.0, + socket_connect_timeout=1.0, + key_prefix="test_cache_manager_clear_pattern:", + ) + await redis_backend.clear() + yield redis_backend + await redis_backend.clear() + + +# --- Constructor warning ------------------------------------------------------- + + +@pytest.mark.parametrize("key_prefix", ["cache[1]:", "a?:", "a*:", "a]:", "a\\:"]) +def test_glob_prefix_warns_at_the_callers_line( + memory_backend: MemoryBackend, key_prefix: str +) -> None: + """A prefix with a glob metacharacter warns, pointing at the caller.""" + with pytest.warns(UserWarning, match=_SLOW_PATH_WARNING) as record: + CacheManager(backend=memory_backend, key_prefix=key_prefix) + + assert len(record) == 1 + assert record[0].filename == __file__ + + +@pytest.mark.parametrize("key_prefix", ["cache:", "", "my-app.v2:"]) +def test_plain_prefix_does_not_warn( + memory_backend: MemoryBackend, key_prefix: str +) -> None: + """A prefix without glob metacharacters constructs silently.""" + with warnings.catch_warnings(): + warnings.simplefilter("error") + CacheManager(backend=memory_backend, key_prefix=key_prefix) + + +# --- Fast path ------------------------------------------------------------------- + + +async def test_plain_prefix_delegates_to_backend_clear_pattern( + memory_backend: MemoryBackend, monkeypatch: pytest.MonkeyPatch +) -> None: + """With a plain prefix, the backend's own clear_pattern gets prefix + pattern.""" + manager = CacheManager(backend=memory_backend, key_prefix="cache:") + calls: list[str] = [] + + async def fake_clear_pattern(pattern: str) -> int: + calls.append(pattern) + return 7 + + async def no_get_all_keys() -> list[str]: # pragma: no cover - must not run + pytest.fail("the fast path must not enumerate keys") + + monkeypatch.setattr(memory_backend, "clear_pattern", fake_clear_pattern) + monkeypatch.setattr(memory_backend, "get_all_keys", no_get_all_keys) + + assert await manager.clear_pattern("user:*") == 7 + assert calls == ["cache:user:*"] + + +# --- Slow path ------------------------------------------------------------------- + + +async def test_bracket_prefix_clears_its_own_keys( + backend: "BaseCacheBackend", +) -> None: + """key_prefix="cache[1]:" is literal, so clear_pattern("*") finds its keys.""" + manager = _globby_manager(backend, "cache[1]:") + await manager.set("a", 1) + await manager.set("b", 2) + await backend.set("cache1:a", _ENTRY) + + removed = await manager.clear_pattern("*") + + assert removed == 2 + assert not await manager.has("a") + assert not await manager.has("b") + assert await backend.get("cache1:a") is not None + + +async def test_question_mark_prefix_leaves_other_namespaces_alone( + backend: "BaseCacheBackend", +) -> None: + """key_prefix="a?:" does not clear the keys of a manager with prefix "ab:".""" + globby = _globby_manager(backend, "a?:") + other = CacheManager(backend=backend, key_prefix="ab:") + await globby.set("x", 1) + await other.set("x", 2) + + removed = await globby.clear_pattern("*") + + assert removed == 1 + assert not await globby.has("x") + assert await other.get("x") == 2 + + +async def test_slow_path_pattern_is_still_a_glob( + backend: "BaseCacheBackend", +) -> None: + """Metacharacters in the pattern part stay live on the slow path.""" + manager = _globby_manager(backend, "c[1]:") + for key in ("user:1", "user:2", "user:10", "User:3", "post:1"): + await manager.set(key, key) + + assert await manager.clear_pattern("user:?") == 2 + assert await manager.clear_pattern("[!u]*") == 2 # "User:3", "post:1" + assert await manager.get("user:10") == "user:10" + assert await manager.clear_pattern("user:*") == 1 + + +async def test_slow_path_matches_the_remainder_not_the_whole_key( + memory_backend: MemoryBackend, +) -> None: + """The prefix must match literally at the start of the key.""" + manager = _globby_manager(memory_backend, "*:") + await manager.set("k", 1) + await memory_backend.set("x:k", _ENTRY) + + assert await manager.clear_pattern("*") == 1 + assert await memory_backend.get("x:k") is not None + + +# --- Memcached --------------------------------------------------------------------- + + +@requires_memcached +@pytest.mark.parametrize( + ("key_prefix", "globby"), + [pytest.param("cache:", False, id="fast"), pytest.param("c[1]:", True, id="slow")], +) +async def test_memcached_returns_zero_with_one_runtime_warning( + key_prefix: str, globby: bool +) -> None: + """Both paths return 0 on Memcached with exactly one RuntimeWarning.""" + from fastapi_cachex.backends import MemcachedBackend + + backend = MemcachedBackend(servers=[MEMCACHED_SERVER]) + manager = ( + _globby_manager(backend, key_prefix) + if globby + else CacheManager(backend=backend, key_prefix=key_prefix) + ) + + with pytest.warns(RuntimeWarning) as record: + removed = await manager.clear_pattern("*") + + assert removed == 0 + assert len(record) == 1