Skip to content

Commit af3a9f7

Browse files
wiebrenclaude
andauthored
[kotlin] fix: explode object query parameters (#24867)
* fix: [kotlin] explode object query parameters in the jvm-okhttp client 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 jvm-okhttp client serialized the whole map with toString() into a single parameter instead (filter={tld=com, ...}), and a declared map came out as concatenated Pair.toString()s. The jvm-ktor library already explodes these correctly. The query building in api.mustache now iterates an exploded map entry by entry, the same way jvm-ktor does. deepObject and explode: false objects keep their previous wire format, byte for byte. The new test fixture covers the four style/explode combinations that decide the wire format; the test fails without the template change. No sample changes: no kotlin sample spec declares a free-form or map-typed query parameter with the default style, the echo api's object query parameters are all model-typed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CxDNCjqJycKTfzVWg2SeTJ * fix: [kotlin] skip nulls, repeat collections and append exploded query entries Follow-up to the review of the exploded object query parameters in the jvm-okhttp client: - a null map entry is left out instead of going on the wire as the literal "null" (Any?.toString() returns "null" rather than throwing) - a collection value repeats the key once per element instead of being sent as the collection's toString(); scalar values and elements go through parameterToString, so dates get the same format as other query parameters - an exploded entry named like another query parameter no longer overwrites it, or gets overwritten by it: the exploded parameters are now emitted after every declared query parameter and append to an existing entry, so both values end up on the wire To place the exploded parameters after the declared ones, the codegen flags them with x-kotlin-explode-form-object (jvm-okhttp libraries only) and api.mustache emits them in a second pass; the three duplicated required/optional/nullable branches of the first version are gone. deepObject and explode: false parameters are unchanged, and no sample changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: [kotlin] trim the exploded query comment and simplify the entry loop One comment line above the loop instead of three; the when now only picks the elements and filterNotNull/parameterToString run once. Drop the defensive !isModel check, which no other port has: a map-typed parameter is never isModel. Fix the fixture description, which claimed four style/explode combinations where it covers three, and retitle the test. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test: [kotlin] keep the shared fixture byte-identical with the other ports Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent aa783ed commit af3a9f7

3 files changed

Lines changed: 68 additions & 0 deletions

File tree

‎modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/KotlinClientCodegen.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,6 +1157,14 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
11571157
.filter(isSerializable)
11581158
.collect(Collectors.toList());
11591159
operation.hasProduces = operation.produces != null && !operation.produces.isEmpty();
1160+
1161+
// form style with explode puts a map-typed query parameter on the wire as one parameter
1162+
// per entry; api.mustache adds those after the declared query parameters
1163+
for (CodegenParameter param : operation.queryParams) {
1164+
if (param.isMap && param.isExplode && !param.isDeepObject) {
1165+
param.vendorExtensions.put("x-kotlin-explode-form-object", true);
1166+
}
1167+
}
11601168
}
11611169

11621170
// set multipart against all relevant operations

