[java] fix: okhttp-gson additionalProperties field makes every allOf child undeserializable

bin/utils/test_file_list.yaml

@@ -10,7 +10,7 @@
 - filename: "samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/ClientTest.java"
   sha256: 325fdd5d7e2c97790c0fb44f712ab7b2ba022d7e1a5b0056f47b07f342682b6d
 - filename: "samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/JSONTest.java"
-  sha256: 08b089934586d0b1564e9e5940c3dc4f3ef0b79d191ba91d36ee2f07356eab38
+  sha256: 4c4ed44007cae9e981b77365da4d8a9652d7ce6c8ba2678fba10c79d6dee5b21
 - filename: "samples/client/petstore/java/okhttp-gson/src/test/java/org/openapitools/client/api/PetApiTest.java"
   sha256: 8b1b8f2a2ad00ccb090873a94a5f73e328b98317d2ec715f53bd7a1accb2a023
 - 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

@@ -1207,6 +1207,25 @@ public CodegenModel fromModel(String name, Schema model) {
         return codegenModel;
     }
 
+    @Override
+    public Map<String, ModelsMap> postProcessAllModels(Map<String, ModelsMap> objs) {
+        objs = super.postProcessAllModels(objs);
+        if (isLibrary(OKHTTP_GSON)) {
+            // gson refuses a class whose hierarchy declares two fields bound to one JSON name, so
+            // the okhttp-gson additionalProperties bag is declared once, on the topmost ancestor
+            // that has one; every descendant inherits it instead of declaring its own copy
+            for (CodegenModel cm : getAllModels(objs).values()) {
+                for (CodegenModel ancestor = cm.getParentModel(); ancestor != null; ancestor = ancestor.getParentModel()) {
+                    if (ancestor.isAdditionalPropertiesTrue) {
+                        cm.vendorExtensions.put("x-inherits-additional-properties", true);
+                        break;
+                    }
+                }
+            }
+        }
+        return objs;
+    }
+
     @Override
     public ModelsMap postProcessModelsEnum(ModelsMap objs) {
         objs = super.postProcessModelsEnum(objs);

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

@@ -1,10 +1,18 @@
 {{#isAdditionalPropertiesTrue}}
+{{^vendorExtensions.x-inherits-additional-properties}}
   /**
    * A container for additional, undeclared properties.
    * This is a holder for any undeclared properties as specified with
    * the 'additionalProperties' keyword in the OAS document.
+   *
+   * Declared once per hierarchy: gson collects the declared fields of every class in the
+   * hierarchy and rejects two bound to one JSON name, so allOf descendants inherit this
+   * field instead of declaring their own. Transient on models without children: the bag is
+   * read and written by this model's TypeAdapterFactory, so gson's reflection does not need
+   * to see it. A parent with children has no factory of its own, so it keeps the field
+   * bound for reflection.
    */
-  private Map<String, Object> additionalProperties;
+  private {{^hasChildren}}transient {{/hasChildren}}Map<String, Object> additionalProperties;
 
   /**
    * Set the additional (undeclared) property with the specified name and value.
@@ -43,4 +51,21 @@
     }
     return this.additionalProperties.get(key);
   }
+{{/vendorExtensions.x-inherits-additional-properties}}
+{{#vendorExtensions.x-inherits-additional-properties}}
+  /**
+   * Set the additional (undeclared) property with the specified name and value.
+   * If the property does not already exist, create it otherwise replace it.
+   * The bag itself is inherited: it is declared once, on the root of the hierarchy.
+   *
+   * @param key name of the property
+   * @param value value of the property
+   * @return the {{classname}} instance itself
+   */
+  @Override
+  public {{classname}} putAdditionalProperty(String key, Object value) {
+    super.putAdditionalProperty(key, value);
+    return this;
+  }
+{{/vendorExtensions.x-inherits-additional-properties}}
 {{/isAdditionalPropertiesTrue}}

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

@@ -206,8 +206,8 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
     }{{#hasVars}}
     {{classname}} {{classVarName}} = ({{classname}}) o;
     return {{#vars}}{{#isByteArray}}Arrays{{/isByteArray}}{{^isByteArray}}Objects{{/isByteArray}}.equals(this.{{name}}, {{classVarName}}.{{name}}){{^-last}} &&
-        {{/-last}}{{/vars}}{{#isAdditionalPropertiesTrue}}&&
-        Objects.equals(this.additionalProperties, {{classVarName}}.additionalProperties){{/isAdditionalPropertiesTrue}}{{#parent}} &&
+        {{/-last}}{{/vars}}{{#isAdditionalPropertiesTrue}}{{^vendorExtensions.x-inherits-additional-properties}}&&
+        Objects.equals(this.additionalProperties, {{classVarName}}.additionalProperties){{/vendorExtensions.x-inherits-additional-properties}}{{/isAdditionalPropertiesTrue}}{{#parent}} &&
         super.equals(o){{/parent}};{{/hasVars}}{{^hasVars}}
     return {{#parent}}super.equals(o){{/parent}}{{^parent}}true{{/parent}};{{/hasVars}}
   {{/useReflectionEqualsHashCode}}
@@ -223,7 +223,7 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
     return HashCodeBuilder.reflectionHashCode(this);
   {{/useReflectionEqualsHashCode}}
   {{^useReflectionEqualsHashCode}}
-    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}});
+    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}});
   {{/useReflectionEqualsHashCode}}
   }{{#vendorExtensions.x-jackson-optional-nullable-helpers}}
 
@@ -245,7 +245,9 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
     sb.append("    {{name}}: ").append({{#isPassword}}"*"{{/isPassword}}{{^isPassword}}toIndentedString({{name}}){{/isPassword}}).append("\n");
     {{/vars}}
 {{#isAdditionalPropertiesTrue}}
+{{^vendorExtensions.x-inherits-additional-properties}}
     sb.append("    additionalProperties: ").append(toIndentedString(additionalProperties)).append("\n");
+{{/vendorExtensions.x-inherits-additional-properties}}
 {{/isAdditionalPropertiesTrue}}
     sb.append("}");
     return sb.toString();
@@ -563,6 +565,14 @@ public class {{classname}} {{#parent}}extends {{{.}}} {{/parent}}{{#vendorExtens
              JsonObject jsonObj = jsonElement.getAsJsonObject();
              // store additional fields in the deserialized instance
              {{classname}} instance = thisAdapter.fromJsonTree(jsonObj);
+             {{#vendorExtensions.x-inherits-additional-properties}}
+             // the inherited bag is visible to reflection, so the delegate above bound a
+             // literal `additionalProperties` key into it; drop that and let the loop below
+             // collect every undeclared key from the raw JSON, this one included
+             if (instance.getAdditionalProperties() != null) {
+               instance.getAdditionalProperties().clear();
+             }
+             {{/vendorExtensions.x-inherits-additional-properties}}
              for (Map.Entry<String, JsonElement> entry : jsonObj.entrySet()) {
                if (!openapiFields.contains(entry.getKey())) {
                  if (entry.getValue().isJsonPrimitive()) { // primitive type

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

@@ -2024,6 +2024,79 @@ public void testJdkHttpClientWithAndWithoutParentExtension() {
                 .content().contains("public class AnotherChild {");
     }
 
+    @Test
+    public void testAdditionalPropertiesFieldIsDeclaredOncePerHierarchyForGson() {
+        final Path output = generateOkHttpGsonWithAdditionalProperties("src/test/resources/3_0/allOf_extension_parent.yaml");
+
+        // gson's reflective adapter refuses a class whose hierarchy declares two fields bound to
+        // one JSON name ("declares multiple JSON fields named 'additionalProperties'"), so an
+        // allOf child inherits the bag instead of declaring its own copy. A parent with
+        // children gets no TypeAdapterFactory of its own ({{^hasChildren}} in pojo.mustache), so
+        // its field stays visible to reflection.
+        assertThat(output.resolve("src/main/java/xyz/abcdef/model/Person.java"))
+                .content()
+                .contains("private Map<String, Object> additionalProperties;")
+                .doesNotContain("private transient Map<String, Object> additionalProperties;");
+        // the inherited field stays visible to reflection, so the delegate adapter binds a
+        // literal `additionalProperties` key into it; the bag is cleared before the extras
+        // loop re-collects that key from the raw JSON, or it would be stored twice
+        assertThat(output.resolve("src/main/java/xyz/abcdef/model/Child.java"))
+                .content()
+                .contains("public class Child extends Person {")
+                .contains("public Child putAdditionalProperty(String key, Object value) {")
+                .contains("Child instance = thisAdapter.fromJsonTree(jsonObj);")
+                .contains("if (instance.getAdditionalProperties() != null) {")
+                .contains("instance.getAdditionalProperties().clear();")
+                .doesNotContain("Map<String, Object> additionalProperties;");
+        // a model that declares the bag itself keeps it transient, so the delegate binds
+        // nothing into it and there is nothing to clear
+        assertThat(output.resolve("src/main/java/xyz/abcdef/model/PersonA.java"))
+                .content()
+                .contains("private transient Map<String, Object> additionalProperties;")
+                .contains("PersonA instance = thisAdapter.fromJsonTree(jsonObj);")
+                .doesNotContain("getAdditionalProperties().clear();");
+    }
+
+    @Test
+    public void testAdditionalPropertiesFieldIsDeclaredOnceAcrossMultiLevelAllOfForGson() {
+        final Path output = generateOkHttpGsonWithAdditionalProperties("src/test/resources/3_0/java/okhttp-gson-additional-properties-allof-chain.yaml");
+
+        // Root <- Middle <- Leaf: Middle has children too, so hiding only the leaf's copy would
+        // still leave Root's and Middle's bound together - exactly one class may declare the bag
+        assertThat(output.resolve("src/main/java/xyz/abcdef/model/Root.java"))
+                .content().contains("private Map<String, Object> additionalProperties;");
+        assertThat(output.resolve("src/main/java/xyz/abcdef/model/Middle.java"))
+                .content()
+                .contains("public class Middle extends Root {")
+                .doesNotContain("Map<String, Object> additionalProperties;");
+        assertThat(output.resolve("src/main/java/xyz/abcdef/model/Leaf.java"))
+                .content()
+                .contains("public class Leaf extends Middle {")
+                .contains("public Leaf putAdditionalProperty(String key, Object value) {")
+                .contains("Leaf instance = thisAdapter.fromJsonTree(jsonObj);")
+                .contains("instance.getAdditionalProperties().clear();")
+                .doesNotContain("Map<String, Object> additionalProperties;");
+    }
+
+    private Path generateOkHttpGsonWithAdditionalProperties(String inputSpec) {
+        final Path output = newTempFolder();
+        final CodegenConfigurator configurator = new CodegenConfigurator()
+                .setGeneratorName(JAVA_GENERATOR)
+                // use default `okhttp-gson`
+                .addAdditionalProperty(CodegenConstants.API_PACKAGE, "xyz.abcdef.api")
+                .addAdditionalProperty(CodegenConstants.MODEL_PACKAGE, "xyz.abcdef.model")
+                .addAdditionalProperty(CodegenConstants.INVOKER_PACKAGE, "xyz.abcdef.invoker")
+                .addAdditionalProperty("disallowAdditionalPropertiesIfNotPresent", "false")
+                .setInputSpec(inputSpec)
+                .setOutputDir(output.toString().replace("\\", "/"));
+
+        DefaultGenerator generator = new DefaultGenerator();
+        generator.setGeneratorPropertyDefault(CodegenConstants.MODELS, "true");
+        List<File> files = generator.opts(configurator.toClientOptInput()).generate();
+        validateJavaSourceFiles(files);
+        return output;
+    }
+
     @Test
     public void allOfWithSeveralRefsAndRefAsParentInAllOfNormalizationIsTrue() {
         final Path output = newTempFolder();

modules/openapi-generator/src/test/resources/3_0/java/okhttp-gson-additional-properties-allof-chain.yaml

@@ -0,0 +1,33 @@
+openapi: 3.0.3
+info: {title: t, version: '1'}
+paths:
+  /c:
+    get:
+      responses:
+        '200':
+          description: ok
+          content:
+            application/json:
+              schema: {$ref: '#/components/schemas/Leaf'}
+components:
+  schemas:
+    Root:
+      type: object
+      required: [kind]
+      discriminator: {propertyName: kind}
+      properties:
+        kind: {type: string}
+        a: {type: string}
+    Middle:
+      allOf:
+        - $ref: '#/components/schemas/Root'
+        - type: object
+          properties:
+            b: {type: string}
+      discriminator: {propertyName: kind}
+    Leaf:
+      allOf:
+        - $ref: '#/components/schemas/Middle'
+        - type: object
+          properties:
+            c: {type: string}

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

@@ -91,8 +91,15 @@ public void setArrayOfStrings(@javax.annotation.Nonnull List<String> arrayOfStri
    * A container for additional, undeclared properties.
    * This is a holder for any undeclared properties as specified with
    * the 'additionalProperties' keyword in the OAS document.
+   *
+   * Declared once per hierarchy: gson collects the declared fields of every class in the
+   * hierarchy and rejects two bound to one JSON name, so allOf descendants inherit this
+   * field instead of declaring their own. Transient on models without children: the bag is
+   * read and written by this model's TypeAdapterFactory, so gson's reflection does not need
+   * to see it. A parent with children has no factory of its own, so it keeps the field
+   * bound for reflection.
    */
-  private Map<String, Object> additionalProperties;
+  private transient Map<String, Object> additionalProperties;
 
   /**
    * Set the additional (undeclared) property with the specified name and value.

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

@@ -106,6 +106,13 @@ public void setColor(@javax.annotation.Nullable String color) {
    * A container for additional, undeclared properties.
    * This is a holder for any undeclared properties as specified with
    * the 'additionalProperties' keyword in the OAS document.
+   *
+   * Declared once per hierarchy: gson collects the declared fields of every class in the
+   * hierarchy and rejects two bound to one JSON name, so allOf descendants inherit this
+   * field instead of declaring their own. Transient on models without children: the bag is
+   * read and written by this model's TypeAdapterFactory, so gson's reflection does not need
+   * to see it. A parent with children has no factory of its own, so it keeps the field
+   * bound for reflection.
    */
   private Map<String, Object> additionalProperties;
 

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

@@ -148,8 +148,15 @@ public void setRefArrayPrefixItems(@javax.annotation.Nullable List<Object> refAr
    * A container for additional, undeclared properties.
    * This is a holder for any undeclared properties as specified with
    * the 'additionalProperties' keyword in the OAS document.
+   *
+   * Declared once per hierarchy: gson collects the declared fields of every class in the
+   * hierarchy and rejects two bound to one JSON name, so allOf descendants inherit this
+   * field instead of declaring their own. Transient on models without children: the bag is
+   * read and written by this model's TypeAdapterFactory, so gson's reflection does not need
+   * to see it. A parent with children has no factory of its own, so it keeps the field
+   * bound for reflection.
    */
-  private Map<String, Object> additionalProperties;
+  private transient Map<String, Object> additionalProperties;
 
   /**
    * Set the additional (undeclared) property with the specified name and value.

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

@@ -156,8 +156,15 @@ public void setArrayFooThree(@javax.annotation.Nullable List<Tag> arrayFooThree)
    * A container for additional, undeclared properties.
    * This is a holder for any undeclared properties as specified with
    * the 'additionalProperties' keyword in the OAS document.
+   *
+   * Declared once per hierarchy: gson collects the declared fields of every class in the
+   * hierarchy and rejects two bound to one JSON name, so allOf descendants inherit this
+   * field instead of declaring their own. Transient on models without children: the bag is
+   * read and written by this model's TypeAdapterFactory, so gson's reflection does not need
+   * to see it. A parent with children has no factory of its own, so it keeps the field
+   * bound for reflection.
    */
-  private Map<String, Object> additionalProperties;
+  private transient Map<String, Object> additionalProperties;
 
   /**
    * Set the additional (undeclared) property with the specified name and value.