feat: Allow path parameters to be positional args [#429 & #464]. Thanks @tsotnikov!

dbanty · tsotnikov · web-flow · commit 9d991eca170d · 2021-08-15T12:37:29.000-06:00
Co-authored-by: Taras Sotnikov &lt;taras.sotnikov@telefonica.com&gt;
diff --git a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/parameters/__init__.py b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/parameters/__init__.py
@@ -6,6 +6,7 @@
     delete_common_parameters_overriding_param,
     get_common_parameters_overriding_param,
     get_same_name_multiple_locations_param,
+    multiple_path_parameters,
 )
 
 
@@ -21,3 +22,7 @@ def delete_common_parameters_overriding_param(cls) -> types.ModuleType:
     @classmethod
     def get_same_name_multiple_locations_param(cls) -> types.ModuleType:
         return get_same_name_multiple_locations_param
+
+    @classmethod
+    def multiple_path_parameters(cls) -> types.ModuleType:
+        return multiple_path_parameters
diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/parameters/delete_common_parameters_overriding_param.py b/end_to_end_tests/golden-record/my_test_api_client/api/parameters/delete_common_parameters_overriding_param.py
@@ -7,9 +7,9 @@
 
 
 def _get_kwargs(
+    param_path: str,
     *,
     client: Client,
-    param_path: str,
     param_query: Union[Unset, None, str] = UNSET,
 ) -> Dict[str, Any]:
     url = "{}/common_parameters_overriding/{param}".format(client.base_url, param=param_path)
@@ -41,14 +41,14 @@ def _build_response(*, response: httpx.Response) -> Response[Any]:
 
 
 def sync_detailed(
+    param_path: str,
     *,
     client: Client,
-    param_path: str,
     param_query: Union[Unset, None, str] = UNSET,
 ) -> Response[Any]:
     kwargs = _get_kwargs(
-        client=client,
         param_path=param_path,
+        client=client,
         param_query=param_query,
     )
 
