From 789f1e99aeb9c116e4feb57abbcafc5e9dda70b8 Mon Sep 17 00:00:00 2001 From: Julien Sagnard Date: Tue, 8 Jan 2019 18:32:08 +0100 Subject: [PATCH 01/17] Support aiohttp handlers to return tuples --- connexion/apis/aiohttp_api.py | 77 ++++++++---- tests/aiohttp/test_aiohttp_simple_api.py | 2 +- tests/aiohttp/test_get_response.py | 142 +++++++++++++++++++++++ tests/conftest.py | 2 +- tests/fakeapi/aiohttp_handlers.py | 18 +-- 5 files changed, 206 insertions(+), 35 deletions(-) create mode 100644 tests/aiohttp/test_get_response.py diff --git a/connexion/apis/aiohttp_api.py b/connexion/apis/aiohttp_api.py index 949b35427..1c63cea80 100644 --- a/connexion/apis/aiohttp_api.py +++ b/connexion/apis/aiohttp_api.py @@ -318,22 +318,26 @@ def get_response(cls, response, mimetype=None, request=None): if isinstance(response, ConnexionResponse): response = cls._get_aiohttp_response_from_connexion(response, mimetype) - - if isinstance(response, web.StreamResponse): - logger.debug('Got stream response with status code (%d)', - response.status, extra={'url': url}) else: - logger.debug('Got data and status code (%d)', - response.status, extra={'data': response.body, 'url': url}) + response = cls._get_aiohttp_response(response, mimetype) + + logger.debug('Got stream response with status code (%d)', + response.status, extra={'url': url}) return response @classmethod def get_connexion_response(cls, response, mimetype=None): - response.body = cls._cast_body(response.body, mimetype) - if isinstance(response, ConnexionResponse): - return response + # If body in ConnexionResponse is not byte, it may not pass schema validation. + # In this case, rebuild response with aiohttp to have consistency + if response.body is None or isinstance(response.body, bytes): + return response + else: + response = cls._get_aiohttp_response_from_connexion(response, mimetype) + + if not isinstance(response, web.StreamResponse): + response = cls._get_aiohttp_response(response, mimetype) return ConnexionResponse( status_code=response.status, @@ -345,31 +349,56 @@ def get_connexion_response(cls, response, mimetype=None): @classmethod def _get_aiohttp_response_from_connexion(cls, response, mimetype): - content_type = response.content_type if response.content_type else \ - response.mimetype if response.mimetype else mimetype - - body = cls._cast_body(response.body, content_type) + content_type = response.content_type or response.mimetype or mimetype - return web.Response( - status=response.status_code, + return cls._build_aiohttp_response( + status_code=response.status_code, content_type=content_type, headers=response.headers, - body=body + data=response.body ) @classmethod - def _cast_body(cls, body, content_type=None): - if not isinstance(body, bytes): - if content_type and is_json_mimetype(content_type): - return cls.jsonifier.dumps(body).encode() + def _get_aiohttp_response(cls, response, mimetype): + if isinstance(response, web.StreamResponse): + return response + + elif isinstance(response, tuple) and len(response) == 3: + data, status_code, headers = response + return cls._build_aiohttp_response(content_type=mimetype, status_code=status_code, data=data, headers=headers) + + elif isinstance(response, tuple) and len(response) == 2: + data, status_code = response + return cls._build_aiohttp_response(content_type=mimetype, status_code=status_code, data=data) + + else: + return cls._build_aiohttp_response(content_type=mimetype, data=response) - elif isinstance(body, str): - return body.encode() + @classmethod + def _build_aiohttp_response(cls, data, content_type, headers=None, status_code=None): + if status_code is None: + if data is None: + status_code = 204 + content_type = None + else: + status_code = 200 + elif hasattr(status_code, "value"): + # If we got an enum instead of an int, extract the value. + status_code = status_code.value + if data is not None and not isinstance(data, bytes): + if content_type and is_json_mimetype(content_type): + text = cls.jsonifier.dumps(data) + elif isinstance(data, str): + text = data else: - return str(body).encode() + text = str(data) + body = None else: - return body + text = None + body = data + + return web.Response(body=body, text=text, headers=headers, status=status_code, content_type=content_type) @classmethod def _set_jsonifier(cls): diff --git a/tests/aiohttp/test_aiohttp_simple_api.py b/tests/aiohttp/test_aiohttp_simple_api.py index 50b204a14..ba9d2dea2 100644 --- a/tests/aiohttp/test_aiohttp_simple_api.py +++ b/tests/aiohttp/test_aiohttp_simple_api.py @@ -283,7 +283,7 @@ def test_validate_responses(aiohttp_app, aiohttp_client): app_client = yield from aiohttp_client(aiohttp_app.app) get_bye = yield from app_client.get('/v1.0/aiohttp_validate_responses') assert get_bye.status == 200 - assert (yield from get_bye.read()) == b'{"validate": true}' + assert (yield from get_bye.read()) == b'{"validate":true}' @asyncio.coroutine diff --git a/tests/aiohttp/test_get_response.py b/tests/aiohttp/test_get_response.py new file mode 100644 index 000000000..4011f2594 --- /dev/null +++ b/tests/aiohttp/test_get_response.py @@ -0,0 +1,142 @@ +import asyncio +from aiohttp import web +import pytest +from connexion.apis.aiohttp_api import AioHttpApi +from connexion.lifecycle import ConnexionResponse + + +@pytest.fixture(scope='module') +def api(aiohttp_api_spec_dir): + yield AioHttpApi(specification=aiohttp_api_spec_dir / 'swagger_secure.yaml') + + +@asyncio.coroutine +def test_get_response_from_aiohttp_response(api): + response = yield from api.get_response(web.Response(text='foo', status=201, headers={'X-header': 'value'})) + assert isinstance(response, web.Response) + assert response.status == 201 + assert response.body == b'foo' + assert response.content_type == 'text/plain' + assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8', 'X-header': 'value'} + + +@asyncio.coroutine +def test_get_response_from_connexion_response(api): + response = yield from api.get_response(ConnexionResponse(status_code=201, mimetype='text/plain', body='foo', headers={'X-header': 'value'})) + assert isinstance(response, web.Response) + assert response.status == 201 + assert response.body == b'foo' + assert response.content_type == 'text/plain' + assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8', 'X-header': 'value'} + + +@asyncio.coroutine +def test_get_response_from_string(api): + response = yield from api.get_response('foo') + assert isinstance(response, web.Response) + assert response.status == 200 + assert response.body == b'foo' + assert response.content_type == 'text/plain' + assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8'} + + +@asyncio.coroutine +def test_get_response_from_string_status(api): + response = yield from api.get_response(('foo', 201)) + assert isinstance(response, web.Response) + assert response.status == 201 + assert response.body == b'foo' + assert response.content_type == 'text/plain' + assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8'} + + +@asyncio.coroutine +def test_get_response_from_string_status_headers(api): + response = yield from api.get_response(('foo', 201, {'X-header': 'value'})) + assert isinstance(response, web.Response) + assert response.status == 201 + assert response.body == b'foo' + assert response.content_type == 'text/plain' + assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8', 'X-header': 'value'} + + +@asyncio.coroutine +def test_get_response_from_dict(api): + response = yield from api.get_response({'foo': 'bar'}) + assert isinstance(response, web.Response) + assert response.status == 200 + assert response.body == b"{'foo': 'bar'}" + assert response.content_type == 'text/plain' + assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8'} + + +@asyncio.coroutine +def test_get_response_from_dict_json(api): + response = yield from api.get_response({'foo': 'bar'}, mimetype='application/json') + assert isinstance(response, web.Response) + assert response.status == 200 + assert response.body == b'{"foo":"bar"}' + assert response.content_type == 'application/json' + assert dict(response.headers) == {'Content-Type': 'application/json; charset=utf-8'} + + +@asyncio.coroutine +def test_get_response_no_data(api): + response = yield from api.get_response(None, mimetype='application/json') + assert isinstance(response, web.Response) + assert response.status == 204 + assert response.body is None + assert response.content_type == 'application/octet-stream' + assert dict(response.headers) == {} + + +@asyncio.coroutine +def test_get_response_binary_json(api): + response = yield from api.get_response(b'{"foo":"bar"}', mimetype='application/json') + assert isinstance(response, web.Response) + assert response.status == 200 + assert response.body == b'{"foo":"bar"}' + assert response.content_type == 'application/json' + assert dict(response.headers) == {'Content-Type': 'application/json'} + + +@asyncio.coroutine +def test_get_response_binary_no_mimetype(api): + response = yield from api.get_response(b'{"foo":"bar"}') + assert isinstance(response, web.Response) + assert response.status == 200 + assert response.body == b'{"foo":"bar"}' + assert response.content_type == 'application/octet-stream' + assert dict(response.headers) == {} + + +@asyncio.coroutine +def test_get_connexion_response_from_aiohttp_response(api): + response = api.get_connexion_response(web.Response(text='foo', status=201, headers={'X-header': 'value'})) + assert isinstance(response, ConnexionResponse) + assert response.status_code == 201 + assert response.body == b'foo' + assert response.content_type == 'text/plain' + assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8', 'X-header': 'value'} + + +@asyncio.coroutine +def test_get_connexion_response_from_connexion_response(api): + response = api.get_connexion_response(ConnexionResponse(status_code=201, content_type='text/plain', body='foo', headers={'X-header': 'value'})) + assert isinstance(response, ConnexionResponse) + assert response.status_code == 201 + assert response.body == b'foo' + assert response.content_type == 'text/plain' + assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8', 'X-header': 'value'} + + +@asyncio.coroutine +def test_get_connexion_response_from_tuple(api): + response = api.get_connexion_response(('foo', 201, {'X-header': 'value'})) + assert isinstance(response, ConnexionResponse) + assert response.status_code == 201 + assert response.body == b'foo' + assert response.content_type == 'text/plain' + assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8', 'X-header': 'value'} + + diff --git a/tests/conftest.py b/tests/conftest.py index 450fa3cd2..a92131851 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -72,7 +72,7 @@ def simple_api_spec_dir(): return FIXTURES_FOLDER / 'simple' -@pytest.fixture +@pytest.fixture(scope='session') def aiohttp_api_spec_dir(): return FIXTURES_FOLDER / 'aiohttp' diff --git a/tests/fakeapi/aiohttp_handlers.py b/tests/fakeapi/aiohttp_handlers.py index 98b601431..607833032 100755 --- a/tests/fakeapi/aiohttp_handlers.py +++ b/tests/fakeapi/aiohttp_handlers.py @@ -16,50 +16,50 @@ def get_bye(name): @asyncio.coroutine def aiohttp_str_response(): - return ConnexionResponse(body='str response') + return 'str response' @asyncio.coroutine def aiohttp_non_str_non_json_response(): - return ConnexionResponse(body=1234) + return 1234 @asyncio.coroutine def aiohttp_bytes_response(): - return ConnexionResponse(body=b'bytes response') + return b'bytes response' @asyncio.coroutine def aiohttp_validate_responses(): - return ConnexionResponse(body=b'{"validate": true}') + return {"validate": True} @asyncio.coroutine def aiohttp_post_greeting(name, **kwargs): data = {'greeting': 'Hello {name}'.format(name=name)} - return ConnexionResponse(body=data) + return data @asyncio.coroutine def aiohttp_access_request_context(request_ctx): assert request_ctx is not None assert isinstance(request_ctx, aiohttp.web.Request) - return ConnexionResponse(status_code=204) + return None @asyncio.coroutine def aiohttp_query_parsing_str(query): - return ConnexionResponse(body={'query': query}) + return {'query': query} @asyncio.coroutine def aiohttp_query_parsing_array(query): - return ConnexionResponse(body={'query': query}) + return {'query': query} @asyncio.coroutine def aiohttp_query_parsing_array_multi(query): - return ConnexionResponse(body={'query': query}) + return {'query': query} USERS = [ From 4211f65db6c82d7b074f2763333c73af13c5eeb7 Mon Sep 17 00:00:00 2001 From: Julien Sagnard Date: Mon, 14 Jan 2019 10:47:08 +0100 Subject: [PATCH 02/17] Minor update from #828 review --- .gitignore | 4 +++- connexion/apis/abstract.py | 25 ++++++++++++++++++++++++- connexion/apis/aiohttp_api.py | 8 +++++--- connexion/apis/flask_api.py | 4 ++-- tests/aiohttp/test_get_response.py | 7 +++++++ 5 files changed, 41 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 694c04f8b..a2afe59d1 100644 --- a/.gitignore +++ b/.gitignore @@ -11,4 +11,6 @@ htmlcov/ *.swp .tox/ .idea/ -venv/ \ No newline at end of file +.vscode/ +venv/ +src/ \ No newline at end of file diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index c21eff128..8a2708f67 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -238,7 +238,9 @@ def get_request(self, *args, **kwargs): @abc.abstractmethod def get_response(self, response, mimetype=None, request=None): """ - This method converts the ConnexionResponse to a user framework response. + This method converts a handler response to a framework response. + The response can be a ConnexionResponse, an operation handler, a framework response or a tuple. + Other type than ConnexionResponse are handled by `cls._response_from_handler` :param response: A response to cast. :param mimetype: The response mimetype. :param request: The request associated with this response (the user framework request). @@ -247,12 +249,33 @@ def get_response(self, response, mimetype=None, request=None): :type mimetype: str """ + @classmethod + @abc.abstractmethod + def _response_from_handler(cls, response, mimetype): + """ + Create a framework response from the operation handler data. + An operation handler can return: + - a framework response + - a body (str / binary / dict / list), a response will be created + with a status code 200 by default and empty headers. + - a tuple of (body: str, status_code: int) + - a tuple of (body: str, status_code: int, headers: dict) + :param response: A response from an operation handler. + :type response Union[Response, str, Tuple[str, int], Tuple[str, int, dict]] + :param mimetype: The response mimetype. + :type mimetype: str + :return A framwork response. + :rtype Response + """ + @classmethod @abc.abstractmethod def get_connexion_response(cls, response, mimetype=None): """ This method converts the user framework response to a ConnexionResponse. + It is used after the user returned a response to give it to response validators. :param response: A response to cast. + :param mimetype: The response mimetype. """ def json_loads(self, data): diff --git a/connexion/apis/aiohttp_api.py b/connexion/apis/aiohttp_api.py index 1c63cea80..83b2a4469 100644 --- a/connexion/apis/aiohttp_api.py +++ b/connexion/apis/aiohttp_api.py @@ -319,7 +319,7 @@ def get_response(cls, response, mimetype=None, request=None): if isinstance(response, ConnexionResponse): response = cls._get_aiohttp_response_from_connexion(response, mimetype) else: - response = cls._get_aiohttp_response(response, mimetype) + response = cls._response_from_handler(response, mimetype) logger.debug('Got stream response with status code (%d)', response.status, extra={'url': url}) @@ -337,7 +337,7 @@ def get_connexion_response(cls, response, mimetype=None): response = cls._get_aiohttp_response_from_connexion(response, mimetype) if not isinstance(response, web.StreamResponse): - response = cls._get_aiohttp_response(response, mimetype) + response = cls._response_from_handler(response, mimetype) return ConnexionResponse( status_code=response.status, @@ -359,9 +359,11 @@ def _get_aiohttp_response_from_connexion(cls, response, mimetype): ) @classmethod - def _get_aiohttp_response(cls, response, mimetype): + def _response_from_handler(cls, response, mimetype): if isinstance(response, web.StreamResponse): return response + elif isinstance(response, tuple) and isinstance(response[0], web.StreamResponse): + raise RuntimeError("Cannot return web.StreamResponse in tuple. Only raw data can be returned in tuple.") elif isinstance(response, tuple) and len(response) == 3: data, status_code, headers = response diff --git a/connexion/apis/flask_api.py b/connexion/apis/flask_api.py index dc79062b7..7d807ff48 100644 --- a/connexion/apis/flask_api.py +++ b/connexion/apis/flask_api.py @@ -146,7 +146,7 @@ def get_response(cls, response, mimetype=None, request=None): if isinstance(response, ConnexionResponse): flask_response = cls._get_flask_response_from_connexion(response, mimetype) else: - flask_response = cls._get_flask_response(response, mimetype) + flask_response = cls._response_from_handler(response, mimetype) logger.debug('Got data and status code (%d)', flask_response.status_code, @@ -207,7 +207,7 @@ def _jsonify_data(cls, data, mimetype): return data @classmethod - def _get_flask_response(cls, response, mimetype): + def _response_from_handler(cls, response, mimetype): if flask_utils.is_flask_response(response): return response diff --git a/tests/aiohttp/test_get_response.py b/tests/aiohttp/test_get_response.py index 4011f2594..f0a8c9f43 100644 --- a/tests/aiohttp/test_get_response.py +++ b/tests/aiohttp/test_get_response.py @@ -60,6 +60,13 @@ def test_get_response_from_string_status_headers(api): assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8', 'X-header': 'value'} +@asyncio.coroutine +def test_get_response_from_tuple_error(api): + with pytest.raises(RuntimeError) as e: + yield from api.get_response((web.Response(text='foo', status=201, headers={'X-header': 'value'}), 200)) + assert str(e.value) == "Cannot return web.StreamResponse in tuple. Only raw data can be returned in tuple." + + @asyncio.coroutine def test_get_response_from_dict(api): response = yield from api.get_response({'foo': 'bar'}) From af8bcb0f4060bd82e5c4d328a5358ad56587514b Mon Sep 17 00:00:00 2001 From: Julien Sagnard Date: Wed, 16 Jan 2019 16:01:19 +0100 Subject: [PATCH 03/17] Factorize more code between Flask and AioHttp response --- connexion/apis/abstract.py | 146 +++++++++++++++++++++-- connexion/apis/aiohttp_api.py | 82 ++++--------- connexion/apis/flask_api.py | 119 ++++++------------ tests/aiohttp/test_aiohttp_simple_api.py | 3 +- tests/aiohttp/test_get_response.py | 11 +- tests/api/test_responses.py | 3 +- 6 files changed, 206 insertions(+), 158 deletions(-) diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index 8a2708f67..96a1b55e7 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -4,14 +4,18 @@ import sys import six +from enum import Enum +from ..decorators.produces import NoContent from ..exceptions import ResolverError from ..http_facts import METHODS from ..jsonifier import Jsonifier +from ..lifecycle import ConnexionResponse from ..operations import make_operation from ..options import ConnexionOptions from ..resolver import Resolver from ..spec import Specification +from ..utils import is_json_mimetype MODULE_PATH = pathlib.Path(__file__).absolute().parent.parent SWAGGER_UI_URL = 'ui' @@ -237,20 +241,45 @@ def get_request(self, *args, **kwargs): @classmethod @abc.abstractmethod def get_response(self, response, mimetype=None, request=None): + """ + This method converts a handler response to a framework response. + This method should just retrieve response from handler then call `cls._get_response`. + It is mainly here to handle AioHttp async handler. + :param response: A response to cast. + :param mimetype: The response mimetype. + :param request: The request associated with this response (the user framework request). + + :type response: Framework Response + :type mimetype: str + """ + + @classmethod + def _get_response(cls, response, mimetype=None, url=None): """ This method converts a handler response to a framework response. The response can be a ConnexionResponse, an operation handler, a framework response or a tuple. Other type than ConnexionResponse are handled by `cls._response_from_handler` :param response: A response to cast. :param mimetype: The response mimetype. - :param request: The request associated with this response (the user framework request). + :param url: The url to write in logs - :type response: ConnexionResponse + :type response: Framework Response :type mimetype: str """ + logger.debug('Getting data and status code', + extra={ + 'data': response, + 'data_type': type(response), + 'url': url + }) + + if isinstance(response, ConnexionResponse): + framework_response = cls._connexion_to_framework_response(response, mimetype) + else: + framework_response = cls._response_from_handler(response, mimetype) + return framework_response @classmethod - @abc.abstractmethod def _response_from_handler(cls, response, mimetype): """ Create a framework response from the operation handler data. @@ -264,20 +293,119 @@ def _response_from_handler(cls, response, mimetype): :type response Union[Response, str, Tuple[str, int], Tuple[str, int, dict]] :param mimetype: The response mimetype. :type mimetype: str - :return A framwork response. + :return A framework response. :rtype Response """ + if cls._is_framework_response(response): + return response + + if isinstance(response, tuple): + len_response = len(response) + if len_response == 2: + if isinstance(response[1], (int, Enum)): + data, status_code = response + return cls._build_response(mimetype=mimetype, data=data, status_code=status_code) + else: + data, headers = response + return cls._build_response(mimetype=mimetype, data=data, headers=headers) + elif len_response == 3: + data, status_code, headers = response + return cls._build_response(mimetype=mimetype, data=data, status_code=status_code, headers=headers) + else: + raise TypeError( + 'The view function did not return a valid response tuple.' + ' The tuple must have the form (body), (body, status, headers),' + ' (body, status), or (body, headers).' + ) + else: + return cls._build_response(mimetype=mimetype, data=response) @classmethod - @abc.abstractmethod def get_connexion_response(cls, response, mimetype=None): + """ Cast framework dependent response to ConnexionResponse used for schema validation """ + if isinstance(response, ConnexionResponse): + # If body in ConnexionResponse is not byte, it may not pass schema validation. + # In this case, rebuild response with aiohttp to have consistency + if response.body is None or isinstance(response.body, six.binary_type): + return response + else: + response = cls._build_response( + data=response.body, + mimetype=mimetype, + content_type=response.content_type, + headers=response.headers, + status_code=response.status_code + ) + + if not cls._is_framework_response(response): + response = cls._response_from_handler(response, mimetype) + return cls._framework_to_connexion_response(response=response, mimetype=mimetype) + + @classmethod + @abc.abstractmethod + def _is_framework_response(cls, response): + """ Return True if `response` is a framework response class """ + + @classmethod + @abc.abstractmethod + def _framework_to_connexion_response(cls, response, mimetype): + """ Cast framework response class to ConnexionResponse used for schema validation """ + + @classmethod + @abc.abstractmethod + def _connexion_to_framework_response(cls, response, mimetype): + """ Cast ConnexionResponse to framework response class """ + + @classmethod + @abc.abstractmethod + def _build_response(cls, data, mimetype, content_type=None, status_code=None, headers=None): """ - This method converts the user framework response to a ConnexionResponse. - It is used after the user returned a response to give it to response validators. - :param response: A response to cast. - :param mimetype: The response mimetype. + Create a framework response from the provided arguments. + :param data: Body data. + :param content_type: The response mimetype. + :type content_type: str + :param content_type: The response status code. + :type status_code: int + :param headers: The response status code. + :type headers: Union[Iterable[Tuple[str, str]], Dict[str, str]] + :return A framework response. + :rtype Response """ + @classmethod + def _prepare_body_and_status_code(cls, data, mimetype, status_code=None): + if data is NoContent: + data = None + + if status_code is None: + if data is None: + status_code = 204 + mimetype = None + else: + status_code = 200 + elif hasattr(status_code, "value"): + # If we got an enum instead of an int, extract the value. + status_code = status_code.value + + if data is not None: + body = cls._jsonify_data(data, mimetype) + else: + body = data + return body, status_code + + @classmethod + def _jsonify_data(cls, data, mimetype): + if not isinstance(data, six.binary_type): + if isinstance(mimetype, six.string_types) and is_json_mimetype(mimetype): + body = cls.jsonifier.dumps(data) + elif isinstance(data, six.text_type): + body = data + else: + body = str(data) + else: + body = data + return body + def json_loads(self, data): return self.jsonifier.loads(data) diff --git a/connexion/apis/aiohttp_api.py b/connexion/apis/aiohttp_api.py index 83b2a4469..de2e4b27b 100644 --- a/connexion/apis/aiohttp_api.py +++ b/connexion/apis/aiohttp_api.py @@ -310,96 +310,56 @@ def get_response(cls, response, mimetype=None, request=None): url = str(request.url) if request else '' - logger.debug('Getting data and status code', - extra={ - 'data': response, - 'url': url - }) - - if isinstance(response, ConnexionResponse): - response = cls._get_aiohttp_response_from_connexion(response, mimetype) - else: - response = cls._response_from_handler(response, mimetype) + response = cls._get_response(response, mimetype=mimetype, url=url) + # TODO: I let this log here for full compatibility. But if we can modify it, it can go to _get_response() logger.debug('Got stream response with status code (%d)', response.status, extra={'url': url}) return response @classmethod - def get_connexion_response(cls, response, mimetype=None): - if isinstance(response, ConnexionResponse): - # If body in ConnexionResponse is not byte, it may not pass schema validation. - # In this case, rebuild response with aiohttp to have consistency - if response.body is None or isinstance(response.body, bytes): - return response - else: - response = cls._get_aiohttp_response_from_connexion(response, mimetype) - - if not isinstance(response, web.StreamResponse): - response = cls._response_from_handler(response, mimetype) + def _is_framework_response(cls, response): + """ Return True if `response` is a framework response class """ + return isinstance(response, web.StreamResponse) + @classmethod + def _framework_to_connexion_response(cls, response, mimetype): + """ Cast framework response class to ConnexionResponse used for schema validation """ return ConnexionResponse( status_code=response.status, - mimetype=response.content_type, + mimetype=mimetype, content_type=response.content_type, headers=response.headers, body=response.body ) @classmethod - def _get_aiohttp_response_from_connexion(cls, response, mimetype): - content_type = response.content_type or response.mimetype or mimetype - - return cls._build_aiohttp_response( + def _connexion_to_framework_response(cls, response, mimetype): + """ Cast ConnexionResponse to framework response class """ + return cls._build_response( + mimetype=response.mimetype or mimetype, status_code=response.status_code, - content_type=content_type, + content_type=response.content_type, headers=response.headers, data=response.body ) @classmethod - def _response_from_handler(cls, response, mimetype): - if isinstance(response, web.StreamResponse): - return response - elif isinstance(response, tuple) and isinstance(response[0], web.StreamResponse): - raise RuntimeError("Cannot return web.StreamResponse in tuple. Only raw data can be returned in tuple.") - - elif isinstance(response, tuple) and len(response) == 3: - data, status_code, headers = response - return cls._build_aiohttp_response(content_type=mimetype, status_code=status_code, data=data, headers=headers) + def _build_response(cls, data, mimetype, content_type=None, headers=None, status_code=None): + if isinstance(data, web.StreamResponse): + raise TypeError("Cannot return web.StreamResponse in tuple. Only raw data can be returned in tuple.") - elif isinstance(response, tuple) and len(response) == 2: - data, status_code = response - return cls._build_aiohttp_response(content_type=mimetype, status_code=status_code, data=data) + data, status_code = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code) - else: - return cls._build_aiohttp_response(content_type=mimetype, data=response) - - @classmethod - def _build_aiohttp_response(cls, data, content_type, headers=None, status_code=None): - if status_code is None: - if data is None: - status_code = 204 - content_type = None - else: - status_code = 200 - elif hasattr(status_code, "value"): - # If we got an enum instead of an int, extract the value. - status_code = status_code.value - - if data is not None and not isinstance(data, bytes): - if content_type and is_json_mimetype(content_type): - text = cls.jsonifier.dumps(data) - elif isinstance(data, str): - text = data - else: - text = str(data) + if isinstance(data, str): + text = data body = None else: text = None body = data + content_type = content_type or mimetype return web.Response(body=body, text=text, headers=headers, status=status_code, content_type=content_type) @classmethod diff --git a/connexion/apis/flask_api.py b/connexion/apis/flask_api.py index 7d807ff48..a4a27e39e 100644 --- a/connexion/apis/flask_api.py +++ b/connexion/apis/flask_api.py @@ -5,7 +5,6 @@ import werkzeug.exceptions from connexion.apis import flask_utils from connexion.apis.abstract import AbstractAPI -from connexion.decorators.produces import NoContent from connexion.handlers import AuthErrorHandler from connexion.jsonifier import Jsonifier from connexion.lifecycle import ConnexionRequest, ConnexionResponse @@ -54,9 +53,9 @@ def add_openapi_yaml(self): self.blueprint.add_url_rule( openapi_spec_path_yaml, endpoint_name, - lambda: FlaskApi._build_flask_response( + lambda: FlaskApi._build_response( status_code=200, - content_type="text/yaml", + mimetype="text/yaml", data=yamldumper(self.specification.raw) ) ) @@ -136,113 +135,73 @@ def get_response(cls, response, mimetype=None, request=None): :type operation_handler_result: flask.Response | (flask.Response, int) | (flask.Response, int, dict) :rtype: ConnexionResponse """ - logger.debug('Getting data and status code', - extra={ - 'data': response, - 'data_type': type(response), - 'url': flask.request.url - }) - - if isinstance(response, ConnexionResponse): - flask_response = cls._get_flask_response_from_connexion(response, mimetype) - else: - flask_response = cls._response_from_handler(response, mimetype) + flask_response = cls._get_response(response, mimetype=mimetype, url=flask.request.url) + # TODO: I let this log here for full compatibility. But if we can modify it, it can go to _get_response() logger.debug('Got data and status code (%d)', flask_response.status_code, extra={ 'data': response, - 'datatype': type(response), 'url': flask.request.url }) return flask_response @classmethod - def _get_flask_response_from_connexion(cls, response, mimetype): - data = response.body - status_code = response.status_code - mimetype = response.mimetype or mimetype - content_type = response.content_type or mimetype - headers = response.headers + def _is_framework_response(cls, response): + """ Return True if provided response is a framework type """ + return flask_utils.is_flask_response(response) - flask_response = cls._build_flask_response(mimetype, content_type, - headers, status_code, data) + @classmethod + def _framework_to_connexion_response(cls, response, mimetype): + """ Cast framework response class to ConnexionResponse used for schema validation """ + return ConnexionResponse( + status_code=response.status_code, + mimetype=response.mimetype, + content_type=response.content_type, + headers=response.headers, + body=response.get_data(), + ) + + @classmethod + def _connexion_to_framework_response(cls, response, mimetype): + """ Cast ConnexionResponse to framework response class """ + flask_response = cls._build_response( + mimetype=response.mimetype or mimetype, + content_type=response.content_type, + headers=response.headers, + status_code=response.status_code, + data=response.body + ) return flask_response @classmethod - def _build_flask_response(cls, mimetype=None, content_type=None, - headers=None, status_code=None, data=None): + def _build_response(cls, mimetype, content_type=None, headers=None, status_code=None, data=None): + if flask_utils.is_flask_response(data): + return flask.current_app.make_response((data, status_code, headers)) + + data, status_code = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code) + kwargs = { 'mimetype': mimetype, 'content_type': content_type, - 'headers': headers + 'headers': headers, + 'response': data, + 'status': status_code } kwargs = {k: v for k, v in six.iteritems(kwargs) if v is not None} - flask_response = flask.current_app.response_class(**kwargs) # type: flask.Response - - if status_code is not None: - # If we got an enum instead of an int, extract the value. - if hasattr(status_code, "value"): - status_code = status_code.value - - flask_response.status_code = status_code - - if data is not None and data is not NoContent: - data = cls._jsonify_data(data, mimetype) - flask_response.set_data(data) - - elif data is NoContent: - flask_response.set_data('') - - return flask_response + return flask.current_app.response_class(**kwargs) # type: flask.Response @classmethod def _jsonify_data(cls, data, mimetype): + # TODO: to discuss: Using jsonifier for all type of data, even when mimetype is not json is strange. Why ? if (isinstance(mimetype, six.string_types) and is_json_mimetype(mimetype)) \ or not (isinstance(data, six.binary_type) or isinstance(data, six.text_type)): return cls.jsonifier.dumps(data) return data - @classmethod - def _response_from_handler(cls, response, mimetype): - if flask_utils.is_flask_response(response): - return response - - elif isinstance(response, tuple) and flask_utils.is_flask_response(response[0]): - return flask.current_app.make_response(response) - - elif isinstance(response, tuple) and len(response) == 3: - data, status_code, headers = response - return cls._build_flask_response(mimetype, None, - headers, status_code, data) - - elif isinstance(response, tuple) and len(response) == 2: - data, status_code = response - return cls._build_flask_response(mimetype, None, None, - status_code, data) - - else: - return cls._build_flask_response(mimetype=mimetype, data=response) - - @classmethod - def get_connexion_response(cls, response, mimetype=None): - if isinstance(response, ConnexionResponse): - return response - - if not isinstance(response, flask.current_app.response_class): - response = cls.get_response(response, mimetype) - - return ConnexionResponse( - status_code=response.status_code, - mimetype=response.mimetype, - content_type=response.content_type, - headers=response.headers, - body=response.get_data(), - ) - @classmethod def get_request(cls, *args, **params): # type: (*Any, **Any) -> ConnexionRequest diff --git a/tests/aiohttp/test_aiohttp_simple_api.py b/tests/aiohttp/test_aiohttp_simple_api.py index ba9d2dea2..94147c17d 100644 --- a/tests/aiohttp/test_aiohttp_simple_api.py +++ b/tests/aiohttp/test_aiohttp_simple_api.py @@ -4,7 +4,6 @@ import pytest import yaml -import aiohttp.web from conftest import TEST_FOLDER from connexion import AioHttpApp @@ -283,7 +282,7 @@ def test_validate_responses(aiohttp_app, aiohttp_client): app_client = yield from aiohttp_client(aiohttp_app.app) get_bye = yield from app_client.get('/v1.0/aiohttp_validate_responses') assert get_bye.status == 200 - assert (yield from get_bye.read()) == b'{"validate":true}' + assert (yield from get_bye.json()) == {"validate": True} @asyncio.coroutine diff --git a/tests/aiohttp/test_get_response.py b/tests/aiohttp/test_get_response.py index f0a8c9f43..74e92686e 100644 --- a/tests/aiohttp/test_get_response.py +++ b/tests/aiohttp/test_get_response.py @@ -1,6 +1,7 @@ import asyncio from aiohttp import web import pytest +import json from connexion.apis.aiohttp_api import AioHttpApi from connexion.lifecycle import ConnexionResponse @@ -62,7 +63,7 @@ def test_get_response_from_string_status_headers(api): @asyncio.coroutine def test_get_response_from_tuple_error(api): - with pytest.raises(RuntimeError) as e: + with pytest.raises(TypeError) as e: yield from api.get_response((web.Response(text='foo', status=201, headers={'X-header': 'value'}), 200)) assert str(e.value) == "Cannot return web.StreamResponse in tuple. Only raw data can be returned in tuple." @@ -82,7 +83,7 @@ def test_get_response_from_dict_json(api): response = yield from api.get_response({'foo': 'bar'}, mimetype='application/json') assert isinstance(response, web.Response) assert response.status == 200 - assert response.body == b'{"foo":"bar"}' + assert json.loads(response.body) == {"foo": "bar"} assert response.content_type == 'application/json' assert dict(response.headers) == {'Content-Type': 'application/json; charset=utf-8'} @@ -93,8 +94,8 @@ def test_get_response_no_data(api): assert isinstance(response, web.Response) assert response.status == 204 assert response.body is None - assert response.content_type == 'application/octet-stream' - assert dict(response.headers) == {} + assert response.content_type == 'application/json' + assert dict(response.headers) == {'Content-Type': 'application/json'} @asyncio.coroutine @@ -102,7 +103,7 @@ def test_get_response_binary_json(api): response = yield from api.get_response(b'{"foo":"bar"}', mimetype='application/json') assert isinstance(response, web.Response) assert response.status == 200 - assert response.body == b'{"foo":"bar"}' + assert json.loads(response.body) == {"foo": "bar"} assert response.content_type == 'application/json' assert dict(response.headers) == {'Content-Type': 'application/json'} diff --git a/tests/api/test_responses.py b/tests/api/test_responses.py index f9c43f8a2..e5a787a4c 100644 --- a/tests/api/test_responses.py +++ b/tests/api/test_responses.py @@ -90,7 +90,7 @@ def test_not_content_response(simple_app): get_no_content_response = app_client.get('/v1.0/test_no_content_response') assert get_no_content_response.status_code == 204 - assert get_no_content_response.content_length in [0, None] + assert get_no_content_response.content_length is None def test_pass_through(simple_app): @@ -311,6 +311,7 @@ def test_get_enum_response(simple_app): resp = app_client.get('/v1.0/get_enum_response') assert resp.status_code == 200 + def test_get_httpstatus_response(simple_app): app_client = simple_app.app.test_client() resp = app_client.get('/v1.0/get_httpstatus_response') From 94a684cf67e212adf538cc8d881deb95b9062578 Mon Sep 17 00:00:00 2001 From: Julien Sagnard Date: Mon, 4 Feb 2019 16:29:24 +0100 Subject: [PATCH 04/17] Fix CI --- connexion/apis/abstract.py | 2 +- connexion/apis/aiohttp_api.py | 2 +- tests/aiohttp/test_get_response.py | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index 96a1b55e7..22fe2759b 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -2,9 +2,9 @@ import logging import pathlib import sys +from enum import Enum import six -from enum import Enum from ..decorators.produces import NoContent from ..exceptions import ResolverError diff --git a/connexion/apis/aiohttp_api.py b/connexion/apis/aiohttp_api.py index de2e4b27b..814824b56 100644 --- a/connexion/apis/aiohttp_api.py +++ b/connexion/apis/aiohttp_api.py @@ -17,7 +17,7 @@ from connexion.jsonifier import JSONEncoder, Jsonifier from connexion.lifecycle import ConnexionRequest, ConnexionResponse from connexion.problem import problem -from connexion.utils import is_json_mimetype, yamldumper +from connexion.utils import yamldumper from werkzeug.exceptions import HTTPException as werkzeug_HTTPException diff --git a/tests/aiohttp/test_get_response.py b/tests/aiohttp/test_get_response.py index 74e92686e..b55f97136 100644 --- a/tests/aiohttp/test_get_response.py +++ b/tests/aiohttp/test_get_response.py @@ -1,7 +1,9 @@ import asyncio -from aiohttp import web -import pytest import json + +import pytest + +from aiohttp import web from connexion.apis.aiohttp_api import AioHttpApi from connexion.lifecycle import ConnexionResponse @@ -83,7 +85,7 @@ def test_get_response_from_dict_json(api): response = yield from api.get_response({'foo': 'bar'}, mimetype='application/json') assert isinstance(response, web.Response) assert response.status == 200 - assert json.loads(response.body) == {"foo": "bar"} + assert json.loads(response.body.decode()) == {"foo": "bar"} assert response.content_type == 'application/json' assert dict(response.headers) == {'Content-Type': 'application/json; charset=utf-8'} @@ -103,7 +105,7 @@ def test_get_response_binary_json(api): response = yield from api.get_response(b'{"foo":"bar"}', mimetype='application/json') assert isinstance(response, web.Response) assert response.status == 200 - assert json.loads(response.body) == {"foo": "bar"} + assert json.loads(response.body.decode()) == {"foo": "bar"} assert response.content_type == 'application/json' assert dict(response.headers) == {'Content-Type': 'application/json'} @@ -146,5 +148,3 @@ def test_get_connexion_response_from_tuple(api): assert response.body == b'foo' assert response.content_type == 'text/plain' assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8', 'X-header': 'value'} - - From 11d05aab3f0ce0d1a4f12fce7edda8a744a1164c Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Wed, 4 Dec 2019 10:50:23 -0600 Subject: [PATCH 05/17] Drop six string types --- connexion/apis/abstract.py | 8 ++++---- connexion/apis/flask_api.py | 4 ++-- connexion/jsonifier.py | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index 22fe2759b..c59604d70 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -326,7 +326,7 @@ def get_connexion_response(cls, response, mimetype=None): if isinstance(response, ConnexionResponse): # If body in ConnexionResponse is not byte, it may not pass schema validation. # In this case, rebuild response with aiohttp to have consistency - if response.body is None or isinstance(response.body, six.binary_type): + if response.body is None or isinstance(response.body, bytes): return response else: response = cls._build_response( @@ -395,10 +395,10 @@ def _prepare_body_and_status_code(cls, data, mimetype, status_code=None): @classmethod def _jsonify_data(cls, data, mimetype): - if not isinstance(data, six.binary_type): - if isinstance(mimetype, six.string_types) and is_json_mimetype(mimetype): + if not isinstance(data, bytes): + if isinstance(mimetype, str) and is_json_mimetype(mimetype): body = cls.jsonifier.dumps(data) - elif isinstance(data, six.text_type): + elif isinstance(data, str): body = data else: body = str(data) diff --git a/connexion/apis/flask_api.py b/connexion/apis/flask_api.py index a4a27e39e..bb754a6c5 100644 --- a/connexion/apis/flask_api.py +++ b/connexion/apis/flask_api.py @@ -196,8 +196,8 @@ def _build_response(cls, mimetype, content_type=None, headers=None, status_code= @classmethod def _jsonify_data(cls, data, mimetype): # TODO: to discuss: Using jsonifier for all type of data, even when mimetype is not json is strange. Why ? - if (isinstance(mimetype, six.string_types) and is_json_mimetype(mimetype)) \ - or not (isinstance(data, six.binary_type) or isinstance(data, six.text_type)): + if (isinstance(mimetype, str) and is_json_mimetype(mimetype)) \ + or not (isinstance(data, bytes) or isinstance(data, str)): return cls.jsonifier.dumps(data) return data diff --git a/connexion/jsonifier.py b/connexion/jsonifier.py index 9bc5744a5..b656bea2b 100644 --- a/connexion/jsonifier.py +++ b/connexion/jsonifier.py @@ -49,11 +49,11 @@ def loads(self, data): """ Central point where JSON deserialization happens inside Connexion. """ - if isinstance(data, six.binary_type): + if isinstance(data, bytes): data = data.decode() try: return self.json.loads(data) except Exception: - if isinstance(data, six.string_types): + if isinstance(data, str): return data From c793e42f942b8a1a8aa7fc4cb5d81427f1eb478a Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Wed, 4 Dec 2019 12:01:43 -0600 Subject: [PATCH 06/17] Standardize response logging --- connexion/apis/abstract.py | 49 +++++++++++++++++++++++++---------- connexion/apis/aiohttp_api.py | 17 +++++------- connexion/apis/flask_api.py | 21 +++++---------- 3 files changed, 48 insertions(+), 39 deletions(-) diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index c59604d70..1d8828142 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -254,33 +254,42 @@ def get_response(self, response, mimetype=None, request=None): """ @classmethod - def _get_response(cls, response, mimetype=None, url=None): + def _get_response(cls, response, mimetype=None, extra_context=None): """ This method converts a handler response to a framework response. The response can be a ConnexionResponse, an operation handler, a framework response or a tuple. Other type than ConnexionResponse are handled by `cls._response_from_handler` :param response: A response to cast. :param mimetype: The response mimetype. - :param url: The url to write in logs + :param extra_context: dict of extra details, like url, to include in logs :type response: Framework Response :type mimetype: str """ + if extra_context is None: + extra_context = {} logger.debug('Getting data and status code', extra={ 'data': response, 'data_type': type(response), - 'url': url + **extra_context }) if isinstance(response, ConnexionResponse): - framework_response = cls._connexion_to_framework_response(response, mimetype) + framework_response = cls._connexion_to_framework_response(response, mimetype, extra_context) else: - framework_response = cls._response_from_handler(response, mimetype) + framework_response = cls._response_from_handler(response, mimetype, extra_context) + + logger.debug('Got framework response', + extra={ + 'response': framework_response, + 'response_type': type(framework_response), + **extra_context + }) return framework_response @classmethod - def _response_from_handler(cls, response, mimetype): + def _response_from_handler(cls, response, mimetype, extra_context=None): """ Create a framework response from the operation handler data. An operation handler can return: @@ -293,6 +302,8 @@ def _response_from_handler(cls, response, mimetype): :type response Union[Response, str, Tuple[str, int], Tuple[str, int, dict]] :param mimetype: The response mimetype. :type mimetype: str + :param extra_context: dict of extra details, like url, to include in logs + :type extra_context: Union[None, dict] :return A framework response. :rtype Response """ @@ -304,13 +315,13 @@ def _response_from_handler(cls, response, mimetype): if len_response == 2: if isinstance(response[1], (int, Enum)): data, status_code = response - return cls._build_response(mimetype=mimetype, data=data, status_code=status_code) + return cls._build_response(mimetype=mimetype, data=data, status_code=status_code, extra_context=extra_context) else: data, headers = response - return cls._build_response(mimetype=mimetype, data=data, headers=headers) + return cls._build_response(mimetype=mimetype, data=data, headers=headers, extra_context=extra_context) elif len_response == 3: data, status_code, headers = response - return cls._build_response(mimetype=mimetype, data=data, status_code=status_code, headers=headers) + return cls._build_response(mimetype=mimetype, data=data, status_code=status_code, headers=headers, extra_context=extra_context) else: raise TypeError( 'The view function did not return a valid response tuple.' @@ -318,7 +329,7 @@ def _response_from_handler(cls, response, mimetype): ' (body, status), or (body, headers).' ) else: - return cls._build_response(mimetype=mimetype, data=response) + return cls._build_response(mimetype=mimetype, data=response, extra_context=extra_context) @classmethod def get_connexion_response(cls, response, mimetype=None): @@ -353,12 +364,12 @@ def _framework_to_connexion_response(cls, response, mimetype): @classmethod @abc.abstractmethod - def _connexion_to_framework_response(cls, response, mimetype): + def _connexion_to_framework_response(cls, response, mimetype, extra_context=None): """ Cast ConnexionResponse to framework response class """ @classmethod @abc.abstractmethod - def _build_response(cls, data, mimetype, content_type=None, status_code=None, headers=None): + def _build_response(cls, data, mimetype, content_type=None, status_code=None, headers=None, extra_context=None): """ Create a framework response from the provided arguments. :param data: Body data. @@ -368,12 +379,14 @@ def _build_response(cls, data, mimetype, content_type=None, status_code=None, he :type status_code: int :param headers: The response status code. :type headers: Union[Iterable[Tuple[str, str]], Dict[str, str]] + :param extra_context: dict of extra details, like url, to include in logs + :type extra_context: Union[None, dict] :return A framework response. :rtype Response """ @classmethod - def _prepare_body_and_status_code(cls, data, mimetype, status_code=None): + def _prepare_body_and_status_code(cls, data, mimetype, status_code=None, extra_context=None): if data is NoContent: data = None @@ -391,6 +404,16 @@ def _prepare_body_and_status_code(cls, data, mimetype, status_code=None): body = cls._jsonify_data(data, mimetype) else: body = data + + if extra_context is None: + extra_context = {} + logger.debug('Prepared body and status code (%d)', + status_code, + extra={ + 'body': body, + **extra_context + }) + return body, status_code @classmethod diff --git a/connexion/apis/aiohttp_api.py b/connexion/apis/aiohttp_api.py index 814824b56..aebfcf064 100644 --- a/connexion/apis/aiohttp_api.py +++ b/connexion/apis/aiohttp_api.py @@ -310,13 +310,7 @@ def get_response(cls, response, mimetype=None, request=None): url = str(request.url) if request else '' - response = cls._get_response(response, mimetype=mimetype, url=url) - - # TODO: I let this log here for full compatibility. But if we can modify it, it can go to _get_response() - logger.debug('Got stream response with status code (%d)', - response.status, extra={'url': url}) - - return response + return cls._get_response(response, mimetype=mimetype, extra_context={"url": url}) @classmethod def _is_framework_response(cls, response): @@ -335,22 +329,23 @@ def _framework_to_connexion_response(cls, response, mimetype): ) @classmethod - def _connexion_to_framework_response(cls, response, mimetype): + def _connexion_to_framework_response(cls, response, mimetype, extra_context=None): """ Cast ConnexionResponse to framework response class """ return cls._build_response( mimetype=response.mimetype or mimetype, status_code=response.status_code, content_type=response.content_type, headers=response.headers, - data=response.body + data=response.body, + extra_context=extra_context, ) @classmethod - def _build_response(cls, data, mimetype, content_type=None, headers=None, status_code=None): + def _build_response(cls, data, mimetype, content_type=None, headers=None, status_code=None, extra_context=None): if isinstance(data, web.StreamResponse): raise TypeError("Cannot return web.StreamResponse in tuple. Only raw data can be returned in tuple.") - data, status_code = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code) + data, status_code = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code, extra_context=extra_context) if isinstance(data, str): text = data diff --git a/connexion/apis/flask_api.py b/connexion/apis/flask_api.py index bb754a6c5..b2487a2f2 100644 --- a/connexion/apis/flask_api.py +++ b/connexion/apis/flask_api.py @@ -135,17 +135,7 @@ def get_response(cls, response, mimetype=None, request=None): :type operation_handler_result: flask.Response | (flask.Response, int) | (flask.Response, int, dict) :rtype: ConnexionResponse """ - flask_response = cls._get_response(response, mimetype=mimetype, url=flask.request.url) - - # TODO: I let this log here for full compatibility. But if we can modify it, it can go to _get_response() - logger.debug('Got data and status code (%d)', - flask_response.status_code, - extra={ - 'data': response, - 'url': flask.request.url - }) - - return flask_response + return cls._get_response(response, mimetype=mimetype, extra_context={"url": flask.request.url}) @classmethod def _is_framework_response(cls, response): @@ -164,24 +154,25 @@ def _framework_to_connexion_response(cls, response, mimetype): ) @classmethod - def _connexion_to_framework_response(cls, response, mimetype): + def _connexion_to_framework_response(cls, response, mimetype, extra_context=None): """ Cast ConnexionResponse to framework response class """ flask_response = cls._build_response( mimetype=response.mimetype or mimetype, content_type=response.content_type, headers=response.headers, status_code=response.status_code, - data=response.body + data=response.body, + extra_context=extra_context, ) return flask_response @classmethod - def _build_response(cls, mimetype, content_type=None, headers=None, status_code=None, data=None): + def _build_response(cls, mimetype, content_type=None, headers=None, status_code=None, data=None, extra_context=None): if flask_utils.is_flask_response(data): return flask.current_app.make_response((data, status_code, headers)) - data, status_code = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code) + data, status_code = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code, extra_context=extra_context) kwargs = { 'mimetype': mimetype, From c77bc4d667c5f9d35fade4fad677f56455556f0e Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Wed, 4 Dec 2019 12:26:24 -0600 Subject: [PATCH 07/17] Handle one-tuples that only contain data --- connexion/apis/abstract.py | 3 +++ connexion/apis/aiohttp_api.py | 2 +- connexion/apis/flask_api.py | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index 1d8828142..d37d056c9 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -312,6 +312,9 @@ def _response_from_handler(cls, response, mimetype, extra_context=None): if isinstance(response, tuple): len_response = len(response) + if len_response == 1: + data, = response + return cls._build_response(mimetype=mimetype, data=data, extra_context=extra_context) if len_response == 2: if isinstance(response[1], (int, Enum)): data, status_code = response diff --git a/connexion/apis/aiohttp_api.py b/connexion/apis/aiohttp_api.py index aebfcf064..078a8f9ed 100644 --- a/connexion/apis/aiohttp_api.py +++ b/connexion/apis/aiohttp_api.py @@ -342,7 +342,7 @@ def _connexion_to_framework_response(cls, response, mimetype, extra_context=None @classmethod def _build_response(cls, data, mimetype, content_type=None, headers=None, status_code=None, extra_context=None): - if isinstance(data, web.StreamResponse): + if cls._is_framework_response(data): raise TypeError("Cannot return web.StreamResponse in tuple. Only raw data can be returned in tuple.") data, status_code = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code, extra_context=extra_context) diff --git a/connexion/apis/flask_api.py b/connexion/apis/flask_api.py index b2487a2f2..424b9a8f2 100644 --- a/connexion/apis/flask_api.py +++ b/connexion/apis/flask_api.py @@ -169,7 +169,7 @@ def _connexion_to_framework_response(cls, response, mimetype, extra_context=None @classmethod def _build_response(cls, mimetype, content_type=None, headers=None, status_code=None, data=None, extra_context=None): - if flask_utils.is_flask_response(data): + if cls._is_framework_response(data): return flask.current_app.make_response((data, status_code, headers)) data, status_code = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code, extra_context=extra_context) From 2ba9e9a6a5b84f9967506032779da7bcaa437f12 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Wed, 4 Dec 2019 14:06:56 -0600 Subject: [PATCH 08/17] clean up a couple of type hint comments --- connexion/apis/abstract.py | 15 ++++++--------- connexion/apis/aiohttp_api.py | 1 + connexion/apis/flask_api.py | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index d37d056c9..495b02512 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -245,12 +245,10 @@ def get_response(self, response, mimetype=None, request=None): This method converts a handler response to a framework response. This method should just retrieve response from handler then call `cls._get_response`. It is mainly here to handle AioHttp async handler. - :param response: A response to cast. + :param response: A response to cast (tuple, framework response, etc). :param mimetype: The response mimetype. + :type mimetype: Union[None, str] :param request: The request associated with this response (the user framework request). - - :type response: Framework Response - :type mimetype: str """ @classmethod @@ -259,12 +257,11 @@ def _get_response(cls, response, mimetype=None, extra_context=None): This method converts a handler response to a framework response. The response can be a ConnexionResponse, an operation handler, a framework response or a tuple. Other type than ConnexionResponse are handled by `cls._response_from_handler` - :param response: A response to cast. + :param response: A response to cast (tuple, framework response, etc). :param mimetype: The response mimetype. + :type mimetype: Union[None, str] :param extra_context: dict of extra details, like url, to include in logs - - :type response: Framework Response - :type mimetype: str + :type extra_context: Union[None, dict] """ if extra_context is None: extra_context = {} @@ -299,7 +296,7 @@ def _response_from_handler(cls, response, mimetype, extra_context=None): - a tuple of (body: str, status_code: int) - a tuple of (body: str, status_code: int, headers: dict) :param response: A response from an operation handler. - :type response Union[Response, str, Tuple[str, int], Tuple[str, int, dict]] + :type response Union[Response, str, Tuple[str,], Tuple[str, int], Tuple[str, int, dict]] :param mimetype: The response mimetype. :type mimetype: str :param extra_context: dict of extra details, like url, to include in logs diff --git a/connexion/apis/aiohttp_api.py b/connexion/apis/aiohttp_api.py index 078a8f9ed..91a7b13c8 100644 --- a/connexion/apis/aiohttp_api.py +++ b/connexion/apis/aiohttp_api.py @@ -303,6 +303,7 @@ def get_response(cls, response, mimetype=None, request=None): """Get response. This method is used in the lifecycle decorators + :type response: aiohttp.web.StreamResponse | (Any,) | (Any, int) | (Any, dict) | (Any, int, dict) :rtype: aiohttp.web.Response """ while asyncio.iscoroutine(response): diff --git a/connexion/apis/flask_api.py b/connexion/apis/flask_api.py index 424b9a8f2..4e100638e 100644 --- a/connexion/apis/flask_api.py +++ b/connexion/apis/flask_api.py @@ -132,7 +132,7 @@ def get_response(cls, response, mimetype=None, request=None): If the returned object is a flask.Response then it will just pass the information needed to recreate it. - :type operation_handler_result: flask.Response | (flask.Response, int) | (flask.Response, int, dict) + :type response: flask.Response | (flask.Response,) | (flask.Response, int) | (flask.Response, dict) | (flask.Response, int, dict) :rtype: ConnexionResponse """ return cls._get_response(response, mimetype=mimetype, extra_context={"url": flask.request.url}) From f3e763efefdacf7e48339bc2db6657430b56cd99 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 6 Dec 2019 19:16:28 -0600 Subject: [PATCH 09/17] Add a few more get_response tests --- tests/aiohttp/test_get_response.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/aiohttp/test_get_response.py b/tests/aiohttp/test_get_response.py index b55f97136..8fbeaa640 100644 --- a/tests/aiohttp/test_get_response.py +++ b/tests/aiohttp/test_get_response.py @@ -43,6 +43,16 @@ def test_get_response_from_string(api): assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8'} +@asyncio.coroutine +def test_get_response_from_string_tuple(api): + response = yield from api.get_response(('foo',)) + assert isinstance(response, web.Response) + assert response.status == 200 + assert response.body == b'foo' + assert response.content_type == 'text/plain' + assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8'} + + @asyncio.coroutine def test_get_response_from_string_status(api): response = yield from api.get_response(('foo', 201)) @@ -53,6 +63,16 @@ def test_get_response_from_string_status(api): assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8'} +@asyncio.coroutine +def test_get_response_from_string_headers(api): + response = yield from api.get_response(('foo', {'X-header': 'value'})) + assert isinstance(response, web.Response) + assert response.status == 200 + assert response.body == b'foo' + assert response.content_type == 'text/plain' + assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8', 'X-header': 'value'} + + @asyncio.coroutine def test_get_response_from_string_status_headers(api): response = yield from api.get_response(('foo', 201, {'X-header': 'value'})) @@ -75,6 +95,8 @@ def test_get_response_from_dict(api): response = yield from api.get_response({'foo': 'bar'}) assert isinstance(response, web.Response) assert response.status == 200 + # odd, yes. but backwards compatible. see test_response_with_non_str_and_non_json_body in tests/aiohttp/test_aiohttp_simple_api.py + # TODO: This should be made into JSON when aiohttp and flask serialization can be harmonized. assert response.body == b"{'foo': 'bar'}" assert response.content_type == 'text/plain' assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8'} From e57e6a61b4f57a23afb731040735f602043e70f5 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 6 Dec 2019 19:24:06 -0600 Subject: [PATCH 10/17] Adjust _prepare_body interface to simplify improving _serialize_data Rename _jsonify_data to _serialize_data to make its purpose easier to understand (this was also known as _cast_body in aiohttp_api). In exploring how to harmonize json serialization between aiothttp and flask, we needed to be able to adjust the mimetype from within _serialize_data. Harmonizing the actual serialization has to wait until backwards incompatible changes can be made, but we can keep the new interface, as these functions were introduced in this PR (#849). --- connexion/apis/abstract.py | 9 +++++---- connexion/apis/aiohttp_api.py | 4 ++-- connexion/apis/flask_api.py | 13 +++++++------ 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index 495b02512..98548ac63 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -401,7 +401,7 @@ def _prepare_body_and_status_code(cls, data, mimetype, status_code=None, extra_c status_code = status_code.value if data is not None: - body = cls._jsonify_data(data, mimetype) + body, mimetype = cls._serialize_data(data, mimetype) else: body = data @@ -414,10 +414,11 @@ def _prepare_body_and_status_code(cls, data, mimetype, status_code=None, extra_c **extra_context }) - return body, status_code + return body, status_code, mimetype @classmethod - def _jsonify_data(cls, data, mimetype): + def _serialize_data(cls, data, mimetype): + # TODO: Harmonize with flask_api. Currently this is the backwards compatible with aiohttp_api._cast_body. if not isinstance(data, bytes): if isinstance(mimetype, str) and is_json_mimetype(mimetype): body = cls.jsonifier.dumps(data) @@ -427,7 +428,7 @@ def _jsonify_data(cls, data, mimetype): body = str(data) else: body = data - return body + return body, mimetype def json_loads(self, data): return self.jsonifier.loads(data) diff --git a/connexion/apis/aiohttp_api.py b/connexion/apis/aiohttp_api.py index 91a7b13c8..e32f6e8bc 100644 --- a/connexion/apis/aiohttp_api.py +++ b/connexion/apis/aiohttp_api.py @@ -346,7 +346,7 @@ def _build_response(cls, data, mimetype, content_type=None, headers=None, status if cls._is_framework_response(data): raise TypeError("Cannot return web.StreamResponse in tuple. Only raw data can be returned in tuple.") - data, status_code = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code, extra_context=extra_context) + data, status_code, serialized_mimetype = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code, extra_context=extra_context) if isinstance(data, str): text = data @@ -355,7 +355,7 @@ def _build_response(cls, data, mimetype, content_type=None, headers=None, status text = None body = data - content_type = content_type or mimetype + content_type = content_type or mimetype or serialized_mimetype return web.Response(body=body, text=text, headers=headers, status=status_code, content_type=content_type) @classmethod diff --git a/connexion/apis/flask_api.py b/connexion/apis/flask_api.py index 4e100638e..2300d8a73 100644 --- a/connexion/apis/flask_api.py +++ b/connexion/apis/flask_api.py @@ -172,10 +172,10 @@ def _build_response(cls, mimetype, content_type=None, headers=None, status_code= if cls._is_framework_response(data): return flask.current_app.make_response((data, status_code, headers)) - data, status_code = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code, extra_context=extra_context) + data, status_code, serialized_mimetype = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code, extra_context=extra_context) kwargs = { - 'mimetype': mimetype, + 'mimetype': mimetype or serialized_mimetype, 'content_type': content_type, 'headers': headers, 'response': data, @@ -185,13 +185,14 @@ def _build_response(cls, mimetype, content_type=None, headers=None, status_code= return flask.current_app.response_class(**kwargs) # type: flask.Response @classmethod - def _jsonify_data(cls, data, mimetype): - # TODO: to discuss: Using jsonifier for all type of data, even when mimetype is not json is strange. Why ? + def _serialize_data(cls, data, mimetype): + # TODO: harmonize flask and aiohttp serialization when mimetype=None or mimetype is not JSON + # (cases where it might not make sense to jsonify the data) if (isinstance(mimetype, str) and is_json_mimetype(mimetype)) \ or not (isinstance(data, bytes) or isinstance(data, str)): - return cls.jsonifier.dumps(data) + return cls.jsonifier.dumps(data), mimetype - return data + return data, mimetype @classmethod def get_request(cls, *args, **params): From 5e9066a307e94c22c688b9083ef7eaa7c71b867b Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Thu, 5 Dec 2019 18:02:52 -0600 Subject: [PATCH 11/17] serialize dicts,lists as json when no mimetype is supplied Also adds a couple of missing get_response tests. --- connexion/apis/abstract.py | 4 +++- tests/aiohttp/test_get_response.py | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index 98548ac63..0329041a7 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -420,7 +420,9 @@ def _prepare_body_and_status_code(cls, data, mimetype, status_code=None, extra_c def _serialize_data(cls, data, mimetype): # TODO: Harmonize with flask_api. Currently this is the backwards compatible with aiohttp_api._cast_body. if not isinstance(data, bytes): - if isinstance(mimetype, str) and is_json_mimetype(mimetype): + # assume mimetype is json if mimetype is None and it looks like json + if (mimetype is None and isinstance(data, (dict, list, tuple))) \ + or (isinstance(mimetype, str) and is_json_mimetype(mimetype)): body = cls.jsonifier.dumps(data) elif isinstance(data, str): body = data diff --git a/tests/aiohttp/test_get_response.py b/tests/aiohttp/test_get_response.py index 8fbeaa640..f70e72a89 100644 --- a/tests/aiohttp/test_get_response.py +++ b/tests/aiohttp/test_get_response.py @@ -92,12 +92,12 @@ def test_get_response_from_tuple_error(api): @asyncio.coroutine def test_get_response_from_dict(api): + # mimetype=None => assume json response = yield from api.get_response({'foo': 'bar'}) assert isinstance(response, web.Response) assert response.status == 200 - # odd, yes. but backwards compatible. see test_response_with_non_str_and_non_json_body in tests/aiohttp/test_aiohttp_simple_api.py - # TODO: This should be made into JSON when aiohttp and flask serialization can be harmonized. - assert response.body == b"{'foo': 'bar'}" + assert json.loads(response.body.decode()) == {"foo": "bar"} + # get_response does not modify the mimetype or content_type magically. It only simplifies serialization. assert response.content_type == 'text/plain' assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8'} From 619ecc8b87a1a8c4c806a1220d68d55d5ee8318a Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 6 Dec 2019 16:50:44 -0600 Subject: [PATCH 12/17] deduplicate DEFAULT_MIMETYPE constant --- connexion/operations/__init__.py | 1 + connexion/operations/abstract.py | 3 +-- connexion/operations/mimetype.py | 1 + connexion/operations/secure.py | 3 +-- 4 files changed, 4 insertions(+), 4 deletions(-) create mode 100644 connexion/operations/mimetype.py diff --git a/connexion/operations/__init__.py b/connexion/operations/__init__.py index 4c44b9f38..16c0998c3 100644 --- a/connexion/operations/__init__.py +++ b/connexion/operations/__init__.py @@ -2,6 +2,7 @@ from .openapi import OpenAPIOperation # noqa from .secure import SecureOperation # noqa from .swagger2 import Swagger2Operation # noqa +from .mimetype import DEFAULT_MIMETYPE # noqa def make_operation(spec, *args, **kwargs): diff --git a/connexion/operations/abstract.py b/connexion/operations/abstract.py index f009f5991..e44d00199 100644 --- a/connexion/operations/abstract.py +++ b/connexion/operations/abstract.py @@ -4,6 +4,7 @@ import six from connexion.operations.secure import SecureOperation +from .mimetype import DEFAULT_MIMETYPE from ..decorators.metrics import UWSGIMetricsCollector from ..decorators.parameter import parameter_to_arg from ..decorators.produces import BaseSerializer, Produces @@ -13,8 +14,6 @@ logger = logging.getLogger('connexion.operations.abstract') -DEFAULT_MIMETYPE = 'application/json' - VALIDATOR_MAP = { 'parameter': ParameterValidator, 'body': RequestBodyValidator, diff --git a/connexion/operations/mimetype.py b/connexion/operations/mimetype.py new file mode 100644 index 000000000..ad0bcf137 --- /dev/null +++ b/connexion/operations/mimetype.py @@ -0,0 +1 @@ +DEFAULT_MIMETYPE = 'application/json' diff --git a/connexion/operations/secure.py b/connexion/operations/secure.py index 4d9c617e7..51cba83c0 100644 --- a/connexion/operations/secure.py +++ b/connexion/operations/secure.py @@ -1,6 +1,7 @@ import functools import logging +from .mimetype import DEFAULT_MIMETYPE from ..decorators.decorator import RequestResponseDecorator from ..decorators.security import (get_apikeyinfo_func, get_basicinfo_func, get_bearerinfo_func, @@ -11,8 +12,6 @@ logger = logging.getLogger("connexion.operations.secure") -DEFAULT_MIMETYPE = 'application/json' - class SecureOperation(object): From ac704789e1684c9d970f5c93c3e9b847a820d227 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 6 Dec 2019 16:51:12 -0600 Subject: [PATCH 13/17] Standardize data serialization The old aiohttp_api._cast_body did not jsonify when mimetype was None. The old flask_api._jsonify_data jsonified data, or raised a TypeError if the data could not be jsonified, even when mimetype was not None and when mimetype was not a JSON mimetype. This unifies the behavior so that only JSON or None mimetypes can trigger jsonification, and only JSON mimetypes will raise a TypeError if the data is not jsonifiable. Non-JSON mimetypes and non-jsonifiable data with None mimetype are stringified. This means that the data can serialize itself when - mimetype is None and data is not jsonifiable - mimetype is not a JSON mimetype This removes the flask_api version of _jsonify_data. This should be backwards compatible because: For the non-jsonifiable data, when mimetype is None or a non-json type, using str() instead of throwing TypeError is an improvement. Could anyone depend on throwing a TypeError in this case? It would just throw 500 errors, but this makes that more useful. So it is a "new feature". The only gotcha is when mimetype is not None and is not JSON, but the data is jsonifiable, we should NOT serialize it as JSON, but today we are for flask but not for aiohttp. Instead flask_api should just cast it as a string (like aiohttp does) assuming that if someone wanted a non-json type, they will handle serialization, possibly by subclassing dict with their own__str__ implementation or something. This is probably a bugfix (and therefore ok to introduce now) because jsonifying data with a non-json mimetype with flask does not make sense, even if the data is jsonifiable. --- connexion/apis/abstract.py | 33 ++++++++++++++++++++++++++++----- connexion/apis/flask_api.py | 10 ---------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index 0329041a7..7626e0d1a 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -11,7 +11,7 @@ from ..http_facts import METHODS from ..jsonifier import Jsonifier from ..lifecycle import ConnexionResponse -from ..operations import make_operation +from ..operations import make_operation, DEFAULT_MIMETYPE from ..options import ConnexionOptions from ..resolver import Resolver from ..spec import Specification @@ -418,14 +418,37 @@ def _prepare_body_and_status_code(cls, data, mimetype, status_code=None, extra_c @classmethod def _serialize_data(cls, data, mimetype): - # TODO: Harmonize with flask_api. Currently this is the backwards compatible with aiohttp_api._cast_body. + """ Serialize data (aka: _jsonify_data or _cast_body). + + The old aiohttp_api._cast_body did not jsonify when mimetype was None. + The old flask_api._jsonify_data jsonified data, or raised a TypeError if the + data could not be jsonified, even with non-JSON or None mimetypes. + + This unifies the behavior so that only JSON or None mimetypes can trigger + jsonification, and only JSON mimetypes will raise a TypeError if the data + is not jsonifiable. Non-JSON mimetypes and non-jsonifiable data with None + mimetype are stringified. + + This means that the data can serialize itself when + - mimetype is None and data is not jsonifiable, or + - mimetype is not a JSON mimetype + + mimetype, in general, is only None when the spec did not define `produces`. + """ if not isinstance(data, bytes): - # assume mimetype is json if mimetype is None and it looks like json - if (mimetype is None and isinstance(data, (dict, list, tuple))) \ - or (isinstance(mimetype, str) and is_json_mimetype(mimetype)): + if isinstance(mimetype, str) and is_json_mimetype(mimetype): body = cls.jsonifier.dumps(data) elif isinstance(data, str): body = data + elif mimetype is None: + try: + # try as json by default + body = cls.jsonifier.dumps(data) + except TypeError: + # or let objects self-serialize + body = str(data) + else: + mimetype = DEFAULT_MIMETYPE else: body = str(data) else: diff --git a/connexion/apis/flask_api.py b/connexion/apis/flask_api.py index 2300d8a73..8fb94f8b9 100644 --- a/connexion/apis/flask_api.py +++ b/connexion/apis/flask_api.py @@ -184,16 +184,6 @@ def _build_response(cls, mimetype, content_type=None, headers=None, status_code= kwargs = {k: v for k, v in six.iteritems(kwargs) if v is not None} return flask.current_app.response_class(**kwargs) # type: flask.Response - @classmethod - def _serialize_data(cls, data, mimetype): - # TODO: harmonize flask and aiohttp serialization when mimetype=None or mimetype is not JSON - # (cases where it might not make sense to jsonify the data) - if (isinstance(mimetype, str) and is_json_mimetype(mimetype)) \ - or not (isinstance(data, bytes) or isinstance(data, str)): - return cls.jsonifier.dumps(data), mimetype - - return data, mimetype - @classmethod def get_request(cls, *args, **params): # type: (*Any, **Any) -> ConnexionRequest From d49b5241d748b1e5600cc8128ee31534ef8820ed Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 6 Dec 2019 17:29:52 -0600 Subject: [PATCH 14/17] fix aiohttp get_response test and add debug to understand better --- connexion/apis/abstract.py | 2 ++ tests/aiohttp/test_get_response.py | 5 ++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index 7626e0d1a..dff4810c4 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -447,9 +447,11 @@ def _serialize_data(cls, data, mimetype): except TypeError: # or let objects self-serialize body = str(data) + logger.debug('_serialize_data mimetype=None and str()') else: mimetype = DEFAULT_MIMETYPE else: + logger.debug('_serialize_data mimetype={} and str()'.format(mimetype)) body = str(data) else: body = data diff --git a/tests/aiohttp/test_get_response.py b/tests/aiohttp/test_get_response.py index f70e72a89..34402436e 100644 --- a/tests/aiohttp/test_get_response.py +++ b/tests/aiohttp/test_get_response.py @@ -97,9 +97,8 @@ def test_get_response_from_dict(api): assert isinstance(response, web.Response) assert response.status == 200 assert json.loads(response.body.decode()) == {"foo": "bar"} - # get_response does not modify the mimetype or content_type magically. It only simplifies serialization. - assert response.content_type == 'text/plain' - assert dict(response.headers) == {'Content-Type': 'text/plain; charset=utf-8'} + assert response.content_type == 'application/json' + assert dict(response.headers) == {'Content-Type': 'application/json; charset=utf-8'} @asyncio.coroutine From bd3056c3617da7d55882a0698d3c06af70704926 Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 6 Dec 2019 17:35:50 -0600 Subject: [PATCH 15/17] special case for text/plain --- connexion/apis/abstract.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index dff4810c4..e367da1e0 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -440,14 +440,14 @@ def _serialize_data(cls, data, mimetype): body = cls.jsonifier.dumps(data) elif isinstance(data, str): body = data - elif mimetype is None: + elif mimetype in [None, "text/plain"]: try: # try as json by default body = cls.jsonifier.dumps(data) except TypeError: # or let objects self-serialize body = str(data) - logger.debug('_serialize_data mimetype=None and str()') + logger.debug('_serialize_data mimetype={} and jsonify failed and str()'.format(mimetype)) else: mimetype = DEFAULT_MIMETYPE else: From 1b141c5af12b019c0c3e8d3f43336ef824a8675f Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 6 Dec 2019 18:11:47 -0600 Subject: [PATCH 16/17] make the default be json --- connexion/decorators/produces.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/connexion/decorators/produces.py b/connexion/decorators/produces.py index b105d889a..1878fffca 100644 --- a/connexion/decorators/produces.py +++ b/connexion/decorators/produces.py @@ -12,7 +12,8 @@ class BaseSerializer(BaseDecorator): - def __init__(self, mimetype='text/plain'): + #def __init__(self, mimetype='text/plain'): + def __init__(self, mimetype='application/json'): """ :type mimetype: str """ From 935533d44860ac3b6d34f877af13218fca14364a Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Fri, 6 Dec 2019 18:36:40 -0600 Subject: [PATCH 17/17] update non_str_non_json aiohttp test --- tests/aiohttp/test_aiohttp_simple_api.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/aiohttp/test_aiohttp_simple_api.py b/tests/aiohttp/test_aiohttp_simple_api.py index 94147c17d..dc979ec6d 100644 --- a/tests/aiohttp/test_aiohttp_simple_api.py +++ b/tests/aiohttp/test_aiohttp_simple_api.py @@ -265,7 +265,9 @@ def test_response_with_non_str_and_non_json_body(aiohttp_app, aiohttp_client): '/v1.0/aiohttp_non_str_non_json_response' ) assert get_bye.status == 200 - assert (yield from get_bye.read()) == b'1234' + # \n comes from jsonifier.dumps. text/plain gets serialized as json if possible + # as that json representation should generally be more useful then python literals. + assert (yield from get_bye.read()) == b'1234\n' @asyncio.coroutine