Fixed #26447 -- Deprecated settings.USE_ETAGS in favor of ConditionalGetMiddleware.

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 </topics/cache>`.
 
+.. 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 <USE_ETAGS>`), 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
 <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):
     """