@@ -60,14 +60,14 @@ def sync_detailed(
 
 
 async def asyncio_detailed(
+    param_path: str,
     *,
     client: Client,
-    param_path: str,
     param_query: Union[Unset, None, str] = UNSET,
 ) -> Response[Any]:
     kwargs = _get_kwargs(
-        client=client,
         param_path=param_path,
+        client=client,
         param_query=param_query,
     )
 
diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/parameters/get_common_parameters_overriding_param.py b/end_to_end_tests/golden-record/my_test_api_client/api/parameters/get_common_parameters_overriding_param.py
@@ -7,9 +7,9 @@
 
 
 def _get_kwargs(
+    param_path: str,
     *,
     client: Client,
-    param_path: str,
     param_query: str = "overriden_in_GET",
 ) -> Dict[str, Any]:
     url = "{}/common_parameters_overriding/{param}".format(client.base_url, param=param_path)
@@ -41,14 +41,14 @@ def _build_response(*, response: httpx.Response) -> Response[Any]:
 
 
 def sync_detailed(
+    param_path: str,
     *,
     client: Client,
-    param_path: str,
     param_query: str = "overriden_in_GET",
 ) -> Response[Any]:
     kwargs = _get_kwargs(
-        client=client,
         param_path=param_path,
+        client=client,
         param_query=param_query,
     )
 
@@ -60,14 +60,14 @@ def sync_detailed(
 
 
 async def asyncio_detailed(
+    param_path: str,
     *,
     client: Client,
-    param_path: str,
     param_query: str = "overriden_in_GET",
 ) -> Response[Any]:
     kwargs = _get_kwargs(
-        client=client,
         param_path=param_path,
+        client=client,
         param_query=param_query,
     )
 
diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/parameters/get_same_name_multiple_locations_param.py b/end_to_end_tests/golden-record/my_test_api_client/api/parameters/get_same_name_multiple_locations_param.py
@@ -7,9 +7,9 @@
 
 
 def _get_kwargs(
+    param_path: str,
     *,
     client: Client,
-    param_path: str,
     param_query: Union[Unset, None, str] = UNSET,
     param_header: Union[Unset, str] = UNSET,
     param_cookie: Union[Unset, str] = UNSET,
@@ -49,16 +49,16 @@ def _build_response(*, response: httpx.Response) -> Response[Any]:
 
 
 def sync_detailed(
+    param_path: str,
     *,
     client: Client,
-    param_path: str,
     param_query: Union[Unset, None, str] = UNSET,
     param_header: Union[Unset, str] = UNSET,
     param_cookie: Union[Unset, str] = UNSET,
 ) -> Response[Any]:
     kwargs = _get_kwargs(
-        client=client,
         param_path=param_path,
+        client=client,
         param_query=param_query,
         param_header=param_header,
         param_cookie=param_cookie,
@@ -72,16 +72,16 @@ def sync_detailed(
 
 
 async def asyncio_detailed(
+    param_path: str,
     *,
     client: Client,
-    param_path: str,
     param_query: Union[Unset, None, str] = UNSET,
     param_header: Union[Unset, str] = UNSET,
     param_cookie: Union[Unset, str] = UNSET,
 ) -> Response[Any]:
     kwargs = _get_kwargs(
-        client=client,
         param_path=param_path,
+        client=client,
         param_query=param_query,
         param_header=param_header,
         param_cookie=param_cookie,
diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/parameters/multiple_path_parameters.py b/end_to_end_tests/golden-record/my_test_api_client/api/parameters/multiple_path_parameters.py
@@ -0,0 +1,83 @@
+from typing import Any, Dict
+
+import httpx
+
+from ...client import Client
+from ...types import Response
+
+
+def _get_kwargs(
+    param4: str,
+    param2: int,
+    param1: str,
+    param3: int,
+    *,
+    client: Client,
+) -> Dict[str, Any]:
+    url = "{}/multiple-path-parameters/{param4}/something/{param2}/{param1}/{param3}".format(
+        client.base_url, param4=param4, param2=param2, param1=param1, param3=param3
+    )
+
+    headers: Dict[str, Any] = client.get_headers()
+    cookies: Dict[str, Any] = client.get_cookies()
+
+    return {
+        "url": url,
+        "headers": headers,
+        "cookies": cookies,
+        "timeout": client.get_timeout(),
+    }
+
+
+def _build_response(*, response: httpx.Response) -> Response[Any]:
+    return Response(
+        status_code=response.status_code,
+        content=response.content,
+        headers=response.headers,
+        parsed=None,
+    )
+
+
+def sync_detailed(
+    param4: str,
+    param2: int,
+    param1: str,
+    param3: int,
+    *,
+    client: Client,
+) -> Response[Any]:
+    kwargs = _get_kwargs(
+        param4=param4,
+        param2=param2,
+        param1=param1,
+        param3=param3,
+        client=client,
+    )
+
+    response = httpx.get(
+        **kwargs,
+    )
+
+    return _build_response(response=response)
+
+
+async def asyncio_detailed(
+    param4: str,
+    param2: int,
+    param1: str,
+    param3: int,
+    *,
+    client: Client,
+) -> Response[Any]:
+    kwargs = _get_kwargs(
+        param4=param4,
+        param2=param2,
+        param1=param1,
+        param3=param3,
+        client=client,
+    )
+
+    async with httpx.AsyncClient() as _client:
+        response = await _client.get(**kwargs)
+
+    return _build_response(response=response)
diff --git a/end_to_end_tests/openapi.json b/end_to_end_tests/openapi.json
@@ -833,6 +833,56 @@
         }
       }
     },
+    "/multiple-path-parameters/{param4}/something/{param2}/{param1}/{param3}": {
+      "description": "Test that multiple path parameters are ordered by appearance in path",
+      "get": {
+        "tags": [
+          "parameters"
+        ],
+        "operationId": "multiple_path_parameters",
+        "parameters": [
+          {
+            "name": "param1",
+            "in": "path",
+            "required": true,
+            "schema": {
+              "type": "string"
+            }
+          },
+          {
+            "name": "param2",
+            "in": "path",
+            "required": true,
+            "schema": {
+              "type": "integer"
+            }
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": "Success"
+          }
+        }
+      },
+      "parameters": [
+        {
+          "name": "param4",
+          "in": "path",
+          "required": true,
+          "schema": {
+            "type": "string"
+          }
+        },
+        {
+          "name": "param3",
+          "in": "path",
+          "required": true,
+          "schema": {
+            "type": "integer"
+          }
+        }
+      ]
+    },
     "/location/query/optionality": {
       "description": "Test what happens with various combinations of required and nullable in query parameters.",
       "get": {
diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py
@@ -1,3 +1,5 @@
+import re
+from collections import OrderedDict
 from copy import deepcopy
 from dataclasses import dataclass, field
 from typing import Any, Dict, Iterator, List, Optional, Set, Tuple, Union
@@ -13,6 +15,8 @@
 from .properties import Class, EnumProperty, ModelProperty, Property, Schemas, build_schemas, property_from_data
 from .responses import Response, response_from_data
 
+_PATH_PARAM_REGEX = re.compile("{([a-zA-Z_][a-zA-Z0-9_]*)}")
+
 
 def import_string_from_class(class_: Class, prefix: str = "") -> str:
     """Create a string which is used to import a reference"""
@@ -48,10 +52,11 @@ def from_data(
                 )
                 # Add `PathItem` parameters
                 if not isinstance(endpoint, ParseError):
-                    endpoint, schemas = Endpoint._add_parameters(
+                    endpoint, schemas = Endpoint.add_parameters(
                         endpoint=endpoint, data=path_data, schemas=schemas, config=config
                     )
-
+                if not isinstance(endpoint, ParseError):
+                    endpoint = Endpoint.sort_parameters(endpoint=endpoint)
                 if isinstance(endpoint, ParseError):
                     endpoint.header = (
                         f"ERROR parsing {method.upper()} {path} within {tag}. Endpoint will not be generated."
@@ -91,7 +96,7 @@ class Endpoint:
     summary: Optional[str] = ""
     relative_imports: Set[str] = field(default_factory=set)
     query_parameters: Dict[str, Property] = field(default_factory=dict)
-    path_parameters: Dict[str, Property] = field(default_factory=dict)
+    path_parameters: "OrderedDict[str, Property]" = field(default_factory=OrderedDict)
     header_parameters: Dict[str, Property] = field(default_factory=dict)
     cookie_parameters: Dict[str, Property] = field(default_factory=dict)
     responses: List[Response] = field(default_factory=list)
@@ -240,7 +245,7 @@ def _add_responses(
         return endpoint, schemas
 
     @staticmethod
-    def _add_parameters(
+    def add_parameters(
         *, endpoint: "Endpoint", data: Union[oai.Operation, oai.PathItem], schemas: Schemas, config: Config
     ) -> Tuple[Union["Endpoint", ParseError], Schemas]:
         endpoint = deepcopy(endpoint)
@@ -259,6 +264,9 @@ def _add_parameters(
             if isinstance(param, oai.Reference) or param.param_schema is None:
                 continue
 
+            if param.param_in == oai.ParameterLocation.PATH and not param.required:
+                return ParseError(data=param, detail="Path parameter must be required"), schemas
+
             unique_param = (param.name, param.param_in)
             if unique_param in unique_parameters:
                 duplication_detail = (
@@ -282,6 +290,7 @@ def _add_parameters(
             if prop.name in parameters_by_location[param.param_in]:
                 # This parameter was defined in the Operation, so ignore the PathItem definition
                 continue
+
             for location, parameters_dict in parameters_by_location.items():
                 if location == param.param_in or prop.name not in parameters_dict:
                     continue
@@ -318,6 +327,24 @@ def _add_parameters(
 
         return endpoint, schemas
 
+    @staticmethod
+    def sort_parameters(*, endpoint: "Endpoint") -> Union["Endpoint", ParseError]:
+        endpoint = deepcopy(endpoint)
+        parameters_from_path = re.findall(_PATH_PARAM_REGEX, endpoint.path)
+        try:
+            sorted_params = sorted(
+                endpoint.path_parameters.values(), key=lambda param: parameters_from_path.index(param.name)
+            )
+            endpoint.path_parameters = OrderedDict((param.name, param) for param in sorted_params)
+        except ValueError:
+            pass  # We're going to catch the difference down below
+        path_parameter_names = [name for name in endpoint.path_parameters]
+        if parameters_from_path != path_parameter_names:
+            return ParseError(
+                detail=f"Incorrect path templating for {endpoint.path} (Path parameters do not match with path)",
+            )
+        return endpoint
+
     @staticmethod
     def from_data(
         *, data: oai.Operation, path: str, method: str, tag: str, schemas: Schemas, config: Config
@@ -339,7 +366,7 @@ def from_data(
             tag=tag,
         )
 
-        result, schemas = Endpoint._add_parameters(endpoint=endpoint, data=data, schemas=schemas, config=config)
+        result, schemas = Endpoint.add_parameters(endpoint=endpoint, data=data, schemas=schemas, config=config)
         if isinstance(result, ParseError):
             return result, schemas
         result, schemas = Endpoint._add_responses(endpoint=result, data=data.responses, schemas=schemas, config=config)
diff --git a/openapi_python_client/templates/endpoint_macros.py.jinja b/openapi_python_client/templates/endpoint_macros.py.jinja
diff --git a/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py
