Split parsing from validation completely (#1934)

During the refactoring for Connexion 3, we deduplicated all
`coerce_type` calls during the refactoring and moved them into the
`uri_parser` so it is done in a single place. When looking into #1931
however, I noticed we are still using the `uri_parser` from more places
than we should.

This PR centralizes the parsing further into the `ConnexionRequest`
class. During validation, we now instantiate a `ConnexionRequest`
instead of a Starlette `Request`, and leverage it instead of calling the
`uri_parser` directly.

I split the parsing and validation in the tests so they can be tested in
isolation.

connexion/validators/parameter.py

@@ -3,9 +3,9 @@
 import logging
 
 from jsonschema import Draft4Validator, ValidationError
-from starlette.requests import Request
 
 from connexion.exceptions import BadRequestProblem, ExtraParameterProblem
+from connexion.lifecycle import ConnexionRequest
 from connexion.utils import boolean, is_null, is_nullable
 
 logger = logging.getLogger("connexion.validators.parameter")
@@ -82,17 +82,11 @@ def validate_query_parameter(self, param, request):
         :type param: dict
         :rtype: str
         """
-        # Convert to dict of lists
-        query_params = {
-            k: request.query_params.getlist(k) for k in request.query_params
-        }
-        query_params = self.uri_parser.resolve_query(query_params)
-        val = query_params.get(param["name"])
+        val = request.query_params.get(param["name"])
         return self.validate_parameter("query", val, param)
 
     def validate_path_parameter(self, param, request):
-        path_params = self.uri_parser.resolve_path(request.path_params)
-        val = path_params.get(param["name"].replace("-", "_"))
+        val = request.path_params.get(param["name"].replace("-", "_"))
         return self.validate_parameter("path", val, param)
 
     def validate_header_parameter(self, param, request):
@@ -106,7 +100,7 @@ def validate_cookie_parameter(self, param, request):
     def validate(self, scope):
         logger.debug("%s validating parameters...", scope.get("path"))
 
-        request = Request(scope)
+        request = ConnexionRequest(scope, uri_parser=self.uri_parser)
         self.validate_request(request)
 
     def validate_request(self, request):

tests/decorators/test_uri_parsing.py

@@ -1,10 +1,13 @@
+from urllib.parse import quote_plus
+
 import pytest
 from connexion.uri_parsing import (
     AlwaysMultiURIParser,
     FirstValueURIParser,
     OpenAPIURIParser,
     Swagger2URIParser,
 )
+from starlette.datastructures import QueryParams
 from werkzeug.datastructures import MultiDict
 
 QUERY1 = MultiDict([("letters", "a"), ("letters", "b,c"), ("letters", "d,e,f")])
@@ -262,3 +265,57 @@ class Request:
     parser = parser_class(parameters, body_defn)
     res = parser.resolve_query(request.query.to_dict(flat=False))
     assert res == expected
+
+
+def test_parameter_coercion():
+    params = [
+        {"name": "p1", "in": "path", "type": "integer", "required": True},
+        {"name": "h1", "in": "header", "type": "string", "enum": ["a", "b"]},
+        {"name": "q1", "in": "query", "type": "integer", "maximum": 3},
+        {
+            "name": "a1",
+            "in": "query",
+            "type": "array",
+            "minItems": 2,
+            "maxItems": 3,
+            "items": {"type": "integer", "minimum": 0},
+        },
+    ]
+
+    uri_parser = Swagger2URIParser(params, {})
+
+    parsed_param = uri_parser.resolve_path({"p1": "123"})
+    assert parsed_param == {"p1": 123}
+
+    parsed_param = uri_parser.resolve_path({"p1": ""})
+    assert parsed_param == {"p1": ""}
+
+    parsed_param = uri_parser.resolve_path({"p1": "foo"})
+    assert parsed_param == {"p1": "foo"}
+
+    parsed_param = uri_parser.resolve_path({"p1": "1.2"})
+    assert parsed_param == {"p1": "1.2"}
+
+    parsed_param = uri_parser.resolve_path({"p1": 1})
+    assert parsed_param == {"p1": 1}
+
+    parsed_param = uri_parser.resolve_query(QueryParams("q1=4"))
+    assert parsed_param == {"q1": 4}
+
+    parsed_param = uri_parser.resolve_query(QueryParams("q1=3"))
+    assert parsed_param == {"q1": 3}
+
+    parsed_param = uri_parser.resolve_query(QueryParams(f"a1={quote_plus('1,2')}"))
+    assert parsed_param == {"a1": [2]}  # Swagger2URIParser
+
+    parsed_param = uri_parser.resolve_query(QueryParams(f"a1={quote_plus('1,a')}"))
+    assert parsed_param == {"a1": ["a"]}  # Swagger2URIParser
+
+    parsed_param = uri_parser.resolve_query(QueryParams(f"a1={quote_plus('1,-1')}"))
+    assert parsed_param == {"a1": [1]}  # Swagger2URIParser
+
+    parsed_param = uri_parser.resolve_query(QueryParams(f"a1=1"))
+    assert parsed_param == {"a1": [1]}  # Swagger2URIParser
+
+    parsed_param = uri_parser.resolve_query(QueryParams(f"a1={quote_plus('1,2,3,4')}"))
+    assert parsed_param == {"a1": [4]}  # Swagger2URIParser

tests/test_validation.py

@@ -3,6 +3,7 @@
 
 import pytest
 from connexion.exceptions import BadRequestProblem
+from connexion.lifecycle import ConnexionRequest
 from connexion.uri_parsing import Swagger2URIParser
 from connexion.validators import AbstractRequestBodyValidator, ParameterValidator
 from starlette.datastructures import QueryParams
@@ -31,115 +32,108 @@ def test_parameter_validator(monkeypatch):
     request = MagicMock(path_params={}, **kwargs)
     with pytest.raises(BadRequestProblem) as exc:
         validator.validate_request(request)
-    assert exc.value.detail == "Missing path parameter 'p1'"
+        assert exc.value.detail == "Missing path parameter 'p1'"
 
     request = MagicMock(path_params={"p1": "123"}, **kwargs)
-    try:
+    with pytest.raises(BadRequestProblem) as exc:
         validator.validate_request(request)
-    except Exception as e:
-        pytest.fail(str(e))
+        assert exc.value.detail.startswith("'123' is not of type 'integer'")
 
     request = MagicMock(path_params={"p1": ""}, **kwargs)
     with pytest.raises(BadRequestProblem) as exc:
         validator.validate_request(request)
-    assert exc.value.detail.startswith("'' is not of type 'integer'")
+        assert exc.value.detail.startswith("'' is not of type 'integer'")
 
     request = MagicMock(path_params={"p1": "foo"}, **kwargs)
     with pytest.raises(BadRequestProblem) as exc:
         validator.validate_request(request)
-    assert exc.value.detail.startswith("'foo' is not of type 'integer'")
+        assert exc.value.detail.startswith("'foo' is not of type 'integer'")
 
     request = MagicMock(path_params={"p1": "1.2"}, **kwargs)
     with pytest.raises(BadRequestProblem) as exc:
         validator.validate_request(request)
-    assert exc.value.detail.startswith("'1.2' is not of type 'integer'")
+        assert exc.value.detail.startswith("'1.2' is not of type 'integer'")
 
     request = MagicMock(
-        path_params={"p1": 1}, query_params=QueryParams("q1=4"), headers={}, cookies={}
+        path_params={"p1": 1}, query_params={"q1": 4}, headers={}, cookies={}
     )
     with pytest.raises(BadRequestProblem) as exc:
         validator.validate_request(request)
-    assert exc.value.detail.startswith("4 is greater than the maximum of 3")
+        assert exc.value.detail.startswith("4 is greater than the maximum of 3")
 
     request = MagicMock(
-        path_params={"p1": 1}, query_params=QueryParams("q1=3"), headers={}, cookies={}
+        path_params={"p1": 1}, query_params={"q1": 3}, headers={}, cookies={}
     )
     try:
         validator.validate_request(request)
     except Exception as e:
         pytest.fail(str(e))
 
-    query_params = QueryParams(f"a1={quote_plus('1,2')}")
     request = MagicMock(
-        path_params={"p1": 1}, query_params=query_params, headers={}, cookies={}
+        path_params={"p1": 1}, query_params={"a1": [1, 2]}, headers={}, cookies={}
     )
     try:
         validator.validate_request(request)
     except Exception as e:
         pytest.fail(str(e))
 
-    query_params = QueryParams(f"a1={quote_plus('1,a')}")
     request = MagicMock(
-        path_params={"p1": 1}, query_params=query_params, headers={}, cookies={}
+        path_params={"p1": 1}, query_params={"a1": [1, "a"]}, headers={}, cookies={}
     )
     with pytest.raises(BadRequestProblem) as exc:
         validator.validate_request(request)
-    assert exc.value.detail.startswith("'a' is not of type 'integer'")
+        assert exc.value.detail.startswith("'a' is not of type 'integer'")
 
     request = MagicMock(
-        path_params={"p1": "123"}, query_params={}, headers={}, cookies={"c1": "b"}
+        path_params={"p1": 123}, query_params={}, headers={}, cookies={"c1": "b"}
     )
     try:
         validator.validate_request(request)
     except Exception as e:
         pytest.fail(str(e))
 
     request = MagicMock(
-        path_params={"p1": "123"}, query={}, headers={}, cookies={"c1": "x"}
+        path_params={"p1": 123}, query={}, headers={}, cookies={"c1": "x"}
     )
     with pytest.raises(BadRequestProblem) as exc:
         assert validator.validate_request(request)
+        assert exc.value.detail.startswith("'x' is not one of ['a', 'b']")
 
-    assert exc.value.detail.startswith("'x' is not one of ['a', 'b']")
-
-    query_params = QueryParams(f"a1={quote_plus('1,-1')}")
     request = MagicMock(
-        path_params={"p1": 1}, query_params=query_params, headers={}, cookies={}
+        path_params={"p1": 1}, query_params={"a1": [1, -1]}, headers={}, cookies={}
     )
     with pytest.raises(BadRequestProblem) as exc:
         validator.validate_request(request)
-    assert exc.value.detail.startswith("-1 is less than the minimum of 0")
+        assert exc.value.detail.startswith("-1 is less than the minimum of 0")
 
-    query_params = QueryParams("a1=1")
     request = MagicMock(
-        path_params={"p1": 1}, query_params=query_params, headers={}, cookies={}
+        path_params={"p1": 1}, query_params={"a1": 1}, headers={}, cookies={}
     )
     with pytest.raises(BadRequestProblem) as exc:
         validator.validate_request(request)
-    assert exc.value.detail.startswith("[1] is too short")
+        assert exc.value.detail.startswith("[1] is too short")
 
-    query_params = QueryParams(f"a1={quote_plus('1,2,3,4')}")
     request = MagicMock(
-        path_params={"p1": 1}, query_params=query_params, headers={}, cookies={}
+        path_params={"p1": 1}, query_params={"a1": [1, 2, 3, 4]}, headers={}, cookies={}
     )
     with pytest.raises(BadRequestProblem) as exc:
         validator.validate_request(request)
-    assert exc.value.detail.startswith("[1, 2, 3, 4] is too long")
+        assert exc.value.detail.startswith("[1, 2, 3, 4] is too long")
 
     request = MagicMock(
-        path_params={"p1": "123"}, query_params={}, headers={"h1": "a"}, cookies={}
+        path_params={"p1": 123}, query_params={}, headers={"h1": "a"}, cookies={}
     )
     try:
         validator.validate_request(request)
     except Exception as e:
         pytest.fail(str(e))
 
     request = MagicMock(
-        path_params={"p1": "123"}, query_params={}, headers={"h1": "x"}, cookies={}
+        path_params={"p1": 123}, query_params={}, headers={"h1": "x"}, cookies={}
     )
     with pytest.raises(BadRequestProblem) as exc:
         validator.validate_request(request)
-    assert exc.value.detail.startswith("'x' is not one of ['a', 'b']")
+        assert exc.value.detail.startswith("'x' is not one of ['a', 'b']")
 
 
 async def test_stream_replay():