From 3f528e10d50ff7ba19a8a9e6cb2f9417a1e7f270 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Mon, 24 Jan 2011 14:24:35 +0000 Subject: [PATCH] Fixed #15012 -- Added post-rendering callbacks to TemplateResponse so that decorators (in particular, the cache decorator) can defer processing until after rendering has occurred. Thanks to Joshua Ginsberg for the draft patch. git-svn-id: http://code.djangoproject.com/svn/django/trunk@15295 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/middleware/cache.py | 8 +- django/template/response.py | 48 +++++++- docs/ref/template-response.txt | 59 +++++++++- tests/regressiontests/generic_views/base.py | 25 +++- .../templates/generic_views/about.html | 3 +- tests/regressiontests/generic_views/urls.py | 4 + .../templates/alternate_urls.py | 4 +- tests/regressiontests/templates/response.py | 110 +++++++++++++++++- .../templates/templates/response.html | 3 +- 9 files changed, 256 insertions(+), 8 deletions(-) diff --git a/django/middleware/cache.py b/django/middleware/cache.py index 907edbb16f..ab72db3c18 100644 --- a/django/middleware/cache.py +++ b/django/middleware/cache.py @@ -52,6 +52,7 @@ from django.conf import settings from django.core.cache import get_cache, DEFAULT_CACHE_ALIAS from django.utils.cache import get_cache_key, learn_cache_key, patch_response_headers, get_max_age + class UpdateCacheMiddleware(object): """ Response-phase cache middleware that updates the cache if the response is @@ -87,7 +88,12 @@ class UpdateCacheMiddleware(object): patch_response_headers(response, timeout) if timeout: cache_key = learn_cache_key(request, response, timeout, self.key_prefix, cache=self.cache) - self.cache.set(cache_key, response, timeout) + if hasattr(response, 'render') and callable(response.render): + response.add_post_render_callback( + lambda r: self.cache.set(cache_key, r, timeout) + ) + else: + self.cache.set(cache_key, response, timeout) return response class FetchFromCacheMiddleware(object): diff --git a/django/template/response.py b/django/template/response.py index 629461aa5e..a6ef893520 100644 --- a/django/template/response.py +++ b/django/template/response.py @@ -19,12 +19,30 @@ class SimpleTemplateResponse(HttpResponse): # a final response. self._is_rendered = False + self._post_render_callbacks = [] + # content argument doesn't make sense here because it will be replaced # with rendered template so we always pass empty string in order to # prevent errors and provide shorter signature. super(SimpleTemplateResponse, self).__init__('', mimetype, status, content_type) + def __getstate__(self): + """Pickling support function. + + Ensures that the object can't be pickled before it has been + rendered, and that the pickled state only includes rendered + data, not the data used to construct the response. + """ + obj_dict = self.__dict__.copy() + if not self._is_rendered: + raise ContentNotRenderedError('The response content must be rendered before it can be pickled.') + del obj_dict['template_name'] + del obj_dict['context_data'] + del obj_dict['_post_render_callbacks'] + + return obj_dict + def resolve_template(self, template): "Accepts a template object, path-to-template or list of paths" if isinstance(template, (list, tuple)): @@ -57,6 +75,16 @@ class SimpleTemplateResponse(HttpResponse): content = template.render(context) return content + def add_post_render_callback(self, callback): + """Add a new post-rendering callback. + + If the response has already been rendered, invoke the callback immediately. + """ + if self._is_rendered: + callback(self) + else: + self._post_render_callbacks.append(callback) + def render(self): """Render (thereby finalizing) the content of the response. @@ -66,6 +94,8 @@ class SimpleTemplateResponse(HttpResponse): """ if not self._is_rendered: self._set_content(self.rendered_content) + for post_callback in self._post_render_callbacks: + post_callback(self) return self is_rendered = property(lambda self: self._is_rendered) @@ -81,7 +111,7 @@ class SimpleTemplateResponse(HttpResponse): return super(SimpleTemplateResponse, self)._get_content() def _set_content(self, value): - "Overrides rendered content, unless you later call render()" + "Sets the content for the response" super(SimpleTemplateResponse, self)._set_content(value) self._is_rendered = True @@ -101,6 +131,20 @@ class TemplateResponse(SimpleTemplateResponse): super(TemplateResponse, self).__init__( template, context, mimetype, status, content_type) + def __getstate__(self): + """Pickling support function. + + Ensures that the object can't be pickled before it has been + rendered, and that the pickled state only includes rendered + data, not the data used to construct the response. + """ + obj_dict = super(TemplateResponse, self).__getstate__() + + del obj_dict['_request'] + del obj_dict['_current_app'] + + return obj_dict + def resolve_context(self, context): """Convert context data into a full RequestContext object (assuming it isn't already a Context object). @@ -109,3 +153,5 @@ class TemplateResponse(SimpleTemplateResponse): return context else: return RequestContext(self._request, context, current_app=self._current_app) + + diff --git a/docs/ref/template-response.txt b/docs/ref/template-response.txt index d4fe2c4ef2..90da00220e 100644 --- a/docs/ref/template-response.txt +++ b/docs/ref/template-response.txt @@ -55,7 +55,6 @@ Attributes A boolean indicating whether the response content has been rendered. - Methods ------- @@ -106,6 +105,20 @@ Methods Override this method in order to customize template rendering. +.. method:: SimpleTemplateResponse.add_post_rendering_callback + + Add a callback that will be invoked after rendering has taken + place. This hook can be used to defer certain processing + operations (such as caching) until after rendering has occurred. + + If the :class:`~django.template.response.SimpleTemplateResponse` + has already been rendered, the callback will be invoked + immediately. + + When called, callbacks will be passed a single argument -- the + rendered :class:`~django.template.response.SimpleTemplateResponse` + instance. + .. method:: SimpleTemplateResponse.render(): Sets :attr:`response.content` to the result obtained by @@ -211,6 +224,50 @@ the content of the response manually:: >>> print t.content New content +Post-render callbacks +--------------------- + +Some operations -- such as caching -- cannot be performed on an +unrendered template. They must be performed on a fully complete and +rendered response. + +If you're using middleware, the solution is easy. Middleware provides +multiple opportunities to process a response on exit from a view. If +you put behavior in the Response middleware is guaranteed to execute +after template rendering has taken place. + +However, if you're using a decorator, the same opportunities do not +exist. Any behavior defined in a decorator is handled immediately. + +To compensate for this (and any other analogous use cases), +:class:`TemplateResponse` allows you to register callbacks that will +be invoked when rendering has completed. Using this callback, you can +defer critical processing until a point where you can guarantee that +rendered content will be available. + +To define a post-render callback, just define a function that takes +a single argument -- response -- and register that function with +the template response:: + + def my_render_callback(response): + # Do content-sensitive processing + do_post_processing() + + def my_view(request): + # Create a response + response = TemplateResponse(request, 'mytemplate.html', {}) + # Register the callback + response.add_post_render_callback(my_render_callback) + # Return the response + return response + +``my_render_callback()`` will be invoked after the ``mytemplate.html`` +has been rendered, and will be provided the fully rendered +:class:`TemplateResponse` instance as an argument. + +If the template has already been rendered, the callback will be +invoked immediately. + Using TemplateResponse and SimpleTemplateResponse ================================================= diff --git a/tests/regressiontests/generic_views/base.py b/tests/regressiontests/generic_views/base.py index f356c90a74..c378315b62 100644 --- a/tests/regressiontests/generic_views/base.py +++ b/tests/regressiontests/generic_views/base.py @@ -1,3 +1,4 @@ +import time import unittest from django.core.exceptions import ImproperlyConfigured @@ -158,7 +159,7 @@ class TemplateViewTest(TestCase): def _assert_about(self, response): response.render() self.assertEqual(response.status_code, 200) - self.assertEqual(response.content, '

