Skip to content
30 changes: 30 additions & 0 deletions modules/openapi-generator/src/main/resources/python/api.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,37 @@ https://github.com/OpenAPITools/openapi-generator/blob/c84b949df1a9ec04ba75989cb
_query_params.append(('{{baseName}}', {{paramName}}))
{{/isDate}}
{{^isDateTime}}{{^isDate}}
{{#isMap}}
{{#isExplode}}
{{#isDeepObject}}
_query_params.append(('{{baseName}}', {{paramName}}))
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
{{/isDeepObject}}
{{^isDeepObject}}
_query_params.extend(self.api_client.explode_query_object('{{baseName}}', {{paramName}}))
{{/isDeepObject}}
{{/isExplode}}
{{^isExplode}}
_query_params.append(('{{baseName}}', {{paramName}}))
{{/isExplode}}
{{/isMap}}
{{#isModel}}
{{#isExplode}}
{{#isDeepObject}}
_query_params.append(('{{baseName}}', {{paramName}}))
{{/isDeepObject}}
{{^isDeepObject}}
_query_params.extend(self.api_client.explode_query_object('{{baseName}}', {{paramName}}))
{{/isDeepObject}}
{{/isExplode}}
{{^isExplode}}
_query_params.append(('{{baseName}}', {{paramName}}))
{{/isExplode}}
{{/isModel}}
{{^isMap}}
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
{{^isModel}}
_query_params.append(('{{baseName}}', {{paramName}}{{#isEnumRef}}.value{{/isEnumRef}}))
{{/isModel}}
{{/isMap}}
{{/isDate}}{{/isDateTime}}
{{/queryParams}}
# process the header parameters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,13 @@ https://github.com/OpenAPITools/openapi-generator/blob/c84b949df1a9ec04ba75989cb
new_params.append((k, v))
return new_params

def explode_query_object(self, name, obj):
"""form style, explode: one query parameter per entry, keyed by the property name; a list repeats the name, None is left out"""
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):
"""Get parameters as list of tuples, formatting collections.

Expand All @@ -689,11 +696,12 @@ https://github.com/OpenAPITools/openapi-generator/blob/c84b949df1a9ec04ba75989cb
if isinstance(v, dict):
v = json.dumps(v)

if k in collection_formats:
# a collection format applies only to a list; an exploded entry may share a declared array parameter's name
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.

collection_format = collection_formats[k]
if collection_format == 'multi':
new_params.extend(
(k, quote(str(value).lower() if isinstance(value, bool) else str(value)))
(quote(str(k)), quote(str(value).lower() if isinstance(value, bool) else str(value)))
for value in v
)
else:
Expand All @@ -706,12 +714,13 @@ https://github.com/OpenAPITools/openapi-generator/blob/c84b949df1a9ec04ba75989cb
else: # csv is the default
delimiter = ','
new_params.append(
(k, delimiter.join(
(quote(str(k)), delimiter.join(
quote(str(value).lower() if isinstance(value, bool) else str(value))
for value in v))
)
else:
new_params.append((k, quote(str(v))))
# names are quoted too: an exploded object's names are runtime data
new_params.append((quote(str(k)), quote(str(v))))

return "&".join(["=".join(map(str, item)) for item in new_params])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,74 @@ public void testInitFileImportsExportsWithCustomApiPackage() throws IOException
assertFileContains(apiInitFile.toPath(), "from my_pkg.my_api.pet_api import PetApi");
}

@Test(description = "Verify a form style, exploded map query parameter goes on the wire one entry per parameter")
public void testExplodedObjectQueryParameter() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();

final CodegenConfigurator configurator = new CodegenConfigurator()
.setGeneratorName("python")
.setInputSpec("src/test/resources/3_0/exploded-object-query-param.yaml")
.setOutputDir(output.getAbsolutePath());

DefaultGenerator generator = new DefaultGenerator();
List<File> files = generator.opts(configurator.toClientOptInput()).generate();
files.forEach(File::deleteOnExit);

Path api = Paths.get(output.getAbsolutePath(), "openapi_client", "api", "default_api.py");

TestUtils.assertFileContains(api,
"_query_params.extend(self.api_client.explode_query_object('filter', filter))",
"_query_params.extend(self.api_client.explode_query_object('typedFilter', typed_filter))");
TestUtils.assertFileNotContains(api, "_query_params.append(('filter', filter))");

// deepObject and form without explode both keep a single parameter
TestUtils.assertFileContains(api,
"_query_params.append(('deepFilter', deep_filter))",
"_query_params.append(('flatFilter', flat_filter))");
TestUtils.assertFileNotContains(api,
"explode_query_object('deepFilter', deep_filter)",
"explode_query_object('flatFilter', flat_filter)");

// 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,

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.

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

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.

}

@Test
public void testExplodedModelQueryParameter() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
output.deleteOnExit();

final CodegenConfigurator configurator = new CodegenConfigurator()
.setGeneratorName("python")
.setInputSpec("src/test/resources/3_0/python/exploded-model-query-param.yaml")
.setOutputDir(output.getAbsolutePath());

DefaultGenerator generator = new DefaultGenerator();
List<File> files = generator.opts(configurator.toClientOptInput()).generate();
files.forEach(File::deleteOnExit);

Path api = Paths.get(output.getAbsolutePath(), "openapi_client", "api", "default_api.py");

// a model is serialized first (wire names), then exploded; a oneOf holding a primitive stays one parameter, one holding a list repeats the name
TestUtils.assertFileContains(api,
"_query_params.extend(self.api_client.explode_query_object('refFilter', ref_filter))",
"_query_params.extend(self.api_client.explode_query_object('inlineFilter', inline_filter))",
"_query_params.extend(self.api_client.explode_query_object('oneOfFilter', one_of_filter))");
TestUtils.assertFileNotContains(api, "_query_params.append(('oneOfFilter', one_of_filter))");

// deepObject and form without explode both keep a single parameter
TestUtils.assertFileContains(api,
"_query_params.append(('deepFilter', deep_filter))",
"_query_params.append(('flatFilter', flat_filter))");
TestUtils.assertFileNotContains(api,
"explode_query_object('deepFilter', deep_filter)",
"explode_query_object('flatFilter', flat_filter)");
}

@Test(description = "Verify default license format uses object notation when poetry1 is false")
public void testLicenseFormatInPyprojectToml() throws IOException {
File output = Files.createTempDirectory("test").toFile().getCanonicalFile();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
openapi: 3.0.3
info:
title: Exploded model query parameters
description: >
Query parameters whose schema is an object with declared properties, so the python
generator turns them into models rather than dicts. With the form/true defaults a model
must be exploded like a map, keyed by the names its properties carry on the wire.
version: 1.0.0
servers:
- url: localhost:8080
paths:
/items:
get:
operationId: listItems
parameters:
# a $ref to a component schema, with style and explode left at their defaults
- in: query
name: refFilter
schema:
$ref: '#/components/schemas/Filter'
# an inline object with declared properties, which becomes a model of its own
- in: query
name: inlineFilter
schema:
type: object
properties:
category:
type: string
# a oneOf model may hold a primitive or a list, neither of which has properties to explode
- in: query
name: oneOfFilter
schema:
$ref: '#/components/schemas/FilterOrTerm'
# deepObject and form without explode both keep a single parameter
- in: query
name: deepFilter
style: deepObject
explode: true
schema:
$ref: '#/components/schemas/Filter'
- in: query
name: flatFilter
style: form
explode: false
schema:
$ref: '#/components/schemas/Filter'
responses:
'200':
description: a list of items
components:
schemas:
Filter:
type: object
properties:
category:
type: string
# the wire name is not a python identifier, so the model attribute is renamed
createdDate:gte:
type: string
FilterOrTerm:
oneOf:
- $ref: '#/components/schemas/Filter'
- type: string
- type: array
items:
type: string
Original file line number Diff line number Diff line change
Expand Up @@ -2204,7 +2204,7 @@ def _test_query_style_form_explode_true_array_string_serialize(
# process the query parameters
if query_object is not None:

_query_params.append(('query_object', query_object))
_query_params.extend(self.api_client.explode_query_object('query_object', query_object))

# process the header parameters
# process the form parameters
Expand Down Expand Up @@ -2466,7 +2466,7 @@ def _test_query_style_form_explode_true_object_serialize(
# process the query parameters
if query_object is not None:

_query_params.append(('query_object', query_object))
_query_params.extend(self.api_client.explode_query_object('query_object', query_object))

# process the header parameters
# process the form parameters
Expand Down Expand Up @@ -2728,7 +2728,7 @@ def _test_query_style_form_explode_true_object_all_of_serialize(
# process the query parameters
if query_object is not None:

_query_params.append(('query_object', query_object))
_query_params.extend(self.api_client.explode_query_object('query_object', query_object))

# process the header parameters
# process the form parameters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,13 @@ def parameters_to_tuples(self, params, collection_formats):
new_params.append((k, v))
return new_params

def explode_query_object(self, name, obj):
"""form style, explode: one query parameter per entry, keyed by the property name; a list repeats the name, None is left out"""
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.


def parameters_to_url_query(self, params, collection_formats):
"""Get parameters as list of tuples, formatting collections.

Expand All @@ -538,11 +545,12 @@ def parameters_to_url_query(self, params, collection_formats):
if isinstance(v, dict):
v = json.dumps(v)

if k in collection_formats:
# a collection format applies only to a list; an exploded entry may share a declared array parameter's name
if k in collection_formats and isinstance(v, (list, tuple)):
collection_format = collection_formats[k]
if collection_format == 'multi':
new_params.extend(
(k, quote(str(value).lower() if isinstance(value, bool) else str(value)))
(quote(str(k)), quote(str(value).lower() if isinstance(value, bool) else str(value)))
for value in v
)
else:
Expand All @@ -555,12 +563,13 @@ def parameters_to_url_query(self, params, collection_formats):
else: # csv is the default
delimiter = ','
new_params.append(
(k, delimiter.join(
(quote(str(k)), delimiter.join(
quote(str(value).lower() if isinstance(value, bool) else str(value))
for value in v))
)
else:
new_params.append((k, quote(str(v))))
# names are quoted too: an exploded object's names are runtime data
new_params.append((quote(str(k)), quote(str(v))))

return "&".join(["=".join(map(str, item)) for item in new_params])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2204,7 +2204,7 @@ def _test_query_style_form_explode_true_array_string_serialize(
# process the query parameters
if query_object is not None:

_query_params.append(('query_object', query_object))
_query_params.extend(self.api_client.explode_query_object('query_object', query_object))

# process the header parameters
# process the form parameters
Expand Down Expand Up @@ -2466,7 +2466,7 @@ def _test_query_style_form_explode_true_object_serialize(
# process the query parameters
if query_object is not None:

_query_params.append(('query_object', query_object))
_query_params.extend(self.api_client.explode_query_object('query_object', query_object))

# process the header parameters
# process the form parameters
Expand Down Expand Up @@ -2728,7 +2728,7 @@ def _test_query_style_form_explode_true_object_all_of_serialize(
# process the query parameters
if query_object is not None:

_query_params.append(('query_object', query_object))
_query_params.extend(self.api_client.explode_query_object('query_object', query_object))

# process the header parameters
# process the form parameters
Expand Down
17 changes: 13 additions & 4 deletions samples/client/echo_api/python/openapi_client/api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,13 @@ def parameters_to_tuples(self, params, collection_formats):
new_params.append((k, v))
return new_params

def explode_query_object(self, name, obj):
"""form style, explode: one query parameter per entry, keyed by the property name; a list repeats the name, None is left out"""
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):
"""Get parameters as list of tuples, formatting collections.

Expand All @@ -538,11 +545,12 @@ def parameters_to_url_query(self, params, collection_formats):
if isinstance(v, dict):
v = json.dumps(v)

if k in collection_formats:
# a collection format applies only to a list; an exploded entry may share a declared array parameter's name
if k in collection_formats and isinstance(v, (list, tuple)):
collection_format = collection_formats[k]
if collection_format == 'multi':
new_params.extend(
(k, quote(str(value).lower() if isinstance(value, bool) else str(value)))
(quote(str(k)), quote(str(value).lower() if isinstance(value, bool) else str(value)))
for value in v
)
else:
Expand All @@ -555,12 +563,13 @@ def parameters_to_url_query(self, params, collection_formats):
else: # csv is the default
delimiter = ','
new_params.append(
(k, delimiter.join(
(quote(str(k)), delimiter.join(
quote(str(value).lower() if isinstance(value, bool) else str(value))
for value in v))
)
else:
new_params.append((k, quote(str(v))))
# names are quoted too: an exploded object's names are runtime data
new_params.append((quote(str(k)), quote(str(v))))

return "&".join(["=".join(map(str, item)) for item in new_params])

Expand Down
Loading
Loading