Repository: falconry/falcon
Pull request #1417 (2019-01-22 03:29:07)

refactor(Response): Remove stream_len property
This patch "removes" the deprecated stream_len property, fully replacing it's use by the content-length property/header. Along the way, the way API uses content-length (in lieu of stream_len) was optimized.

codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1417?src=pr&el=h1) Report
> Merging [#1417](https://codecov.io/gh/falconry/falcon/pull/1417?src=pr&el=desc) into [master](https://codecov.io/gh/falconry/falcon/commit/11af547518eab28998fecfbde0463645a786442a?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1417/graphs/tree.svg?width=650&token=zOtkfNQ3h6&height=150&src=pr)](https://codecov.io/gh/falconry/falcon/pull/1417?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           master   #1417   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files          38      38           
  Lines        2543    2546    +3     
  Branches      394     394           
======================================
+ Hits         2543    2546    +3
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1417?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/response.py](https://codecov.io/gh/falconry/falcon/pull/1417/diff?src=pr&el=tree#diff-ZmFsY29uL3Jlc3BvbnNlLnB5) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1417?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1417?src=pr&el=footer). Last update [11af547...7786c61](https://codecov.io/gh/falconry/falcon/pull/1417?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

kgriffs: Note: I have temporarily disabled the hug gate. See also: https://github.com/timothycrosley/hug/pull/721

Patch:
diff --git a/.travis.yml b/.travis.yml
index 1e76b3a..68b60f6 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -42,8 +42,10 @@ matrix:
           env: TOXENV=py36_smoke_cython
         - python: 2.7
           env: TOXENV=docs
-        - python: 3.6
-          env: TOXENV=hug
+        # TODO(kgriffs): Re-enable once we have mitigated the stream_len
+        #   breaking change in hug.
+        # - python: 3.6
+        #   env: TOXENV=hug
         - python: 3.6
           env: TOXENV=check_vendored
 
diff --git a/CHANGES.rst b/CHANGES.rst
index 16cd6ee..05016e0 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -16,6 +16,9 @@ Breaking Changes
   ``False``.
 - ``RequestOptions.auto_parse_qs_csv`` now defaults to ``False`` instead of
   ``True``.
+- The deprecated ``stream_len`` property was removed from the ``Response``
+  class. Please use ``Response.set_stream()`` or ``Response.content_length``
+  instead.
 - ``Request.context_type`` was changed from dict to a subclass of dict.
 - ``Response.context_type`` was changed from dict to a subclass of dict.
 - ``JSONHandler`` and ``HTTPError`` no longer use
diff --git a/docs/changes/2.0.0.rst b/docs/changes/2.0.0.rst
index 2f3a445..65b94d1 100644
--- a/docs/changes/2.0.0.rst
+++ b/docs/changes/2.0.0.rst
@@ -16,6 +16,9 @@ Breaking Changes
   instead of ``False``.
 - :attr:`~.RequestOptions.auto_parse_qs_csv` now defaults to ``False``
   instead of ``True``.
+- The deprecated ``stream_len`` property was removed from the
+  :class:`~.Response` class. Please use :meth:`~.Response.set_stream` or
+  :attr:`~.Response.content_length` instead.
 - Request :attr:`~.Request.context_type` was changed from dict to a subclass of
   dict.
 - Response :attr:`~.Response.context_type` was changed from dict to a subclass
diff --git a/falcon/api.py b/falcon/api.py
index a93deb9..e20d825 100644
--- a/falcon/api.py
+++ b/falcon/api.py
@@ -1,3 +1,5 @@
+# -*- coding: utf-8 -*-
+
 # Copyright 2013 by Rackspace Hosting, Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -304,10 +306,11 @@ class API(object):
 
         else:
             body, length = self._get_body(resp, env.get('wsgi.file_wrapper'))
-            if resp.content_length is None and length is not None:
+
+            # PERF(kgriffs): Böse mußt sein. Operate directly on resp._headers
+            #   to reduce overhead since this is a hot/critical code path.
+            if 'content-length' not in resp._headers and length is not None:
                 resp._headers['content-length'] = str(length)
-            elif resp.content_length is not None:
-                resp._headers['content-length'] = str(resp.content_length)
 
         headers = resp._wsgi_headers(media_type)
 
@@ -745,13 +748,15 @@ class API(object):
             The length is returned as ``None`` when unknown. The
             iterable is determined as follows:
 
-                * If resp.body is not ``None``, returns [resp.body],
+                * If resp.body is not ``None``, returns
+                  ([resp.body], len(resp.body)),
                   encoded as UTF-8 if it is a Unicode string.
                   Bytestrings are returned as-is.
-                * If resp.data is not ``None``, returns [resp.data]
+                * If resp.data is not ``None``, returns ([resp.data], len(resp.data))
                 * If resp.stream is not ``None``, returns resp.stream
-                  iterable using wsgi.file_wrapper, if possible.
-                * Otherwise, returns []
+                  iterable using wsgi.file_wrapper, if necessary:
+                  (closeable_iterator, None)
+                * Otherwise, returns ([], 0)
 
         """
         body = resp.body
@@ -782,9 +787,6 @@ class API(object):
             else:
                 iterable = stream
 
-            # NOTE(pshello): resp.stream_len is deprecated in favor of
-            # resp.content_length. The caller of _get_body should give
-            # preference to resp.content_length if it has been set.
-            return iterable, resp.stream_len
+            return iterable, None
 
         return [], 0
diff --git a/falcon/response.py b/falcon/response.py
index 373737e..f36fa0d 100644
--- a/falcon/response.py
+++ b/falcon/response.py
@@ -37,6 +37,11 @@ CookieError = compat.http_cookies.CookieError
 
 GMT_TIMEZONE = TimezoneGMT()
 
+_STREAM_LEN_REMOVED_MSG = (
+    'The deprecated stream_len property was removed in Falcon 2.0. '
+    'Please use Response.set_stream() or Response.content_length instead.'
+)
+
 
 class Response(object):
     """Represents an HTTP response to a client request.
@@ -106,7 +111,9 @@ class Response(object):
                 resource cleanup, it can implement a close() method to do so.
                 The close() method will be called upon completion of the request.
 
-        stream_len (int): Deprecated alias for :py:attr:`content_length`.
+        context (dict): Dictionary to hold any data about the response which is
+            specific to your app. Falcon itself will not interact with this
+            attribute after it has been initialized.
 
         context (object): Empty object to hold any data (in its attributes)
             about the response which is specific to your app (e.g. session
@@ -148,7 +155,6 @@ class Response(object):
         'options',
         'status',
         'stream',
-        'stream_len',
         '_cookies',
         '_data',
         '_headers',
@@ -171,7 +177,6 @@ class Response(object):
 
         self.body = None
         self.stream = None
-        self.stream_len = None
         self._data = None
         self._media = None
 
@@ -228,6 +233,18 @@ class Response(object):
         # just be thrown away.
         self._data = None
 
+    @property
+    def stream_len(self):
+        # NOTE(kgriffs): Provide some additional information by raising the
+        #   error explicitly.
+        raise AttributeError(_STREAM_LEN_REMOVED_MSG)
+
+    @stream_len.setter
+    def stream_len(self, value):
+        # NOTE(kgriffs): We explicitly disallow setting the deprecated attribute
+        #   so that apps relying on it do not fail silently.
+        raise AttributeError(_STREAM_LEN_REMOVED_MSG)
+
     def __repr__(self):
         return '<%s: %s>' % (self.__class__.__name__, self.status)
 
@@ -237,7 +254,8 @@ class Response(object):
         Although the `stream` and `content_length` properties may be set
         directly, using this method ensures `content_length` is not
         accidentally neglected when the length of the stream is known in
-        advance.
+        advance. Using this method is also slightly more performant
+        as compared to setting the properties individually.
 
         Note:
             If the stream length is unknown, you can set `stream`
@@ -252,7 +270,10 @@ class Response(object):
         """
 
         self.stream = stream
-        self.stream_len = content_length  # NOTE(pshello): Deprecated in favor of `content_length`
+
+        # PERF(kgriffs): Set directly rather than incur the overhead of
+        #   the self.content_length property.
+        self._headers['content-length'] = str(content_length)
 
     def set_cookie(self, name, value, expires=None, max_age=None,
                    domain=None, path=None, secure=None, http_only=True):


Test patch:
diff --git a/tests/test_hello.py b/tests/test_hello.py
index 09657e0..1b3956d 100644
--- a/tests/test_hello.py
+++ b/tests/test_hello.py
@@ -59,7 +59,7 @@ class HelloResource(object):
                 resp.set_stream(stream, stream_len)
             else:
                 resp.stream = stream
-                resp.stream_len = stream_len
+                resp.content_length = stream_len
 
         if 'body' in self.mode:
             if 'bytes' in self.mode:
@@ -173,7 +173,7 @@ class TestHelloWorld(object):
         result = client.simulate_get('/stream')
         assert resource.called
 
-        expected_len = resource.resp.stream_len
+        expected_len = int(resource.resp.content_length)
         actual_len = int(result.headers['content-length'])
         assert actual_len == expected_len
         assert len(result.content) == expected_len
@@ -187,7 +187,7 @@ class TestHelloWorld(object):
             result = client.simulate_get('/filelike', file_wrapper=file_wrapper)
             assert resource.called
 
-            expected_len = resource.resp.stream_len
+            expected_len = int(resource.resp.content_length)
             actual_len = int(result.headers['content-length'])
             assert actual_len == expected_len
             assert len(result.content) == expected_len
@@ -196,7 +196,7 @@ class TestHelloWorld(object):
             result = client.simulate_get('/filelike', file_wrapper=file_wrapper)
             assert resource.called
 
-            expected_len = resource.resp.stream_len
+            expected_len = int(resource.resp.content_length)
             actual_len = int(result.headers['content-length'])
             assert actual_len == expected_len
             assert len(result.content) == expected_len
@@ -212,7 +212,7 @@ class TestHelloWorld(object):
         result = client.simulate_get('/filelike-closing', file_wrapper=None)
         assert resource.called
 
-        expected_len = resource.resp.stream_len
+        expected_len = int(resource.resp.content_length)
         actual_len = int(result.headers['content-length'])
         assert actual_len == expected_len
         assert len(result.content) == expected_len
@@ -227,7 +227,7 @@ class TestHelloWorld(object):
         result = client.simulate_get('/filelike-helper')
         assert resource.called
 
-        expected_len = resource.resp.stream_len
+        expected_len = int(resource.resp.content_length)
         actual_len = int(result.headers['content-length'])
         assert actual_len == expected_len
         assert len(result.content) == expected_len
diff --git a/tests/test_response.py b/tests/test_response.py
index 661c99e..80b59b5 100644
--- a/tests/test_response.py
+++ b/tests/test_response.py
@@ -49,3 +49,13 @@ def test_response_attempt_to_set_read_only_headers():
     assert headers['x-things1'] == 'thing-1'
     assert headers['x-things2'] == 'thing-2'
     assert headers['x-things3'] == 'thing-3a, thing-3b'
+
+
+def test_response_removed_stream_len():
+    resp = falcon.Response()
+
+    with pytest.raises(AttributeError):
+        resp.stream_len = 128
+
+    with pytest.raises(AttributeError):
+        resp.stream_len