Serialize model query params #316
Conversation
Codecov Report
@@ Coverage Diff @@
## main #316 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 47 47
Lines 1390 1395 +5
=========================================
+ Hits 1390 1395 +5
Continue to review full report at Codecov.
|
|
@dbanty fixed merge conflicts |
| @@ -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 | |||
There was a problem hiding this comment.
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
| "$ref": "#/components/schemas/ModelWithUnionProperty" | ||
| }, | ||
| "name": "model_prop", | ||
| "in": "query" |
There was a problem hiding this comment.
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.
| 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: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Oops, yeah, probably a mistake from my merge. I think the two .update()s you're referring to are actually updating different things, though.
Fixes #295.
This PR serializes model query params as if
style=form, explode=true. Although we don't supportstyleandexplode, we currently already act as ifstyle=form, explode=true. In other words, in today's client, if you pass in a list query parameter, it will already be serialized aslist=val&list=val2&list=val3. This is just the behavior ofhttpx.Therefore, I think this PR, which will cause a dict to be serialized as
key1=val1&key2=val2&key3=val3, would be consistent, as a holdover until we get to the point where we fully support allstyle/explodecombinations.