Skip to content

feat: Serialize model query parameters with style=form, explode=true BNCH-18023 #39

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
merged 2 commits into from
Jan 27, 2021
Merged

feat: Serialize model query parameters with style=form, explode=true BNCH-18023 #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7424688
Serialize model query params
forest-benchling Jan 27, 2021
5700bb1
Address comment
forest-benchling Jan 27, 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
28 changes: 18 additions & 10 deletions 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 @@ -13,6 +13,7 @@

from ...models.an_enum import AnEnum
from ...models.http_validation_error import HTTPValidationError
from ...models.model_with_union_property import ModelWithUnionProperty
from ...types import UNSET, Unset


Expand Down Expand Up @@ -50,6 +51,7 @@ def httpx_request(
union_prop: Union[Unset, float, str] = "not a float",
union_prop_with_ref: Union[Unset, float, AnEnum] = 0.6,
enum_prop: Union[Unset, AnEnum] = UNSET,
model_prop: Union[ModelWithUnionProperty, Unset] = UNSET,
) -> Response[Union[None, HTTPValidationError]]:

json_datetime_prop: Union[Unset, str] = UNSET
Expand Down Expand Up @@ -89,27 +91,33 @@ def httpx_request(
if not isinstance(enum_prop, Unset):
json_enum_prop = enum_prop

json_model_prop: Union[Unset, Dict[str, Any]] = UNSET
if not isinstance(model_prop, Unset):
json_model_prop = model_prop.to_dict()

params: Dict[str, Any] = {}
if string_prop is not UNSET:
if not isinstance(string_prop, Unset) and string_prop is not None:
params["string_prop"] = string_prop
if datetime_prop is not UNSET:
if not isinstance(json_datetime_prop, Unset) and json_datetime_prop is not None:
params["datetime_prop"] = json_datetime_prop
if date_prop is not UNSET:
if not isinstance(json_date_prop, Unset) and json_date_prop is not None:
params["date_prop"] = json_date_prop
if float_prop is not UNSET:
if not isinstance(float_prop, Unset) and float_prop is not None:
params["float_prop"] = float_prop
if int_prop is not UNSET:
if not isinstance(int_prop, Unset) and int_prop is not None:
params["int_prop"] = int_prop
if boolean_prop is not UNSET:
if not isinstance(boolean_prop, Unset) and boolean_prop is not None:
params["boolean_prop"] = boolean_prop
if list_prop is not UNSET:
if not isinstance(json_list_prop, Unset) and json_list_prop is not None:
params["list_prop"] = json_list_prop
if union_prop is not UNSET:
if not isinstance(json_union_prop, Unset) and json_union_prop is not None:
params["union_prop"] = json_union_prop
if union_prop_with_ref is not UNSET:
if not isinstance(json_union_prop_with_ref, Unset) and json_union_prop_with_ref is not None:
params["union_prop_with_ref"] = json_union_prop_with_ref
if enum_prop is not UNSET:
if not isinstance(json_enum_prop, Unset) and json_enum_prop is not None:
params["enum_prop"] = json_enum_prop
if not isinstance(json_model_prop, Unset) and json_model_prop is not None:
params.update(json_model_prop)

response = client.request(
"post",
Expand Down
2 changes: 1 addition & 1 deletion ...ts/golden-record-custom/custom_e2e/api/tests/optional_value_tests_optional_query_param.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def httpx_request(
json_query_param = query_param

params: Dict[str, Any] = {}
if query_param is not UNSET:
if not isinstance(json_query_param, Unset) and json_query_param is not None:
params["query_param"] = json_query_param

response = client.request(
Expand Down
36 changes: 26 additions & 10 deletions 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 @@ -7,6 +7,7 @@
from ...client import Client
from ...models.an_enum import AnEnum
from ...models.http_validation_error import HTTPValidationError
from ...models.model_with_union_property import ModelWithUnionProperty
from ...types import UNSET, Response, Unset


Expand All @@ -23,6 +24,7 @@ def _get_kwargs(
union_prop: Union[Unset, float, str] = "not a float",
union_prop_with_ref: Union[Unset, float, AnEnum] = 0.6,
enum_prop: Union[Unset, AnEnum] = UNSET,
model_prop: Union[ModelWithUnionProperty, Unset] = UNSET,
) -> Dict[str, Any]:
url = "{}/tests/defaults".format(client.base_url)

Expand Down Expand Up @@ -65,27 +67,33 @@ def _get_kwargs(
if not isinstance(enum_prop, Unset):
json_enum_prop = enum_prop

json_model_prop: Union[Unset, Dict[str, Any]] = UNSET
if not isinstance(model_prop, Unset):
json_model_prop = model_prop.to_dict()

params: Dict[str, Any] = {}
if string_prop is not UNSET:
if not isinstance(string_prop, Unset) and string_prop is not None:
params["string_prop"] = string_prop
if datetime_prop is not UNSET:
if not isinstance(json_datetime_prop, Unset) and json_datetime_prop is not None:
params["datetime_prop"] = json_datetime_prop
if date_prop is not UNSET:
if not isinstance(json_date_prop, Unset) and json_date_prop is not None:
params["date_prop"] = json_date_prop
if float_prop is not UNSET:
if not isinstance(float_prop, Unset) and float_prop is not None:
params["float_prop"] = float_prop
if int_prop is not UNSET:
if not isinstance(int_prop, Unset) and int_prop is not None:
params["int_prop"] = int_prop
if boolean_prop is not UNSET:
if not isinstance(boolean_prop, Unset) and boolean_prop is not None:
params["boolean_prop"] = boolean_prop
if list_prop is not UNSET:
if not isinstance(json_list_prop, Unset) and json_list_prop is not None:
params["list_prop"] = json_list_prop
if union_prop is not UNSET:
if not isinstance(json_union_prop, Unset) and json_union_prop is not None:
params["union_prop"] = json_union_prop
if union_prop_with_ref is not UNSET:
if not isinstance(json_union_prop_with_ref, Unset) and json_union_prop_with_ref is not None:
params["union_prop_with_ref"] = json_union_prop_with_ref
if enum_prop is not UNSET:
if not isinstance(json_enum_prop, Unset) and json_enum_prop is not None:
params["enum_prop"] = json_enum_prop
if not isinstance(json_model_prop, Unset) and json_model_prop is not None:
params.update(json_model_prop)

return {
"url": url,
Expand Down Expand Up @@ -130,6 +138,7 @@ def sync_detailed(
union_prop: Union[Unset, float, str] = "not a float",
union_prop_with_ref: Union[Unset, float, AnEnum] = 0.6,
enum_prop: Union[Unset, AnEnum] = UNSET,
model_prop: Union[ModelWithUnionProperty, Unset] = UNSET,
) -> Response[Union[None, HTTPValidationError]]:
kwargs = _get_kwargs(
client=client,
Expand All @@ -143,6 +152,7 @@ def sync_detailed(
union_prop=union_prop,
union_prop_with_ref=union_prop_with_ref,
enum_prop=enum_prop,
model_prop=model_prop,
)

response = httpx.post(
Expand All @@ -165,6 +175,7 @@ def sync(
union_prop: Union[Unset, float, str] = "not a float",
union_prop_with_ref: Union[Unset, float, AnEnum] = 0.6,
enum_prop: Union[Unset, AnEnum] = UNSET,
model_prop: Union[ModelWithUnionProperty, Unset] = UNSET,
) -> Optional[Union[None, HTTPValidationError]]:
""" """

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


Expand All @@ -196,6 +208,7 @@ async def asyncio_detailed(
union_prop: Union[Unset, float, str] = "not a float",
union_prop_with_ref: Union[Unset, float, AnEnum] = 0.6,
enum_prop: Union[Unset, AnEnum] = UNSET,
model_prop: Union[ModelWithUnionProperty, Unset] = UNSET,
) -> Response[Union[None, HTTPValidationError]]:
kwargs = _get_kwargs(
client=client,
Expand All @@ -209,6 +222,7 @@ async def asyncio_detailed(
union_prop=union_prop,
union_prop_with_ref=union_prop_with_ref,
enum_prop=enum_prop,
model_prop=model_prop,
)

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

Expand All @@ -246,5 +261,6 @@ async def asyncio(
union_prop=union_prop,
union_prop_with_ref=union_prop_with_ref,
enum_prop=enum_prop,
model_prop=model_prop,
)
).parsed
2 changes: 1 addition & 1 deletion ...s/golden-record/my_test_api_client/api/tests/optional_value_tests_optional_query_param.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def _get_kwargs(
json_query_param = query_param

params: Dict[str, Any] = {}
if query_param is not UNSET:
if not isinstance(json_query_param, Unset) and json_query_param is not None:
params["query_param"] = json_query_param

return {
Expand Down
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 @@ -396,6 +396,14 @@
},
"name": "enum_prop",
"in": "query"
},
{
"required": false,
"schema": {
"$ref": "#/components/schemas/ModelWithUnionProperty"
},
"name": "model_prop",
"in": "query"
}
],
"responses": {
Expand Down
1 change: 1 addition & 0 deletions openapi_python_client/parser/properties/model_property.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class ModelProperty(Property):
additional_properties: Union[bool, Property]

template: ClassVar[str] = "model_property.pyi"
json_is_dict: ClassVar[bool] = True
Copy link

@packyg packyg Jan 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

You should add this classvar to the parent class to be consistent:

json_is_dict: ClassVar[bool] = False

It's not erroring now because Jinja is permissive, but if you were to do property.json_is_dict outside of a template, it would error for non-model properties.


def resolve_references(
self, components: Dict[str, Union[oai.Reference, oai.Schema]], schemas: Schemas
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
9 changes: 5 additions & 4 deletions openapi_python_client/templates/endpoint_macros.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@ params: Dict[str, Any] = {
}
{% for property in endpoint.query_parameters %}
{% if not property.required %}
if {{ property.python_name }} is not UNSET:
{% if property.template %}
params["{{ property.name }}"] = {{ "json_" + property.python_name }}
{% set property_name = "json_" + property.python_name if property.template else property.python_name %}
if {% if not property.required %}not isinstance({{ property_name }}, Unset) and {% endif %}{{ property_name }} is not None:
{% if property.json_is_dict %}
params.update({{ property_name }})
Copy link

Choose a reason for hiding this comment

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

Does this do the prefixing as expected?

I think I would expect something like

Suggested change
params.update({{ property_name }})
params.update({
f"{property.name}.{p}": v, for p, v in {{ property_name }}.items()
})

in order for it to build the params with e.g. schemaField.Plasmid=seq_XYZ instead of just Plasmid=seq_XYZ

Copy link
Author

Choose a reason for hiding this comment

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

It does not handle the prefixing. According to the OpenAPI spec, the parameter name itself does not show up at all in the serialization.

See https://swagger.io/specification/ under "Style Examples"

{% else %}
params["{{ property.name }}"] = {{ property.python_name }}
params["{{ property.name }}"] = {{ property_name }}
{% endif %}
{% endif %}
{% endfor %}
Expand Down