Avoid using replicated default values for Lists when creating from builder or instance

builder/codegen/src/main/java/io/helidon/builder/codegen/GenerateAbstractBuilder.java

@@ -354,6 +354,13 @@ private static void fromInstanceMethod(InnerClass.Builder builder, TypeContext t
         for (PrototypeProperty property : typeContext.propertyData().properties()) {
             TypeName declaredType = property.typeHandler().declaredType();
             if (declaredType.isSet() || declaredType.isList() || declaredType.isMap()) {
+                if (declaredType.isList()) {
+                    // A list might contain only default values. If it has not been updated, clear it of default values
+                    // before adding the values from the other builder.
+                    methodBuilder.addContentLine("if (!is" + capitalize(property.name()) + "Mutated) {")
+                            .addContentLine(property.name() + ".clear();")
+                            .addContentLine("}");
+                }
                 methodBuilder.addContent("add");
                 methodBuilder.addContent(capitalize(property.name()));
                 methodBuilder.addContent("(prototype.");
@@ -412,11 +419,42 @@ private static void fromBuilderMethod(InnerClass.Builder classBuilder,
                 methodBuilder.addContentLine("builder." + getterName + "().ifPresent(this::" + setterName + ");");
             } else {
                 if (declaredType.isSet() || declaredType.isList() || declaredType.isMap()) {
-                    methodBuilder.addContent("add" + capitalize(property.name()));
+                    if (declaredType.isList()) {
+                        /*
+                        If this builder's list HAS been mutated, add the other builder's values ONLY if they are not defaults.
+
+                        If this builder's list HAS NOT been mutated, clear it (to remove defaults) and add the other builder's
+                        values regardless of whether they are defaulted or explicitly set.
+
+                        Generated code:
+
+                        if (isXxxMutated) {
+                            if (builder.isXxxMutated) {
+                                xxx.addAll(builder.xxx());
+                            }
+                        } else {
+                            xxx.clear();
+                            xxx.addAll(builder.xxx());
+                        }
+                        */
+                        String isMutatedProperty = TypeHandlerList.isMutatedField(property.name());
+                        methodBuilder.addContentLine("if (" + isMutatedProperty + ") {")
+                                .addContentLine("if (builder." + isMutatedProperty + ") {")
+                                .addContentLine("add" + capitalize(property.name()) + "(builder." + property.name() + ");")
+                                .addContentLine("} else {")
+                                .addContentLine(property.name() + ".clear();")
+                                .addContentLine("add" + capitalize(property.name()) + "(builder." + property.name() + ");")
+                                .addContentLine("}")
+                                .addContentLine("}");
+
+                    } else {
+                        // Non-list collection case.
+                        methodBuilder.addContentLine("add" + capitalize(property.name()) + "(builder." + property.name() + ");");
+                    }
                 } else {
-                    methodBuilder.addContent(setterName);
+                    // Non-collection case.
+                    methodBuilder.addContentLine(setterName + "(builder." + getterName + "());");
                 }
-                methodBuilder.addContentLine("(builder." + getterName + "());");
             }
 
         }
@@ -453,6 +491,11 @@ private static void fields(InnerClass.Builder classBuilder, TypeContext typeCont
                         .name(child.name() + "DiscoverServices")
                         .defaultValue(String.valueOf(child.configuredOption().providerDiscoverServices())));
             }
+            if (isBuilder && child.typeHandler().declaredType().isList()) {
+                classBuilder.addField(builder -> builder.type(boolean.class)
+                        .name("is" + capitalize(child.name()) + "Mutated")
+                        .accessModifier(AccessModifier.PRIVATE));
+            }
         }
     }
 

builder/codegen/src/main/java/io/helidon/builder/codegen/TypeHandlerCollection.java

