Browse Source

Fixed #30765 -- Made cache_page decorator take precedence over max-age Cache-Control directive.

Flavio Curella 5 years ago
parent
commit
d08d4f464a

+ 14 - 9
django/middleware/cache.py

@@ -63,6 +63,7 @@ class UpdateCacheMiddleware(MiddlewareMixin):
     """
     """
     def __init__(self, get_response=None):
     def __init__(self, get_response=None):
         self.cache_timeout = settings.CACHE_MIDDLEWARE_SECONDS
         self.cache_timeout = settings.CACHE_MIDDLEWARE_SECONDS
+        self.page_timeout = None
         self.key_prefix = settings.CACHE_MIDDLEWARE_KEY_PREFIX
         self.key_prefix = settings.CACHE_MIDDLEWARE_KEY_PREFIX
         self.cache_alias = settings.CACHE_MIDDLEWARE_ALIAS
         self.cache_alias = settings.CACHE_MIDDLEWARE_ALIAS
         self.cache = caches[self.cache_alias]
         self.cache = caches[self.cache_alias]
@@ -89,15 +90,18 @@ class UpdateCacheMiddleware(MiddlewareMixin):
         if 'private' in response.get('Cache-Control', ()):
         if 'private' in response.get('Cache-Control', ()):
             return response
             return response
 
 
-        # Try to get the timeout from the "max-age" section of the "Cache-
-        # Control" header before reverting to using the default cache_timeout
-        # length.
-        timeout = get_max_age(response)
+        # Page timeout takes precedence over the "max-age" and the default
+        # cache timeout.
+        timeout = self.page_timeout
         if timeout is None:
         if timeout is None:
-            timeout = self.cache_timeout
-        elif timeout == 0:
-            # max-age was set to 0, don't bother caching.
-            return response
+            # The timeout from the "max-age" section of the "Cache-Control"
+            # header takes precedence over the default cache timeout.
+            timeout = get_max_age(response)
+            if timeout is None:
+                timeout = self.cache_timeout
+            elif timeout == 0:
+                # max-age was set to 0, don't cache.
+                return response
         patch_response_headers(response, timeout)
         patch_response_headers(response, timeout)
         if timeout and response.status_code == 200:
         if timeout and response.status_code == 200:
             cache_key = learn_cache_key(request, response, timeout, self.key_prefix, cache=self.cache)
             cache_key = learn_cache_key(request, response, timeout, self.key_prefix, cache=self.cache)
@@ -160,7 +164,7 @@ class CacheMiddleware(UpdateCacheMiddleware, FetchFromCacheMiddleware):
     Also used as the hook point for the cache decorator, which is generated
     Also used as the hook point for the cache decorator, which is generated
     using the decorator-from-middleware utility.
     using the decorator-from-middleware utility.
     """
     """
-    def __init__(self, get_response=None, cache_timeout=None, **kwargs):
+    def __init__(self, get_response=None, cache_timeout=None, page_timeout=None, **kwargs):
         self.get_response = get_response
         self.get_response = get_response
         # We need to differentiate between "provided, but using default value",
         # We need to differentiate between "provided, but using default value",
         # and "not provided". If the value is provided using a default, then
         # and "not provided". If the value is provided using a default, then
@@ -186,4 +190,5 @@ class CacheMiddleware(UpdateCacheMiddleware, FetchFromCacheMiddleware):
         if cache_timeout is None:
         if cache_timeout is None:
             cache_timeout = settings.CACHE_MIDDLEWARE_SECONDS
             cache_timeout = settings.CACHE_MIDDLEWARE_SECONDS
         self.cache_timeout = cache_timeout
         self.cache_timeout = cache_timeout
+        self.page_timeout = page_timeout
         self.cache = caches[self.cache_alias]
         self.cache = caches[self.cache_alias]

+ 1 - 1
django/views/decorators/cache.py

@@ -20,7 +20,7 @@ def cache_page(timeout, *, cache=None, key_prefix=None):
     into account on caching -- just like the middleware does.
     into account on caching -- just like the middleware does.
     """
     """
     return decorator_from_middleware_with_args(CacheMiddleware)(
     return decorator_from_middleware_with_args(CacheMiddleware)(
-        cache_timeout=timeout, cache_alias=cache, key_prefix=key_prefix
+        page_timeout=timeout, cache_alias=cache, key_prefix=key_prefix,
     )
     )
 
 
 
 

