Skip to content

Serialize model query params #316

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.

Sign up for GitHub

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
dbanty merged 6 commits into from
Feb 10, 2021
Merged

Serialize model query params #316

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f5846ce
Serialize model query params
forest-benchling Jan 27, 2021
b405fce
Merge main and reimplement
forest-benchling Feb 9, 2021
147fe01
Always check
forest-benchling Feb 9, 2021
8b27773
Add to base Property class
forest-benchling Feb 10, 2021
284a750
Revert change to if statement in endpoint_macros
forest-benchling Feb 10, 2021
8d6d644
Reuse variable
forest-benchling Feb 10, 2021
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
6 changes: 5 additions & 1 deletion end_to_end_tests/golden-record-custom/custom_e2e/api/tests/defaults_tests_defaults_post.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def httpx_request(
union_prop_with_ref: Union[Unset, float, AnEnum] = 0.6,
enum_prop: Union[Unset, AnEnum] = UNSET,
model_prop: Union[ModelWithUnionProperty, Unset] = UNSET,
required_model_prop: ModelWithUnionProperty,
) -> Response[Union[None, HTTPValidationError]]:

json_datetime_prop: Union[Unset, str] = UNSET
Expand Down Expand Up @@ -95,6 +96,8 @@ def httpx_request(
if not isinstance(model_prop, Unset):
json_model_prop = model_prop.to_dict()

json_required_model_prop = required_model_prop.to_dict()

params: Dict[str, Any] = {
"string_prop": string_prop,
"datetime_prop": json_datetime_prop,
Expand All @@ -107,8 +110,9 @@ def httpx_request(
"union_prop_with_ref": json_union_prop_with_ref,
"enum_prop": json_enum_prop,
}
if not isinstance(json_model_prop, Unset):
if not isinstance(json_model_prop, Unset) and json_model_prop is not None:
params.update(json_model_prop)
params.update(json_required_model_prop)
params = {k: v for k, v in params.items() if v is not UNSET and v is not None}

response = client.request(
Expand Down
14 changes: 13 additions & 1 deletion end_to_end_tests/golden-record/my_test_api_client/api/tests/defaults_tests_defaults_post.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def _get_kwargs(
union_prop_with_ref: Union[Unset, float, AnEnum] = 0.6,
enum_prop: Union[Unset, AnEnum] = UNSET,
model_prop: Union[ModelWithUnionProperty, Unset] = UNSET,
required_model_prop: ModelWithUnionProperty,
) -> Dict[str, Any]:
url = "{}/tests/defaults".format(client.base_url)

Expand Down Expand Up @@ -71,6 +72,8 @@ def _get_kwargs(
if not isinstance(model_prop, Unset):
json_model_prop = model_prop.to_dict()

json_required_model_prop = required_model_prop.to_dict()

params: Dict[str, Any] = {
"string_prop": string_prop,
"datetime_prop": json_datetime_prop,
Expand All @@ -83,8 +86,9 @@ def _get_kwargs(
"union_prop_with_ref": json_union_prop_with_ref,
"enum_prop": json_enum_prop,
}
if not isinstance(json_model_prop, Unset):
if not isinstance(json_model_prop, Unset) and json_model_prop is not None:
params.update(json_model_prop)
params.update(json_required_model_prop)
params = {k: v for k, v in params.items() if v is not UNSET and v is not None}

return {
Expand Down Expand Up @@ -131,6 +135,7 @@ def sync_detailed(
union_prop_with_ref: Union[Unset, float, AnEnum] = 0.6,
enum_prop: Union[Unset, AnEnum] = UNSET,
model_prop: Union[ModelWithUnionProperty, Unset] = UNSET,
required_model_prop: ModelWithUnionProperty,
) -> Response[Union[None, HTTPValidationError]]:
kwargs = _get_kwargs(
client=client,
Expand All @@ -145,6 +150,7 @@ def sync_detailed(
union_prop_with_ref=union_prop_with_ref,
enum_prop=enum_prop,
model_prop=model_prop,
required_model_prop=required_model_prop,
)

response = httpx.post(
Expand All @@ -168,6 +174,7 @@ def sync(
union_prop_with_ref: Union[Unset, float, AnEnum] = 0.6,
enum_prop: Union[Unset, AnEnum] = UNSET,
model_prop: Union[ModelWithUnionProperty, Unset] = UNSET,
required_model_prop: ModelWithUnionProperty,
) -> Optional[Union[None, HTTPValidationError]]:
""" """

Expand All @@ -184,6 +191,7 @@ def sync(
union_prop_with_ref=union_prop_with_ref,
enum_prop=enum_prop,
model_prop=model_prop,
required_model_prop=required_model_prop,
).parsed


Expand All @@ -201,6 +209,7 @@ async def asyncio_detailed(
union_prop_with_ref: Union[Unset, float, AnEnum] = 0.6,
enum_prop: Union[Unset, AnEnum] = UNSET,
model_prop: Union[ModelWithUnionProperty, Unset] = UNSET,
required_model_prop: ModelWithUnionProperty,
) -> Response[Union[None, HTTPValidationError]]:
kwargs = _get_kwargs(
client=client,
Expand All @@ -215,6 +224,7 @@ async def asyncio_detailed(
union_prop_with_ref=union_prop_with_ref,
enum_prop=enum_prop,
model_prop=model_prop,
required_model_prop=required_model_prop,
)

async with httpx.AsyncClient() as _client:
Expand All @@ -237,6 +247,7 @@ async def asyncio(
union_prop_with_ref: Union[Unset, float, AnEnum] = 0.6,
enum_prop: Union[Unset, AnEnum] = UNSET,
model_prop: Union[ModelWithUnionProperty, Unset] = UNSET,
required_model_prop: ModelWithUnionProperty,
) -> Optional[Union[None, HTTPValidationError]]:
""" """

Expand All @@ -254,5 +265,6 @@ async def asyncio(
union_prop_with_ref=union_prop_with_ref,
enum_prop=enum_prop,
model_prop=model_prop,
required_model_prop=required_model_prop,
)
).parsed
8 changes: 8 additions & 0 deletions end_to_end_tests/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,14 @@
},
"name": "model_prop",
"in": "query"
Copy link
Collaborator

@dbanty dbanty Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably also add nullable and not required and nullable examples as query params just to verify the template is working in all those cases using e2e tests.

Copy link
Collaborator Author

@forest-benchling forest-benchling Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbanty Is it OK if I add that test coverage in #332? Since it's not possible AFAIK to make a nullable model query parameter without making it a union type, and if I make it a union type then it depends on #332.

Copy link
Collaborator

@dbanty dbanty Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep that's fine.

},
{
"required": true,
"schema": {
"$ref": "#/components/schemas/ModelWithUnionProperty"
},
"name": "required_model_prop",
"in": "query"
}
],
"responses": {
Expand Down
1 change: 1 addition & 0 deletions openapi_python_client/parser/properties/property.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class Property:
python_name: str = attr.ib(init=False)

template: ClassVar[Optional[str]] = None
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)))
Expand Down
2 changes: 1 addition & 1 deletion openapi_python_client/templates/endpoint_macros.py.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ params: Dict[str, Any] = {
{% if property.required and not property.nullable %}
params.update({{ property_name }})
{% else %}
if {% if not property.required %}not isinstance({{ property_name }}, Unset){% endif %}{% if not property.required and property.nullable %} and {% endif %}{% if property.nullable %}{{ property_name }} is not None{% endif %}:
if not isinstance({{ property_name }}, Unset) and {{ property_name }} is not None:
Copy link
Collaborator

@dbanty dbanty Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was a mistake since now we get .update() twice in the generated code. Also mypy probably will get mad about non-overlapping type checks or whatever.

Copy link
Collaborator Author

@forest-benchling forest-benchling Feb 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, yeah, probably a mistake from my merge. I think the two .update()s you're referring to are actually updating different things, though.

params.update({{ "json_" + property.python_name }})
{% endif %}
{% endif %}
Expand Down