About

') + self.assertContains(response, '

About

') def test_get(self): """ @@ -197,6 +198,28 @@ class TemplateViewTest(TestCase): self.assertEqual(response.context['params'], {'foo': 'bar'}) self.assertEqual(response.context['key'], 'value') + def test_cached_views(self): + """ + A template view can be cached + """ + response = self.client.get('/template/cached/bar/') + self.assertEqual(response.status_code, 200) + + time.sleep(1.0) + + response2 = self.client.get('/template/cached/bar/') + self.assertEqual(response2.status_code, 200) + + self.assertEqual(response.content, response2.content) + + time.sleep(2.0) + + # Let the cache expire and test again + response2 = self.client.get('/template/cached/bar/') + self.assertEqual(response2.status_code, 200) + + self.assertNotEqual(response.content, response2.content) + class RedirectViewTest(unittest.TestCase): rf = RequestFactory() diff --git a/tests/regressiontests/generic_views/templates/generic_views/about.html b/tests/regressiontests/generic_views/templates/generic_views/about.html index 7d877fdd78..f946f630cf 100644 --- a/tests/regressiontests/generic_views/templates/generic_views/about.html +++ b/tests/regressiontests/generic_views/templates/generic_views/about.html @@ -1 +1,2 @@ -

About

\ No newline at end of file +

