Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return tuple for aiohttp #849

Merged
merged 11 commits into from
Dec 11, 2019
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,6 @@ htmlcov/
*.swp
.tox/
.idea/
venv/
.vscode/
venv/
src/
187 changes: 182 additions & 5 deletions connexion/apis/abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,20 @@
import logging
import pathlib
import sys
from enum import Enum

import six

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'
Expand Down Expand Up @@ -238,22 +242,195 @@ 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.
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: ConnexionResponse
:type response: Framework Response
dtkav marked this conversation as resolved.
Show resolved Hide resolved
:type mimetype: str
"""

@classmethod
@abc.abstractmethod
def get_connexion_response(cls, response, mimetype=None):
def _get_response(cls, response, mimetype=None, extra_context=None):
"""
This method converts the user framework response to a ConnexionResponse.
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 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),
**extra_context
})

if isinstance(response, ConnexionResponse):
framework_response = cls._connexion_to_framework_response(response, mimetype, extra_context)
else:
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, extra_context=None):
"""
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
: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
"""
if cls._is_framework_response(response):
return response

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
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, 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, extra_context=extra_context)
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, extra_context=extra_context)

@classmethod
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, bytes):
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, 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, extra_context=None):
"""
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]]
: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, extra_context=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

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
def _jsonify_data(cls, data, mimetype):
if not isinstance(data, bytes):
if isinstance(mimetype, str) and is_json_mimetype(mimetype):
body = cls.jsonifier.dumps(data)
elif isinstance(data, str):
body = data
else:
body = str(data)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are some other datatypes where the string representation is better? I'm having trouble thinking of any.
For both int and float, I think I'd prefer the json representation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. Now, when mimetype is None, anything that can be jsonified will be, catching TypeError to rescue with str(). Duck-typing for the win - if it quacks like JSON and the mimetype is None, then it is JSON.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is getting really close.
Eventually we want to be able to register serializers for arbitrary mimetypes, so I think there's a default path (mimetype is none, try to do json), and then maybe some built-in overridable serializers (text/plain uses str() for example).
So if we can get backwards compatible behavior with flask (default serializer is json), but an escape hatch registering new mimetypes/serializers then I think we're golden.
One thing I don't think we should do is introduce anything opinionated, or anything too magical/surprising.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out that it is magical and surprising no matter how I look at it. From both aiohttp and flask sides, it feels a little off. :(

else:
body = data
return body
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compare this (from aiohttp_api, now in abstract) with the flask_api _jsonify_data.


def json_loads(self, data):
return self.jsonifier.loads(data)
Expand Down
72 changes: 29 additions & 43 deletions connexion/apis/aiohttp_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -310,66 +310,52 @@ 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)

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})

return response
return cls._get_response(response, mimetype=mimetype, extra_context={"url": url})

@classmethod
def get_connexion_response(cls, response, mimetype=None):
response.body = cls._cast_body(response.body, mimetype)

if isinstance(response, ConnexionResponse):
return response
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 if response.content_type else \
response.mimetype if response.mimetype else mimetype

body = cls._cast_body(response.body, content_type)

return web.Response(
status=response.status_code,
content_type=content_type,
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,
body=body
data=response.body,
extra_context=extra_context,
)

@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 _build_response(cls, data, mimetype, content_type=None, headers=None, status_code=None, extra_context=None):
if cls._is_framework_response(data):
raise TypeError("Cannot return web.StreamResponse in tuple. Only raw data can be returned in tuple.")

elif isinstance(body, str):
return body.encode()
data, status_code = cls._prepare_body_and_status_code(data=data, mimetype=mimetype, status_code=status_code, extra_context=extra_context)

else:
return str(body).encode()
if isinstance(data, str):
text = data
body = None
else:
return body
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
def _set_jsonifier(cls):
Expand Down
Loading