-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
[Fix] [Regression] Resolve the discriminator type from a 3.1 sibling #22634
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?
[Fix] [Regression] Resolve the discriminator type from a 3.1 sibling #22634
Conversation
19c7b1d to
977c551
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9 issues found across 117 files
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="samples/client/others/java/webclient-sealedInterface_3_1/src/main/java/org/openapitools/client/model/Animal.java">
<violation number="1" location="samples/client/others/java/webclient-sealedInterface_3_1/src/main/java/org/openapitools/client/model/Animal.java:33">
P1: The `@JsonIgnoreProperties` annotation has an empty `value` where the discriminator property name should be. This will cause Jackson serialization issues. Compare to `BarRefOrValue.java` and `Fruit.java` where the discriminator property (e.g., `"@type"`, `"fruitType"`) is correctly populated.</violation>
<violation number="2" location="samples/client/others/java/webclient-sealedInterface_3_1/src/main/java/org/openapitools/client/model/Animal.java:36">
P1: The `@JsonTypeInfo` annotation has an empty `property` value. This will prevent Jackson from correctly deserializing the polymorphic type since it won't know which JSON property identifies the subtype. Compare to other generated interfaces like `Fruit.java` which correctly has `property = "fruitType"`.</violation>
</file>
<file name=".github/workflows/samples-java-client-jdk17.yaml">
<violation number="1" location=".github/workflows/samples-java-client-jdk17.yaml:10">
P1: Path mismatch: the actual directory is `webclient-sealedInterface_3_1` (underscores) but workflow references `webclient-sealedInterface-3.1` (hyphen with dot). This will prevent the workflow from triggering and cause build failures.</violation>
<violation number="2" location=".github/workflows/samples-java-client-jdk17.yaml:39">
P1: Path mismatch: should be `webclient-sealedInterface_3_1` to match the actual directory name. The build will fail with this incorrect path.</violation>
</file>
<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java">
<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java:4073">
P2: Use `fileContains` instead of `fileContainsPattern` for literal string matching. The parentheses in `"public FruitType getFruitType()"` are regex special characters (grouping operators). While this works by accident (empty capture groups match nothing), it's fragile and semantically incorrect. The `fileContains` method exists for literal string matching.</violation>
</file>
<file name="samples/client/others/java/webclient-sealedInterface_3_1/src/main/java/org/openapitools/client/ServerConfiguration.java">
<violation number="1" location="samples/client/others/java/webclient-sealedInterface_3_1/src/main/java/org/openapitools/client/ServerConfiguration.java:55">
P2: Potential `NullPointerException` if `serverVariable.enumValues` is null. The `ServerVariable.enumValues` field defaults to null.</violation>
</file>
<file name="samples/client/others/java/webclient-sealedInterface_3_1/build.gradle">
<violation number="1" location="samples/client/others/java/webclient-sealedInterface_3_1/build.gradle:138">
P2: JUnit 5 test configuration is incomplete. Missing `junit-jupiter-engine` dependency which is required at runtime to execute tests. Without it, tests will compile but not run.</violation>
</file>
<file name="samples/client/others/java/webclient-sealedInterface_3_1/README.md">
<violation number="1" location="samples/client/others/java/webclient-sealedInterface_3_1/README.md:19">
P2: Incorrect Java version requirement. The README states 'Java 1.8+' but this sample uses sealed interfaces which require Java 17+. The pom.xml correctly specifies Java 17. Consider updating the README template to reflect the actual Java version requirement for sealed interface samples.</violation>
</file>
<file name="samples/client/others/java/webclient-sealedInterface_3_1/src/main/java/org/openapitools/client/RFC3339JavaTimeModule.java">
<violation number="1" location="samples/client/others/java/webclient-sealedInterface_3_1/src/main/java/org/openapitools/client/RFC3339JavaTimeModule.java:32">
P1: Deserializers added after `super.setupModule(context)` will not be registered. In Jackson's `SimpleModule`, `setupModule` processes the deserializer collection when called. Move the `addDeserializer` calls to the constructor, or call them before `super.setupModule(context)`.</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
- 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.
|
|
||
| @javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", comments = "Generator version: 7.19.0-SNAPSHOT") | ||
| @JsonIgnoreProperties( | ||
| value = "", // ignore manually set , it will be automatically generated by Jackson during serialization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: The @JsonIgnoreProperties annotation has an empty value where the discriminator property name should be. This will cause Jackson serialization issues. Compare to BarRefOrValue.java and Fruit.java where the discriminator property (e.g., "@type", "fruitType") is correctly populated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/webclient-sealedInterface_3_1/src/main/java/org/openapitools/client/model/Animal.java, line 33:
<comment>The `@JsonIgnoreProperties` annotation has an empty `value` where the discriminator property name should be. This will cause Jackson serialization issues. Compare to `BarRefOrValue.java` and `Fruit.java` where the discriminator property (e.g., `"@type"`, `"fruitType"`) is correctly populated.</comment>
<file context>
@@ -0,0 +1,40 @@
+
[email protected](value = "org.openapitools.codegen.languages.JavaClientCodegen", comments = "Generator version: 7.19.0-SNAPSHOT")
+@JsonIgnoreProperties(
+ value = "", // ignore manually set , it will be automatically generated by Jackson during serialization
+ allowSetters = true // allows the to be set during deserialization
+)
</file context>
| value = "", // ignore manually set , it will be automatically generated by Jackson during serialization | ||
| allowSetters = true // allows the to be set during deserialization | ||
| ) | ||
| @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "", visible = true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: The @JsonTypeInfo annotation has an empty property value. This will prevent Jackson from correctly deserializing the polymorphic type since it won't know which JSON property identifies the subtype. Compare to other generated interfaces like Fruit.java which correctly has property = "fruitType".
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/webclient-sealedInterface_3_1/src/main/java/org/openapitools/client/model/Animal.java, line 36:
<comment>The `@JsonTypeInfo` annotation has an empty `property` value. This will prevent Jackson from correctly deserializing the polymorphic type since it won't know which JSON property identifies the subtype. Compare to other generated interfaces like `Fruit.java` which correctly has `property = "fruitType"`.</comment>
<file context>
@@ -0,0 +1,40 @@
+ value = "", // ignore manually set , it will be automatically generated by Jackson during serialization
+ allowSetters = true // allows the to be set during deserialization
+)
+@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "", visible = true)
+
+public sealed interface Animal permits Dog, Cat {
</file context>
| super.setupModule(context); | ||
|
|
||
| addDeserializer(Instant.class, RFC3339InstantDeserializer.INSTANT); | ||
| addDeserializer(OffsetDateTime.class, RFC3339InstantDeserializer.OFFSET_DATE_TIME); | ||
| addDeserializer(ZonedDateTime.class, RFC3339InstantDeserializer.ZONED_DATE_TIME); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Deserializers added after super.setupModule(context) will not be registered. In Jackson's SimpleModule, setupModule processes the deserializer collection when called. Move the addDeserializer calls to the constructor, or call them before super.setupModule(context).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/webclient-sealedInterface_3_1/src/main/java/org/openapitools/client/RFC3339JavaTimeModule.java, line 32:
<comment>Deserializers added after `super.setupModule(context)` will not be registered. In Jackson's `SimpleModule`, `setupModule` processes the deserializer collection when called. Move the `addDeserializer` calls to the constructor, or call them before `super.setupModule(context)`.</comment>
<file context>
@@ -0,0 +1,39 @@
+
+ @Override
+ public void setupModule(SetupContext context) {
+ super.setupModule(context);
+
+ addDeserializer(Instant.class, RFC3339InstantDeserializer.INSTANT);
</file context>
| super.setupModule(context); | |
| addDeserializer(Instant.class, RFC3339InstantDeserializer.INSTANT); | |
| addDeserializer(OffsetDateTime.class, RFC3339InstantDeserializer.OFFSET_DATE_TIME); | |
| addDeserializer(ZonedDateTime.class, RFC3339InstantDeserializer.ZONED_DATE_TIME); | |
| addDeserializer(Instant.class, RFC3339InstantDeserializer.INSTANT); | |
| addDeserializer(OffsetDateTime.class, RFC3339InstantDeserializer.OFFSET_DATE_TIME); | |
| addDeserializer(ZonedDateTime.class, RFC3339InstantDeserializer.ZONED_DATE_TIME); | |
| super.setupModule(context); |
...les/openapi-generator/src/test/java/org/openapitools/codegen/java/JavaClientCodegenTest.java
Outdated
Show resolved
Hide resolved
|
|
||
| if (variables != null && variables.containsKey(name)) { | ||
| value = variables.get(name); | ||
| if (serverVariable.enumValues.size() > 0 && !serverVariable.enumValues.contains(value)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Potential NullPointerException if serverVariable.enumValues is null. The ServerVariable.enumValues field defaults to null.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/webclient-sealedInterface_3_1/src/main/java/org/openapitools/client/ServerConfiguration.java, line 55:
<comment>Potential `NullPointerException` if `serverVariable.enumValues` is null. The `ServerVariable.enumValues` field defaults to null.</comment>
<file context>
@@ -0,0 +1,72 @@
+
+ if (variables != null && variables.containsKey(name)) {
+ value = variables.get(name);
+ if (serverVariable.enumValues.size() > 0 && !serverVariable.enumValues.contains(value)) {
+ throw new IllegalArgumentException("The variable " + name + " in the server URL has invalid value " + value + ".");
+ }
</file context>
| if (serverVariable.enumValues.size() > 0 && !serverVariable.enumValues.contains(value)) { | |
| if (serverVariable.enumValues != null && !serverVariable.enumValues.isEmpty() && !serverVariable.enumValues.contains(value)) { |
| implementation "org.openapitools:jackson-databind-nullable:$jackson_databind_nullable_version" | ||
| implementation "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:$jackson_version" | ||
| implementation "jakarta.annotation:jakarta.annotation-api:$jakarta_annotation_version" | ||
| testImplementation "org.junit.jupiter:junit-jupiter-api:$junit_version" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: JUnit 5 test configuration is incomplete. Missing junit-jupiter-engine dependency which is required at runtime to execute tests. Without it, tests will compile but not run.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/webclient-sealedInterface_3_1/build.gradle, line 138:
<comment>JUnit 5 test configuration is incomplete. Missing `junit-jupiter-engine` dependency which is required at runtime to execute tests. Without it, tests will compile but not run.</comment>
<file context>
@@ -0,0 +1,139 @@
+ implementation "org.openapitools:jackson-databind-nullable:$jackson_databind_nullable_version"
+ implementation "com.fasterxml.jackson.datatype:jackson-datatype-jsr310:$jackson_version"
+ implementation "jakarta.annotation:jakarta.annotation-api:$jakarta_annotation_version"
+ testImplementation "org.junit.jupiter:junit-jupiter-api:$junit_version"
+}
</file context>
| testImplementation "org.junit.jupiter:junit-jupiter-api:$junit_version" | |
| testImplementation "org.junit.jupiter:junit-jupiter-api:$junit_version" | |
| testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:$junit_version" |
|
|
||
| Building the API client library requires: | ||
|
|
||
| 1. Java 1.8+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Incorrect Java version requirement. The README states 'Java 1.8+' but this sample uses sealed interfaces which require Java 17+. The pom.xml correctly specifies Java 17. Consider updating the README template to reflect the actual Java version requirement for sealed interface samples.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/others/java/webclient-sealedInterface_3_1/README.md, line 19:
<comment>Incorrect Java version requirement. The README states 'Java 1.8+' but this sample uses sealed interfaces which require Java 17+. The pom.xml correctly specifies Java 17. Consider updating the README template to reflect the actual Java version requirement for sealed interface samples.</comment>
<file context>
@@ -0,0 +1,162 @@
+
+Building the API client library requires:
+
+1. Java 1.8+
+2. Maven/Gradle
+
</file context>
| 1. Java 1.8+ | |
| 1. Java 17+ |
977c551 to
b7d5e42
Compare
|
|
With 7.18 I have started to encounter an error when I generate a oneOf-interface. The issue is that the interface's getter type has been changed from
EnumValuetoString, while all implementors still have the correctEnumValuethat they are meant to implement.I have tracked down the issue to the fact that I am using a 3.1 specification, where my disciminator also defines a unique description for the enum. This, combined with the introduction of #22364, has made it so that the definition
is normalized to
which makes it so that the discriminator calculator loses the type
FruitType, and instead falls back toString.I have adjusted for this by adding so that the discriminator calculator considers this sibling structure as a possible scenario, and that it will go one
$refdeeper if it sees that the initially encountered discriminator property is anallOfwithout any property.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)Summary by cubic
Fix discriminator type resolution for OpenAPI 3.1 when the discriminator property is wrapped in an allOf sibling, so oneOf interface getters return the enum type (e.g., FruitType) instead of String. Restores correct typing in generated Java webclient sealed interfaces.
Bug Fixes
New Features
Written for commit a7dd54f. Summary will update on new commits.