URI parsing decorator (#613)

- array logic refactored into one place.
- validation.py and parameter.py no longer try to join the array, and the split it again.
- validation of defaults now works, because the validator is given the correct type.
- some additional classes that change the behavior of deduplicating query parameters that are defined multiple times
  - **AlwaysMultiURIParser** that is backwards compatible, warts and all (used by default)
  - **Swagger2URIParser** that adheres to the spec's definition of `collectionFormat: multi` and uses the last-defined query parameter value (ex. `query?a=1&a=2` => `a = 2`)
  - **FirstValueURIParser** that behaves like Swagger2URIParser, except that the first-defined value is used (ex. `query?a=1&a=2` => `a=1`)

.travis.yml

@@ -6,7 +6,7 @@ python:
   - "3.5"
   - "3.6"
 install:
-  - pip install "setuptools>=17.1" tox tox-travis coveralls
+  - pip install --upgrade setuptools tox tox-travis coveralls
 script:
   - tox
 after_success:

README.rst

@@ -278,6 +278,42 @@ If you use the ``array`` type In the Swagger definition, you can define the
 ``collectionFormat`` so that it won't be recognized. Connexion currently
 supports collection formats "pipes" and "csv". The default format is "csv".
 
+Connexion is opinionated about how the URI is parsed for ``array`` types.
+The default behavior for query parameters that have been defined multiple
+times is to join them all together. For example, if you provide a URI with
+the the query string ``?letters=a,b,c&letters=d,e,f``, connexion will set
+``letters = ['a', 'b', 'c', 'd', 'e', 'f']``.
+
+You can override this behavior by specifying the URI parser in the app or
+api options.
+
+.. code-block:: python
+
+   from connexion.decorators.uri_parsing import Swagger2URIParser
+   options = {'uri_parsing_class': Swagger2URIParser}
+   app = connexion.App(__name__, specification_dir='swagger/', options=options)
+
+You can implement your own URI parsing behavior by inheriting from
+``connextion.decorators.uri_parsing.AbstractURIParser``.
+
+There are three URI parsers included with connection.
+1. AlwaysMultiURIParser (default)
+   This parser is backwards compatible, and joins together multiple instances
+   of the same query parameter.
+2. Swagger2URIParser
+   This parser adheres to the Swagger 2.0 spec, and will only join together
+   multiple instance of the same query parameter if the ``collectionFormat``
+   is set to ``multi``. Query parameters are parsed from left to right, so
+   if a query parameter is defined twice, then the right-most definition wins.
+   For example, if you provided a URI with the query string
+   ``?letters=a,b,c&letters=d,e,f``, and ``collectionFormat: csv``, then
+   connexion will set ``letters = ['d', 'e', 'f']``
+3. FirstValueURIParser
+   This parser behaves like the Swagger2URIParser, except that it prefers the
+   first defined value. For example, if you provided a URI with the query
+   string ``?letters=a,b,c&letters=d,e,f`` and ``collectionFormat: csv``
+   then connexion will set ``letters = ['a', 'b', 'c']``
+
 Parameter validation
 ^^^^^^^^^^^^^^^^^^^^
 

connexion/apis/abstract.py

@@ -200,7 +200,8 @@ class AbstractAPI(object):
                               validator_map=self.validator_map,
                               strict_validation=self.strict_validation,
                               resolver=self.resolver,
-                              pythonic_params=self.pythonic_params)
+                              pythonic_params=self.pythonic_params,
+                              uri_parser_class=self.options.uri_parser_class)
         self._add_operation_internal(method, path, operation)
 
     @abc.abstractmethod

connexion/decorators/parameter.py

@@ -61,12 +61,8 @@ def get_val_from_param(value, query_param):
     if is_nullable(query_param) and is_null(value):
         return None
 
