Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Make `HttpResponse` compatible with unofficially supported streaming …

…behaviour.

- Don't automatically close file-like objects assigned as content to
  `HttpResponse` objects.

- Don't consume iterators assigned as content to `HttpResponse` until `content`
  is accessed, `write() is called, or `tell()` is called.
  • Loading branch information...
commit da7a8dc5cca67aa4c8f148f3779192413cb78777 1 parent 61c1ca0
Tai Lee authored
View
117 django/http/__init__.py
@@ -538,7 +538,7 @@ def __init__(self, content_type=None, status=None, mimetype=None):
# value. Both the name of the header and its value are ASCII strings.
self._headers = {}
self._charset = settings.DEFAULT_CHARSET
- self._iterable_content = []
+ self._file_like_objects = []
if mimetype:
warnings.warn("Using mimetype keyword argument is deprecated, use"
" content_type instead", PendingDeprecationWarning)
@@ -682,48 +682,52 @@ def delete_cookie(self, key, path='/', domain=None):
self.set_cookie(key, max_age=0, path=path, domain=domain,
expires='Thu, 01-Jan-1970 00:00:00 GMT')
- def _set_container(self, value, consume_iterable):
+ def _set_container(self, value, stream):
+ # The `stream` argument should be `True` if the response must be
+ # streamed, in which case we make sure the assigned content can only be
+ # iterated once.
if isinstance(value, collections.Iterable) \
and not isinstance(value, (bytes, six.text_type)):
- if consume_iterable:
- # convert iterable to list, so we can iterate over it many
- # times and append to it. Ensure .content can be accessed
- # multiple times.
- self._container = list(value)
- # close iterable after consuming it.
+ if stream:
+ # Ensure iterable values can only be iterated once by wrapping
+ # them in a new iterator.
+ self._container = iter(value)
+ # Keep a reference to file-like objects, so we can close them
+ # later.
if hasattr(value, 'close') and callable(value.close):
- value.close()
+ self._file_like_objects.append(value)
else:
- # convert sequence to an iterable, so we can only iterate over
- # it once. Ensure .streaming_content can't be accesed but
- # once.
- self._container = iter(value)
- # keep a reference to the iterable, so we can close it later,
- # if necessary (e.g. file objects).
+ # Assign the original iterable directly to `self._container`
+ # instead of converting to a list, in order to preserve the
+ # unofficially supported streaming behaviour, which only works
+ # when `response.content` is not accessed by middleware before
+ # the response itself is iterated.
+ self._container = value
+ # Keep a reference to file-like objects, so we can close them
+ # later.
if hasattr(value, 'close') and callable(value.close):
- self._iterable_content.append(value)
+ self._file_like_objects.append(value)
else:
- # Make sure ._containter is always in consistent format, and that
- # multi/single access to .[streaming_]content is guaranteed to
- # hold.
- if consume_iterable:
- self._container = [value]
- else:
+ # Make sure `self._container` is always in a consistent format, and
+ # that multi/single access to [streaming_]content is guaranteed.
+ if stream:
self._container = iter([value])
+ else:
+ self._container = [value]
def make_bytes(self, value):
- # normalise values we know can be safely encoded as ascii, to text.
+ # Convert integer values to text.
if isinstance(value, int):
value = six.text_type(value)
- # if the value is text, it will need to be encoded.
+ # If the value is text, it will need to be encoded.
if isinstance(value, six.text_type):
- # if the response already has a content-encoding, encode as ascii.
+ # If the response already has a content-encoding, encode as ascii.
if self.has_header('Content-Encoding'):
value = value.encode('ascii')
- # otherwise encode with the character set of the response.
+ # Otherwise encode with the character set of the response.
else:
value = value.encode(self._charset)
- # finally, force conversion to bytes in case value is a subclass.
+ # Finally, force conversion to bytes in case value is a subclass.
return bytes(value)
def __iter__(self):
@@ -731,28 +735,26 @@ def __iter__(self):
return self
def __next__(self):
- try:
- return self.make_bytes(next(self._iterator))
- except StopIteration:
- self.close()
- raise
+ return self.make_bytes(next(self._iterator))
next = __next__ # Python 2 compatibility
def close(self):
- while self._iterable_content:
- self._iterable_content.pop().close()
+ while self._file_like_objects:
+ self._file_like_objects.pop().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):
+ # This should be implemented in subclasses that support the feature.
raise Exception("This %s instance is not writable" % self.__class__)
def flush(self):
pass
def tell(self):
+ # This should be implemented in subclasses that support the feature.
raise Exception(
"This %s instance cannot tell its position" % self.__class__)
@@ -765,7 +767,7 @@ class HttpResponse(HttpResponseBase):
def __init__(self, content='', *args, **kwargs):
super(HttpResponse, self).__init__(*args, **kwargs)
- # content is a bytestring. See the content property methods.
+ # Content is a bytestring. See the `content` property methods.
self.content = content
def serialize(self):
@@ -777,23 +779,31 @@ def serialize(self):
else:
__str__ = serialize
+ def _consume_content(self):
+ # Convert `self._container` to a list, so we can append to it and
+ # iterate it multiple times. Call this first in any method that needs
+ # to iterate or append to the content container.
+ if not isinstance(self._container, list):
+ self._container = list(self._container)
+
@property
def content(self):
- # We must iterate over `self._container` and call `self.make_bytes()`
- # on each chunk instead of just joining on `self`, which does the same
- # thing, to avoid creating an iterator at `self._iterator`. Thus, the
- # response can still be pickled after the content has been accessed,
- # which is required by cache middleware.
+ self._consume_content()
+ # Iterate over `self._container` and call `self.make_bytes()` on each
+ # chunk instead of just joining on `self`, because the new iterator
+ # that would be created can't be pickled by cache middleware.
return b''.join(self.make_bytes(chunk) for chunk in self._container)
@content.setter
def content(self, value):
- self._set_container(value, consume_iterable=True)
+ self._set_container(value, stream=False)
def write(self, content):
+ self._consume_content()
self._container.append(content)
def tell(self):
+ self._consume_content()
return sum([len(chunk) for chunk in self])
class HttpStreamingResponse(HttpResponseBase):
@@ -807,8 +817,8 @@ class HttpStreamingResponse(HttpResponseBase):
def __init__(self, content='', *args, **kwargs):
super(HttpStreamingResponse, self).__init__(*args, **kwargs)
- # streaming_content is an iterator that yields bytestrings.
- # See the streaming_content property methods.
+ # Streaming_content is an iterator that yields bytestrings. See the
+ # `streaming_content` property methods.
self.streaming_content = content
if six.PY3:
@@ -824,15 +834,26 @@ def content(self):
@property
def streaming_content(self):
- # we must iterate over `self._container` and call `self.make_bytes()`
- # on each chunk instead of just returning `iter(self)`, which does the
- # same thing. Thus, we can safely wrap `streaming_content` with a new
- # generator.
+ # Iterate over `self._container` and call `self.make_bytes()` on each
+ # chunk instead of just returning `iter(self)`, so we can safely wrap
+ # `streaming_content` in a new generator without raising a
+ # "ValueError: generator already executing" exception.
return (self.make_bytes(chunk) for chunk in self._container)
@streaming_content.setter
def streaming_content(self, value):
- self._set_container(value, consume_iterable=False)
+ self._set_container(value, stream=True)
+
+ def __next__(self):
+ # Close all file-like objects assigned as content, after the iterator
+ # has been exhausted.
+ try:
+ return self.make_bytes(next(self._iterator))
+ except StopIteration:
+ self.close()
+ raise
+
+ next = __next__ # Python 2 compatibility
class HttpResponseRedirectBase(HttpResponse):
allowed_schemes = ['http', 'https', 'ftp']
View
27 docs/ref/request-response.txt
@@ -565,12 +565,18 @@ strings.
.. versionchanged:: 1.5
-When an iterator is assigned as content to an :class:`HttpResponse`, Django
-will consume the iterator immediately. If you want to stream your response, use
-the new :class:`HttpStreamingResponse` class, instead.
+In previous versions, passing an iterator as content to :class:`HttpResponse`
+would create a streaming response if (and only if) no middleware accessed the
+:attr:`HttpResponse.content` attribute before the response could be returned.
-As a result, you can now use the :class:`HttpResponse` as a file-like object
-even when the content was given as an iterator.
+If you want to guarantee that your response will stream to the client, you
+should use the new :class:`HttpStreamingResponse` class, instead.
+
+.. versionchanged:: 1.5
+
+You can now use the :meth:`HttpResponse.write()` method even when passing an
+iterator as content to :class:`HttpResponse`, because Django will consume and
+cache the iterator on first access.
Setting headers
~~~~~~~~~~~~~~~
@@ -608,10 +614,6 @@ Attributes
A string representing the content, encoded from a Unicode
object if necessary.
- .. note:: If an iterable that has a ``close()`` method is assigned to
- ``content``, its ``close()`` method will be called after the iterator
- has been consumed.
-
.. attribute:: HttpResponse.status_code
The `HTTP Status code`_ for the response.
@@ -816,11 +818,14 @@ identical, with the following notable differences:
* You cannot use the file-like object ``tell()`` or ``write()`` methods.
Doing so will raise ``Exception``.
+* Any iterators that have a ``close()`` method and are assigned as content will
+ be closed automatically after the response has been iterated.
+
Note that :class:`HttpStreamingResponse` 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. Caching
+many middlewares can't function normally. For example the ``ETag`` and
+``Content-Length`` headers can't be generated for streaming responses. Caching
done in middleware will be disabled.
Attributes
View
42 tests/regressiontests/httpwrappers/tests.py
@@ -449,21 +449,59 @@ def test_streaming_response(self):
with self.assertRaises(Exception):
r.tell()
+ def test_response(self):
+ # provide compatibility for unofficially supported streaming behaviour
+ # by not consuming an iterator assigned to HttpResponse as content
+ # until first access of `content`, first call to `write()` or first
+ # call to `tell()`.
+
+ # access content.
+ r = HttpResponse(iter(['hello', 'world']))
+ self.assertNotIsInstance(r._container, list)
+ r.content
+ self.assertIsInstance(r._container, list)
+
+ # call write().
+ r = HttpResponse(iter(['hello', 'world']))
+ self.assertNotIsInstance(r._container, list)
+ r.write('!')
+ self.assertIsInstance(r._container, list)
+
+ # call tell().
+ r = HttpResponse(iter(['hello', 'world']))
+ self.assertNotIsInstance(r._container, list)
+ r.tell()
+ self.assertIsInstance(r._container, list)
+
class FileCloseTests(TestCase):
def test_response(self):
filename = os.path.join(os.path.dirname(__file__), 'abc.txt')
- # file is closed when it is consumed, when it is assigned.
+ # 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.
+ # 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)
Please sign in to comment.
Something went wrong with that request. Please try again.