Conversation
A query parameter whose schema is an object and whose style/explode are left at their defaults — style: form, explode: true — must go on the wire as one parameter per entry, keyed by the property name alone. The python client JSON-encoded the whole object into a single parameter instead. python/api.mustache appended the object whole, and it was JSON encoded by ApiClient.parameters_to_url_query, which sees no style or explode. Exploded maps are now appended entry by entry. parameters_to_url_query's dict branch is left alone: it is shared by every call site and is still the right fallback for a parameter that is not exploded. python/api_client.mustache quoted values and never quoted names. That was harmless while every name came from baseName in the document, but an exploded object takes its names from the object, so the names became runtime data for the first time and a name carrying & or = would break the URL. Names are now quoted too.
|
Split per language as requested, out of #24797. This is the python half; the siblings are:
The three touch disjoint sets of files under Each branch was tested on its own after the split, not just as part of the original combined |
There was a problem hiding this comment.
All reported issues were addressed across 17 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…entry
An exploded object query parameter takes its names from the object, so a
property name can collide with the name of a sibling array parameter that
declares a collection format. parameters_to_url_query looked the name up in
collection_formats without checking that the value was a collection, so the
scalar was iterated character by character.
In the petstore fixture, testQueryParameterCollectionFormat declares
context: multi alongside the exploded map language. Calling it with
language={"context": "en"} produced context=e&context=n, and a csv sibling
turned https://x.test into h,t,t,p,s,%3A,/,/,x,.,t,e,s,t.
The collection format now applies only when the value really is a list or a
tuple. Every genuine array parameter is unaffected — multi, csv, ssv and pipes
all serialize byte for byte as before.
Reported by cubic on OpenAPITools#24802.
|
Thanks cubic — one of these was a real regression and is now fixed in 82c90b6. P2, collection format collision — valid, fixed. This was introduced by this PR and I had
The fix gates the branch on the value actually being a collection: if k in collection_formats and isinstance(v, (list, tuple)):Checked all four collection formats against real array parameters — Worth noting your suggested framing — serialize exploded entries separately from the P1, declared object models — valid, but pre-existing and deliberately out of scope. The |
There was a problem hiding this comment.
3 issues found across 10 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/python/PythonClientCodegenTest.java">
<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/python/PythonClientCodegenTest.java:766">
P3: The collision scenario this comment describes (an exploded object property named `context` alongside a `context: multi` array parameter) is not present in the fixture, so the assertion only checks that the guard line exists in the shared api_client.mustache output. That line is generated for every Python client regardless of the fixture, so the test would pass even if the collision behavior regressed. Add a colliding parameter/property pair to exploded-object-query-param.yaml and assert the generated wire behavior (e.g., that the exploded entry is not split by the sibling array's collection format) to actually exercise the fix.</violation>
</file>
<file name="samples/client/others/python-legacy-model-dictionaries/legacy_model_dict_client/api_client.py">
<violation number="1" location="samples/client/others/python-legacy-model-dictionaries/legacy_model_dict_client/api_client.py:653">
P2: The guard and name-quoting fixes were applied only to the `python` template; the `python-pydantic-v1` template variant still generates `if k in collection_formats:` without the `isinstance(v, (list, tuple))` guard and without quoting parameter names. Pydantic-v1 generated clients therefore still have the collection-format collision bug (and unquoted names) for exploded object query params. The test only exercises the `python` generator, so the pydantic-v1 gap is not covered. Mirror the change in python-pydantic-v1/api_client.mustache and regenerate the pydantic-v1 samples.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/python/api_client.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/python/api_client.mustache:696">
P3: The isinstance check only prevents the collision for scalar values. If an exploded object property value is itself a list/tuple and its name collides with a sibling array parameter, the collection format is still applied, so the property is still joined or repeated as if it were the sibling's list. For example, `filter: {context: ['en', 'fr']}` alongside a `context` array parameter with `multi` format still emits `context=en&context=fr` from the object entry, which is exactly the behavior the comment says must not happen. Distinguishing declared array parameter names from runtime object property names (e.g., only applying collection formats to keys that are declared query parameters) would close the gap; the value-type check alone cannot.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| # collection. An exploded object query parameter takes its names from the | ||
| # object, so a property name that happens to match a sibling array parameter | ||
| # must not be joined or repeated as if it were that parameter's list. | ||
| if k in collection_formats and isinstance(v, (list, tuple)): |
There was a problem hiding this comment.
P2: The guard and name-quoting fixes were applied only to the python template; the python-pydantic-v1 template variant still generates if k in collection_formats: without the isinstance(v, (list, tuple)) guard and without quoting parameter names. Pydantic-v1 generated clients therefore still have the collection-format collision bug (and unquoted names) for exploded object query params. The test only exercises the python generator, so the pydantic-v1 gap is not covered. Mirror the change in python-pydantic-v1/api_client.mustache and regenerate the pydantic-v1 samples.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/python-legacy-model-dictionaries/legacy_model_dict_client/api_client.py, line 653:
<comment>The guard and name-quoting fixes were applied only to the `python` template; the `python-pydantic-v1` template variant still generates `if k in collection_formats:` without the `isinstance(v, (list, tuple))` guard and without quoting parameter names. Pydantic-v1 generated clients therefore still have the collection-format collision bug (and unquoted names) for exploded object query params. The test only exercises the `python` generator, so the pydantic-v1 gap is not covered. Mirror the change in python-pydantic-v1/api_client.mustache and regenerate the pydantic-v1 samples.</comment>
<file context>
@@ -646,7 +646,11 @@ def parameters_to_url_query(self, params, collection_formats):
+ # collection. An exploded object query parameter takes its names from the
+ # object, so a property name that happens to match a sibling array parameter
+ # must not be joined or repeated as if it were that parameter's list.
+ if k in collection_formats and isinstance(v, (list, tuple)):
collection_format = collection_formats[k]
if collection_format == 'multi':
</file context>
There was a problem hiding this comment.
Not needed there — python-pydantic-v1 never got the behavior these fixes protect. This PR
touches only the python templates; the pydantic-v1 api.mustache has no explode branch,
so an object query parameter still goes out on the single-parameter path and no runtime
property name ever reaches its parameters_to_url_query. The unguarded if k in collection_formats: there only ever sees the declared parameter names it always saw, and
the quoting fix protects runtime data that template never produces. Extending the explode
behavior to pydantic-v1 would be its own change with its own samples churn; happy to file
it separately if maintainers want the template variants to converge.
| // to a value that actually is a collection, or "context": "en" alongside a | ||
| // context: multi array parameter would go on the wire as context=e&context=n. | ||
| Path apiClient = Paths.get(output.getAbsolutePath(), "openapi_client", "api_client.py"); | ||
| TestUtils.assertFileContains(apiClient, |
There was a problem hiding this comment.
P3: The collision scenario this comment describes (an exploded object property named context alongside a context: multi array parameter) is not present in the fixture, so the assertion only checks that the guard line exists in the shared api_client.mustache output. That line is generated for every Python client regardless of the fixture, so the test would pass even if the collision behavior regressed. Add a colliding parameter/property pair to exploded-object-query-param.yaml and assert the generated wire behavior (e.g., that the exploded entry is not split by the sibling array's collection format) to actually exercise the fix.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/test/java/org/openapitools/codegen/python/PythonClientCodegenTest.java, line 766:
<comment>The collision scenario this comment describes (an exploded object property named `context` alongside a `context: multi` array parameter) is not present in the fixture, so the assertion only checks that the guard line exists in the shared api_client.mustache output. That line is generated for every Python client regardless of the fixture, so the test would pass even if the collision behavior regressed. Add a colliding parameter/property pair to exploded-object-query-param.yaml and assert the generated wire behavior (e.g., that the exploded entry is not split by the sibling array's collection format) to actually exercise the fix.</comment>
<file context>
@@ -757,6 +757,14 @@ public void testExplodedObjectQueryParameter() throws IOException {
+ // to a value that actually is a collection, or "context": "en" alongside a
+ // context: multi array parameter would go on the wire as context=e&context=n.
+ Path apiClient = Paths.get(output.getAbsolutePath(), "openapi_client", "api_client.py");
+ TestUtils.assertFileContains(apiClient,
+ "if k in collection_formats and isinstance(v, (list, tuple)):");
}
</file context>
There was a problem hiding this comment.
Fair — the fixture asserted the guard's text, not its behavior. Added two runtime tests in
c3929d4, in the hand-maintained petstore sample tests (tests/test_api_client.py, which CI
executes): a scalar entry named like a declared multi array parameter goes out as itself
(language=nl&context=abc), and the declared array parameter itself still gets its
collection format (context=a&context=b). Both run against the generated
parameters_to_url_query, so the collision path is now exercised, not just present.
| # collection. An exploded object query parameter takes its names from the | ||
| # object, so a property name that happens to match a sibling array parameter | ||
| # must not be joined or repeated as if it were that parameter's list. | ||
| if k in collection_formats and isinstance(v, (list, tuple)): |
There was a problem hiding this comment.
P3: The isinstance check only prevents the collision for scalar values. If an exploded object property value is itself a list/tuple and its name collides with a sibling array parameter, the collection format is still applied, so the property is still joined or repeated as if it were the sibling's list. For example, filter: {context: ['en', 'fr']} alongside a context array parameter with multi format still emits context=en&context=fr from the object entry, which is exactly the behavior the comment says must not happen. Distinguishing declared array parameter names from runtime object property names (e.g., only applying collection formats to keys that are declared query parameters) would close the gap; the value-type check alone cannot.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/python/api_client.mustache, line 696:
<comment>The isinstance check only prevents the collision for scalar values. If an exploded object property value is itself a list/tuple and its name collides with a sibling array parameter, the collection format is still applied, so the property is still joined or repeated as if it were the sibling's list. For example, `filter: {context: ['en', 'fr']}` alongside a `context` array parameter with `multi` format still emits `context=en&context=fr` from the object entry, which is exactly the behavior the comment says must not happen. Distinguishing declared array parameter names from runtime object property names (e.g., only applying collection formats to keys that are declared query parameters) would close the gap; the value-type check alone cannot.</comment>
<file context>
@@ -689,7 +689,11 @@ https://github.com/OpenAPITools/openapi-generator/blob/c84b949df1a9ec04ba75989cb
+ # collection. An exploded object query parameter takes its names from the
+ # object, so a property name that happens to match a sibling array parameter
+ # must not be joined or repeated as if it were that parameter's list.
+ if k in collection_formats and isinstance(v, (list, tuple)):
collection_format = collection_formats[k]
if collection_format == 'multi':
</file context>
There was a problem hiding this comment.
Accurate as a mechanism, and left as is deliberately. A list-valued property of an
exploded object has no defined serialization in OpenAPI at all (form/explode is specified
for objects with scalar-ish members; nested collections are undefined), so when such a name
also collides with a declared array parameter there is no "right answer" to restore — the
guard covers the case the spec does define, a scalar property shadowing an array parameter.
Distinguishing the two sources for nested lists would mean tagging exploded entries through
the whole _query_params pipeline, which is a larger restructure than this fix warrants.
The new runtime tests in c3929d4 pin the two defined behaviors either side of the guard.
There was a problem hiding this comment.
Follow-up: 0a261c6 partly supersedes this. An exploded entry holding a list is now expanded into one scalar entry per item in the generated api (photoUrls=a&photoUrls=b, as the other generators do), so it no longer reaches parameters_to_url_query as a list and can't pick up a sibling's collection format. That covers the filter: {context: ['en', 'fr']} example: it now goes out as context=en&context=fr because that's what form explode produces for it, not because of context's multi.
Two runtime tests on parameters_to_url_query in the hand-maintained petstore sample tests: a scalar entry named like a declared array parameter goes out as itself, and the declared array parameter still gets its collection format. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Go3AyndcGwv5tFTwo9aBfy
An object with declared properties becomes a model rather than a dict, so it was flagged isModel, missed the isMap branch and was still json encoded as a single parameter. With form style and explode a model is now serialized with sanitize_for_serialization first, so its entries carry the names the properties have on the wire and unset properties are left out, and then exploded like a map. A oneOf or anyOf model holding a primitive does not serialize to a dict and stays a single parameter; deepObject and explode: false are unchanged. An exploded entry holding a list now repeats its name per item (photoUrls=a&photoUrls=b), as java, go and typescript-axios do, instead of going out as the str() of the list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/echo_api/python/tests/test_manual.py">
<violation number="1" location="samples/client/echo_api/python/tests/test_manual.py:103">
P3: These new tests never exercise the name-quoting fix. Every property name on the wire (`id`, `name`, `photoUrls`, `status`, `outcomes`, `text`, `date`) is made of unreserved URL characters, so `quote(str(k))` in parameters_to_url_query always passes a name through unchanged. Since the PR's motivation for quoting names is that exploded object parameters take their names from runtime object properties (unlike baseNames from the spec), that code path is left untested here. Add a case with a property name containing a reserved character (e.g. `&`, `=`, or `:`) and assert its encoded form, mirroring the `createdDate%3Agte` scenario from the PR description.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| pet = openapi_client.Pet(id=12345, name="Hello World", photoUrls=["http://a.com", "http://b.com"], status="available") | ||
| api_response = api_instance.test_query_style_form_explode_true_object(pet) | ||
| e = EchoServerResponseParser(api_response) | ||
| self.assertEqual(e.path, "/query/style_form/explode_true/object?id=12345&name=Hello%20World&photoUrls=http%3A//a.com&photoUrls=http%3A//b.com&status=available") |
There was a problem hiding this comment.
P3: These new tests never exercise the name-quoting fix. Every property name on the wire (id, name, photoUrls, status, outcomes, text, date) is made of unreserved URL characters, so quote(str(k)) in parameters_to_url_query always passes a name through unchanged. Since the PR's motivation for quoting names is that exploded object parameters take their names from runtime object properties (unlike baseNames from the spec), that code path is left untested here. Add a case with a property name containing a reserved character (e.g. &, =, or :) and assert its encoded form, mirroring the createdDate%3Agte scenario from the PR description.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/echo_api/python/tests/test_manual.py, line 103:
<comment>These new tests never exercise the name-quoting fix. Every property name on the wire (`id`, `name`, `photoUrls`, `status`, `outcomes`, `text`, `date`) is made of unreserved URL characters, so `quote(str(k))` in parameters_to_url_query always passes a name through unchanged. Since the PR's motivation for quoting names is that exploded object parameters take their names from runtime object properties (unlike baseNames from the spec), that code path is left untested here. Add a case with a property name containing a reserved character (e.g. `&`, `=`, or `:`) and assert its encoded form, mirroring the `createdDate%3Agte` scenario from the PR description.</comment>
<file context>
@@ -92,6 +92,24 @@ def test_query_style_form_explode_false_array_string_test(self):
+ pet = openapi_client.Pet(id=12345, name="Hello World", photoUrls=["http://a.com", "http://b.com"], status="available")
+ api_response = api_instance.test_query_style_form_explode_true_object(pet)
+ e = EchoServerResponseParser(api_response)
+ self.assertEqual(e.path, "/query/style_form/explode_true/object?id=12345&name=Hello%20World&photoUrls=http%3A//a.com&photoUrls=http%3A//b.com&status=available")
+
+ def test_query_style_form_explode_true_object_all_of_test(self):
</file context>
There was a problem hiding this comment.
Agreed. The runtime asserts in test_manual.py only have reserved characters in values (http%3A//a.com, 03%3A04%3A05); every property name they use is already URL-safe, so the quoting of names was covered only as generated text.
An echo_api case does not work for this, because the form/explode endpoints there take $ref models (Pet, DataQuery) and there is no free-form object parameter to hang a reserved name on. So in dc592a9 I added the case to samples/openapi3/client/petstore/python/tests/test_api_client.py instead, next to the test_parameters_to_url_query_* tests this PR already added there:
params = self.api_client.parameters_to_url_query(
params=[('createdDate:gte', '2023-01-01'), ('a&b', 'c')],
collection_formats={})
self.assertEqual(params, "createdDate%3Agte=2023-01-01&a%26b=c")That file is hand-maintained and not listed in .openapi-generator/FILES, so it survives regeneration. I also confirmed the same string end to end on a generated client: a free-form object parameter carrying those two names goes out as ?createdDate%3Agte=2023-01-01&a%26b=c.
A null map value went on the wire as k=None and a null list item as l=None, because parameters_to_url_query quotes str(None). Both exploded branches now skip a null value and a null item, as kotlin, dart and go do. The model branch also appends the serialized value rather than the model. For a oneOf or anyOf holding a primitive that changes nothing on the wire, since param_serialize runs sanitize_for_serialization over every query parameter before parameters_to_url_query, but one holding a list now repeats the parameter name per item instead of sending the list's repr. Adds a runtime test for the quoting of exploded property names, which only the generated text covered until now, and one for a model whose optional properties are left unset. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/main/resources/python/api.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/python/api.mustache:363">
P3: Null handling is now inconsistent across query-parameter serialization paths. This change makes exploded object/model entries skip null list items so `str(None)` never reaches the wire, but genuine array parameters still pass their list straight to `parameters_to_url_query`, whose `multi`/csv branches do `quote(str(value))` on every item — a `None` item there still serializes as the literal string `"None"`. The test comment added here states the same rationale ("quotes str(None) and puts the literal 'None' on the wire"), so the guard arguably belongs at the `parameters_to_url_query` level (or the generic array branch) too, not only in the exploded blocks.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| if _value is None: | ||
| continue | ||
| if isinstance(_value, (list, tuple)): | ||
| _query_params.extend((_key, _item) for _item in _value if _item is not None) |
There was a problem hiding this comment.
P3: Null handling is now inconsistent across query-parameter serialization paths. This change makes exploded object/model entries skip null list items so str(None) never reaches the wire, but genuine array parameters still pass their list straight to parameters_to_url_query, whose multi/csv branches do quote(str(value)) on every item — a None item there still serializes as the literal string "None". The test comment added here states the same rationale ("quotes str(None) and puts the literal 'None' on the wire"), so the guard arguably belongs at the parameters_to_url_query level (or the generic array branch) too, not only in the exploded blocks.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/resources/python/api.mustache, line 363:
<comment>Null handling is now inconsistent across query-parameter serialization paths. This change makes exploded object/model entries skip null list items so `str(None)` never reaches the wire, but genuine array parameters still pass their list straight to `parameters_to_url_query`, whose `multi`/csv branches do `quote(str(value))` on every item — a `None` item there still serializes as the literal string `"None"`. The test comment added here states the same rationale ("quotes str(None) and puts the literal 'None' on the wire"), so the guard arguably belongs at the `parameters_to_url_query` level (or the generic array branch) too, not only in the exploded blocks.</comment>
<file context>
@@ -355,9 +355,12 @@ https://github.com/OpenAPITools/openapi-generator/blob/c84b949df1a9ec04ba75989cb
+ continue
if isinstance(_value, (list, tuple)):
- _query_params.extend((_key, _item) for _item in _value)
+ _query_params.extend((_key, _item) for _item in _value if _item is not None)
else:
_query_params.append((_key, _value))
</file context>
There was a problem hiding this comment.
True, and it is not new: a declared array parameter goes through the collection-format branch of parameters_to_url_query untouched on master as well. Checked with a client generated from this branch: parameters_to_url_query([("ids", ["a", None, "b"])], {"ids": "multi"}) gives ids=a&ids=None&ids=b (csv: ids=a,None,b), and master's multi and csv branches pass each item through the same way.
This PR only guards the entries it introduces, the exploded object and model branches, because before it those went out as one JSON blob and there was nothing on the wire to be None. Skipping None in declared arrays changes behaviour for every array parameter in every python client, so it belongs in its own change rather than in this one.
The map and model branches repeated the same loop in every generated operation. ApiClient.explode_query_object now serializes the value and returns one pair per entry, so each branch is a single call, and the helper gets direct unit tests. Comments shrink to one line, the shared fixture's description names the three combinations it covers, and the test titles say what they check. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 21 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="samples/client/echo_api/python-disallowAdditionalPropertiesIfNotPresent/openapi_client/api_client.py">
<violation number="1" location="samples/client/echo_api/python-disallowAdditionalPropertiesIfNotPresent/openapi_client/api_client.py:528">
P3: `explode_query_object` returns object-property values that are themselves dicts (nested object or list-of-objects property) as `(k, dict)` entries; `parameters_to_url_query` then JSON-encodes those through `if isinstance(v, dict): v = json.dumps(v)`. That contradicts this PR's goal that form/explode object entries go on the wire as their own key=value pair and also bypasses the new per-name quoting path, so such a property ends up as one JSON parameter. The PR's known-gaps section only lists deepObject and explode:false, so this case is not documented as a limitation. Either recurse into nested dicts here (or note the behavior explicitly), and add a test for an object-typed property in the exploded-object fixture.</violation>
</file>
<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/python/PythonClientCodegenTest.java">
<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/python/PythonClientCodegenTest.java:759">
P3: The refactor dropped the only assertions pinning the None-handling behavior: the old test checked the generated API contained `if _value is None:` and `if _item is not None`, while the new tests only check that `explode_query_object` is called and exists, so removing the helper's `if item is not None` filter would not fail any test here. The echo runtime tests don't cover it either (they use Pet models, which omit unset props during serialization before the helper sees them). Assert the None filter in the generated helper body.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| obj = self.sanitize_for_serialization(obj) | ||
| if not isinstance(obj, dict): | ||
| obj = {name: obj} | ||
| return [(k, item) for k, v in obj.items() for item in (v if isinstance(v, (list, tuple)) else [v]) if item is not None] |
There was a problem hiding this comment.
P3: explode_query_object returns object-property values that are themselves dicts (nested object or list-of-objects property) as (k, dict) entries; parameters_to_url_query then JSON-encodes those through if isinstance(v, dict): v = json.dumps(v). That contradicts this PR's goal that form/explode object entries go on the wire as their own key=value pair and also bypasses the new per-name quoting path, so such a property ends up as one JSON parameter. The PR's known-gaps section only lists deepObject and explode:false, so this case is not documented as a limitation. Either recurse into nested dicts here (or note the behavior explicitly), and add a test for an object-typed property in the exploded-object fixture.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/echo_api/python-disallowAdditionalPropertiesIfNotPresent/openapi_client/api_client.py, line 528:
<comment>`explode_query_object` returns object-property values that are themselves dicts (nested object or list-of-objects property) as `(k, dict)` entries; `parameters_to_url_query` then JSON-encodes those through `if isinstance(v, dict): v = json.dumps(v)`. That contradicts this PR's goal that form/explode object entries go on the wire as their own key=value pair and also bypasses the new per-name quoting path, so such a property ends up as one JSON parameter. The PR's known-gaps section only lists deepObject and explode:false, so this case is not documented as a limitation. Either recurse into nested dicts here (or note the behavior explicitly), and add a test for an object-typed property in the exploded-object fixture.</comment>
<file context>
@@ -520,6 +520,13 @@ def parameters_to_tuples(self, params, collection_formats):
+ obj = self.sanitize_for_serialization(obj)
+ if not isinstance(obj, dict):
+ obj = {name: obj}
+ return [(k, item) for k, v in obj.items() for item in (v if isinstance(v, (list, tuple)) else [v]) if item is not None]
+
def parameters_to_url_query(self, params, collection_formats):
</file context>
There was a problem hiding this comment.
Intended. OpenAPI does not define how form/explode serializes a nested object, so a nested object or list of objects stays one JSON-encoded parameter under the entry's name, and its name is still quoted. The description lists it under Known gaps.
| "def explode_query_object(self, name, obj):", | ||
| "if k in collection_formats and isinstance(v, (list, tuple)):"); |
There was a problem hiding this comment.
P3: The refactor dropped the only assertions pinning the None-handling behavior: the old test checked the generated API contained if _value is None: and if _item is not None, while the new tests only check that explode_query_object is called and exists, so removing the helper's if item is not None filter would not fail any test here. The echo runtime tests don't cover it either (they use Pet models, which omit unset props during serialization before the helper sees them). Assert the None filter in the generated helper body.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/test/java/org/openapitools/codegen/python/PythonClientCodegenTest.java, line 759:
<comment>The refactor dropped the only assertions pinning the None-handling behavior: the old test checked the generated API contained `if _value is None:` and `if _item is not None`, while the new tests only check that `explode_query_object` is called and exists, so removing the helper's `if item is not None` filter would not fail any test here. The echo runtime tests don't cover it either (they use Pet models, which omit unset props during serialization before the helper sees them). Assert the None filter in the generated helper body.</comment>
<file context>
@@ -740,37 +740,23 @@ public void testExplodedObjectQueryParameter() throws IOException {
+ // a collection format applies only to a list, so an exploded "context": "en" next to a context: multi array stays context=en
Path apiClient = Paths.get(output.getAbsolutePath(), "openapi_client", "api_client.py");
TestUtils.assertFileContains(apiClient,
+ "def explode_query_object(self, name, obj):",
"if k in collection_formats and isinstance(v, (list, tuple)):");
}
</file context>
| "def explode_query_object(self, name, obj):", | |
| "if k in collection_formats and isinstance(v, (list, tuple)):"); | |
| TestUtils.assertFileContains(apiClient, | |
| "def explode_query_object(self, name, obj):", | |
| "if item is not None", | |
| "if k in collection_formats and isinstance(v, (list, tuple)):"); |
There was a problem hiding this comment.
The None skip is pinned at runtime rather than as template text: test_explode_query_object and test_explode_query_object_not_a_dict in samples/openapi3/client/petstore/python/tests/test_api_client.py (8ed9c17) cover a None entry and a None list item being left out, and a list repeating the key. Checked by dropping if item is not None from the helper: both fail.
…-query-parameters-python Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
They landed on master after this branch was cut, so they did not carry the exploded query object changes yet. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Bug
A query parameter whose schema is an object, with
style/explodeleft at their defaults (form,true), must go on the wire as one parameter per entry, keyed by the property name alone. Thepythonclient JSON encoded the whole object into one parameter instead, both for a free-form object or map and for an object with declared properties (a model).called with
{"category": "books", "createdDate:gte": "2023-01-01"}:category=books&createdDate%3Agte=2023-01-01filter=%7B%22category%22:%20%22books%22,%20...%7DSeries
One of six per-language PRs for the same bug: #24797 go, #24802 python, #24803 typescript-fetch, #24867 kotlin, #24868 dart, #24869 ruby. No shared
main/code; each adds the same fixture,3_0/exploded-object-query-param.yaml.Fix
ApiClient.explode_query_object(name, obj), one call per parameter inapi.mustache. It serializes the value withsanitize_for_serializationand returns one(name, value)pair per entry.photoUrls, notphoto_urls) and unset properties are left out. A oneOf holding a primitive stays one parameter; one holding a list repeats the parameter name per item.Noneentry or list item is left out instead of going out ask=None. An entry holding a list repeats its name per item.parameters_to_url_queryquoted values but not names, and exploded names are runtime data. Declared names are quoted too; one with reserved characters, such aspage[size], now goes out aspage%5Bsize%5D, which servers decode to the same name.languagemap sits next tocontext: multi, solanguage={"context": "en"}went out ascontext=e&context=n. The format now applies only when the value is a list or tuple.Verified
A client generated from the fixture and the echo_api sample, against a server that echoes the request line:
filter={"category": "books", "createdDate:gte": "2023-01-01"}category=books&createdDate%3Agte=2023-01-01typedFilter={"category": "books"}category=booksfilter={"k": None, "a": "b", "l": ["x", None, 2]}a=b&l=x&l=2Pet(name="Hello World", photoUrls=["http://a.com"])name=Hello%20World&photoUrls=http%3A//a.comdeepFilter/flatFilterpage[size]page%5Bsize%5D=5Tests:
PythonClientCodegenTest#testExplodedObjectQueryParameterand#testExplodedModelQueryParameter(python-only fixture3_0/python/exploded-model-query-param.yaml); the echo_api runtime teststest_query_style_form_explode_true_object*insamples/client/echo_api/python/tests/test_manual.py; andtest_explode_query_object*andtest_parameters_to_url_query_*insamples/openapi3/client/petstore/python/tests/test_api_client.py.Known gaps
deepObjectandexplode: falsestill go out as a single JSON parameter; the spec wantsdeepFilter[category]=booksandflatFilter=category,books. Both are pre-existing and left for a follow-up.An entry whose value is itself an object (a nested object, or a list of objects) goes out as one JSON-encoded parameter under the entry's name. OpenAPI doesn't define how form/explode serializes nested objects, so it's left as it is.
PR checklist
./bin/generate-samples.sh bin/configs/python*.yaml): 15 generated files,api_client.py(the helper, quoting and guard) andfake_api.py/query_api.py(petstore'slanguagemap and the echo API's models). Thepython-pydantic-v1samples are untouched; that generator has its ownapi_client.mustache.Generated with Claude Code