-    if query_param["type"] == "array":  # then logic is more complex
-        if query_param.get("collectionFormat") and query_param.get("collectionFormat") == "pipes":
-            parts = value.split("|")
-        else:  # default: csv
-            parts = value.split(",")
-        return [make_type(part, query_param["items"]["type"]) for part in parts]
+    if query_param["type"] == "array":
+        return [make_type(v, query_param["items"]["type"]) for v in value]
     else:
         return make_type(value, query_param["type"])
 
@@ -105,23 +101,6 @@ def parameter_to_arg(parameters, consumes, function, pythonic_params=False):
             name = snake_and_shadow(name)
         return name and re.sub('^[^a-zA-Z_]+', '', re.sub('[^0-9a-zA-Z_]', '', name))
 
-    def make_request_query(request):
-        request_query = {}
-        try:
-            for k, v in request.query.to_dict(flat=False).items():
-                k = sanitize_param(k)
-                query_param = query_types.get(k, None)
-                if query_param is not None and query_param["type"] == "array":
-                    if query_param.get("collectionFormat", None) == "pipes":
-                        request_query[k] = "|".join(v)
-                    else:
-                        request_query[k] = ",".join(v)
-                else:
-                    request_query[k] = v[0]
-        except AttributeError:
-            request_query = {sanitize_param(k): v for k, v in request.query.items()}
-        return request_query
-
     body_parameters = [parameter for parameter in parameters if parameter['in'] == 'body'] or [{}]
     body_name = sanitize_param(body_parameters[0].get('name'))
     default_body = body_parameters[0].get('schema', {}).get('default')
@@ -168,8 +147,9 @@ def parameter_to_arg(parameters, consumes, function, pythonic_params=False):
 
         # Add query parameters
         query_arguments = copy.deepcopy(default_query_params)
-        query_arguments.update(make_request_query(request))
+        query_arguments.update(request.query)
         for key, value in query_arguments.items():
+            key = sanitize_param(key)
             if not has_kwargs and key not in arguments:
                 logger.debug("Query Parameter '%s' not in function arguments", key)
             else:

connexion/decorators/uri_parsing.py

