Skip to content

[python] fix: explode object query parameters - #24802

Open
wiebren wants to merge 8 commits into
OpenAPITools:masterfrom
wiebren:fix/exploded-object-query-parameters-python
Open

wiebren wants to merge 8 commits into
OpenAPITools:masterfrom
wiebren:fix/exploded-object-query-parameters-python

Conversation

@wiebren

@wiebren wiebren commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Bug

A query parameter whose schema is an object, with style/explode left at their defaults (form, 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 one parameter instead, both for a free-form object or map and for an object with declared properties (a model).

parameters:
  - in: query
    name: filter
    schema:
      type: object

called with {"category": "books", "createdDate:gte": "2023-01-01"}:

on the wire
expected category=books&createdDate%3Agte=2023-01-01
python before filter=%7B%22category%22:%20%22books%22,%20...%7D

Series

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

  1. Maps explode. A form-style, exploded map parameter goes through a new ApiClient.explode_query_object(name, obj), one call per parameter in api.mustache. It serializes the value with sanitize_for_serialization and returns one (name, value) pair per entry.
  2. Models explode too. Because the value is serialized first, a model explodes under its wire names (photoUrls, not photo_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.
  3. None is skipped. A None entry or list item is left out instead of going out as k=None. An entry holding a list repeats its name per item.
  4. Names are quoted. parameters_to_url_query quoted values but not names, and exploded names are runtime data. Declared names are quoted too; one with reserved characters, such as page[size], now goes out as page%5Bsize%5D, which servers decode to the same name.
  5. Collection formats apply only to lists. An exploded name can match a declared array parameter: petstore's language map sits next to context: multi, so language={"context": "en"} went out as context=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:

input on the wire
filter={"category": "books", "createdDate:gte": "2023-01-01"} category=books&createdDate%3Agte=2023-01-01
typedFilter={"category": "books"} category=books
filter={"k": None, "a": "b", "l": ["x", None, 2]} a=b&l=x&l=2
Pet(name="Hello World", photoUrls=["http://a.com"]) name=Hello%20World&photoUrls=http%3A//a.com
deepFilter / flatFilter one JSON parameter (as on master)
declared page[size] page%5Bsize%5D=5

Tests: PythonClientCodegenTest#testExplodedObjectQueryParameter and #testExplodedModelQueryParameter (python-only fixture 3_0/python/exploded-model-query-param.yaml); the echo_api runtime tests test_query_style_form_explode_true_object* in samples/client/echo_api/python/tests/test_manual.py; and test_explode_query_object* and test_parameters_to_url_query_* in samples/openapi3/client/petstore/python/tests/test_api_client.py.

Known gaps

deepObject and explode: false still go out as a single JSON parameter; the spec wants deepFilter[category]=books and flatFilter=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

  • Read the contribution guidelines.
  • Built the project and updated samples (./bin/generate-samples.sh bin/configs/python*.yaml): 15 generated files, api_client.py (the helper, quoting and guard) and fake_api.py/query_api.py (petstore's language map and the echo API's models). The python-pydantic-v1 samples are untouched; that generator has its own api_client.mustache.
  • Technical committee: @cbornet @tomplus @arun-nalla

Generated with Claude Code

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.
@wiebren

wiebren commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Split per language as requested, out of #24797. This is the python half; the siblings are:

The three touch disjoint sets of files under main/ and have no ordering dependency, so they
can be reviewed and merged independently. Each adds
modules/openapi-generator/src/test/resources/3_0/exploded-object-query-param.yaml at the
same path with identical content, so whichever lands first, the others rebase cleanly.

Each branch was tested on its own after the split, not just as part of the original combined
branch.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 17 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread modules/openapi-generator/src/main/resources/python/api.mustache
Comment thread samples/openapi3/client/petstore/python-aiohttp/petstore_api/api/fake_api.py Outdated
…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.
@wiebren

wiebren commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

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
missed it. parameters_to_url_query looked the name up in collection_formats without
checking the value was a collection. Harmless while every name came from baseName, but the
exploded names are runtime data, so a property name colliding with a sibling array parameter
sent the scalar down the list path and it was iterated character by character. Reproduced
against the petstore fixture, which declares context: multi next to the exploded map
language:

input before after
language={"context": "en", "region": "eu"} context=e&context=n&region=eu context=en&region=eu
language={"url": "https://x.test"} url=h,t,t,p,s,%3A,/,/,x,.,t,e,s,t url=https%3A//x.test

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 — multi, csv, ssv,
pipes — and each serializes byte for byte as before, so the guard only removes the
mis-dispatch. PythonClientCodegenTest#testExplodedObjectQueryParameter now asserts it.

Worth noting your suggested framing — serialize exploded entries separately from the
_collection_formats lookup — isn't reachable from api.mustache: everything lands in the
same _query_params list that parameters_to_url_query consumes, so the guard has to live
in the serializer. Same outcome, different seam.

P1, declared object models — valid, but pre-existing and deliberately out of scope. The
isMap gate does leave a $refed object model on the single-parameter path. That is not a
regression: every object was json encoded before this change, so models are no worse off,
maps are better off. Folding models in is not a template branch either — it needs the
generated model's wire names, not its python attribute names, so it has to route through the
serializer the way typescript-fetch has to route through {{dataType}}ToJSON (same gap,
see #24803). I have added it to the known-gaps section rather than half-fix it here. Happy to
take it as a follow-up.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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)):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread modules/openapi-generator/src/main/resources/python/api.mustache
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")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Comment on lines +759 to +760
"def explode_query_object(self, name, obj):",
"if k in collection_formats and isinstance(v, (list, tuple)):");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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>
Suggested change
"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)):");

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

wiebren and others added 2 commits September 23, 2026 11:51
…-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>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants