fix(kotlin): support oneOf with primitive types in kotlinx_serialization#22986
fix(kotlin): support oneOf with primitive types in kotlinx_serialization#22986pvcresin wants to merge 1 commit intoOpenAPITools:masterfrom
Conversation
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all 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/kotlin-client/oneof_class.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/kotlin-client/oneof_class.mustache:217">
P2: oneOf deserialization returns on the first successful decode without checking for multiple matches, so ambiguous primitives (e.g., Int/Long/Double) will silently select the first schema instead of enforcing exactly one match.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
modules/openapi-generator/src/main/resources/kotlin-client/oneof_class.mustache
Show resolved
Hide resolved
|
@cubic-dev-ai rerun a review |
@pvcresin I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all 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/kotlin-client/oneof_class.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/kotlin-client/oneof_class.mustache:198">
P2: `{{{dataType}}}.serializer()` will not compile for parameterized/collection oneOf types (e.g., `List<Foo>`/`Map<String,Bar>`), so the generated Kotlin for oneOf with array/map schemas will fail to compile. Use a collection serializer or the top‑level `serializer<T>()` instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
modules/openapi-generator/src/main/resources/kotlin-client/oneof_class.mustache
Show resolved
Hide resolved
|
@cubic-dev-ai rerun a review |
@pvcresin I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all 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/kotlin-client/oneof_class.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/kotlin-client/oneof_class.mustache:217">
P2: Non-discriminator oneOf deserialization returns on the first successful decode, so overlapping primitive schemas can match multiple types but the first one wins, violating oneOf’s “exactly one” semantics.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
modules/openapi-generator/src/main/resources/kotlin-client/oneof_class.mustache
Show resolved
Hide resolved
|
This is a PR regarding Kotlin. Could you please review it? @karismann @Zomzog @andrewemery @4brunu @yutaka0m @stefankoppier @e5l @dennisameling |
d673376 to
cff7fbe
Compare
Rebased on the latest master. The code has not been changed since the PR was created. |
|
@pvcresin @e5l I've opened #23007 which tackles the same problem with a different approach — using Would appreciate your review on #23007 as well. Thanks! |
The kotlin-client generator with `jvm-retrofit2` library and `kotlinx_serialization` failed to generate working code for oneOf schemas containing primitive types (e.g. string, integer) without a discriminator. The existing template only handled discriminator-based oneOf via sealed interfaces, producing an empty class for primitive oneOf. Changes: - Add a non-discriminator fallback in oneof_class.mustache that generates a data class with actualInstance and a custom KSerializer, ported from the multiplatform template - Add isLong (int64) handling to the serialize branch, with an isInteger guard to avoid generating a dead branch for int64 types - Add test for oneOf primitive types with kotlinx_serialization
a84ec77 to
7296d21
Compare
|
Rebased on the latest master. The code has not been changed since the PR was created. |
|
@cubic-dev-ai rerun a review |
@pvcresin I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (all 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/kotlin-client/oneof_class.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/kotlin-client/oneof_class.mustache:198">
P2: Generated code uses `{{{dataType}}}.serializer()` for non‑primitive oneOf types, which breaks for generic/array types (e.g., `List<String>.serializer()` is invalid Kotlin). This causes compile errors for oneOf schemas containing arrays or other generics.</violation>
<violation number="2" location="modules/openapi-generator/src/main/resources/kotlin-client/oneof_class.mustache:216">
P2: Missing import for kotlinx.serialization.json.decodeFromJsonElement causes generated code to fail to compile.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| {{#composedSchemas}} | ||
| {{#oneOf}} | ||
| try { | ||
| val instance = jsonDecoder.json.decodeFromJsonElement<{{{dataType}}}>(jsonElement) |
There was a problem hiding this comment.
P2: Missing import for kotlinx.serialization.json.decodeFromJsonElement causes generated code to fail to compile.
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/kotlin-client/oneof_class.mustache, line 216:
<comment>Missing import for kotlinx.serialization.json.decodeFromJsonElement causes generated code to fail to compile.</comment>
<file context>
@@ -150,6 +154,78 @@ import java.io.IOException
+ {{#composedSchemas}}
+ {{#oneOf}}
+ try {
+ val instance = jsonDecoder.json.decodeFromJsonElement<{{{dataType}}}>(jsonElement)
+ return {{classname}}(actualInstance = instance)
+ } catch (e: Exception) {
</file context>
| {{/isNumber}} | ||
| {{/isPrimitiveType}} | ||
| {{^isPrimitiveType}} | ||
| is {{{dataType}}} -> jsonEncoder.encodeSerializableValue({{{dataType}}}.serializer(), instance) |
There was a problem hiding this comment.
P2: Generated code uses {{{dataType}}}.serializer() for non‑primitive oneOf types, which breaks for generic/array types (e.g., List<String>.serializer() is invalid Kotlin). This causes compile errors for oneOf schemas containing arrays or other generics.
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/kotlin-client/oneof_class.mustache, line 198:
<comment>Generated code uses `{{{dataType}}}.serializer()` for non‑primitive oneOf types, which breaks for generic/array types (e.g., `List<String>.serializer()` is invalid Kotlin). This causes compile errors for oneOf schemas containing arrays or other generics.</comment>
<file context>
@@ -150,6 +154,78 @@ import java.io.IOException
+ {{/isNumber}}
+ {{/isPrimitiveType}}
+ {{^isPrimitiveType}}
+ is {{{dataType}}} -> jsonEncoder.encodeSerializableValue({{{dataType}}}.serializer(), instance)
+ {{/isPrimitiveType}}
+ {{/oneOf}}
</file context>
|
please follow step 3 to update the samples let us know if you need help on that |
Description
The kotlin-client generator with
jvm-retrofit2library andkotlinx_serializationfailed to generate working code for oneOf schemas containing primitive types (e.g. string, integer) without a discriminator. The existing template only handled discriminator-based oneOf via sealed interfaces, producing an empty class for primitive oneOf.Changes:
Before
After
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description) (no existing issue)Summary by cubic
Fixes Kotlin client generation for jvm-retrofit2 + kotlinx_serialization when oneOf includes primitive types without a discriminator. Generates a data-class wrapper with a nested KSerializer that handles primitives and null, and tries each type on decode.
Written for commit 7296d21. Summary will update on new commits.