From d6ece0a98509f5da91cca598b2e4e4a868c2780d Mon Sep 17 00:00:00 2001 From: Taras Sotnikov Date: Fri, 14 May 2021 17:03:16 +0200 Subject: [PATCH 01/10] 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 --- .../get_same_name_multiple_locations_param.py | 10 +-- .../parameters/multiple_path_parameters.py | 71 +++++++++++++++++++ end_to_end_tests/openapi.json | 32 +++++++++ openapi_python_client/parser/openapi.py | 4 ++ .../templates/endpoint_macros.py.jinja | 10 +-- tests/test_parser/test_openapi.py | 17 +++++ 6 files changed, 134 insertions(+), 10 deletions(-) create mode 100644 end_to_end_tests/golden-record/my_test_api_client/api/parameters/multiple_path_parameters.py 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 4bd74d91a..f37cacd7e 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: 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 new file mode 100644 index 000000000..207025f6a --- /dev/null +++ b/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) diff --git a/end_to_end_tests/openapi.json b/end_to_end_tests/openapi.json index 8ae566b5d..566b43cca 100644 --- a/end_to_end_tests/openapi.json +++ b/end_to_end_tests/openapi.json @@ -833,6 +833,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" + } + } + } + }, "/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 index fdf2245d7..42dae51a4 100644 --- a/openapi_python_client/parser/openapi.py +++ b/openapi_python_client/parser/openapi.py @@ -259,6 +259,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 +285,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 diff --git a/openapi_python_client/templates/endpoint_macros.py.jinja b/openapi_python_client/templates/endpoint_macros.py.jinja index 3d5365bb0..60baa4230 100644 --- a/openapi_python_client/templates/endpoint_macros.py.jinja +++ b/openapi_python_client/templates/endpoint_macros.py.jinja @@ -84,6 +84,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.values() %} +{{ parameter.to_string() }}, +{% endfor %} *, {# Proper client based on whether or not the endpoint requires authentication #} {% if endpoint.requires_security %} @@ -91,10 +95,6 @@ client: AuthenticatedClient, {% else %} client: Client, {% endif %} -{# path parameters #} -{% for parameter in endpoint.path_parameters.values() %} -{{ parameter.to_string() }}, -{% endfor %} {# Form data if any #} {% if endpoint.form_body_class %} form_data: {{ endpoint.form_body_class.name }}, @@ -122,10 +122,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.values() %} {{ 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 7a1fe6099..bab225049 100644 --- a/tests/test_parser/test_openapi.py +++ b/tests/test_parser/test_openapi.py @@ -530,6 +530,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_validation_error_when_location_not_supported(self, mocker): parsed_schemas = mocker.MagicMock() mocker.patch(f"{MODULE_NAME}.property_from_data", return_value=(mocker.MagicMock(), parsed_schemas)) From b90bb6a676a156a4c73b4186f074ada16777fa9e Mon Sep 17 00:00:00 2001 From: Taras Sotnikov Date: Tue, 18 May 2021 19:17:35 +0200 Subject: [PATCH 02/10] Ensure path parameters are ordered by appearance in path --- .../parameters/multiple_path_parameters.py | 24 +++++++++---- end_to_end_tests/openapi.json | 24 +++++++++++-- openapi_python_client/parser/openapi.py | 20 ++++++++++- tests/test_parser/test_openapi.py | 34 +++++++++++++++++++ 4 files changed, 92 insertions(+), 10 deletions(-) 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 index 207025f6a..b5df06c1f 100644 --- 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 @@ -7,12 +7,16 @@ def _get_kwargs( - param_1: str, + param_4: str, param_2: int, + param_1: str, + param_3: int, *, client: Client, ) -> Dict[str, Any]: - url = "{}/multiple-path-parameters/{param1}/{param2}".format(client.base_url, param1=param_1, param2=param_2) + 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() @@ -35,14 +39,18 @@ def _build_response(*, response: httpx.Response) -> Response[None]: def sync_detailed( - param_1: str, + param_4: str, param_2: int, + param_1: str, + param_3: int, *, client: Client, ) -> Response[None]: kwargs = _get_kwargs( - param_1=param_1, + param_4=param_4, param_2=param_2, + param_1=param_1, + param_3=param_3, client=client, ) @@ -54,14 +62,18 @@ def sync_detailed( async def asyncio_detailed( - param_1: str, + param_4: str, param_2: int, + param_1: str, + param_3: int, *, client: Client, ) -> Response[None]: kwargs = _get_kwargs( - param_1=param_1, + param_4=param_4, param_2=param_2, + param_1=param_1, + param_3=param_3, client=client, ) diff --git a/end_to_end_tests/openapi.json b/end_to_end_tests/openapi.json index 566b43cca..4ec035a76 100644 --- a/end_to_end_tests/openapi.json +++ b/end_to_end_tests/openapi.json @@ -833,8 +833,8 @@ } } }, - "/multiple-path-parameters/{param1}/{param2}": { - "description": "Test with multiple path parameters", + "/multiple-path-parameters/{param4}/{param2}/{param1}/{param3}": { + "description": "Test that multiple path parameters are ordered by appearance in path", "get": { "tags": [ "parameters" @@ -863,7 +863,25 @@ "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.", diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py index 42dae51a4..bceec2167 100644 --- a/openapi_python_client/parser/openapi.py +++ b/openapi_python_client/parser/openapi.py @@ -1,3 +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 @@ -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""" @@ -51,7 +55,6 @@ def from_data( endpoint, schemas = Endpoint._add_parameters( endpoint=endpoint, data=path_data, schemas=schemas, config=config ) - if isinstance(endpoint, ParseError): endpoint.header = ( f"ERROR parsing {method.upper()} {path} within {tag}. Endpoint will not be generated." @@ -61,6 +64,7 @@ def from_data( for error in endpoint.errors: error.header = f"WARNING parsing {method.upper()} {path} within {tag}." collection.parse_errors.append(error) + endpoint = Endpoint.sort_parameters(endpoint=endpoint, path=path) collection.endpoints.append(endpoint) return endpoints_by_tag, schemas @@ -322,6 +326,20 @@ def _add_parameters( return endpoint, schemas + @staticmethod + def _sort_parameters(*, endpoint: "Endpoint", path: str) -> Union["Endpoint", ParseError]: + endpoint = deepcopy(endpoint) + parameters_form_path = re.findall(_PATH_PARAM_REGEX, path) + path_parameter_names = [p.name for p in endpoint.path_parameters] + if sorted(parameters_form_path) != sorted(path_parameter_names): + return ParseError( + data=endpoint.path_parameters, + detail="Incorrect path templating (Path parameters do not match with path)", + ) + + endpoint.path_parameters.sort(key=lambda p: parameters_form_path.index(p.name)) + return endpoint + @staticmethod def from_data( *, data: oai.Operation, path: str, method: str, tag: str, schemas: Schemas, config: Config diff --git a/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py index bab225049..f65066c94 100644 --- a/tests/test_parser/test_openapi.py +++ b/tests/test_parser/test_openapi.py @@ -798,6 +798,40 @@ def test__add_parameters_duplicate_properties_different_location(self): assert result.path_parameters["test"].name == "test" assert result.query_parameters["test"].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=f"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 == [param] + assert "Incorrect path templating" in result.detail + def test_from_data_bad_params(self, mocker): from openapi_python_client.parser.openapi import Endpoint From f06947abfb423406ef3177d81487f89ab47d5988 Mon Sep 17 00:00:00 2001 From: Taras Sotnikov Date: Tue, 18 May 2021 20:45:55 +0200 Subject: [PATCH 03/10] Fix unused fstring --- tests/test_parser/test_openapi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py index f65066c94..0ef506a8d 100644 --- a/tests/test_parser/test_openapi.py +++ b/tests/test_parser/test_openapi.py @@ -822,7 +822,7 @@ def test__sort_parameters_invalid_path_templating(self, mocker): endpoint = self.make_endpoint() path = "/multiple-path-parameters/{param1}/{param2}" param = oai.Parameter.construct( - name=f"param1", required=True, param_schema=mocker.MagicMock(), param_in=oai.ParameterLocation.PATH + name="param1", required=True, param_schema=mocker.MagicMock(), param_in=oai.ParameterLocation.PATH ) endpoint.path_parameters.append(param) From e958fd8ced138496cbc2d614493bd419d52c320e Mon Sep 17 00:00:00 2001 From: Taras Sotnikov Date: Tue, 25 May 2021 10:13:54 +0200 Subject: [PATCH 04/10] Improve sort of path parameters Also fix typo Co-authored-by: Dylan Anthony <43723790+dbanty@users.noreply.github.com> --- openapi_python_client/parser/openapi.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py index bceec2167..bcae6c07d 100644 --- a/openapi_python_client/parser/openapi.py +++ b/openapi_python_client/parser/openapi.py @@ -329,15 +329,17 @@ def _add_parameters( @staticmethod def _sort_parameters(*, endpoint: "Endpoint", path: str) -> Union["Endpoint", ParseError]: endpoint = deepcopy(endpoint) - parameters_form_path = re.findall(_PATH_PARAM_REGEX, path) + 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 sorted(parameters_form_path) != sorted(path_parameter_names): + if parameters_from_path != path_parameter_names: return ParseError( data=endpoint.path_parameters, detail="Incorrect path templating (Path parameters do not match with path)", ) - - endpoint.path_parameters.sort(key=lambda p: parameters_form_path.index(p.name)) return endpoint @staticmethod From c89129f59df9a4fb4122d53442a29b6a517473b6 Mon Sep 17 00:00:00 2001 From: Taras Sotnikov Date: Tue, 25 May 2021 10:27:37 +0200 Subject: [PATCH 05/10] Pass endpoint instead of endpoint.path_parameters to ParseError --- openapi_python_client/parser/openapi.py | 2 +- tests/test_parser/test_openapi.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py index bcae6c07d..050afd59f 100644 --- a/openapi_python_client/parser/openapi.py +++ b/openapi_python_client/parser/openapi.py @@ -337,7 +337,7 @@ def _sort_parameters(*, endpoint: "Endpoint", path: str) -> Union["Endpoint", Pa path_parameter_names = [p.name for p in endpoint.path_parameters] if parameters_from_path != path_parameter_names: return ParseError( - data=endpoint.path_parameters, + data=endpoint, detail="Incorrect path templating (Path parameters do not match with path)", ) return endpoint diff --git a/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py index 0ef506a8d..8cbd4fb54 100644 --- a/tests/test_parser/test_openapi.py +++ b/tests/test_parser/test_openapi.py @@ -829,7 +829,7 @@ def test__sort_parameters_invalid_path_templating(self, mocker): result = Endpoint._sort_parameters(endpoint=endpoint, path=path) assert isinstance(result, ParseError) - assert result.data == [param] + assert result.data == endpoint assert "Incorrect path templating" in result.detail def test_from_data_bad_params(self, mocker): From 7dece0fc88e1fcae8321f2a494be13585e77f78a Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Sat, 7 Aug 2021 10:46:25 -0600 Subject: [PATCH 06/10] refactor: Rebase on main, make Endpoint publicly-called methods public. --- .../api/parameters/__init__.py | 5 ++ .../parameters/multiple_path_parameters.py | 48 +++++------ openapi_python_client/parser/openapi.py | 13 ++- tests/test_parser/test_openapi.py | 84 ++++++++++--------- 4 files changed, 78 insertions(+), 72 deletions(-) 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 index 0d34e1f2d..26e6450c7 100644 --- 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/multiple_path_parameters.py b/end_to_end_tests/golden-record/my_test_api_client/api/parameters/multiple_path_parameters.py index b5df06c1f..98da76ef3 100644 --- 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 @@ -7,15 +7,15 @@ def _get_kwargs( - param_4: str, - param_2: int, - param_1: str, - param_3: int, + param4: str, + param2: int, + param1: str, + param3: 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 + client.base_url, param4=param4, param2=param2, param1=param1, param3=param3 ) headers: Dict[str, Any] = client.get_headers() @@ -29,7 +29,7 @@ def _get_kwargs( } -def _build_response(*, response: httpx.Response) -> Response[None]: +def _build_response(*, response: httpx.Response) -> Response[Any]: return Response( status_code=response.status_code, content=response.content, @@ -39,18 +39,18 @@ def _build_response(*, response: httpx.Response) -> Response[None]: def sync_detailed( - param_4: str, - param_2: int, - param_1: str, - param_3: int, + param4: str, + param2: int, + param1: str, + param3: int, *, client: Client, -) -> Response[None]: +) -> Response[Any]: kwargs = _get_kwargs( - param_4=param_4, - param_2=param_2, - param_1=param_1, - param_3=param_3, + param4=param4, + param2=param2, + param1=param1, + param3=param3, client=client, ) @@ -62,18 +62,18 @@ def sync_detailed( async def asyncio_detailed( - param_4: str, - param_2: int, - param_1: str, - param_3: int, + param4: str, + param2: int, + param1: str, + param3: int, *, client: Client, -) -> Response[None]: +) -> Response[Any]: kwargs = _get_kwargs( - param_4=param_4, - param_2=param_2, - param_1=param_1, - param_3=param_3, + param4=param4, + param2=param2, + param1=param1, + param3=param3, client=client, ) diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py index 050afd59f..1b6701ebc 100644 --- a/openapi_python_client/parser/openapi.py +++ b/openapi_python_client/parser/openapi.py @@ -52,7 +52,7 @@ 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 isinstance(endpoint, ParseError): @@ -244,7 +244,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) @@ -327,9 +327,9 @@ def _add_parameters( return endpoint, schemas @staticmethod - def _sort_parameters(*, endpoint: "Endpoint", path: str) -> Union["Endpoint", ParseError]: + def sort_parameters(*, endpoint: "Endpoint") -> Union["Endpoint", ParseError]: endpoint = deepcopy(endpoint) - parameters_from_path = re.findall(_PATH_PARAM_REGEX, path) + parameters_from_path = re.findall(_PATH_PARAM_REGEX, endpoint.path) try: endpoint.path_parameters.sort(key=lambda p: parameters_from_path.index(p.name)) except ValueError: @@ -337,8 +337,7 @@ def _sort_parameters(*, endpoint: "Endpoint", path: str) -> Union["Endpoint", Pa 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)", + detail=f"Incorrect path templating for {endpoint.path} (Path parameters do not match with path)", ) return endpoint @@ -363,7 +362,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/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py index 8cbd4fb54..6010a04d2 100644 --- a/tests/test_parser/test_openapi.py +++ b/tests/test_parser/test_openapi.py @@ -496,7 +496,7 @@ def test__add_responses(self, mocker): } assert response_schemas == schemas_2 - def test__add_parameters_handles_no_params(self): + def test_add_parameters_handles_no_params(self): from openapi_python_client.parser.openapi import Endpoint, Schemas endpoint = self.make_endpoint() @@ -504,14 +504,14 @@ def test__add_parameters_handles_no_params(self): config = MagicMock() # Just checking there's no exception here - assert Endpoint._add_parameters( + assert Endpoint.add_parameters( endpoint=endpoint, data=oai.Operation.construct(), schemas=schemas, config=config ) == ( endpoint, schemas, ) - def test__add_parameters_parse_error(self, mocker): + def test_add_parameters_parse_error(self, mocker): from openapi_python_client.parser.openapi import Endpoint endpoint = self.make_endpoint() @@ -522,7 +522,7 @@ def test__add_parameters_parse_error(self, mocker): param = oai.Parameter.construct(name="test", required=True, param_schema=mocker.MagicMock(), param_in="cookie") config = MagicMock() - result = Endpoint._add_parameters( + result = Endpoint.add_parameters( endpoint=endpoint, data=oai.Operation.construct(parameters=[param]), schemas=initial_schemas, config=config ) assert result == ( @@ -530,7 +530,7 @@ def test__add_parameters_parse_error(self, mocker): property_schemas, ) - def test__add_parameters_parse_error_on_non_required_path_param(self, mocker): + 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() @@ -542,7 +542,7 @@ def test__add_parameters_parse_error_on_non_required_path_param(self, mocker): schemas = Schemas() config = MagicMock() - result = Endpoint._add_parameters( + 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) @@ -597,7 +597,7 @@ def test__add_parameters_with_location_postfix_conflict1(self, mocker): initial_schemas = mocker.MagicMock() config = MagicMock() - result = Endpoint._add_parameters(endpoint=endpoint, data=data, schemas=initial_schemas, config=config)[0] + result = Endpoint.add_parameters(endpoint=endpoint, data=data, schemas=initial_schemas, config=config)[0] assert isinstance(result, ParseError) assert result.detail == "Parameters with same Python identifier `prop_name_path` detected" @@ -643,7 +643,7 @@ def test__add_parameters_with_location_postfix_conflict2(self, mocker): initial_schemas = mocker.MagicMock() config = MagicMock() - result = Endpoint._add_parameters(endpoint=endpoint, data=data, schemas=initial_schemas, config=config)[0] + result = Endpoint.add_parameters(endpoint=endpoint, data=data, schemas=initial_schemas, config=config)[0] assert isinstance(result, ParseError) assert result.detail == "Parameters with same Python identifier `prop_name_path` detected" @@ -753,7 +753,7 @@ def test__add_parameters_query_optionality(self): assert param.nullable assert not param.required - def test__add_parameters_duplicate_properties(self): + def test_add_parameters_duplicate_properties(self): from openapi_python_client.parser.openapi import Endpoint, Schemas endpoint = self.make_endpoint() @@ -764,7 +764,7 @@ def test__add_parameters_duplicate_properties(self): schemas = Schemas() config = MagicMock() - result = Endpoint._add_parameters(endpoint=endpoint, data=data, schemas=schemas, config=config) + result = Endpoint.add_parameters(endpoint=endpoint, data=data, schemas=schemas, config=config) assert result == ( ParseError( data=data, @@ -775,7 +775,7 @@ def test__add_parameters_duplicate_properties(self): schemas, ) - def test__add_parameters_duplicate_properties_different_location(self): + def test_add_parameters_duplicate_properties_different_location(self): from openapi_python_client.parser.openapi import Endpoint, Schemas endpoint = self.make_endpoint() @@ -788,7 +788,7 @@ def test__add_parameters_duplicate_properties_different_location(self): schemas = Schemas() config = MagicMock() - result = Endpoint._add_parameters( + result = Endpoint.add_parameters( endpoint=endpoint, data=oai.Operation.construct(parameters=[path_param, query_param]), schemas=schemas, @@ -798,11 +798,11 @@ def test__add_parameters_duplicate_properties_different_location(self): assert result.path_parameters["test"].name == "test" assert result.query_parameters["test"].name == "test" - def test__sort_parameters(self, mocker): + 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}" + endpoint.path = "/multiple-path-parameters/{param4}/{param2}/{param1}/{param3}" for i in range(1, 5): param = oai.Parameter.construct( @@ -810,27 +810,27 @@ def test__sort_parameters(self, mocker): ) endpoint.path_parameters.append(param) - result = Endpoint._sort_parameters(endpoint=endpoint, path=path) + result = Endpoint.sort_parameters(endpoint=endpoint) 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): + 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}" + 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) + result = Endpoint.sort_parameters(endpoint=endpoint) assert isinstance(result, ParseError) - assert result.data == endpoint assert "Incorrect path templating" in result.detail + assert endpoint.path in result.detail def test_from_data_bad_params(self, mocker): from openapi_python_client.parser.openapi import Endpoint @@ -839,7 +839,7 @@ def test_from_data_bad_params(self, mocker): method = mocker.MagicMock() parse_error = ParseError(data=mocker.MagicMock()) return_schemas = mocker.MagicMock() - _add_parameters = mocker.patch.object(Endpoint, "_add_parameters", return_value=(parse_error, return_schemas)) + add_parameters = mocker.patch.object(Endpoint, "add_parameters", return_value=(parse_error, return_schemas)) data = oai.Operation.construct( description=mocker.MagicMock(), operationId=mocker.MagicMock(), @@ -862,8 +862,8 @@ def test_from_data_bad_responses(self, mocker): method = mocker.MagicMock() parse_error = ParseError(data=mocker.MagicMock()) param_schemas = mocker.MagicMock() - _add_parameters = mocker.patch.object( - Endpoint, "_add_parameters", return_value=(mocker.MagicMock(), param_schemas) + add_parameters = mocker.patch.object( + Endpoint, "add_parameters", return_value=(mocker.MagicMock(), param_schemas) ) response_schemas = mocker.MagicMock() _add_responses = mocker.patch.object(Endpoint, "_add_responses", return_value=(parse_error, response_schemas)) @@ -889,7 +889,7 @@ def test_from_data_standard(self, mocker): method = mocker.MagicMock() param_schemas = mocker.MagicMock() param_endpoint = mocker.MagicMock() - _add_parameters = mocker.patch.object(Endpoint, "_add_parameters", return_value=(param_endpoint, param_schemas)) + add_parameters = mocker.patch.object(Endpoint, "add_parameters", return_value=(param_endpoint, param_schemas)) response_schemas = mocker.MagicMock() response_endpoint = mocker.MagicMock() _add_responses = mocker.patch.object( @@ -915,7 +915,7 @@ def test_from_data_standard(self, mocker): assert endpoint == _add_body.return_value - _add_parameters.assert_called_once_with( + add_parameters.assert_called_once_with( endpoint=Endpoint( path=path, method=method, @@ -941,8 +941,8 @@ def test_from_data_no_operation_id(self, mocker): path = "/path/with/{param}/" method = "get" - _add_parameters = mocker.patch.object( - Endpoint, "_add_parameters", return_value=(mocker.MagicMock(), mocker.MagicMock()) + add_parameters = mocker.patch.object( + Endpoint, "add_parameters", return_value=(mocker.MagicMock(), mocker.MagicMock()) ) _add_responses = mocker.patch.object( Endpoint, "_add_responses", return_value=(mocker.MagicMock(), mocker.MagicMock()) @@ -962,7 +962,7 @@ def test_from_data_no_operation_id(self, mocker): assert result == _add_body.return_value - _add_parameters.assert_called_once_with( + add_parameters.assert_called_once_with( endpoint=Endpoint( path=path, method=method, @@ -977,9 +977,9 @@ def test_from_data_no_operation_id(self, mocker): config=config, ) _add_responses.assert_called_once_with( - endpoint=_add_parameters.return_value[0], + endpoint=add_parameters.return_value[0], data=data.responses, - schemas=_add_parameters.return_value[1], + schemas=add_parameters.return_value[1], config=config, ) _add_body.assert_called_once_with( @@ -995,8 +995,8 @@ def test_from_data_no_security(self, mocker): security=None, responses=mocker.MagicMock(), ) - _add_parameters = mocker.patch.object( - Endpoint, "_add_parameters", return_value=(mocker.MagicMock(), mocker.MagicMock()) + add_parameters = mocker.patch.object( + Endpoint, "add_parameters", return_value=(mocker.MagicMock(), mocker.MagicMock()) ) _add_responses = mocker.patch.object( Endpoint, "_add_responses", return_value=(mocker.MagicMock(), mocker.MagicMock()) @@ -1010,7 +1010,7 @@ def test_from_data_no_security(self, mocker): Endpoint.from_data(data=data, path=path, method=method, tag="a", schemas=schemas, config=config) - _add_parameters.assert_called_once_with( + add_parameters.assert_called_once_with( endpoint=Endpoint( path=path, method=method, @@ -1025,9 +1025,9 @@ def test_from_data_no_security(self, mocker): config=config, ) _add_responses.assert_called_once_with( - endpoint=_add_parameters.return_value[0], + endpoint=add_parameters.return_value[0], data=data.responses, - schemas=_add_parameters.return_value[1], + schemas=add_parameters.return_value[1], config=config, ) _add_body.assert_called_once_with( @@ -1080,9 +1080,9 @@ def test_from_data(self, mocker): "path_1": oai.PathItem.construct(post=path_1_post, put=path_1_put), "path_2": oai.PathItem.construct(get=path_2_get), } - endpoint_1 = mocker.MagicMock(autospec=Endpoint, tag="default", relative_imports={"1", "2"}) - endpoint_2 = mocker.MagicMock(autospec=Endpoint, tag="tag_2", relative_imports={"2"}) - endpoint_3 = mocker.MagicMock(autospec=Endpoint, tag="default", relative_imports={"2", "3"}) + endpoint_1 = mocker.MagicMock(autospec=Endpoint, tag="default", relative_imports={"1", "2"}, path="path_1") + endpoint_2 = mocker.MagicMock(autospec=Endpoint, tag="tag_2", relative_imports={"2"}, path="path_1") + endpoint_3 = mocker.MagicMock(autospec=Endpoint, tag="default", relative_imports={"2", "3"}, path="path_2") schemas_1 = mocker.MagicMock() schemas_2 = mocker.MagicMock() schemas_3 = mocker.MagicMock() @@ -1163,7 +1163,7 @@ def test_from_data_errors(self, mocker): side_effect=[ (ParseError(data="1"), schemas_1), (ParseError(data="2"), schemas_2), - (mocker.MagicMock(errors=[ParseError(data="3")]), schemas_3), + (mocker.MagicMock(errors=[ParseError(data="3")], path="path_2"), schemas_3), ], ) schemas = mocker.MagicMock() @@ -1199,9 +1199,11 @@ def test_from_data_tags_snake_case_sanitizer(self, mocker): "path_1": oai.PathItem.construct(post=path_1_post, put=path_1_put), "path_2": oai.PathItem.construct(get=path_2_get), } - endpoint_1 = mocker.MagicMock(autospec=Endpoint, tag="default", relative_imports={"1", "2"}) - endpoint_2 = mocker.MagicMock(autospec=Endpoint, tag="AMFSubscriptionInfo (Document)", relative_imports={"2"}) - endpoint_3 = mocker.MagicMock(autospec=Endpoint, tag="default", relative_imports={"2", "3"}) + endpoint_1 = mocker.MagicMock(autospec=Endpoint, tag="default", relative_imports={"1", "2"}, path="path_1") + endpoint_2 = mocker.MagicMock( + autospec=Endpoint, tag="AMFSubscriptionInfo (Document)", relative_imports={"2"}, path="path_1" + ) + endpoint_3 = mocker.MagicMock(autospec=Endpoint, tag="default", relative_imports={"2", "3"}, path="path_2") schemas_1 = mocker.MagicMock() schemas_2 = mocker.MagicMock() schemas_3 = mocker.MagicMock() From aa63b86f805b8cb4c887c50e27e1e66286840eae Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Sun, 15 Aug 2021 12:01:27 -0600 Subject: [PATCH 07/10] fix: Update for rebase on main --- ...lete_common_parameters_overriding_param.py | 10 ++--- .../get_common_parameters_overriding_param.py | 10 ++--- openapi_python_client/parser/openapi.py | 14 +++--- tests/test_parser/test_openapi.py | 44 +++++++++---------- 4 files changed, 39 insertions(+), 39 deletions(-) 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 index 5c6efb5d9..5dec6f543 100644 --- 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 index 2fb194af7..7a0566aae 100644 --- 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/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py index 1b6701ebc..276db7761 100644 --- a/openapi_python_client/parser/openapi.py +++ b/openapi_python_client/parser/openapi.py @@ -1,5 +1,5 @@ -import itertools 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 @@ -55,6 +55,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) if isinstance(endpoint, ParseError): endpoint.header = ( f"ERROR parsing {method.upper()} {path} within {tag}. Endpoint will not be generated." @@ -64,7 +66,6 @@ def from_data( for error in endpoint.errors: error.header = f"WARNING parsing {method.upper()} {path} within {tag}." collection.parse_errors.append(error) - endpoint = Endpoint.sort_parameters(endpoint=endpoint, path=path) collection.endpoints.append(endpoint) return endpoints_by_tag, schemas @@ -95,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) @@ -331,10 +332,13 @@ def sort_parameters(*, endpoint: "Endpoint") -> Union["Endpoint", ParseError]: endpoint = deepcopy(endpoint) parameters_from_path = re.findall(_PATH_PARAM_REGEX, endpoint.path) try: - endpoint.path_parameters.sort(key=lambda p: parameters_from_path.index(p.name)) + 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 = [p.name for p in endpoint.path_parameters] + 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)", diff --git a/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py index 6010a04d2..c5e8be700 100644 --- a/tests/test_parser/test_openapi.py +++ b/tests/test_parser/test_openapi.py @@ -530,22 +530,20 @@ 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 - + def test__add_parameters_parse_error_on_non_required_path_param(self): 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 + name="test", + required=False, + param_schema=oai.Schema.construct(type="string"), + 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 + 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) + assert result == (ParseError(data=param, detail="Path parameter must be required"), schemas) def test_validation_error_when_location_not_supported(self, mocker): parsed_schemas = mocker.MagicMock() @@ -656,7 +654,7 @@ def test__add_parameters_skips_references(self): ] ) - (endpoint, _) = endpoint._add_parameters(endpoint=endpoint, data=data, schemas=Schemas(), config=Config()) + (endpoint, _) = endpoint.add_parameters(endpoint=endpoint, data=data, schemas=Schemas(), config=Config()) assert isinstance(endpoint, Endpoint) assert ( @@ -679,7 +677,7 @@ def test__add_parameters_skips_params_without_schemas(self): ] ) - (endpoint, _) = endpoint._add_parameters(endpoint=endpoint, data=data, schemas=Schemas(), config=Config()) + (endpoint, _) = endpoint.add_parameters(endpoint=endpoint, data=data, schemas=Schemas(), config=Config()) assert isinstance(endpoint, Endpoint) assert len(endpoint.path_parameters) == 0 @@ -692,11 +690,13 @@ def test__add_parameters_same_identifier_conflict(self): name="param", param_in="path", param_schema=oai.Schema.construct(nullable=False, type="string"), + required=True, ), oai.Parameter.construct( name="param_path", param_in="path", param_schema=oai.Schema.construct(nullable=False, type="string"), + required=True, ), oai.Parameter.construct( name="param", @@ -706,7 +706,7 @@ def test__add_parameters_same_identifier_conflict(self): ] ) - (err, _) = endpoint._add_parameters(endpoint=endpoint, data=data, schemas=Schemas(), config=Config()) + (err, _) = endpoint.add_parameters(endpoint=endpoint, data=data, schemas=Schemas(), config=Config()) assert isinstance(err, ParseError) assert "param_path" in err.detail @@ -742,7 +742,7 @@ def test__add_parameters_query_optionality(self): ] ) - (endpoint, _) = endpoint._add_parameters(endpoint=endpoint, data=data, schemas=Schemas(), config=Config()) + (endpoint, _) = endpoint.add_parameters(endpoint=endpoint, data=data, schemas=Schemas(), config=Config()) assert len(endpoint.query_parameters) == 4, "Not all query params were added" for param in endpoint.query_parameters.values(): @@ -798,33 +798,29 @@ def test_add_parameters_duplicate_properties_different_location(self): assert result.path_parameters["test"].name == "test" assert result.query_parameters["test"].name == "test" - def test_sort_parameters(self, mocker): + def test_sort_parameters(self, string_property_factory): from openapi_python_client.parser.openapi import Endpoint endpoint = self.make_endpoint() 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) + prop = string_property_factory(name=f"param{i}") + endpoint.path_parameters[prop.name] = prop result = Endpoint.sort_parameters(endpoint=endpoint) - result_names = [p.name for p in result.path_parameters] + result_names = [name for name 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): + def test_sort_parameters_invalid_path_templating(self, string_property_factory): from openapi_python_client.parser.openapi import Endpoint endpoint = self.make_endpoint() 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) + param = string_property_factory(name="param1") + endpoint.path_parameters[param.name] = param result = Endpoint.sort_parameters(endpoint=endpoint) From 20f17b830f034766cb6638bdf798f49bcec2fc6a Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Sun, 15 Aug 2021 12:06:29 -0600 Subject: [PATCH 08/10] test: Add test for multiple path params with constants interspersed. --- .../api/parameters/multiple_path_parameters.py | 2 +- end_to_end_tests/openapi.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 index 98da76ef3..54caf75e8 100644 --- 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 @@ -14,7 +14,7 @@ def _get_kwargs( *, client: Client, ) -> Dict[str, Any]: - url = "{}/multiple-path-parameters/{param4}/{param2}/{param1}/{param3}".format( + url = "{}/multiple-path-parameters/{param4}/something/{param2}/{param1}/{param3}".format( client.base_url, param4=param4, param2=param2, param1=param1, param3=param3 ) diff --git a/end_to_end_tests/openapi.json b/end_to_end_tests/openapi.json index 4ec035a76..0ee32fd34 100644 --- a/end_to_end_tests/openapi.json +++ b/end_to_end_tests/openapi.json @@ -833,7 +833,7 @@ } } }, - "/multiple-path-parameters/{param4}/{param2}/{param1}/{param3}": { + "/multiple-path-parameters/{param4}/something/{param2}/{param1}/{param3}": { "description": "Test that multiple path parameters are ordered by appearance in path", "get": { "tags": [ From f0fe788a37df03822fb9024cb675ab5218f2da46 Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Sun, 15 Aug 2021 12:07:51 -0600 Subject: [PATCH 09/10] fix: OrderedDict typing on older Pythons --- openapi_python_client/parser/openapi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py index 276db7761..ff733a496 100644 --- a/openapi_python_client/parser/openapi.py +++ b/openapi_python_client/parser/openapi.py @@ -96,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: OrderedDict[str, Property] = field(default_factory=OrderedDict) + 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) From b672f24199a5cca86052514f0cebb4003c0519ba Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Sun, 15 Aug 2021 12:31:00 -0600 Subject: [PATCH 10/10] ci: Add test for extra path param --- tests/test_parser/test_openapi.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py index c5e8be700..49eb09705 100644 --- a/tests/test_parser/test_openapi.py +++ b/tests/test_parser/test_openapi.py @@ -814,7 +814,7 @@ def test_sort_parameters(self, string_property_factory): assert result_names == expected_names - def test_sort_parameters_invalid_path_templating(self, string_property_factory): + def test_sort_parameters_missing_param(self, string_property_factory): from openapi_python_client.parser.openapi import Endpoint endpoint = self.make_endpoint() @@ -828,6 +828,20 @@ def test_sort_parameters_invalid_path_templating(self, string_property_factory): assert "Incorrect path templating" in result.detail assert endpoint.path in result.detail + def test_sort_parameters_extra_param(self, string_property_factory): + from openapi_python_client.parser.openapi import Endpoint + + endpoint = self.make_endpoint() + endpoint.path = "/multiple-path-parameters" + param = string_property_factory(name="param1") + endpoint.path_parameters[param.name] = param + + result = Endpoint.sort_parameters(endpoint=endpoint) + + assert isinstance(result, ParseError) + assert "Incorrect path templating" in result.detail + assert endpoint.path in result.detail + def test_from_data_bad_params(self, mocker): from openapi_python_client.parser.openapi import Endpoint