fix(go): preserve json tags for properties with oneOf - #24920
Conversation
There was a problem hiding this comment.
9 issues found across 65 files
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/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/ApiClient.java">
<violation number="1" location="samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/ApiClient.java:67">
P1: With the fixture's relative server URL, the default client cannot make any request: server selection yields `/ping` instead of an absolute URL. Resolve relative server URLs against the configured `basePath` (or otherwise provide an absolute default) before constructing the OkHttp request.</violation>
<violation number="2" location="samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/ApiClient.java:343">
P3: `setDateFormat` leaves the field returned by `getDateFormat` unchanged, so the public getter does not reflect the configured format. Assign `this.dateFormat` before updating the JSON adapter.</violation>
<violation number="3" location="samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/ApiClient.java:1451">
P2: When a multipart parameter is a list of non-file values, this branch adds the entire list once per item. Pass `item` here so each scalar list element is emitted as its own multipart part.</violation>
</file>
<file name="samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/auth/HttpBearerAuth.java">
<violation number="1" location="samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/auth/HttpBearerAuth.java:40">
P2: When `getBearerToken()` is called before a token is configured, it throws a `NullPointerException` even though `applyToParams` supports an unset token. Return `null` when `tokenSupplier` is unset.</violation>
</file>
<file name="samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/JSON.java">
<violation number="1" location="samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/JSON.java:72">
P3: This schema has no discriminator models, so these private discriminator helpers are dead code in the generated client and cannot affect deserialization. Emit them only when discriminator support is needed, or remove them from this fixture.</violation>
</file>
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java:1807">
P2: Custom Java generators that override `escapeText` no longer apply their escaping to String parameter examples. Preserve an overridable literal-escaping hook while omitting only the JavaDoc delimiter replacement.</violation>
<violation number="2" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java:1807">
P3: The behavior change in setParameterExampleValue() (String examples no longer get */ and /* escaped) has no regression test. The PR only adds EscapeJavaDocLambdaTest for the new lambda, but the String-example escaping change is a distinct code path and the existing AbstractJavaCodegenExampleValuesTest already exercises setParameterExampleValue() for String types. Add a case there (or in a new test) asserting that a String example/default like "*/*" now yields "*/*" (not "*_/*") to lock in the intended behavior.</violation>
</file>
<file name="samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/Pair.java">
<violation number="1" location="samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/Pair.java:17">
P3: This PR is scoped to the Go generator JSON-tag fix (#24916), but it adds a new Java client sample directory (wildcard-accept-header/java) that is unrelated to that change and unmentioned in the description. If this is not part of the intended change, remove it; if it is intentional, note it so reviewers don't have to guess.</violation>
</file>
<file name="samples/client/wildcard-accept-header/java/build.sbt">
<violation number="1" location="samples/client/wildcard-accept-header/java/build.sbt:20">
P3: The `jakarta.annotation-api` dependency is declared twice with identical group, artifact, version, and scope (lines 19 and 21). Remove the duplicate entry so the dependency list stays unambiguous and matches the pom.xml/build.gradle declarations.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| protected String basePath = "http://localhost"; | ||
| protected List<ServerConfiguration> servers = new ArrayList<ServerConfiguration>(Arrays.asList( | ||
| new ServerConfiguration( | ||
| "", |
There was a problem hiding this comment.
P1: With the fixture's relative server URL, the default client cannot make any request: server selection yields /ping instead of an absolute URL. Resolve relative server URLs against the configured basePath (or otherwise provide an absolute default) before constructing the OkHttp request.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/ApiClient.java, line 67:
<comment>With the fixture's relative server URL, the default client cannot make any request: server selection yields `/ping` instead of an absolute URL. Resolve relative server URLs against the configured `basePath` (or otherwise provide an absolute default) before constructing the OkHttp request.</comment>
<file context>
@@ -0,0 +1,1642 @@
+ protected String basePath = "http://localhost";
+ protected List<ServerConfiguration> servers = new ArrayList<ServerConfiguration>(Arrays.asList(
+ new ServerConfiguration(
+ "",
+ "No description provided",
+ new HashMap<String, ServerVariable>()
</file context>
| if (item instanceof File) { | ||
| addPartToMultiPartBuilder(mpBuilder, param.getKey(), (File) item); | ||
| } else { | ||
| addPartToMultiPartBuilder(mpBuilder, param.getKey(), param.getValue()); |
There was a problem hiding this comment.
P2: When a multipart parameter is a list of non-file values, this branch adds the entire list once per item. Pass item here so each scalar list element is emitted as its own multipart part.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/ApiClient.java, line 1451:
<comment>When a multipart parameter is a list of non-file values, this branch adds the entire list once per item. Pass `item` here so each scalar list element is emitted as its own multipart part.</comment>
<file context>
@@ -0,0 +1,1642 @@
+ if (item instanceof File) {
+ addPartToMultiPartBuilder(mpBuilder, param.getKey(), (File) item);
+ } else {
+ addPartToMultiPartBuilder(mpBuilder, param.getKey(), param.getValue());
+ }
+ }
</file context>
| addPartToMultiPartBuilder(mpBuilder, param.getKey(), param.getValue()); | |
| addPartToMultiPartBuilder(mpBuilder, param.getKey(), item); |
| * @return The bearer token | ||
| */ | ||
| public String getBearerToken() { | ||
| return tokenSupplier.get(); |
There was a problem hiding this comment.
P2: When getBearerToken() is called before a token is configured, it throws a NullPointerException even though applyToParams supports an unset token. Return null when tokenSupplier is unset.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/auth/HttpBearerAuth.java, line 40:
<comment>When `getBearerToken()` is called before a token is configured, it throws a `NullPointerException` even though `applyToParams` supports an unset token. Return `null` when `tokenSupplier` is unset.</comment>
<file context>
@@ -0,0 +1,75 @@
+ * @return The bearer token
+ */
+ public String getBearerToken() {
+ return tokenSupplier.get();
+ }
+
</file context>
| return tokenSupplier.get(); | |
| return Optional.ofNullable(tokenSupplier).map(Supplier::get).orElse(null); |
| example = p.paramName + "_example"; | ||
| } | ||
| example = "\"" + escapeText(example) + "\""; | ||
| example = "\"" + escapeStringLiteral(example) + "\""; |
There was a problem hiding this comment.
P2: Custom Java generators that override escapeText no longer apply their escaping to String parameter examples. Preserve an overridable literal-escaping hook while omitting only the JavaDoc delimiter replacement.
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/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java, line 1807:
<comment>Custom Java generators that override `escapeText` no longer apply their escaping to String parameter examples. Preserve an overridable literal-escaping hook while omitting only the JavaDoc delimiter replacement.</comment>
<file context>
@@ -1803,7 +1804,7 @@ public void setParameterExampleValue(CodegenParameter p) {
example = p.paramName + "_example";
}
- example = "\"" + escapeText(example) + "\"";
+ example = "\"" + escapeStringLiteral(example) + "\"";
} else if ("Integer".equals(type) || "Short".equals(type)) {
if (example == null) {
</file context>
| * @return a {@link org.openapitools.client.ApiClient} object | ||
| */ | ||
| public ApiClient setDateFormat(DateFormat dateFormat) { | ||
| JSON.setDateFormat(dateFormat); |
There was a problem hiding this comment.
P3: setDateFormat leaves the field returned by getDateFormat unchanged, so the public getter does not reflect the configured format. Assign this.dateFormat before updating the JSON adapter.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/ApiClient.java, line 343:
<comment>`setDateFormat` leaves the field returned by `getDateFormat` unchanged, so the public getter does not reflect the configured format. Assign `this.dateFormat` before updating the JSON adapter.</comment>
<file context>
@@ -0,0 +1,1642 @@
+ * @return a {@link org.openapitools.client.ApiClient} object
+ */
+ public ApiClient setDateFormat(DateFormat dateFormat) {
+ JSON.setDateFormat(dateFormat);
+ return this;
+ }
</file context>
| JSON.setDateFormat(dateFormat); | |
| this.dateFormat = dateFormat; | |
| JSON.setDateFormat(dateFormat); |
| return builder; | ||
| } | ||
|
|
||
| private static String getDiscriminatorValue(JsonElement readElement, String discriminatorField) { |
There was a problem hiding this comment.
P3: This schema has no discriminator models, so these private discriminator helpers are dead code in the generated client and cannot affect deserialization. Emit them only when discriminator support is needed, or remove them from this fixture.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/JSON.java, line 72:
<comment>This schema has no discriminator models, so these private discriminator helpers are dead code in the generated client and cannot affect deserialization. Emit them only when discriminator support is needed, or remove them from this fixture.</comment>
<file context>
@@ -0,0 +1,482 @@
+ return builder;
+ }
+
+ private static String getDiscriminatorValue(JsonElement readElement, String discriminatorField) {
+ JsonElement element = readElement.getAsJsonObject().get(discriminatorField);
+ if (null == element) {
</file context>
| package org.openapitools.client; | ||
|
|
||
| @javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", comments = "Generator version: 7.26.0-SNAPSHOT") | ||
| public class Pair { |
There was a problem hiding this comment.
P3: This PR is scoped to the Go generator JSON-tag fix (#24916), but it adds a new Java client sample directory (wildcard-accept-header/java) that is unrelated to that change and unmentioned in the description. If this is not part of the intended change, remove it; if it is intentional, note it so reviewers don't have to guess.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/wildcard-accept-header/java/src/main/java/org/openapitools/client/Pair.java, line 17:
<comment>This PR is scoped to the Go generator JSON-tag fix (#24916), but it adds a new Java client sample directory (wildcard-accept-header/java) that is unrelated to that change and unmentioned in the description. If this is not part of the intended change, remove it; if it is intentional, note it so reviewers don't have to guess.</comment>
<file context>
@@ -0,0 +1,37 @@
+package org.openapitools.client;
+
+@javax.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", comments = "Generator version: 7.26.0-SNAPSHOT")
+public class Pair {
+ private final String name;
+ private final String value;
</file context>
| "jakarta.ws.rs" % "jakarta.ws.rs-api" % "2.1.6", | ||
| "org.openapitools" % "jackson-databind-nullable" % "0.2.11", | ||
| "io.gsonfire" % "gson-fire" % "1.9.0" % "compile", | ||
| "jakarta.annotation" % "jakarta.annotation-api" % "1.3.5" % "compile", |
There was a problem hiding this comment.
P3: The jakarta.annotation-api dependency is declared twice with identical group, artifact, version, and scope (lines 19 and 21). Remove the duplicate entry so the dependency list stays unambiguous and matches the pom.xml/build.gradle declarations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At samples/client/wildcard-accept-header/java/build.sbt, line 20:
<comment>The `jakarta.annotation-api` dependency is declared twice with identical group, artifact, version, and scope (lines 19 and 21). Remove the duplicate entry so the dependency list stays unambiguous and matches the pom.xml/build.gradle declarations.</comment>
<file context>
@@ -0,0 +1,27 @@
+ "jakarta.ws.rs" % "jakarta.ws.rs-api" % "2.1.6",
+ "org.openapitools" % "jackson-databind-nullable" % "0.2.11",
+ "io.gsonfire" % "gson-fire" % "1.9.0" % "compile",
+ "jakarta.annotation" % "jakarta.annotation-api" % "1.3.5" % "compile",
+ "com.google.code.findbugs" % "jsr305" % "3.0.2" % "compile",
+ "jakarta.annotation" % "jakarta.annotation-api" % "1.3.5" % "compile",
</file context>
| example = p.paramName + "_example"; | ||
| } | ||
| example = "\"" + escapeText(example) + "\""; | ||
| example = "\"" + escapeStringLiteral(example) + "\""; |
There was a problem hiding this comment.
P3: The behavior change in setParameterExampleValue() (String examples no longer get / and / escaped) has no regression test. The PR only adds EscapeJavaDocLambdaTest for the new lambda, but the String-example escaping change is a distinct code path and the existing AbstractJavaCodegenExampleValuesTest already exercises setParameterExampleValue() for String types. Add a case there (or in a new test) asserting that a String example/default like "/" now yields "/" (not "_/") to lock in the intended behavior.
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/java/org/openapitools/codegen/languages/AbstractJavaCodegen.java, line 1807:
<comment>The behavior change in setParameterExampleValue() (String examples no longer get */ and /* escaped) has no regression test. The PR only adds EscapeJavaDocLambdaTest for the new lambda, but the String-example escaping change is a distinct code path and the existing AbstractJavaCodegenExampleValuesTest already exercises setParameterExampleValue() for String types. Add a case there (or in a new test) asserting that a String example/default like "*/*" now yields "*/*" (not "*_/*") to lock in the intended behavior.</comment>
<file context>
@@ -1803,7 +1804,7 @@ public void setParameterExampleValue(CodegenParameter p) {
example = p.paramName + "_example";
}
- example = "\"" + escapeText(example) + "\"";
+ example = "\"" + escapeStringLiteral(example) + "\"";
} else if ("Integer".equals(type) || "Short".equals(type)) {
if (example == null) {
</file context>
Fixes OpenAPITools#24916. The Go generator could omit json struct tags when a schema contained both normal properties and a schema-level oneOf/anyOf. The issue occurs because model property processing could select inherited properties from composed schemas instead of the model's own properties. For validation-only oneOf constraints, those inherited properties can be empty, causing generated fields to lose their JSON tags. This change ensures that existing model properties retain their generated JSON tags even when schema-level composition is present, while preserving the behavior of pure oneOf models.
8efdcc5 to
876ed03
Compare
Description
Fixes #24916.
The Go generator could omit
jsonstruct tags when a schema contained bothnormal properties and a schema-level
oneOf/anyOf.The issue occurs because model property processing could select inherited
properties from composed schemas instead of the model's own properties.
For validation-only
oneOfconstraints, those inherited properties can beempty, causing generated fields to lose their JSON tags.
This change ensures that existing model properties retain their generated
JSON tags even when schema-level composition is present, while preserving
the behavior of pure
oneOfmodels.Tests
schema-level
oneOf