Fix bug 1054472: Add ETag header to responses.

snippets/base/templates/base/fetch_snippets.html

@@ -5,4 +5,3 @@
   {% endfor %}
   {% include 'base/includes/snippet_js.html' %}
 </div>
-<!-- Content generated at {{ current_time }} -->

snippets/base/tests/test_views.py

@@ -2,13 +2,16 @@
 
 from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
+from django.http import HttpResponse
+from django.test.client import RequestFactory
 from django.test.utils import override_settings
 
 from funfactory.helpers import urlparams
 from mock import patch
 from nose.tools import eq_, ok_
 
 import snippets.base.models
+from snippets.base import views
 from snippets.base.models import Client
 from snippets.base.tests import (JSONSnippetFactory, SnippetFactory,
                                  SnippetTemplateFactory, TestCase)
@@ -18,6 +21,23 @@
 
 
 class FetchSnippetsTests(TestCase):
+    def setUp(self):
+        self.factory = RequestFactory()
+        self.client_items = [
+            ('startpage_version', '4'),
+            ('name', 'Firefox'),
+            ('version', '23.0a1'),
+            ('appbuildid', '20130510041606'),
+            ('build_target', 'Darwin_Universal-gcc3'),
+            ('locale', 'en-US'),
+            ('channel', 'nightly'),
+            ('os_version', 'Darwin 10.8.0'),
+            ('distribution', 'default'),
+            ('distribution_version', 'default_version'),
+        ]
+        self.client_params = [v[1] for v in self.client_items]
+        self.client_kwargs = dict(self.client_items)
+
     def test_base(self):
         # Matching snippets.
         snippet_1 = SnippetFactory.create(on_nightly=True)
@@ -29,9 +49,7 @@ def test_base(self):
         SnippetFactory.create(on_nightly=False),
 
         snippets_ok = [snippet_1]
-        params = ('4', 'Firefox', '23.0a1', '20130510041606',
-                  'Darwin_Universal-gcc3', 'en-US', 'nightly',
-                  'Darwin%2010.8.0', 'default', 'default_version')
+        params = self.client_params
         response = self.client.get('/{0}/'.format('/'.join(params)))
 
         eq_(set(snippets_ok), set(response.context['snippets']))
@@ -43,35 +61,37 @@ def test_client_construction(self, ClientMock):
         Ensure that the client object is constructed correctly from the URL
         arguments.
         """
-        params = ('4', 'Firefox', '23.0a1', '20130510041606',
-                  'Darwin_Universal-gcc3', 'en-US', 'nightly',
-                  'Darwin%2010.8.0', 'default', 'default_version')
+        params = self.client_params
         self.client.get('/{0}/'.format('/'.join(params)))
 
-        ClientMock.assert_called_with(startpage_version='4',
-                                      name='Firefox',
-                                      version='23.0a1',
-                                      appbuildid='20130510041606',
-                                      build_target='Darwin_Universal-gcc3',
-                                      locale='en-US',
-                                      channel='nightly',
-                                      os_version='Darwin 10.8.0',
-                                      distribution='default',
-                                      distribution_version='default_version')
+        ClientMock.assert_called_with(**self.client_kwargs)
 
     @override_settings(SNIPPET_HTTP_MAX_AGE=75)
     def test_cache_headers(self):
         """
-        view_snippets should always have Cache-control set to
-        'public, max-age={settings.SNIPPET_HTTP_MAX_AGE}' and no Vary header,
-        even after middleware is executed.
+        fetch_snippets should always have Cache-control set to
+        'public, max-age={settings.SNIPPET_HTTP_MAX_AGE}' and a Vary
+        header for 'If-None-Match'.
         """
-        params = ('4', 'Firefox', '23.0a1', '20130510041606',
-                  'Darwin_Universal-gcc3', 'en-US', 'nightly',
-                  'Darwin%2010.8.0', 'default', 'default_version')
+        params = self.client_params
         response = self.client.get('/{0}/'.format('/'.join(params)))
         eq_(response['Cache-control'], 'public, max-age=75')
-        ok_('Vary' not in response)
+        eq_(response['Vary'], 'If-None-Match')
+
+    def test_etag(self):
+        """
+        The response returned by fetch_snippets should have a ETag set
+        to the sha256 hash of the response content.
+        """
+        request = self.factory.get('/')
+
+        with patch.object(views, 'render') as mock_render:
+            mock_render.return_value = HttpResponse('asdf')
+            response = views.fetch_snippets(request, **self.client_kwargs)
+
+            # sha256 of 'asdf'
+            expected = 'f0e4c2f76c58916ec258f246851bea091d14d4247a2fc3e18694461b1816e13b'
+            eq_(response['ETag'], expected)
 
 
 class JSONSnippetsTests(TestCase):

snippets/base/views.py

@@ -1,11 +1,12 @@
+import hashlib
 import json
-from time import gmtime, strftime
 
 from django.conf import settings
 from django.contrib.auth.decorators import permission_required
 from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger
 from django.http import Http404,HttpResponse, HttpResponseBadRequest
 from django.shortcuts import get_object_or_404, render
+from django.utils.cache import patch_vary_headers
 from django.utils.functional import lazy
 from django.views.decorators.cache import cache_control
 from django.views.decorators.csrf import csrf_exempt
@@ -67,13 +68,18 @@ def fetch_snippets(request, **kwargs):
                          .select_related('template')
                          .filter_by_available())
 
-    return render(request, 'base/fetch_snippets.html', {
+    response = render(request, 'base/fetch_snippets.html', {
         'snippets': matching_snippets,
         'client': client,
-        'current_time': strftime('%Y-%m-%dT%H:%M:%SZ', gmtime()),
         'locale': client.locale,
     })
 
+    # ETag will be a hash of the response content.
+    response['ETag'] = hashlib.sha256(response.content).hexdigest()
+    patch_vary_headers(response, ['If-None-Match'])
+
+    return response
+
 
 @cache_control(public=True, max_age=HTTP_MAX_AGE)
 @access_control(max_age=HTTP_MAX_AGE)