-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
[python] fix: explode object query parameters #24802
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
1956b67
82c90b6
c3929d4
0a261c6
dc592a9
8ed9c17
16c7d8c
dc3b575
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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. | ||
|
|
||
|
|
@@ -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)): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, Prompt for AI agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( |
||
| 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: | ||
|
|
@@ -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]) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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, | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The collision scenario this comment describes (an exploded object property named Prompt for AI agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||||||||||
| "def explode_query_object(self, name, obj):", | ||||||||||||||
| "if k in collection_formats and isinstance(v, (list, tuple)):"); | ||||||||||||||
|
Comment on lines
+838
to
+839
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Prompt for AI agents
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||||||||||
| 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(); | ||||||||||||||
|
|
||||||||||||||
| 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 |
|---|---|---|
|
|
@@ -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] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Prompt for AI agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
|
||
|
|
@@ -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: | ||
|
|
@@ -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]) | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.