Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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

  • Loading branch information...
commit 278d6a20cc94934b741286437e4b77989a6df2e6 1 parent b5f63cb
Aymeric Augustin authored
4 django/core/handlers/wsgi.py
@@ -253,8 +253,8 @@ def __call__(self, environ, start_response):
253 253 response = http.HttpResponseBadRequest()
254 254 else:
255 255 response = self.get_response(request)
256   - finally:
257   - signals.request_finished.send(sender=self.__class__)
  256 +
  257 + response._handler_class = self.__class__
258 258
259 259 try:
260 260 status_text = STATUS_CODE_TEXT[response.status_code]
5 django/http/response.py
@@ -10,6 +10,7 @@
10 10 from urlparse import urlparse
11 11
12 12 from django.conf import settings
  13 +from django.core import signals
13 14 from django.core import signing
14 15 from django.core.exceptions import SuspiciousOperation
15 16 from django.http.cookie import SimpleCookie
@@ -40,6 +41,9 @@ def __init__(self, content_type=None, status=None, mimetype=None):
40 41 self._headers = {}
41 42 self._charset = settings.DEFAULT_CHARSET
42 43 self._closable_objects = []
  44 + # This parameter is set by the handler. It's necessary to preserve the
  45 + # historical behavior of request_finished.
  46 + self._handler_class = None
