From 4b27813198ae31892f1159d437e492f7745761a0 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sat, 20 Oct 2012 17:40:14 +0200 Subject: [PATCH] Fixed #7581 -- Added streaming responses. Thanks mrmachine and everyone else involved on this long-standing ticket. --- django/http/__init__.py | 178 ++++++++++++++++---- django/http/utils.py | 12 +- django/middleware/common.py | 16 +- django/middleware/gzip.py | 24 ++- django/middleware/http.py | 2 +- django/test/testcases.py | 4 +- django/utils/cache.py | 3 +- django/utils/text.py | 31 ++++ django/views/generic/base.py | 2 +- django/views/static.py | 6 +- docs/ref/request-response.txt | 87 +++++++++- docs/releases/1.5.txt | 18 ++ docs/topics/http/middleware.txt | 17 ++ tests/regressiontests/cache/tests.py | 26 ++- tests/regressiontests/httpwrappers/abc.txt | 1 + tests/regressiontests/httpwrappers/tests.py | 111 +++++++++++- tests/regressiontests/middleware/tests.py | 43 ++++- tests/regressiontests/views/tests/static.py | 27 ++- 18 files changed, 533 insertions(+), 75 deletions(-) create mode 100644 tests/regressiontests/httpwrappers/abc.txt diff --git a/django/http/__init__.py b/django/http/__init__.py index b67c182c37..49acd57af3 100644 --- a/django/http/__init__.py +++ b/django/http/__init__.py @@ -528,18 +528,23 @@ def parse_cookie(cookie): class BadHeaderError(ValueError): pass -class HttpResponse(object): - """A basic HTTP response, with content and dictionary-accessed headers.""" +class HttpResponseBase(object): + """ + An HTTP response base class with dictionary-accessed headers. + + This class doesn't handle content. It should not be used directly. + Use the HttpResponse and StreamingHttpResponse subclasses instead. + """ status_code = 200 - def __init__(self, content='', content_type=None, status=None, - mimetype=None): + def __init__(self, content_type=None, status=None, mimetype=None): # _headers is a mapping of the lower-case name to the original case of # the header (required for working with legacy systems) and the header # value. Both the name of the header and its value are ASCII strings. self._headers = {} self._charset = settings.DEFAULT_CHARSET + self._closable_objects = [] if mimetype: warnings.warn("Using mimetype keyword argument is deprecated, use" " content_type instead", PendingDeprecationWarning) @@ -547,26 +552,24 @@ class HttpResponse(object): if not content_type: content_type = "%s; charset=%s" % (settings.DEFAULT_CONTENT_TYPE, self._charset) - # content is a bytestring. See the content property methods. - self.content = content self.cookies = SimpleCookie() if status: self.status_code = status self['Content-Type'] = content_type - def serialize(self): - """Full HTTP message, including headers, as a bytestring.""" + def serialize_headers(self): + """HTTP headers as a bytestring.""" headers = [ ('%s: %s' % (key, value)).encode('us-ascii') for key, value in self._headers.values() ] - return b'\r\n'.join(headers) + b'\r\n\r\n' + self.content + return b'\r\n'.join(headers) if six.PY3: - __bytes__ = serialize + __bytes__ = serialize_headers else: - __str__ = serialize + __str__ = serialize_headers def _convert_to_charset(self, value, charset, mime_encode=False): """Converts headers key/value to ascii/latin1 native strings. @@ -690,24 +693,75 @@ class HttpResponse(object): self.set_cookie(key, max_age=0, path=path, domain=domain, expires='Thu, 01-Jan-1970 00:00:00 GMT') + # Common methods used by subclasses + + def make_bytes(self, value): + """Turn a value into a bytestring encoded in the output charset.""" + # For backwards compatibility, this method supports values that are + # unlikely to occur in real applications. It has grown complex and + # should be refactored. It also overlaps __next__. See #18796. + if self.has_header('Content-Encoding'): + if isinstance(value, int): + value = six.text_type(value) + if isinstance(value, six.text_type): + value = value.encode('ascii') + # force conversion to bytes in case chunk is a subclass + return bytes(value) + else: + return force_bytes(value, self._charset) + + # These methods partially implement the file-like object interface. + # See http://docs.python.org/lib/bltin-file-objects.html + + # The WSGI server must call this method upon completion of the request. + # 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() + + def write(self, content): + raise Exception("This %s instance is not writable" % self.__class__.__name__) + + def flush(self): + pass + + def tell(self): + raise Exception("This %s instance cannot tell its position" % self.__class__.__name__) + +class HttpResponse(HttpResponseBase): + """ + An HTTP response class with a string as content. + + This content that can be read, appended to or replaced. + """ + + streaming = False + + def __init__(self, content='', *args, **kwargs): + super(HttpResponse, self).__init__(*args, **kwargs) + # Content is a bytestring. See the `content` property methods. + self.content = content + + def serialize(self): + """Full HTTP message, including headers, as a bytestring.""" + return self.serialize_headers() + b'\r\n\r\n' + self.content + + if six.PY3: + __bytes__ = serialize + else: + __str__ = serialize + @property def content(self): - if self.has_header('Content-Encoding'): - def make_bytes(value): - if isinstance(value, int): - value = six.text_type(value) - if isinstance(value, six.text_type): - value = value.encode('ascii') - # force conversion to bytes in case chunk is a subclass - return bytes(value) - return b''.join(make_bytes(e) for e in self._container) - return b''.join(force_bytes(e, self._charset) for e in self._container) + return b''.join(self.make_bytes(e) for e in self._container) @content.setter def content(self, value): if hasattr(value, '__iter__') and not isinstance(value, (bytes, six.string_types)): self._container = value self._base_content_is_iter = True + if hasattr(value, 'close'): + self._closable_objects.append(value) else: self._container = [value] self._base_content_is_iter = False @@ -727,25 +781,85 @@ class HttpResponse(object): next = __next__ # Python 2 compatibility - def close(self): - if hasattr(self._container, 'close'): - self._container.close() - - # The remaining methods partially implement the file-like object interface. - # See http://docs.python.org/lib/bltin-file-objects.html def write(self, content): if self._base_content_is_iter: - raise Exception("This %s instance is not writable" % self.__class__) + raise Exception("This %s instance is not writable" % self.__class__.__name__) self._container.append(content) - def flush(self): - pass - def tell(self): if self._base_content_is_iter: - raise Exception("This %s instance cannot tell its position" % self.__class__) + raise Exception("This %s instance cannot tell its position" % self.__class__.__name__) return sum([len(chunk) for chunk in self]) +class StreamingHttpResponse(HttpResponseBase): + """ + A streaming HTTP response class with an iterator as content. + + This should only be iterated once, when the response is streamed to the + client. However, it can be appended to or replaced with a new iterator + that wraps the original content (or yields entirely new content). + """ + + streaming = True + + def __init__(self, streaming_content=(), *args, **kwargs): + super(StreamingHttpResponse, self).__init__(*args, **kwargs) + # `streaming_content` should be an iterable of bytestrings. + # See the `streaming_content` property methods. + self.streaming_content = streaming_content + + @property + def content(self): + raise AttributeError("This %s instance has no `content` attribute. " + "Use `streaming_content` instead." % self.__class__.__name__) + + @property + def streaming_content(self): + return self._iterator + + @streaming_content.setter + def streaming_content(self, value): + # Ensure we can never iterate on "value" more than once. + self._iterator = iter(value) + if hasattr(value, 'close'): + self._closable_objects.append(value) + + def __iter__(self): + return self + + def __next__(self): + return self.make_bytes(next(self._iterator)) + + next = __next__ # Python 2 compatibility + +class CompatibleStreamingHttpResponse(StreamingHttpResponse): + """ + This class maintains compatibility with middleware that doesn't know how + to handle the content of a streaming response by exposing a `content` + attribute that will consume and cache the content iterator when accessed. + + These responses will stream only if no middleware attempts to access the + `content` attribute. Otherwise, they will behave like a regular response, + and raise a `PendingDeprecationWarning`. + """ + @property + def content(self): + warnings.warn( + 'Accessing the `content` attribute on a streaming response is ' + 'deprecated. Use the `streaming_content` attribute instead.', + PendingDeprecationWarning) + content = b''.join(self) + self.streaming_content = [content] + return content + + @content.setter + def content(self, content): + warnings.warn( + 'Accessing the `content` attribute on a streaming response is ' + 'deprecated. Use the `streaming_content` attribute instead.', + PendingDeprecationWarning) + self.streaming_content = [content] + class HttpResponseRedirectBase(HttpResponse): allowed_schemes = ['http', 'https', 'ftp'] diff --git a/django/http/utils.py b/django/http/utils.py index 01808648ba..f7ff477f09 100644 --- a/django/http/utils.py +++ b/django/http/utils.py @@ -26,10 +26,16 @@ def conditional_content_removal(request, response): responses. Ensures compliance with RFC 2616, section 4.3. """ if 100 <= response.status_code < 200 or response.status_code in (204, 304): - response.content = '' - response['Content-Length'] = 0 + if response.streaming: + response.streaming_content = [] + else: + response.content = '' + response['Content-Length'] = '0' if request.method == 'HEAD': - response.content = '' + if response.streaming: + response.streaming_content = [] + else: + response.content = '' return response def fix_IE_for_attach(request, response): diff --git a/django/middleware/common.py b/django/middleware/common.py index 0ec17fbe92..6fbbf43044 100644 --- a/django/middleware/common.py +++ b/django/middleware/common.py @@ -113,14 +113,18 @@ class CommonMiddleware(object): if settings.USE_ETAGS: if response.has_header('ETag'): etag = response['ETag'] + elif response.streaming: + etag = None else: etag = '"%s"' % hashlib.md5(response.content).hexdigest() - if response.status_code >= 200 and response.status_code < 300 and request.META.get('HTTP_IF_NONE_MATCH') == etag: - cookies = response.cookies - response = http.HttpResponseNotModified() - response.cookies = cookies - else: - response['ETag'] = etag + if etag is not None: + if (200 <= response.status_code < 300 + and request.META.get('HTTP_IF_NONE_MATCH') == etag): + cookies = response.cookies + response = http.HttpResponseNotModified() + response.cookies = cookies + else: + response['ETag'] = etag return response diff --git a/django/middleware/gzip.py b/django/middleware/gzip.py index 69f938cf0a..fb54501a03 100644 --- a/django/middleware/gzip.py +++ b/django/middleware/gzip.py @@ -1,6 +1,6 @@ import re -from django.utils.text import compress_string +from django.utils.text import compress_sequence, compress_string from django.utils.cache import patch_vary_headers re_accepts_gzip = re.compile(r'\bgzip\b') @@ -13,7 +13,7 @@ class GZipMiddleware(object): """ def process_response(self, request, response): # It's not worth attempting to compress really short responses. - if len(response.content) < 200: + if not response.streaming and len(response.content) < 200: return response patch_vary_headers(response, ('Accept-Encoding',)) @@ -32,15 +32,21 @@ class GZipMiddleware(object): if not re_accepts_gzip.search(ae): return response - # Return the compressed content only if it's actually shorter. - compressed_content = compress_string(response.content) - if len(compressed_content) >= len(response.content): - return response + if response.streaming: + # Delete the `Content-Length` header for streaming content, because + # we won't know the compressed size until we stream it. + response.streaming_content = compress_sequence(response.streaming_content) + del response['Content-Length'] + else: + # Return the compressed content only if it's actually shorter. + compressed_content = compress_string(response.content) + if len(compressed_content) >= len(response.content): + return response + response.content = compressed_content + response['Content-Length'] = str(len(response.content)) if response.has_header('ETag'): response['ETag'] = re.sub('"$', ';gzip"', response['ETag']) - - response.content = compressed_content response['Content-Encoding'] = 'gzip' - response['Content-Length'] = str(len(response.content)) + return response diff --git a/django/middleware/http.py b/django/middleware/http.py index 86e46cea82..5a46e04946 100644 --- a/django/middleware/http.py +++ b/django/middleware/http.py @@ -10,7 +10,7 @@ class ConditionalGetMiddleware(object): """ def process_response(self, request, response): response['Date'] = http_date() - if not response.has_header('Content-Length'): + if not response.streaming and not response.has_header('Content-Length'): response['Content-Length'] = str(len(response.content)) if response.has_header('ETag'): diff --git a/django/test/testcases.py b/django/test/testcases.py index 1d52fed69f..cfa2cde643 100644 --- a/django/test/testcases.py +++ b/django/test/testcases.py @@ -596,7 +596,9 @@ class TransactionTestCase(SimpleTestCase): msg_prefix + "Couldn't retrieve content: Response code was %d" " (expected %d)" % (response.status_code, status_code)) text = force_text(text, encoding=response._charset) - content = response.content.decode(response._charset) + content = b''.join(response).decode(response._charset) + # Avoid ResourceWarning about unclosed files. + response.close() if html: content = assert_and_parse_html(self, content, None, "Response's content is not valid HTML:") diff --git a/django/utils/cache.py b/django/utils/cache.py index 91c4796988..0fceaa96e6 100644 --- a/django/utils/cache.py +++ b/django/utils/cache.py @@ -95,7 +95,8 @@ def get_max_age(response): pass def _set_response_etag(response): - response['ETag'] = '"%s"' % hashlib.md5(response.content).hexdigest() + if not response.streaming: + response['ETag'] = '"%s"' % hashlib.md5(response.content).hexdigest() return response def patch_response_headers(response, cache_timeout=None): diff --git a/django/utils/text.py b/django/utils/text.py index c19708458b..d75ca8dbca 100644 --- a/django/utils/text.py +++ b/django/utils/text.py @@ -288,6 +288,37 @@ def compress_string(s): zfile.close() return zbuf.getvalue() +class StreamingBuffer(object): + def __init__(self): + self.vals = [] + + def write(self, val): + self.vals.append(val) + + def read(self): + ret = b''.join(self.vals) + self.vals = [] + return ret + + def flush(self): + return + + def close(self): + return + +# Like compress_string, but for iterators of strings. +def compress_sequence(sequence): + buf = StreamingBuffer() + zfile = GzipFile(mode='wb', compresslevel=6, fileobj=buf) + # Output headers... + yield buf.read() + for item in sequence: + zfile.write(item) + zfile.flush() + yield buf.read() + zfile.close() + yield buf.read() + ustring_re = re.compile("([\u0080-\uffff])") def javascript_quote(s, quote_double_quotes=False): diff --git a/django/views/generic/base.py b/django/views/generic/base.py index d2349e1fca..23e18c54a0 100644 --- a/django/views/generic/base.py +++ b/django/views/generic/base.py @@ -99,7 +99,7 @@ class View(object): """ response = http.HttpResponse() response['Allow'] = ', '.join(self._allowed_methods()) - response['Content-Length'] = 0 + response['Content-Length'] = '0' return response def _allowed_methods(self): diff --git a/django/views/static.py b/django/views/static.py index 7dd44c5772..f61ba28bd5 100644 --- a/django/views/static.py +++ b/django/views/static.py @@ -14,7 +14,8 @@ try: except ImportError: # Python 2 from urllib import unquote -from django.http import Http404, HttpResponse, HttpResponseRedirect, HttpResponseNotModified +from django.http import (CompatibleStreamingHttpResponse, Http404, + HttpResponse, HttpResponseRedirect, HttpResponseNotModified) from django.template import loader, Template, Context, TemplateDoesNotExist from django.utils.http import http_date, parse_http_date from django.utils.translation import ugettext as _, ugettext_noop @@ -62,8 +63,7 @@ def serve(request, path, document_root=None, show_indexes=False): if not was_modified_since(request.META.get('HTTP_IF_MODIFIED_SINCE'), statobj.st_mtime, statobj.st_size): return HttpResponseNotModified() - with open(fullpath, 'rb') as f: - response = HttpResponse(f.read(), content_type=mimetype) + response = CompatibleStreamingHttpResponse(open(fullpath, 'rb'), content_type=mimetype) response["Last-Modified"] = http_date(statobj.st_mtime) if stat.S_ISREG(statobj.st_mode): response["Content-Length"] = statobj.st_size diff --git a/docs/ref/request-response.txt b/docs/ref/request-response.txt index d7266f0aff..89d0fe847c 100644 --- a/docs/ref/request-response.txt +++ b/docs/ref/request-response.txt @@ -566,13 +566,21 @@ file-like object:: Passing iterators ~~~~~~~~~~~~~~~~~ -Finally, you can pass ``HttpResponse`` an iterator rather than passing it -hard-coded strings. If you use this technique, follow these guidelines: +Finally, you can pass ``HttpResponse`` an iterator rather than strings. If you +use this technique, the iterator should return strings. -* The iterator should return strings. -* If an :class:`HttpResponse` has been initialized with an iterator as its - content, you can't use the :class:`HttpResponse` instance as a file-like - object. Doing so will raise ``Exception``. +.. versionchanged:: 1.5 + + Passing an iterator as content to :class:`HttpResponse` creates a + streaming response if (and only if) no middleware accesses the + :attr:`HttpResponse.content` attribute before the response is returned. + + If you want to guarantee that your response will stream to the client, you + should use the new :class:`StreamingHttpResponse` class instead. + +If an :class:`HttpResponse` instance has been initialized with an iterator as +its content, you can't use it as a file-like object. Doing so will raise an +exception. Setting headers ~~~~~~~~~~~~~~~ @@ -614,6 +622,13 @@ Attributes The `HTTP Status code`_ for the response. +.. attribute:: HttpResponse.streaming + + This is always ``False``. + + This attribute exists so middleware can treat streaming responses + differently from regular responses. + Methods ------- @@ -781,3 +796,63 @@ types of HTTP responses. Like ``HttpResponse``, these subclasses live in method, Django will treat it as emulating a :class:`~django.template.response.SimpleTemplateResponse`, and the ``render`` method must itself return a valid response object. + +StreamingHttpResponse objects +============================= + +.. versionadded:: 1.5 + +.. class:: StreamingHttpResponse + +The :class:`StreamingHttpResponse` class is used to stream a response from +Django to the browser. You might want to do this if generating the response +takes too long or uses too much memory. For instance, it's useful for +generating large CSV files. + +.. admonition:: Performance considerations + + Django is designed for short-lived requests. Streaming responses will tie + a worker process and keep a database connection idle in transaction for + the entire duration of the response. This may result in poor performance. + + Generally speaking, you should perform expensive tasks outside of the + request-response cycle, rather than resorting to a streamed response. + +The :class:`StreamingHttpResponse` is not a subclass of :class:`HttpResponse`, +because it features a slightly different API. However, it is almost identical, +with the following notable differences: + +* It should be given an iterator that yields strings as content. + +* You cannot access its content, except by iterating the response object + itself. This should only occur when the response is returned to the client. + +* It has no ``content`` attribute. Instead, it has a + :attr:`~StreamingHttpResponse.streaming_content` attribute. + +* You cannot use the file-like object ``tell()`` or ``write()`` methods. + Doing so will raise an exception. + +* Any iterators that have a ``close()`` method and are assigned as content will + be closed automatically after the response has been iterated. + +:class:`StreamingHttpResponse` should only be used in situations where it is +absolutely required that the whole content isn't iterated before transferring +the data to the client. Because the content can't be accessed, many +middlewares can't function normally. For example the ``ETag`` and ``Content- +Length`` headers can't be generated for streaming responses. + +Attributes +---------- + +.. attribute:: StreamingHttpResponse.streaming_content + + An iterator of strings representing the content. + +.. attribute:: HttpResponse.status_code + + The `HTTP Status code`_ for the response. + +.. attribute:: HttpResponse.streaming + + This is always ``True``. diff --git a/docs/releases/1.5.txt b/docs/releases/1.5.txt index d30bd5ff7e..f7467bc06a 100644 --- a/docs/releases/1.5.txt +++ b/docs/releases/1.5.txt @@ -84,6 +84,24 @@ For one-to-one relationships, both sides can be cached. For many-to-one relationships, only the single side of the relationship can be cached. This is particularly helpful in combination with ``prefetch_related``. +Explicit support for streaming responses +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Before Django 1.5, it was possible to create a streaming response by passing +an iterator to :class:`~django.http.HttpResponse`. But this was unreliable: +any middleware that accessed the :attr:`~django.http.HttpResponse.content` +attribute would consume the iterator prematurely. + +You can now explicitly generate a streaming response with the new +:class:`~django.http.StreamingHttpResponse` class. This class exposes a +:class:`~django.http.StreamingHttpResponse.streaming_content` attribute which +is an iterator. + +Since :class:`~django.http.StreamingHttpResponse` does not have a ``content`` +attribute, middleware that need access to the response content must test for +streaming responses and behave accordingly. See :ref:`response-middleware` for +more information. + ``{% verbatim %}`` template tag ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/topics/http/middleware.txt b/docs/topics/http/middleware.txt index a8347e52a0..c27e7e8690 100644 --- a/docs/topics/http/middleware.txt +++ b/docs/topics/http/middleware.txt @@ -164,6 +164,23 @@ an earlier middleware method returned an :class:`~django.http.HttpResponse` classes are applied in reverse order, from the bottom up. This means classes defined at the end of :setting:`MIDDLEWARE_CLASSES` will be run first. +.. versionchanged:: 1.5 + ``response`` may also be an :class:`~django.http.StreamingHttpResponse` + object. + +Unlike :class:`~django.http.HttpResponse`, +:class:`~django.http.StreamingHttpResponse` does not have a ``content`` +attribute. As a result, middleware can no longer assume that all responses +will have a ``content`` attribute. If they need access to the content, they +must test for streaming responses and adjust their behavior accordingly:: + + if response.streaming: + response.streaming_content = wrap_streaming_content(response.streaming_content) + else: + response.content = wrap_content(response.content) + +``streaming_content`` should be assumed to be too large to hold in memory. +Middleware may wrap it in a new generator, but must not consume it. .. _exception-middleware: diff --git a/tests/regressiontests/cache/tests.py b/tests/regressiontests/cache/tests.py index de27bc9476..a6eff8950b 100644 --- a/tests/regressiontests/cache/tests.py +++ b/tests/regressiontests/cache/tests.py @@ -19,7 +19,8 @@ from django.core.cache import get_cache from django.core.cache.backends.base import (CacheKeyWarning, InvalidCacheBackendError) from django.db import router -from django.http import HttpResponse, HttpRequest, QueryDict +from django.http import (HttpResponse, HttpRequest, StreamingHttpResponse, + QueryDict) from django.middleware.cache import (FetchFromCacheMiddleware, UpdateCacheMiddleware, CacheMiddleware) from django.template import Template @@ -1416,6 +1417,29 @@ class CacheI18nTest(TestCase): # reset the language translation.deactivate() + @override_settings( + CACHE_MIDDLEWARE_KEY_PREFIX="test", + CACHE_MIDDLEWARE_SECONDS=60, + USE_ETAGS=True, + ) + def test_middleware_with_streaming_response(self): + # cache with non empty request.GET + request = self._get_request_cache(query_string='foo=baz&other=true') + + # first access, cache must return None + get_cache_data = FetchFromCacheMiddleware().process_request(request) + self.assertEqual(get_cache_data, None) + + # pass streaming response through UpdateCacheMiddleware. + content = 'Check for cache with QUERY_STRING and streaming content' + response = StreamingHttpResponse(content) + UpdateCacheMiddleware().process_response(request, response) + + # second access, cache must still return None, because we can't cache + # streaming response. + get_cache_data = FetchFromCacheMiddleware().process_request(request) + self.assertEqual(get_cache_data, None) + @override_settings( CACHES={ diff --git a/tests/regressiontests/httpwrappers/abc.txt b/tests/regressiontests/httpwrappers/abc.txt new file mode 100644 index 0000000000..6bac42b3ad --- /dev/null +++ b/tests/regressiontests/httpwrappers/abc.txt @@ -0,0 +1 @@ +random content diff --git a/tests/regressiontests/httpwrappers/tests.py b/tests/regressiontests/httpwrappers/tests.py index 4c6aed1b97..bfb4ae1fd5 100644 --- a/tests/regressiontests/httpwrappers/tests.py +++ b/tests/regressiontests/httpwrappers/tests.py @@ -2,12 +2,13 @@ from __future__ import unicode_literals import copy +import os import pickle from django.core.exceptions import SuspiciousOperation from django.http import (QueryDict, HttpResponse, HttpResponseRedirect, HttpResponsePermanentRedirect, HttpResponseNotAllowed, - HttpResponseNotModified, + HttpResponseNotModified, StreamingHttpResponse, SimpleCookie, BadHeaderError, parse_cookie) from django.test import TestCase @@ -351,7 +352,6 @@ class HttpResponseTests(unittest.TestCase): self.assertRaises(SuspiciousOperation, HttpResponsePermanentRedirect, url) - class HttpResponseSubclassesTests(TestCase): def test_redirect(self): response = HttpResponseRedirect('/redirected/') @@ -379,6 +379,113 @@ class HttpResponseSubclassesTests(TestCase): content_type='text/html') self.assertContains(response, 'Only the GET method is allowed', status_code=405) +class StreamingHttpResponseTests(TestCase): + def test_streaming_response(self): + r = StreamingHttpResponse(iter(['hello', 'world'])) + + # iterating over the response itself yields bytestring chunks. + chunks = list(r) + self.assertEqual(chunks, [b'hello', b'world']) + for chunk in chunks: + self.assertIsInstance(chunk, six.binary_type) + + # and the response can only be iterated once. + self.assertEqual(list(r), []) + + # even when a sequence that can be iterated many times, like a list, + # is given as content. + r = StreamingHttpResponse(['abc', 'def']) + self.assertEqual(list(r), [b'abc', b'def']) + self.assertEqual(list(r), []) + + # streaming responses don't have a `content` attribute. + self.assertFalse(hasattr(r, 'content')) + + # and you can't accidentally assign to a `content` attribute. + with self.assertRaises(AttributeError): + r.content = 'xyz' + + # but they do have a `streaming_content` attribute. + self.assertTrue(hasattr(r, 'streaming_content')) + + # that exists so we can check if a response is streaming, and wrap or + # replace the content iterator. + r.streaming_content = iter(['abc', 'def']) + r.streaming_content = (chunk.upper() for chunk in r.streaming_content) + self.assertEqual(list(r), [b'ABC', b'DEF']) + + # coercing a streaming response to bytes doesn't return a complete HTTP + # message like a regular response does. it only gives us the headers. + r = StreamingHttpResponse(iter(['hello', 'world'])) + self.assertEqual( + six.binary_type(r), b'Content-Type: text/html; charset=utf-8') + + # and this won't consume its content. + self.assertEqual(list(r), [b'hello', b'world']) + + # additional content cannot be written to the response. + r = StreamingHttpResponse(iter(['hello', 'world'])) + with self.assertRaises(Exception): + r.write('!') + + # and we can't tell the current position. + with self.assertRaises(Exception): + r.tell() + +class FileCloseTests(TestCase): + def test_response(self): + filename = os.path.join(os.path.dirname(__file__), 'abc.txt') + + # file isn't closed until we close the response. + file1 = open(filename) + r = HttpResponse(file1) + self.assertFalse(file1.closed) + r.close() + self.assertTrue(file1.closed) + + # don't automatically close file when we finish iterating the response. + file1 = open(filename) + r = HttpResponse(file1) + self.assertFalse(file1.closed) + list(r) + self.assertFalse(file1.closed) + r.close() + self.assertTrue(file1.closed) + + # when multiple file are assigned as content, make sure they are all + # closed with the response. + file1 = open(filename) + file2 = open(filename) + r = HttpResponse(file1) + r.content = file2 + self.assertFalse(file1.closed) + self.assertFalse(file2.closed) + r.close() + self.assertTrue(file1.closed) + self.assertTrue(file2.closed) + + def test_streaming_response(self): + filename = os.path.join(os.path.dirname(__file__), 'abc.txt') + + # file isn't closed until we close the response. + file1 = open(filename) + r = StreamingHttpResponse(file1) + self.assertFalse(file1.closed) + r.close() + self.assertTrue(file1.closed) + + # when multiple file are assigned as content, make sure they are all + # closed with the response. + file1 = open(filename) + file2 = open(filename) + r = StreamingHttpResponse(file1) + r.streaming_content = file2 + self.assertFalse(file1.closed) + self.assertFalse(file2.closed) + r.close() + self.assertTrue(file1.closed) + self.assertTrue(file2.closed) + class CookieTests(unittest.TestCase): def test_encode(self): """ diff --git a/tests/regressiontests/middleware/tests.py b/tests/regressiontests/middleware/tests.py index eb66f2bbf3..de901f4a80 100644 --- a/tests/regressiontests/middleware/tests.py +++ b/tests/regressiontests/middleware/tests.py @@ -8,7 +8,7 @@ from io import BytesIO from django.conf import settings from django.core import mail from django.http import HttpRequest -from django.http import HttpResponse +from django.http import HttpResponse, StreamingHttpResponse from django.middleware.clickjacking import XFrameOptionsMiddleware from django.middleware.common import CommonMiddleware from django.middleware.http import ConditionalGetMiddleware @@ -322,6 +322,12 @@ class ConditionalGetMiddlewareTest(TestCase): self.assertTrue('Content-Length' in self.resp) self.assertEqual(int(self.resp['Content-Length']), content_length) + def test_content_length_header_not_added(self): + resp = StreamingHttpResponse('content') + self.assertFalse('Content-Length' in resp) + resp = ConditionalGetMiddleware().process_response(self.req, resp) + self.assertFalse('Content-Length' in resp) + def test_content_length_header_not_changed(self): bad_content_length = len(self.resp.content) + 10 self.resp['Content-Length'] = bad_content_length @@ -351,6 +357,29 @@ class ConditionalGetMiddlewareTest(TestCase): self.resp = ConditionalGetMiddleware().process_response(self.req, self.resp) self.assertEqual(self.resp.status_code, 200) + @override_settings(USE_ETAGS=True) + def test_etag(self): + req = HttpRequest() + res = HttpResponse('content') + self.assertTrue( + CommonMiddleware().process_response(req, res).has_header('ETag')) + + @override_settings(USE_ETAGS=True) + def test_etag_streaming_response(self): + req = HttpRequest() + res = StreamingHttpResponse(['content']) + res['ETag'] = 'tomatoes' + self.assertEqual( + CommonMiddleware().process_response(req, res).get('ETag'), + 'tomatoes') + + @override_settings(USE_ETAGS=True) + def test_no_etag_streaming_response(self): + req = HttpRequest() + res = StreamingHttpResponse(['content']) + self.assertFalse( + CommonMiddleware().process_response(req, res).has_header('ETag')) + # Tests for the Last-Modified header def test_if_modified_since_and_no_last_modified(self): @@ -511,6 +540,7 @@ class GZipMiddlewareTest(TestCase): short_string = b"This string is too short to be worth compressing." compressible_string = b'a' * 500 uncompressible_string = b''.join(six.int2byte(random.randint(0, 255)) for _ in xrange(500)) + sequence = [b'a' * 500, b'b' * 200, b'a' * 300] def setUp(self): self.req = HttpRequest() @@ -525,6 +555,8 @@ class GZipMiddlewareTest(TestCase): self.resp.status_code = 200 self.resp.content = self.compressible_string self.resp['Content-Type'] = 'text/html; charset=UTF-8' + self.stream_resp = StreamingHttpResponse(self.sequence) + self.stream_resp['Content-Type'] = 'text/html; charset=UTF-8' @staticmethod def decompress(gzipped_string): @@ -539,6 +571,15 @@ class GZipMiddlewareTest(TestCase): self.assertEqual(r.get('Content-Encoding'), 'gzip') self.assertEqual(r.get('Content-Length'), str(len(r.content))) + def test_compress_streaming_response(self): + """ + Tests that compression is performed on responses with streaming content. + """ + r = GZipMiddleware().process_response(self.req, self.stream_resp) + self.assertEqual(self.decompress(b''.join(r)), b''.join(self.sequence)) + self.assertEqual(r.get('Content-Encoding'), 'gzip') + self.assertFalse(r.has_header('Content-Length')) + def test_compress_non_200_response(self): """ Tests that compression is performed on responses with a status other than 200. diff --git a/tests/regressiontests/views/tests/static.py b/tests/regressiontests/views/tests/static.py index 38cf38ce46..221244a4a5 100644 --- a/tests/regressiontests/views/tests/static.py +++ b/tests/regressiontests/views/tests/static.py @@ -31,28 +31,35 @@ class StaticTests(TestCase): media_files = ['file.txt', 'file.txt.gz'] for filename in media_files: response = self.client.get('/views/%s/%s' % (self.prefix, filename)) + response_content = b''.join(response) + response.close() file_path = path.join(media_dir, filename) with open(file_path, 'rb') as fp: - self.assertEqual(fp.read(), response.content) - self.assertEqual(len(response.content), int(response['Content-Length'])) + self.assertEqual(fp.read(), response_content) + self.assertEqual(len(response_content), int(response['Content-Length'])) self.assertEqual(mimetypes.guess_type(file_path)[1], response.get('Content-Encoding', None)) def test_unknown_mime_type(self): response = self.client.get('/views/%s/file.unknown' % self.prefix) + response.close() self.assertEqual('application/octet-stream', response['Content-Type']) def test_copes_with_empty_path_component(self): file_name = 'file.txt' response = self.client.get('/views/%s//%s' % (self.prefix, file_name)) + response_content = b''.join(response) + response.close() with open(path.join(media_dir, file_name), 'rb') as fp: - self.assertEqual(fp.read(), response.content) + self.assertEqual(fp.read(), response_content) def test_is_modified_since(self): file_name = 'file.txt' response = self.client.get('/views/%s/%s' % (self.prefix, file_name), HTTP_IF_MODIFIED_SINCE='Thu, 1 Jan 1970 00:00:00 GMT') + response_content = b''.join(response) + response.close() with open(path.join(media_dir, file_name), 'rb') as fp: - self.assertEqual(fp.read(), response.content) + self.assertEqual(fp.read(), response_content) def test_not_modified_since(self): file_name = 'file.txt' @@ -74,9 +81,11 @@ class StaticTests(TestCase): invalid_date = 'Mon, 28 May 999999999999 28:25:26 GMT' response = self.client.get('/views/%s/%s' % (self.prefix, file_name), HTTP_IF_MODIFIED_SINCE=invalid_date) + response_content = b''.join(response) + response.close() with open(path.join(media_dir, file_name), 'rb') as fp: - self.assertEqual(fp.read(), response.content) - self.assertEqual(len(response.content), + self.assertEqual(fp.read(), response_content) + self.assertEqual(len(response_content), int(response['Content-Length'])) def test_invalid_if_modified_since2(self): @@ -89,9 +98,11 @@ class StaticTests(TestCase): invalid_date = ': 1291108438, Wed, 20 Oct 2010 14:05:00 GMT' response = self.client.get('/views/%s/%s' % (self.prefix, file_name), HTTP_IF_MODIFIED_SINCE=invalid_date) + response_content = b''.join(response) + response.close() with open(path.join(media_dir, file_name), 'rb') as fp: - self.assertEqual(fp.read(), response.content) - self.assertEqual(len(response.content), + self.assertEqual(fp.read(), response_content) + self.assertEqual(len(response_content), int(response['Content-Length']))