From f6c991667f6f1ab8d2fd2afc1ee2b5801e78b7e3 Mon Sep 17 00:00:00 2001 From: Jannis Leidel Date: Wed, 2 Mar 2011 12:47:36 +0000 Subject: [PATCH] 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 | 4 ++- django/middleware/cache.py | 4 +-- django/utils/cache.py | 14 +++++----- docs/releases/1.3.txt | 3 +++ docs/topics/cache.txt | 8 ++++-- tests/regressiontests/cache/tests.py | 39 +++++++++++++++++++++++++--- 6 files changed, 57 insertions(+), 15 deletions(-) diff --git a/django/http/__init__.py b/django/http/__init__.py index 74a5afa84a..f9e1494e26 100644 --- a/django/http/__init__.py +++ b/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): """ diff --git a/django/middleware/cache.py b/django/middleware/cache.py index 6302a172c7..34bf0ca4a4 100644 --- a/django/middleware/cache.py +++ b/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. diff --git a/django/utils/cache.py b/django/utils/cache.py index 4b3b5af04c..63c4af3494 100644 --- a/django/utils/cache.py +++ b/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 - request phase because it pulls the list of headers to take into account - from the global path registry and uses those to build a cache key to check - against. + Returns a cache key based on the request path and query. It can be used + in the request phase because it pulls the list of headers to take into + account from the global path registry and uses those to build a cache key + 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) diff --git a/docs/releases/1.3.txt b/docs/releases/1.3.txt index 29e4fd46f5..66f78ba0c4 100644 --- a/docs/releases/1.3.txt +++ b/docs/releases/1.3.txt @@ -169,6 +169,9 @@ Secondly, :ref:`Versioning `, :ref:`site-wide prefixing ` and :ref:`transformation ` has been added to the cache API. +Thirdly, the :ref:`cache key creation ` has been +updated to take the GET request query string into account. + Lastly, support for pylibmc_ has been added to the memcached cache backend. diff --git a/docs/topics/cache.txt b/docs/topics/cache.txt index 5acd53eeb8..4eb9d21536 100644 --- a/docs/topics/cache.txt +++ b/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 -to that URL will use the same cached version, regardless of user-agent +path and query -- e.g., ``"/stories/2005/?order_by=author"``. This means every +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`` diff --git a/tests/regressiontests/cache/tests.py b/tests/regressiontests/cache/tests.py index b31f9a4c55..4935bd0e57 100644 --- a/tests/regressiontests/cache/tests.py +++ b/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!"