+ 4 - 0
docs/releases/3.1.txt

@@ -430,6 +430,10 @@ Miscellaneous
   used with :setting:`DEFAULT_EXCEPTION_REPORTER_FILTER` needs to inherit from
   used with :setting:`DEFAULT_EXCEPTION_REPORTER_FILTER` needs to inherit from
   :class:`django.views.debug.SafeExceptionReporterFilter`.
   :class:`django.views.debug.SafeExceptionReporterFilter`.
 
 
+* The cache timeout set by :func:`~django.views.decorators.cache.cache_page`
+  decorator now takes precedence over the ``max-age`` directive from the
+  ``Cache-Control`` header.
+
 .. _deprecated-features-3.1:
 .. _deprecated-features-3.1:
 
 
 Features deprecated in 3.1
 Features deprecated in 3.1

+ 8 - 0
docs/topics/cache.txt

@@ -570,6 +570,9 @@ minutes. (Note that we've written it as ``60 * 15`` for the purpose of
 readability. ``60 * 15`` will be evaluated to ``900`` -- that is, 15 minutes
 readability. ``60 * 15`` will be evaluated to ``900`` -- that is, 15 minutes
 multiplied by 60 seconds per minute.)
 multiplied by 60 seconds per minute.)
 
 
+The cache timeout set by ``cache_page`` takes precedence over the ``max-age``
+directive from the ``Cache-Control`` header.
+
 The per-view cache, like the per-site cache, is keyed off of the URL. If
 The per-view cache, like the per-site cache, is keyed off of the URL. If
 multiple URLs point at the same view, each URL will be cached separately.
 multiple URLs point at the same view, each URL will be cached separately.
 Continuing the ``my_view`` example, if your URLconf looks like this::
 Continuing the ``my_view`` example, if your URLconf looks like this::
@@ -605,6 +608,11 @@ The ``key_prefix`` and ``cache`` arguments may be specified together. The
 ``key_prefix`` argument and the :setting:`KEY_PREFIX <CACHES-KEY_PREFIX>`
 ``key_prefix`` argument and the :setting:`KEY_PREFIX <CACHES-KEY_PREFIX>`
 specified under :setting:`CACHES` will be concatenated.
 specified under :setting:`CACHES` will be concatenated.
 
 
+.. versionchanged:: 3.1
+
+    In older versions, the ``max-age`` directive from the ``Cache-Control``
+    header had precedence over the cache timeout set by ``cache_page``.
+
 Specifying per-view cache in the URLconf
 Specifying per-view cache in the URLconf
 ----------------------------------------
 ----------------------------------------
 
 

+ 23 - 0
tests/cache/tests.py

@@ -2188,6 +2188,29 @@ class CacheMiddlewareTest(SimpleTestCase):
         response = other_with_prefix_view(request, '16')
         response = other_with_prefix_view(request, '16')
         self.assertEqual(response.content, b'Hello World 16')
         self.assertEqual(response.content, b'Hello World 16')
 
 
+    def test_cache_page_timeout(self):
+        # Page timeout takes precedence over the "max-age" section of the
+        # "Cache-Control".
+        tests = [
+            (1, 3),  # max_age < page_timeout.
+            (3, 1),  # max_age > page_timeout.
+        ]
+        for max_age, page_timeout in tests:
+            with self.subTest(max_age=max_age, page_timeout=page_timeout):
+                view = cache_page(timeout=page_timeout)(
+                    cache_control(max_age=max_age)(hello_world_view)
+                )
+                request = self.factory.get('/view/')
+                response = view(request, '1')
+                self.assertEqual(response.content, b'Hello World 1')
+                time.sleep(1)
+                response = view(request, '2')
+                self.assertEqual(
+                    response.content,
+                    b'Hello World 1' if page_timeout > max_age else b'Hello World 2',
+                )
+            cache.clear()
+
     def test_cached_control_private_not_cached(self):
     def test_cached_control_private_not_cached(self):
         """Responses with 'Cache-Control: private' are not cached."""
         """Responses with 'Cache-Control: private' are not cached."""
         view_with_private_cache = cache_page(3)(cache_control(private=True)(hello_world_view))
         view_with_private_cache = cache_page(3)(cache_control(private=True)(hello_world_view))