bug 730714: Use caching and 304's with kumascript

* Constance-configurable default max-age for kumascript requests

* Use ETag and Last-Modified for conditional GET and locally cached
  content on 304 responses.

* Kumascript cache invalidation on document edits

* Allow end-users to trigger cache invalidation with reload and
  shift-reload.

* Set an X-Kumascript-Caching header that reports on the results of
  kumascript caching. Can be inspected in the Web Console net log.

* kumascript update, which supports all the above.

apps/wiki/tests/test_views.py

@@ -3,6 +3,7 @@
 
 from django.conf import settings
 from django.contrib.sites.models import Site
+from django.core.cache import cache
 
 import mock
 from nose import SkipTest
@@ -134,6 +135,14 @@ def test_json_view(self):
         eq_('an article title', data['title'])
 
 
+class FakeResponse:
+    """Quick and dirty mocking stand-in for a response object"""
+    def __init__(self, **entries): 
+        self.__dict__.update(entries)
+    def read(self):
+        return self.body
+
+
 class KumascriptIntegrationTests(TestCaseBase):
     """Tests for usage of the kumascript service.
     
@@ -162,6 +171,7 @@ def tearDown(self):
         super(KumascriptIntegrationTests, self).tearDown()
 
         constance.config.KUMASCRIPT_TIMEOUT = 0.0
+        constance.config.KUMASCRIPT_MAX_AGE = 600
 
         # NOTE: We could do this instead of using the @patch decorator over and
         # over, but it requires an upgrade of mock to 0.8.0
@@ -210,6 +220,98 @@ def test_raw_macros(self, mock_perform_kumascript_request):
         ok_(mock_perform_kumascript_request.called,
             "kumascript should have been used")
 
+    @mock.patch('requests.get')
+    def test_ua_max_age_zero(self, mock_requests_get):
+        """Authenticated users can request a zero max-age for kumascript"""
+        trap = {}
+        def my_requests_get(url, headers=None, timeout=None):
+            trap['headers'] = headers
+            return FakeResponse(status_code=200,
+                headers={}, body='HELLO WORLD')
+
+        mock_requests_get.side_effect = my_requests_get
+
+        constance.config.KUMASCRIPT_TIMEOUT = 1.0
+        constance.config.KUMASCRIPT_MAX_AGE = 1234
+
+        response = self.client.get(self.url, follow=False,
+                HTTP_CACHE_CONTROL='max-age=0')
+        eq_('max-age=1234', trap['headers']['Cache-Control'])
+
+        self.client.login(username='admin', password='testpass')
+        response = self.client.get(self.url, follow=False,
+                HTTP_CACHE_CONTROL='max-age=0')
+        eq_('max-age=0', trap['headers']['Cache-Control'])
+
+    @mock.patch('requests.get')
+    def test_ua_no_cache(self, mock_requests_get):
+        """Authenticated users can request no-cache for kumascript"""
+        trap = {}
+        def my_requests_get(url, headers=None, timeout=None):
+            trap['headers'] = headers
+            return FakeResponse(status_code=200,
+                headers={}, body='HELLO WORLD')
+
+        mock_requests_get.side_effect = my_requests_get
+
+        constance.config.KUMASCRIPT_TIMEOUT = 1.0
+        constance.config.KUMASCRIPT_MAX_AGE = 1234
+
+        response = self.client.get(self.url, follow=False,
+                HTTP_CACHE_CONTROL='no-cache')
+        eq_('max-age=1234', trap['headers']['Cache-Control'])
+
+        self.client.login(username='admin', password='testpass')
+        response = self.client.get(self.url, follow=False,
+                HTTP_CACHE_CONTROL='no-cache')
+        eq_('no-cache', trap['headers']['Cache-Control'])
+
+    @mock.patch('requests.get')
+    def test_conditional_get(self, mock_requests_get):
+        """Ensure conditional GET in requests to kumascript work as expected"""
+        expected_etag = "8675309JENNY"
+        expected_modified = "Wed, 14 Mar 2012 22:29:17 GMT"
+        expected_content = "HELLO THERE, WORLD"
+
+        trap = dict( req_cnt=0 )
+        def my_requests_get(url, headers=None, timeout=None):
+            trap['req_cnt'] += 1
+            trap['headers'] = headers
+
+            if trap['req_cnt'] in [1, 2]:
+                return FakeResponse(status_code=200, body=expected_content,
+                    headers = { 
+                        "etag": expected_etag,
+                        "last-modified": expected_modified,
+                        "age": 456
+                    })
+            else:
+                return FakeResponse(status_code=304, body='',
+                    headers = { 
+                        "etag": expected_etag,
+                        "last-modified": expected_modified,
+                        "age": 123
+                    })
+
+        mock_requests_get.side_effect = my_requests_get
+
+        constance.config.KUMASCRIPT_TIMEOUT = 1.0
+        constance.config.KUMASCRIPT_MAX_AGE = 1234
+
+        # First request to let the view cache etag / last-modified
+        response = self.client.get(self.url)
+
+        # Second request to verify the view sends them back
+        response = self.client.get(self.url)
+        eq_(expected_etag, trap['headers']['If-None-Match'])
+        eq_(expected_modified, trap['headers']['If-Modified-Since'])
+        eq_('200 OK, Age: 456', response['X-Kumascript-Caching'])
+
+        # Third request to verify content was cached and served on a 304
+        response = self.client.get(self.url)
+        ok_(expected_content in response.content)
+        eq_('304 Not Modified, Age: 123', response['X-Kumascript-Caching'])
+
 
 class DocumentEditingTests(TestCaseBase):
     """Tests for the document-editing view"""

apps/wiki/views.py

@@ -14,6 +14,7 @@
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.exceptions import PermissionDenied
+from django.core.cache import cache
 from django.http import (HttpResponse, HttpResponseRedirect,
                          HttpResponsePermanentRedirect,
                          Http404, HttpResponseBadRequest)
@@ -190,11 +191,15 @@ def document(request, document_slug, document_locale):
             pass
 
     # Utility to set common headers used by all response exit points
+    response_headers = dict()
     def set_common_headers(r):
         r['ETag'] = doc.etag
         r['Last-Modified'] = doc.last_modified
         if doc.current_revision:
             r['x-kuma-revision'] = doc.current_revision.id
+        # Finally, set any extra headers. update() doesn't work here.
+        for k,v in response_headers.items():
+            r[k] = v
         return r
 
     related = doc.related_documents.order_by('-related_to__in_common')[0:5]
@@ -230,7 +235,8 @@ def set_common_headers(r):
         #     (eg. ?nomacros)
         #   * The request *has* asked for macro evaluation 
         #     (eg. ?raw&macros)
-        resp_body = _perform_kumascript_request(document_locale,
+        resp_body = _perform_kumascript_request(request, response_headers,
+                                                document_locale,
                                                 document_slug)
         if resp_body:
             doc_html = resp_body
@@ -266,7 +272,27 @@ def set_common_headers(r):
     return set_common_headers(response)
 
 
-def _perform_kumascript_request(document_locale, document_slug):
+def _build_kumascript_cache_keys(document_locale, document_slug):
+    """Build the cache keys used for Kumascript"""
+    cache_key = ('kumascript:%s/%s:%s' %
+                 (document_locale, document_slug, '%s'))
+    ck_etag = cache_key % 'etag'
+    ck_modified = cache_key % 'modified'
+    ck_body = cache_key % 'body'
+    return (ck_etag, ck_modified, ck_body)
+
+
+def _invalidate_kumascript_cache(document):
+    """Invalidate the cached kumascript response for a given document"""
+    if constance.config.KUMASCRIPT_TIMEOUT == 0:
+        # Do nothing if kumascript is disabled
+        return
+    cache.delete_many(_build_kumascript_cache_keys(document.slug,
+                                                   document.locale))
+
+
+def _perform_kumascript_request(request, response_headers, document_locale,
+                                document_slug):
     """Perform a kumascript GET request for a document locale and slug.
 
     This is broken out into its own utility function, both to make the view
