Telefonica · tsotnikov · May 14, 2021 · May 18, 2021 · May 18, 2021 · May 25, 2021
diff --git a/...golden-record/my_test_api_client/api/parameters/get_same_name_multiple_locations_param.py b/...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: Union[Unset, str] = UNSET,
     param_query: Union[Unset, str] = UNSET,
 ) -> Dict[str, Any]:
     url = "{}/same-name-multiple-locations/{param}".format(client.base_url, param=param_path)
@@ -41,14 +41,14 @@ def _build_response(*, response: httpx.Response) -> Response[None]:
 
 
 def sync_detailed(
+    param_path: str,
     *,
     client: Client,
-    param_path: Union[Unset, str] = UNSET,
     param_query: Union[Unset, str] = UNSET,
 ) -> Response[None]:
     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: Union[Unset, str] = UNSET,
     param_query: Union[Unset, str] = UNSET,
 ) -> Response[None]:
     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/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(
+    param_4: str,
+    param_2: int,
+    param_1: str,
+    param_3: int,
+    *,
+    client: Client,
+) -> Dict[str, Any]:
+    url = "{}/multiple-path-parameters/{param4}/{param2}/{param1}/{param3}".format(
+        client.base_url, param4=param_4, param2=param_2, param1=param_1, param3=param_3
+    )
+
+    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[None]:
+    return Response(
+        status_code=response.status_code,
+        content=response.content,
+        headers=response.headers,
+        parsed=None,
+    )
+
+
+def sync_detailed(
+    param_4: str,
+    param_2: int,
+    param_1: str,
+    param_3: int,
+    *,
+    client: Client,
+) -> Response[None]:
+    kwargs = _get_kwargs(
+        param_4=param_4,
+        param_2=param_2,
+        param_1=param_1,
+        param_3=param_3,
+        client=client,
+    )
+
+    response = httpx.get(
+        **kwargs,
+    )
+
+    return _build_response(response=response)
+
+
+async def asyncio_detailed(
+    param_4: str,
+    param_2: int,
+    param_1: str,
+    param_3: int,
+    *,
+    client: Client,
+) -> Response[None]:
+    kwargs = _get_kwargs(
+        param_4=param_4,
+        param_2=param_2,
+        param_1=param_1,
+        param_3=param_3,
+        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
@@ -782,6 +782,7 @@
           {
             "name": "param",
             "in": "path",
+            "required": true,
             "schema": {
               "type": "string"
             }
@@ -793,6 +794,56 @@
           }
         }
       }
