From a840710e1e38bc9e55412bb36eca92eff94ebd2c Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Sun, 3 Apr 2016 12:15:10 +0200 Subject: [PATCH] Fixed #26447 -- Deprecated settings.USE_ETAGS in favor of ConditionalGetMiddleware. --- django/conf/global_settings.py | 2 ++ django/middleware/common.py | 13 ++++++-- django/middleware/http.py | 17 ++++++++-- django/utils/cache.py | 9 ++++++ docs/internals/deprecation.txt | 3 ++ docs/ref/middleware.txt | 13 +++++++- docs/ref/settings.txt | 5 +++ docs/ref/utils.txt | 5 +++ docs/releases/1.11.txt | 9 ++++++ docs/topics/conditional-view-processing.txt | 19 +++++------ docs/topics/i18n/translation.txt | 4 +-- docs/topics/performance.txt | 3 +- tests/admin_views/tests.py | 6 ++-- tests/cache/tests.py | 6 ++-- tests/middleware/tests.py | 36 ++++++++++++++++++++- 15 files changed, 126 insertions(+), 24 deletions(-) diff --git a/django/conf/global_settings.py b/django/conf/global_settings.py index 9dc4cdbade..9dbbdc9f88 100644 --- a/django/conf/global_settings.py +++ b/django/conf/global_settings.py @@ -23,6 +23,8 @@ DEBUG = False DEBUG_PROPAGATE_EXCEPTIONS = False # Whether to use the "ETag" header. This saves bandwidth but slows down performance. +# Deprecated (RemovedInDjango21Warning) in favor of ConditionalGetMiddleware +# which sets the ETag regardless of this setting. USE_ETAGS = False # People who get code error notifications. diff --git a/django/middleware/common.py b/django/middleware/common.py index dc89bcb9fa..820609e3a5 100644 --- a/django/middleware/common.py +++ b/django/middleware/common.py @@ -1,4 +1,5 @@ import re +import warnings from django import http from django.conf import settings @@ -8,7 +9,7 @@ from django.urls import is_valid_path from django.utils.cache import ( cc_delim_re, get_conditional_response, set_response_etag, ) -from django.utils.deprecation import MiddlewareMixin +from django.utils.deprecation import MiddlewareMixin, RemovedInDjango21Warning from django.utils.encoding import force_text from django.utils.six.moves.urllib.parse import urlparse @@ -34,7 +35,8 @@ class CommonMiddleware(MiddlewareMixin): - ETags: If the USE_ETAGS setting is set, ETags will be calculated from the entire page content and Not Modified responses will be returned - appropriately. + appropriately. USE_ETAGS is deprecated in favor of + ConditionalGetMiddleware. """ response_redirect_class = http.HttpResponsePermanentRedirect @@ -115,6 +117,13 @@ class CommonMiddleware(MiddlewareMixin): return self.response_redirect_class(self.get_full_path_with_slash(request)) if settings.USE_ETAGS and self.needs_etag(response): + warnings.warn( + "The USE_ETAGS setting is deprecated in favor of " + "ConditionalGetMiddleware which sets the ETag regardless of " + "the setting. CommonMiddleware won't do ETag processing in " + "Django 2.1.", + RemovedInDjango21Warning + ) if not response.has_header('ETag'): set_response_etag(response) diff --git a/django/middleware/http.py b/django/middleware/http.py index 83c0f866e4..2f9186f82c 100644 --- a/django/middleware/http.py +++ b/django/middleware/http.py @@ -1,4 +1,6 @@ -from django.utils.cache import get_conditional_response +from django.utils.cache import ( + cc_delim_re, get_conditional_response, set_response_etag, +) from django.utils.deprecation import MiddlewareMixin from django.utils.http import http_date, parse_http_date_safe @@ -7,7 +9,8 @@ class ConditionalGetMiddleware(MiddlewareMixin): """ Handles conditional GET operations. If the response has an ETag or Last-Modified header, and the request has If-None-Match or - If-Modified-Since, the response is replaced by an HttpNotModified. + If-Modified-Since, the response is replaced by an HttpNotModified. An ETag + header is added if needed. Also sets the Date and Content-Length response-headers. """ @@ -16,6 +19,9 @@ class ConditionalGetMiddleware(MiddlewareMixin): if not response.streaming and not response.has_header('Content-Length'): response['Content-Length'] = str(len(response.content)) + if self.needs_etag(response) and not response.has_header('ETag'): + set_response_etag(response) + etag = response.get('ETag') last_modified = response.get('Last-Modified') if last_modified: @@ -30,3 +36,10 @@ class ConditionalGetMiddleware(MiddlewareMixin): ) return response + + def needs_etag(self, response): + """ + Return True if an ETag header should be added to response. + """ + cache_control_headers = cc_delim_re.split(response.get('Cache-Control', '')) + return all(header.lower() != 'no-store' for header in cache_control_headers) diff --git a/django/utils/cache.py b/django/utils/cache.py index e74bfbea5d..8d0ce4dac5 100644 --- a/django/utils/cache.py +++ b/django/utils/cache.py @@ -22,10 +22,12 @@ import hashlib import logging import re import time +import warnings from django.conf import settings from django.core.cache import caches from django.http import HttpResponse, HttpResponseNotModified +from django.utils.deprecation import RemovedInDjango21Warning from django.utils.encoding import force_bytes, force_text, iri_to_uri from django.utils.http import ( http_date, parse_etags, parse_http_date_safe, quote_etag, @@ -242,6 +244,13 @@ def patch_response_headers(response, cache_timeout=None): if cache_timeout < 0: cache_timeout = 0 # Can't have max-age negative if settings.USE_ETAGS and not response.has_header('ETag'): + warnings.warn( + "The USE_ETAGS setting is deprecated in favor of " + "ConditionalGetMiddleware which sets the ETag regardless of the " + "setting. patch_response_headers() won't do ETag processing in " + "Django 2.1.", + RemovedInDjango21Warning + ) if hasattr(response, 'render') and callable(response.render): response.add_post_render_callback(set_response_etag) else: diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index a7ef1b8be5..2d247d8f65 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -43,6 +43,9 @@ details on these changes. * The ``django.db.models.permalink()`` decorator will be removed. +* The ``USE_ETAGS`` setting will be removed. ``CommonMiddleware`` and + ``django.utils.cache.patch_response_headers()`` will no longer set ETags. + .. _deprecation-removed-in-2.0: 2.0 diff --git a/docs/ref/middleware.txt b/docs/ref/middleware.txt index 5d8939e3ef..432f5437ff 100644 --- a/docs/ref/middleware.txt +++ b/docs/ref/middleware.txt @@ -72,6 +72,12 @@ Adds a few conveniences for perfectionists: Older versions didn't set the ``Content-Length`` header. +.. deprecated:: 1.11 + + The :setting:`USE_ETAGS` setting is deprecated in favor of using + :class:`~django.middleware.http.ConditionalGetMiddleware` for ETag + processing. + .. attribute:: CommonMiddleware.response_redirect_class Defaults to :class:`~django.http.HttpResponsePermanentRedirect`. Subclass @@ -166,13 +172,18 @@ Conditional GET middleware .. class:: ConditionalGetMiddleware -Handles conditional GET operations. If the response has a ``ETag`` or +Handles conditional GET operations. If the response doesn't have an ``ETag`` +header, the middleware adds one if needed. If the response has a ``ETag`` or ``Last-Modified`` header, and the request has ``If-None-Match`` or ``If-Modified-Since``, the response is replaced by an :class:`~django.http.HttpResponseNotModified`. Also sets the ``Date`` and ``Content-Length`` response-headers. +.. versionchanged:: 1.11 + + In older versions, the middleware didn't set the ``ETag`` header. + Locale middleware ----------------- diff --git a/docs/ref/settings.txt b/docs/ref/settings.txt index 05faa734b2..28240034e5 100644 --- a/docs/ref/settings.txt +++ b/docs/ref/settings.txt @@ -2532,6 +2532,11 @@ bandwidth but slows down performance. This is used by the :class:`~django.middleware.common.CommonMiddleware` and in the :doc:`cache framework `. +.. deprecated:: 1.11 + + This setting is deprecated in favor of using ``ConditionalGetMiddleware``, + which sets an ETag regardless of this setting. + .. setting:: USE_I18N ``USE_I18N`` diff --git a/docs/ref/utils.txt b/docs/ref/utils.txt index a9072f9442..45ffd80fc4 100644 --- a/docs/ref/utils.txt +++ b/docs/ref/utils.txt @@ -65,6 +65,11 @@ need to distinguish caches by the ``Accept-language`` header. In older versions, the ``Last-Modified`` header was also set. + .. deprecated:: 1.11 + + Since the ``USE_ETAGS`` setting is deprecated, this function won't set + the ``ETag`` header when the deprecation ends in Django 2.1. + .. function:: add_never_cache_headers(response) Adds a ``Cache-Control: max-age=0, no-cache, no-store, must-revalidate`` diff --git a/docs/releases/1.11.txt b/docs/releases/1.11.txt index ff77ebeabb..3be654eff5 100644 --- a/docs/releases/1.11.txt +++ b/docs/releases/1.11.txt @@ -327,6 +327,9 @@ Requests and Responses * Added the :setting:`SECURE_HSTS_PRELOAD` setting to allow appending the ``preload`` directive to the ``Strict-Transport-Security`` header. +* :class:`~django.middleware.http.ConditionalGetMiddleware` now adds the + ``ETag`` header to responses. + Serialization ~~~~~~~~~~~~~ @@ -633,3 +636,9 @@ Miscellaneous * :func:`~django.contrib.auth.authenticate` now passes a ``request`` argument to the ``authenticate()`` method of authentication backends. Support for methods that don't accept ``request`` will be removed in Django 2.1. + +* The ``USE_ETAGS`` setting is deprecated in favor of + :class:`~django.middleware.http.ConditionalGetMiddleware` which now adds the + ``ETag`` header to responses regardless of the setting. ``CommonMiddleware`` + and ``django.utils.cache.patch_response_headers()`` will no longer set ETags + when the deprecation ends. diff --git a/docs/topics/conditional-view-processing.txt b/docs/topics/conditional-view-processing.txt index 7398b3fd05..7f1fde0fb9 100644 --- a/docs/topics/conditional-view-processing.txt +++ b/docs/topics/conditional-view-processing.txt @@ -11,7 +11,7 @@ used for all HTTP methods (``POST``, ``PUT``, ``DELETE``, etc.). For each page (response) that Django sends back from a view, it might provide two HTTP headers: the ``ETag`` header and the ``Last-Modified`` header. These headers are optional on HTTP responses. They can be set by your view function, -or you can rely on the :class:`~django.middleware.common.CommonMiddleware` +or you can rely on the :class:`~django.middleware.http.ConditionalGetMiddleware` middleware to set the ``ETag`` header. When the client next requests the same resource, it might send along a header @@ -189,17 +189,14 @@ every time. Comparison with middleware conditional processing ================================================= -You may notice that Django already provides simple and straightforward -conditional ``GET`` handling via the -:class:`django.middleware.http.ConditionalGetMiddleware` and -:class:`~django.middleware.common.CommonMiddleware`. While certainly being -easy to use and suitable for many situations, those pieces of middleware -functionality have limitations for advanced usage: +Django provides simple and straightforward conditional ``GET`` handling via +:class:`django.middleware.http.ConditionalGetMiddleware`. While being easy to +use and suitable for many situations, the middleware has limitations for +advanced usage: -* They are applied globally to all views in your project -* They don't save you from generating the response itself, which may be - expensive -* They are only appropriate for HTTP ``GET`` requests. +* It's applied globally to all views in your project. +* It doesn't save you from generating the response, which may be expensive. +* It's only appropriate for HTTP ``GET`` requests. You should choose the most appropriate tool for your particular problem here. If you have a way to compute ETags and modification times quickly and if some diff --git a/docs/topics/i18n/translation.txt b/docs/topics/i18n/translation.txt index 4ef0ae45a3..db891f75d7 100644 --- a/docs/topics/i18n/translation.txt +++ b/docs/topics/i18n/translation.txt @@ -1372,8 +1372,8 @@ URL:: ] Client-side caching will save bandwidth and make your site load faster. If -you're using ETags (:setting:`USE_ETAGS = True `), you're already -covered. Otherwise, you can apply :ref:`conditional decorators +you're using ETags (:class:`~django.middleware.http.ConditionalGetMiddleware`), +you're already covered. Otherwise, you can apply :ref:`conditional decorators `. In the following example, the cache is invalidated whenever you restart your application server:: diff --git a/docs/topics/performance.txt b/docs/topics/performance.txt index c8c9d231af..8f2516a4eb 100644 --- a/docs/topics/performance.txt +++ b/docs/topics/performance.txt @@ -262,7 +262,8 @@ that can help optimize your site's performance. They include: ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Adds support for modern browsers to conditionally GET responses based on the -``ETag`` and ``Last-Modified`` headers. +``ETag`` and ``Last-Modified`` headers. It also calculates and sets an ETag if +needed. :class:`~django.middleware.gzip.GZipMiddleware` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index ba15e18a2c..3007764b5d 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -35,7 +35,9 @@ from django.urls import NoReverseMatch, resolve, reverse from django.utils import formats, six, translation from django.utils._os import upath from django.utils.cache import get_max_age -from django.utils.deprecation import RemovedInDjango20Warning +from django.utils.deprecation import ( + RemovedInDjango20Warning, RemovedInDjango21Warning, +) from django.utils.encoding import force_bytes, force_text, iri_to_uri from django.utils.html import escape from django.utils.http import urlencode @@ -6074,7 +6076,7 @@ class TestETagWithAdminView(SimpleTestCase): self.assertEqual(response.status_code, 302) self.assertFalse(response.has_header('ETag')) - with self.settings(USE_ETAGS=True): + with self.settings(USE_ETAGS=True), ignore_warnings(category=RemovedInDjango21Warning): response = self.client.get(reverse('admin:index')) self.assertEqual(response.status_code, 302) self.assertTrue(response.has_header('ETag')) diff --git a/tests/cache/tests.py b/tests/cache/tests.py index 7ae39278fd..122017b54e 100644 --- a/tests/cache/tests.py +++ b/tests/cache/tests.py @@ -33,8 +33,8 @@ from django.template import engines from django.template.context_processors import csrf from django.template.response import TemplateResponse from django.test import ( - RequestFactory, SimpleTestCase, TestCase, TransactionTestCase, mock, - override_settings, + RequestFactory, SimpleTestCase, TestCase, TransactionTestCase, + ignore_warnings, mock, override_settings, ) from django.test.signals import setting_changed from django.utils import six, timezone, translation @@ -1856,6 +1856,7 @@ class CacheI18nTest(TestCase): "Cache keys should include the time zone name when time zones are active" ) + @ignore_warnings(category=RemovedInDjango21Warning) # USE_ETAGS=True @override_settings( CACHE_MIDDLEWARE_KEY_PREFIX="test", CACHE_MIDDLEWARE_SECONDS=60, @@ -2262,6 +2263,7 @@ class TestWithTemplateResponse(SimpleTestCase): response = response.render() self.assertFalse(response.has_header('ETag')) + @ignore_warnings(category=RemovedInDjango21Warning) @override_settings(USE_ETAGS=True) def test_with_etag(self): template = engines['django'].from_string("This is a test") diff --git a/tests/middleware/tests.py b/tests/middleware/tests.py index 29de46841e..7bf53cfdeb 100644 --- a/tests/middleware/tests.py +++ b/tests/middleware/tests.py @@ -20,8 +20,11 @@ from django.middleware.common import ( ) from django.middleware.gzip import GZipMiddleware from django.middleware.http import ConditionalGetMiddleware -from django.test import RequestFactory, SimpleTestCase, override_settings +from django.test import ( + RequestFactory, SimpleTestCase, ignore_warnings, override_settings, +) from django.utils import six +from django.utils.deprecation import RemovedInDjango21Warning from django.utils.encoding import force_str from django.utils.six.moves import range from django.utils.six.moves.urllib.parse import quote @@ -256,12 +259,14 @@ class CommonMiddlewareTest(SimpleTestCase): # ETag + If-Not-Modified support tests + @ignore_warnings(category=RemovedInDjango21Warning) @override_settings(USE_ETAGS=True) def test_etag(self): req = HttpRequest() res = HttpResponse('content') self.assertTrue(CommonMiddleware().process_response(req, res).has_header('ETag')) + @ignore_warnings(category=RemovedInDjango21Warning) @override_settings(USE_ETAGS=True) def test_etag_streaming_response(self): req = HttpRequest() @@ -269,12 +274,14 @@ class CommonMiddlewareTest(SimpleTestCase): res['ETag'] = 'tomatoes' self.assertEqual(CommonMiddleware().process_response(req, res).get('ETag'), 'tomatoes') + @ignore_warnings(category=RemovedInDjango21Warning) @override_settings(USE_ETAGS=True) def test_no_etag_streaming_response(self): req = HttpRequest() res = StreamingHttpResponse(['content']) self.assertFalse(CommonMiddleware().process_response(req, res).has_header('ETag')) + @ignore_warnings(category=RemovedInDjango21Warning) @override_settings(USE_ETAGS=True) def test_no_etag_no_store_cache(self): req = HttpRequest() @@ -282,6 +289,7 @@ class CommonMiddlewareTest(SimpleTestCase): res['Cache-Control'] = 'No-Cache, No-Store, Max-age=0' self.assertFalse(CommonMiddleware().process_response(req, res).has_header('ETag')) + @ignore_warnings(category=RemovedInDjango21Warning) @override_settings(USE_ETAGS=True) def test_etag_extended_cache_control(self): req = HttpRequest() @@ -289,6 +297,7 @@ class CommonMiddlewareTest(SimpleTestCase): res['Cache-Control'] = 'my-directive="my-no-store"' self.assertTrue(CommonMiddleware().process_response(req, res).has_header('ETag')) + @ignore_warnings(category=RemovedInDjango21Warning) @override_settings(USE_ETAGS=True) def test_if_none_match(self): first_req = HttpRequest() @@ -502,6 +511,30 @@ class ConditionalGetMiddlewareTest(SimpleTestCase): # Tests for the ETag header + def test_middleware_calculates_etag(self): + self.assertNotIn('ETag', self.resp) + self.resp = ConditionalGetMiddleware().process_response(self.req, self.resp) + self.assertEqual(self.resp.status_code, 200) + self.assertNotEqual('', self.resp['ETag']) + + def test_middleware_wont_overwrite_etag(self): + self.resp['ETag'] = 'eggs' + self.resp = ConditionalGetMiddleware().process_response(self.req, self.resp) + self.assertEqual(self.resp.status_code, 200) + self.assertEqual('eggs', self.resp['ETag']) + + def test_no_etag_streaming_response(self): + res = StreamingHttpResponse(['content']) + self.assertFalse(ConditionalGetMiddleware().process_response(self.req, res).has_header('ETag')) + + def test_no_etag_no_store_cache(self): + self.resp['Cache-Control'] = 'No-Cache, No-Store, Max-age=0' + self.assertFalse(ConditionalGetMiddleware().process_response(self.req, self.resp).has_header('ETag')) + + def test_etag_extended_cache_control(self): + self.resp['Cache-Control'] = 'my-directive="my-no-store"' + self.assertTrue(ConditionalGetMiddleware().process_response(self.req, self.resp).has_header('ETag')) + def test_if_none_match_and_no_etag(self): self.req.META['HTTP_IF_NONE_MATCH'] = 'spam' self.resp = ConditionalGetMiddleware().process_response(self.req, self.resp) @@ -796,6 +829,7 @@ class GZipMiddlewareTest(SimpleTestCase): self.assertIsNone(r.get('Content-Encoding')) +@ignore_warnings(category=RemovedInDjango21Warning) @override_settings(USE_ETAGS=True) class ETagGZipMiddlewareTest(SimpleTestCase): """