@@ -276,17 +302,73 @@ def _perform_kumascript_request(document_locale, document_slug):
 
     try:
         url_tmpl = settings.KUMASCRIPT_URL_TEMPLATE
-        resp = requests.get(
-            url_tmpl.format(path='%s/%s' % (document_locale,
-                                            document_slug)),
+        url = url_tmpl.format(path='%s/%s' %
+                                   (document_locale, document_slug))
+
+        ck_etag, ck_modified, ck_body = _build_kumascript_cache_keys(
+                                            document_slug, document_locale)
+
+        # Default to the configured max-age for cache control.
+        max_age = constance.config.KUMASCRIPT_MAX_AGE
+        cache_control = 'max-age=%s' % max_age
+
+        # TODO: Wrap this in a waffle flag for primitive access control?
+        if request.user.is_authenticated():
+            # Restricting to auth'd users places a speed bump on end-user
+            # triggered cache invalidation.
+            ua_cc = request.META.get('HTTP_CACHE_CONTROL')
+
+            if ua_cc == 'no-cache':
+                # Firefox issues no-cache on shift-reload, so this lets end-users
+                # trigger cache invalidation. kumascript will react to no-cache
+                # by reloading both document and template sources from Kuma.
+                cache_control = 'no-cache'
+
+            elif ua_cc == 'max-age=0':
+                # Firefox sends Cache-Control: max-age=0 on reload. kumascript
+                # will react to max-age=0 by reloading just the document source
+                # from Kuma and use cached templates. (pending bug 730715)
+                cache_control = 'max-age=0'
+
+        headers = { 'Cache-Control': cache_control }
+
+        # Set up for conditional GET, if we have the details cached.
+        c_meta = cache.get_many([ck_etag, ck_modified])
+        if ck_etag in c_meta:
+            headers['If-None-Match'] = c_meta[ck_etag]
+        if ck_modified in c_meta:
+            headers['If-Modified-Since'] = c_meta[ck_modified]
+
+        # Finally, fire off the request.
+        resp = requests.get(url, headers=headers,
             timeout=constance.config.KUMASCRIPT_TIMEOUT)
+
         if resp.status_code == 200:
             # HACK: Assume we're getting UTF-8, which we should be.
             # TODO: Better solution would be to upgrade the requests module
             # in vendor from 0.6.1 to at least 0.10.6, and use resp.text,
             # which does auto-detection. But, that will break things.
             resp_body = resp.read().decode('utf8')
 
+            # Set a header so we can see what happened in caching.
+            response_headers['X-Kumascript-Caching'] = (
+                    '200 OK, Age: %s' % resp.headers.get('age', 0))
+
+            # Cache the request for conditional GET, but use the max_age for
+            # the cache timeout here too.
+            cache.set_many({
+                ck_etag: resp.headers.get('etag'),
+                ck_modified: resp.headers.get('last-modified'),
+                ck_body: resp_body
+            }, timeout=max_age)
+
+        elif resp.status_code == 304:
+            # Conditional GET was a pass, so use the cached body.
+            resp_body = cache.get(ck_body)
+            # Set a header so we can see what happened in caching.
+            response_headers['X-Kumascript-Caching'] = (
+                    '304 Not Modified, Age: %s' % resp.headers.get('age', 0))
+
     except Exception, e:
         # Do nothing, if the kumascript service fails in some way.
         # TODO: Log the failure more usefully here.
@@ -435,6 +517,7 @@ def edit_document(request, document_slug, document_locale, revision_id=None):
                 if doc_form.is_valid():
                     # Get the possibly new slug for the imminent redirection:
                     doc = doc_form.save(None)
+                    _invalidate_kumascript_cache(doc)
 
                     # Do we need to rebuild the KB?
                     _maybe_schedule_rebuild(doc_form)
@@ -1033,6 +1116,8 @@ def _save_rev_and_notify(rev_form, creator, document):
     """Save the given RevisionForm and send notifications."""
     new_rev = rev_form.save(creator, document)
 
+    _invalidate_kumascript_cache(document)
+
     # Enqueue notifications
     ReviewableRevisionInLocaleEvent(new_rev).fire(exclude=new_rev.creator)
     EditDocumentEvent(new_rev).fire(exclude=new_rev.creator)

settings.py

@@ -958,6 +958,12 @@ def read_only_mode(env):
         'evaluation as an attempt at graceful failure. NOTE: a value of 0 '
         'disables kumascript altogether.'
     ),
+    KUMASCRIPT_MAX_AGE = (
+        600,
+        'Maximum acceptable age (in seconds) of a cached response from '
+        'kumascript. Passed along in a Cache-Control: max-age={value} header, '
+        'which tells kumascript whether or not to serve up a cached response.'
+    ),
 )
 
 BROWSERID_VERIFICATION_URL = 'https://browserid.org/verify'