From 0e7171b6cdf788ff1bd41ee622549ea7937ed08a Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Mon, 19 Feb 2024 19:26:15 -0700 Subject: [PATCH 1/5] Fix mixed case properties & params --- ...eters_with_names_differing_only_by_case.md | 10 ++ ...ing_conflicts_with_properties_in_models.md | 32 ++++ end_to_end_tests/baseline_openapi_3.0.json | 44 +++++ end_to_end_tests/baseline_openapi_3.1.yaml | 44 +++++ .../my_test_api_client/api/naming/__init__.py | 6 +- .../api/naming/mixed_case.py | 169 ++++++++++++++++++ .../my_test_api_client/models/__init__.py | 2 + .../models/mixed_case_response_200.py | 67 +++++++ openapi_python_client/parser/openapi.py | 51 +++--- .../parser/properties/model_property.py | 25 ++- .../parser/properties/protocol.py | 4 +- openapi_python_client/utils.py | 7 +- 12 files changed, 427 insertions(+), 34 deletions(-) create mode 100644 .changeset/allow_parameters_with_names_differing_only_by_case.md create mode 100644 .changeset/fix_naming_conflicts_with_properties_in_models.md create mode 100644 end_to_end_tests/golden-record/my_test_api_client/api/naming/mixed_case.py create mode 100644 end_to_end_tests/golden-record/my_test_api_client/models/mixed_case_response_200.py diff --git a/.changeset/allow_parameters_with_names_differing_only_by_case.md b/.changeset/allow_parameters_with_names_differing_only_by_case.md new file mode 100644 index 000000000..6e0cd803e --- /dev/null +++ b/.changeset/allow_parameters_with_names_differing_only_by_case.md @@ -0,0 +1,10 @@ +--- +default: patch +--- + +# Allow parameters with names differing only by case + +If you have two parameters to an endpoint named `mixedCase` and `mixed_case`, previously, this was a conflict and the endpoint would not be generated. +Now, the generator will skip snake-casing the parameters and use the names as-is. Note that this means if neither of the parameters _was_ snake case, neither _will be_ in the generated code. + +Fixes #922 reported by @macmoritz & @benedikt-bartscher. diff --git a/.changeset/fix_naming_conflicts_with_properties_in_models.md b/.changeset/fix_naming_conflicts_with_properties_in_models.md new file mode 100644 index 000000000..683803a78 --- /dev/null +++ b/.changeset/fix_naming_conflicts_with_properties_in_models.md @@ -0,0 +1,32 @@ +--- +default: patch +--- + +# Fix naming conflicts with properties in models with mixed casing + +If you had an object with two properties, where the names differed only by case, conflicting properties would be generated in the model, which then failed the linting step (when using default config). For example, this: + +```yaml +type: "object" +properties: + MixedCase: + type: "string" + mixedCase: + type: "string" +``` + +Would generate a class like this: + +```python +class MyModel: + mixed_case: str + mixed_case: str +``` + +Now, neither of the properties will be forced into snake case, and the generated code will look like this: + +```python +class MyModel: + MixedCase: str + mixedCase: str +``` \ No newline at end of file diff --git a/end_to_end_tests/baseline_openapi_3.0.json b/end_to_end_tests/baseline_openapi_3.0.json index af8badc10..2ebc52322 100644 --- a/end_to_end_tests/baseline_openapi_3.0.json +++ b/end_to_end_tests/baseline_openapi_3.0.json @@ -1413,6 +1413,50 @@ } } }, + "/naming/mixed-case": { + "get": { + "tags": ["naming"], + "operationId": "mixed_case", + "parameters": [ + { + "name": "mixed_case", + "in": "query", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "mixedCase", + "in": "query", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "Successful response", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "mixed_case": { + "type": "string" + }, + "mixedCase": { + "type": "string" + } + } + } + } + } + } + } + } + }, "/parameter-references/{path_param}": { "get": { "tags": [ diff --git a/end_to_end_tests/baseline_openapi_3.1.yaml b/end_to_end_tests/baseline_openapi_3.1.yaml index f29cdfd21..4c715082c 100644 --- a/end_to_end_tests/baseline_openapi_3.1.yaml +++ b/end_to_end_tests/baseline_openapi_3.1.yaml @@ -1407,6 +1407,50 @@ info: } } }, + "/naming/mixed-case": { + "get": { + "tags": [ "naming" ], + "operationId": "mixed_case", + "parameters": [ + { + "name": "mixed_case", + "in": "query", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "mixedCase", + "in": "query", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "Successful response", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "mixed_case": { + "type": "string" + }, + "mixedCase": { + "type": "string" + } + } + } + } + } + } + } + } + }, "/parameter-references/{path_param}": { "get": { "tags": [ diff --git a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/naming/__init__.py b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/naming/__init__.py index 2931a25d4..af01857db 100644 --- a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/naming/__init__.py +++ b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/naming/__init__.py @@ -2,10 +2,14 @@ import types -from . import post_naming_property_conflict_with_import +from . import mixed_case, post_naming_property_conflict_with_import class NamingEndpoints: @classmethod def post_naming_property_conflict_with_import(cls) -> types.ModuleType: return post_naming_property_conflict_with_import + + @classmethod + def mixed_case(cls) -> types.ModuleType: + return mixed_case diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/naming/mixed_case.py b/end_to_end_tests/golden-record/my_test_api_client/api/naming/mixed_case.py new file mode 100644 index 000000000..4f6321261 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/api/naming/mixed_case.py @@ -0,0 +1,169 @@ +from http import HTTPStatus +from typing import Any, Dict, Optional, Union + +import httpx + +from ... import errors +from ...client import AuthenticatedClient, Client +from ...models.mixed_case_response_200 import MixedCaseResponse200 +from ...types import UNSET, Response + + +def _get_kwargs( + *, + mixed_case: str, + mixedCase: str, +) -> Dict[str, Any]: + params: Dict[str, Any] = {} + + params["mixed_case"] = mixed_case + + params["mixedCase"] = mixedCase + + params = {k: v for k, v in params.items() if v is not UNSET and v is not None} + + _kwargs: Dict[str, Any] = { + "method": "get", + "url": "/naming/mixed-case", + "params": params, + } + + return _kwargs + + +def _parse_response( + *, client: Union[AuthenticatedClient, Client], response: httpx.Response +) -> Optional[MixedCaseResponse200]: + if response.status_code == HTTPStatus.OK: + response_200 = MixedCaseResponse200.from_dict(response.json()) + + return response_200 + if client.raise_on_unexpected_status: + raise errors.UnexpectedStatus(response.status_code, response.content) + else: + return None + + +def _build_response( + *, client: Union[AuthenticatedClient, Client], response: httpx.Response +) -> Response[MixedCaseResponse200]: + return Response( + status_code=HTTPStatus(response.status_code), + content=response.content, + headers=response.headers, + parsed=_parse_response(client=client, response=response), + ) + + +def sync_detailed( + *, + client: Union[AuthenticatedClient, Client], + mixed_case: str, + mixedCase: str, +) -> Response[MixedCaseResponse200]: + """ + Args: + mixed_case (str): + mixedCase (str): + + Raises: + errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. + httpx.TimeoutException: If the request takes longer than Client.timeout. + + Returns: + Response[MixedCaseResponse200] + """ + + kwargs = _get_kwargs( + mixed_case=mixed_case, + mixedCase=mixedCase, + ) + + response = client.get_httpx_client().request( + **kwargs, + ) + + return _build_response(client=client, response=response) + + +def sync( + *, + client: Union[AuthenticatedClient, Client], + mixed_case: str, + mixedCase: str, +) -> Optional[MixedCaseResponse200]: + """ + Args: + mixed_case (str): + mixedCase (str): + + Raises: + errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. + httpx.TimeoutException: If the request takes longer than Client.timeout. + + Returns: + MixedCaseResponse200 + """ + + return sync_detailed( + client=client, + mixed_case=mixed_case, + mixedCase=mixedCase, + ).parsed + + +async def asyncio_detailed( + *, + client: Union[AuthenticatedClient, Client], + mixed_case: str, + mixedCase: str, +) -> Response[MixedCaseResponse200]: + """ + Args: + mixed_case (str): + mixedCase (str): + + Raises: + errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. + httpx.TimeoutException: If the request takes longer than Client.timeout. + + Returns: + Response[MixedCaseResponse200] + """ + + kwargs = _get_kwargs( + mixed_case=mixed_case, + mixedCase=mixedCase, + ) + + response = await client.get_async_httpx_client().request(**kwargs) + + return _build_response(client=client, response=response) + + +async def asyncio( + *, + client: Union[AuthenticatedClient, Client], + mixed_case: str, + mixedCase: str, +) -> Optional[MixedCaseResponse200]: + """ + Args: + mixed_case (str): + mixedCase (str): + + Raises: + errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. + httpx.TimeoutException: If the request takes longer than Client.timeout. + + Returns: + MixedCaseResponse200 + """ + + return ( + await asyncio_detailed( + client=client, + mixed_case=mixed_case, + mixedCase=mixedCase, + ) + ).parsed diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py index 39c021a37..cf12278fc 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py +++ b/end_to_end_tests/golden-record/my_test_api_client/models/__init__.py @@ -40,6 +40,7 @@ from .http_validation_error import HTTPValidationError from .import_ import Import from .json_like_body import JsonLikeBody +from .mixed_case_response_200 import MixedCaseResponse200 from .model_from_all_of import ModelFromAllOf from .model_name import ModelName from .model_reference_with_periods import ModelReferenceWithPeriods @@ -116,6 +117,7 @@ "HTTPValidationError", "Import", "JsonLikeBody", + "MixedCaseResponse200", "ModelFromAllOf", "ModelName", "ModelReferenceWithPeriods", diff --git a/end_to_end_tests/golden-record/my_test_api_client/models/mixed_case_response_200.py b/end_to_end_tests/golden-record/my_test_api_client/models/mixed_case_response_200.py new file mode 100644 index 000000000..21bdd918d --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/models/mixed_case_response_200.py @@ -0,0 +1,67 @@ +from typing import Any, Dict, List, Type, TypeVar, Union + +from attrs import define as _attrs_define +from attrs import field as _attrs_field + +from ..types import UNSET, Unset + +T = TypeVar("T", bound="MixedCaseResponse200") + + +@_attrs_define +class MixedCaseResponse200: + """ + Attributes: + mixed_case (Union[Unset, str]): + mixedCase (Union[Unset, str]): + """ + + mixed_case: Union[Unset, str] = UNSET + mixedCase: Union[Unset, str] = UNSET + additional_properties: Dict[str, Any] = _attrs_field(init=False, factory=dict) + + def to_dict(self) -> Dict[str, Any]: + mixed_case = self.mixed_case + + mixedCase = self.mixedCase + + field_dict: Dict[str, Any] = {} + field_dict.update(self.additional_properties) + field_dict.update({}) + if mixed_case is not UNSET: + field_dict["mixed_case"] = mixed_case + if mixedCase is not UNSET: + field_dict["mixedCase"] = mixedCase + + return field_dict + + @classmethod + def from_dict(cls: Type[T], src_dict: Dict[str, Any]) -> T: + d = src_dict.copy() + mixed_case = d.pop("mixed_case", UNSET) + + mixedCase = d.pop("mixedCase", UNSET) + + mixed_case_response_200 = cls( + mixed_case=mixed_case, + mixedCase=mixedCase, + ) + + mixed_case_response_200.additional_properties = d + return mixed_case_response_200 + + @property + def additional_keys(self) -> List[str]: + return list(self.additional_properties.keys()) + + def __getitem__(self, key: str) -> Any: + return self.additional_properties[key] + + def __setitem__(self, key: str, value: Any) -> None: + self.additional_properties[key] = value + + def __delitem__(self, key: str) -> None: + del self.additional_properties[key] + + def __contains__(self, key: str) -> bool: + return key in self.additional_properties diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py index 37d50bca5..f569cd429 100644 --- a/openapi_python_client/parser/openapi.py +++ b/openapi_python_client/parser/openapi.py @@ -134,14 +134,13 @@ class Endpoint: tag: str 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) - header_parameters: Dict[str, Property] = field(default_factory=dict) - cookie_parameters: Dict[str, Property] = field(default_factory=dict) + query_parameters: Dict[PythonIdentifier, Property] = field(default_factory=dict) + path_parameters: "OrderedDict[PythonIdentifier, Property]" = field(default_factory=OrderedDict) + header_parameters: Dict[PythonIdentifier, Property] = field(default_factory=dict) + cookie_parameters: Dict[PythonIdentifier, Property] = field(default_factory=dict) responses: List[Response] = field(default_factory=list) bodies: List[Body] = field(default_factory=list) errors: List[ParseError] = field(default_factory=list) - used_python_identifiers: Set[PythonIdentifier] = field(default_factory=set) @staticmethod def _add_responses( @@ -228,7 +227,7 @@ def add_parameters( # noqa: PLR0911 endpoint = deepcopy(endpoint) unique_parameters: Set[Tuple[str, oai.ParameterLocation]] = set() - parameters_by_location: Dict[str, Dict[str, Property]] = { + parameters_by_location: Dict[str, Dict[PythonIdentifier, Property]] = { oai.ParameterLocation.QUERY: endpoint.query_parameters, oai.ParameterLocation.PATH: endpoint.path_parameters, oai.ParameterLocation.HEADER: endpoint.header_parameters, @@ -303,41 +302,39 @@ def add_parameters( # noqa: PLR0911 # This parameter was defined in the Operation, so ignore the PathItem definition continue + # Check for conflicting parameters for location, parameters_dict in parameters_by_location.items(): - if location == param.param_in or prop.name not in parameters_dict: + conflicting_prop = parameters_dict.pop(prop.python_name, None) + if conflicting_prop is None: continue - existing_prop: Property = parameters_dict[prop.name] - # Existing should be converted too for consistency - endpoint.used_python_identifiers.discard(existing_prop.python_name) - existing_prop.set_python_name(new_name=f"{existing_prop.name}_{location}", config=config) - if existing_prop.python_name in endpoint.used_python_identifiers: + if location != param.param_in: # Use the location to differentiate + conflicting_prop.set_python_name(new_name=f"{conflicting_prop.python_name}_{location}", config=config) + prop.set_python_name(new_name=f"{param.name}_{param.param_in}", config=config) + elif conflicting_prop.name != prop.name: # Use the name to differentiate + conflicting_prop.set_python_name(new_name=conflicting_prop.name, config=config, skip_snake_case=True) + prop.set_python_name(new_name=prop.name, config=config, skip_snake_case=True) + parameters_dict[conflicting_prop.python_name] = conflicting_prop + + conflicting_name = None + if conflicting_prop.python_name in location: + conflicting_name = conflicting_prop.python_name + elif prop.python_name in parameters_by_location[param.param_in]: + conflicting_name = prop.python_name + if conflicting_name is not None: return ( ParseError( - detail=f"Parameters with same Python identifier `{existing_prop.python_name}` detected", + detail=f"Parameters with same Python identifier `{conflicting_name}` detected", data=data, ), schemas, parameters, ) - endpoint.used_python_identifiers.add(existing_prop.python_name) - prop.set_python_name(new_name=f"{param.name}_{param.param_in}", config=config) - - if prop.python_name in endpoint.used_python_identifiers: - return ( - ParseError( - detail=f"Parameters with same Python identifier `{prop.python_name}` detected", - data=data, - ), - schemas, - parameters, - ) # No reasons to use lazy imports in endpoints, so add lazy imports to relative here. endpoint.relative_imports.update(prop.get_lazy_imports(prefix=models_relative_prefix)) endpoint.relative_imports.update(prop.get_imports(prefix=models_relative_prefix)) - endpoint.used_python_identifiers.add(prop.python_name) - parameters_by_location[param.param_in][prop.name] = prop + parameters_by_location[param.param_in][prop.python_name] = prop return endpoint, schemas, parameters diff --git a/openapi_python_client/parser/properties/model_property.py b/openapi_python_client/parser/properties/model_property.py index 76c55a97c..bde45ac05 100644 --- a/openapi_python_client/parser/properties/model_property.py +++ b/openapi_python_client/parser/properties/model_property.py @@ -267,6 +267,17 @@ def _merge_properties(first: Property, second: Property) -> Property | PropertyE ) +def _resolve_naming_conflict(first: Property, second: Property, config: Config) -> PropertyError | None: + first.set_python_name(first.name, config=config, skip_snake_case=True) + second.set_python_name(second.name, config=config, skip_snake_case=True) + if first.python_name == second.python_name: + return PropertyError( + header="Conflicting property names", + detail=f"Properties {first.name} and {second.name} have the same python_name", + ) + return None + + class _PropertyData(NamedTuple): optional_props: list[Property] required_props: list[Property] @@ -293,13 +304,23 @@ def _process_properties( # noqa: PLR0912, PLR0911 def _add_if_no_conflict(new_prop: Property) -> PropertyError | None: nonlocal properties - existing = properties.get(new_prop.name) - merged_prop_or_error = _merge_properties(existing, new_prop) if existing else new_prop + name_conflict = properties.get(new_prop.name) + merged_prop_or_error = _merge_properties(name_conflict, new_prop) if name_conflict else new_prop if isinstance(merged_prop_or_error, PropertyError): merged_prop_or_error.header = ( f"Found conflicting properties named {new_prop.name} when creating {class_name}" ) return merged_prop_or_error + + for other_prop in properties.values(): + if other_prop.name == merged_prop_or_error.name: + continue # Same property, probably just got merged + if other_prop.python_name != merged_prop_or_error.python_name: + continue + naming_error = _resolve_naming_conflict(merged_prop_or_error, other_prop, config) + if naming_error is not None: + return naming_error + properties[merged_prop_or_error.name] = merged_prop_or_error return None diff --git a/openapi_python_client/parser/properties/protocol.py b/openapi_python_client/parser/properties/protocol.py index c7907da98..9a1404065 100644 --- a/openapi_python_client/parser/properties/protocol.py +++ b/openapi_python_client/parser/properties/protocol.py @@ -67,14 +67,14 @@ def validate_location(self, location: oai.ParameterLocation) -> ParseError | Non return ParseError(detail="Path parameter must be required") return None - def set_python_name(self, new_name: str, config: Config) -> None: + def set_python_name(self, new_name: str, config: Config, skip_snake_case: bool = False) -> None: """Mutates this Property to set a new python_name. Required to mutate due to how Properties are stored and the difficulty of updating them in-dict. `new_name` will be validated before it is set, so `python_name` is not guaranteed to equal `new_name` after calling this. """ - object.__setattr__(self, "python_name", PythonIdentifier(value=new_name, prefix=config.field_prefix)) + object.__setattr__(self, "python_name", PythonIdentifier(value=new_name, prefix=config.field_prefix, skip_snake_case=skip_snake_case)) def get_base_type_string(self, *, quoted: bool = False) -> str: """Get the string describing the Python type of this property. Base types no require quoting.""" diff --git a/openapi_python_client/utils.py b/openapi_python_client/utils.py index ea19622c4..834e2666c 100644 --- a/openapi_python_client/utils.py +++ b/openapi_python_client/utils.py @@ -12,8 +12,11 @@ class PythonIdentifier(str): """A snake_case string which has been validated / transformed into a valid identifier for Python""" - def __new__(cls, value: str, prefix: str) -> PythonIdentifier: - new_value = fix_reserved_words(snake_case(sanitize(value))) + def __new__(cls, value: str, prefix: str, skip_snake_case: bool = False) -> PythonIdentifier: + new_value = sanitize(value) + if not skip_snake_case: + new_value = snake_case(new_value) + new_value = fix_reserved_words(new_value) if not new_value.isidentifier() or value.startswith("_"): new_value = f"{prefix}{new_value}" From 98d0b5485d5043782854087f60b7520dbc1b0e10 Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Tue, 20 Feb 2024 18:05:06 -0700 Subject: [PATCH 2/5] Fix tests and lints --- ...tes_changed_type_of_endpoint_parameters.md | 16 ++ .../updated_generated_config_for_ruff_v02.md | 7 + ...ing_strategy_for_conflicting_properties.md | 8 + .../get_parameter_references_path_param.py | 4 +- ...lete_common_parameters_overriding_param.py | 4 +- .../get_common_parameters_overriding_param.py | 4 +- .../get_same_name_multiple_locations_param.py | 4 +- end_to_end_tests/golden-record/pyproject.toml | 4 +- .../metadata_snapshots/pdm.pyproject.toml | 4 +- .../metadata_snapshots/poetry.pyproject.toml | 4 +- .../test-3-1-golden-record/pyproject.toml | 4 +- .../api/const/post_const_path.py | 8 +- integration-tests/pyproject.toml | 3 +- openapi_python_client/parser/openapi.py | 148 ++++++++++-------- .../parser/properties/protocol.py | 6 +- .../templates/endpoint_macros.py.jinja | 22 +-- .../templates/endpoint_module.py.jinja | 2 +- .../templates/pyproject_ruff.toml.jinja | 4 +- pdm.lock | 2 +- pyproject.toml | 8 +- tests/test_parser/test_openapi.py | 114 +------------- 21 files changed, 175 insertions(+), 205 deletions(-) create mode 100644 .changeset/for_custom_templates_changed_type_of_endpoint_parameters.md create mode 100644 .changeset/updated_generated_config_for_ruff_v02.md create mode 100644 .changeset/updated_naming_strategy_for_conflicting_properties.md diff --git a/.changeset/for_custom_templates_changed_type_of_endpoint_parameters.md b/.changeset/for_custom_templates_changed_type_of_endpoint_parameters.md new file mode 100644 index 000000000..907d439ec --- /dev/null +++ b/.changeset/for_custom_templates_changed_type_of_endpoint_parameters.md @@ -0,0 +1,16 @@ +--- +default: major +--- + +# For custom templates, changed type of endpoint parameters + +**This does not affect projects that are not using `--custom-template-path`** + +The type of these properties on `Endpoint` has been changed from `Dict[str, Property]` to `List[Property]`: + +- `path_parameters` +- `query_parameters` +- `header_parameters` +- `cookie_parameters` + +If your templates are very close to the default templates, you can probably just remove `.values()` anywhere it appears. diff --git a/.changeset/updated_generated_config_for_ruff_v02.md b/.changeset/updated_generated_config_for_ruff_v02.md new file mode 100644 index 000000000..88517d56c --- /dev/null +++ b/.changeset/updated_generated_config_for_ruff_v02.md @@ -0,0 +1,7 @@ +--- +default: major +--- + +# Updated generated config for Ruff v0.2 + +This only affects projects using the `generate` command, not the `update` command. The `pyproject.toml` file generated which configures Ruff for linting and formatting has been updated to the 0.2 syntax, which means it will no longer work with Ruff 0.1. diff --git a/.changeset/updated_naming_strategy_for_conflicting_properties.md b/.changeset/updated_naming_strategy_for_conflicting_properties.md new file mode 100644 index 000000000..5660fed07 --- /dev/null +++ b/.changeset/updated_naming_strategy_for_conflicting_properties.md @@ -0,0 +1,8 @@ +--- +default: major +--- + +# Updated naming strategy for conflicting properties + +While fixing #922, some naming strategies were updated. These should mostly be backwards compatible, but there may be +some small differences in generated code. Make sure to check your diffs before pushing updates to consumers! diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/parameter_references/get_parameter_references_path_param.py b/end_to_end_tests/golden-record/my_test_api_client/api/parameter_references/get_parameter_references_path_param.py index 4b034ffe4..332c87874 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/api/parameter_references/get_parameter_references_path_param.py +++ b/end_to_end_tests/golden-record/my_test_api_client/api/parameter_references/get_parameter_references_path_param.py @@ -73,9 +73,9 @@ def sync_detailed( """Test different types of parameter references Args: - path_param (str): string_param (Union[Unset, str]): integer_param (Union[Unset, int]): Default: 0. + path_param (str): header_param (Union[None, Unset, str]): cookie_param (Union[Unset, str]): @@ -114,9 +114,9 @@ async def asyncio_detailed( """Test different types of parameter references Args: - path_param (str): string_param (Union[Unset, str]): integer_param (Union[Unset, int]): Default: 0. + path_param (str): header_param (Union[None, Unset, str]): cookie_param (Union[Unset, str]): 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 8203fa750..61e248529 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 @@ -54,8 +54,8 @@ def sync_detailed( ) -> Response[Any]: """ Args: - param_path (str): param_query (Union[Unset, str]): + param_path (str): Raises: errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. @@ -85,8 +85,8 @@ async def asyncio_detailed( ) -> Response[Any]: """ Args: - param_path (str): param_query (Union[Unset, str]): + param_path (str): Raises: errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. 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 985e92c20..91a7b2c17 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 @@ -55,9 +55,9 @@ def sync_detailed( """Test that if you have an overriding property from `PathItem` in `Operation`, it produces valid code Args: - param_path (str): param_query (str): A parameter with the same name as another. Default: 'overridden_in_GET'. Example: an example string. + param_path (str): Raises: errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. @@ -88,9 +88,9 @@ async def asyncio_detailed( """Test that if you have an overriding property from `PathItem` in `Operation`, it produces valid code Args: - param_path (str): param_query (str): A parameter with the same name as another. Default: 'overridden_in_GET'. Example: an example string. + param_path (str): Raises: errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. 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 43f3b8993..622181296 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 @@ -68,8 +68,8 @@ def sync_detailed( ) -> Response[Any]: """ Args: - param_path (str): param_query (Union[Unset, str]): + param_path (str): param_header (Union[Unset, str]): param_cookie (Union[Unset, str]): @@ -105,8 +105,8 @@ async def asyncio_detailed( ) -> Response[Any]: """ Args: - param_path (str): param_query (Union[Unset, str]): + param_path (str): param_header (Union[Unset, str]): param_cookie (Union[Unset, str]): diff --git a/end_to_end_tests/golden-record/pyproject.toml b/end_to_end_tests/golden-record/pyproject.toml index 2eed9cffe..4ce4ebb31 100644 --- a/end_to_end_tests/golden-record/pyproject.toml +++ b/end_to_end_tests/golden-record/pyproject.toml @@ -21,5 +21,7 @@ requires = ["poetry-core>=1.0.0"] build-backend = "poetry.core.masonry.api" [tool.ruff] -select = ["F", "I", "UP"] line-length = 120 + +[tool.ruff.lint] +select = ["F", "I", "UP"] diff --git a/end_to_end_tests/metadata_snapshots/pdm.pyproject.toml b/end_to_end_tests/metadata_snapshots/pdm.pyproject.toml index 0cc547a17..7c353bd07 100644 --- a/end_to_end_tests/metadata_snapshots/pdm.pyproject.toml +++ b/end_to_end_tests/metadata_snapshots/pdm.pyproject.toml @@ -19,5 +19,7 @@ requires = ["pdm-backend"] build-backend = "pdm.backend" [tool.ruff] -select = ["F", "I", "UP"] line-length = 120 + +[tool.ruff.lint] +select = ["F", "I", "UP"] diff --git a/end_to_end_tests/metadata_snapshots/poetry.pyproject.toml b/end_to_end_tests/metadata_snapshots/poetry.pyproject.toml index 1db67cd2b..e15331173 100644 --- a/end_to_end_tests/metadata_snapshots/poetry.pyproject.toml +++ b/end_to_end_tests/metadata_snapshots/poetry.pyproject.toml @@ -21,5 +21,7 @@ requires = ["poetry-core>=1.0.0"] build-backend = "poetry.core.masonry.api" [tool.ruff] -select = ["F", "I", "UP"] line-length = 120 + +[tool.ruff.lint] +select = ["F", "I", "UP"] diff --git a/end_to_end_tests/test-3-1-golden-record/pyproject.toml b/end_to_end_tests/test-3-1-golden-record/pyproject.toml index 1db67cd2b..e15331173 100644 --- a/end_to_end_tests/test-3-1-golden-record/pyproject.toml +++ b/end_to_end_tests/test-3-1-golden-record/pyproject.toml @@ -21,5 +21,7 @@ requires = ["poetry-core>=1.0.0"] build-backend = "poetry.core.masonry.api" [tool.ruff] -select = ["F", "I", "UP"] line-length = 120 + +[tool.ruff.lint] +select = ["F", "I", "UP"] diff --git a/end_to_end_tests/test-3-1-golden-record/test_3_1_features_client/api/const/post_const_path.py b/end_to_end_tests/test-3-1-golden-record/test_3_1_features_client/api/const/post_const_path.py index 3f864b3dc..9539ed41f 100644 --- a/end_to_end_tests/test-3-1-golden-record/test_3_1_features_client/api/const/post_const_path.py +++ b/end_to_end_tests/test-3-1-golden-record/test_3_1_features_client/api/const/post_const_path.py @@ -74,9 +74,9 @@ def sync_detailed( ) -> Response[Literal["Why have a fixed response? I dunno"]]: """ Args: - path (Literal['this goes in the path']): required_query (Literal['this always goes in the query']): optional_query (Union[Literal['this sometimes goes in the query'], Unset]): + path (Literal['this goes in the path']): body (PostConstPathBody): Raises: @@ -111,9 +111,9 @@ def sync( ) -> Optional[Literal["Why have a fixed response? I dunno"]]: """ Args: - path (Literal['this goes in the path']): required_query (Literal['this always goes in the query']): optional_query (Union[Literal['this sometimes goes in the query'], Unset]): + path (Literal['this goes in the path']): body (PostConstPathBody): Raises: @@ -143,9 +143,9 @@ async def asyncio_detailed( ) -> Response[Literal["Why have a fixed response? I dunno"]]: """ Args: - path (Literal['this goes in the path']): required_query (Literal['this always goes in the query']): optional_query (Union[Literal['this sometimes goes in the query'], Unset]): + path (Literal['this goes in the path']): body (PostConstPathBody): Raises: @@ -178,9 +178,9 @@ async def asyncio( ) -> Optional[Literal["Why have a fixed response? I dunno"]]: """ Args: - path (Literal['this goes in the path']): required_query (Literal['this always goes in the query']): optional_query (Union[Literal['this sometimes goes in the query'], Unset]): + path (Literal['this goes in the path']): body (PostConstPathBody): Raises: diff --git a/integration-tests/pyproject.toml b/integration-tests/pyproject.toml index c2cbd3226..0d43b24a8 100644 --- a/integration-tests/pyproject.toml +++ b/integration-tests/pyproject.toml @@ -26,6 +26,7 @@ requires = ["pdm-backend"] build-backend = "pdm.backend" [tool.ruff] -select = ["F", "I"] line-length = 120 +[tool.ruff.lint] +select = ["F", "I"] diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py index f569cd429..e111a0f51 100644 --- a/openapi_python_client/parser/openapi.py +++ b/openapi_python_client/parser/openapi.py @@ -1,5 +1,4 @@ import re -from collections import OrderedDict from copy import deepcopy from dataclasses import dataclass, field from http import HTTPStatus @@ -14,7 +13,6 @@ from .bodies import Body, body_from_data from .errors import GeneratorError, ParseError, PropertyError from .properties import ( - AnyProperty, Class, EnumProperty, ModelProperty, @@ -134,10 +132,10 @@ class Endpoint: tag: str summary: Optional[str] = "" relative_imports: Set[str] = field(default_factory=set) - query_parameters: Dict[PythonIdentifier, Property] = field(default_factory=dict) - path_parameters: "OrderedDict[PythonIdentifier, Property]" = field(default_factory=OrderedDict) - header_parameters: Dict[PythonIdentifier, Property] = field(default_factory=dict) - cookie_parameters: Dict[PythonIdentifier, Property] = field(default_factory=dict) + query_parameters: List[Property] = field(default_factory=list) + path_parameters: List[Property] = field(default_factory=list) + header_parameters: List[Property] = field(default_factory=list) + cookie_parameters: List[Property] = field(default_factory=list) responses: List[Response] = field(default_factory=list) bodies: List[Body] = field(default_factory=list) errors: List[ParseError] = field(default_factory=list) @@ -190,7 +188,7 @@ def _add_responses( return endpoint, schemas @staticmethod - def add_parameters( # noqa: PLR0911 + def add_parameters( *, endpoint: "Endpoint", data: Union[oai.Operation, oai.PathItem], @@ -227,22 +225,11 @@ def add_parameters( # noqa: PLR0911 endpoint = deepcopy(endpoint) unique_parameters: Set[Tuple[str, oai.ParameterLocation]] = set() - parameters_by_location: Dict[str, Dict[PythonIdentifier, Property]] = { + parameters_by_location: Dict[str, List[Property]] = { oai.ParameterLocation.QUERY: endpoint.query_parameters, oai.ParameterLocation.PATH: endpoint.path_parameters, oai.ParameterLocation.HEADER: endpoint.header_parameters, oai.ParameterLocation.COOKIE: endpoint.cookie_parameters, - "RESERVED": { # These can't be param names because codegen needs them as vars, the properties don't matter - "client": AnyProperty( - "client", - True, - None, - PythonIdentifier("client", ""), - None, - None, - ), - "url": AnyProperty("url", True, None, PythonIdentifier("url", ""), None, None), - }, } for param in data.parameters: @@ -272,6 +259,12 @@ def add_parameters( # noqa: PLR0911 unique_parameters.add(unique_param) + if any( + other_param for other_param in parameters_by_location[param.param_in] if other_param.name == param.name + ): + # Defined at the operation level, ignore it here + continue + prop, new_schemas = property_from_data( name=param.name, required=param.required, @@ -298,45 +291,69 @@ def add_parameters( # noqa: PLR0911 location_error.data = param return location_error, schemas, parameters - if prop.name in parameters_by_location[param.param_in]: - # This parameter was defined in the Operation, so ignore the PathItem definition + # No reasons to use lazy imports in endpoints, so add lazy imports to relative here. + endpoint.relative_imports.update(prop.get_lazy_imports(prefix=models_relative_prefix)) + endpoint.relative_imports.update(prop.get_imports(prefix=models_relative_prefix)) + parameters_by_location[param.param_in].append(prop) + + return endpoint._check_parameters_for_conflicts(config=config), schemas, parameters + + def _check_parameters_for_conflicts( + self, + *, + config: Config, + previously_modified_params: Optional[Set[Tuple[oai.ParameterLocation, str]]] = None, + ) -> Union["Endpoint", ParseError]: + """Check for conflicting parameters + + For parameters that have the same python_name but are in different locations, append the location to the + python_name. For parameters that have the same name but are in the same location, use their raw name without + snake casing instead. + + Function stops when there's a conflict that can't be resolved or all parameters are guaranteed to have a + unique python_name. + """ + modified_params = previously_modified_params or set() + used_python_names: Dict[PythonIdentifier, Tuple[oai.ParameterLocation, Property]] = {} + reserved_names = ["client", "url"] + for parameter in self.iter_all_parameters(): + location, prop = parameter + + if prop.python_name in reserved_names: + prop.set_python_name(new_name=f"{prop.python_name}_{location}", config=config) + modified_params.add((location, prop.name)) continue - # Check for conflicting parameters - for location, parameters_dict in parameters_by_location.items(): - conflicting_prop = parameters_dict.pop(prop.python_name, None) - if conflicting_prop is None: - continue + conflicting = used_python_names.pop(prop.python_name, None) + if conflicting is None: + used_python_names[prop.python_name] = parameter + continue + conflicting_location, conflicting_prop = conflicting + if (conflicting_location, conflicting_prop.name) in modified_params or ( + location, + prop.name, + ) in modified_params: + return ParseError( + detail=f"Parameters with same Python identifier {conflicting_prop.python_name} detected", + ) - if location != param.param_in: # Use the location to differentiate - conflicting_prop.set_python_name(new_name=f"{conflicting_prop.python_name}_{location}", config=config) - prop.set_python_name(new_name=f"{param.name}_{param.param_in}", config=config) - elif conflicting_prop.name != prop.name: # Use the name to differentiate - conflicting_prop.set_python_name(new_name=conflicting_prop.name, config=config, skip_snake_case=True) - prop.set_python_name(new_name=prop.name, config=config, skip_snake_case=True) - parameters_dict[conflicting_prop.python_name] = conflicting_prop - - conflicting_name = None - if conflicting_prop.python_name in location: - conflicting_name = conflicting_prop.python_name - elif prop.python_name in parameters_by_location[param.param_in]: - conflicting_name = prop.python_name - if conflicting_name is not None: - return ( - ParseError( - detail=f"Parameters with same Python identifier `{conflicting_name}` detected", - data=data, - ), - schemas, - parameters, - ) + if location != conflicting_location: + conflicting_prop.set_python_name( + new_name=f"{conflicting_prop.python_name}_{conflicting_location}", config=config + ) + prop.set_python_name(new_name=f"{prop.python_name}_{location}", config=config) + elif conflicting_prop.name != prop.name: # Use the name to differentiate + conflicting_prop.set_python_name(new_name=conflicting_prop.name, config=config, skip_snake_case=True) + prop.set_python_name(new_name=prop.name, config=config, skip_snake_case=True) - # No reasons to use lazy imports in endpoints, so add lazy imports to relative here. - endpoint.relative_imports.update(prop.get_lazy_imports(prefix=models_relative_prefix)) - endpoint.relative_imports.update(prop.get_imports(prefix=models_relative_prefix)) - parameters_by_location[param.param_in][prop.python_name] = prop + modified_params.add((location, conflicting_prop.name)) + modified_params.add((conflicting_location, conflicting_prop.name)) + used_python_names[prop.python_name] = parameter + used_python_names[conflicting_prop.python_name] = conflicting - return endpoint, schemas, parameters + if len(modified_params) > 0 and modified_params != previously_modified_params: + return self._check_parameters_for_conflicts(config=config, previously_modified_params=modified_params) + return self @staticmethod def sort_parameters(*, endpoint: "Endpoint") -> Union["Endpoint", ParseError]: @@ -353,15 +370,13 @@ 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(), + endpoint.path_parameters.sort( 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 - if parameters_from_path != list(endpoint.path_parameters): + if parameters_from_path != [param.name for param in endpoint.path_parameters]: return ParseError( detail=f"Incorrect path templating for {endpoint.path} (Path parameters do not match with path)", ) @@ -439,17 +454,22 @@ def response_type(self) -> str: return self.responses[0].prop.get_type_string(quoted=False) return f"Union[{', '.join(types)}]" - def iter_all_parameters(self) -> Iterator[Property]: + def iter_all_parameters(self) -> Iterator[Tuple[oai.ParameterLocation, Property]]: """Iterate through all the parameters of this endpoint""" - yield from self.path_parameters.values() - yield from self.query_parameters.values() - yield from self.header_parameters.values() - yield from self.cookie_parameters.values() - yield from (body.prop for body in self.bodies) + yield from ((oai.ParameterLocation.PATH, param) for param in self.path_parameters) + yield from ((oai.ParameterLocation.QUERY, param) for param in self.query_parameters) + yield from ((oai.ParameterLocation.HEADER, param) for param in self.header_parameters) + yield from ((oai.ParameterLocation.COOKIE, param) for param in self.cookie_parameters) def list_all_parameters(self) -> List[Property]: """Return a List of all the parameters of this endpoint""" - return list(self.iter_all_parameters()) + return ( + self.query_parameters + + self.path_parameters + + self.header_parameters + + self.cookie_parameters + + [body.prop for body in self.bodies] + ) @dataclass diff --git a/openapi_python_client/parser/properties/protocol.py b/openapi_python_client/parser/properties/protocol.py index 9a1404065..b8237923d 100644 --- a/openapi_python_client/parser/properties/protocol.py +++ b/openapi_python_client/parser/properties/protocol.py @@ -74,7 +74,11 @@ def set_python_name(self, new_name: str, config: Config, skip_snake_case: bool = `new_name` will be validated before it is set, so `python_name` is not guaranteed to equal `new_name` after calling this. """ - object.__setattr__(self, "python_name", PythonIdentifier(value=new_name, prefix=config.field_prefix, skip_snake_case=skip_snake_case)) + object.__setattr__( + self, + "python_name", + PythonIdentifier(value=new_name, prefix=config.field_prefix, skip_snake_case=skip_snake_case), + ) def get_base_type_string(self, *, quoted: bool = False) -> str: """Get the string describing the Python type of this property. Base types no require quoting.""" diff --git a/openapi_python_client/templates/endpoint_macros.py.jinja b/openapi_python_client/templates/endpoint_macros.py.jinja index b4a7713fe..da02b5c4a 100644 --- a/openapi_python_client/templates/endpoint_macros.py.jinja +++ b/openapi_python_client/templates/endpoint_macros.py.jinja @@ -5,7 +5,7 @@ {% if endpoint.header_parameters or endpoint.bodies | length > 0 %} headers: Dict[str, Any] = {} {% if endpoint.header_parameters %} - {% for parameter in endpoint.header_parameters.values() %} + {% for parameter in endpoint.header_parameters %} {% import "property_templates/" + parameter.template as param_template %} {% if param_template.transform_header %} {% set expression = param_template.transform_header(parameter.python_name) %} @@ -22,7 +22,7 @@ headers: Dict[str, Any] = {} {% macro cookie_params(endpoint) %} {% if endpoint.cookie_parameters %} cookies = {} - {% for parameter in endpoint.cookie_parameters.values() %} + {% for parameter in endpoint.cookie_parameters %} {% if parameter.required %} cookies["{{ parameter.name}}"] = {{ parameter.python_name }} {% else %} @@ -39,7 +39,7 @@ if {{ parameter.python_name }} is not UNSET: {% if endpoint.query_parameters %} params: Dict[str, Any] = {} -{% for property in endpoint.query_parameters.values() %} +{% for property in endpoint.query_parameters %} {% set destination = property.python_name %} {% import "property_templates/" + property.template as prop_template %} {% if prop_template.transform %} @@ -91,7 +91,7 @@ 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, include_client=True) %} {# path parameters #} -{% for parameter in endpoint.path_parameters.values() %} +{% for parameter in endpoint.path_parameters %} {{ parameter.to_string() }}, {% endfor %} {% if include_client or ((endpoint.list_all_parameters() | length) > (endpoint.path_parameters | length)) %} @@ -116,21 +116,21 @@ body: Union[ ], {% endif %} {# query parameters #} -{% for parameter in endpoint.query_parameters.values() %} +{% for parameter in endpoint.query_parameters %} {{ parameter.to_string() }}, {% endfor %} -{% for parameter in endpoint.header_parameters.values() %} +{% for parameter in endpoint.header_parameters %} {{ parameter.to_string() }}, {% endfor %} {# cookie parameters #} -{% for parameter in endpoint.cookie_parameters.values() %} +{% for parameter in endpoint.cookie_parameters %} {{ parameter.to_string() }}, {% endfor %} {% endmacro %} {# Just lists all kwargs to endpoints as name=name for passing to other functions #} {% macro kwargs(endpoint, include_client=True) %} -{% for parameter in endpoint.path_parameters.values() %} +{% for parameter in endpoint.path_parameters %} {{ parameter.python_name }}={{ parameter.python_name }}, {% endfor %} {% if include_client %} @@ -139,13 +139,13 @@ client=client, {% if endpoint.bodies | length > 0 %} body=body, {% endif %} -{% for parameter in endpoint.query_parameters.values() %} +{% for parameter in endpoint.query_parameters %} {{ parameter.python_name }}={{ parameter.python_name }}, {% endfor %} -{% for parameter in endpoint.header_parameters.values() %} +{% for parameter in endpoint.header_parameters %} {{ parameter.python_name }}={{ parameter.python_name }}, {% endfor %} -{% for parameter in endpoint.cookie_parameters.values() %} +{% for parameter in endpoint.cookie_parameters %} {{ parameter.python_name }}={{ parameter.python_name }}, {% endfor %} {% endmacro %} diff --git a/openapi_python_client/templates/endpoint_module.py.jinja b/openapi_python_client/templates/endpoint_module.py.jinja index 8bd4430d7..46f4eb365 100644 --- a/openapi_python_client/templates/endpoint_module.py.jinja +++ b/openapi_python_client/templates/endpoint_module.py.jinja @@ -30,7 +30,7 @@ def _get_kwargs( "method": "{{ endpoint.method }}", {% if endpoint.path_parameters %} "url": "{{ endpoint.path }}".format( - {%- for parameter in endpoint.path_parameters.values() -%} + {%- for parameter in endpoint.path_parameters -%} {{parameter.name}}={{parameter.python_name}}, {%- endfor -%} ), diff --git a/openapi_python_client/templates/pyproject_ruff.toml.jinja b/openapi_python_client/templates/pyproject_ruff.toml.jinja index c65287746..c2e4ce24c 100644 --- a/openapi_python_client/templates/pyproject_ruff.toml.jinja +++ b/openapi_python_client/templates/pyproject_ruff.toml.jinja @@ -1,3 +1,5 @@ [tool.ruff] -select = ["F", "I", "UP"] line-length = 120 + +[tool.ruff.lint] +select = ["F", "I", "UP"] diff --git a/pdm.lock b/pdm.lock index 20490f470..c09cb9d23 100644 --- a/pdm.lock +++ b/pdm.lock @@ -5,7 +5,7 @@ groups = ["default", "dev"] strategy = ["cross_platform", "inherit_metadata"] lock_version = "4.4.1" -content_hash = "sha256:95755a37ee3e6a924fa539d7bf00ac5255bb5e3649405afd078ad8a8e8c85002" +content_hash = "sha256:a99cba70da9ca76f93d5eb6f1278fd8adb19836bed10fe05638c3720b795068f" [[package]] name = "annotated-types" diff --git a/pyproject.toml b/pyproject.toml index 2939d310c..2b1c81e7e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,7 +14,7 @@ dependencies = [ "python-dateutil>=2.8.1,<3.0.0", "httpx>=0.20.0,<0.27.0", "PyYAML>=6.0,<7.0", - "ruff>=0.1.2,<1.0.0", + "ruff>=0.2,<0.3", "typing-extensions>=4.8.0,<5.0.0", ] name = "openapi-python-client" @@ -47,7 +47,6 @@ repository = "https://github.com/openapi-generators/openapi-python-client" openapi-python-client = "openapi_python_client.cli:app" [tool.ruff] -select = ["E", "F", "I", "UP", "B", "PL", "RUF"] line-length = 120 exclude = [ ".git", @@ -57,9 +56,12 @@ exclude = [ "end_to_end_tests/*", "tests/test_templates/*", ] + +[tool.ruff.lint] +select = ["E", "F", "I", "UP", "B", "PL", "RUF"] ignore = ["E501", "PLR0913"] -[tool.ruff.per-file-ignores] +[tool.ruff.lint.per-file-ignores] "openapi_python_client/cli.py" = ["B008"] [tool.coverage.run] diff --git a/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py index 0bb8df491..43a9e8c42 100644 --- a/tests/test_parser/test_openapi.py +++ b/tests/test_parser/test_openapi.py @@ -293,104 +293,6 @@ def test_validation_error_when_location_not_supported(self, mocker): with pytest.raises(pydantic.ValidationError): oai.Parameter(name="test", required=True, param_schema=mocker.MagicMock(), param_in="error_location") - def test__add_parameters_with_location_postfix_conflict1(self, mocker, any_property_factory): - """Checks when the PythonIdentifier of new parameter already used.""" - from openapi_python_client.parser.openapi import Endpoint - - endpoint = self.make_endpoint() - - path_prop_conflicted = any_property_factory(name="prop_name_path", required=True, default=None) - query_prop = any_property_factory(name="prop_name", required=True, default=None) - path_prop = any_property_factory(name="prop_name", required=True, default=None) - - schemas_1 = mocker.MagicMock() - schemas_2 = mocker.MagicMock() - schemas_3 = mocker.MagicMock() - mocker.patch( - f"{MODULE_NAME}.property_from_data", - side_effect=[ - (path_prop_conflicted, schemas_1), - (query_prop, schemas_2), - (path_prop, schemas_3), - ], - ) - path_conflicted_schema = mocker.MagicMock() - query_schema = mocker.MagicMock() - path_schema = mocker.MagicMock() - - data = oai.Operation.model_construct( - parameters=[ - oai.Parameter.model_construct( - name=path_prop_conflicted.name, required=True, param_schema=path_conflicted_schema, param_in="path" - ), - oai.Parameter.model_construct( - name=query_prop.name, required=False, param_schema=query_schema, param_in="query" - ), - oai.Parameter.model_construct( - name=path_prop.name, required=True, param_schema=path_schema, param_in="path" - ), - oai.Reference.model_construct(), # Should be ignored - oai.Parameter.model_construct(), # Should be ignored - ] - ) - initial_schemas = mocker.MagicMock() - parameters = mocker.MagicMock() - config = MagicMock() - - result = Endpoint.add_parameters( - endpoint=endpoint, data=data, schemas=initial_schemas, parameters=parameters, config=config - )[0] - assert isinstance(result, ParseError) - assert result.detail == "Parameters with same Python identifier `prop_name_path` detected" - - def test__add_parameters_with_location_postfix_conflict2(self, mocker, any_property_factory): - """Checks when an existing parameter has a conflicting PythonIdentifier after renaming.""" - from openapi_python_client.parser.openapi import Endpoint - - endpoint = self.make_endpoint() - path_prop_conflicted = any_property_factory(name="prop_name_path", required=True, default=None) - path_prop = any_property_factory(name="prop_name", required=True, default=None) - query_prop = any_property_factory(name="prop_name", required=True, default=None) - schemas_1 = mocker.MagicMock() - schemas_2 = mocker.MagicMock() - schemas_3 = mocker.MagicMock() - mocker.patch( - f"{MODULE_NAME}.property_from_data", - side_effect=[ - (path_prop_conflicted, schemas_1), - (path_prop, schemas_2), - (query_prop, schemas_3), - ], - ) - path_conflicted_schema = mocker.MagicMock() - path_schema = mocker.MagicMock() - query_schema = mocker.MagicMock() - - data = oai.Operation.model_construct( - parameters=[ - oai.Parameter.model_construct( - name=path_prop_conflicted.name, required=True, param_schema=path_conflicted_schema, param_in="path" - ), - oai.Parameter.model_construct( - name=path_prop.name, required=True, param_schema=path_schema, param_in="path" - ), - oai.Parameter.model_construct( - name=query_prop.name, required=False, param_schema=query_schema, param_in="query" - ), - oai.Reference.model_construct(), # Should be ignored - oai.Parameter.model_construct(), # Should be ignored - ] - ) - initial_schemas = mocker.MagicMock() - parameters = mocker.MagicMock() - config = MagicMock() - - result = Endpoint.add_parameters( - endpoint=endpoint, data=data, schemas=initial_schemas, parameters=parameters, config=config - )[0] - assert isinstance(result, ParseError) - assert result.detail == "Parameters with same Python identifier `prop_name_path` detected" - def test__add_parameters_handles_invalid_references(self, config): """References are not supported as direct params yet""" endpoint = self.make_endpoint() @@ -505,7 +407,7 @@ def test__add_parameters_query_optionality(self, config): ) assert len(endpoint.query_parameters) == 2, "Not all query params were added" # noqa: PLR2004 - for param in endpoint.query_parameters.values(): + for param in endpoint.query_parameters: if param.name == "required": assert param.required else: @@ -557,8 +459,8 @@ def test_add_parameters_duplicate_properties_different_location(self, config): config=config, )[0] assert isinstance(result, Endpoint) - assert result.path_parameters["test"].name == "test" - assert result.query_parameters["test"].name == "test" + assert result.path_parameters[0].name == "test" + assert result.query_parameters[0].name == "test" def test_sort_parameters(self, string_property_factory): from openapi_python_client.parser.openapi import Endpoint @@ -568,10 +470,10 @@ def test_sort_parameters(self, string_property_factory): for i in range(1, 5): prop = string_property_factory(name=f"param{i}") - endpoint.path_parameters[prop.name] = prop + endpoint.path_parameters.append(prop) result = Endpoint.sort_parameters(endpoint=endpoint) - result_names = [name for name in result.path_parameters] + result_names = [param.name for param in result.path_parameters] expected_names = [f"param{i}" for i in (4, 2, 1, 3)] assert result_names == expected_names @@ -582,7 +484,7 @@ def test_sort_parameters_missing_param(self, string_property_factory): endpoint = self.make_endpoint() endpoint.path = "/multiple-path-parameters/{param1}/{param2}" param = string_property_factory(name="param1") - endpoint.path_parameters[param.name] = param + endpoint.path_parameters.append(param) result = Endpoint.sort_parameters(endpoint=endpoint) @@ -596,7 +498,7 @@ def test_sort_parameters_extra_param(self, string_property_factory): endpoint = self.make_endpoint() endpoint.path = "/multiple-path-parameters" param = string_property_factory(name="param1") - endpoint.path_parameters[param.name] = param + endpoint.path_parameters.append(param) result = Endpoint.sort_parameters(endpoint=endpoint) @@ -996,7 +898,7 @@ def test_from_data_overrides_path_item_params_with_operation_params(self, config config=config, ) collection: EndpointCollection = collections["default"] - assert isinstance(collection.endpoints[0].query_parameters["param"], IntProperty) + assert isinstance(collection.endpoints[0].query_parameters[0], IntProperty) def test_from_data_errors(self, mocker, config): from openapi_python_client.parser.openapi import ParseError From a572e905963ecade255f434021b0c856b1cc8136 Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Wed, 21 Feb 2024 17:47:08 -0700 Subject: [PATCH 3/5] test: Add coverage --- .../test_properties/test_model_property.py | 88 +++++-------------- 1 file changed, 23 insertions(+), 65 deletions(-) diff --git a/tests/test_parser/test_properties/test_model_property.py b/tests/test_parser/test_properties/test_model_property.py index 60629b40f..917582042 100644 --- a/tests/test_parser/test_properties/test_model_property.py +++ b/tests/test_parser/test_properties/test_model_property.py @@ -1,12 +1,12 @@ from typing import Optional -from unittest.mock import MagicMock import pytest -from attr._funcs import evolve +from attr import evolve import openapi_python_client.schema as oai from openapi_python_client.parser.errors import PropertyError -from openapi_python_client.parser.properties import StringProperty +from openapi_python_client.parser.properties import Schemas, StringProperty +from openapi_python_client.parser.properties.model_property import _process_properties MODULE_NAME = "openapi_python_client.parser.properties.model_property" @@ -167,7 +167,7 @@ def test_happy_path(self, model_property_factory, string_property_factory, date_ ) def test_model_name_conflict(self, config): - from openapi_python_client.parser.properties import ModelProperty, Schemas + from openapi_python_client.parser.properties import ModelProperty data = oai.Schema.model_construct() schemas = Schemas(classes_by_name={"OtherModel": None}) @@ -214,7 +214,7 @@ def test_model_naming( expected: str, config, ): - from openapi_python_client.parser.properties import ModelProperty, Schemas + from openapi_python_client.parser.properties import ModelProperty data = oai.Schema( title=title, @@ -234,7 +234,7 @@ def test_model_naming( assert result.class_info.name == expected def test_model_bad_properties(self, config): - from openapi_python_client.parser.properties import ModelProperty, Schemas + from openapi_python_client.parser.properties import ModelProperty data = oai.Schema( properties={ @@ -254,7 +254,7 @@ def test_model_bad_properties(self, config): assert isinstance(result, PropertyError) def test_model_bad_additional_properties(self, config): - from openapi_python_client.parser.properties import ModelProperty, Schemas + from openapi_python_client.parser.properties import ModelProperty additional_properties = oai.Schema( type="object", @@ -276,7 +276,7 @@ def test_model_bad_additional_properties(self, config): assert isinstance(result, PropertyError) def test_process_properties_false(self, model_property_factory, config): - from openapi_python_client.parser.properties import Class, ModelProperty, Schemas + from openapi_python_client.parser.properties import Class, ModelProperty name = "prop" required = True @@ -327,9 +327,6 @@ class TestProcessProperties: def test_conflicting_properties_different_types( self, model_property_factory, string_property_factory, date_time_property_factory, config ): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct( allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")] ) @@ -349,9 +346,6 @@ def test_conflicting_properties_different_types( assert isinstance(result, PropertyError) def test_process_properties_reference_not_exist(self, config): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema( properties={ "bad": oai.Reference.model_construct(ref="#/components/schema/NotExist"), @@ -363,9 +357,6 @@ def test_process_properties_reference_not_exist(self, config): assert isinstance(result, PropertyError) def test_process_properties_all_of_reference_not_exist(self, config): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct(allOf=[oai.Reference.model_construct(ref="#/components/schema/NotExist")]) result = _process_properties(data=data, class_name="", schemas=Schemas(), config=config, roots={"root"}) @@ -373,9 +364,6 @@ def test_process_properties_all_of_reference_not_exist(self, config): assert isinstance(result, PropertyError) def test_process_properties_model_property_roots(self, model_property_factory, config): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - roots = {"root"} data = oai.Schema(properties={"test_model_property": oai.Schema.model_construct(type="object")}) @@ -384,9 +372,6 @@ def test_process_properties_model_property_roots(self, model_property_factory, c assert all(root in result.optional_props[0].roots for root in roots) def test_invalid_reference(self, config): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct(allOf=[oai.Reference.model_construct(ref="ThisIsNotGood")]) schemas = Schemas() @@ -395,9 +380,6 @@ def test_invalid_reference(self, config): assert isinstance(result, PropertyError) def test_non_model_reference(self, enum_property_factory, config): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct(allOf=[oai.Reference.model_construct(ref="#/First")]) schemas = Schemas( classes_by_reference={ @@ -410,9 +392,6 @@ def test_non_model_reference(self, enum_property_factory, config): assert isinstance(result, PropertyError) def test_reference_not_processed(self, model_property_factory, config): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct(allOf=[oai.Reference.model_construct(ref="#/Unprocessed")]) schemas = Schemas( classes_by_reference={ @@ -425,9 +404,6 @@ def test_reference_not_processed(self, model_property_factory, config): assert isinstance(result, PropertyError) def test_conflicting_properties_same_types(self, model_property_factory, string_property_factory, config): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct( allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")] ) @@ -449,9 +425,6 @@ def test_conflicting_properties_same_types(self, model_property_factory, string_ def test_allof_string_and_string_enum( self, model_property_factory, enum_property_factory, string_property_factory, config ): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct( allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")] ) @@ -474,9 +447,6 @@ def test_allof_string_and_string_enum( def test_allof_string_enum_and_string( self, model_property_factory, enum_property_factory, string_property_factory, config ): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct( allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")] ) @@ -498,9 +468,6 @@ def test_allof_string_enum_and_string( assert result.optional_props[0] == enum_property def test_allof_int_and_int_enum(self, model_property_factory, enum_property_factory, int_property_factory, config): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct( allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")] ) @@ -521,9 +488,6 @@ def test_allof_int_and_int_enum(self, model_property_factory, enum_property_fact def test_allof_enum_incompatible_type( self, model_property_factory, enum_property_factory, int_property_factory, config ): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct( allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")] ) @@ -542,9 +506,6 @@ def test_allof_enum_incompatible_type( assert isinstance(result, PropertyError) def test_allof_string_enums(self, model_property_factory, enum_property_factory, config): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct( allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")] ) @@ -569,9 +530,6 @@ def test_allof_string_enums(self, model_property_factory, enum_property_factory, assert result.required_props[0] == enum_property1 def test_allof_int_enums(self, model_property_factory, enum_property_factory, config): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct( allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")] ) @@ -596,9 +554,6 @@ def test_allof_int_enums(self, model_property_factory, enum_property_factory, co assert result.required_props[0] == enum_property2 def test_allof_enums_are_not_subsets(self, model_property_factory, enum_property_factory, config): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct( allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")] ) @@ -623,9 +578,6 @@ def test_allof_enums_are_not_subsets(self, model_property_factory, enum_property assert isinstance(result, PropertyError) def test_duplicate_properties(self, model_property_factory, string_property_factory, config): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct( allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")] ) @@ -649,10 +601,8 @@ def test_mixed_requirements( first_required, second_required, string_property_factory, + config, ): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - data = oai.Schema.model_construct( allOf=[oai.Reference.model_construct(ref="#/First"), oai.Reference.model_construct(ref="#/Second")] ) @@ -670,7 +620,7 @@ def test_mixed_requirements( ) roots = {"root"} - result = _process_properties(data=data, schemas=schemas, class_name="", config=MagicMock(), roots=roots) + result = _process_properties(data=data, schemas=schemas, class_name="", config=config, roots=roots) required = first_required or second_required expected_prop = string_property_factory( @@ -683,10 +633,7 @@ def test_mixed_requirements( else: assert result.required_props == [expected_prop] - def test_direct_properties_non_ref(self, string_property_factory): - from openapi_python_client.parser.properties import Schemas - from openapi_python_client.parser.properties.model_property import _process_properties - + def test_direct_properties_non_ref(self, string_property_factory, config): data = oai.Schema.model_construct( allOf=[ oai.Schema.model_construct( @@ -700,11 +647,22 @@ def test_direct_properties_non_ref(self, string_property_factory): ) schemas = Schemas() - result = _process_properties(data=data, schemas=schemas, class_name="", config=MagicMock(), roots={"root"}) + result = _process_properties(data=data, schemas=schemas, class_name="", config=config, roots={"root"}) assert result.optional_props == [string_property_factory(name="second", required=False)] assert result.required_props == [string_property_factory(name="first", required=True)] + def test_conflicting_property_names(self, config): + data = oai.Schema.model_construct( + properties={ + "int": oai.Schema.model_construct(type="integer"), + "int_": oai.Schema.model_construct(type="string"), + } + ) + schemas = Schemas() + result = _process_properties(data=data, schemas=schemas, class_name="", config=config, roots={"root"}) + assert isinstance(result, PropertyError) + class TestProcessModel: def test_process_model_error(self, mocker, model_property_factory, config): From b7ee6064597f9c6a6fe117021ff0c187e91fd48f Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Wed, 21 Feb 2024 17:54:51 -0700 Subject: [PATCH 4/5] Restore docstrings to original order --- .../get_parameter_references_path_param.py | 4 ++-- .../delete_common_parameters_overriding_param.py | 4 ++-- .../parameters/get_common_parameters_overriding_param.py | 4 ++-- .../parameters/get_same_name_multiple_locations_param.py | 4 ++-- .../test_3_1_features_client/api/const/post_const_path.py | 8 ++++---- openapi_python_client/parser/openapi.py | 4 ++-- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/end_to_end_tests/golden-record/my_test_api_client/api/parameter_references/get_parameter_references_path_param.py b/end_to_end_tests/golden-record/my_test_api_client/api/parameter_references/get_parameter_references_path_param.py index 332c87874..4b034ffe4 100644 --- a/end_to_end_tests/golden-record/my_test_api_client/api/parameter_references/get_parameter_references_path_param.py +++ b/end_to_end_tests/golden-record/my_test_api_client/api/parameter_references/get_parameter_references_path_param.py @@ -73,9 +73,9 @@ def sync_detailed( """Test different types of parameter references Args: + path_param (str): string_param (Union[Unset, str]): integer_param (Union[Unset, int]): Default: 0. - path_param (str): header_param (Union[None, Unset, str]): cookie_param (Union[Unset, str]): @@ -114,9 +114,9 @@ async def asyncio_detailed( """Test different types of parameter references Args: + path_param (str): string_param (Union[Unset, str]): integer_param (Union[Unset, int]): Default: 0. - path_param (str): header_param (Union[None, Unset, str]): cookie_param (Union[Unset, str]): 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 61e248529..8203fa750 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 @@ -54,8 +54,8 @@ def sync_detailed( ) -> Response[Any]: """ Args: - param_query (Union[Unset, str]): param_path (str): + param_query (Union[Unset, str]): Raises: errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. @@ -85,8 +85,8 @@ async def asyncio_detailed( ) -> Response[Any]: """ Args: - param_query (Union[Unset, str]): param_path (str): + param_query (Union[Unset, str]): Raises: errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. 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 91a7b2c17..985e92c20 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 @@ -55,9 +55,9 @@ def sync_detailed( """Test that if you have an overriding property from `PathItem` in `Operation`, it produces valid code Args: + param_path (str): param_query (str): A parameter with the same name as another. Default: 'overridden_in_GET'. Example: an example string. - param_path (str): Raises: errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. @@ -88,9 +88,9 @@ async def asyncio_detailed( """Test that if you have an overriding property from `PathItem` in `Operation`, it produces valid code Args: + param_path (str): param_query (str): A parameter with the same name as another. Default: 'overridden_in_GET'. Example: an example string. - param_path (str): Raises: errors.UnexpectedStatus: If the server returns an undocumented status code and Client.raise_on_unexpected_status is True. 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 622181296..43f3b8993 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 @@ -68,8 +68,8 @@ def sync_detailed( ) -> Response[Any]: """ Args: - param_query (Union[Unset, str]): param_path (str): + param_query (Union[Unset, str]): param_header (Union[Unset, str]): param_cookie (Union[Unset, str]): @@ -105,8 +105,8 @@ async def asyncio_detailed( ) -> Response[Any]: """ Args: - param_query (Union[Unset, str]): param_path (str): + param_query (Union[Unset, str]): param_header (Union[Unset, str]): param_cookie (Union[Unset, str]): diff --git a/end_to_end_tests/test-3-1-golden-record/test_3_1_features_client/api/const/post_const_path.py b/end_to_end_tests/test-3-1-golden-record/test_3_1_features_client/api/const/post_const_path.py index 9539ed41f..3f864b3dc 100644 --- a/end_to_end_tests/test-3-1-golden-record/test_3_1_features_client/api/const/post_const_path.py +++ b/end_to_end_tests/test-3-1-golden-record/test_3_1_features_client/api/const/post_const_path.py @@ -74,9 +74,9 @@ def sync_detailed( ) -> Response[Literal["Why have a fixed response? I dunno"]]: """ Args: + path (Literal['this goes in the path']): required_query (Literal['this always goes in the query']): optional_query (Union[Literal['this sometimes goes in the query'], Unset]): - path (Literal['this goes in the path']): body (PostConstPathBody): Raises: @@ -111,9 +111,9 @@ def sync( ) -> Optional[Literal["Why have a fixed response? I dunno"]]: """ Args: + path (Literal['this goes in the path']): required_query (Literal['this always goes in the query']): optional_query (Union[Literal['this sometimes goes in the query'], Unset]): - path (Literal['this goes in the path']): body (PostConstPathBody): Raises: @@ -143,9 +143,9 @@ async def asyncio_detailed( ) -> Response[Literal["Why have a fixed response? I dunno"]]: """ Args: + path (Literal['this goes in the path']): required_query (Literal['this always goes in the query']): optional_query (Union[Literal['this sometimes goes in the query'], Unset]): - path (Literal['this goes in the path']): body (PostConstPathBody): Raises: @@ -178,9 +178,9 @@ async def asyncio( ) -> Optional[Literal["Why have a fixed response? I dunno"]]: """ Args: + path (Literal['this goes in the path']): required_query (Literal['this always goes in the query']): optional_query (Union[Literal['this sometimes goes in the query'], Unset]): - path (Literal['this goes in the path']): body (PostConstPathBody): Raises: diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py index e111a0f51..20a5e9376 100644 --- a/openapi_python_client/parser/openapi.py +++ b/openapi_python_client/parser/openapi.py @@ -464,8 +464,8 @@ def iter_all_parameters(self) -> Iterator[Tuple[oai.ParameterLocation, Property] def list_all_parameters(self) -> List[Property]: """Return a List of all the parameters of this endpoint""" return ( - self.query_parameters - + self.path_parameters + self.path_parameters + + self.query_parameters + self.header_parameters + self.cookie_parameters + [body.prop for body in self.bodies] From 8c133537eef027c10abe2cd8331396570fe17df4 Mon Sep 17 00:00:00 2001 From: Dylan Anthony Date: Wed, 21 Feb 2024 17:58:00 -0700 Subject: [PATCH 5/5] Mention change to `iter_all_parameters()` --- .../for_custom_templates_changed_type_of_endpoint_parameters.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.changeset/for_custom_templates_changed_type_of_endpoint_parameters.md b/.changeset/for_custom_templates_changed_type_of_endpoint_parameters.md index 907d439ec..e6fb93c75 100644 --- a/.changeset/for_custom_templates_changed_type_of_endpoint_parameters.md +++ b/.changeset/for_custom_templates_changed_type_of_endpoint_parameters.md @@ -14,3 +14,5 @@ The type of these properties on `Endpoint` has been changed from `Dict[str, Prop - `cookie_parameters` If your templates are very close to the default templates, you can probably just remove `.values()` anywhere it appears. + +The type of `iter_all_parameters()` is also different, you probably want `list_all_parameters()` instead.