Fixed #4992 -- Respect the GET request query string when creating cache keys. Thanks PeterKz and guettli for the initial patch.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@15705 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/http/__init__.py

@@ -166,7 +166,9 @@ class HttpRequest(object):
         return host
 
     def get_full_path(self):
-        return ''
+        # RFC 3986 requires query string arguments to be in the ASCII range.
+        # Rather than crash if this doesn't happen, we encode defensively.
+        return '%s%s' % (self.path, self.META.get('QUERY_STRING', '') and ('?' + iri_to_uri(self.META.get('QUERY_STRING', ''))) or '')
 
     def build_absolute_uri(self, location=None):
         """

django/middleware/cache.py

@@ -23,7 +23,7 @@ Django's ``LocaleMiddleware``.
 
 More details about how the caching works:
 
-* Only parameter-less GET or HEAD-requests with status code 200 are cached.
+* Only GET or HEAD-requests with status code 200 are cached.
 
 * The number of seconds each page is stored for is set by the "max-age" section
   of the response's "Cache-Control" header, falling back to the
@@ -135,7 +135,7 @@ class FetchFromCacheMiddleware(object):
         Checks whether the page is already cached and returns the cached
         version if available.
         """
-        if not request.method in ('GET', 'HEAD') or request.GET:
+        if not request.method in ('GET', 'HEAD'):
             request._cache_update_cache = False
             return None # Don't bother checking the cache.
 

django/utils/cache.py

@@ -160,24 +160,24 @@ def _generate_cache_key(request, method, headerlist, key_prefix):
         value = request.META.get(header, None)
         if value is not None:
             ctx.update(value)
-    path = md5_constructor(iri_to_uri(request.path))
+    path = md5_constructor(iri_to_uri(request.get_full_path()))
     cache_key = 'views.decorators.cache.cache_page.%s.%s.%s.%s' % (
         key_prefix, request.method, path.hexdigest(), ctx.hexdigest())
     return _i18n_cache_key_suffix(request, cache_key)
 
 def _generate_cache_header_key(key_prefix, request):
     """Returns a cache key for the header cache."""
