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 all 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
12 changes: 12 additions & 0 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 numberDiff line numberDiff 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 DownExpand Up@@ -50,6 +51,8 @@ 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,
required_model_prop: ModelWithUnionProperty,
) -> Response[Union[None, HTTPValidationError]]:

json_datetime_prop: Union[Unset, str] = UNSET
Expand DownExpand Up@@ -89,6 +92,12 @@ 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()

json_required_model_prop = required_model_prop.to_dict()

params: Dict[str, Any] = {
"string_prop": string_prop,
"datetime_prop": json_datetime_prop,
Expand All@@ -101,6 +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):
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
28 changes: 28 additions & 0 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 numberDiff line numberDiff 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,8 @@ 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,
required_model_prop: ModelWithUnionProperty,
) -> Dict[str, Any]:
url = "{}/tests/defaults".format(client.base_url)

Expand DownExpand Up@@ -65,6 +68,12 @@ 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()

json_required_model_prop = required_model_prop.to_dict()

params: Dict[str, Any] = {
"string_prop": string_prop,
"datetime_prop": json_datetime_prop,
Expand All@@ -77,6 +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):
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 DownExpand Up@@ -122,6 +134,8 @@ 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,
required_model_prop: ModelWithUnionProperty,
) -> Response[Union[None, HTTPValidationError]]:
kwargs = _get_kwargs(
client=client,
Expand All@@ -135,6 +149,8 @@ def sync_detailed(
union_prop=union_prop,
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@@ -157,6 +173,8 @@ 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,
required_model_prop: ModelWithUnionProperty,
) -> Optional[Union[None, HTTPValidationError]]:
""" """

Expand All@@ -172,6 +190,8 @@ def sync(
union_prop=union_prop,
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@@ -188,6 +208,8 @@ 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,
required_model_prop: ModelWithUnionProperty,
) -> Response[Union[None, HTTPValidationError]]:
kwargs = _get_kwargs(
client=client,
Expand All@@ -201,6 +223,8 @@ async def asyncio_detailed(
union_prop=union_prop,
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@@ -222,6 +246,8 @@ 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,
required_model_prop: ModelWithUnionProperty,
) -> Optional[Union[None, HTTPValidationError]]:
""" """

Expand All@@ -238,5 +264,7 @@ async def asyncio(
union_prop=union_prop,
union_prop_with_ref=union_prop_with_ref,
enum_prop=enum_prop,
model_prop=model_prop,
required_model_prop=required_model_prop,
)
).parsed
16 changes: 16 additions & 0 deletions end_to_end_tests/openapi.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -396,6 +396,22 @@
},
"name": "enum_prop",
"in": "query"
},
{
"required": false,
"schema": {
"$ref": "#/components/schemas/ModelWithUnionProperty"
},
"name": "model_prop",
"in": "query"

@dbantydbantyFeb 10, 2021

Copy link
Copy Markdown
Collaborator

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.

@forest-benchlingforest-benchlingFeb 10, 2021

Copy link
Copy Markdown
CollaboratorAuthor

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.

@dbantydbantyFeb 10, 2021

Copy link
Copy Markdown
Collaborator

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/model_property.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -19,6 +19,7 @@ class ModelProperty(Property):
additional_properties: Union[bool, Property]

template: ClassVar[str] = "model_property.py.jinja"
json_is_dict: ClassVar[bool] = True

@dbantydbantyFeb 10, 2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should probably exist on the base Property as False right? I guess Jinja is handling it properly but I would expect an exception when checking if not property.json_is_dict for non- ModelProperty


def get_type_string(self, no_optional: bool = False) -> str:
""" Get a string representation of type that should be used when declaring this property """
Expand Down
1 change: 1 addition & 0 deletions openapi_python_client/parser/properties/property.py
View file
Open in desktop
Original file line numberDiff line numberDiff 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
17 changes: 15 additions & 2 deletions openapi_python_client/templates/endpoint_macros.py.jinja
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -22,13 +22,26 @@ if {{ parameter.python_name }} is not UNSET:
{% endfor %}
params: Dict[str, Any] = {
{% for property in endpoint.query_parameters %}
{% if property.template %}
{% if not property.json_is_dict %}
{% if property.template %}
"{{ property.name }}": {{ "json_" + property.python_name }},
{% else %}
{% else %}
"{{ property.name }}": {{ property.python_name }},
{% endif %}
{% endif %}
{% endfor %}
}
{% for property in endpoint.query_parameters %}
{% if property.json_is_dict %}
{% set property_name = "json_" + property.python_name %}
{% 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 %}:
params.update({{ property_name }})
{% endif %}
{% endif %}
{% endfor %}
params = {k: v for k, v in params.items() if v is not UNSET and v is not None}
{% endif %}
{% endmacro %}
Expand Down