diff --git a/django/core/handlers/wsgi.py b/django/core/handlers/wsgi.py index 426679ca7b..a9fa094429 100644 --- a/django/core/handlers/wsgi.py +++ b/django/core/handlers/wsgi.py @@ -253,8 +253,8 @@ class WSGIHandler(base.BaseHandler): response = http.HttpResponseBadRequest() else: response = self.get_response(request) - finally: - signals.request_finished.send(sender=self.__class__) + + response._handler_class = self.__class__ try: status_text = STATUS_CODE_TEXT[response.status_code] diff --git a/django/http/response.py b/django/http/response.py index d667ba6eed..48a401adcb 100644 --- a/django/http/response.py +++ b/django/http/response.py @@ -10,6 +10,7 @@ except ImportError: from urlparse import urlparse from django.conf import settings +from django.core import signals from django.core import signing from django.core.exceptions import SuspiciousOperation from django.http.cookie import SimpleCookie @@ -40,6 +41,9 @@ class HttpResponseBase(six.Iterator): self._headers = {} self._charset = settings.DEFAULT_CHARSET self._closable_objects = [] + # This parameter is set by the handler. It's necessary to preserve the + # historical behavior of request_finished. + self._handler_class = None if mimetype: warnings.warn("Using mimetype keyword argument is deprecated, use" " content_type instead", @@ -226,7 +230,11 @@ class HttpResponseBase(six.Iterator): # See http://blog.dscpl.com.au/2012/10/obligations-for-calling-close-on.html def close(self): for closable in self._closable_objects: - closable.close() + try: + closable.close() + except Exception: + pass + signals.request_finished.send(sender=self._handler_class) def write(self, content): raise Exception("This %s instance is not writable" % self.__class__.__name__) diff --git a/django/test/client.py b/django/test/client.py index 015ee1309a..77d4de0524 100644 --- a/django/test/client.py +++ b/django/test/client.py @@ -26,7 +26,6 @@ from django.utils.http import urlencode from django.utils.importlib import import_module from django.utils.itercompat import is_iterable from django.utils import six -from django.db import close_connection from django.test.utils import ContextList __all__ = ('Client', 'RequestFactory', 'encode_file', 'encode_multipart') @@ -72,6 +71,14 @@ class FakePayload(object): self.__len += len(content) +def closing_iterator_wrapper(iterable, close): + try: + for item in iterable: + yield item + finally: + close() + + class ClientHandler(BaseHandler): """ A HTTP Handler that can be used for testing purposes. @@ -92,18 +99,20 @@ class ClientHandler(BaseHandler): self.load_middleware() signals.request_started.send(sender=self.__class__) - try: - request = WSGIRequest(environ) - # sneaky little hack so that we can easily get round - # CsrfViewMiddleware. This makes life easier, and is probably - # required for backwards compatibility with external tests against - # admin views. - request._dont_enforce_csrf_checks = not self.enforce_csrf_checks - response = self.get_response(request) - finally: - signals.request_finished.disconnect(close_connection) - signals.request_finished.send(sender=self.__class__) - signals.request_finished.connect(close_connection) + request = WSGIRequest(environ) + # sneaky little hack so that we can easily get round + # CsrfViewMiddleware. This makes life easier, and is probably + # required for backwards compatibility with external tests against + # admin views. + request._dont_enforce_csrf_checks = not self.enforce_csrf_checks + response = self.get_response(request) + # We're emulating a WSGI server; we must call the close method + # on completion. + if response.streaming: + response.streaming_content = closing_iterator_wrapper( + response.streaming_content, response.close) + else: + response.close() return response diff --git a/django/test/utils.py b/django/test/utils.py index 8114ae0e6a..a1ff826d6e 100644 --- a/django/test/utils.py +++ b/django/test/utils.py @@ -4,6 +4,8 @@ from xml.dom.minidom import parseString, Node from django.conf import settings, UserSettingsHolder from django.core import mail +from django.core.signals import request_finished +from django.db import close_connection from django.test.signals import template_rendered, setting_changed from django.template import Template, loader, TemplateDoesNotExist from django.template.loaders import cached @@ -68,8 +70,10 @@ def setup_test_environment(): """Perform any global pre-test setup. This involves: - Installing the instrumented test renderer - - Set the email backend to the locmem email backend. + - Setting the email backend to the locmem email backend. - Setting the active locale to match the LANGUAGE_CODE setting. + - Disconnecting the request_finished signal to avoid closing + the database connection within tests. """ Template.original_render = Template._render Template._render = instrumented_test_render @@ -81,6 +85,8 @@ def setup_test_environment(): deactivate() + request_finished.disconnect(close_connection) + def teardown_test_environment(): """Perform any global post-test teardown. This involves: diff --git a/docs/ref/request-response.txt b/docs/ref/request-response.txt index ae1da6cb4b..a8e0ef3f51 100644 --- a/docs/ref/request-response.txt +++ b/docs/ref/request-response.txt @@ -790,6 +790,8 @@ types of HTTP responses. Like ``HttpResponse``, these subclasses live in :class:`~django.template.response.SimpleTemplateResponse`, and the ``render`` method must itself return a valid response object. +.. _httpresponse-streaming: + StreamingHttpResponse objects ============================= diff --git a/docs/ref/signals.txt b/docs/ref/signals.txt index b27a4f87cc..f2f1459bf0 100644 --- a/docs/ref/signals.txt +++ b/docs/ref/signals.txt @@ -448,6 +448,18 @@ request_finished Sent when Django finishes processing an HTTP request. +.. note:: + + When a view returns a :ref:`streaming response `, + this signal is sent only after the entire response is consumed by the + client (strictly speaking, by the WSGI gateway). + +.. versionchanged:: 1.5 + + Before Django 1.5, this signal was fired before sending the content to the + client. In order to accomodate streaming responses, it is now fired after + sending the content. + Arguments sent with this signal: ``sender`` diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index a449f4ab12..c5e8c61922 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -411,6 +411,19 @@ attribute. Developers wishing to access the raw POST data for these cases, should use the :attr:`request.body ` attribute instead. +:data:`~django.core.signals.request_finished` signal +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Django used to send the :data:`~django.core.signals.request_finished` signal +as soon as the view function returned a response. This interacted badly with +:ref:`streaming responses ` that delay content +generation. + +This signal is now sent after the content is fully consumed by the WSGI +gateway. This might be backwards incompatible if you rely on the signal being +fired before sending the response content to the client. If you do, you should +consider using a middleware instead. + OPTIONS, PUT and DELETE requests in the test client ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/regressiontests/handlers/tests.py b/tests/regressiontests/handlers/tests.py index 9cd5816219..3cab2aca57 100644 --- a/tests/regressiontests/handlers/tests.py +++ b/tests/regressiontests/handlers/tests.py @@ -1,10 +1,11 @@ from django.core.handlers.wsgi import WSGIHandler -from django.test import RequestFactory +from django.core import signals +from django.test import RequestFactory, TestCase from django.test.utils import override_settings from django.utils import six -from django.utils import unittest -class HandlerTests(unittest.TestCase): + +class HandlerTests(TestCase): # Mangle settings so the handler will fail @override_settings(MIDDLEWARE_CLASSES=42) @@ -27,3 +28,34 @@ class HandlerTests(unittest.TestCase): handler = WSGIHandler() response = handler(environ, lambda *a, **k: None) self.assertEqual(response.status_code, 400) + + +class SignalsTests(TestCase): + urls = 'regressiontests.handlers.urls' + + def setUp(self): + self.signals = [] + signals.request_started.connect(self.register_started) + signals.request_finished.connect(self.register_finished) + + def tearDown(self): + signals.request_started.disconnect(self.register_started) + signals.request_finished.disconnect(self.register_finished) + + def register_started(self, **kwargs): + self.signals.append('started') + + def register_finished(self, **kwargs): + self.signals.append('finished') + + def test_request_signals(self): + response = self.client.get('/regular/') + self.assertEqual(self.signals, ['started', 'finished']) + self.assertEqual(response.content, b"regular content") + + def test_request_signals_streaming_response(self): + response = self.client.get('/streaming/') + self.assertEqual(self.signals, ['started']) + # Avoid self.assertContains, because it explicitly calls response.close() + self.assertEqual(b''.join(response.streaming_content), b"streaming content") + self.assertEqual(self.signals, ['started', 'finished']) diff --git a/tests/regressiontests/handlers/urls.py b/tests/regressiontests/handlers/urls.py new file mode 100644 index 0000000000..8570f04696 --- /dev/null +++ b/tests/regressiontests/handlers/urls.py @@ -0,0 +1,9 @@ +from __future__ import unicode_literals + +from django.conf.urls import patterns, url +from django.http import HttpResponse, StreamingHttpResponse + +urlpatterns = patterns('', + url(r'^regular/$', lambda request: HttpResponse(b"regular content")), + url(r'^streaming/$', lambda request: StreamingHttpResponse([b"streaming", b" ", b"content"])), +)