Refactor additional properties handling

- Remove every additional property
before building the DTO. An
additional property may have a
different type than the explicit
property, thus it may be possible
to create an invalid DTO.

Issue: #185

plugin/src/main/java/com/github/muehmar/gradle/openapi/generator/java/generator/pojo/builder/AdditionalPropertiesSetterGenerator.java

@@ -3,7 +3,6 @@
 import static com.github.muehmar.gradle.openapi.generator.java.generator.pojo.RefsGenerator.ref;
 import static com.github.muehmar.gradle.openapi.generator.java.model.JavaAdditionalProperties.additionalPropertiesName;
 import static com.github.muehmar.gradle.openapi.util.Booleans.not;
-import static io.github.muehmar.codegenerator.Generator.constant;
 import static io.github.muehmar.codegenerator.java.JavaModifier.PRIVATE;
 import static io.github.muehmar.codegenerator.java.JavaModifier.PUBLIC;
 import static io.github.muehmar.codegenerator.java.MethodGen.Argument.argument;
@@ -12,138 +11,67 @@
 import com.github.muehmar.gradle.openapi.generator.java.generator.pojo.RefsGenerator;
 import com.github.muehmar.gradle.openapi.generator.java.generator.shared.jackson.JacksonAnnotationGenerator;
 import com.github.muehmar.gradle.openapi.generator.java.model.JavaAdditionalProperties;
-import com.github.muehmar.gradle.openapi.generator.java.model.member.JavaPojoMember;
 import com.github.muehmar.gradle.openapi.generator.java.model.pojo.JavaObjectPojo;
 import com.github.muehmar.gradle.openapi.generator.java.ref.JavaRefs;
 import com.github.muehmar.gradle.openapi.generator.settings.PojoSettings;
 import io.github.muehmar.codegenerator.Generator;
 import io.github.muehmar.codegenerator.java.JavaModifiers;
 import io.github.muehmar.codegenerator.java.MethodGen.Argument;
 import io.github.muehmar.codegenerator.java.MethodGenBuilder;