@@ -0,0 +1,190 @@
+# Decorators to split query and path parameters
+import abc
+import functools
+import logging
+
+import six
+
+from .decorator import BaseDecorator
+
+logger = logging.getLogger('connexion.decorators.uri_parsing')
+
+
+@six.add_metaclass(abc.ABCMeta)
+class AbstractURIParser(BaseDecorator):
+    def __init__(self, param_defns):
+        """
+        a URI parser is initialized with parameter definitions.
+        When called with a request object, it handles array types in the URI
+        both in the path and query according to the spec.
+        Some examples include:
+         - https://mysite.fake/in/path/1,2,3/            # path parameters
+         - https://mysite.fake/?in_query=a,b,c           # simple query params
+         - https://mysite.fake/?in_query=a|b|c           # various separators
+         - https://mysite.fake/?in_query=a&in_query=b,c  # complex query params
+        """
+        self._param_defns = {p["name"]: p
+                             for p in param_defns
+                             if p["in"] in ["query", "path"]}
+
+    @abc.abstractproperty
+    def param_defns(self):
+        """
+        returns the parameter definitions by name
+        """
+
+    @abc.abstractproperty
+    def param_schemas(self):
+        """
+        returns the parameter schemas by name
+        """
+
+    def __repr__(self):
+        """
+        :rtype: str
+        """
+        return "<{classname}>".format(classname=self.__class__.__name__)
+
+    @abc.abstractmethod
+    def _resolve_param_duplicates(self, values, param_defn):
+        """ Resolve cases where query parameters are provided multiple times.
+            For example, if the query string is '?a=1,2,3&a=4,5,6' the value of
+            `a` could be "4,5,6", or "1,2,3" or "1,2,3,4,5,6" depending on the
+            implementation.
+        """
+
+    @abc.abstractmethod
+    def _split(self, value, param_defn):
+        """
+        takes a string, and a parameter definition, and returns
+        an array that has been constructed according to the parameter
+        definition.
+        """
+
+    def resolve_params(self, params, resolve_duplicates=False):
+        """
+        takes a dict of parameters, and resolves the values into
+        the correct array type handling duplicate values, and splitting
+        based on the collectionFormat defined in the spec.
+        """
+        resolved_param = {}
+        for k, values in params.items():
+            param_defn = self.param_defns.get(k)
+            param_schema = self.param_schemas.get(k)
+            if not (param_defn or param_schema):
+                # rely on validation
+                resolved_param[k] = values
+                continue
+
+            if not resolve_duplicates:
+                values = [values]
+
+            if (param_schema is not None and param_schema['type'] == 'array'):
+                # resolve variable re-assignment, handle explode
+                values = self._resolve_param_duplicates(values, param_defn)
+                # handle array styles
+                resolved_param[k] = self._split(values, param_defn)
+            else:
+                resolved_param[k] = values[-1]
+
+        return resolved_param
+
+    def __call__(self, function):
+        """
+        :type function: types.FunctionType
+        :rtype: types.FunctionType
+        """
+
+        @functools.wraps(function)
+        def wrapper(request):
+
+            try:
+                query = request.query.to_dict(flat=False)
+            except AttributeError:
+                query = dict(request.query.items())
+
+            try:
+                path_params = request.path_params.to_dict(flat=False)
+            except AttributeError:
+                path_params = dict(request.path_params.items())
+
+            request.query = self.resolve_params(query, resolve_duplicates=True)
+            request.path_params = self.resolve_params(path_params)
+            response = function(request)
+            return response
+
+        return wrapper
+
+
+class Swagger2URIParser(AbstractURIParser):
+    """
+    Adheres to the Swagger2 spec,
+    Assumes the the last defined query parameter should be used.
+    """
+
+    @property
+    def param_defns(self):
+        return self._param_defns
+
+    @property
+    def param_schemas(self):
+        return self._param_defns  # swagger2 conflates defn and schema
+
+    @staticmethod
+    def _resolve_param_duplicates(values, param_defn):
+        """ Resolve cases where query parameters are provided multiple times.
+            The default behavior is to use the first-defined value.
+            For example, if the query string is '?a=1,2,3&a=4,5,6' the value of
+            `a` would be "4,5,6".
+            However, if 'collectionFormat' is 'multi' then the duplicate values
+            are concatenated together and `a` would be "1,2,3,4,5,6".
+        """
+        if param_defn.get('collectionFormat') == 'multi':
+            return ','.join(values)
+        # default to last defined value
+        return values[-1]
+
+    @staticmethod
+    def _split(value, param_defn):
+        if param_defn.get("collectionFormat") == 'pipes':
+            return value.split('|')
+        return value.split(',')
+
+
+class FirstValueURIParser(Swagger2URIParser):
+    """
+    Adheres to the Swagger2 spec
+    Assumes that the first defined query parameter should be used
+    """
+
+    @staticmethod
+    def _resolve_param_duplicates(values, param_defn):
+        """ Resolve cases where query parameters are provided multiple times.
+            The default behavior is to use the first-defined value.
+            For example, if the query string is '?a=1,2,3&a=4,5,6' the value of
+            `a` would be "1,2,3".
+            However, if 'collectionFormat' is 'multi' then the duplicate values
+            are concatenated together and `a` would be "1,2,3,4,5,6".
+        """
+        if param_defn.get('collectionFormat') == 'multi':
+            return ','.join(values)
+        # default to first defined value
+        return values[0]
+
+
+class AlwaysMultiURIParser(Swagger2URIParser):
+    """
+    Does not adhere to the Swagger2 spec, but is backwards compatible with
+    connexion behavior in version 1.4.2
+    """
+
+    @staticmethod
+    def _resolve_param_duplicates(values, param_defn):
+        """ Resolve cases where query parameters are provided multiple times.
+            The default behavior is to join all provided parameters together.
+            For example, if the query string is '?a=1,2,3&a=4,5,6' the value of
+            `a` would be "1,2,3,4,5,6".
+        """
+        if param_defn.get('collectionFormat') == 'pipes':
+            return '|'.join(values)
+        return ','.join(values)