@@ -438,6 +438,7 @@ private void declaredSetters(InnerClass.Builder classBuilder,
                 .accessModifier(setterAccessModifier(configured))
                 .addContent(Objects.class)
                 .addContentLine(".requireNonNull(" + name() + ");")
+                .update(this::extraSetterContent)
                 .addContentLine("this." + name() + ".clear();")
                 .addContentLine("this." + name() + ".addAll(" + name() + ");")
                 .addContentLine("return self();");
@@ -446,8 +447,18 @@ private void declaredSetters(InnerClass.Builder classBuilder,
         builder.name("add" + capitalize(name()))
                 .clearContent()
                 .addContentLine("Objects.requireNonNull(" + name() + ");") //Overwrites existing content
+                .update(this::extraAdderContent)
                 .addContentLine("this." + name() + ".addAll(" + name() + ");")
                 .addContentLine("return self();");
         classBuilder.addMethod(builder);
     }
+
+    Method.Builder extraSetterContent(Method.Builder builder) {
+        return builder;
+    }
+
+    Method.Builder extraAdderContent(Method.Builder builder) {
+        return builder;
+    }
+
 }

builder/codegen/src/main/java/io/helidon/builder/codegen/TypeHandlerList.java

@@ -18,6 +18,8 @@
 
 import java.util.Optional;
 
+import io.helidon.codegen.CodegenUtil;
+import io.helidon.codegen.classmodel.Method;
 import io.helidon.common.types.TypeName;
 
 import static io.helidon.common.types.TypeNames.LIST;
@@ -27,4 +29,22 @@ class TypeHandlerList extends TypeHandlerCollection {
     TypeHandlerList(String name, String getterName, String setterName, TypeName declaredType) {
         super(name, getterName, setterName, declaredType, LIST, "toList()", Optional.empty());
     }
+
+    static String isMutatedField(String propertyName) {
+        return "is" + CodegenUtil.capitalize(propertyName) + "Mutated";
+    }
+
+    @Override
+    Method.Builder extraAdderContent(Method.Builder builder) {
+        return builder.addContentLine(isMutatedField() + " = true;");
+    }
+
+    @Override
+    Method.Builder extraSetterContent(Method.Builder builder) {
+        return builder.addContentLine(isMutatedField() + " = true;");
+    }
+
+    private String isMutatedField() {
+        return isMutatedField(name());
+    }
 }

builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/DualValuedDefaultValuesBlueprint.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.helidon.builder.test.testsubjects;
+
+import java.util.List;
+
+import io.helidon.builder.api.Option;
+import io.helidon.builder.api.Prototype;
+
+@Prototype.Blueprint
+interface DualValuedDefaultValuesBlueprint {
+
+    String DEFAULT_1 = "default1";
+    String DEFAULT_2 = "default2";
+
+    @Option.Default({DEFAULT_1, DEFAULT_2})
+    List<String> strings();
+
+}

builder/tests/builder/src/main/java/io/helidon/builder/test/testsubjects/SingleValuedDefaultValuesBlueprint.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.helidon.builder.test.testsubjects;
+
+import java.util.List;
+
+import io.helidon.builder.api.Option;
+import io.helidon.builder.api.Prototype;
+
+@Prototype.Blueprint
+interface SingleValuedDefaultValuesBlueprint {
+
+    String DEFAULT_STRING = "defaultValue";
+
+    @Option.Default(DEFAULT_STRING)
+    List<String> strings();
+
+}

builder/tests/builder/src/test/java/io/helidon/builder/test/ListDefaultValuesTest.java

