Return tuple for aiohttp (#849)

* Support aiohttp handlers to return tuples

* Minor update from #828 review

* Factorize more code between Flask and AioHttp response

* Fix CI

* Drop six string types

* Standardize response logging

* Handle one-tuples that only contain data

* clean up a couple of type hint comments

* Add a few more get_response tests

* 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).

* Add deprecation warnings about implicit serialization

.gitignore

@@ -11,4 +11,6 @@ htmlcov/
 *.swp
 .tox/
 .idea/
-venv/
+.vscode/
+venv/
+src/

connexion/apis/abstract.py

@@ -2,16 +2,21 @@
 import logging
 import pathlib
 import sys
+import warnings
+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'
@@ -238,23 +243,203 @@ 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.
-        :param response: A response to cast.
+        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 (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: ConnexionResponse
+    @classmethod
+    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 (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 extra_context: Union[None, dict]
+        """
+        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,], 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
-    @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, 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):
         """
-        This method converts the user framework response to a ConnexionResponse.
-        :param response: A response to cast.
+        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, mimetype = cls._serialize_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, mimetype
+
+    @classmethod
+    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)
+            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
+        return body, mimetype
+
     def json_loads(self, data):
         return self.jsonifier.loads(data)
 

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
 
 
@@ -303,73 +303,60 @@ 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):
             response = yield from response
 
         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, serialized_mimetype = 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 or serialized_mimetype
+        return web.Response(body=body, text=text, headers=headers, status=status_code, content_type=content_type)
 
     @classmethod
     def _set_jsonifier(cls):