Make the Region Invalidation Strategy more flexible

[JonathanWylie]

I have a use case where I can only determine if cache entry has expired by knowing the key and the time it was created.
As it stands the RegionInvalidationStrategy interface only receives the creation time.
If it was passed the key as well then I could implement the behaviour I need.

The use case is this:
I am querying the google analytics API, and caching the results in Redis. Although you can query for recent data, it is not guaranteed to be up to date.
The key is the datetime for the analytics data, I want to expire a cache entry if it was created less than a day after key (i.e it was recent data when it was created), but only if it is now more than an hour after it was created.
I have currently implemented this by overriding Region._is_cache_miss in a subclass. This is not great, because it only applies to the get_or_create class of methods, get_multi and get use self._unexpired_value_fn, which of course I could also override, but given RegionInvalidationStrategy is the proper way to manage invalidation I think it should be done there, and it also means it only has to be done in one place.

[zzzeek]

hi there -

I agree it seems strange I went through all the trouble to add a custom region invalidation strategy and did not think that the key, which is available, should be passed through as well. that extension was created for someone else's use case, I have no idea what it was.

here is a proposed API. if you could vet this for sanity etc. that would be very helpful, I rarely work with dogpile.cache so I came up with this through rote:

diff --git a/dogpile/cache/region.py b/dogpile/cache/region.py
index 65ed334..224b148 100644
--- a/dogpile/cache/region.py
+++ b/dogpile/cache/region.py
@@ -104,6 +104,9 @@ class RegionInvalidationStrategy:
                 return (self._hard_invalidated and
                         timestamp < self._hard_invalidated)
 
+            def key_is_hard_invalidated(self, key, timestamp):
+                return self.is_hard_invalidated(timestamp)
+
             def was_soft_invalidated(self):
                 return bool(self._soft_invalidated)
 
@@ -111,6 +114,9 @@ class RegionInvalidationStrategy:
                 return (self._soft_invalidated and
                         timestamp < self._soft_invalidated)
 
+            def key_is_soft_invalidated(self, key, timestamp):
+                return self.is_soft_invalidated(timestamp)
+
     The custom implementation is injected into a :class:`.CacheRegion`
     at configure time using the
     :paramref:`.CacheRegion.configure.region_invalidator` parameter::
@@ -164,6 +170,21 @@ class RegionInvalidationStrategy:
 
         raise NotImplementedError()
 
+    def key_is_hard_invalidated(self, key: KeyType, timestamp: float) -> bool:
+        """Check timestamp and key to determine if it was hard invalidated.
+
+        Calls :meth:`.RegionInvalidator.is_hard_invalidated` by default.
+
+        :return: Boolean. True if ``timestamp`` is older than
+         the last region invalidation time and region is invalidated
+         in soft mode.
+
+        .. versionadded:: 1.1.2
+
+        """
+
+        return self.is_hard_invalidated(timestamp)
+
     def is_soft_invalidated(self, timestamp: float) -> bool:
         """Check timestamp to determine if it was soft invalidated.
 
@@ -175,6 +196,21 @@ class RegionInvalidationStrategy:
 
         raise NotImplementedError()
 
+    def key_is_soft_invalidated(self, key: KeyType, timestamp: float) -> bool:
+        """Check timestamp and key to determine if it was soft invalidated.
+
+        Calls :meth:`.RegionInvalidator.is_soft_invalidated` by default.
+
+        :return: Boolean. True if ``timestamp`` is older than
+         the last region invalidation time and region is invalidated
+         in soft mode.
+
+        .. versionadded:: 1.1.2
+
+        """
+
+        return self.is_soft_invalidated(timestamp)
+
     def is_invalidated(self, timestamp: float) -> bool:
         """Check timestamp to determine if it was invalidated.
 
@@ -185,6 +221,18 @@ class RegionInvalidationStrategy:
 
         raise NotImplementedError()
 
+    def key_is_invalidated(self, key: KeyType, timestamp: float) -> bool:
+        """Check timestamp and key to determine if it was invalidated.
+
+        :return: Boolean. True if ``timestamp`` is older than
+         the last region invalidation time.
+
+        .. versionadded:: 1.1.2
+
+        """
+
+        return self.is_invalidated(timestamp)
+
     def was_soft_invalidated(self) -> bool:
         """Indicate the region was invalidated in soft mode.
 
@@ -761,21 +809,21 @@ class CacheRegion:
             key = self.key_mangler(key)
         value = self._get_from_backend(key)
         value = self._unexpired_value_fn(expiration_time, ignore_expiration)(
-            value
+            key, value
         )
 
         return value.payload
 
     def _unexpired_value_fn(self, expiration_time, ignore_expiration):
         if ignore_expiration:
-            return lambda value: value
+            return lambda key, value: value
         else:
             if expiration_time is None:
                 expiration_time = self.expiration_time
 
             current_time = time.time()
 
-            def value_fn(value):
+            def value_fn(key, value):
                 if value is NO_VALUE:
                     return value
                 elif (
@@ -783,8 +831,8 @@ class CacheRegion:
                     and current_time - value.metadata["ct"] > expiration_time
                 ):
                     return NO_VALUE
-                elif self.region_invalidator.is_invalidated(
-                    value.metadata["ct"]
+                elif self.region_invalidator.key_is_invalidated(
+                    key, value.metadata["ct"]
                 ):
                     return NO_VALUE
                 else:
@@ -838,7 +886,7 @@ class CacheRegion:
         return [
             value.payload if value is not NO_VALUE else value
             for value in (
-                _unexpired_value_fn(value) for value in backend_values
+                _unexpired_value_fn(key, value) for key, value in zip(keys, backend_values)
             )
         ]
 
@@ -858,7 +906,7 @@ class CacheRegion:
             log.debug("No value present for key: %r", orig_key)
         elif value.metadata["v"] != value_version:
             log.debug("Dogpile version update for key: %r", orig_key)
-        elif self.region_invalidator.is_hard_invalidated(value.metadata["ct"]):
+        elif self.region_invalidator.key_is_hard_invalidated(orig_key, value.metadata["ct"]):
             log.debug("Hard invalidation detected for key: %r", orig_key)
         else:
             return False
@@ -965,7 +1013,7 @@ class CacheRegion:
                 raise NeedRegenerationException()
 
             ct = cast(CachedValue, value).metadata["ct"]
-            if self.region_invalidator.is_soft_invalidated(ct):
+            if self.region_invalidator.key_is_soft_invalidated(key, ct):
                 if expiration_time is None:
                     raise exception.DogpileCacheException(
                         "Non-None expiration time required "

[zzzeek]

looks like #38 , sort of midway through, is where this came about. I'll name some of the usual suspects in case this issue is interesting to them, @morganfainberg @jvanasco

[jvanasco]

I like the extensions that @zzzeek proposed, however...

Is this best implemented with a region invalidation though?

This looks like a usecase for “wraps” or a custom deserializer/backend. At least I have done similar things with that concept. (I’m
not sure if the internal payload is available in wraps hooks or not). You can just perform the date operations there, and issue a miss so it repopulates with the generator.

[JonathanWylie]

This interface looks fine to me. How would wraps work, isn't that essentially just another way to do what you can do when subclassing? I actually had an implementation using wraps and a proxy, but it wasn't nice, you have to handle the same thing in multiple places.
I think it is reasonable for it to be doable in the CustomInvalidationStrategy, after all that is exactly what it is, a custom implementation of invalidating. This allows you to implement it in a clean way.