-
Notifications
You must be signed in to change notification settings - Fork 0
Open
Labels
enhancementNew feature or requestNew feature or request
Description
Severity
Medium
Location
File: src/redis_func_cache/policies/abstract.py
Lines: 44-65
Description
The AbstractPolicy.cache property uses a weakref proxy that raises RuntimeError if the cache is garbage collected (line 64). This creates an implicit bidirectional dependency where:
- Policies cannot exist independently of a cache instance
- The _cache attribute is set externally by RedisFuncCache.init (cache.py:271)
- Policies cannot be tested independently
- Creates potential for use-after-free bugs
Current implementation:
@property
def cache(self) -> CallableProxyType[RedisFuncCache]:
if self._cache is None:
raise RuntimeError("Policy instance is not bound to a RedisFuncCache")
return self._cacheImpact
- Policies cannot be unit tested without a full RedisFuncCache instance
- Creates tight coupling between components
- Makes dependency injection difficult
- Violates Single Responsibility Principle
Proposed Solution
Use constructor injection with an explicit interface:
class AbstractPolicy(ABC):
def __init__(self, cache_provider: Optional[Callable[[], RedisFuncCache]] = None):
self._cache_provider = cache_provider
self._lua_scripts = None
@property
def cache(self) -> RedisFuncCache:
if self._cache_provider is None:
raise RuntimeError("No cache provider configured")
return self._cache_provider()
def bind_cache(self, cache: RedisFuncCache) -> None:
\"\"\"Bind this policy to a cache instance.\"\"\"
def provider() -> RedisFuncCache:
return cache
self._cache_provider = providerAdditional Context
- Weakref was chosen to prevent circular references
- Alternative: use abstract interfaces instead of concrete coupling
- Would improve testability by allowing mock cache providers
- Could simplify migration to v0.7+ API
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
enhancementNew feature or requestNew feature or request