+    },
+    "/multiple-path-parameters/{param4}/{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"
+          }
+        }
+      ]
     }
   },
   "components": {

diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py
@@ -1,4 +1,5 @@
 import itertools
+import re
 from copy import deepcopy
 from dataclasses import dataclass, field
 from typing import Any, Dict, Iterator, List, Optional, Set, Tuple, Union
@@ -12,6 +13,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"""
@@ -49,6 +52,8 @@ def from_data(
                     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, path=path)
                 if isinstance(endpoint, ParseError):
                     endpoint.header = (
                         f"ERROR parsing {method.upper()} {path} within {tag}. Endpoint will not be generated."
@@ -245,6 +250,8 @@ def _add_parameters(
             if param.param_in == oai.ParameterLocation.QUERY:
                 endpoint.query_parameters.append(prop)
             elif param.param_in == oai.ParameterLocation.PATH:
+                if not param.required:
+                    return ParseError(data=param, detail="Path parameter must be required"), schemas
                 endpoint.path_parameters.append(prop)
             elif param.param_in == oai.ParameterLocation.HEADER:
                 endpoint.header_parameters.append(prop)
@@ -266,6 +273,22 @@ def _add_parameters(
 
         return endpoint, schemas
 
+    @staticmethod
+    def _sort_parameters(*, endpoint: "Endpoint", path: str) -> Union["Endpoint", ParseError]:
+        endpoint = deepcopy(endpoint)
+        parameters_from_path = re.findall(_PATH_PARAM_REGEX, path)
+        try:
+            endpoint.path_parameters.sort(key=lambda p: parameters_from_path.index(p.name))
+        except ValueError:
+            pass  # We're going to catch the difference down below
+        path_parameter_names = [p.name for p in endpoint.path_parameters]
+        if parameters_from_path != path_parameter_names:
+            return ParseError(
+                data=endpoint,
+                detail="Incorrect path templating (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

diff --git a/openapi_python_client/templates/endpoint_macros.py.jinja b/openapi_python_client/templates/endpoint_macros.py.jinja
@@ -73,17 +73,17 @@ params = {k: v for k, v in params.items() if v is not UNSET and v is not None}
 
 {# The all the kwargs passed into an endpoint (and variants thereof)) #}
 {% macro arguments(endpoint) %}
+{# path parameters #}
+{% for parameter in endpoint.path_parameters %}
+{{ parameter.to_string() }},
+{% endfor %}
 *,
 {# Proper client based on whether or not the endpoint requires authentication #}
 {% if endpoint.requires_security %}
 client: AuthenticatedClient,
 {% else %}
 client: Client,
 {% endif %}
-{# path parameters #}
-{% for parameter in endpoint.path_parameters %}
-{{ parameter.to_string() }},
-{% endfor %}
 {# Form data if any #}
 {% if endpoint.form_body_class %}
 form_data: {{ endpoint.form_body_class.name }},
@@ -111,10 +111,10 @@ json_body: {{ endpoint.json_body.get_type_string() }},
 
 {# Just lists all kwargs to endpoints as name=name for passing to other functions #}
 {% macro kwargs(endpoint) %}
-client=client,
 {% for parameter in endpoint.path_parameters %}
 {{ parameter.python_name }}={{ parameter.python_name }},
 {% endfor %}
+client=client,
 {% if endpoint.form_body_class %}
 form_data=form_data,
 {% endif %}

diff --git a/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py
@@ -447,6 +447,23 @@ def test__add_parameters_parse_error(self, mocker):
             property_schemas,
         )
 
+    def test__add_parameters_parse_error_on_non_required_path_param(self, mocker):
+        from openapi_python_client.parser.openapi import Endpoint, Schemas
+
+        endpoint = self.make_endpoint()
+        parsed_schemas = mocker.MagicMock()
+        mocker.patch(f"{MODULE_NAME}.property_from_data", return_value=(mocker.MagicMock(), parsed_schemas))
+        param = oai.Parameter.construct(
+            name="test", required=False, param_schema=mocker.MagicMock(), param_in=oai.ParameterLocation.PATH
+        )
+        schemas = Schemas()
+        config = MagicMock()
+
+        result = Endpoint._add_parameters(
+            endpoint=endpoint, data=oai.Operation.construct(parameters=[param]), schemas=schemas, config=config
+        )
+        assert result == (ParseError(data=param, detail="Path parameter must be required"), parsed_schemas)
+
     def test__add_parameters_fail_loudly_when_location_not_supported(self, mocker):
         from openapi_python_client.parser.openapi import Endpoint, Schemas
 
@@ -589,6 +606,40 @@ def test__add_parameters_duplicate_properties_different_location(self):
         assert result.query_parameters[0].python_name == "test_query"
         assert result.query_parameters[0].name == "test"
 
+    def test__sort_parameters(self, mocker):
+        from openapi_python_client.parser.openapi import Endpoint
+
+        endpoint = self.make_endpoint()
+        path = "/multiple-path-parameters/{param4}/{param2}/{param1}/{param3}"
+
+        for i in range(1, 5):
+            param = oai.Parameter.construct(
+                name=f"param{i}", required=True, param_schema=mocker.MagicMock(), param_in=oai.ParameterLocation.PATH
+            )
+            endpoint.path_parameters.append(param)
+
+        result = Endpoint._sort_parameters(endpoint=endpoint, path=path)
+        result_names = [p.name for p in result.path_parameters]
+        expected_names = [f"param{i}" for i in (4, 2, 1, 3)]
+
+        assert result_names == expected_names
+
+    def test__sort_parameters_invalid_path_templating(self, mocker):
+        from openapi_python_client.parser.openapi import Endpoint
+
+        endpoint = self.make_endpoint()
+        path = "/multiple-path-parameters/{param1}/{param2}"
+        param = oai.Parameter.construct(
+            name="param1", required=True, param_schema=mocker.MagicMock(), param_in=oai.ParameterLocation.PATH
+        )
+        endpoint.path_parameters.append(param)
+
+        result = Endpoint._sort_parameters(endpoint=endpoint, path=path)
+
+        assert isinstance(result, ParseError)
+        assert result.data == endpoint
+        assert "Incorrect path templating" in result.detail
+
     def test_from_data_bad_params(self, mocker):
         from openapi_python_client.parser.openapi import Endpoint