-import lombok.Value;
 
 class AdditionalPropertiesSetterGenerator {
   private AdditionalPropertiesSetterGenerator() {}
 
   public static Generator<JavaObjectPojo, PojoSettings> additionalPropertiesSetterGenerator() {
-    return singlePropSetter(true)
-        .filter(pojo -> pojo.getAdditionalProperties().isNotValueAnyType())
+    return Generator.<JavaObjectPojo, PojoSettings>emptyGen()
+        .append(additionalPropertiesSetters(), JavaObjectPojo::getAdditionalProperties);
+  }
+
+  private static Generator<JavaAdditionalProperties, PojoSettings> additionalPropertiesSetters() {
+    return singleAdditionalPropertiesSetter(true)
+        .filter(JavaAdditionalProperties::isNotValueAnyType)
         .appendSingleBlankLine()
-        .append(singlePropSetter(false))
+        .append(singleAdditionalPropertiesSetter(false))
         .appendSingleBlankLine()
-        .append(allPropsSetter(), JavaObjectPojo::getAdditionalProperties);
+        .append(allAdditionalPropertiesSetter());
   }
 
-  private static Generator<JavaObjectPojo, PojoSettings> singlePropSetter(boolean forObjectType) {
-    final Generator<JavaObjectPojo, PojoSettings> method =
-        MethodGenBuilder.<JavaObjectPojo, PojoSettings>create()
+  private static Generator<JavaAdditionalProperties, PojoSettings> singleAdditionalPropertiesSetter(
+      boolean forObjectType) {
+    final Generator<JavaAdditionalProperties, PojoSettings> method =
+        MethodGenBuilder.<JavaAdditionalProperties, PojoSettings>create()
             .modifiers(
-                pojo ->
-                    createModifiersForSinglePropSetter(
-                        pojo.getAdditionalProperties(), forObjectType))
+                props -> createModifiersForSingleAdditionalPropertiesSetter(props, forObjectType))
             .noGenericTypes()
             .returnType("Builder")
             .methodName("addAdditionalProperty")
             .arguments(
-                pojo ->
+                props ->
                     PList.of(
                         new Argument("String", "key"),
                         new Argument(
                             forObjectType
                                 ? "Object"
-                                : pojo.getAdditionalProperties()
-                                    .getType()
-                                    .getParameterizedClassName()
-                                    .asString(),
+                                : props.getType().getParameterizedClassName().asString(),
                             "value")))
-            .content(singlePropSetterContent())
+            .content(
+                (props, s, w) ->
+                    w.println("this.%s.put(key, value);", additionalPropertiesName())
+                        .println("return this;"))
             .build()
-            .append(RefsGenerator.javaTypeRefs(), pojo -> pojo.getAdditionalProperties().getType());
-    return JacksonAnnotationGenerator.<JavaObjectPojo>jsonAnySetter()
+            .append(RefsGenerator.javaTypeRefs(), JavaAdditionalProperties::getType);
+    return JacksonAnnotationGenerator.<JavaAdditionalProperties>jsonAnySetter()
         .filter(ignore -> not(forObjectType))
         .append(method);
   }
 
-  private static Generator<JavaObjectPojo, PojoSettings> singlePropSetterContent() {
-    return Generator.<JavaObjectPojo, PojoSettings>emptyGen()
-        .appendList(singlePropSetterContentMemberCondition(), PojoAndMember::fromPojo)
-        .append(
-            Generator.<JavaObjectPojo, PojoSettings>newLine()
-                .filter(pojo -> pojo.getAllMembers().nonEmpty()))
-        .append((pam, s, w) -> w.println("this.%s.put(key, value);", additionalPropertiesName()))
-        .append(constant("return this;"));
-  }
-
-  private static Generator<PojoAndMember, PojoSettings> singlePropSetterContentMemberCondition() {
-    return Generator.<PojoAndMember, PojoSettings>emptyGen()
-        .append(singlePropSetterRequiredNotNullableMemberCondition())
-        .append(singlePropSetterRequiredNullableMemberCondition())
-        .append(singlePropSetterOptionalNotNullableMemberCondition())
-        .append(singlePropSetterOptionalNullableMemberCondition());
-  }
-
-  private static Generator<PojoAndMember, PojoSettings>
-      singlePropSetterRequiredNotNullableMemberCondition() {
-    return Generator.<PojoAndMember, PojoSettings>emptyGen()
-        .append(
-            (pam, s, w) ->
-                w.println(
-                    "if (\"%s\".equals(key) && %s != null) {",
-                    pam.getMember().getName().getOriginalName(), pam.getMember().getName()))
-        .append(constant("return this;"), 1)
-        .append(constant("}"))
-        .filter(pam -> pam.getMember().isRequiredAndNotNullable());
-  }
-
-  private static Generator<PojoAndMember, PojoSettings>
-      singlePropSetterRequiredNullableMemberCondition() {
-    return Generator.<PojoAndMember, PojoSettings>emptyGen()
-        .append(
-            (pam, s, w) ->
-                w.println(
-                    "if (\"%s\".equals(key) && %s) {",
-                    pam.getMember().getName().getOriginalName(),
-                    pam.getMember().getIsPresentFlagName()))
-        .append(constant("return this;"), 1)
-        .append(constant("}"))
-        .filter(pam -> pam.getMember().isRequiredAndNullable());
-  }
-
-  private static Generator<PojoAndMember, PojoSettings>
-      singlePropSetterOptionalNotNullableMemberCondition() {
-    return Generator.<PojoAndMember, PojoSettings>emptyGen()
-        .append(
-            (pam, s, w) ->
-                w.println(
-                    "if (\"%s\".equals(key) && %s != null) {",
-                    pam.getMember().getName().getOriginalName(), pam.getMember().getName()))
-        .append(constant("return this;"), 1)
-        .append(constant("}"))
-        .filter(pam -> pam.getMember().isOptionalAndNotNullable());
-  }
-
-  private static Generator<PojoAndMember, PojoSettings>
-      singlePropSetterOptionalNullableMemberCondition() {
-    return Generator.<PojoAndMember, PojoSettings>emptyGen()
-        .append(
-            (pam, s, w) ->
-                w.println(
-                    "if (\"%s\".equals(key) && (%s || %s != null)) {",
-                    pam.getMember().getName().getOriginalName(),
-                    pam.getMember().getIsNullFlagName(),
-                    pam.getMember().getName()))
-        .append(constant("return this;"), 1)
-        .append(constant("}"))
-        .filter(pam -> pam.getMember().isOptionalAndNullable());
-  }
-
-  private static JavaModifiers createModifiersForSinglePropSetter(
+  private static JavaModifiers createModifiersForSingleAdditionalPropertiesSetter(
       JavaAdditionalProperties props, boolean forObjectType) {
     final boolean privateMethod = forObjectType || not(props.isAllowed());
     return JavaModifiers.of(privateMethod ? PRIVATE : PUBLIC);
   }
 
-  private static Generator<JavaAdditionalProperties, PojoSettings> allPropsSetter() {
+  private static Generator<JavaAdditionalProperties, PojoSettings> allAdditionalPropertiesSetter() {
     return MethodGenBuilder.<JavaAdditionalProperties, PojoSettings>create()
         .modifiers(PUBLIC)
         .noGenericTypes()
@@ -156,23 +84,14 @@ private static Generator<JavaAdditionalProperties, PojoSettings> allPropsSetter(
                     additionalPropertiesName()))
         .content(
             (props, s, w) ->
-                w.println("this.%s.clear();", additionalPropertiesName())
-                    .println("%s.forEach(this::addAdditionalProperty);", additionalPropertiesName())
+                w.println(
+                        "this.%s = new HashMap<>(%s);",
+                        additionalPropertiesName(), additionalPropertiesName())
                     .println("return this;"))
         .build()
         .append(RefsGenerator.javaTypeRefs(), JavaAdditionalProperties::getType)
         .append(ref(JavaRefs.JAVA_UTIL_MAP))
         .append(ref(JavaRefs.JAVA_UTIL_HASH_MAP))
         .filter(JavaAdditionalProperties::isAllowed);
   }
-
-  @Value
-  private static class PojoAndMember {
-    JavaObjectPojo pojo;
-    JavaPojoMember member;
-
-    static PList<PojoAndMember> fromPojo(JavaObjectPojo pojo) {
-      return pojo.getAllMembers().map(member -> new PojoAndMember(pojo, member));
-    }
-  }
 }

plugin/src/main/java/com/github/muehmar/gradle/openapi/generator/java/generator/pojo/builder/BuildMethodGenerator.java

@@ -1,5 +1,6 @@
 package com.github.muehmar.gradle.openapi.generator.java.generator.pojo.builder;
 
+import static com.github.muehmar.gradle.openapi.generator.java.model.JavaAdditionalProperties.additionalPropertiesName;
 import static io.github.muehmar.codegenerator.java.JavaModifier.PUBLIC;
 
 import com.github.muehmar.gradle.openapi.generator.java.model.member.TechnicalPojoMember;
@@ -23,6 +24,20 @@ public static Generator<JavaObjectPojo, PojoSettings> buildMethodGenerator() {
   }
 
   private static Generator<JavaObjectPojo, PojoSettings> buildMethodContent() {
+    return Generator.<JavaObjectPojo, PojoSettings>emptyGen()
+        .appendList(
+            (m, s, w) ->
+                w.println(
+                    "%s.remove(\"%s\");",
+                    additionalPropertiesName(), m.getName().getOriginalName()),
+            JavaObjectPojo::getAllMembers)
+        .append(
+            Generator.<JavaObjectPojo, PojoSettings>newLine()
+                .filter(p -> p.getAllMembers().nonEmpty()))
+        .append(buildMethodCall());
+  }
+
+  private static Generator<JavaObjectPojo, PojoSettings> buildMethodCall() {
     return (pojo, settings, writer) ->
         writer.print(
             "return new %s(%s);",

plugin/src/main/java/com/github/muehmar/gradle/openapi/generator/java/generator/pojo/builder/SetterGenerator.java

@@ -73,7 +73,6 @@ private static Generator<JavaPojoMember, PojoSettings> setterMethodContent() {
                 writer.println(
                     "this.%s = %s == null;", member.getIsNullFlagName(), member.getName()),
             JavaPojoMember::isOptionalAndNullable)
-        .append((m, s, w) -> w.println("%s", removeAdditionalPropertyLine(m)))
         .append(w -> w.println("return this;"));
   }
 
@@ -100,7 +99,6 @@ private static Generator<JavaPojoMember, PojoSettings> requiredNullableSetter()
                     writer
                         .println("this.%s = %s.orElse(null);", member.getName(), member.getName())
                         .println("this.%s = true;", member.getIsPresentFlagName())
-                        .println("%s", removeAdditionalPropertyLine(member))
                         .println("return this;")
                         .ref(JavaRefs.JAVA_UTIL_OPTIONAL))
             .build();
@@ -131,7 +129,6 @@ private static Generator<JavaPojoMember, PojoSettings> optionalSetter() {
                 (member, settings, writer) ->
                     writer
                         .println("this.%s = %s.orElse(null);", member.getName(), member.getName())
-                        .println("%s", removeAdditionalPropertyLine(member))
                         .println("return this;")
                         .ref(JavaRefs.JAVA_UTIL_OPTIONAL))
             .build();
@@ -169,7 +166,6 @@ private static Generator<JavaPojoMember, PojoSettings> optionalNullableSetter()
                             member.getIsNullFlagName(),
                             member.getName(),
                             member.tristateToIsNullFlag())
