From c3143d4f4baac03ec02a75e8aa92d67dd9a946ba Mon Sep 17 00:00:00 2001 From: Flaper Fesp Date: Thu, 18 Jul 2013 22:01:03 +0200 Subject: [PATCH 1/4] fix: Encode body and update resp.body The patch sets the encoded body to resp.body so that other functions / calls using it can operate on the updated value, which is the one that will be sent back to the client. fixes #158 --- falcon/api.py | 6 +++++- falcon/api_helpers.py | 6 +++++- falcon/tests/test_after_hooks.py | 9 ++++----- falcon/tests/test_hello.py | 2 +- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/falcon/api.py b/falcon/api.py index 637c9ed..f96c84d 100644 --- a/falcon/api.py +++ b/falcon/api.py @@ -123,8 +123,12 @@ class API(object): # use_body = not helpers.should_ignore_body(resp.status, req.method) if use_body: - helpers.set_content_length(resp) + # get_body must be called before + # set_content_length so that all + # encodings and transformations + # on the body can be applied first. body = helpers.get_body(resp) + helpers.set_content_length(resp) else: # Default: return an empty body body = [] diff --git a/falcon/api_helpers.py b/falcon/api_helpers.py index bbf17fc..fee5b31 100644 --- a/falcon/api_helpers.py +++ b/falcon/api_helpers.py @@ -99,6 +99,9 @@ def set_content_length(resp): def get_body(resp): """Converts resp content into an iterable as required by PEP 333 + Post: + If resp.body is set, it'll be encoded. + Args: resp: Instance of falcon.Response @@ -115,7 +118,8 @@ def get_body(resp): if body is not None: if isinstance(body, six.text_type): - return [body.encode('utf-8')] + resp.body = body.encode('utf-8') + return [resp.body] else: return [body] diff --git a/falcon/tests/test_after_hooks.py b/falcon/tests/test_after_hooks.py index 3849589..17d831b 100644 --- a/falcon/tests/test_after_hooks.py +++ b/falcon/tests/test_after_hooks.py @@ -85,7 +85,7 @@ class TestHooks(testing.TestBase): self.api.add_route(self.test_route, zoo_resource) self.simulate_request(self.test_route) - self.assertEqual('fluffy', zoo_resource.resp.body) + self.assertEqual(b'fluffy', zoo_resource.resp.body) def test_multiple_global_hook(self): self.api = falcon.API(after=[fluffiness, cuteness]) @@ -94,7 +94,7 @@ class TestHooks(testing.TestBase): self.api.add_route(self.test_route, zoo_resource) self.simulate_request(self.test_route) - self.assertEqual('fluffy and cute', zoo_resource.resp.body) + self.assertEqual(b'fluffy and cute', zoo_resource.resp.body) def test_output_validator(self): self.simulate_request(self.test_route) @@ -105,10 +105,10 @@ class TestHooks(testing.TestBase): self.simulate_request(self.test_route, method='PUT') actual_body = self.resource.resp.body - self.assertEqual('{"animal": "falcon"}', actual_body) + self.assertEqual(b'{"animal": "falcon"}', actual_body) def test_wrapped_resource(self): - expected = 'fluffy and cute' + expected = b'fluffy and cute' self.simulate_request('/wrapped') self.assertEqual(falcon.HTTP_200, self.srmock.status) @@ -116,7 +116,6 @@ class TestHooks(testing.TestBase): self.simulate_request('/wrapped', method='HEAD') self.assertEqual(falcon.HTTP_200, self.srmock.status) - self.assertEqual(expected, self.wrapped_resource.resp.body) self.simulate_request('/wrapped', method='POST') self.assertEqual(falcon.HTTP_405, self.srmock.status) diff --git a/falcon/tests/test_hello.py b/falcon/tests/test_hello.py index 878cef7..b1626ef 100644 --- a/falcon/tests/test_hello.py +++ b/falcon/tests/test_hello.py @@ -101,7 +101,7 @@ class TestHelloWorld(testing.TestBase): self.assertEquals(self.srmock.status, self.resource.sample_status) self.assertEquals(resp.status, self.resource.sample_status) - self.assertEquals(resp.body, self.resource.sample_unicode) + self.assertEquals(resp.body, self.resource.sample_utf8) self.assertEquals(body, [self.resource.sample_utf8]) def test_body_bytes(self): From 115d316acda8625fec087c7080aeae609dd6ed29 Mon Sep 17 00:00:00 2001 From: Flaper Fesp Date: Fri, 26 Jul 2013 11:44:04 +0200 Subject: [PATCH 2/4] fix: keep body and make encoding process explicit --- falcon/api.py | 6 +----- falcon/api_helpers.py | 17 ++++----------- falcon/response.py | 37 ++++++++++++++++++++++++++++++-- falcon/tests/test_after_hooks.py | 10 ++++----- falcon/tests/test_hello.py | 4 ++-- 5 files changed, 47 insertions(+), 27 deletions(-) diff --git a/falcon/api.py b/falcon/api.py index f96c84d..637c9ed 100644 --- a/falcon/api.py +++ b/falcon/api.py @@ -123,12 +123,8 @@ class API(object): # use_body = not helpers.should_ignore_body(resp.status, req.method) if use_body: - # get_body must be called before - # set_content_length so that all - # encodings and transformations - # on the body can be applied first. - body = helpers.get_body(resp) helpers.set_content_length(resp) + body = helpers.get_body(resp) else: # Default: return an empty body body = [] diff --git a/falcon/api_helpers.py b/falcon/api_helpers.py index fee5b31..a323196 100644 --- a/falcon/api_helpers.py +++ b/falcon/api_helpers.py @@ -19,8 +19,6 @@ limitations under the License. import re from functools import wraps -import six - from falcon import responders, HTTP_METHODS import falcon.status_codes as status @@ -77,10 +75,10 @@ def set_content_length(resp): content_length = 0 - if resp.body is not None: + if resp.body_encoded is not None: # Since body is assumed to be a byte string (str in Python 2, bytes in # Python 3), figure out the length using standard functions. - content_length = len(resp.body) + content_length = len(resp.body_encoded) elif resp.data is not None: content_length = len(resp.data) elif resp.stream is not None: @@ -99,9 +97,6 @@ def set_content_length(resp): def get_body(resp): """Converts resp content into an iterable as required by PEP 333 - Post: - If resp.body is set, it'll be encoded. - Args: resp: Instance of falcon.Response @@ -114,14 +109,10 @@ def get_body(resp): """ - body = resp.body + body = resp.body_encoded if body is not None: - if isinstance(body, six.text_type): - resp.body = body.encode('utf-8') - return [resp.body] - else: - return [body] + return [body] elif resp.data is not None: return [resp.data] diff --git a/falcon/response.py b/falcon/response.py index 450a0fe..942c611 100644 --- a/falcon/response.py +++ b/falcon/response.py @@ -16,6 +16,8 @@ limitations under the License. """ +import six + from falcon.response_helpers import header_property, format_range from falcon.util import dt_to_http, percent_escape @@ -36,7 +38,8 @@ class Response(object): """ __slots__ = ( - 'body', # Stuff + '_body', # Stuff + '_body_encoded', # Stuff 'data', '_headers', 'status', @@ -55,11 +58,41 @@ class Response(object): self.status = '200 OK' self._headers = {} - self.body = None + self._body = None + self._body_encoded = None self.data = None self.stream = None self.stream_len = None + def _get_body(self): + return self._body + + def _set_body(self, value): + self._body = value + self._body_encoded = None + + # NOTE(flaper87): Lets use a property + # for the body in case its content was + # encoded and then modified. + body = property(_get_body, _set_body) + + @property + def body_encoded(self): + # NOTE(flaper87): Notice this property + # is not thread-safe. If body is modified + # before this property returns, we might + # end up returning None. + body = self._body + if body and not self._body_encoded: + + # NOTE(flaper87): Assume it is an + # encoded str, then check and encode + # if it isn't. + self._body_encoded = body + if isinstance(body, six.text_type): + self._body_encoded = body.encode('utf-8') + return self._body_encoded + def set_header(self, name, value): """Set a header for this response to a given value. diff --git a/falcon/tests/test_after_hooks.py b/falcon/tests/test_after_hooks.py index 17d831b..11dec80 100644 --- a/falcon/tests/test_after_hooks.py +++ b/falcon/tests/test_after_hooks.py @@ -85,7 +85,7 @@ class TestHooks(testing.TestBase): self.api.add_route(self.test_route, zoo_resource) self.simulate_request(self.test_route) - self.assertEqual(b'fluffy', zoo_resource.resp.body) + self.assertEqual(b'fluffy', zoo_resource.resp.body_encoded) def test_multiple_global_hook(self): self.api = falcon.API(after=[fluffiness, cuteness]) @@ -94,17 +94,17 @@ class TestHooks(testing.TestBase): self.api.add_route(self.test_route, zoo_resource) self.simulate_request(self.test_route) - self.assertEqual(b'fluffy and cute', zoo_resource.resp.body) + self.assertEqual(b'fluffy and cute', zoo_resource.resp.body_encoded) def test_output_validator(self): self.simulate_request(self.test_route) self.assertEqual(falcon.HTTP_723, self.srmock.status) - self.assertEqual(None, self.resource.resp.body) + self.assertEqual(None, self.resource.resp.body_encoded) def test_serializer(self): self.simulate_request(self.test_route, method='PUT') - actual_body = self.resource.resp.body + actual_body = self.resource.resp.body_encoded self.assertEqual(b'{"animal": "falcon"}', actual_body) def test_wrapped_resource(self): @@ -112,7 +112,7 @@ class TestHooks(testing.TestBase): self.simulate_request('/wrapped') self.assertEqual(falcon.HTTP_200, self.srmock.status) - self.assertEqual(expected, self.wrapped_resource.resp.body) + self.assertEqual(expected, self.wrapped_resource.resp.body_encoded) self.simulate_request('/wrapped', method='HEAD') self.assertEqual(falcon.HTTP_200, self.srmock.status) diff --git a/falcon/tests/test_hello.py b/falcon/tests/test_hello.py index b1626ef..59b29a8 100644 --- a/falcon/tests/test_hello.py +++ b/falcon/tests/test_hello.py @@ -101,7 +101,7 @@ class TestHelloWorld(testing.TestBase): self.assertEquals(self.srmock.status, self.resource.sample_status) self.assertEquals(resp.status, self.resource.sample_status) - self.assertEquals(resp.body, self.resource.sample_utf8) + self.assertEquals(resp.body_encoded, self.resource.sample_utf8) self.assertEquals(body, [self.resource.sample_utf8]) def test_body_bytes(self): @@ -113,7 +113,7 @@ class TestHelloWorld(testing.TestBase): self.assertEquals(self.srmock.status, self.resource.sample_status) self.assertEquals(resp.status, self.resource.sample_status) - self.assertEquals(resp.body, self.resource.sample_utf8) + self.assertEquals(resp.body_encoded, self.resource.sample_utf8) self.assertEquals(body, [self.resource.sample_utf8]) def test_data(self): From b4f38655149e94c00f5407b7db5b51729c53b3ec Mon Sep 17 00:00:00 2001 From: Flaper Fesp Date: Fri, 2 Aug 2013 19:14:52 +0200 Subject: [PATCH 3/4] Added some docstrings and tests --- AUTHORS | 1 + falcon/response.py | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/AUTHORS b/AUTHORS index 371fa09..57ac3ef 100644 --- a/AUTHORS +++ b/AUTHORS @@ -7,6 +7,7 @@ by date of contribution: * Chad Lung (chadlung) * Josh Brand (joshbrand) * Jamie Painter (painterjd) +* Flavio Percoco (flaper87) * Randall Burt (rs-randallburt) * Zhihao Yuan (lichray) * Ashutosh Das (pyprism) diff --git a/falcon/response.py b/falcon/response.py index 942c611..a9ed6c2 100644 --- a/falcon/response.py +++ b/falcon/response.py @@ -65,9 +65,11 @@ class Response(object): self.stream_len = None def _get_body(self): + """Returns the body as-is.""" return self._body def _set_body(self, value): + """Sets the body and clears the encoded cache.""" self._body = value self._body_encoded = None @@ -78,12 +80,18 @@ class Response(object): @property def body_encoded(self): + """Encode the body and return it + + This property will encode `_body` and + cache the result in the `_body_encoded` + attribute. + """ # NOTE(flaper87): Notice this property # is not thread-safe. If body is modified # before this property returns, we might # end up returning None. body = self._body - if body and not self._body_encoded: + if body and self._body_encoded is None: # NOTE(flaper87): Assume it is an # encoded str, then check and encode @@ -91,6 +99,7 @@ class Response(object): self._body_encoded = body if isinstance(body, six.text_type): self._body_encoded = body.encode('utf-8') + return self._body_encoded def set_header(self, name, value): From 5bdfa775814eb096681425bfe7e113d147fb43cb Mon Sep 17 00:00:00 2001 From: Flaper Fesp Date: Fri, 2 Aug 2013 19:23:11 +0200 Subject: [PATCH 4/4] fix: Added missing tests and rebased fixes #158 --- falcon/tests/test_hello.py | 2 +- falcon/tests/test_response_body.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 falcon/tests/test_response_body.py diff --git a/falcon/tests/test_hello.py b/falcon/tests/test_hello.py index 59b29a8..728c38a 100644 --- a/falcon/tests/test_hello.py +++ b/falcon/tests/test_hello.py @@ -97,7 +97,7 @@ class TestHelloWorld(testing.TestBase): resp = self.resource.resp content_length = int(self.srmock.headers_dict['Content-Length']) - self.assertEquals(content_length, len(self.resource.sample_unicode)) + self.assertEquals(content_length, len(self.resource.sample_utf8)) self.assertEquals(self.srmock.status, self.resource.sample_status) self.assertEquals(resp.status, self.resource.sample_status) diff --git a/falcon/tests/test_response_body.py b/falcon/tests/test_response_body.py new file mode 100644 index 0000000..1c12ae2 --- /dev/null +++ b/falcon/tests/test_response_body.py @@ -0,0 +1,17 @@ + +import falcon +import falcon.testing as testing + + +class TestResponseBody(testing.TestBase): + + def test_append_body(self): + text = "Hello beautiful world! " + resp = falcon.Response() + resp.body = "" + + for token in text.split(): + resp.body += token + resp.body += " " + + self.assertEquals(resp.body, text)