Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Trigger request_finished in the WSGI iterable's close(). #612

Closed
wants to merge 3 commits into from

2 participants

@aaugustin
Owner

No description provided.

django/test/simple.py
@@ -17,6 +19,10 @@
doctestOutputChecker = OutputChecker()
+# Disconnect the signal that was connected in django.db
+# (imported above) so the database connection stays open.
+signals.request_finished.disconnect(close_connection)
+
@akaariai Collaborator

Should this be done as part of running the tests instead of doing it as side effect of import. I am wondering if somebody could import the django.test.simple without running tests.

@aaugustin Owner

Indeed, that's quite a footgun. I will see if it can be done in a better way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@aaugustin
Owner

Merged today.

@aaugustin aaugustin closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
4 django/core/handlers/wsgi.py
@@ -253,8 +253,8 @@ def __call__(self, environ, start_response):
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]
View
11 django/core/servers/basehttp.py
@@ -109,6 +109,17 @@ def error_output(self, environ, start_response):
super(ServerHandler, self).error_output(environ, start_response)
return ['\n'.join(traceback.format_exception(*sys.exc_info()))]
+ # Backport of http://hg.python.org/cpython/rev/d5af1b235dab. See #16241.
+ # This can be removed when support for Python <= 2.7.3 is deprecated.
+ def finish_response(self):
+ try:
+ if not self.result_is_file() or not self.sendfile():
+ for data in self.result:
+ self.write(data)
+ self.finish_content()
+ finally:
+ self.close()
+
class WSGIServer(simple_server.WSGIServer, object):
"""BaseHTTPServer that implements the Python WSGI protocol"""
View
10 django/http/response.py
@@ -10,6 +10,7 @@
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 @@ def __init__(self, content_type=None, status=None, mimetype=None):
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 @@ def __next__(self):
# 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__)
View
35 django/test/client.py
@@ -26,7 +26,6 @@
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 @@ def write(self, content):
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 @@ def __call__(self, environ):
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
View
8 django/test/utils.py
@@ -4,6 +4,8 @@
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:
View
2  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
=============================
View
12 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``
View
13 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
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
View
38 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 @@ def test_bad_path_info(self):
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'])
View
9 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"])),
+)
Something went wrong with that request. Please try again.