-                        .println("%s", removeAdditionalPropertyLine(member))
                         .println("return this;")
                         .ref(OpenApiUtilRefs.TRISTATE))
             .build();
@@ -180,9 +176,4 @@ private static Generator<JavaPojoMember, PojoSettings> optionalNullableSetter()
         .appendNewLine()
         .filter(JavaPojoMember::isOptionalAndNullable);
   }
-
-  private static String removeAdditionalPropertyLine(JavaPojoMember member) {
-    return String.format(
-        "this.additionalProperties.remove(\"%s\");", member.getName().getOriginalName());
-  }
 }

plugin/src/main/java/com/github/muehmar/gradle/openapi/generator/java/generator/pojo/safebuilder/SafeBuilderGenerator.java

@@ -21,8 +21,7 @@
 public class SafeBuilderGenerator implements Generator<JavaObjectPojo, PojoSettings> {
   private static final String FACTORY_JAVA_DOC =
       "Instantiates a new staged builder. Explicit properties have precedence over additional properties, i.e. an "
-          + "additional property with the same name as an explicit property will be discarded if the explicit "
-          + "property is set or if it gets set.";
+          + "additional property with the same name as an explicit property will be discarded.";
   private final Generator<JavaObjectPojo, PojoSettings> delegate;
 
   public SafeBuilderGenerator(SafeBuilderVariant builderVariant) {

plugin/src/test/java/com/github/muehmar/gradle/openapi/generator/java/__snapshots__/JavaPojoGeneratorTest.snap

@@ -361,7 +361,6 @@ public class SampleObjectPojo1Dto {
     @JsonProperty("stringVal")
     private Builder setStringVal(String stringVal) {
       this.stringVal = stringVal;
-      this.additionalProperties.remove("stringVal");
       return this;
     }
 
@@ -371,7 +370,6 @@ public class SampleObjectPojo1Dto {
     @JsonProperty("intVal")
     private Builder setIntVal(Integer intVal) {
       this.intVal = intVal;
-      this.additionalProperties.remove("intVal");
       return this;
     }
 
@@ -381,42 +379,33 @@ public class SampleObjectPojo1Dto {
     @JsonProperty("doubleVal")
     private Builder setDoubleVal(Double doubleVal) {
       this.doubleVal = doubleVal;
-      this.additionalProperties.remove("doubleVal");
       return this;
     }
 
     @JsonAnySetter
     public Builder addAdditionalProperty(String key, Object value) {
-      if ("stringVal".equals(key) && stringVal != null) {
-        return this;
-      }
-      if ("intVal".equals(key) && intVal != null) {
-        return this;
-      }
-      if ("doubleVal".equals(key) && doubleVal != null) {
-        return this;
-      }
-
       this.additionalProperties.put(key, value);
       return this;
     }
 
     public Builder setAdditionalProperties(Map<String, Object> additionalProperties) {
-      this.additionalProperties.clear();
-      additionalProperties.forEach(this::addAdditionalProperty);
+      this.additionalProperties = new HashMap<>(additionalProperties);
       return this;
     }
 
     public SampleObjectPojo1Dto build() {
+      additionalProperties.remove("stringVal");
+      additionalProperties.remove("intVal");
+      additionalProperties.remove("doubleVal");
+
       return new SampleObjectPojo1Dto(stringVal, intVal, doubleVal, additionalProperties);
     }
   }
 
   /**
    * Instantiates a new staged builder. Explicit properties have precedence over
    * additional properties, i.e. an additional property with the same name as an
-   * explicit property will be discarded if the explicit property is set or if it
-   * gets set.
+   * explicit property will be discarded.
    */
   public static FullPropertyBuilder0 fullBuilder() {
     return new FullPropertyBuilder0(new Builder());
@@ -425,8 +414,7 @@ public class SampleObjectPojo1Dto {
   /**
    * Instantiates a new staged builder. Explicit properties have precedence over
    * additional properties, i.e. an additional property with the same name as an
-   * explicit property will be discarded if the explicit property is set or if it
-   * gets set.
+   * explicit property will be discarded.
    */
   public static FullPropertyBuilder0 fullSampleObjectPojo1DtoBuilder() {
     return new FullPropertyBuilder0(new Builder());
@@ -503,8 +491,7 @@ public class SampleObjectPojo1Dto {
   /**
    * Instantiates a new staged builder. Explicit properties have precedence over
    * additional properties, i.e. an additional property with the same name as an
-   * explicit property will be discarded if the explicit property is set or if it
-   * gets set.
+   * explicit property will be discarded.
    */
   public static PropertyBuilder0 builder() {
     return new PropertyBuilder0(new Builder());
@@ -513,8 +500,7 @@ public class SampleObjectPojo1Dto {
   /**
    * Instantiates a new staged builder. Explicit properties have precedence over
    * additional properties, i.e. an additional property with the same name as an
-   * explicit property will be discarded if the explicit property is set or if it
-   * gets set.
+   * explicit property will be discarded.
    */
   public static PropertyBuilder0 sampleObjectPojo1DtoBuilder() {
     return new PropertyBuilder0(new Builder());