@@ -0,0 +1,143 @@
+/*
+ * Copyright (c) 2024 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.helidon.builder.test;
+
+import java.util.List;
+
+import io.helidon.builder.test.testsubjects.DualValuedDefaultValues;
+import io.helidon.builder.test.testsubjects.SingleValuedDefaultValues;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.hasItem;
+import static org.hamcrest.Matchers.hasItems;
+import static org.hamcrest.Matchers.iterableWithSize;
+
+class ListDefaultValuesTest {
+
+    @Test
+    void checkSingleDefaultFromInstance() {
+        SingleValuedDefaultValues original = SingleValuedDefaultValues.create();
+        assertThat("Original value",
+                   original.strings(),
+                   allOf(hasItem(SingleValuedDefaultValues.DEFAULT_STRING),
+                         iterableWithSize(1)));
+        SingleValuedDefaultValues copy = SingleValuedDefaultValues.builder().from(original).build();
+        assertThat("Copied value",
+                   copy.strings(),
+                   allOf(hasItem(SingleValuedDefaultValues.DEFAULT_STRING),
+                         iterableWithSize(1)));
+
+    }
+
+    @Test
+    void checkSingleDefaultFromBuilder() {
+        SingleValuedDefaultValues.Builder builder = SingleValuedDefaultValues.builder();
+
+        SingleValuedDefaultValues copy = SingleValuedDefaultValues.builder().from(builder).build();
+
+        assertThat("Copied value",
+                   copy.strings(),
+                   allOf(hasItem(SingleValuedDefaultValues.DEFAULT_STRING),
+                         iterableWithSize(1)));
+
+    }
+
+    @Test
+    void checkDualDefaultFromInstance() {
+        DualValuedDefaultValues original = DualValuedDefaultValues.create();
+        assertThat("Original values",
+                   original.strings(),
+                   allOf(hasItems(DualValuedDefaultValues.DEFAULT_1, DualValuedDefaultValues.DEFAULT_2),
+                         iterableWithSize(2)));
+
+        DualValuedDefaultValues copy = DualValuedDefaultValues.builder().from(original).build();
+        assertThat("Copied values",
+                   original.strings(),
+                   allOf(hasItems(DualValuedDefaultValues.DEFAULT_1, DualValuedDefaultValues.DEFAULT_2),
+                         iterableWithSize(2)));
+    }
+
+    @Test
+    void checkDualDefaultFromBuilder() {
+        DualValuedDefaultValues.Builder builder = DualValuedDefaultValues.builder();
+
+        DualValuedDefaultValues copy = DualValuedDefaultValues.builder().from(builder).build();
+        assertThat("Copied values",
+                   copy.strings(),
+                   allOf(hasItems(DualValuedDefaultValues.DEFAULT_1, DualValuedDefaultValues.DEFAULT_2),
+                         iterableWithSize(2)));
+    }
+
+    @Test
+    void checkSingleDefaultDoubledInBuilder() {
+        SingleValuedDefaultValues original = SingleValuedDefaultValues.builder()
+                .strings(List.of(SingleValuedDefaultValues.DEFAULT_STRING, SingleValuedDefaultValues.DEFAULT_STRING))
+                .build();
+
+        SingleValuedDefaultValues copy = SingleValuedDefaultValues.builder()
+                .from(original)
+                .build();
+        assertThat("Copied values",
+                   copy.strings(),
+                   allOf(hasItems(SingleValuedDefaultValues.DEFAULT_STRING, SingleValuedDefaultValues.DEFAULT_STRING),
+                         iterableWithSize(2)));
+    }
+
+    @Test
+    void checkSingleDefaultWithUpdate() {
+        String value = "non-default";
+        SingleValuedDefaultValues original = SingleValuedDefaultValues.builder()
+                .addStrings(List.of(value))
+                .build();
+
+        assertThat("Original with update",
+                   original.strings(),
+                   allOf(hasItems(value, SingleValuedDefaultValues.DEFAULT_STRING),
+                         iterableWithSize(2)));
+    }
+
+    @Test
+    void checkDualDefaultWithUpdate() {
+        String value = "non-default";
+        DualValuedDefaultValues original = DualValuedDefaultValues.builder()
+                .addStrings(List.of(value))
+                .build();
+        assertThat("Original with update", original.strings(), allOf(hasItems(value,
+                                                                              DualValuedDefaultValues.DEFAULT_1,
+                                                                              DualValuedDefaultValues.DEFAULT_2),
+                                                                     iterableWithSize(3)));
+    }
+
+    @Test
+    void checkSingleDefaultWithoutUpdate() {
+        SingleValuedDefaultValues original = SingleValuedDefaultValues.create();
+
+        assertThat("Original with no update", original.strings(), allOf(hasItems(SingleValuedDefaultValues.DEFAULT_STRING),
+                                                                        iterableWithSize(1)));
+    }
+
+    @Test
+    void checkDualDefaultWithoutUpdate() {
+        DualValuedDefaultValues original = DualValuedDefaultValues.create();
+
+        assertThat("Original with no updates", original.strings(), allOf(hasItems(DualValuedDefaultValues.DEFAULT_1,
+                                                                                  DualValuedDefaultValues.DEFAULT_2),
+                                                                         iterableWithSize(2)));
+    }
+}