connexion/decorators/validation.py

@@ -48,20 +48,15 @@ class TypeValidationError(Exception):
 def validate_type(param, value, parameter_type, parameter_name=None):
     param_type = param.get('type')
     parameter_name = parameter_name if parameter_name else param['name']
-    if param_type == "array":  # then logic is more complex
-        if param.get("collectionFormat") and param.get("collectionFormat") == "pipes":
-            parts = value.split("|")
-        else:  # default: csv
-            parts = value.split(",")
-
-        converted_parts = []
-        for part in parts:
+    if param_type == "array":
+        converted_params = []
+        for v in value:
             try:
-                converted = make_type(part, param["items"]["type"])
+                converted = make_type(v, param["items"]["type"])
             except (ValueError, TypeError):
-                converted = part
-            converted_parts.append(converted)
-        return converted_parts
+                converted = v
+            converted_params.append(converted)
+        return converted_params
     else:
         try:
             return make_type(value, param_type)

connexion/operation.py

@@ -14,6 +14,7 @@ from .decorators.response import ResponseValidator
 from .decorators.security import (get_tokeninfo_func, get_tokeninfo_url,
                                   security_passthrough, verify_oauth_local,
                                   verify_oauth_remote)
+from .decorators.uri_parsing import AlwaysMultiURIParser
 from .decorators.validation import (ParameterValidator, RequestBodyValidator,
                                     TypeValidationError)
 from .exceptions import InvalidSpecification
@@ -141,7 +142,7 @@ class Operation(SecureOperation):
                  path_parameters=None, app_security=None, security_definitions=None,
                  definitions=None, parameter_definitions=None, response_definitions=None,
                  validate_responses=False, strict_validation=False, randomize_endpoint=None,
-                 validator_map=None, pythonic_params=False):
+                 validator_map=None, pythonic_params=False, uri_parser_class=None):
         """
         This class uses the OperationID identify the module and function that will handle the operation
 
@@ -188,6 +189,8 @@ class Operation(SecureOperation):
         :param pythonic_params: When True CamelCase parameters are converted to snake_case and an underscore is appended
         to any shadowed built-ins
         :type pythonic_params: bool
+        :param uri_parser_class: A URI parser class that inherits from AbstractURIParser
+        :type uri_parser_class: AbstractURIParser
         """
 
         self.api = api
@@ -209,6 +212,7 @@ class Operation(SecureOperation):
         self.operation = operation
         self.randomize_endpoint = randomize_endpoint
         self.pythonic_params = pythonic_params
+        self.uri_parser_class = uri_parser_class or AlwaysMultiURIParser
 
         # todo support definition references
         # todo support references to application level parameters
@@ -389,6 +393,10 @@ class Operation(SecureOperation):
         for validation_decorator in self.__validation_decorators:
             function = validation_decorator(function)
 
+        uri_parsing_decorator = self.__uri_parsing_decorator
+        logging.debug('... Adding uri parsing decorator (%r)', uri_parsing_decorator)
+        function = uri_parsing_decorator(function)
+
         # NOTE: the security decorator should be applied last to check auth before anything else :-)
         security_decorator = self.security_decorator
         logger.debug('... Adding security decorator (%r)', security_decorator)
@@ -402,6 +410,16 @@ class Operation(SecureOperation):
 
         return function
 
+    @property
+    def __uri_parsing_decorator(self):
+        """
+        Get uri parsing decorator
+
+        This decorator handles query and path parameter deduplication and
+        array types.
+        """
+        return self.uri_parser_class(self.parameters)
+
     @property
     def __content_type_decorator(self):
         """

connexion/options.py

@@ -73,6 +73,15 @@ class ConnexionOptions(object):
         """
         return self._options.get('swagger_path', INTERNAL_CONSOLE_UI_PATH)
 
