diff --git a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py index e5f9a66ed..f516aefcf 100644 --- a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py +++ b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/__init__.py @@ -2,10 +2,10 @@ from typing import Type -from my_test_api_client.api.default import DefaultEndpoints -from my_test_api_client.api.parameters import ParametersEndpoints -from my_test_api_client.api.tag1 import Tag1Endpoints -from my_test_api_client.api.tests import TestsEndpoints +from .default import DefaultEndpoints +from .parameters import ParametersEndpoints +from .tag1 import Tag1Endpoints +from .tests import TestsEndpoints class MyTestApiClientApi: diff --git a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/default/__init__.py b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/default/__init__.py index 4d0eb4fb5..a4580103f 100644 --- a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/default/__init__.py +++ b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/default/__init__.py @@ -2,7 +2,7 @@ import types -from my_test_api_client.api.default import get_common_parameters, post_common_parameters +from . import get_common_parameters, post_common_parameters class DefaultEndpoints: 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 b92c6d96b..0d34e1f2d 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 @@ -2,10 +2,22 @@ import types -from my_test_api_client.api.parameters import get_same_name_multiple_locations_param +from . import ( + delete_common_parameters_overriding_param, + get_common_parameters_overriding_param, + get_same_name_multiple_locations_param, +) class ParametersEndpoints: + @classmethod + def get_common_parameters_overriding_param(cls) -> types.ModuleType: + return get_common_parameters_overriding_param + + @classmethod + def delete_common_parameters_overriding_param(cls) -> types.ModuleType: + return delete_common_parameters_overriding_param + @classmethod def get_same_name_multiple_locations_param(cls) -> types.ModuleType: return get_same_name_multiple_locations_param diff --git a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/tag1/__init__.py b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/tag1/__init__.py index b91db35b0..556ca84e8 100644 --- a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/tag1/__init__.py +++ b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/tag1/__init__.py @@ -2,7 +2,7 @@ import types -from my_test_api_client.api.tag1 import get_tag_with_number +from . import get_tag_with_number class Tag1Endpoints: diff --git a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/tests/__init__.py b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/tests/__init__.py index 28a4f10c1..8fc547a66 100644 --- a/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/tests/__init__.py +++ b/end_to_end_tests/custom-templates-golden-record/my_test_api_client/api/tests/__init__.py @@ -2,7 +2,7 @@ import types -from my_test_api_client.api.tests import ( +from . import ( defaults_tests_defaults_post, get_basic_list_of_booleans, get_basic_list_of_floats, 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 new file mode 100644 index 000000000..91ea6fd5f --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/api/parameters/delete_common_parameters_overriding_param.py @@ -0,0 +1,77 @@ +from typing import Any, Dict, Union + +import httpx + +from ...client import Client +from ...types import UNSET, Response, Unset + + +def _get_kwargs( + *, + client: Client, + param_path: str, + param_query: Union[Unset, str] = UNSET, +) -> Dict[str, Any]: + url = "{}/common_parameters_overriding/{param}".format(client.base_url, param=param_path) + + headers: Dict[str, Any] = client.get_headers() + cookies: Dict[str, Any] = client.get_cookies() + + params: Dict[str, Any] = { + "param": param_query, + } + params = {k: v for k, v in params.items() if v is not UNSET and v is not None} + + return { + "url": url, + "headers": headers, + "cookies": cookies, + "timeout": client.get_timeout(), + "params": params, + } + + +def _build_response(*, response: httpx.Response) -> Response[Any]: + return Response( + status_code=response.status_code, + content=response.content, + headers=response.headers, + parsed=None, + ) + + +def sync_detailed( + *, + client: Client, + param_path: str, + param_query: Union[Unset, str] = UNSET, +) -> Response[Any]: + kwargs = _get_kwargs( + client=client, + param_path=param_path, + param_query=param_query, + ) + + response = httpx.delete( + **kwargs, + ) + + return _build_response(response=response) + + +async def asyncio_detailed( + *, + client: Client, + param_path: str, + param_query: Union[Unset, str] = UNSET, +) -> Response[Any]: + kwargs = _get_kwargs( + client=client, + param_path=param_path, + param_query=param_query, + ) + + async with httpx.AsyncClient() as _client: + response = await _client.delete(**kwargs) + + return _build_response(response=response) 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 new file mode 100644 index 000000000..2fb194af7 --- /dev/null +++ b/end_to_end_tests/golden-record/my_test_api_client/api/parameters/get_common_parameters_overriding_param.py @@ -0,0 +1,77 @@ +from typing import Any, Dict + +import httpx + +from ...client import Client +from ...types import UNSET, Response + + +def _get_kwargs( + *, + 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) + + headers: Dict[str, Any] = client.get_headers() + cookies: Dict[str, Any] = client.get_cookies() + + params: Dict[str, Any] = { + "param": param_query, + } + params = {k: v for k, v in params.items() if v is not UNSET and v is not None} + + return { + "url": url, + "headers": headers, + "cookies": cookies, + "timeout": client.get_timeout(), + "params": params, + } + + +def _build_response(*, response: httpx.Response) -> Response[Any]: + return Response( + status_code=response.status_code, + content=response.content, + headers=response.headers, + parsed=None, + ) + + +def sync_detailed( + *, + client: Client, + param_path: str, + param_query: str = "overriden_in_GET", +) -> Response[Any]: + kwargs = _get_kwargs( + client=client, + param_path=param_path, + param_query=param_query, + ) + + response = httpx.get( + **kwargs, + ) + + return _build_response(response=response) + + +async def asyncio_detailed( + *, + client: Client, + param_path: str, + param_query: str = "overriden_in_GET", +) -> Response[Any]: + kwargs = _get_kwargs( + client=client, + param_path=param_path, + param_query=param_query, + ) + + async with httpx.AsyncClient() as _client: + response = await _client.get(**kwargs) + + return _build_response(response=response) 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 2013768aa..3f01bc592 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 @@ -9,14 +9,22 @@ def _get_kwargs( *, client: Client, - param_path: Union[Unset, str] = UNSET, + param_path: str, param_query: Union[Unset, str] = UNSET, + param_header: Union[Unset, str] = UNSET, + param_cookie: Union[Unset, str] = UNSET, ) -> Dict[str, Any]: url = "{}/same-name-multiple-locations/{param}".format(client.base_url, param=param_path) headers: Dict[str, Any] = client.get_headers() cookies: Dict[str, Any] = client.get_cookies() + if param_header is not UNSET: + headers["param"] = param_header + + if param_cookie is not UNSET: + cookies["param"] = param_cookie + params: Dict[str, Any] = { "param": param_query, } @@ -43,13 +51,17 @@ def _build_response(*, response: httpx.Response) -> Response[Any]: def sync_detailed( *, client: Client, - param_path: Union[Unset, str] = UNSET, + param_path: str, param_query: Union[Unset, 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, param_query=param_query, + param_header=param_header, + param_cookie=param_cookie, ) response = httpx.get( @@ -62,13 +74,17 @@ def sync_detailed( async def asyncio_detailed( *, client: Client, - param_path: Union[Unset, str] = UNSET, + param_path: str, param_query: Union[Unset, 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, param_query=param_query, + param_header=param_header, + param_cookie=param_cookie, ) async with httpx.AsyncClient() as _client: diff --git a/end_to_end_tests/openapi.json b/end_to_end_tests/openapi.json index ab92c345a..093752212 100644 --- a/end_to_end_tests/openapi.json +++ b/end_to_end_tests/openapi.json @@ -193,7 +193,7 @@ "tags": [ "tests" ], - "sumnary": "Post from data", + "summary": "Post from data", "description": "Post form data", "operationId": "post_form_data", "requestBody": { @@ -850,6 +850,95 @@ } } }, + "/common_parameters_overriding/{param}": { + "description": "Test that if you have an overriding property from `PathItem` in `Operation`, it produces valid code", + "get": { + "tags": [ + "parameters" + ], + "parameters": [ + { + "name": "param", + "in": "query", + "required": true, + "schema": { + "type": "string", + "default": "overriden_in_GET" + } + } + ], + "responses": { + "200": { + "description": "" + } + } + }, + "delete": { + "tags": [ + "parameters" + ], + "responses": { + "200": { + "description": "" + } + } + }, + "parameters": [ + { + "name": "param", + "in": "query", + "schema": { + "type": "string" + } + }, + { + "name": "param", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + } + ] + }, + "/same_identifier_conflict/{param}/{param_path}": { + "description": "Test that if you have properties with the same Python identifier, it not produces code", + "get": { + "tags": [ + "parameters" + ], + "parameters": [ + { + "name": "param_path", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "param", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "param", + "in": "header", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "" + } + } + } + }, "/same-name-multiple-locations/{param}": { "description": "Test that if you have a property of the same name in multiple locations, it produces valid code", "get": { @@ -864,9 +953,24 @@ "type": "string" } }, + { + "name": "param", + "in": "header", + "schema": { + "type": "string" + } + }, + { + "name": "param", + "in": "cookie", + "schema": { + "type": "string" + } + }, { "name": "param", "in": "path", + "required": true, "schema": { "type": "string" } @@ -881,7 +985,7 @@ }, "/tag_with_number": { "get": { - "tags": [1], + "tags": ["1"], "responses": { "200": { "description": "Success" @@ -1387,7 +1491,7 @@ "type": "string" }, "type_enum": { - "type": "int", + "type": "integer", "enum": [0, 1] } } @@ -1404,7 +1508,7 @@ "enum": ["submodel"] }, "type_enum": { - "type": "int", + "type": "integer", "enum": [0] } } diff --git a/end_to_end_tests/test_custom_templates/api_init.py.jinja b/end_to_end_tests/test_custom_templates/api_init.py.jinja index 03c2a2f6f..5f6a23523 100644 --- a/end_to_end_tests/test_custom_templates/api_init.py.jinja +++ b/end_to_end_tests/test_custom_templates/api_init.py.jinja @@ -2,7 +2,7 @@ from typing import Type {% for tag in endpoint_collections_by_tag.keys() %} -from {{ package_name }}.api.{{ tag }} import {{ utils.pascal_case(tag) }}Endpoints +from .{{ tag }} import {{ utils.pascal_case(tag) }}Endpoints {% endfor %} class {{ utils.pascal_case(package_name) }}Api: diff --git a/end_to_end_tests/test_custom_templates/endpoint_init.py.jinja b/end_to_end_tests/test_custom_templates/endpoint_init.py.jinja index 57e8ba124..ca9851679 100644 --- a/end_to_end_tests/test_custom_templates/endpoint_init.py.jinja +++ b/end_to_end_tests/test_custom_templates/endpoint_init.py.jinja @@ -2,7 +2,7 @@ import types {% for endpoint in endpoint_collection.endpoints %} -from {{ package_name }}.api.{{ endpoint_collection.tag }} import {{ utils.snake_case(endpoint.name) }} +from . import {{ utils.snake_case(endpoint.name) }} {% endfor %} class {{ utils.pascal_case(endpoint_collection.tag) }}Endpoints: diff --git a/openapi_python_client/parser/openapi.py b/openapi_python_client/parser/openapi.py index e22b43a7c..3d143224a 100644 --- a/openapi_python_client/parser/openapi.py +++ b/openapi_python_client/parser/openapi.py @@ -1,4 +1,3 @@ -import itertools from copy import deepcopy from dataclasses import dataclass, field from typing import Any, Dict, Iterator, List, Optional, Set, Tuple, Union @@ -9,6 +8,7 @@ from .. import schema as oai from .. import utils from ..config import Config +from ..utils import PythonIdentifier from .errors import GeneratorError, ParseError, PropertyError from .properties import Class, EnumProperty, ModelProperty, Property, Schemas, build_schemas, property_from_data from .responses import Response, response_from_data @@ -46,10 +46,12 @@ def from_data( endpoint, schemas = Endpoint.from_data( data=operation, path=path, method=method, tag=tag, schemas=schemas, config=config ) + # Add `PathItem` parameters if not isinstance(endpoint, ParseError): 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." @@ -88,15 +90,16 @@ class Endpoint: tag: str summary: Optional[str] = "" relative_imports: Set[str] = field(default_factory=set) - 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) + query_parameters: Dict[str, Property] = field(default_factory=dict) + path_parameters: Dict[str, Property] = field(default_factory=dict) + 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) form_body_class: Optional[Class] = None json_body: Optional[Property] = None multipart_body: Optional[Property] = None errors: List[ParseError] = field(default_factory=list) + used_python_identifiers: Set[PythonIdentifier] = field(default_factory=set) @staticmethod def parse_request_form_body(*, body: oai.RequestBody, config: Config) -> Optional[Class]: @@ -241,12 +244,31 @@ def _add_parameters( *, endpoint: "Endpoint", data: Union[oai.Operation, oai.PathItem], schemas: Schemas, config: Config ) -> Tuple[Union["Endpoint", ParseError], Schemas]: endpoint = deepcopy(endpoint) - used_python_names: Dict[str, Tuple[Property, oai.ParameterLocation]] = {} if data.parameters is None: return endpoint, schemas + + unique_parameters: Set[Tuple[str, oai.ParameterLocation]] = set() + parameters_by_location = { + oai.ParameterLocation.QUERY: endpoint.query_parameters, + oai.ParameterLocation.PATH: endpoint.path_parameters, + oai.ParameterLocation.HEADER: endpoint.header_parameters, + oai.ParameterLocation.COOKIE: endpoint.cookie_parameters, + } + for param in data.parameters: if isinstance(param, oai.Reference) or param.param_schema is None: continue + + unique_param = (param.name, param.param_in) + if unique_param in unique_parameters: + duplication_detail = ( + "Parameters MUST NOT contain duplicates. " + "A unique parameter is defined by a combination of a name and location. " + f"Duplicated parameters named `{param.name}` detected in `{param.param_in}`." + ) + return ParseError(data=data, detail=duplication_detail), schemas + unique_parameters.add(unique_param) + prop, schemas = property_from_data( name=param.name, required=param.required, @@ -257,38 +279,39 @@ def _add_parameters( ) if isinstance(prop, ParseError): return ParseError(detail=f"cannot parse parameter of endpoint {endpoint.name}", data=prop.data), schemas - - if prop.python_name in used_python_names: - duplicate, duplicate_location = used_python_names[prop.python_name] - if duplicate.python_name == prop.python_name: # Existing should be converted too for consistency - duplicate.set_python_name(f"{duplicate.python_name}_{duplicate_location}", config=config) - prop.set_python_name(f"{prop.python_name}_{param.param_in}", config=config) - else: - used_python_names[prop.python_name] = (prop, param.param_in) + 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 + existing_prop: Property = parameters_dict[prop.name] + # Existing should be converted too for consistency + endpoint.used_python_identifiers.remove(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: + return ( + ParseError( + detail=f"Parameters with same Python identifier `{existing_prop.python_name}` detected", + data=data, + ), + schemas, + ) + endpoint.used_python_identifiers.add(existing_prop.python_name) + prop.set_python_name(new_name=f"{param.name}_{param.param_in}", config=config) endpoint.relative_imports.update(prop.get_imports(prefix="...")) - if param.param_in == oai.ParameterLocation.QUERY: - endpoint.query_parameters.append(prop) - elif param.param_in == oai.ParameterLocation.PATH: - endpoint.path_parameters.append(prop) - elif param.param_in == oai.ParameterLocation.HEADER: - endpoint.header_parameters.append(prop) - elif param.param_in == oai.ParameterLocation.COOKIE: - endpoint.cookie_parameters.append(prop) - else: - return ParseError(data=param, detail="Parameter must be declared in path or query"), schemas - - name_check = set() - for prop in itertools.chain( - endpoint.query_parameters, endpoint.path_parameters, endpoint.header_parameters, endpoint.cookie_parameters - ): - if prop.python_name in name_check: + if prop.python_name in endpoint.used_python_identifiers: return ( - ParseError(data=data, detail=f"Could not reconcile duplicate parameters named {prop.python_name}"), + ParseError( + detail=f"Parameters with same Python identifier `{prop.python_name}` detected", data=data + ), schemas, ) - name_check.add(prop.python_name) + endpoint.used_python_identifiers.add(prop.python_name) + parameters_by_location[param.param_in][prop.name] = prop return endpoint, schemas diff --git a/openapi_python_client/templates/endpoint_macros.py.jinja b/openapi_python_client/templates/endpoint_macros.py.jinja index add4c68b2..3d5365bb0 100644 --- a/openapi_python_client/templates/endpoint_macros.py.jinja +++ b/openapi_python_client/templates/endpoint_macros.py.jinja @@ -1,11 +1,11 @@ {% macro header_params(endpoint) %} {% if endpoint.header_parameters %} - {% for parameter in endpoint.header_parameters %} + {% for parameter in endpoint.header_parameters.values() %} {% if parameter.required %} -headers["{{ parameter.python_name | kebabcase}}"] = {{ parameter.python_name }} +headers["{{ parameter.name | kebabcase}}"] = {{ parameter.python_name }} {% else %} if {{ parameter.python_name }} is not UNSET: - headers["{{ parameter.python_name | kebabcase}}"] = {{ parameter.python_name }} + headers["{{ parameter.name | kebabcase}}"] = {{ parameter.python_name }} {% endif %} {% endfor %} {% endif %} @@ -13,7 +13,7 @@ if {{ parameter.python_name }} is not UNSET: {% macro cookie_params(endpoint) %} {% if endpoint.cookie_parameters %} - {% for parameter in endpoint.cookie_parameters %} + {% for parameter in endpoint.cookie_parameters.values() %} {% if parameter.required %} cookies["{{ parameter.name}}"] = {{ parameter.python_name }} {% else %} @@ -27,7 +27,7 @@ if {{ parameter.python_name }} is not UNSET: {% macro query_params(endpoint) %} {% if endpoint.query_parameters %} - {% for property in endpoint.query_parameters %} + {% for property in endpoint.query_parameters.values() %} {% set destination = "json_" + property.python_name %} {% if property.template %} {% from "property_templates/" + property.template import transform %} @@ -35,7 +35,7 @@ if {{ parameter.python_name }} is not UNSET: {% endif %} {% endfor %} params: Dict[str, Any] = { - {% for property in endpoint.query_parameters %} + {% for property in endpoint.query_parameters.values() %} {% if not property.json_is_dict %} {% if property.template %} "{{ property.name }}": {{ "json_" + property.python_name }}, @@ -45,7 +45,7 @@ params: Dict[str, Any] = { {% endif %} {% endfor %} } - {% for property in endpoint.query_parameters %} + {% for property in endpoint.query_parameters.values() %} {% if property.json_is_dict %} {% set property_name = "json_" + property.python_name %} {% if property.required and not property.nullable %} @@ -92,7 +92,7 @@ client: AuthenticatedClient, client: Client, {% endif %} {# path parameters #} -{% for parameter in endpoint.path_parameters %} +{% for parameter in endpoint.path_parameters.values() %} {{ parameter.to_string() }}, {% endfor %} {# Form data if any #} @@ -108,14 +108,14 @@ multipart_data: {{ endpoint.multipart_body.get_type_string() }}, json_body: {{ endpoint.json_body.get_type_string() }}, {% endif %} {# query parameters #} -{% for parameter in endpoint.query_parameters %} +{% for parameter in endpoint.query_parameters.values() %} {{ parameter.to_string() }}, {% endfor %} -{% for parameter in endpoint.header_parameters %} +{% for parameter in endpoint.header_parameters.values() %} {{ parameter.to_string() }}, {% endfor %} {# cookie parameters #} -{% for parameter in endpoint.cookie_parameters %} +{% for parameter in endpoint.cookie_parameters.values() %} {{ parameter.to_string() }}, {% endfor %} {% endmacro %} @@ -123,7 +123,7 @@ 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 %} +{% for parameter in endpoint.path_parameters.values() %} {{ parameter.python_name }}={{ parameter.python_name }}, {% endfor %} {% if endpoint.form_body_class %} @@ -135,13 +135,13 @@ multipart_data=multipart_data, {% if endpoint.json_body %} json_body=json_body, {% endif %} -{% for parameter in endpoint.query_parameters %} +{% for parameter in endpoint.query_parameters.values() %} {{ parameter.python_name }}={{ parameter.python_name }}, {% endfor %} -{% for parameter in endpoint.header_parameters %} +{% for parameter in endpoint.header_parameters.values() %} {{ parameter.python_name }}={{ parameter.python_name }}, {% endfor %} -{% for parameter in endpoint.cookie_parameters %} +{% for parameter in endpoint.cookie_parameters.values() %} {{ 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 0624394f0..d347c0510 100644 --- a/openapi_python_client/templates/endpoint_module.py.jinja +++ b/openapi_python_client/templates/endpoint_module.py.jinja @@ -19,7 +19,7 @@ def _get_kwargs( ) -> Dict[str, Any]: url = "{}{{ endpoint.path }}".format( client.base_url - {%- for parameter in endpoint.path_parameters -%} + {%- for parameter in endpoint.path_parameters.values() -%} ,{{parameter.name}}={{parameter.python_name}} {%- endfor -%} ) diff --git a/tests/test_parser/test_openapi.py b/tests/test_parser/test_openapi.py index ec2ee1fde..c3b1fe289 100644 --- a/tests/test_parser/test_openapi.py +++ b/tests/test_parser/test_openapi.py @@ -1,5 +1,6 @@ from unittest.mock import MagicMock +import pydantic import pytest import openapi_python_client.schema as oai @@ -527,57 +528,173 @@ def test__add_parameters_parse_error(self, mocker): property_schemas, ) - def test__add_parameters_fail_loudly_when_location_not_supported(self, mocker): - from openapi_python_client.parser.openapi import Endpoint, Schemas - - endpoint = self.make_endpoint() + 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)) - param = oai.Parameter.construct( - name="test", required=True, param_schema=mocker.MagicMock(), param_in="error_location" + 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): + """Checks when the PythonIdentifier of new parameter already used.""" + from openapi_python_client.parser.openapi import Endpoint + from openapi_python_client.parser.properties import Property + + endpoint = self.make_endpoint() + + path_prop_conflicted = Property( + name="prop_name_path", required=False, nullable=False, default=None, python_name="prop_name_path" ) - schemas = Schemas() + query_prop = Property(name="prop_name", required=False, nullable=False, default=None, python_name="prop_name") + path_prop = Property(name="prop_name", required=False, nullable=False, default=None, python_name="prop_name") + + schemas_1 = mocker.MagicMock() + schemas_2 = mocker.MagicMock() + schemas_3 = mocker.MagicMock() + property_from_data = 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.construct( + parameters=[ + oai.Parameter.construct( + name=path_prop_conflicted.name, required=True, param_schema=path_conflicted_schema, param_in="path" + ), + oai.Parameter.construct( + name=query_prop.name, required=False, param_schema=query_schema, param_in="query" + ), + oai.Parameter.construct(name=path_prop.name, required=True, param_schema=path_schema, param_in="path"), + oai.Reference.construct(), # Should be ignored + oai.Parameter.construct(), # Should be ignored + ] + ) + initial_schemas = mocker.MagicMock() config = MagicMock() - result = Endpoint._add_parameters( - endpoint=endpoint, data=oai.Operation.construct(parameters=[param]), schemas=schemas, config=config + 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" + + def test__add_parameters_with_location_postfix_conflict2(self, mocker): + """Checks when an existing parameter has a conflicting PythonIdentifier after renaming.""" + from openapi_python_client.parser.openapi import Endpoint + from openapi_python_client.parser.properties import Property + + endpoint = self.make_endpoint() + path_prop_conflicted = Property( + name="prop_name_path", required=False, nullable=False, default=None, python_name="prop_name_path" + ) + path_prop = Property(name="prop_name", required=False, nullable=False, default=None, python_name="prop_name") + query_prop = Property(name="prop_name", required=False, nullable=False, default=None, python_name="prop_name") + schemas_1 = mocker.MagicMock() + schemas_2 = mocker.MagicMock() + schemas_3 = mocker.MagicMock() + property_from_data = 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.construct( + parameters=[ + oai.Parameter.construct( + name=path_prop_conflicted.name, required=True, param_schema=path_conflicted_schema, param_in="path" + ), + oai.Parameter.construct(name=path_prop.name, required=True, param_schema=path_schema, param_in="path"), + oai.Parameter.construct( + name=query_prop.name, required=False, param_schema=query_schema, param_in="query" + ), + oai.Reference.construct(), # Should be ignored + oai.Parameter.construct(), # Should be ignored + ] ) - assert result == (ParseError(data=param, detail="Parameter must be declared in path or query"), parsed_schemas) + initial_schemas = mocker.MagicMock() + config = MagicMock() + + 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" def test__add_parameters_happy(self, mocker): from openapi_python_client.parser.openapi import Endpoint from openapi_python_client.parser.properties import Property endpoint = self.make_endpoint() + path_prop_name = "path_prop_name" path_prop = mocker.MagicMock(autospec=Property) path_prop_import = mocker.MagicMock() path_prop.get_imports = mocker.MagicMock(return_value={path_prop_import}) + path_prop.python_name = "path_prop_name" + + query_prop_name = "query_prop_name" query_prop = mocker.MagicMock(autospec=Property) query_prop_import = mocker.MagicMock() query_prop.get_imports = mocker.MagicMock(return_value={query_prop_import}) - header_prop = mocker.MagicMock(autospec=Property) - header_prop_import = mocker.MagicMock() - header_prop.get_imports = mocker.MagicMock(return_value={header_prop_import}) + query_prop.python_name = "query_prop_name" + + header_prop_name = "header_prop_name" + header_prop_operation = mocker.MagicMock(autospec=Property) + header_prop_operation.name = header_prop_name + header_prop_operation.required = False + header_prop_operation_import = mocker.MagicMock() + header_prop_operation.get_imports = mocker.MagicMock(return_value={header_prop_operation_import}) + header_prop_operation.python_name = "header_prop_name" + + header_prop_path = mocker.MagicMock(autospec=Property) + header_prop_path.name = header_prop_name + header_prop_path.required = True + header_prop_path_import = mocker.MagicMock() + header_prop_path.get_imports = mocker.MagicMock(return_value={header_prop_path_import}) + header_prop_path.python_name = "header_prop_name" + + cookie_prop_name = "cookie_prop_name" + cookie_prop = mocker.MagicMock(autospec=Property) + cookie_prop_import = mocker.MagicMock() + cookie_prop.get_imports = mocker.MagicMock(return_value={cookie_prop_import}) + cookie_prop.python_name = "cookie_prop_name" + schemas_1 = mocker.MagicMock() schemas_2 = mocker.MagicMock() schemas_3 = mocker.MagicMock() + schemas_4 = mocker.MagicMock() + schemas_5 = mocker.MagicMock() property_from_data = mocker.patch( f"{MODULE_NAME}.property_from_data", - side_effect=[(path_prop, schemas_1), (query_prop, schemas_2), (header_prop, schemas_3)], + side_effect=[ + (path_prop, schemas_1), + (query_prop, schemas_2), + (header_prop_operation, schemas_3), + (header_prop_path, schemas_4), + (cookie_prop, schemas_5), + ], ) path_schema = mocker.MagicMock() query_schema = mocker.MagicMock() - header_schema = mocker.MagicMock() - data = oai.Operation.construct( + header_operation_schema = mocker.MagicMock() + cookie_schema = mocker.MagicMock() + header_pathitem_schema = mocker.MagicMock() + + operation_data = oai.Operation.construct( parameters=[ + oai.Parameter.construct(name=path_prop_name, required=True, param_schema=path_schema, param_in="path"), oai.Parameter.construct( - name="path_prop_name", required=True, param_schema=path_schema, param_in="path" - ), - oai.Parameter.construct( - name="query_prop_name", required=False, param_schema=query_schema, param_in="query" + name=query_prop_name, required=False, param_schema=query_schema, param_in="query" ), oai.Parameter.construct( - name="header_prop_name", required=False, param_schema=header_schema, param_in="header" + name=header_prop_name, required=False, param_schema=header_operation_schema, param_in="header" ), oai.Reference.construct(), # Should be ignored oai.Parameter.construct(), # Should be ignored @@ -587,13 +704,28 @@ def test__add_parameters_happy(self, mocker): config = MagicMock() (endpoint, schemas) = Endpoint._add_parameters( - endpoint=endpoint, data=data, schemas=initial_schemas, config=config + endpoint=endpoint, data=operation_data, schemas=initial_schemas, config=config + ) + path_item_data = oai.PathItem.construct( + parameters=[ + oai.Parameter.construct( + name=header_prop_name, required=True, param_schema=header_pathitem_schema, param_in="header" + ), + oai.Parameter.construct( + name=cookie_prop_name, required=False, param_schema=cookie_schema, param_in="cookie" + ), + oai.Reference.construct(), # Should be ignored + oai.Parameter.construct(), # Should be ignored + ] + ) + (endpoint, schemas) = Endpoint._add_parameters( + endpoint=endpoint, data=path_item_data, schemas=schemas, config=config ) property_from_data.assert_has_calls( [ mocker.call( - name="path_prop_name", + name=path_prop_name, required=True, data=path_schema, schemas=initial_schemas, @@ -601,7 +733,7 @@ def test__add_parameters_happy(self, mocker): config=config, ), mocker.call( - name="query_prop_name", + name=query_prop_name, required=False, data=query_schema, schemas=schemas_1, @@ -609,23 +741,48 @@ def test__add_parameters_happy(self, mocker): config=config, ), mocker.call( - name="header_prop_name", + name=header_prop_name, required=False, - data=header_schema, + data=header_operation_schema, schemas=schemas_2, parent_name="name", config=config, ), + mocker.call( + name=header_prop_name, + required=True, + data=header_pathitem_schema, + schemas=schemas_3, + parent_name="name", + config=config, + ), + mocker.call( + name=cookie_prop_name, + required=False, + data=cookie_schema, + schemas=schemas_4, + parent_name="name", + config=config, + ), ] ) path_prop.get_imports.assert_called_once_with(prefix="...") query_prop.get_imports.assert_called_once_with(prefix="...") - header_prop.get_imports.assert_called_once_with(prefix="...") - assert endpoint.relative_imports == {"import_3", path_prop_import, query_prop_import, header_prop_import} - assert endpoint.path_parameters == [path_prop] - assert endpoint.query_parameters == [query_prop] - assert endpoint.header_parameters == [header_prop] - assert schemas == schemas_3 + header_prop_operation.get_imports.assert_called_once_with(prefix="...") + cookie_prop.get_imports.assert_called_once_with(prefix="...") + assert endpoint.relative_imports == { + "import_3", + path_prop_import, + query_prop_import, + header_prop_operation_import, + cookie_prop_import, + } + assert endpoint.path_parameters == {path_prop.name: path_prop} + assert endpoint.query_parameters == {query_prop.name: query_prop} + assert endpoint.header_parameters == {header_prop_operation.name: header_prop_operation} + assert endpoint.header_parameters[header_prop_operation.name].required is False + assert endpoint.cookie_parameters == {cookie_prop.name: cookie_prop} + assert schemas == schemas_5 def test__add_parameters_duplicate_properties(self, mocker): from openapi_python_client.parser.openapi import Endpoint, Schemas @@ -640,7 +797,12 @@ def test__add_parameters_duplicate_properties(self, mocker): result = Endpoint._add_parameters(endpoint=endpoint, data=data, schemas=schemas, config=config) assert result == ( - ParseError(data=data, detail="Could not reconcile duplicate parameters named test_path"), + ParseError( + data=data, + detail="Parameters MUST NOT contain duplicates. " + "A unique parameter is defined by a combination of a name and location. " + "Duplicated parameters named `test` detected in `path`.", + ), schemas, ) @@ -664,10 +826,8 @@ def test__add_parameters_duplicate_properties_different_location(self): config=config, )[0] assert isinstance(result, Endpoint) - assert result.path_parameters[0].python_name == "test_path" - assert result.path_parameters[0].name == "test" - assert result.query_parameters[0].python_name == "test_query" - assert result.query_parameters[0].name == "test" + assert result.path_parameters["test"].name == "test" + assert result.query_parameters["test"].name == "test" def test_from_data_bad_params(self, mocker): from openapi_python_client.parser.openapi import Endpoint