[JAVA, JAVA-SPRING, KOTLIN-CLIENT, KOTLIN-SPRING] - feature - add x-jackson-default-impl / typeInfoDefaultImpls support to allow specifying of fallback schema when deserializing - #23955
Conversation
…foDefaultImpls support
Emit `@JsonTypeInfo(defaultImpl = ...)` for both deduction-based and
discriminator-based oneOf interfaces.
Two configuration sources (config option takes precedence):
- Schema extension: `x-jackson-default-impl: ClassName`
- Config option: `typeInfoDefaultImpls: {SchemaName: ClassName}`
The resolved class name passes through `toModelName()` so schemaMapping,
importMapping, and DTO prefix/suffix are all honoured. A LOGGER.warn is
emitted when the config option shadows a schema-level annotation.
There was a problem hiding this comment.
1 issue found across 9 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| public static final String USE_SEALED_RESPONSE_INTERFACES = "useSealedResponseInterfaces"; | ||
| public static final String COMPANION_OBJECT = "companionObject"; | ||
| public static final String SUSPEND_FUNCTIONS = "suspendFunctions"; | ||
| public static final String TYPE_INFO_DEFAULT_IMPLS = "typeInfoDefaultImpls"; |
There was a problem hiding this comment.
@Picazsoo Could you also add the feature to the java generators?
See src/main/resources/Java/deductionAnnotation.mustache and the different typeInfoAnnotation.mustache under java/libraries
There was a problem hiding this comment.
I will take a look at it. And this is early implementation without any polish. I marked temporarily as "ready for review" just to gather early feedback from Cubic
There was a problem hiding this comment.
But thanks for the review! I will properly tag you once I think it is ready for wasting human review time (-:
There was a problem hiding this comment.
Hi @jpfinne, I added it to the java generators as well.
There was a problem hiding this comment.
This is now ready for review
# Conflicts: # modules/openapi-generator/src/test/java/org/openapitools/codegen/kotlin/spring/KotlinSpringServerCodegenTest.java
…Java generators - Add TYPE_INFO_DEFAULT_IMPLS/DESC constants to CodegenConstants (addresses reviewer request to avoid duplication across SpringCodegen and KotlinSpringServerCodegen) - Move typeInfoDefaultImpls field, option registration, processOpts parsing, and x-jackson-resolved-default-impl resolution loop from SpringCodegen into AbstractJavaCodegen so all Java generators inherit the feature - Remove duplicated code from SpringCodegen; update KotlinSpringServerCodegen to reference CodegenConstants.TYPE_INFO_DEFAULT_IMPLS and apply the same safe instanceof cast fix already present in SpringCodegen - Update Java/deductionAnnotation.mustache: emit defaultImpl = X.class on @JsonTypeInfo(use = DEDUCTION) when x-jackson-resolved-default-impl is set - Update Java/typeInfoAnnotation.mustache: emit defaultImpl = X.class on both @JsonTypeInfo(use = NAME) variants (discriminator present/absent) via discriminator.vendorExtensions.x-jackson-resolved-default-impl - Add JavaClientCodegenTest tests covering deduction, discriminator, typeInfoDefaultImpls config option, and absence cases for Java client Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
All reported issues were addressed across 17 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
9 issues found across 27 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/kotlin-spring/typeInfoAnnotation.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/kotlin-spring/typeInfoAnnotation.mustache:6">
P1: Missing validation that the resolved defaultImpl is a subtype of the polymorphic base; misconfigured `x-jackson-default-impl` or `typeInfoDefaultImpls` values will compile but fail at runtime during deserialization.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
# Conflicts: # modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java
|
Tagging @wing328 - please consider if this contribution makes sense and if it would make sense for 7.24.0 |
# Conflicts: # docs/generators/kotlin-spring.md # docs/generators/kotlin.md # modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java # modules/openapi-generator/src/test/java/org/openapitools/codegen/java/spring/SpringCodegenTest.java
There was a problem hiding this comment.
All reported issues were addressed across 333 files
Note: This PR contains a large number of files. cubic only reviews up to 200 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 74 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Hi @wing328 , I wonder if this feature is acceptable or not as is? |
…on-default # Conflicts: # docs/generators/java-camel.md # docs/generators/spring.md # modules/openapi-generator/src/main/java/org/openapitools/codegen/VendorExtension.java # modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/SpringCodegen.java
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
2 issues found across 30 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/server/petstore/kotlin-spring-oneof-jackson-default-impl/src/main/kotlin/org/openapitools/model/Soda.kt">
<violation number="1">
P2: This sample regeneration removes `@JsonInclude(NON_NULL)` and `@JsonSetter(nulls = Nulls.SKIP)` from the optional nullable field `sugarGrams`, so the generated `Soda` model will now serialize `"sugarGrams": null` instead of dropping the key when the field is null. This null-handling change is unrelated to the defaultImpl feature and contradicts the PR's stated 'no behavioral change unless opt-in / byte-for-byte identical' guarantee. Confirm this is an intended generator-output change (and matches what the generator actually emits) rather than an unintended side effect of the sample regeneration; if the generator change is spurious it should be reverted in the templates, otherwise update the PR description which claims output is otherwise unchanged.</violation>
</file>
<file name="samples/openapi3/server/petstore/spring-boot-oneof-jackson-default-impl/src/main/java/org/openapitools/model/Apple.java">
<violation number="1">
P2: This regeneration drops `@JsonInclude(JsonInclude.Include.NON_NULL)` from the optional `seeds` field (spec `type: integer`, non-nullable). With the annotation removed, a null `seeds` is now serialized as `"seeds": null` where it was previously omitted. This contradicts the generator's own matrix (optional & non-nullable -> NON_NULL in JsonAnnotationPolicyUtils.resolveJsonIncludePolicy) and diverges from sibling samples (spring-boot-oneof `Pizza.pizzaSize` and spring-boot-springdoc `Tag.id` keep NON_NULL on equivalent optional fields). Restore the annotation and the JsonInclude import, or regenerate the sample under the same config as the sibling samples.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| @@ -0,0 +1,33 @@ | |||
| package org.openapitools.model | |||
There was a problem hiding this comment.
P2: This sample regeneration removes @JsonInclude(NON_NULL) and @JsonSetter(nulls = Nulls.SKIP) from the optional nullable field sugarGrams, so the generated Soda model will now serialize "sugarGrams": null instead of dropping the key when the field is null. This null-handling change is unrelated to the defaultImpl feature and contradicts the PR's stated 'no behavioral change unless opt-in / byte-for-byte identical' guarantee. Confirm this is an intended generator-output change (and matches what the generator actually emits) rather than an unintended side effect of the sample regeneration; if the generator change is spurious it should be reverted in the templates, otherwise update the PR description which claims output is otherwise unchanged.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/server/petstore/kotlin-spring-oneof-jackson-default-impl/src/main/kotlin/org/openapitools/model/Soda.kt, line 32:
<comment>This sample regeneration removes `@JsonInclude(NON_NULL)` and `@JsonSetter(nulls = Nulls.SKIP)` from the optional nullable field `sugarGrams`, so the generated `Soda` model will now serialize `"sugarGrams": null` instead of dropping the key when the field is null. This null-handling change is unrelated to the defaultImpl feature and contradicts the PR's stated 'no behavioral change unless opt-in / byte-for-byte identical' guarantee. Confirm this is an intended generator-output change (and matches what the generator actually emits) rather than an unintended side effect of the sample regeneration; if the generator change is spurious it should be reverted in the templates, otherwise update the PR description which claims output is otherwise unchanged.</comment>
<file context>
@@ -1,10 +1,7 @@
package org.openapitools.model
import java.util.Objects
-import com.fasterxml.jackson.annotation.JsonInclude
import com.fasterxml.jackson.annotation.JsonProperty
-import com.fasterxml.jackson.annotation.JsonSetter
-import com.fasterxml.jackson.annotation.Nulls
import com.fasterxml.jackson.annotation.JsonIgnoreProperties
import com.fasterxml.jackson.annotation.JsonSubTypes
</file context>
| @@ -0,0 +1,121 @@ | |||
| package org.openapitools.model; | |||
There was a problem hiding this comment.
P2: This regeneration drops @JsonInclude(JsonInclude.Include.NON_NULL) from the optional seeds field (spec type: integer, non-nullable). With the annotation removed, a null seeds is now serialized as "seeds": null where it was previously omitted. This contradicts the generator's own matrix (optional & non-nullable -> NON_NULL in JsonAnnotationPolicyUtils.resolveJsonIncludePolicy) and diverges from sibling samples (spring-boot-oneof Pizza.pizzaSize and spring-boot-springdoc Tag.id keep NON_NULL on equivalent optional fields). Restore the annotation and the JsonInclude import, or regenerate the sample under the same config as the sibling samples.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/openapi3/server/petstore/spring-boot-oneof-jackson-default-impl/src/main/java/org/openapitools/model/Apple.java, line 31:
<comment>This regeneration drops `@JsonInclude(JsonInclude.Include.NON_NULL)` from the optional `seeds` field (spec `type: integer`, non-nullable). With the annotation removed, a null `seeds` is now serialized as `"seeds": null` where it was previously omitted. This contradicts the generator's own matrix (optional & non-nullable -> NON_NULL in JsonAnnotationPolicyUtils.resolveJsonIncludePolicy) and diverges from sibling samples (spring-boot-oneof `Pizza.pizzaSize` and spring-boot-springdoc `Tag.id` keep NON_NULL on equivalent optional fields). Restore the annotation and the JsonInclude import, or regenerate the sample under the same config as the sibling samples.</comment>
<file context>
@@ -28,7 +27,6 @@ public class Apple implements Fruit {
- @JsonInclude(JsonInclude.Include.NON_NULL)
private @Nullable Integer seeds;
public Apple() {
</file context>
Purpose
Complements the current oneOf Jackson polymorphism by making it possible to supply a default fallback value for deserialization. This is useful e.g. when you want to specify a default for deduction-based polymorphism (FasterXML/jackson-databind#3055), or when you want to fall back to some no-op class.
What it adds
Emits
@JsonTypeInfo(defaultImpl = ...)on polymorphiconeOfinterfaces. Two opt-in ways to configure it (both default to disabled):x-jackson-default-impl: <ClassName>on a schema.typeInfoDefaultImpls: {SchemaName: ClassName}, which overrides the vendor extension when both are set for the same schema.Class names are resolved via
toModelName(honoring model mappings and DTO prefix/suffix); warns on overrides and unknown targets.Scope
java(client)springkotlin-springkotlin(client)java-microprofileResolution is centralized in
AbstractJavaCodegen(shared by Java + Spring). Templates only emitdefaultImplwhen a value is resolved, so output is byte-for-byte identical to before for anyone who doesn't opt in.Tests & docs
kotlin-spring, andkotlinclient, covering deduction/discriminator cases, config override, model-name suffix handling, and anoDefaultImplWhenNeitherSourceIsSetcase that confirms no change when unconfigured.3_0/spring/jackson-default-impl.yaml.java,spring,kotlin-spring,kotlin, andjava-camel; newx-jackson-default-implvendor extension registered.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.
Summary by cubic
Adds opt-in Jackson default deserialization for
oneOfinterfaces. Previously no default was set; now@JsonTypeInfo(defaultImpl=...)is emitted when configured, with no change when unconfigured.x-jackson-default-implandtypeInfoDefaultImplsinAbstractJavaCodegentox-jackson-resolved-default-impl, honoringtoModelName, schema/import mappings, and DTO prefix/suffix.typeInfoDefaultImplsoverrides the vendor extension and warns on shadowed/unknown targets.defaultImplfor bothId.NAME(discriminator) andId.DEDUCTION.kotlin-spring: supports both deduction and discriminator.kotlinclient: discriminator-only.java-microprofile: disabled.defaultImplin type-info annotations across Java, Spring,kotlin-spring, andkotlinclient.CodegenConstants(TYPE_INFO_DEFAULT_IMPLS) and documented across generators; added samples and CI for Java RestClient, Spring Boot,kotlin-spring,kotlinclient, andjava-camel.kotlin-spring, andkotlinclient covering deduction/discriminator cases and config precedence.Migration
x-jackson-default-impl: ClassNameon a schema or configuretypeInfoDefaultImpls: {SchemaName: ClassName}; the config option overrides the extension.Written for commit 8d5365a. Summary will update on new commits.