From 3e6ac1ddb598b20c4e91c1e92be2281e92ece196 Mon Sep 17 00:00:00 2001 From: Ahter Sonmez Date: Thu, 1 Jun 2023 16:11:03 +0100 Subject: [PATCH 1/8] Add `key_group` as a new parameter We'd like to group all the cache-related objects into a single identifier so that it opens up new doors for the various cache invalidation operations. --- django/views/decorators/cache.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/django/views/decorators/cache.py b/django/views/decorators/cache.py index aa1679baff1c..f59b8c35c29f 100644 --- a/django/views/decorators/cache.py +++ b/django/views/decorators/cache.py @@ -7,7 +7,7 @@ from django.utils.decorators import decorator_from_middleware_with_args -def cache_page(timeout, *, cache=None, key_prefix=None): +def cache_page(timeout, *, cache=None, key_prefix=None, key_group=None): """ Decorator for views that tries getting the page from the cache and populates the cache if the page isn't in the cache yet. @@ -25,6 +25,7 @@ def cache_page(timeout, *, cache=None, key_prefix=None): page_timeout=timeout, cache_alias=cache, key_prefix=key_prefix, + key_group=key_group, ) From d2d182ef128b784a055cee6fe6c2a6ce241cd9d0 Mon Sep 17 00:00:00 2001 From: Ahter Sonmez Date: Thu, 1 Jun 2023 16:38:57 +0100 Subject: [PATCH 2/8] Refactor the key generation methods WIP explantions here. --- django/utils/cache.py | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/django/utils/cache.py b/django/utils/cache.py index cf797d0279a4..04d7e0769a5b 100644 --- a/django/utils/cache.py +++ b/django/utils/cache.py @@ -29,6 +29,7 @@ cc_delim_re = _lazy_re_compile(r"\s*,\s*") +PATH_PREFIX = "views.decorators.cache" def patch_cache_control(response, **kwargs): """ @@ -347,29 +348,35 @@ def _i18n_cache_key_suffix(request, cache_key): def _generate_cache_key(request, method, headerlist, key_prefix): - """Return a cache key from the headers given in the header list.""" + """ + Return a cache key from the headers given in the header list. + """ + cache_type = "cache_page" + ctx = md5(usedforsecurity=False) for header in headerlist: value = request.META.get(header) if value is not None: ctx.update(value.encode()) url = md5(request.build_absolute_uri().encode("ascii"), usedforsecurity=False) - cache_key = "views.decorators.cache.cache_page.%s.%s.%s.%s" % ( - key_prefix, - method, - url.hexdigest(), - ctx.hexdigest(), - ) + + url_hex = url.hexdigest() + ctx_hex = ctx.hexdigest() + cache_key = f"{PATH_PREFIX}.{cache_type}.{key_prefix}.{method}.{url_hex}.{ctx_hex}" + return _i18n_cache_key_suffix(request, cache_key) def _generate_cache_header_key(key_prefix, request): - """Return a cache key for the header cache.""" + """ + Return a cache key for the header cache. + """ + cache_type = "cache_header" + url = md5(request.build_absolute_uri().encode("ascii"), usedforsecurity=False) - cache_key = "views.decorators.cache.cache_header.%s.%s" % ( - key_prefix, - url.hexdigest(), - ) + url_hex = url.hexdigest() + cache_key = f"{PATH_PREFIX}.{cache_type}.{key_prefix}.{url_hex}" + return _i18n_cache_key_suffix(request, cache_key) From 13b50760e37deeb6d240ead0662406bbbe1b6d91 Mon Sep 17 00:00:00 2001 From: Ahter Sonmez Date: Thu, 1 Jun 2023 17:01:55 +0100 Subject: [PATCH 3/8] Populate the cache key with `key_group` The key group will enable us to invalidate the cache based on many different groupings. --- django/utils/cache.py | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/django/utils/cache.py b/django/utils/cache.py index 04d7e0769a5b..fe8c0ca8bf6e 100644 --- a/django/utils/cache.py +++ b/django/utils/cache.py @@ -31,6 +31,7 @@ PATH_PREFIX = "views.decorators.cache" + def patch_cache_control(response, **kwargs): """ Patch the Cache-Control header by adding all keyword arguments to it. @@ -347,7 +348,7 @@ def _i18n_cache_key_suffix(request, cache_key): return cache_key -def _generate_cache_key(request, method, headerlist, key_prefix): +def _generate_cache_key(request, method, headerlist, key_prefix, key_group: str): """ Return a cache key from the headers given in the header list. """ @@ -362,12 +363,12 @@ def _generate_cache_key(request, method, headerlist, key_prefix): url_hex = url.hexdigest() ctx_hex = ctx.hexdigest() - cache_key = f"{PATH_PREFIX}.{cache_type}.{key_prefix}.{method}.{url_hex}.{ctx_hex}" + cache_key = f"{PATH_PREFIX}.{cache_type}.{key_group}.{key_prefix}.{method}.{url_hex}.{ctx_hex}" return _i18n_cache_key_suffix(request, cache_key) -def _generate_cache_header_key(key_prefix, request): +def _generate_cache_header_key(key_prefix: str, key_group: str, request) -> str: """ Return a cache key for the header cache. """ @@ -375,12 +376,12 @@ def _generate_cache_header_key(key_prefix, request): url = md5(request.build_absolute_uri().encode("ascii"), usedforsecurity=False) url_hex = url.hexdigest() - cache_key = f"{PATH_PREFIX}.{cache_type}.{key_prefix}.{url_hex}" + cache_key = f"{PATH_PREFIX}.{cache_type}.{key_group}.{key_prefix}.{url_hex}" return _i18n_cache_key_suffix(request, cache_key) -def get_cache_key(request, key_prefix=None, method="GET", cache=None): +def get_cache_key(request, key_group, key_prefix=None, method="GET", cache=None): """ Return a cache key based on the request URL and query. It can be used in the request phase because it pulls the list of headers to take into @@ -397,13 +398,22 @@ def get_cache_key(request, key_prefix=None, method="GET", cache=None): cache = caches[settings.CACHE_MIDDLEWARE_ALIAS] headerlist = cache.get(cache_key) if headerlist is not None: - return _generate_cache_key(request, method, headerlist, key_prefix) + return _generate_cache_key(request, method, headerlist, key_prefix, key_group) else: return None -def learn_cache_key(request, response, cache_timeout=None, key_prefix=None, cache=None): +def learn_cache_key( + request, response, cache_timeout=None, key_prefix=None, key_group=None, cache=None +): """ + :param request: The request object. + :param response: The response object. + :param cache_timeout: The cache timeout in seconds. + :param key_prefix: The cache key prefix. + :param key_group: The cache key group. + :param cache: The cache object. + Learn what headers to take into account for some request URL from the response object. Store those headers in a global URL registry so that later access to that URL will know what headers to take into account @@ -419,9 +429,13 @@ def learn_cache_key(request, response, cache_timeout=None, key_prefix=None, cach key_prefix = settings.CACHE_MIDDLEWARE_KEY_PREFIX if cache_timeout is None: cache_timeout = settings.CACHE_MIDDLEWARE_SECONDS - cache_key = _generate_cache_header_key(key_prefix, request) + + # Generate the cache key for the header cache. + cache_key = _generate_cache_header_key(key_prefix, key_group, request) if cache is None: cache = caches[settings.CACHE_MIDDLEWARE_ALIAS] + + # Handle the Vary header. if response.has_header("Vary"): is_accept_language_redundant = settings.USE_I18N # If i18n is used, the generated cache key will be suffixed with the @@ -435,12 +449,14 @@ def learn_cache_key(request, response, cache_timeout=None, key_prefix=None, cach headerlist.append("HTTP_" + header) headerlist.sort() cache.set(cache_key, headerlist, cache_timeout) - return _generate_cache_key(request, request.method, headerlist, key_prefix) + return _generate_cache_key( + request, request.method, headerlist, key_prefix, key_group + ) else: # if there is no Vary header, we still need a cache key # for the request.build_absolute_uri() cache.set(cache_key, [], cache_timeout) - return _generate_cache_key(request, request.method, [], key_prefix) + return _generate_cache_key(request, request.method, [], key_prefix, key_group) def _to_tuple(s): From 725f3139813644906428d699ba7b979aa9c95068 Mon Sep 17 00:00:00 2001 From: msilva Date: Fri, 2 Jun 2023 11:31:14 +0100 Subject: [PATCH 4/8] Add support for `key_group` on `CacheMiddleware` and it's test case --- django/middleware/cache.py | 21 ++++++++++++++++++--- django/utils/cache.py | 2 +- tests/cache/tests.py | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/django/middleware/cache.py b/django/middleware/cache.py index 0fdffe1bbeee..07c95b84ebdc 100644 --- a/django/middleware/cache.py +++ b/django/middleware/cache.py @@ -71,6 +71,7 @@ def __init__(self, get_response): self.page_timeout = None self.key_prefix = settings.CACHE_MIDDLEWARE_KEY_PREFIX self.cache_alias = settings.CACHE_MIDDLEWARE_ALIAS + self.key_group = None @property def cache(self): @@ -115,8 +116,11 @@ def process_response(self, request, response): return response patch_response_headers(response, timeout) if timeout and response.status_code == 200: + key_group = self.key_group + if callable(key_group): + key_group = key_group(request) cache_key = learn_cache_key( - request, response, timeout, self.key_prefix, cache=self.cache + request, response, timeout, self.key_prefix, key_group, cache=self.cache, ) if hasattr(response, "render") and callable(response.render): response.add_post_render_callback( @@ -140,6 +144,7 @@ def __init__(self, get_response): super().__init__(get_response) self.key_prefix = settings.CACHE_MIDDLEWARE_KEY_PREFIX self.cache_alias = settings.CACHE_MIDDLEWARE_ALIAS + self.key_group = None @property def cache(self): @@ -148,14 +153,17 @@ def cache(self): def process_request(self, request): """ Check whether the page is already cached and return the cached - version if available. + version if available, otherwise returns None. """ if request.method not in ("GET", "HEAD"): request._cache_update_cache = False return None # Don't bother checking the cache. # try and get the cached GET response - cache_key = get_cache_key(request, self.key_prefix, "GET", cache=self.cache) + key_group = self.key_group + if callable(key_group): + key_group = key_group(request) + cache_key = get_cache_key(request, key_group, self.key_prefix, "GET", cache=self.cache) if cache_key is None: request._cache_update_cache = True return None # No cache information available, need to rebuild. @@ -205,6 +213,13 @@ def __init__(self, get_response, cache_timeout=None, page_timeout=None, **kwargs self.cache_alias = cache_alias except KeyError: pass + try: + key_group = kwargs["key_group"] + if key_group is None: + key_group = "" + self.key_group = key_group + except KeyError: + pass if cache_timeout is not None: self.cache_timeout = cache_timeout diff --git a/django/utils/cache.py b/django/utils/cache.py index fe8c0ca8bf6e..04e1a54ed159 100644 --- a/django/utils/cache.py +++ b/django/utils/cache.py @@ -393,7 +393,7 @@ def get_cache_key(request, key_group, key_prefix=None, method="GET", cache=None) """ if key_prefix is None: key_prefix = settings.CACHE_MIDDLEWARE_KEY_PREFIX - cache_key = _generate_cache_header_key(key_prefix, request) + cache_key = _generate_cache_header_key(key_prefix, key_group, request) if cache is None: cache = caches[settings.CACHE_MIDDLEWARE_ALIAS] headerlist = cache.get(cache_key) diff --git a/tests/cache/tests.py b/tests/cache/tests.py index fcce9579d48c..12a37b2945f6 100644 --- a/tests/cache/tests.py +++ b/tests/cache/tests.py @@ -2578,6 +2578,40 @@ def test_middleware(self): self.assertIsNotNone(result) self.assertEqual(result.content, b"Hello World 1") + def test_middleware_group(self): + def get_group2(*args, **kwargs): + return "group2" + + middleware_group1 = CacheMiddleware(hello_world_view, key_group="group1") + middleware_group2_callable = CacheMiddleware(hello_world_view, key_group=get_group2) + + request = self.factory.get("/view/") + + # Put the request through the request middleware + result = middleware_group1.process_request(request) + self.assertIsNone(result) # Is None because there's no cache + + response = hello_world_view(request, "1") + + # Now put the response through the response middleware + response = middleware_group1.process_response(request, response) + + # Repeating the request should result in a cache hit + result = middleware_group1.process_request(request) + self.assertIsNotNone(result) + + # Put the request through the request middleware + result = middleware_group2_callable.process_request(request) + self.assertIsNone(result) # Is None because there's no cache + + # Now put the response through the response middleware + response = middleware_group2_callable.process_response(request, response) + + # Repeating the request should result in a cache hit + result = middleware_group2_callable.process_request(request) + self.assertIsNotNone(result) + + def test_view_decorator(self): # decorate the same view with different cache decorators default_view = cache_page(3)(hello_world_view) From 5b976812047b51c90abe6f4c784b861d526ec998 Mon Sep 17 00:00:00 2001 From: Ahter Sonmez Date: Fri, 2 Jun 2023 13:30:31 +0100 Subject: [PATCH 5/8] Add a simple test to check cache invalidation This uses the key group to invalidate the cache that is stored for the view. --- tests/cache/tests.py | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/tests/cache/tests.py b/tests/cache/tests.py index 12a37b2945f6..4a78253c10d2 100644 --- a/tests/cache/tests.py +++ b/tests/cache/tests.py @@ -2583,7 +2583,9 @@ def get_group2(*args, **kwargs): return "group2" middleware_group1 = CacheMiddleware(hello_world_view, key_group="group1") - middleware_group2_callable = CacheMiddleware(hello_world_view, key_group=get_group2) + middleware_group2_callable = CacheMiddleware( + hello_world_view, key_group=get_group2 + ) request = self.factory.get("/view/") @@ -2611,7 +2613,6 @@ def get_group2(*args, **kwargs): result = middleware_group2_callable.process_request(request) self.assertIsNotNone(result) - def test_view_decorator(self): # decorate the same view with different cache decorators default_view = cache_page(3)(hello_world_view) @@ -2972,3 +2973,32 @@ def test_all(self): # .all() initializes all caches. self.assertEqual(len(test_caches.all(initialized_only=True)), 2) self.assertEqual(test_caches.all(), test_caches.all(initialized_only=True)) + + +class TestCacheInvalidation(SimpleTestCase): + def test_view_cache_invalidation(self): + """ + Test that the view cache is correctly invalidated. + """ + # Set up the view. Decorate it with the cache_page decorator. + decorated_view = cache_page(timeout=3, key_group="test-key-group")( + hello_world_view + ) + + # Set up the request. + request = self.factory.get("/view/") + + # Request the view for the first time. + response = decorated_view(request, "1") + self.assertEqual(response.content, b"Hello World 1") + + # Request again -- hit the cache. + response = decorated_view(request, "2") + self.assertEqual(response.content, b"Hello World 1") + + # Run the cache invalidation. + invalidate_key_group(key_group="test-key-group") + + # Request again -- should NOT hit the cache. + response = decorated_view(request, "3") + self.assertEqual(response.content, b"Hello World 3") From e43824c544db9637ac5476ab5a1bf563f08224b4 Mon Sep 17 00:00:00 2001 From: msilva Date: Fri, 2 Jun 2023 13:49:42 +0100 Subject: [PATCH 6/8] Add cache key_group invalidation --- django/middleware/cache.py | 27 +++++++++++++++++++++------ django/utils/cache.py | 7 +++++++ tests/cache/tests.py | 24 ++++++++---------------- 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/django/middleware/cache.py b/django/middleware/cache.py index 07c95b84ebdc..14c0ada363cc 100644 --- a/django/middleware/cache.py +++ b/django/middleware/cache.py @@ -43,7 +43,11 @@ """ +from functools import cache +from hashlib import md5 +import uuid from django.conf import settings +from django.utils import timezone from django.core.cache import DEFAULT_CACHE_ALIAS, caches from django.utils.cache import ( get_cache_key, @@ -116,9 +120,9 @@ def process_response(self, request, response): return response patch_response_headers(response, timeout) if timeout and response.status_code == 200: - key_group = self.key_group - if callable(key_group): - key_group = key_group(request) + + key_group = handle_key_group(self.key_group, self.cache) + cache_key = learn_cache_key( request, response, timeout, self.key_prefix, key_group, cache=self.cache, ) @@ -160,9 +164,8 @@ def process_request(self, request): return None # Don't bother checking the cache. # try and get the cached GET response - key_group = self.key_group - if callable(key_group): - key_group = key_group(request) + key_group = handle_key_group(self.key_group, self.cache) + cache_key = get_cache_key(request, key_group, self.key_prefix, "GET", cache=self.cache) if cache_key is None: request._cache_update_cache = True @@ -184,6 +187,18 @@ def process_request(self, request): return response +def handle_key_group(key_group, cache): + group_cache_header = "decorators.group_caching" + group_key = f"{group_cache_header}.{key_group}" + result = cache.get(group_key) + if not result: + hash = md5(f"{group_key}.{uuid.uuid4()}".encode("ascii"), usedforsecurity=False).hexdigest() + cache.set(group_key, hash) + result = hash + + return result + + class CacheMiddleware(UpdateCacheMiddleware, FetchFromCacheMiddleware): """ Cache middleware that provides basic behavior for many simple sites. diff --git a/django/utils/cache.py b/django/utils/cache.py index 04e1a54ed159..1b74fdf7d3cc 100644 --- a/django/utils/cache.py +++ b/django/utils/cache.py @@ -464,3 +464,10 @@ def _to_tuple(s): if len(t) == 2: return t[0].lower(), t[1] return t[0].lower(), True + + +def invalidate_key_group(key_group, cache): + group_cache_header = "decorators.group_caching" + group_key = f"{group_cache_header}.{key_group}" + cache.delete(group_key) + diff --git a/tests/cache/tests.py b/tests/cache/tests.py index 4a78253c10d2..ee854bc50844 100644 --- a/tests/cache/tests.py +++ b/tests/cache/tests.py @@ -56,6 +56,7 @@ from django.utils import timezone, translation from django.utils.cache import ( get_cache_key, + invalidate_key_group, learn_cache_key, patch_cache_control, patch_vary_headers, @@ -2579,13 +2580,8 @@ def test_middleware(self): self.assertEqual(result.content, b"Hello World 1") def test_middleware_group(self): - def get_group2(*args, **kwargs): - return "group2" - - middleware_group1 = CacheMiddleware(hello_world_view, key_group="group1") - middleware_group2_callable = CacheMiddleware( - hello_world_view, key_group=get_group2 - ) + group_name = "group1" + middleware_group1 = CacheMiddleware(hello_world_view, key_group=group_name) request = self.factory.get("/view/") @@ -2602,16 +2598,12 @@ def get_group2(*args, **kwargs): result = middleware_group1.process_request(request) self.assertIsNotNone(result) - # Put the request through the request middleware - result = middleware_group2_callable.process_request(request) - self.assertIsNone(result) # Is None because there's no cache - - # Now put the response through the response middleware - response = middleware_group2_callable.process_response(request, response) + # Invalidating the whole group + invalidate_key_group(group_name, middleware_group1.cache) - # Repeating the request should result in a cache hit - result = middleware_group2_callable.process_request(request) - self.assertIsNotNone(result) + # Repeating the request should result in a cache non hit + result = middleware_group1.process_request(request) + self.assertIsNone(result) def test_view_decorator(self): # decorate the same view with different cache decorators From 09e86490a7bff76a6f733ecf1bea4fa7b4eeeaa0 Mon Sep 17 00:00:00 2001 From: msilva Date: Fri, 2 Jun 2023 13:58:14 +0100 Subject: [PATCH 7/8] delete unused imports --- django/middleware/cache.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/django/middleware/cache.py b/django/middleware/cache.py index 14c0ada363cc..e11bb0c63619 100644 --- a/django/middleware/cache.py +++ b/django/middleware/cache.py @@ -43,11 +43,9 @@ """ -from functools import cache from hashlib import md5 import uuid from django.conf import settings -from django.utils import timezone from django.core.cache import DEFAULT_CACHE_ALIAS, caches from django.utils.cache import ( get_cache_key, From 1896533e680fdb44118c84ac32badb755a9aa10e Mon Sep 17 00:00:00 2001 From: Ahter Sonmez Date: Fri, 2 Jun 2023 13:30:31 +0100 Subject: [PATCH 8/8] Add a simple test to check cache invalidation This uses the key group to invalidate the cache that is stored for the view. --- django/utils/cache.py | 12 ++++++++++++ tests/cache/tests.py | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/django/utils/cache.py b/django/utils/cache.py index 04e1a54ed159..8184f0f60c71 100644 --- a/django/utils/cache.py +++ b/django/utils/cache.py @@ -464,3 +464,15 @@ def _to_tuple(s): if len(t) == 2: return t[0].lower(), t[1] return t[0].lower(), True + + +def invalidate_cache_by_key_group(key_group: str, cache=None) -> None: + """ + Invalidates all cache entries for a given key group. + """ + if cache is None: + cache = caches[settings.CACHE_MIDDLEWARE_ALIAS] + + # Get the group cache key and delete it. + group_cache_key = f"{PATH_PREFIX}.cache_group.{key_group}" + cache.delete(group_cache_key) diff --git a/tests/cache/tests.py b/tests/cache/tests.py index 12a37b2945f6..0d7025642ecb 100644 --- a/tests/cache/tests.py +++ b/tests/cache/tests.py @@ -56,6 +56,7 @@ from django.utils import timezone, translation from django.utils.cache import ( get_cache_key, + invalidate_cache_by_key_group, learn_cache_key, patch_cache_control, patch_vary_headers, @@ -2583,7 +2584,9 @@ def get_group2(*args, **kwargs): return "group2" middleware_group1 = CacheMiddleware(hello_world_view, key_group="group1") - middleware_group2_callable = CacheMiddleware(hello_world_view, key_group=get_group2) + middleware_group2_callable = CacheMiddleware( + hello_world_view, key_group=get_group2 + ) request = self.factory.get("/view/") @@ -2611,7 +2614,6 @@ def get_group2(*args, **kwargs): result = middleware_group2_callable.process_request(request) self.assertIsNotNone(result) - def test_view_decorator(self): # decorate the same view with different cache decorators default_view = cache_page(3)(hello_world_view) @@ -2972,3 +2974,32 @@ def test_all(self): # .all() initializes all caches. self.assertEqual(len(test_caches.all(initialized_only=True)), 2) self.assertEqual(test_caches.all(), test_caches.all(initialized_only=True)) + + +class TestCacheInvalidation(SimpleTestCase): + def test_view_cache_invalidation(self): + """ + Test that the view cache is correctly invalidated. + """ + # Set up the view. Decorate it with the cache_page decorator. + decorated_view = cache_page(timeout=3, key_group="test-key-group")( + hello_world_view + ) + + # Set up the request. + request = self.factory.get("/view/") + + # Request the view for the first time. + response = decorated_view(request, "1") + self.assertEqual(response.content, b"Hello World 1") + + # Request again -- hit the cache. + response = decorated_view(request, "2") + self.assertEqual(response.content, b"Hello World 1") + + # Run the cache invalidation. + invalidate_cache_by_key_group(key_group="test-key-group") + + # Request again -- should NOT hit the cache. + response = decorated_view(request, "3") + self.assertEqual(response.content, b"Hello World 3")