+    @property
+    def uri_parser_class(self):
+        # type: () -> str
+        """
+        The class to use for parsing URIs into path and query parameters.
+        Default: None
+        """
+        return self._options.get('uri_parser_class', None)
+
 
 def filter_values(dictionary):
     # type: (dict) -> dict

tests/api/test_bootstrap.py

@@ -1,3 +1,5 @@
+import json
+
 import jinja2
 import yaml
 from swagger_spec_validator.common import SwaggerValidationError
@@ -34,6 +36,23 @@ def test_app_with_different_server_option(simple_api_spec_dir):
     assert get_bye.data == b'Goodbye jsantos'
 
 
+def test_app_with_different_uri_parser(simple_api_spec_dir):
+    from connexion.decorators.uri_parsing import Swagger2URIParser
+    app = App(__name__, port=5001,
+              specification_dir='..' / simple_api_spec_dir.relative_to(TEST_FOLDER),
+              options={"uri_parser_class": Swagger2URIParser},
+              debug=True)
+    app.add_api('swagger.yaml')
+
+    app_client = app.app.test_client()
+    resp = app_client.get(
+        '/v1.0/test_array_csv_query_param?items=a,b,c&items=d,e,f'
+    )  # type: flask.Response
+    assert resp.status_code == 200
+    j = json.loads(resp.get_data(as_text=True))
+    assert j == ['d', 'e', 'f']
+
+
 def test_no_swagger_ui(simple_api_spec_dir):
     app = App(__name__, port=5001, specification_dir=simple_api_spec_dir,
               swagger_ui=False, debug=True)

tests/api/test_parameters.py

@@ -42,6 +42,10 @@ def test_required_query_param(simple_app):
 def test_array_query_param(simple_app):
     app_client = simple_app.app.test_client()
     headers = {'Content-type': 'application/json'}
+    url = '/v1.0/test_array_csv_query_param'
+    response = app_client.get(url, headers=headers)
+    array_response = json.loads(response.data.decode('utf-8', 'replace'))  # type: [str]
+    assert array_response == ['squash', 'banana']
     url = '/v1.0/test_array_csv_query_param?items=one,two,three'
     response = app_client.get(url, headers=headers)
     array_response = json.loads(response.data.decode('utf-8', 'replace'))  # type: [str]

tests/decorators/test_uri_parsing.py

