Skip to content

Commit 1f57b0e

Browse files
wiebrenclaude
andauthored
[java] fix: okhttp-gson additionalProperties field makes every allOf child undeserializable (#24895)
* [java] fix: the okhttp-gson additionalProperties bag breaks every allOf child With disallowAdditionalPropertiesIfNotPresent=false every model declares private Map<String, Object> additionalProperties - so an allOf child declares the field twice, once itself and once inherited, and gson refuses the class before the model's own TypeAdapterFactory can run: IllegalArgumentException: Class Child declares multiple JSON fields named 'additionalProperties' Every allOf child model is undeserializable (and unserializable). Mark the field transient: the custom TypeAdapterFactory already reads and writes the bag explicitly around the reflective delegate - on write it even removes the additionalProperties key the delegate emitted - so hiding it from gson's reflection changes nothing for flat models and unbreaks the children. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcwZ1arjLZNpetHz2a3TJz * [java] fix: keep the bag visible on parents that have no adapter of their own Review caught that models with children get no CustomTypeAdapterFactory ({{^hasChildren}} in pojo.mustache), so a transient field on such a parent silently dropped its bag - reflection was its only carrier. The transient modifier is now gated the same way the factory is: parents with children keep the field visible (their reflective round-trip is unchanged), and an allOf child's transient declaration shadows it, so the duplicate-field conflict still cannot arise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcwZ1arjLZNpetHz2a3TJz * [java] docs: describe gson's field binding accurately Gson does not use Java field-hiding semantics: it collects the declared fields of each class in the hierarchy and refuses two bound to one JSON name, so the child's transient declaration excludes its own copy rather than hiding the parent's. Same behaviour, correct description - the comment ships into every generated model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GcwZ1arjLZNpetHz2a3TJz * [java] fix: declare the okhttp-gson bag once per allOf hierarchy Review caught that a three-level allOf chain (Root <- Middle <- Leaf) still broke: only models without children marked their copy transient, so Root and Middle - both parents - each kept a reflectively bound additionalProperties field, and gson refused Middle and Leaf alike ("declares multiple JSON fields named 'additionalProperties'"). Marking Middle's copy transient as well would not do: a parent has no TypeAdapterFactory, so reflection would fill Root's field while Middle's accessors read its own, dropping the bag. Instead the field is now declared exactly once, on the topmost ancestor that has one; JavaClientCodegen tags every descendant with x-inherits-additional-properties, and those models inherit the field and its getters, override putAdditionalProperty only to keep the covariant return type, and leave the bag to super.equals, super.hashCode and super.toString. Adds a multi-level regression test (fails without this change) and regenerates the okhttp-gson samples. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [java] fix: round-trip a literal additionalProperties key on allOf children Review caught that a descendant inherits the bag from a class that has children, so the field is not transient and gson's reflective delegate binds it. An undeclared property literally named `additionalProperties` therefore landed in the bag twice: the delegate stored it under its own name, and the descendant's adapter then read the same key from the raw JSON and flattened its entries into the bag. Writing back emitted those entries as top-level properties, so {"className":"Dog","breed":"b","additionalProperties":{"x":1},"extra":"e"} came out with a phantom top-level "x":1.0. The adapter now clears the delegate-bound bag right after fromJsonTree, before the loop that collects the undeclared keys. That loop re-reads every one of them from the raw JSON, the literal key included, so nothing is lost and it round-trips as a single entry. The clear is emitted only for models tagged x-inherits-additional-properties: a model that declares the bag itself keeps it transient, so the delegate binds nothing into it and there is nothing to clear. Extends the codegen tests with the emitted clear and adds a runtime round-trip to the okhttp-gson sample's hand-written JSONTest. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * test: [java] record the new checksum of the okhttp-gson JSONTest Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * [java] fix: hide a literal additionalProperties key from the okhttp-gson delegate On an allOf child the inherited bag is visible to gson's reflection, so the delegate adapter tried to bind a literal `additionalProperties` key into the Map-typed field and threw on a primitive or array value. The adapter now hands the delegate a copy without that key (only when present) and the extras loop collects it from the raw JSON like any other undeclared property. The delegate no longer binds anything into the bag, so the clear() is dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [java] refactor: drop transient and the literal-key handling from the okhttp-gson bag Declaring the additionalProperties field once per hierarchy already removes the duplicate that gson rejects, so `transient` fixed nothing: it only changed flat models, which were never broken (and dropped the bag from Java serialization under serializableModel). Flat models are byte-identical to master again. The delegate copy that hid a literal `additionalProperties` JSON key from the reflective adapter is removed too. Every okhttp-gson model on master binds that key through the delegate, so it is a pre-existing gap for a separate fix, not part of this one. allOf descendants now behave exactly like flat models for it. The codegen tests assert only the invariant: one class per hierarchy declares the bag, descendants override putAdditionalProperty. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test: [java] round-trip an allOf child's additional properties in the okhttp-gson JSONTest Replaces the two literal-`additionalProperties`-key cases with a plain Dog round-trip, the case that threw on master, and records the new checksum. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent e0f8a28 commit 1f57b0e

12 files changed

Lines changed: 165 additions & 187 deletions

File tree

‎bin/utils/test_file_list.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
- filename: "samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/ClientTest.java"
1111
sha256: 325fdd5d7e2c97790c0fb44f712ab7b2ba022d7e1a5b0056f47b07f342682b6d
1212
- filename: "samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/JSONTest.java"
13-
sha256: 08b089934586d0b1564e9e5940c3dc4f3ef0b79d191ba91d36ee2f07356eab38
13+
sha256: ddd53d2f640aee7712299f1540779cdb508f0b9c1e96082191686e218eadb04c
1414
- filename: "samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/api/PetApiTest.java"
1515
sha256: 8b1b8f2a2ad00ccb090873a94a5f73e328b98317d2ec715f53bd7a1accb2a023
1616
- filename: "samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/model/PetTest.java"

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,6 +1233,25 @@ public CodegenModel fromModel(String name, Schema model) {
12331233
return codegenModel;
12341234
}
12351235

1236+
@Override
1237+
public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs) {
1238+
objs = super.postProcessAllModels(objs);
1239+
if (isLibrary(OKHTTP_GSON)) {
1240+
// gson refuses a class whose hierarchy declares two fields bound to one JSON name, so
1241+
// the okhttp-gson additionalProperties bag is declared once, on the topmost ancestor
1242+
// that has one; every descendant inherits it instead of declaring its own copy
1243+
for (CodegenModel cm : getAllModels(objs).values()) {
1244+
for (CodegenModel ancestor = cm.getParentModel(); ancestor != null; ancestor = ancestor.getParentModel()) {
1245+
if (ancestor.isAdditionalPropertiesTrue) {
1246+
cm.vendorExtensions.put("x-inherits-additional-properties", true);
1247+
break;
1248+
}
1249+
}
1250+
}
1251+
}
1252+
return objs;
1253+
}
1254+
12361255
@Override
12371256
public ModelsMap postProcessModelsEnum(ModelsMap objs) {
12381257
objs = super.postProcessModelsEnum(objs);

‎modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/additional_properties.mustache‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
{{#isAdditionalPropertiesTrue}}
2+
{{^vendorExtensions.x-inherits-additional-properties}}
23
/**
34
* A container for additional, undeclared properties.
45
* This is a holder for any undeclared properties as specified with
@@ -43,4 +44,21 @@
4344
}
4445
return this.additionalProperties.get(key);
4546
}
47+
{{/vendorExtensions.x-inherits-additional-properties}}
48+
{{#vendorExtensions.x-inherits-additional-properties}}
49+
/**
50+
* Set the additional (undeclared) property with the specified name and value.
51+
* If the property does not already exist, create it otherwise replace it.
52+
* The bag is inherited from the superclass.
53+
*
54+
* @param key name of the property
55+
* @param value value of the property
56+
* @return the {{classname}} instance itself
57+
*/
58+
@Override
59+
public {{classname}} putAdditionalProperty(String key, Object value) {
60+
super.putAdditionalProperty(key, value);
61+
return this;
62+
}
63+
{{/vendorExtensions.x-inherits-additional-properties}}
4664
{{/isAdditionalPropertiesTrue}}

‎modules/openapi-generator/src/main/resources/Java/libraries/okhttp-gson/pojo.mustache‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,8 +206,8 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
206206
}{{#hasVars}}
207207
{{classname}} {{classVarName}} = ({{classname}}) o;
208208
return {{#vars}}{{#isByteArray}}Arrays{{/isByteArray}}{{^isByteArray}}Objects{{/isByteArray}}.equals(this.{{name}}, {{classVarName}}.{{name}}){{^-last}} &&
209-
{{/-last}}{{/vars}}{{#isAdditionalPropertiesTrue}}&&
210-
Objects.equals(this.additionalProperties, {{classVarName}}.additionalProperties){{/isAdditionalPropertiesTrue}}{{#parent}} &&
209+
{{/-last}}{{/vars}}{{#isAdditionalPropertiesTrue}}{{^vendorExtensions.x-inherits-additional-properties}}&&
210+
Objects.equals(this.additionalProperties, {{classVarName}}.additionalProperties){{/vendorExtensions.x-inherits-additional-properties}}{{/isAdditionalPropertiesTrue}}{{#parent}} &&
211211
super.equals(o){{/parent}};{{/hasVars}}{{^hasVars}}
212212
return {{#parent}}super.equals(o){{/parent}}{{^parent}}true{{/parent}};{{/hasVars}}
213213
{{/useReflectionEqualsHashCode}}
@@ -223,7 +223,7 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
223223
return HashCodeBuilder.reflectionHashCode(this);
224224
{{/useReflectionEqualsHashCode}}
225225
{{^useReflectionEqualsHashCode}}
226-
return Objects.hash({{#vars}}{{^isByteArray}}{{name}}{{/isByteArray}}{{#isByteArray}}Arrays.hashCode({{name}}){{/isByteArray}}{{^-last}}, {{/-last}}{{/vars}}{{#parent}}{{#hasVars}}, {{/hasVars}}super.hashCode(){{/parent}}{{#isAdditionalPropertiesTrue}}{{#hasVars}}, {{/hasVars}}{{^hasVars}}{{#parent}}, {{/parent}}{{/hasVars}}additionalProperties{{/isAdditionalPropertiesTrue}});
226+
return Objects.hash({{#vars}}{{^isByteArray}}{{name}}{{/isByteArray}}{{#isByteArray}}Arrays.hashCode({{name}}){{/isByteArray}}{{^-last}}, {{/-last}}{{/vars}}{{#parent}}{{#hasVars}}, {{/hasVars}}super.hashCode(){{/parent}}{{#isAdditionalPropertiesTrue}}{{^vendorExtensions.x-inherits-additional-properties}}{{#hasVars}}, {{/hasVars}}{{^hasVars}}{{#parent}}, {{/parent}}{{/hasVars}}additionalProperties{{/vendorExtensions.x-inherits-additional-properties}}{{/isAdditionalPropertiesTrue}});
227227
{{/useReflectionEqualsHashCode}}
228228
}{{#vendorExtensions.x-jackson-optional-nullable-helpers}}
229229

@@ -245,7 +245,9 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
245245
sb.append(" {{name}}: ").append({{#isPassword}}"*"{{/isPassword}}{{^isPassword}}toIndentedString({{name}}){{/isPassword}}).append("\n");
246246
{{/vars}}
247247
{{#isAdditionalPropertiesTrue}}
248+
{{^vendorExtensions.x-inherits-additional-properties}}
248249
sb.append(" additionalProperties: ").append(toIndentedString(additionalProperties)).append("\n");
250+
{{/vendorExtensions.x-inherits-additional-properties}}
249251
{{/isAdditionalPropertiesTrue}}
250252
sb.append("}");
251253
return sb.toString();

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2227,6 +2227,61 @@ public void testJdkHttpClientWithAndWithoutParentExtension() {
22272227
.content().contains("public class AnotherChild {");
22282228
}
22292229

2230+
@Test
2231+
public void testAdditionalPropertiesFieldIsDeclaredOncePerHierarchyForGson() {
2232+
final Path output = generateOkHttpGsonWithAdditionalProperties("src/test/resources/3_0/allOf_extension_parent.yaml");
2233+
2234+
// exactly one class per hierarchy declares the bag; descendants inherit it
2235+
assertThat(output.resolve("src/main/java/xyz/abcdef/model/Person.java"))
2236+
.content().contains("private Map<String, Object> additionalProperties;");
2237+
assertThat(output.resolve("src/main/java/xyz/abcdef/model/Child.java"))
2238+
.content()
2239+
.contains("public class Child extends Person {")
2240+
.contains("public Child putAdditionalProperty(String key, Object value) {")
2241+
.doesNotContain("Map<String, Object> additionalProperties;");
2242+
// a model without an allOf parent declares its own bag, as before
2243+
assertThat(output.resolve("src/main/java/xyz/abcdef/model/PersonA.java"))
2244+
.content().contains("private Map<String, Object> additionalProperties;");
2245+
}
2246+
2247+
@Test
2248+
public void testAdditionalPropertiesFieldIsDeclaredOnceAcrossMultiLevelAllOfForGson() {
2249+
final Path output = generateOkHttpGsonWithAdditionalProperties("src/test/resources/3_0/java/okhttp-gson-additional-properties-allof-chain.yaml");
2250+
2251+
// exactly one class per hierarchy declares the bag, also across Root <- Middle <- Leaf
2252+
assertThat(output.resolve("src/main/java/xyz/abcdef/model/Root.java"))
2253+
.content().contains("private Map<String, Object> additionalProperties;");
2254+
assertThat(output.resolve("src/main/java/xyz/abcdef/model/Middle.java"))
2255+
.content()
2256+
.contains("public class Middle extends Root {")
2257+
.contains("public Middle putAdditionalProperty(String key, Object value) {")
2258+
.doesNotContain("Map<String, Object> additionalProperties;");
2259+
assertThat(output.resolve("src/main/java/xyz/abcdef/model/Leaf.java"))
2260+
.content()
2261+
.contains("public class Leaf extends Middle {")
2262+
.contains("public Leaf putAdditionalProperty(String key, Object value) {")
2263+
.doesNotContain("Map<String, Object> additionalProperties;");
2264+
}
2265+
2266+
private Path generateOkHttpGsonWithAdditionalProperties(String inputSpec) {
2267+
final Path output = newTempFolder();
2268+
final CodegenConfigurator configurator = new CodegenConfigurator()
2269+
.setGeneratorName(JAVA_GENERATOR)
2270+
// use default `okhttp-gson`
2271+
.addAdditionalProperty(CodegenConstants.API_PACKAGE, "xyz.abcdef.api")
2272+
.addAdditionalProperty(CodegenConstants.MODEL_PACKAGE, "xyz.abcdef.model")
2273+
.addAdditionalProperty(CodegenConstants.INVOKER_PACKAGE, "xyz.abcdef.invoker")
2274+
.addAdditionalProperty("disallowAdditionalPropertiesIfNotPresent", "false")
2275+
.setInputSpec(inputSpec)
2276+
.setOutputDir(output.toString().replace("\\", "/"));
2277+
2278+
DefaultGenerator generator = new DefaultGenerator();
2279+
generator.setGeneratorPropertyDefault(CodegenConstants.MODELS, "true");
2280+
List<File> files = generator.opts(configurator.toClientOptInput()).generate();
2281+
validateJavaSourceFiles(files);
2282+
return output;
2283+
}
2284+
22302285
@Test
22312286
public void allOfWithSeveralRefsAndRefAsParentInAllOfNormalizationIsTrue() {
22322287
final Path output = newTempFolder();
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
openapi: 3.0.3
2+
info: {title: t, version: '1'}
3+
paths:
4+
/c:
5+
get:
6+
responses:
7+
'200':
8+
description: ok
9+
content:
10+
application/json:
11+
schema: {$ref: '#/components/schemas/Leaf'}
12+
components:
13+
schemas:
14+
Root:
15+
type: object
16+
required: [kind]
17+
discriminator: {propertyName: kind}
18+
properties:
19+
kind: {type: string}
20+
a: {type: string}
21+
Middle:
22+
allOf:
23+
- $ref: '#/components/schemas/Root'
24+
- type: object
25+
properties:
26+
b: {type: string}
27+
discriminator: {propertyName: kind}
28+
Leaf:
29+
allOf:
30+
- $ref: '#/components/schemas/Middle'
31+
- type: object
32+
properties:
33+
c: {type: string}

‎samples/client/petstore/java/okhttp-gson-3.1/src/main/java/org/openapitools/client/model/Cat.java‎

Lines changed: 5 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -79,51 +79,21 @@ public void setDeclawed(@javax.annotation.Nullable Boolean declawed) {
7979
this.declawed = declawed;
8080
}
8181

82-
/**
83-
* A container for additional, undeclared properties.
84-
* This is a holder for any undeclared properties as specified with
85-
* the 'additionalProperties' keyword in the OAS document.
86-
*/
87-
private Map<String, Object> additionalProperties;
88-
8982
/**
9083
* Set the additional (undeclared) property with the specified name and value.
9184
* If the property does not already exist, create it otherwise replace it.
85+
* The bag is inherited from the superclass.
9286
*
9387
* @param key name of the property
9488
* @param value value of the property
9589
* @return the Cat instance itself
9690
*/
91+
@Override
9792
public Cat putAdditionalProperty(String key, Object value) {
98-
if (this.additionalProperties == null) {
99-
this.additionalProperties = new HashMap<String, Object>();
100-
}
101-
this.additionalProperties.put(key, value);
93+
super.putAdditionalProperty(key, value);
10294
return this;
10395
}
10496

105-
/**
106-
* Return the additional (undeclared) property.
107-
*
108-
* @return a map of objects
109-
*/
110-
public Map<String, Object> getAdditionalProperties() {
111-
return additionalProperties;
112-
}
113-
114-
/**
115-
* Return the additional (undeclared) property with the specified name.
116-
*
117-
* @param key name of the property
118-
* @return an object
119-
*/
120-
public Object getAdditionalProperty(String key) {
121-
if (this.additionalProperties == null) {
122-
return null;
123-
}
124-
return this.additionalProperties.get(key);
125-
}
126-
12797

12898
@Override
12999
public boolean equals(Object o) {
@@ -134,14 +104,13 @@ public boolean equals(Object o) {
134104
return false;
135105
}
136106
Cat cat = (Cat) o;
137-
return Objects.equals(this.declawed, cat.declawed)&&
138-
Objects.equals(this.additionalProperties, cat.additionalProperties) &&
107+
return Objects.equals(this.declawed, cat.declawed) &&
139108
super.equals(o);
140109
}
141110

142111
@Override
143112
public int hashCode() {
144-
return Objects.hash(declawed, super.hashCode(), additionalProperties);
113+
return Objects.hash(declawed, super.hashCode());
145114
}
146115

147116
@Override
@@ -150,7 +119,6 @@ public String toString() {
150119
sb.append("class Cat {\n");
151120
sb.append(" ").append(toIndentedString(super.toString())).append("\n");
152121
sb.append(" declawed: ").append(toIndentedString(declawed)).append("\n");
153-
sb.append(" additionalProperties: ").append(toIndentedString(additionalProperties)).append("\n");
154122
sb.append("}");
155123
return sb.toString();
156124
}

‎samples/client/petstore/java/okhttp-gson-3.1/src/main/java/org/openapitools/client/model/Dog.java‎

Lines changed: 5 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -79,51 +79,21 @@ public void setBreed(@javax.annotation.Nullable String breed) {
7979
this.breed = breed;
8080
}
8181

82-
/**
83-
* A container for additional, undeclared properties.
84-
* This is a holder for any undeclared properties as specified with
85-
* the 'additionalProperties' keyword in the OAS document.
86-
*/
87-
private Map<String, Object> additionalProperties;
88-
8982
/**
9083
* Set the additional (undeclared) property with the specified name and value.
9184
* If the property does not already exist, create it otherwise replace it.
85+
* The bag is inherited from the superclass.
9286
*
9387
* @param key name of the property
9488
* @param value value of the property
9589
* @return the Dog instance itself
9690
*/
91+
@Override
9792
public Dog putAdditionalProperty(String key, Object value) {
98-
if (this.additionalProperties == null) {
99-
this.additionalProperties = new HashMap<String, Object>();
100-
}
101-
this.additionalProperties.put(key, value);
93+
super.putAdditionalProperty(key, value);
10294
return this;
10395
}
10496

105-
/**
106-
* Return the additional (undeclared) property.
107-
*
108-
* @return a map of objects
109-
*/
110-
public Map<String, Object> getAdditionalProperties() {
111-
return additionalProperties;
112-
}
113-
114-
/**
115-
* Return the additional (undeclared) property with the specified name.
116-
*
117-
* @param key name of the property
118-
* @return an object
119-
*/
120-
public Object getAdditionalProperty(String key) {
121-
if (this.additionalProperties == null) {
122-
return null;
123-
}
124-
return this.additionalProperties.get(key);
125-
}
126-
12797

12898
@Override
12999
public boolean equals(Object o) {
@@ -134,14 +104,13 @@ public boolean equals(Object o) {
134104
return false;
135105
}
136106
Dog dog = (Dog) o;
137-
return Objects.equals(this.breed, dog.breed)&&
138-
Objects.equals(this.additionalProperties, dog.additionalProperties) &&
107+
return Objects.equals(this.breed, dog.breed) &&
139108
super.equals(o);
140109
}
141110

142111
@Override
143112
public int hashCode() {
144-
return Objects.hash(breed, super.hashCode(), additionalProperties);
113+
return Objects.hash(breed, super.hashCode());
145114
}
146115

147116
@Override
@@ -150,7 +119,6 @@ public String toString() {
150119
sb.append("class Dog {\n");
151120
sb.append(" ").append(toIndentedString(super.toString())).append("\n");
152121
sb.append(" breed: ").append(toIndentedString(breed)).append("\n");
153-
sb.append(" additionalProperties: ").append(toIndentedString(additionalProperties)).append("\n");
154122
sb.append("}");
155123
return sb.toString();
156124
}

0 commit comments

Comments
 (0)