[1.5.x] Fixed #19519 -- Fired request_finished in the WSGI iterable's close().

Backport of acc5396.

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]

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", PendingDeprecationWarning)
@@ -225,7 +229,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__)

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
 

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:

docs/ref/request-response.txt

@@ -804,6 +804,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
 =============================
 

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 <httpresponse-streaming>`,
+    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``

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 <django.http.HttpRequest.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 <httpresponse-streaming>` 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
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

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'])

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"])),
+)