diff --git a/CHANGELOG.md b/CHANGELOG.md index 49c5c12a..14c49b55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,45 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). --- +## v26.06.15 (2026-06-05) + +### Fixed + +- **`CacheManager` now satisfies the `CacheAdapter` protocol.** It implemented + only `get`/`put`/`evict`/`clear`, so `isinstance(mgr, CacheAdapter)` was `False` + and passing one as a `@cacheable` backend raised `AttributeError: 'CacheManager' + object has no attribute 'exists'` on the first null-cached hit. Added the + missing `exists` / `put_if_absent` / `evict_by_prefix` / `start` / `stop` + (mirrored to both primary and fallback). +- **Cache decorators reject a sync target with a clear error.** `@cacheable` / + `@cache_evict` / `@cache_put` await an async backend, so decorating a sync + function used to fail with a cryptic `await` `TypeError` at call time; it now + raises a clear `TypeError` at decoration time (cache adapters are async-only). +- **Bad key templates raise a clear `ValueError`.** A `{param}` template + referencing a name not in the function signature raised a bare `KeyError` at + call time; it now names the unknown parameter. + +### Added + +- **`InMemoryCache(max_size=...)` with LRU eviction.** The in-memory adapter was + unbounded (the advertised `max_size` stat was always `None`). It now accepts an + optional `max_size` (wired from `pyfly.cache.max-size`) and evicts the + least-recently-used entry on overflow; the default remains unbounded. + +### Notes + +- Documented two by-design properties surfaced by the audit: cache **keys are + namespaced by the backend instance + key template** (reuse the same template + across methods only for the same logical entry — that is what lets + `@cache_evict` invalidate a `@cacheable` entry), and the **Redis JSON + round-trip is lossy** (a cached Pydantic model returns as a `dict` on a Redis + hit, unlike the in-memory adapter). + +These surfaced in an edge-case audit while validating the `implement-cache-strategy` +skill (which validated clean — cache hits skip the source and evict invalidates). + +--- + ## v26.06.14 (2026-06-05) ### Fixed diff --git a/README.md b/README.md index 92c493f8..bda8d443 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ Firefly Framework Python 3.12+ License: Apache 2.0 - Version: 26.06.14 + Version: 26.06.15 Type Checked: mypy strict Code Style: Ruff Async First diff --git a/pyproject.toml b/pyproject.toml index 7b054c34..91c87359 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ name = "pyfly" # CalVer YY.MM.PATCH — package metadata uses PEP 440 normalized form (26.5.4); # git tag, GitHub release and human-readable display use leading-zero form # (v26.05.04) to match the Java/.NET/Go siblings. -version = "26.6.14" +version = "26.6.15" description = "The official Python implementation of the Firefly Framework — DI, CQRS, EDA, hexagonal architecture, and more." readme = "README.md" license = "Apache-2.0" diff --git a/src/pyfly/__init__.py b/src/pyfly/__init__.py index c970b557..6a96548e 100644 --- a/src/pyfly/__init__.py +++ b/src/pyfly/__init__.py @@ -13,4 +13,4 @@ # limitations under the License. """PyFly — Enterprise Python Framework.""" -__version__ = "26.06.14" +__version__ = "26.06.15" diff --git a/src/pyfly/cache/adapters/memory.py b/src/pyfly/cache/adapters/memory.py index e80652fc..fa24c4f2 100644 --- a/src/pyfly/cache/adapters/memory.py +++ b/src/pyfly/cache/adapters/memory.py @@ -16,19 +16,26 @@ from __future__ import annotations import time +from collections import OrderedDict from datetime import timedelta from typing import Any class InMemoryCache: - """In-memory cache with optional TTL support. + """In-memory cache with optional TTL and LRU bounding. Suitable for development, testing, and single-process applications. Also serves as the default fallback in CacheManager. + + Args: + max_size: When set, the cache holds at most this many entries and evicts + the least-recently-used entry on overflow. ``None`` (default) leaves + the cache unbounded — rely on TTLs to bound memory. """ - def __init__(self) -> None: - self._store: dict[str, tuple[Any, float | None]] = {} + def __init__(self, max_size: int | None = None) -> None: + self._store: OrderedDict[str, tuple[Any, float | None]] = OrderedDict() + self._max_size = max_size self._hits = 0 self._misses = 0 self._evictions = 0 @@ -46,15 +53,21 @@ async def get(self, key: str) -> Any | None: self._misses += 1 return None + self._store.move_to_end(key) # mark most-recently-used (LRU) self._hits += 1 return value async def put(self, key: str, value: Any, ttl: timedelta | None = None) -> None: - """Store a value with optional TTL.""" + """Store a value with optional TTL, evicting the LRU entry when full.""" expires_at = None if ttl is not None: expires_at = time.monotonic() + ttl.total_seconds() self._store[key] = (value, expires_at) + self._store.move_to_end(key) + if self._max_size is not None: + while len(self._store) > self._max_size: + self._store.popitem(last=False) # evict least-recently-used + self._evictions += 1 async def put_if_absent(self, key: str, value: Any, ttl: timedelta | None = None) -> bool: """Store *value* only if *key* is absent — atomic under asyncio (audit #75).""" @@ -98,7 +111,7 @@ def get_stats(self) -> dict[str, Any]: return { "size": active, "type": "memory", - "max_size": None, + "max_size": self._max_size, "requests": requests, "hits": self._hits, "misses": self._misses, diff --git a/src/pyfly/cache/auto_configuration.py b/src/pyfly/cache/auto_configuration.py index ef5a082e..cdabfbc3 100644 --- a/src/pyfly/cache/auto_configuration.py +++ b/src/pyfly/cache/auto_configuration.py @@ -57,7 +57,9 @@ def cache_adapter(self, config: Config) -> CacheAdapter: from pyfly.cache.adapters.memory import InMemoryCache - return InMemoryCache() + raw_max_size = config.get("pyfly.cache.max-size", None) + max_size = int(raw_max_size) if raw_max_size is not None else None + return InMemoryCache(max_size=max_size) @bean @conditional_on_property("pyfly.observability.health.enabled", having_value="true", match_if_missing=True) diff --git a/src/pyfly/cache/decorators.py b/src/pyfly/cache/decorators.py index 32af6168..07f1d54c 100644 --- a/src/pyfly/cache/decorators.py +++ b/src/pyfly/cache/decorators.py @@ -26,6 +26,39 @@ F = TypeVar("F", bound=Callable[..., Any]) +def _require_async(func: Callable[..., Any], decorator_name: str) -> None: + """Reject a sync target with a clear error at decoration time. + + Cache adapters are async, so the wrappers must ``await`` the backend; a sync + target would otherwise fail with a cryptic ``await`` ``TypeError`` at call + time. Make a synchronous function an explicit, immediate error instead. + """ + if not inspect.iscoroutinefunction(func): + raise TypeError( + f"{decorator_name} requires an async function; " + f"'{func.__qualname__}' is synchronous (cache adapters are async-only)." + ) + + +def _resolve_key(func: Callable[..., Any], key: str, args: tuple[Any, ...], kwargs: dict[str, Any]) -> str: + """Resolve a ``{param}`` key template against the call's bound arguments. + + The cache key must uniquely identify the value *within its backend*: the + backend instance plus this key form the cache namespace. Reuse the same + template across methods only when they refer to the same logical entry — that + is what lets a ``@cache_evict`` invalidate a ``@cacheable`` entry; two + unrelated methods sharing one backend must use distinct templates. + """ + bound = inspect.signature(func).bind(*args, **kwargs) + bound.apply_defaults() + try: + return key.format(**bound.arguments) + except (KeyError, IndexError) as exc: + raise ValueError( + f"Cache key template {key!r} for '{func.__qualname__}' references unknown parameter {exc}." + ) from exc + + def cache( backend: CacheAdapter, key: str, @@ -44,13 +77,11 @@ def cache( """ def decorator(func: F) -> F: + _require_async(func, "@cache/@cacheable") + @functools.wraps(func) async def wrapper(*args: Any, **kwargs: Any) -> Any: - # Resolve the cache key from function arguments - sig = inspect.signature(func) - bound = sig.bind(*args, **kwargs) - bound.apply_defaults() - resolved_key = key.format(**bound.arguments) + resolved_key = _resolve_key(func, key, args, kwargs) # Check cache. A present-but-None entry is a hit (null caching / # cache-penetration protection), distinguished via exists (audit #80). @@ -101,17 +132,15 @@ def cache_evict( """ def decorator(func: F) -> F: + _require_async(func, "@cache_evict") + @functools.wraps(func) async def wrapper(*args: Any, **kwargs: Any) -> Any: result = await func(*args, **kwargs) if all_entries: await backend.clear() else: - sig = inspect.signature(func) - bound = sig.bind(*args, **kwargs) - bound.apply_defaults() - resolved_key = key.format(**bound.arguments) - await backend.evict(resolved_key) + await backend.evict(_resolve_key(func, key, args, kwargs)) return result return wrapper # type: ignore[return-value] @@ -137,14 +166,12 @@ def cache_put( """ def decorator(func: F) -> F: + _require_async(func, "@cache_put") + @functools.wraps(func) async def wrapper(*args: Any, **kwargs: Any) -> Any: result = await func(*args, **kwargs) - sig = inspect.signature(func) - bound = sig.bind(*args, **kwargs) - bound.apply_defaults() - resolved_key = key.format(**bound.arguments) - await backend.put(resolved_key, result, ttl=ttl) + await backend.put(_resolve_key(func, key, args, kwargs), result, ttl=ttl) return result return wrapper # type: ignore[return-value] diff --git a/src/pyfly/cache/manager.py b/src/pyfly/cache/manager.py index 3964fe91..1b7fb70c 100644 --- a/src/pyfly/cache/manager.py +++ b/src/pyfly/cache/manager.py @@ -75,3 +75,51 @@ async def clear(self) -> None: logger.warning("Primary cache failed for CLEAR") await self._fallback.clear() + + async def put_if_absent(self, key: str, value: Any, ttl: timedelta | None = None) -> bool: + """Store only if absent; mirror to both caches.""" + result = False + try: + result = await self._primary.put_if_absent(key, value, ttl=ttl) + except Exception: + logger.warning("Primary cache failed for PUT_IF_ABSENT '%s', using fallback only", key) + + fallback_result = await self._fallback.put_if_absent(key, value, ttl=ttl) + return result or fallback_result + + async def evict_by_prefix(self, prefix: str) -> int: + """Evict matching keys from both caches; return the total removed.""" + primary_count = 0 + try: + primary_count = await self._primary.evict_by_prefix(prefix) + except Exception: + logger.warning("Primary cache failed for EVICT_BY_PREFIX '%s'", prefix) + + fallback_count = await self._fallback.evict_by_prefix(prefix) + return primary_count + fallback_count + + async def exists(self, key: str) -> bool: + """True if either cache holds the key.""" + try: + if await self._primary.exists(key): + return True + except Exception: + logger.warning("Primary cache failed for EXISTS '%s', falling back", key) + + return await self._fallback.exists(key) + + async def start(self) -> None: + """Start both cache adapters.""" + for adapter in (self._primary, self._fallback): + try: + await adapter.start() + except Exception: + logger.warning("A cache adapter failed to start") + + async def stop(self) -> None: + """Stop both cache adapters.""" + for adapter in (self._primary, self._fallback): + try: + await adapter.stop() + except Exception: + logger.warning("A cache adapter failed to stop") diff --git a/src/pyfly/cache/serialization.py b/src/pyfly/cache/serialization.py index 4f85815b..43f71a26 100644 --- a/src/pyfly/cache/serialization.py +++ b/src/pyfly/cache/serialization.py @@ -15,8 +15,15 @@ Plain ``json.dumps`` raises ``TypeError`` on datetime/Decimal/UUID/set/Pydantic values — extremely common in this Pydantic-heavy framework — which previously -crashed the cache write path (audit #72). This encoder converts those to a -JSON-safe form so a cache put never raises. +crashed the cache write path (audit #72). This encoder converts the common +framework types to a JSON-safe form; a value that is still not serializable +raises ``TypeError`` on write. + +Note: the round-trip is lossy because storage is JSON. A Redis cache hit returns +JSON types — a cached Pydantic model comes back as a ``dict``, and +``Decimal``/``UUID``/``datetime`` as strings — unlike the in-memory adapter, +which stores the live object by reference. Reconstruct from the declared type if +you need the original object on a Redis-backed cache. """ from __future__ import annotations @@ -52,7 +59,11 @@ def cache_dumps(value: Any) -> bytes: def cache_loads(raw: Any) -> Any: - """Deserialize cached JSON bytes/str back to a Python object.""" + """Deserialize cached JSON bytes/str back to a Python object. + + Returns plain JSON types (dict/list/str/int/float/bool/None); typed values + such as Pydantic models are not reconstructed (see the module docstring). + """ if isinstance(raw, bytes): raw = raw.decode("utf-8") return json.loads(raw) diff --git a/tests/cache/test_cache_hardening.py b/tests/cache/test_cache_hardening.py new file mode 100644 index 00000000..cc0b0f92 --- /dev/null +++ b/tests/cache/test_cache_hardening.py @@ -0,0 +1,105 @@ +# Copyright 2026 Firefly Software Foundation. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Regression tests for cache hardening (v26.06.15). + +- CacheManager satisfies the full CacheAdapter protocol and works as a decorator + backend (it used to lack exists/put_if_absent/evict_by_prefix/start/stop). +- Cache decorators reject a sync target with a clear error at decoration time. +- A bad key template raises a clear ValueError (not a cryptic KeyError). +- InMemoryCache(max_size=...) bounds the cache with LRU eviction; default is unbounded. +""" + +from __future__ import annotations + +import pytest + +from pyfly.cache import CacheAdapter, CacheManager, cacheable +from pyfly.cache.adapters.memory import InMemoryCache + + +class TestCacheManagerProtocol: + def test_satisfies_cache_adapter_protocol(self) -> None: + mgr = CacheManager(InMemoryCache(), InMemoryCache()) + assert isinstance(mgr, CacheAdapter) + + @pytest.mark.asyncio + async def test_new_methods_delegate_to_both(self) -> None: + mgr = CacheManager(InMemoryCache(), InMemoryCache()) + await mgr.start() + assert await mgr.put_if_absent("k", "v") is True + assert await mgr.put_if_absent("k", "v2") is False # already present + assert await mgr.exists("k") is True + + await mgr.put("p:1", 1) + await mgr.put("p:2", 2) + assert await mgr.evict_by_prefix("p:") >= 2 + assert await mgr.exists("p:1") is False + await mgr.stop() + + @pytest.mark.asyncio + async def test_usable_as_decorator_backend(self) -> None: + # The cacheable decorator calls backend.exists() on the null-caching path; + # a CacheManager used to AttributeError there. Now it works. + mgr = CacheManager(InMemoryCache(), InMemoryCache()) + calls = {"n": 0} + + @cacheable(backend=mgr, key="x:{n}") + async def get(n: int) -> None: + calls["n"] += 1 + return None + + assert await get(1) is None + assert await get(1) is None # cached None -> exists() path, no AttributeError + assert calls["n"] == 1 + + +class TestDecoratorGuards: + def test_sync_function_rejected_at_decoration(self) -> None: + with pytest.raises(TypeError, match="requires an async function"): + + @cacheable(backend=InMemoryCache(), key="k") + def sync_fn() -> int: + return 1 + + @pytest.mark.asyncio + async def test_bad_key_template_gives_clear_error(self) -> None: + @cacheable(backend=InMemoryCache(), key="x:{missing}") + async def fn(n: int) -> int: + return n + + with pytest.raises(ValueError, match="unknown parameter"): + await fn(1) + + +class TestInMemoryMaxSize: + @pytest.mark.asyncio + async def test_lru_eviction_when_full(self) -> None: + cache = InMemoryCache(max_size=2) + await cache.put("a", 1) + await cache.put("b", 2) + assert await cache.get("a") == 1 # 'a' becomes most-recently-used; 'b' is LRU + await cache.put("c", 3) # over capacity -> evict LRU ('b') + + assert await cache.exists("b") is False + assert await cache.get("a") == 1 + assert await cache.get("c") == 3 + assert cache.get_stats()["max_size"] == 2 + + @pytest.mark.asyncio + async def test_unbounded_by_default(self) -> None: + cache = InMemoryCache() + for i in range(100): + await cache.put(f"k{i}", i) + assert len(cache.get_keys()) == 100 + assert cache.get_stats()["max_size"] is None diff --git a/uv.lock b/uv.lock index 7c484705..9f39851e 100644 --- a/uv.lock +++ b/uv.lock @@ -1967,7 +1967,7 @@ wheels = [ [[package]] name = "pyfly" -version = "26.6.14" +version = "26.6.15" source = { editable = "." } dependencies = [ { name = "pydantic" },