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 index eeb4daff7..cec075ba0 100644 --- 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: 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 new file mode 100644 index 000000000..b5df06c1f --- /dev/null +++ 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 index 64ccfdbc9..fffe74ba3 100644 --- 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 index f0b9774ce..48fb7f2dc 100644 --- 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 index 66d6209b3..23149b9e3 100644 --- a/openapi_python_client/templates/endpoint_macros.py.jinja +++ b/openapi_python_client/templates/endpoint_macros.py.jinja @@ -73,6 +73,10 @@ 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 %} @@ -80,10 +84,6 @@ 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 index a220d243c..59fa520ad 100644 --- 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