Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: on-get callback #38

Merged
merged 3 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 34 additions & 9 deletions src/cacheout/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,6 @@
from .stats import CacheStatsTracker


F = t.TypeVar("F", bound=t.Callable[..., t.Any])
T_DECORATOR = t.Callable[[F], F]
T_TTL = t.Union[int, float]
T_FILTER = t.Union[str, t.List[t.Hashable], t.Pattern, t.Callable]

UNSET = object()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should stay at the top right below the imports.



class RemovalCause(Enum):
"""
An enum to represent the cause for the removal of a cache entry.
Expand All @@ -46,6 +38,31 @@ class RemovalCause(Enum):
POPITEM = auto()


F = t.TypeVar("F", bound=t.Callable[..., t.Any])
T_DECORATOR = t.Callable[[F], F]
T_TTL = t.Union[int, float]
T_FILTER = t.Union[str, t.List[t.Hashable], t.Pattern, t.Callable]
ON_GET_CALLBACK = t.Optional[t.Callable[[t.Hashable, t.Any, bool], None]]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefix these with T_*

"""
Callback that will be executed when a cache entry is retrieved.

It is called with arguments ``(key, value, exists)`` where `key` is the cache key,
`value` is the value retrieved (could be the default),
and `exists` is whether the cache key exists or not.
"""
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert these to #: * prefixed comments above the variable declaration like:

#: Callback that will be executed when a cache entry is retrieved. It is called with arguments
#: ``(key, value, exists)`` where `key` is the cache key, `value` is the value retrieved (could be
#: the default), and `exists` is whether the cache key exists or not.
T_ON_DELETE_CALLBACK = ...


ON_DELETE_CALLBACK = t.Optional[t.Callable[[t.Hashable, t.Any, RemovalCause], None]]
"""
Callback that will be executed when a cache entry is removed.

It is called with arguments ``(key, value, cause)`` where `key` is the cache key,
`value` is the cached value at the time of deletion,
and `cause` is the reason the key was removed (see :class:`RemovalCause` for enumerated causes).
"""

UNSET = object()


class Cache:
"""
An in-memory, FIFO cache object.
Expand Down Expand Up @@ -74,6 +91,7 @@ class Cache:
default: Default value or function to use in :meth:`get` when key is not found. If callable,
it will be passed a single argument, ``key``, and its return value will be set for that
cache key.
on_get: Callback which will be executed when a cache entry is retrieved.
on_delete: Callback which will be executed when a cache entry is removed.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking of putting the variable descriptions here was so that it would be prominent when viewing the class documentation. Does any of the details from the constant docstrings show up here in the generated docs?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could add a class reference so that there's at least a link to the variable docs using something like:

See :class:`T_ON_DELETE_CALLBACK`.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking of putting the variable descriptions here was so that it would be prominent when viewing the class documentation. Does any of the details from the constant docstrings show up here in the generated docs?

Yes, it works.

stats: Cache statistics.
"""
Expand All @@ -90,12 +108,14 @@ def __init__(
timer: t.Callable[[], T_TTL] = time.time,
default: t.Any = None,
enable_stats: bool = False,
on_delete: t.Optional[t.Callable[[t.Hashable, t.Any, RemovalCause], None]] = None,
on_get: ON_GET_CALLBACK = None,
on_delete: ON_DELETE_CALLBACK = None,
):
self.maxsize = maxsize
self.ttl = ttl
self.timer = timer
self.default = default
self.on_get = on_get
self.on_delete = on_delete
self.stats = CacheStatsTracker(self, enable=enable_stats)

Expand Down Expand Up @@ -255,6 +275,7 @@ def get(self, key: t.Hashable, default: t.Any = None) -> t.Any:
return self._get(key, default=default)

def _get(self, key: t.Hashable, default: t.Any = None) -> t.Any:
existed = True
try:
value = self._cache[key]

Expand All @@ -263,6 +284,7 @@ def _get(self, key: t.Hashable, default: t.Any = None) -> t.Any:
raise KeyError
self.stats.inc_hit_count()
except KeyError:
existed = False
self.stats.inc_miss_count()
if default is None:
default = self.default
Expand All @@ -273,6 +295,9 @@ def _get(self, key: t.Hashable, default: t.Any = None) -> t.Any:
else:
value = default

if self.on_get:
self.on_get(key, value, existed)

return value

def get_many(self, iteratee: T_FILTER) -> dict:
Expand Down
30 changes: 24 additions & 6 deletions tests/test_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -719,32 +719,50 @@ def test_cache_on_delete(cache: Cache, timer: Timer):

def on_delete(key, value, cause):
nonlocal log
log = f"{key}:{value} {cause.value}"
log = f"{key}={value}, RemovalCause={cause.value}"

cache.on_delete = on_delete
cache.set("DELETE", 1)
cache.delete("DELETE")
assert log == f"DELETE:1 {RemovalCause.DELETE.value}"
assert log == f"DELETE=1, RemovalCause={RemovalCause.DELETE.value}"

cache.set("SET", 1)
cache.set("SET", 2)
assert log == f"SET:1 {RemovalCause.SET.value}"
assert log == f"SET=1, RemovalCause={RemovalCause.SET.value}"

cache.clear()
cache.set("POPITEM", 1)
cache.popitem()
assert log == f"POPITEM:1 {RemovalCause.POPITEM.value}"
assert log == f"POPITEM=1, RemovalCause={RemovalCause.POPITEM.value}"

cache.set("EXPIRED", 1, ttl=1)
timer.time = 1
cache.delete_expired()
assert log == f"EXPIRED:1 {RemovalCause.EXPIRED.value}"
assert log == f"EXPIRED=1, RemovalCause={RemovalCause.EXPIRED.value}"

cache.clear()
cache.maxsize = 1
cache.set("FULL", 1)
cache.set("OVERFLOW", 2)
assert log == f"FULL:1 {RemovalCause.FULL.value}"
assert log == f"FULL=1, RemovalCause={RemovalCause.FULL.value}"


def test_cache_on_get(cache: Cache):
"""Test that on_get(cache) callback."""
log = ""

def on_get(key, value, existed):
nonlocal log
log = f"{key}={value}, existed={existed}"

cache.on_get = on_get
cache.set("hit", 1)

cache.get("hit")
assert log == "hit=1, existed=True"

cache.get("miss")
assert log == "miss=None, existed=False"


def test_cache_stats__disabled_by_default(cache: Cache):
Expand Down