-    path = md5_constructor(iri_to_uri(request.path))
+    path = md5_constructor(iri_to_uri(request.get_full_path()))
     cache_key = 'views.decorators.cache.cache_header.%s.%s' % (
         key_prefix, path.hexdigest())
     return _i18n_cache_key_suffix(request, cache_key)
 
 def get_cache_key(request, key_prefix=None, method='GET', cache=None):
     """
-    Returns a cache key based on the request path. It can be used in the
+    Returns a cache key based on the request path and query. It can be used
-    request phase because it pulls the list of headers to take into account
+    in the request phase because it pulls the list of headers to take into
-    from the global path registry and uses those to build a cache key to check
+    account from the global path registry and uses those to build a cache key
-    against.
+    to check against.
 
     If there is no headerlist stored, the page needs to be rebuilt, so this
     function returns None.
@@ -220,7 +220,7 @@ def learn_cache_key(request, response, cache_timeout=None, key_prefix=None, cach
         return _generate_cache_key(request, request.method, headerlist, key_prefix)
     else:
         # if there is no Vary header, we still need a cache key
-        # for the request.path
+        # for the request.get_full_path()
         cache.set(cache_key, [], cache_timeout)
         return _generate_cache_key(request, request.method, [], key_prefix)
 

docs/releases/1.3.txt

@@ -169,6 +169,9 @@ Secondly, :ref:`Versioning <cache_versioning>`, :ref:`site-wide
 prefixing <cache_key_prefixing>` and :ref:`transformation
 <cache_key_transformation>` has been added to the cache API.
 
+Thirdly, the :ref:`cache key creation <using-vary-headers>` has been
+updated to take the GET request query string into account.
+
 Lastly, support for pylibmc_ has been added to the memcached cache
 backend.
 

docs/topics/cache.txt

@@ -962,9 +962,13 @@ mechanism should take into account when building its cache key. For example, if
 the contents of a Web page depend on a user's language preference, the page is
 said to "vary on language."
 
+.. versionchanged:: 1.3
+    In Django 1.3 the full request path -- including the query -- is used
+    to create the cache keys, instead of only the path component in Django 1.2.
+
 By default, Django's cache system creates its cache keys using the requested
-path (e.g., ``"/stories/2005/jun/23/bank_robbed/"``). This means every request
+path and query -- e.g., ``"/stories/2005/?order_by=author"``. This means every
-to that URL will use the same cached version, regardless of user-agent
+request to that URL will use the same cached version, regardless of user-agent
 differences such as cookies or language preferences. However, if this page
 produces different content based on some difference in request headers -- such
 as a cookie, or a language, or a user-agent -- you'll need to use the ``Vary``

tests/regressiontests/cache/tests.py

@@ -12,7 +12,7 @@ from django.conf import settings
 from django.core import management
 from django.core.cache import get_cache, DEFAULT_CACHE_ALIAS
 from django.core.cache.backends.base import CacheKeyWarning
-from django.http import HttpResponse, HttpRequest
+from django.http import HttpResponse, HttpRequest, QueryDict
 from django.middleware.cache import FetchFromCacheMiddleware, UpdateCacheMiddleware, CacheMiddleware
 from django.test import RequestFactory
 from django.test.utils import get_warnings_state, restore_warnings_state
@@ -920,10 +920,20 @@ class CacheUtils(unittest.TestCase):
         # Set headers to an empty list.
         learn_cache_key(request, response)
         self.assertEqual(get_cache_key(request), 'views.decorators.cache.cache_page.settingsprefix.GET.a8c87a3d8c44853d7f79474f7ffe4ad5.d41d8cd98f00b204e9800998ecf8427e')
-        # Verify that a specified key_prefix is taken in to account.
+        # Verify that a specified key_prefix is taken into account.
         learn_cache_key(request, response, key_prefix=key_prefix)
         self.assertEqual(get_cache_key(request, key_prefix=key_prefix), 'views.decorators.cache.cache_page.localprefix.GET.a8c87a3d8c44853d7f79474f7ffe4ad5.d41d8cd98f00b204e9800998ecf8427e')
 
+    def test_get_cache_key_with_query(self):
+        request = self._get_request(self.path + '?test=1')
+        response = HttpResponse()
+        # Expect None if no headers have been set yet.
+        self.assertEqual(get_cache_key(request), None)
+        # Set headers to an empty list.
+        learn_cache_key(request, response)
+        # Verify that the querystring is taken into account.
+        self.assertEqual(get_cache_key(request), 'views.decorators.cache.cache_page.settingsprefix.GET.bd889c5a59603af44333ed21504db3cd.d41d8cd98f00b204e9800998ecf8427e')
+
     def test_learn_cache_key(self):
         request = self._get_request(self.path, 'HEAD')
         response = HttpResponse()
@@ -1039,12 +1049,15 @@ class CacheI18nTest(unittest.TestCase):
         request.path = request.path_info = self.path
         return request
 
-    def _get_request_cache(self):
+    def _get_request_cache(self, query_string=None):
         request = HttpRequest()
         request.META = {
             'SERVER_NAME': 'testserver',
             'SERVER_PORT': 80,
         }
+        if query_string:
+            request.META['QUERY_STRING'] = query_string
+            request.GET = QueryDict(query_string)
         request.path = request.path_info = self.path
         request._cache_update_cache = True
         request.method = 'GET'
@@ -1085,6 +1098,26 @@ class CacheI18nTest(unittest.TestCase):
         }
         settings.USE_ETAGS = True
         settings.USE_I18N = True
+
+        # cache with non empty request.GET
+        request = self._get_request_cache(query_string='foo=bar&other=true')
+        get_cache_data = FetchFromCacheMiddleware().process_request(request)
+        # first access, cache must return None
+        self.assertEqual(get_cache_data, None)
+        response = HttpResponse()
+        content = 'Check for cache with QUERY_STRING'
+        response.content = content
+        UpdateCacheMiddleware().process_response(request, response)
+        get_cache_data = FetchFromCacheMiddleware().process_request(request)
+        # cache must return content
+        self.assertNotEqual(get_cache_data, None)
+        self.assertEqual(get_cache_data.content, content)
+        # different QUERY_STRING, cache must be empty
+        request = self._get_request_cache(query_string='foo=bar&somethingelse=true')
+        get_cache_data = FetchFromCacheMiddleware().process_request(request)
+        self.assertEqual(get_cache_data, None)
+
+        # i18n tests
         en_message ="Hello world!"
         es_message ="Hola mundo!"