From 789f1e99aeb9c116e4feb57abbcafc5e9dda70b8 Mon Sep 17 00:00:00 2001 From: Julien Sagnard Date: Tue, 8 Jan 2019 18:32:08 +0100 Subject: [PATCH 01/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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 12755118f98a2cb3a3985f6549ac30ccf7d0e83e Mon Sep 17 00:00:00 2001 From: Jacob Floyd Date: Sat, 7 Dec 2019 00:02:55 -0600 Subject: [PATCH 11/11] Add deprecation warnings about implicit serialization --- connexion/apis/abstract.py | 10 ++++++++++ connexion/apis/flask_api.py | 21 +++++++++++++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/connexion/apis/abstract.py b/connexion/apis/abstract.py index 98548ac63..09a055fe9 100644 --- a/connexion/apis/abstract.py +++ b/connexion/apis/abstract.py @@ -2,6 +2,7 @@ import logging import pathlib import sys +import warnings from enum import Enum import six @@ -425,6 +426,15 @@ def _serialize_data(cls, data, mimetype): elif isinstance(data, str): body = data else: + warnings.warn( + "Implicit (aiohttp) serialization with str() will change in the next major version. " + "This is triggered because a non-JSON response body is being stringified. " + "This will be replaced by something that is mimetype-specific and may " + "serialize some things as JSON or throw an error instead of silently " + "stringifying unknown response bodies. " + "Please make sure to specify media/mime types in your specs.", + FutureWarning # a Deprecation targeted at application users. + ) body = str(data) else: body = data diff --git a/connexion/apis/flask_api.py b/connexion/apis/flask_api.py index 2300d8a73..7c4d5131b 100644 --- a/connexion/apis/flask_api.py +++ b/connexion/apis/flask_api.py @@ -1,4 +1,5 @@ import logging +import warnings import flask import six @@ -188,11 +189,23 @@ def _build_response(cls, mimetype, content_type=None, headers=None, status_code= 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 + if (isinstance(mimetype, str) and is_json_mimetype(mimetype)): + body = cls.jsonifier.dumps(data) + elif not (isinstance(data, bytes) or isinstance(data, str)): + warnings.warn( + "Implicit (flask) JSON serialization will change in the next major version. " + "This is triggered because a response body is being serialized as JSON " + "even though the mimetype is not a JSON type. " + "This will be replaced by something that is mimetype-specific and may " + "raise an error instead of silently converting everything to JSON. " + "Please make sure to specify media/mime types in your specs.", + FutureWarning # a Deprecation targeted at application users. + ) + body = cls.jsonifier.dumps(data) + else: + body = data - return data, mimetype + return body, mimetype @classmethod def get_request(cls, *args, **params):