Path parameters as function positional arguments

OpenAPI Specification indicates that if parameter location is `path`
the `required` property is REQUIRED and its value MUST be `true`

Ref: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#parameter-object

With this in mind, this PR:

- Tighten the parsing by throwing an error for path parameters with required=false
- Update the templates to pass the path parameters to the function as positional arguments instead of kwargs
to improve usability

Author: tsotnikov

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: 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,
     )
 

end_to_end_tests/golden-record/my_test_api_client/api/parameters/multiple_path_parameters.py

@@ -0,0 +1,71 @@
+from typing import Any, Dict
+
+import httpx
+
+from ...client import Client
+from ...types import Response
+
+
+def _get_kwargs(
+    param_1: str,
+    param_2: int,
+    *,
+    client: Client,
+) -> Dict[str, Any]:
+    url = "{}/multiple-path-parameters/{param1}/{param2}".format(client.base_url, param1=param_1, param2=param_2)
+
+    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_1: str,
+    param_2: int,
+    *,
+    client: Client,
+) -> Response[None]:
+    kwargs = _get_kwargs(
+        param_1=param_1,
+        param_2=param_2,
+        client=client,
+    )
+
+    response = httpx.get(
+        **kwargs,
+    )
+
+    return _build_response(response=response)
+
+
+async def asyncio_detailed(
+    param_1: str,
+    param_2: int,
+    *,
+    client: Client,
+) -> Response[None]:
+    kwargs = _get_kwargs(
+        param_1=param_1,
+        param_2=param_2,
+        client=client,
+    )
+
+    async with httpx.AsyncClient() as _client:
+        response = await _client.get(**kwargs)
+
+    return _build_response(response=response)

end_to_end_tests/openapi.json

@@ -782,6 +782,7 @@
           {
             "name": "param",
             "in": "path",
+            "required": true,
             "schema": {
               "type": "string"
             }
@@ -793,6 +794,38 @@
           }
         }
       }
+    },
+    "/multiple-path-parameters/{param1}/{param2}": {
+      "description": "Test with multiple path parameters",
+      "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"
+          }
+        }
+      }
     }
   },
   "components": {

openapi_python_client/parser/openapi.py

@@ -245,6 +245,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)

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 %}

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