About

+{% now "U.u" %} diff --git a/tests/regressiontests/generic_views/urls.py b/tests/regressiontests/generic_views/urls.py index bd997e7e6d..4c847ac78d 100644 --- a/tests/regressiontests/generic_views/urls.py +++ b/tests/regressiontests/generic_views/urls.py @@ -1,5 +1,6 @@ from django.conf.urls.defaults import * from django.views.generic import TemplateView +from django.views.decorators.cache import cache_page import views @@ -15,6 +16,9 @@ urlpatterns = patterns('', (r'^template/custom/(?P\w+)/$', views.CustomTemplateView.as_view(template_name='generic_views/about.html')), + (r'^template/cached/(?P\w+)/$', + cache_page(2.0)(TemplateView.as_view(template_name='generic_views/about.html'))), + # DetailView (r'^detail/obj/$', views.ObjectDetail.as_view()), diff --git a/tests/regressiontests/templates/alternate_urls.py b/tests/regressiontests/templates/alternate_urls.py index 31590a5dbc..e68f1c5875 100644 --- a/tests/regressiontests/templates/alternate_urls.py +++ b/tests/regressiontests/templates/alternate_urls.py @@ -1,10 +1,12 @@ # coding: utf-8 from django.conf.urls.defaults import * + from regressiontests.templates import views + urlpatterns = patterns('', # View returning a template response - (r'^template_response_view/', views.template_response_view), + (r'^template_response_view/$', views.template_response_view), # A view that can be hard to find... url(r'^snark/', views.snark, name='snark'), diff --git a/tests/regressiontests/templates/response.py b/tests/regressiontests/templates/response.py index f658b35ac3..1e42d66663 100644 --- a/tests/regressiontests/templates/response.py +++ b/tests/regressiontests/templates/response.py @@ -1,4 +1,7 @@ +from datetime import datetime import os +import pickle +import time from django.utils import unittest from django.test import RequestFactory, TestCase from django.conf import settings @@ -147,6 +150,49 @@ class SimpleTemplateResponseTest(BaseTemplateResponseTest): self.assertEqual(response['content-type'], 'application/json') self.assertEqual(response.status_code, 504) + def test_post_callbacks(self): + "Rendering a template response triggers the post-render callbacks" + post = [] + + def post1(obj): + post.append('post1') + def post2(obj): + post.append('post2') + + response = SimpleTemplateResponse('first/test.html', {}) + response.add_post_render_callback(post1) + response.add_post_render_callback(post2) + + # When the content is rendered, all the callbacks are invoked, too. + response.render() + self.assertEqual('First template\n', response.content) + self.assertEquals(post, ['post1','post2']) + + + def test_pickling(self): + # Create a template response. The context is + # known to be unpickleable (e.g., a function). + response = SimpleTemplateResponse('first/test.html', { + 'value': 123, + 'fn': datetime.now, + }) + self.assertRaises(ContentNotRenderedError, + pickle.dumps, response) + + # But if we render the response, we can pickle it. + response.render() + pickled_response = pickle.dumps(response) + unpickled_response = pickle.loads(pickled_response) + + self.assertEquals(unpickled_response.content, response.content) + self.assertEquals(unpickled_response['content-type'], response['content-type']) + self.assertEquals(unpickled_response.status_code, response.status_code) + + # ...and the unpickled reponse doesn't have the + # template-related attributes, so it can't be re-rendered + self.assertFalse(hasattr(unpickled_response, 'template_name')) + self.assertFalse(hasattr(unpickled_response, 'context_data')) + self.assertFalse(hasattr(unpickled_response, '_post_render_callbacks')) class TemplateResponseTest(BaseTemplateResponseTest): @@ -187,6 +233,33 @@ class TemplateResponseTest(BaseTemplateResponseTest): self.assertEqual(rc.current_app, 'foobar') + def test_pickling(self): + # Create a template response. The context is + # known to be unpickleable (e.g., a function). + response = TemplateResponse(self.factory.get('/'), + 'first/test.html', { + 'value': 123, + 'fn': datetime.now, + }) + self.assertRaises(ContentNotRenderedError, + pickle.dumps, response) + + # But if we render the response, we can pickle it. + response.render() + pickled_response = pickle.dumps(response) + unpickled_response = pickle.loads(pickled_response) + + self.assertEquals(unpickled_response.content, response.content) + self.assertEquals(unpickled_response['content-type'], response['content-type']) + self.assertEquals(unpickled_response.status_code, response.status_code) + + # ...and the unpickled reponse doesn't have the + # template-related attributes, so it can't be re-rendered + self.assertFalse(hasattr(unpickled_response, '_request')) + self.assertFalse(hasattr(unpickled_response, 'template_name')) + self.assertFalse(hasattr(unpickled_response, 'context_data')) + self.assertFalse(hasattr(unpickled_response, '_post_render_callbacks')) + class CustomURLConfTest(TestCase): urls = 'regressiontests.templates.urls' @@ -203,6 +276,41 @@ class CustomURLConfTest(TestCase): def test_custom_urlconf(self): response = self.client.get('/template_response_view/') self.assertEqual(response.status_code, 200) - self.assertEqual(response.content, 'This is where you can find the snark: /snark/') + self.assertContains(response, 'This is where you can find the snark: /snark/') +class CacheMiddlewareTest(TestCase): + urls = 'regressiontests.templates.alternate_urls' + + def setUp(self): + self.old_MIDDLEWARE_CLASSES = settings.MIDDLEWARE_CLASSES + self.CACHE_MIDDLEWARE_SECONDS = settings.CACHE_MIDDLEWARE_SECONDS + + settings.CACHE_MIDDLEWARE_SECONDS = 2.0 + settings.MIDDLEWARE_CLASSES = list(settings.MIDDLEWARE_CLASSES) + [ + 'django.middleware.cache.FetchFromCacheMiddleware', + 'django.middleware.cache.UpdateCacheMiddleware', + ] + + def tearDown(self): + settings.MIDDLEWARE_CLASSES = self.old_MIDDLEWARE_CLASSES + settings.CACHE_MIDDLEWARE_SECONDS = self.CACHE_MIDDLEWARE_SECONDS + + def test_middleware_caching(self): + response = self.client.get('/template_response_view/') + self.assertEqual(response.status_code, 200) + + time.sleep(1.0) + + response2 = self.client.get('/template_response_view/') + self.assertEqual(response2.status_code, 200) + + self.assertEqual(response.content, response2.content) + + time.sleep(2.0) + + # Let the cache expire and test again + response2 = self.client.get('/template_response_view/') + self.assertEqual(response2.status_code, 200) + + self.assertNotEqual(response.content, response2.content) diff --git a/tests/regressiontests/templates/templates/response.html b/tests/regressiontests/templates/templates/response.html index 7c442fb453..7535fa7606 100644 --- a/tests/regressiontests/templates/templates/response.html +++ b/tests/regressiontests/templates/templates/response.html @@ -1 +1,2 @@ -{% load url from future %}This is where you can find the snark: {% url "snark" %} \ No newline at end of file +{% load url from future %}This is where you can find the snark: {% url "snark" %} +{% now "U.u" %} \ No newline at end of file