Fix for aiohttp and multipart/form-data uploads

connexion/apis/aiohttp_api.py

@@ -293,13 +293,53 @@ async def get_request(cls, req):
         :rtype: ConnexionRequest
         """
         url = str(req.url)
-        logger.debug('Getting data and status code',
-                     extra={'has_body': req.has_body, 'url': url})
+
+        logger.debug(
+            'Getting data and status code',
+            extra={
+                # has_body | can_read_body report if
+                # body has been read or not
+                # body_exists refers to underlying stream of data
+                'body_exists': req.body_exists,
+                'can_read_body': req.can_read_body,
+                'content_type': req.content_type,
+                'url': url,
+            },
+        )
 
         query = parse_qs(req.rel_url.query_string)
         headers = req.headers
         body = None
-        if req.body_exists:
+
+        # if request is not multipart, `data` will be empty dict
+        # and stream will not be consumed
+        post_data = await req.post()
+
+        # set those up beforehand, they are needed anyway
+        files = {}
+        form = {}
+
+        if post_data:
+            logger.debug('Reading multipart data from request')
+            for k, v in post_data.items():
+                if isinstance(v, web.FileField):
+                    if k in files:
+                        # if multiple files arrive under the same name in the
+                        # request, downstream requires that we put them all into
+                        # a list under the same key in the files dict.
+                        if isinstance(files[k], list):
+                            files[k].append(v)
+                        else:
+                            files[k] = [files[k], v]
+                    else:
+                        files[k] = v
+                else:
+                    # put normal fields as an array, that's how werkzeug does that for Flask
+                    # and that's what Connexion expects in its processing functions
+                    form[k] = [v]
+            body = b''
+        else:
+            logger.debug('Reading data from request')
             body = await req.read()
 
         return ConnexionRequest(url=url,
@@ -309,7 +349,8 @@ async def get_request(cls, req):
                                 headers=headers,
                                 body=body,
                                 json_getter=lambda: cls.jsonifier.loads(body),
-                                files={},
+                                form=form,
+                                files=files,
                                 context=req)
 
     @classmethod

connexion/operations/openapi.py

@@ -268,36 +268,79 @@ def body_definition(self):
         return {}
 
     def _get_body_argument(self, body, arguments, has_kwargs, sanitize):
-        x_body_name = sanitize(self.body_schema.get('x-body-name', 'body'))
+        if len(arguments) <= 0 and not has_kwargs:
+            return {}
+
+        x_body_name_default = 'body'
+        x_body_name_explicit = True
+        x_body_name = sanitize(self.body_schema.get('x-body-name', ''))
+        if not x_body_name:
+            x_body_name_explicit = False
+            x_body_name = x_body_name_default
+
+        # if the body came in null, and the schema says it can be null, we decide
+        # to include no value for the body argument, rather than the default body
         if is_nullable(self.body_schema) and is_null(body):
-            return {x_body_name: None}
+            if x_body_name in arguments or has_kwargs:
+                return {x_body_name: None}
+            return {}
 
+        # now determine the actual value for the body (whether it came in or is default)
         default_body = self.body_schema.get('default', {})
-        body_props = {k: {"schema": v} for k, v
+        body_props = {sanitize(k): {"schema": v} for k, v
                       in self.body_schema.get("properties", {}).items()}
 
-        # by OpenAPI specification `additionalProperties` defaults to `true`
-        # see: https://github.com/OAI/OpenAPI-Specification/blame/3.0.2/versions/3.0.2.md#L2305
-        additional_props = self.body_schema.get("additionalProperties", True)
-
         if body is None:
             body = deepcopy(default_body)
 
+        # if the body isn't even an object, then none of the concerns below matter
         if self.body_schema.get("type") != "object":
             if x_body_name in arguments or has_kwargs:
                 return {x_body_name: body}
             return {}
 
-        body_arg = deepcopy(default_body)
-        body_arg.update(body or {})
+        # by OpenAPI specification `additionalProperties` defaults to `true`
+        # see: https://github.com/OAI/OpenAPI-Specification/blame/3.0.2/versions/3.0.2.md#L2305
+        additional_props = self.body_schema.get("additionalProperties", True)
 
-        res = {}
-        if body_props or additional_props:
-            res = self._get_typed_body_values(body_arg, body_props, additional_props)
+        # supply the initial defaults and convert all values to the proper types by schema
+        x = deepcopy(default_body)
+        x.update(body or {})
+        converted_body = self._get_typed_body_values(x, body_props, additional_props)
+
+        # NOTE:
+        # Handlers could have a single argument to receive the whole body or they
+        # could have individual arguments to receive all the body's parameters, or
+        # they may have a **kwargs, arguments to receive anything.  So, the question
+        # arises that if kwargs is given, do we pass to the handler a single body
+        # argument, or the broken out arguments, or both?
+        #
+        # #1 If 'x-body-arg' is explicitly given and it exists in [arguments], then the
+        # body, as a whole, will be passed to the handler with that name. STOP.
+        #
+        # #2 If kwargs is given, then we don't know what the handler cares about, so we
+        # pass the body as a whole as an argument named, 'body', along with the
+        # individual body properties. STOP.
+        #
+        # #3 Else, we pass the body's individual properties which  exist in [arguments].
+        #
+        # #4 Finally, if that resulting argument list is empty, then we include an argument
+        # named 'body' to the handler, but only if 'body' exists in [arguments]
+
+        if x_body_name_explicit and x_body_name in arguments: #1
+            return {x_body_name: converted_body}
+
+        if has_kwargs: #2
+            converted_body[x_body_name_default] = copy(converted_body) # copy just to avoid circular ref
+            return converted_body
+
+        r = {k: converted_body[k] for k in converted_body if k in arguments} #3
+
+        if len(r) <= 0 and x_body_name_default in arguments: #4
+            r[x_body_name_default] = converted_body
+
+        return r
 
-        if x_body_name in arguments or has_kwargs:
-            return {x_body_name: res}
-        return {}
 
     def _get_typed_body_values(self, body_arg, body_props, additional_props):
         """