‎modules/openapi-generator/src/main/resources/kotlin-client/libraries/jvm-okhttp/api.mustache‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ import {{packageName}}.infrastructure.Serializer
209209
{{/hasQueryParams}}{{#hasQueryParams}}mutableMapOf<kotlin.String, kotlin.collections.List<kotlin.String>>()
210210
.apply {
211211
{{#queryParams}}
212+
{{^vendorExtensions.x-kotlin-explode-form-object}}
212213
{{^required}}
213214
if ({{{paramName}}} != null) {
214215
{{#isModel}}
@@ -251,6 +252,23 @@ import {{packageName}}.infrastructure.Serializer
251252
{{/isModel}}
252253
{{/isNullable}}
253254
{{/required}}
255+
{{/vendorExtensions.x-kotlin-explode-form-object}}
256+
{{/queryParams}}
257+
{{#queryParams}}
258+
{{#vendorExtensions.x-kotlin-explode-form-object}}
259+
// form style, explode: one query parameter per entry, keyed by the property name, appended after the declared ones; null is left out, a collection repeats the key
260+
({{{paramName}}} as? kotlin.collections.Map<*, *>)?.forEach { (key, value) ->
261+
val values = when (value) {
262+
is kotlin.collections.Iterable<*> -> value.toList()
263+
is kotlin.Array<*> -> value.toList()
264+
else -> listOf(value)
265+
}.filterNotNull().map { parameterToString(it) }
266+
if (key != null && values.isNotEmpty()) {
267+
val name = key.toString()
268+
put(name, getOrElse(name) { emptyList() } + values)
269+
}
270+
}
271+
{{/vendorExtensions.x-kotlin-explode-form-object}}
254272
{{/queryParams}}
255273
}
256274
{{/hasQueryParams}}

‎modules/openapi-generator/src/test/java/org/openapitools/codegen/kotlin/KotlinClientCodegenApiTest.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
import java.io.File;
1717
import java.io.IOException;
18+
import java.nio.charset.StandardCharsets;
1819
import java.nio.file.Files;
1920
import java.nio.file.Paths;
2021
import java.util.List;
@@ -253,6 +254,47 @@ public void testJvmKtorQueryParamWithTypeObject() throws IOException {
253254
assertFileNotContains(defaultApi.toPath(), "mapDeep?.apply {");
254255
}
255256

257+
@Test(description = "Verify a form style, exploded map query parameter goes on the wire one entry per parameter")
258+
public void testExplodedObjectQueryParameterJvmOkhttp() throws IOException {
259+
OpenAPI openAPI = readOpenAPI("src/test/resources/3_0/exploded-object-query-param.yaml");
260+
261+
KotlinClientCodegen codegen = createCodegen(ClientLibrary.JVM_OKHTTP4);
262+
DefaultGenerator generator = new DefaultGenerator();
263+
enableOnlyApiGeneration(generator);
264+
265+
List<File> files = generator.opts(createClientOptInput(openAPI, codegen)).generate();
266+
File defaultApi = files.stream().filter(file -> file.getName().equals("DefaultApi.kt")).findAny().orElseThrow();
267+
268+
String content = new String(Files.readAllBytes(defaultApi.toPath()), StandardCharsets.UTF_8);
269+
270+
// form style with explode - the default - puts every entry on the wire under its own
271+
// property name. Serializing the whole map with toString() is what used to happen.
272+
assertFileContains(defaultApi.toPath(), "(filter as? kotlin.collections.Map<*, *>)?.forEach { (key, value) ->");
273+
assertFileNotContains(defaultApi.toPath(), "put(\"filter\", listOf(filter.toString()))");
274+
275+
// a declared map behaves the same way
276+
assertFileContains(defaultApi.toPath(), "(typedFilter as? kotlin.collections.Map<*, *>)?.forEach { (key, value) ->");
277+
assertFileNotContains(defaultApi.toPath(), "put(\"typedFilter\"");
278+
279+
// a null entry is left out rather than sent as "null", a collection repeats the key
280+
// once per element rather than going out as its toString(), and an entry is appended
281+
// rather than replacing a query parameter of the same name
282+
assertFileContains(defaultApi.toPath(),
283+
"is kotlin.collections.Iterable<*> -> value.toList()",
284+
"}.filterNotNull().map { parameterToString(it) }",
285+
"put(name, getOrElse(name) { emptyList() } + values)");
286+
287+
// deepObject and form without explode both keep a single parameter
288+
assertFileContains(defaultApi.toPath(),
289+
"put(\"deepFilter\", listOf(deepFilter.toString()))",
290+
"put(\"flatFilter\", listOf(flatFilter.toString()))");
291+
292+
// the exploded entries are added after every declared parameter, so a declared
293+
// parameter's put cannot overwrite an entry that happens to share its name
294+
Assert.assertTrue(content.indexOf("(filter as? kotlin.collections.Map") > content.indexOf("put(\"flatFilter\""),
295+
"exploded entries must be added after the declared query parameters");
296+
}
297+
256298
private static void assertFileContainsLine(List<String> lines, String line) {
257299
Assert.assertListContains(lines, s -> s.equals(line), line);
258300
}

0 commit comments

Comments
 (0)