43 47 if mimetype:
44 48 warnings.warn("Using mimetype keyword argument is deprecated, use"
45 49 " content_type instead",
@@ -230,6 +234,7 @@ def close(self):
230 234 closable.close()
231 235 except Exception:
232 236 pass
  237 + signals.request_finished.send(sender=self._handler_class)
233 238
234 239 def write(self, content):
235 240 raise Exception("This %s instance is not writable" % self.__class__.__name__)
35 django/test/client.py
@@ -26,7 +26,6 @@
26 26 from django.utils.importlib import import_module
27 27 from django.utils.itercompat import is_iterable
28 28 from django.utils import six
29   -from django.db import close_connection
30 29 from django.test.utils import ContextList
31 30
32 31 __all__ = ('Client', 'RequestFactory', 'encode_file', 'encode_multipart')
@@ -72,6 +71,14 @@ def write(self, content):
72 71 self.__len += len(content)
73 72
74 73
  74 +def closing_iterator_wrapper(iterable, close):
  75 + try:
  76 + for item in iterable:
  77 + yield item
  78 + finally:
  79 + close()
  80 +
  81 +
75 82 class ClientHandler(BaseHandler):
76 83 """
77 84 A HTTP Handler that can be used for testing purposes.
@@ -92,18 +99,20 @@ def __call__(self, environ):
92 99 self.load_middleware()
93 100
94 101 signals.request_started.send(sender=self.__class__)
95   - try:
96   - request = WSGIRequest(environ)
97   - # sneaky little hack so that we can easily get round
98   - # CsrfViewMiddleware. This makes life easier, and is probably
99   - # required for backwards compatibility with external tests against
100   - # admin views.
101   - request._dont_enforce_csrf_checks = not self.enforce_csrf_checks
102   - response = self.get_response(request)
103   - finally:
104   - signals.request_finished.disconnect(close_connection)
105   - signals.request_finished.send(sender=self.__class__)
106   - signals.request_finished.connect(close_connection)
  102 + request = WSGIRequest(environ)
  103 + # sneaky little hack so that we can easily get round
  104 + # CsrfViewMiddleware. This makes life easier, and is probably
  105 + # required for backwards compatibility with external tests against
  106 + # admin views.
  107 + request._dont_enforce_csrf_checks = not self.enforce_csrf_checks
  108 + response = self.get_response(request)
  109 + # We're emulating a WSGI server; we must call the close method
  110 + # on completion.
  111 + if response.streaming:
  112 + response.streaming_content = closing_iterator_wrapper(
  113 + response.streaming_content, response.close)
  114 + else:
  115 + response.close()
107 116
108 117 return response
109 118
8 django/test/utils.py
@@ -4,6 +4,8 @@
4 4
5 5 from django.conf import settings, UserSettingsHolder
6 6 from django.core import mail
  7 +from django.core.signals import request_finished
  8 +from django.db import close_connection
7 9 from django.test.signals import template_rendered, setting_changed
8 10 from django.template import Template, loader, TemplateDoesNotExist
9 11 from django.template.loaders import cached
@@ -68,8 +70,10 @@ def setup_test_environment():
68 70 """Perform any global pre-test setup. This involves:
69 71
70 72 - Installing the instrumented test renderer
71   - - Set the email backend to the locmem email backend.
  73 + - Setting the email backend to the locmem email backend.
72 74 - Setting the active locale to match the LANGUAGE_CODE setting.
  75 + - Disconnecting the request_finished signal to avoid closing
  76 + the database connection within tests.
73 77 """
74 78 Template.original_render = Template._render
75 79 Template._render = instrumented_test_render
@@ -81,6 +85,8 @@ def setup_test_environment():
81 85
82 86 deactivate()
83 87
  88 + request_finished.disconnect(close_connection)
  89 +
84 90
85 91 def teardown_test_environment():
86 92 """Perform any global post-test teardown. This involves:
2  docs/ref/request-response.txt
@@ -790,6 +790,8 @@ types of HTTP responses. Like ``HttpResponse``, these subclasses live in
790 790 :class:`~django.template.response.SimpleTemplateResponse`, and the
791 791 ``render`` method must itself return a valid response object.
792 792
  793 +.. _httpresponse-streaming:
  794 +
793 795 StreamingHttpResponse objects
794 796 =============================
795 797
12 docs/ref/signals.txt
@@ -448,6 +448,18 @@ request_finished
448 448
449 449 Sent when Django finishes processing an HTTP request.
450 450
  451 +.. note::
  452 +
  453 + When a view returns a :ref:`streaming response <httpresponse-streaming>`,
  454 + this signal is sent only after the entire response is consumed by the
  455 + client (strictly speaking, by the WSGI gateway).
  456 +
  457 +.. versionchanged:: 1.5
  458 +
  459 + Before Django 1.5, this signal was fired before sending the content to the
  460 + client. In order to accomodate streaming responses, it is now fired after
  461 + sending the content.
  462 +
451 463 Arguments sent with this signal:
452 464
453 465 ``sender``
13 docs/releases/1.5.txt
@@ -411,6 +411,19 @@ attribute. Developers wishing to access the raw POST data for these cases,
411 411 should use the :attr:`request.body <django.http.HttpRequest.body>` attribute
412 412 instead.
413 413
  414 +:data:`~django.core.signals.request_finished` signal
  415 +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  416 +
  417 +Django used to send the :data:`~django.core.signals.request_finished` signal
  418 +as soon as the view function returned a response. This interacted badly with
  419 +:ref:`streaming responses <httpresponse-streaming>` that delay content
  420 +generation.
  421 +
  422 +This signal is now sent after the content is fully consumed by the WSGI
  423 +gateway. This might be backwards incompatible if you rely on the signal being
  424 +fired before sending the response content to the client. If you do, you should
  425 +consider using a middleware instead.
  426 +
414 427 OPTIONS, PUT and DELETE requests in the test client
415 428 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
416 429
38 tests/regressiontests/handlers/tests.py
... ... @@ -1,10 +1,11 @@
1 1 from django.core.handlers.wsgi import WSGIHandler
2   -from django.test import RequestFactory
  2 +from django.core import signals
  3 +from django.test import RequestFactory, TestCase
3 4 from django.test.utils import override_settings
4 5 from django.utils import six
5   -from django.utils import unittest
6 6
7   -class HandlerTests(unittest.TestCase):
  7 +
  8 +class HandlerTests(TestCase):
8 9
9 10 # Mangle settings so the handler will fail
10 11 @override_settings(MIDDLEWARE_CLASSES=42)
@@ -27,3 +28,34 @@ def test_bad_path_info(self):
27 28 handler = WSGIHandler()
28 29 response = handler(environ, lambda *a, **k: None)
29 30 self.assertEqual(response.status_code, 400)
  31 +
  32 +
  33 +class SignalsTests(TestCase):
  34 + urls = 'regressiontests.handlers.urls'
  35 +
  36 + def setUp(self):
  37 + self.signals = []
  38 + signals.request_started.connect(self.register_started)
  39 + signals.request_finished.connect(self.register_finished)
  40 +
  41 + def tearDown(self):
  42 + signals.request_started.disconnect(self.register_started)
  43 + signals.request_finished.disconnect(self.register_finished)
  44 +
  45 + def register_started(self, **kwargs):
  46 + self.signals.append('started')
  47 +
  48 + def register_finished(self, **kwargs):
  49 + self.signals.append('finished')
  50 +
  51 + def test_request_signals(self):
  52 + response = self.client.get('/regular/')
  53 + self.assertEqual(self.signals, ['started', 'finished'])
  54 + self.assertEqual(response.content, b"regular content")
  55 +
  56 + def test_request_signals_streaming_response(self):
  57 + response = self.client.get('/streaming/')
  58 + self.assertEqual(self.signals, ['started'])
  59 + # Avoid self.assertContains, because it explicitly calls response.close()
  60 + self.assertEqual(b''.join(response.streaming_content), b"streaming content")
  61 + self.assertEqual(self.signals, ['started', 'finished'])
9 tests/regressiontests/handlers/urls.py
... ... @@ -0,0 +1,9 @@
  1 +from __future__ import unicode_literals
  2 +
  3 +from django.conf.urls import patterns, url
  4 +from django.http import HttpResponse, StreamingHttpResponse
  5 +
  6 +urlpatterns = patterns('',
  7 + url(r'^regular/$', lambda request: HttpResponse(b"regular content")),
  8 + url(r'^streaming/$', lambda request: StreamingHttpResponse([b"streaming", b" ", b"content"])),
  9 +)

0 comments on commit 278d6a2

Please sign in to comment.
Something went wrong with that request. Please try again.