setup.py

@@ -32,7 +32,7 @@ def read_version(package):
 swagger_ui_require = 'swagger-ui-bundle>=0.0.2'
 flask_require = 'flask>=1.0.4'
 aiohttp_require = [
-    'aiohttp>=2.3.10',
+    'aiohttp>=2.3.10,<4',
     'aiohttp-jinja2>=0.14.0'
 ]
 

tests/aiohttp/test_aiohttp_multipart.py

@@ -0,0 +1,118 @@
+import os
+
+import aiohttp
+import pytest
+from pathlib import Path
+
+from connexion import AioHttpApp
+
+try:
+    import ujson as json
+except ImportError:
+    import json
+
+
+@pytest.fixture
+def aiohttp_app(aiohttp_api_spec_dir):
+    app = AioHttpApp(__name__, port=5001,
+                     specification_dir=aiohttp_api_spec_dir,
+                     debug=True)
+    app.add_api(
+        'openapi_multipart.yaml',
+        validate_responses=True,
+        strict_validation=True,
+        pythonic_params=True,
+        pass_context_arg_name='request_ctx',
+    )
+    return app
+
+
+async def test_single_file_upload(aiohttp_app, aiohttp_client):
+    app_client = await aiohttp_client(aiohttp_app.app)
+
+    resp = await app_client.post(
+        '/v1.0/upload_file',
+        data=aiohttp.FormData(fields=[('funky_funky', open(__file__, 'rb'))])(),
+    )
+
+    data = await resp.json()
+    assert resp.status == 200
+    assert data['fileName'] == f'{__name__}.py'
+    assert data['content'] == Path(__file__).read_bytes().decode('utf8')
+
+
+async def test_many_files_upload(aiohttp_app, aiohttp_client):
+    app_client = await aiohttp_client(aiohttp_app.app)
+
+    dir_name = os.path.dirname(__file__)
+    files_field = [
+        ('files', open(f'{dir_name}/{file_name}', 'rb')) \
+            for file_name in sorted(os.listdir(dir_name)) if file_name.endswith('.py')
+    ]
+
+    form_data = aiohttp.FormData(fields=files_field)
+
+    resp = await app_client.post(
+        '/v1.0/upload_files',
+        data=form_data(),
+    )
+
+    data = await resp.json()
+
+    assert resp.status == 200
+    assert data['filesCount'] == len(files_field)
+    assert data['contents'] == [
+        Path(f'{dir_name}/{file_name}').read_bytes().decode('utf8') \
+            for file_name in sorted(os.listdir(dir_name)) if file_name.endswith('.py')
+    ]
+
+
+async def test_mixed_multipart_single_file(aiohttp_app, aiohttp_client):
+    app_client = await aiohttp_client(aiohttp_app.app)
+
+    form_data = aiohttp.FormData()
+    form_data.add_field('dirName', os.path.dirname(__file__))
+    form_data.add_field('funky_funky', open(__file__, 'rb'))
+
+    resp = await app_client.post(
+        '/v1.0/mixed_single_file',
+        data=form_data(),
+    )
+
+    data = await resp.json()
+
+    assert resp.status == 200
+    assert data['dirName'] == os.path.dirname(__file__)
+    assert data['fileName'] == f'{__name__}.py'
+    assert data['content'] == Path(__file__).read_bytes().decode('utf8')
+
+
+
+async def test_mixed_multipart_many_files(aiohttp_app, aiohttp_client):
+    app_client = await aiohttp_client(aiohttp_app.app)
+
+    dir_name = os.path.dirname(__file__)
+    files_field = [
+        ('files', open(f'{dir_name}/{file_name}', 'rb')) \
+            for file_name in sorted(os.listdir(dir_name)) if file_name.endswith('.py')
+    ]
+
+    form_data = aiohttp.FormData(fields=files_field)
+    form_data.add_field('dirName', os.path.dirname(__file__))
+    form_data.add_field('testCount', str(len(files_field)))
+
+    resp = await app_client.post(
+        '/v1.0/mixed_many_files',
+        data=form_data(),
+    )
+
+    data = await resp.json()
+
+    assert resp.status == 200
+    assert data['dirName'] == os.path.dirname(__file__)
+    assert data['testCount'] == len(files_field)
+    assert data['filesCount'] == len(files_field)
+    assert data['contents'] == [
+        Path(f'{dir_name}/{file_name}').read_bytes().decode('utf8') \
+            for file_name in sorted(os.listdir(dir_name)) if file_name.endswith('.py')
+    ]

