Skip to content

fix(parser): Attempt to deduplicate endpoint parameters based on name and location (fixes #305) #406

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 4, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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: Union[Unset, str] = UNSET,
param: 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()

params: Dict[str, Any] = {
"param": param,
}
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[None]:
return Response(
status_code=response.status_code,
content=response.content,
headers=response.headers,
parsed=None,
)


def sync_detailed(
*,
client: Client,
param_path: Union[Unset, str] = UNSET,
param: Union[Unset, str] = UNSET,
) -> Response[None]:
kwargs = _get_kwargs(
client=client,
param_path=param_path,
param=param,
)

response = httpx.get(
**kwargs,
)

return _build_response(response=response)


async def asyncio_detailed(
*,
client: Client,
param_path: Union[Unset, str] = UNSET,
param: Union[Unset, str] = UNSET,
) -> Response[None]:
kwargs = _get_kwargs(
client=client,
param_path=param_path,
param=param,
)

async with httpx.AsyncClient() as _client:
response = await _client.get(**kwargs)

return _build_response(response=response)
27 changes: 27 additions & 0 deletions end_to_end_tests/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,33 @@
"200": {"description": "Success"}
}
}
},
"/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": {
"tags": ["parameters"],
"parameters": [
{
"name": "param",
"in": "query",
"schema": {
"type": "string"
}
},
{
"name": "param",
"in": "path",
"schema": {
"type": "string"
}
}
],
"responses": {
"200": {
"description": ""
}
}
}
}
},
"components": {
Expand Down
11 changes: 11 additions & 0 deletions openapi_python_client/parser/openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ 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 = set()
if data.parameters is None:
return endpoint, schemas
for param in data.parameters:
Expand All @@ -232,6 +233,16 @@ 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:
prop.set_python_name(f"{prop.python_name}_{param.param_in}")

if prop.python_name in used_python_names:
return (
ParseError(detail=f"Encountered duplicate properties named {prop.python_name}", data=data),
schemas,
)
used_python_names.add(prop.python_name)
endpoint.relative_imports.update(prop.get_imports(prefix="..."))

if param.param_in == ParameterLocation.QUERY:
Expand Down
5 changes: 4 additions & 1 deletion openapi_python_client/parser/properties/property.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ class Property:
json_is_dict: ClassVar[bool] = False

def __attrs_post_init__(self) -> None:
object.__setattr__(self, "python_name", utils.to_valid_python_identifier(utils.snake_case(self.name)))
self.set_python_name(self.name)

def set_python_name(self, new_name: str) -> None:
object.__setattr__(self, "python_name", utils.to_valid_python_identifier(utils.snake_case(new_name)))

def get_base_type_string(self) -> str:
return self._type_string
Expand Down
41 changes: 41 additions & 0 deletions tests/test_parser/test_openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,47 @@ def test__add_parameters_happy(self, mocker):
assert endpoint.header_parameters == [header_prop]
assert schemas == schemas_3

def test__add_parameters_duplicate_properties(self, mocker):
from openapi_python_client.parser.openapi import Endpoint, Schemas

endpoint = self.make_endpoint()
parsed_schemas = mocker.MagicMock()
mocker.patch(
f"{MODULE_NAME}.property_from_data", return_value=(mocker.MagicMock(python_name="test"), parsed_schemas)
)
param = oai.Parameter.construct(name="test", required=True, param_schema=mocker.MagicMock(), param_in="path")
data = oai.Operation.construct(parameters=[param, param, param])
schemas = Schemas()
config = MagicMock()

result = Endpoint._add_parameters(endpoint=endpoint, data=data, schemas=schemas, config=config)
assert result == (ParseError(data=data, detail="Encountered duplicate properties named test"), parsed_schemas)

def test__add_parameters_duplicate_properties_different_location(self):
from openapi_python_client.parser.openapi import Endpoint, Schemas

endpoint = self.make_endpoint()
path_param = oai.Parameter.construct(
name="test", required=True, param_schema=oai.Schema.construct(type="string"), param_in="path"
)
query_param = oai.Parameter.construct(
name="test", required=True, param_schema=oai.Schema.construct(type="string"), param_in="query"
)
schemas = Schemas()
config = MagicMock()

result = Endpoint._add_parameters(
endpoint=endpoint,
data=oai.Operation.construct(parameters=[path_param, query_param]),
schemas=schemas,
config=config,
)[0]
assert isinstance(result, Endpoint)
assert result.path_parameters[0].python_name == "test"
assert result.path_parameters[0].name == "test"
assert result.query_parameters[0].python_name == "test_query"
assert result.query_parameters[0].name == "test"

def test_from_data_bad_params(self, mocker):
from openapi_python_client.parser.openapi import Endpoint

Expand Down