From 2135294e2ef30328cd514668bc3ce7bcde5f34ce Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Thu, 1 Feb 2018 11:52:55 +0100 Subject: [PATCH 1/8] changed None to return empty string instead of null string --- sanic/response.py | 3 ++- tests/test_requests.py | 5 +++-- tests/test_response.py | 12 ++++++++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/sanic/response.py b/sanic/response.py index 3873d90f..2402b432 100644 --- a/sanic/response.py +++ b/sanic/response.py @@ -239,7 +239,8 @@ def json(body, status=200, headers=None, :param headers: Custom Headers. :param kwargs: Remaining arguments that are passed to the json encoder. """ - return HTTPResponse(dumps(body, **kwargs), headers=headers, + _body = dumps(body, **kwargs) if body else None + return HTTPResponse(_body, headers=headers, status=status, content_type=content_type) diff --git a/tests/test_requests.py b/tests/test_requests.py index 9eb88243..49e73a55 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -109,12 +109,13 @@ def test_empty_json(): @app.route('/') async def handler(request): - assert request.json == None + assert request.json is None return json(request.json) request, response = app.test_client.get('/') assert response.status == 200 - assert response.text == 'null' + assert response.text == '' + def test_invalid_json(): app = Sanic('test_json') diff --git a/tests/test_response.py b/tests/test_response.py index 6ac77cec..88bd8a3a 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -63,6 +63,10 @@ def json_app(): async def test(request): return json(JSON_DATA) + @app.delete("/") + async def test_delete(request): + return json(None, status=204) + return app @@ -73,6 +77,14 @@ def test_json_response(json_app): assert response.text == json_dumps(JSON_DATA) assert response.json == JSON_DATA + +def test_no_content(json_app): + request, response = json_app.test_client.delete('/') + assert response.status == 204 + assert response.text == '' + assert response.headers['Content-Length'] == '0' + + @pytest.fixture def streaming_app(): app = Sanic('streaming') From 68fd1b66b50e5a1ea6348570b9a31fac43ec8c8e Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Thu, 1 Feb 2018 17:51:51 +0100 Subject: [PATCH 2/8] Response model now handles the 204 no content --- sanic/response.py | 13 ++++++++----- tests/test_requests.py | 3 ++- tests/test_response.py | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/sanic/response.py b/sanic/response.py index 2402b432..7a490ffe 100644 --- a/sanic/response.py +++ b/sanic/response.py @@ -195,8 +195,12 @@ class HTTPResponse(BaseHTTPResponse): timeout_header = b'' if keep_alive and keep_alive_timeout is not None: timeout_header = b'Keep-Alive: %d\r\n' % keep_alive_timeout - self.headers['Content-Length'] = self.headers.get( - 'Content-Length', len(self.body)) + + content_length = 0 + if self.status is not 204: + content_length = self.headers.get('Content-Length', len(self.body)) + self.headers['Content-Length'] = content_length + self.headers['Content-Type'] = self.headers.get( 'Content-Type', self.content_type) @@ -218,7 +222,7 @@ class HTTPResponse(BaseHTTPResponse): b'keep-alive' if keep_alive else b'close', timeout_header, headers, - self.body + self.body if self.status is not 204 else b'' ) @property @@ -239,8 +243,7 @@ def json(body, status=200, headers=None, :param headers: Custom Headers. :param kwargs: Remaining arguments that are passed to the json encoder. """ - _body = dumps(body, **kwargs) if body else None - return HTTPResponse(_body, headers=headers, + return HTTPResponse(dumps(body, **kwargs), headers=headers, status=status, content_type=content_type) diff --git a/tests/test_requests.py b/tests/test_requests.py index 49e73a55..14da4b0b 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -104,6 +104,7 @@ def test_json(): assert results.get('test') == True + def test_empty_json(): app = Sanic('test_json') @@ -114,7 +115,7 @@ def test_empty_json(): request, response = app.test_client.get('/') assert response.status == 200 - assert response.text == '' + assert response.text == 'null' def test_invalid_json(): diff --git a/tests/test_response.py b/tests/test_response.py index 88bd8a3a..475da8b6 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -43,7 +43,7 @@ def test_method_not_allowed(): return response.json({'hello': 'world'}) request, response = app.test_client.head('/') - assert response.headers['Allow']== 'GET' + assert response.headers['Allow'] == 'GET' @app.post('/') async def test(request): From 4b6e89a5266ccf2e471252fc2e4a5ae4441d8fab Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Thu, 1 Feb 2018 20:00:32 +0100 Subject: [PATCH 3/8] added one more test --- tests/test_response.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/test_response.py b/tests/test_response.py index 475da8b6..e7690c11 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -63,8 +63,12 @@ def json_app(): async def test(request): return json(JSON_DATA) + @app.get("/no-content") + async def no_content_handler(request): + return json(JSON_DATA, status=204) + @app.delete("/") - async def test_delete(request): + async def delete_handler(request): return json(None, status=204) return app @@ -79,6 +83,11 @@ def test_json_response(json_app): def test_no_content(json_app): + request, response = json_app.test_client.get('/no-content') + assert response.status == 204 + assert response.text == '' + assert response.headers['Content-Length'] == '0' + request, response = json_app.test_client.delete('/') assert response.status == 204 assert response.text == '' From 0ab64e9803fbf2f82caa1c8e10115d4add376fbb Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Fri, 2 Feb 2018 09:29:54 +0100 Subject: [PATCH 4/8] simplified logic when handling the body --- sanic/response.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sanic/response.py b/sanic/response.py index 7a490ffe..01d0843d 100644 --- a/sanic/response.py +++ b/sanic/response.py @@ -196,11 +196,13 @@ class HTTPResponse(BaseHTTPResponse): if keep_alive and keep_alive_timeout is not None: timeout_header = b'Keep-Alive: %d\r\n' % keep_alive_timeout + body = b'' content_length = 0 if self.status is not 204: + body = self.body content_length = self.headers.get('Content-Length', len(self.body)) - self.headers['Content-Length'] = content_length + self.headers['Content-Length'] = content_length self.headers['Content-Type'] = self.headers.get( 'Content-Type', self.content_type) @@ -222,7 +224,7 @@ class HTTPResponse(BaseHTTPResponse): b'keep-alive' if keep_alive else b'close', timeout_header, headers, - self.body if self.status is not 204 else b'' + body ) @property From 7ca3ad5d4cc25e16a45a98ec1eaa42d2eb4af6c2 Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Fri, 2 Feb 2018 13:24:51 +0100 Subject: [PATCH 5/8] no body and content length to 0 when 304 response is returned --- sanic/response.py | 2 +- tests/test_response.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/sanic/response.py b/sanic/response.py index 01d0843d..26da64ae 100644 --- a/sanic/response.py +++ b/sanic/response.py @@ -198,7 +198,7 @@ class HTTPResponse(BaseHTTPResponse): body = b'' content_length = 0 - if self.status is not 204: + if self.status is not 204 and self.status != 304: body = self.body content_length = self.headers.get('Content-Length', len(self.body)) diff --git a/tests/test_response.py b/tests/test_response.py index e7690c11..79f2b74f 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -67,6 +67,14 @@ def json_app(): async def no_content_handler(request): return json(JSON_DATA, status=204) + @app.get("/no-content/unmodified") + async def no_content_unmodified_handler(request): + return json(None, status=304) + + @app.get("/unmodified") + async def unmodified_handler(request): + return json(JSON_DATA, status=304) + @app.delete("/") async def delete_handler(request): return json(None, status=204) @@ -88,6 +96,16 @@ def test_no_content(json_app): assert response.text == '' assert response.headers['Content-Length'] == '0' + request, response = json_app.test_client.get('/no-content/unmodified') + assert response.status == 304 + assert response.text == '' + assert response.headers['Content-Length'] == '0' + + request, response = json_app.test_client.get('/unmodified') + assert response.status == 304 + assert response.text == '' + assert response.headers['Content-Length'] == '0' + request, response = json_app.test_client.delete('/') assert response.status == 204 assert response.text == '' From 86fed12d913ee407ca35fb93a007e354d41d580f Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Fri, 2 Feb 2018 14:05:57 +0100 Subject: [PATCH 6/8] less flake8 warnings in response test --- tests/test_response.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/test_response.py b/tests/test_response.py index 79f2b74f..b9ad9fb4 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -16,7 +16,6 @@ from unittest.mock import MagicMock JSON_DATA = {'ok': True} - def test_response_body_not_a_string(): """Test when a response body sent from the application is not a string""" app = Sanic('response_body_not_a_string') @@ -35,6 +34,7 @@ async def sample_streaming_fn(response): await asyncio.sleep(.001) response.write('bar') + def test_method_not_allowed(): app = Sanic('method_not_allowed') @@ -195,9 +195,11 @@ def get_file_content(static_file_directory, file_name): with open(os.path.join(static_file_directory, file_name), 'rb') as file: return file.read() + @pytest.mark.parametrize('file_name', ['test.file', 'decode me.txt', 'python.png']) def test_file_response(file_name, static_file_directory): app = Sanic('test_file_helper') + @app.route('/files/', methods=['GET']) def file_route(request, filename): file_path = os.path.join(static_file_directory, filename) @@ -209,10 +211,12 @@ def test_file_response(file_name, static_file_directory): assert response.body == get_file_content(static_file_directory, file_name) assert 'Content-Disposition' not in response.headers + @pytest.mark.parametrize('source,dest', [ ('test.file', 'my_file.txt'), ('decode me.txt', 'readme.md'), ('python.png', 'logo.png')]) def test_file_response_custom_filename(source, dest, static_file_directory): app = Sanic('test_file_helper') + @app.route('/files/', methods=['GET']) def file_route(request, filename): file_path = os.path.join(static_file_directory, filename) @@ -224,9 +228,11 @@ def test_file_response_custom_filename(source, dest, static_file_directory): assert response.body == get_file_content(static_file_directory, source) assert response.headers['Content-Disposition'] == 'attachment; filename="{}"'.format(dest) + @pytest.mark.parametrize('file_name', ['test.file', 'decode me.txt']) def test_file_head_response(file_name, static_file_directory): app = Sanic('test_file_helper') + @app.route('/files/', methods=['GET', 'HEAD']) async def file_route(request, filename): file_path = os.path.join(static_file_directory, filename) @@ -251,25 +257,29 @@ def test_file_head_response(file_name, static_file_directory): 'Content-Length']) == len( get_file_content(static_file_directory, file_name)) + @pytest.mark.parametrize('file_name', ['test.file', 'decode me.txt', 'python.png']) def test_file_stream_response(file_name, static_file_directory): app = Sanic('test_file_helper') + @app.route('/files/', methods=['GET']) def file_route(request, filename): file_path = os.path.join(static_file_directory, filename) file_path = os.path.abspath(unquote(file_path)) return file_stream(file_path, chunk_size=32, - mime_type=guess_type(file_path)[0] or 'text/plain') + mime_type=guess_type(file_path)[0] or 'text/plain') request, response = app.test_client.get('/files/{}'.format(file_name)) assert response.status == 200 assert response.body == get_file_content(static_file_directory, file_name) assert 'Content-Disposition' not in response.headers + @pytest.mark.parametrize('source,dest', [ ('test.file', 'my_file.txt'), ('decode me.txt', 'readme.md'), ('python.png', 'logo.png')]) def test_file_stream_response_custom_filename(source, dest, static_file_directory): app = Sanic('test_file_helper') + @app.route('/files/', methods=['GET']) def file_route(request, filename): file_path = os.path.join(static_file_directory, filename) @@ -281,9 +291,11 @@ def test_file_stream_response_custom_filename(source, dest, static_file_director assert response.body == get_file_content(static_file_directory, source) assert response.headers['Content-Disposition'] == 'attachment; filename="{}"'.format(dest) + @pytest.mark.parametrize('file_name', ['test.file', 'decode me.txt']) def test_file_stream_head_response(file_name, static_file_directory): app = Sanic('test_file_helper') + @app.route('/files/', methods=['GET', 'HEAD']) async def file_route(request, filename): file_path = os.path.join(static_file_directory, filename) From f5a2d191993da337d499327459433de5073c8adb Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Fri, 2 Feb 2018 14:13:14 +0100 Subject: [PATCH 7/8] touch commit --- tests/test_response.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_response.py b/tests/test_response.py index b9ad9fb4..57e01cb6 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -312,7 +312,7 @@ def test_file_stream_head_response(file_name, static_file_directory): content_type=guess_type(file_path)[0] or 'text/plain') else: return file_stream(file_path, chunk_size=32, headers=headers, - mime_type=guess_type(file_path)[0] or 'text/plain') + mime_type=guess_type(file_path)[0] or 'text/plain') request, response = app.test_client.head('/files/{}'.format(file_name)) assert response.status == 200 From f2c0489452cd3bf2a5f2ba0af55f7346273e8e08 Mon Sep 17 00:00:00 2001 From: Arnulfo Solis Date: Fri, 2 Feb 2018 20:19:15 +0100 Subject: [PATCH 8/8] replaced comparison for in operator --- sanic/response.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sanic/response.py b/sanic/response.py index 26da64ae..9349ce81 100644 --- a/sanic/response.py +++ b/sanic/response.py @@ -72,6 +72,8 @@ STATUS_CODES = { 511: b'Network Authentication Required' } +EMPTY_STATUS_CODES = [204, 304] + class BaseHTTPResponse: def _encode_body(self, data): @@ -198,7 +200,7 @@ class HTTPResponse(BaseHTTPResponse): body = b'' content_length = 0 - if self.status is not 204 and self.status != 304: + if self.status not in EMPTY_STATUS_CODES: body = self.body content_length = self.headers.get('Content-Length', len(self.body))