tests/api/test_parameters.py

@@ -352,6 +352,9 @@ def test_nullable_parameter(simple_app):
     resp = app_client.put('/v1.0/nullable-parameters', data="None", headers=headers)
     assert json.loads(resp.data.decode('utf-8', 'replace')) == 'it was None'
 
+    resp = app_client.put('/v1.0/nullable-parameters-noargs', data="None", headers=headers)
+    assert json.loads(resp.data.decode('utf-8', 'replace')) == 'hello'
+
 
 def test_args_kwargs(simple_app):
     app_client = simple_app.app.test_client()
@@ -363,6 +366,16 @@ def test_args_kwargs(simple_app):
     assert resp.status_code == 200
     assert json.loads(resp.data.decode('utf-8', 'replace')) == {'foo': 'a'}
 
+    if simple_app._spec_file == 'openapi.yaml':
+        body = { 'foo': 'a', 'bar': 'b' }
+        resp = app_client.post(
+            '/v1.0/body-params-as-kwargs',
+            data=json.dumps(body),
+            headers={'Content-Type': 'application/json'})
+        assert resp.status_code == 200
+        # having only kwargs and no explicit x-body-name, the handler would have been passed 'body' and the individual params from body
+        assert json.loads(resp.data.decode('utf-8', 'replace')) == {'body': {'foo': 'a', 'bar': 'b'}, 'foo': 'a', 'bar': 'b'}
+
 
 def test_param_sanitization(simple_app):
     app_client = simple_app.app.test_client()

tests/fakeapi/aiohttp_handlers.py

@@ -84,3 +84,42 @@ async def get_date():
 
 async def get_uuid():
     return ConnexionResponse(body={'value': uuid.UUID(hex='e7ff66d0-3ec2-4c4e-bed0-6e4723c24c51')})
+
+
+async def aiohttp_multipart_single_file(funky_funky):
+    return aiohttp.web.json_response(
+        data={
+			'fileName': funky_funky.filename,
+			'content': funky_funky.file.read().decode('utf8')
+		},
+    )
+
+
+async def aiohttp_multipart_many_files(files):
+    return aiohttp.web.json_response(
+        data={
+			'filesCount': len(files),
+            'contents': [ f.file.read().decode('utf8') for f in files ]
+		},
+    )
+
+
+async def aiohttp_multipart_mixed_single_file(dir_name, funky_funky):
+    return aiohttp.web.json_response(
+        data={
+            'dirName': dir_name,
+            'fileName': funky_funky.filename,
+            'content': funky_funky.file.read().decode('utf8'),
+        },
+    )
+
+
+async def aiohttp_multipart_mixed_many_files(dir_name, test_count, files):
+    return aiohttp.web.json_response(
+        data={
+            'filesCount': len(files),
+            'dirName': dir_name,
+            'testCount': test_count,
+            'contents': [ f.file.read().decode('utf8') for f in files ]
+        },
+    )

tests/fakeapi/hello.py

@@ -403,6 +403,9 @@ def test_nullable_param_put(contents):
         return 'it was None'
     return contents
 
+def test_nullable_param_put_noargs(dummy=''):
+    return 'hello'
+
 
 def test_custom_json_response():
     return {'theResult': DummyClass()}, 200
@@ -460,6 +463,9 @@ def optional_auth(**kwargs):
 def test_args_kwargs(*args, **kwargs):
     return kwargs
 
+def test_args_kwargs_post(*args, **kwargs):
+    return kwargs
+
 
 def test_param_sanitization(query=None, form=None):
     result = {}