@@ -0,0 +1,65 @@
+import pytest
+from connexion.decorators.uri_parsing import (AlwaysMultiURIParser,
+                                              FirstValueURIParser,
+                                              Swagger2URIParser)
+
+QUERY1 = ["a", "b,c", "d,e,f"]
+QUERY2 = ["a", "b|c", "d|e|f"]
+PATH1 = "d,e,f"
+PATH2 = "d|e|f"
+CSV = "csv"
+PIPES = "pipes"
+MULTI = "multi"
+
+
+@pytest.mark.parametrize("parser_class, expected, query_in, collection_format", [
+        (Swagger2URIParser, ['d', 'e', 'f'], QUERY1, CSV),
+        (FirstValueURIParser, ['a'], QUERY1, CSV),
+        (AlwaysMultiURIParser, ['a', 'b', 'c', 'd', 'e', 'f'], QUERY1, CSV),
+        (Swagger2URIParser, ['a', 'b', 'c', 'd', 'e', 'f'], QUERY1, MULTI),
+        (FirstValueURIParser, ['a', 'b', 'c', 'd', 'e', 'f'], QUERY1, MULTI),
+        (AlwaysMultiURIParser, ['a', 'b', 'c', 'd', 'e', 'f'], QUERY1, MULTI),
+        (Swagger2URIParser, ['d', 'e', 'f'], QUERY2, PIPES),
+        (FirstValueURIParser, ['a'], QUERY2, PIPES),
+        (AlwaysMultiURIParser, ['a', 'b', 'c', 'd', 'e', 'f'], QUERY2, PIPES)])
+def test_uri_parser_query_params(parser_class, expected, query_in, collection_format):
+    class Request(object):
+        query = {"letters": query_in}
+        path_params = {}
+
+    request = Request()
+    parameters = [
+        {"name": "letters",
+         "in": "query",
+         "type": "array",
+         "items": {"type": "string"},
+         "collectionFormat": collection_format}
+    ]
+    p = parser_class(parameters)
+    res = p(lambda x: x)(request)
+    assert res.query["letters"] == expected
+
+
+@pytest.mark.parametrize("parser_class, expected, query_in, collection_format", [
+        (Swagger2URIParser, ['d', 'e', 'f'], PATH1, CSV),
+        (FirstValueURIParser, ['d', 'e', 'f'], PATH1, CSV),
+        (AlwaysMultiURIParser, ['d', 'e', 'f'], PATH1, CSV),
+        (Swagger2URIParser, ['d', 'e', 'f'], PATH2, PIPES),
+        (FirstValueURIParser, ['d', 'e', 'f'], PATH2, PIPES),
+        (AlwaysMultiURIParser, ['d', 'e', 'f'], PATH2, PIPES)])
+def test_uri_parser_path_params(parser_class, expected, query_in, collection_format):
+    class Request(object):
+        query = {}
+        path_params = {"letters": query_in}
+
+    request = Request()
+    parameters = [
+        {"name": "letters",
+         "in": "path",
+         "type": "array",
+         "items": {"type": "string"},
+         "collectionFormat": collection_format}
+    ]
+    p = parser_class(parameters)
+    res = p(lambda x: x)(request)
+    assert res.path_params["letters"] == expected

tests/fixtures/simple/swagger.yaml

@@ -251,11 +251,11 @@ paths:
         - name: items
           in: query
           description: An comma separated array of items
-          required: true
           type: array
           items:
             type: string
           collectionFormat: csv
+          default: ["squash", "banana"]
       responses:
         200:
           description: OK

tests/test_validation.py

@@ -47,15 +47,15 @@ def test_parameter_validator(monkeypatch):
     request = MagicMock(path_params={'p1': 1}, query={'q1': '3'}, headers={})
     assert handler(request) == 'OK'
 
-    request = MagicMock(path_params={'p1': 1}, query={'a1': "1,2"}, headers={})
+    request = MagicMock(path_params={'p1': 1}, query={'a1': ['1', '2']}, headers={})
     assert handler(request) == "OK"
-    request = MagicMock(path_params={'p1': 1}, query={'a1': "1,a"}, headers={})
+    request = MagicMock(path_params={'p1': 1}, query={'a1': ['1', 'a']}, headers={})
     assert json.loads(handler(request).data.decode())['detail'].startswith("'a' is not of type 'integer'")
-    request = MagicMock(path_params={'p1': 1}, query={'a1': "1,-1"}, headers={})
+    request = MagicMock(path_params={'p1': 1}, query={'a1': ['1', '-1']}, headers={})
     assert json.loads(handler(request).data.decode())['detail'].startswith("-1 is less than the minimum of 0")
-    request = MagicMock(path_params={'p1': 1}, query={'a1': "1"}, headers={})
+    request = MagicMock(path_params={'p1': 1}, query={'a1': ['1']}, headers={})
     assert json.loads(handler(request).data.decode())['detail'].startswith("[1] is too short")
-    request = MagicMock(path_params={'p1': 1}, query={'a1': "1,2,3,4"}, headers={})
+    request = MagicMock(path_params={'p1': 1}, query={'a1': ['1', '2', '3', '4']}, headers={})
     assert json.loads(handler(request).data.decode())['detail'].startswith("[1, 2, 3, 4] is too long")
 
     request = MagicMock(path_params={'p1': '123'}, query={}, headers={'h1': 'a'})