Skip to content

Commit

Permalink
Use isMutated flags for lists to know how to handle them in 'from' me…
Browse files Browse the repository at this point in the history 
…thods
  • Loading branch information
@tjquinno
tjquinno committed Mar 24, 2024
1 parent 93a4cdb commit 8a7528c
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 52 deletions.
32 changes: 22 additions & 10 deletions builder/codegen/src/main/java/io/helidon/builder/codegen/GenerateAbstractBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,11 +354,15 @@ 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()) {
// Sets and maps take care of enforcing distinct values themselves, but do so ourselves for lists when
// adding values from an existing builder which will already have the default(s).
methodBuilder.addContent("add"
+ (declaredType.isList() ? "Distinct" : "")
+ capitalize(property.name()));
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.");
methodBuilder.addContent(property.typeHandler().getterName());
methodBuilder.addContentLine("());");
Expand Down Expand Up @@ -415,11 +419,14 @@ private static void fromBuilderMethod(InnerClass.Builder classBuilder,
methodBuilder.addContentLine("builder." + getterName + "().ifPresent(this::" + setterName + ");");
} else {
if (declaredType.isSet() || declaredType.isList() || declaredType.isMap()) {
// Sets and maps take care of enforcing distinct values themselves, but do so ourselves for lists when
// adding values from an existing builder which will already have the default(s).
methodBuilder.addContent("add"
+ (declaredType.isList() ? "Distinct" : "")
+ capitalize(property.name()));
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" + capitalize(property.name()));
} else {
methodBuilder.addContent(setterName);
}
Expand Down Expand Up @@ -460,6 +467,11 @@ private static void fields(InnerClass.Builder classBuilder, TypeContext typeCont
.name(child.name() + "DiscoverServices")
.defaultValue(String.valueOf(child.configuredOption().providerDiscoverServices())));
}
if (child.typeHandler().declaredType().isList()) {
classBuilder.addField(builder -> builder.type(boolean.class)
.name("is" + capitalize(child.name()) + "Mutated")
.accessModifier(AccessModifier.PRIVATE));
}
}
}

Expand Down
11 changes: 11 additions & 0 deletions builder/codegen/src/main/java/io/helidon/builder/codegen/TypeHandlerCollection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();");
Expand All @@ -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;
}

}
44 changes: 9 additions & 35 deletions builder/codegen/src/main/java/io/helidon/builder/codegen/TypeHandlerList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,11 @@
package io.helidon.builder.codegen;

import java.util.Optional;
import java.util.function.Predicate;

import io.helidon.codegen.classmodel.InnerClass;
import io.helidon.codegen.classmodel.Javadoc;
import io.helidon.codegen.CodegenUtil;
import io.helidon.codegen.classmodel.Method;
import io.helidon.common.types.AccessModifier;
import io.helidon.common.types.TypeName;

import static io.helidon.codegen.CodegenUtil.capitalize;
import static io.helidon.common.types.TypeNames.LIST;

class TypeHandlerList extends TypeHandlerCollection {
Expand All @@ -35,38 +31,16 @@ class TypeHandlerList extends TypeHandlerCollection {
}

@Override
void setters(InnerClass.Builder classBuilder,
AnnotationDataOption configured,
FactoryMethods factoryMethods,
TypeName returnType,
Javadoc blueprintJavadoc) {
super.setters(classBuilder, configured, factoryMethods, returnType, blueprintJavadoc);
declaredDistinctSetter(classBuilder, returnType, blueprintJavadoc);
Method.Builder extraAdderContent(Method.Builder builder) {
return builder.addContentLine(isMutatedField() + " = true;");
}

private void declaredDistinctSetter(InnerClass.Builder classBuilder, TypeName returnType, Javadoc blueprintJavadoc) {

// Primarily for use in preventing default values from being included in a list twice using Builder.from.
classBuilder.addImport(Predicate.class);
Method.Builder builder = Method.builder()
.name("addDistinct" + capitalize(name()))
.accessModifier(AccessModifier.PROTECTED)
.returnType(returnType, "updated builder instance")
.description("Adds distinct values")
.addJavadocTag("see", "#" + getterName() + "()")
.addParameter(param -> param.name(name())
.type(argumentTypeName())
.description(blueprintJavadoc.returnDescription()))
@Override
Method.Builder extraSetterContent(Method.Builder builder) {
return builder.addContentLine(isMutatedField() + " = true;");
}

.addContentLine("Objects.requireNonNull(" + name() + ");")
.addContentLine(name() + ".stream()")
.increaseContentPadding()
.increaseContentPadding()
.addContentLine(".filter(Predicate.not(this." + name() + "::contains))")
.addContentLine(".forEach(this." + name() + "::add);")
.decreaseContentPadding()
.decreaseContentPadding()
.addContentLine("return self();");
classBuilder.addMethod(builder);
private String isMutatedField() {
return "is" + CodegenUtil.capitalize(name()) + "Mutated";
}
}
6 changes: 5 additions & 1 deletion builder/tests/builder/src/test/java/io/helidon/builder/test/CustomNamedTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ void testIt() throws Exception {
.build();
DefaultPrettyPrinter printer = new DefaultPrettyPrinter();
String json = mapper.writer(printer).writeValueAsString(customNamed);
assertThat(json, equalTo("{\n" + " \"stringSet\" : [ \"b\", \"a\", \"y\" ]\n" + "}"));
assertThat(json, equalTo("""
{
"isStringListMutated" : false,
"stringSet" : [ "b", "a", "y" ]
}"""));
}

}
79 changes: 73 additions & 6 deletions builder/tests/builder/src/test/java/io/helidon/builder/test/ListDefaultValuesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,18 @@
*/
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 {

Expand All @@ -31,11 +35,13 @@ void checkSingleDefaultFromInstance() {
SingleValuedDefaultValues original = SingleValuedDefaultValues.create();
assertThat("Original value",
original.strings(),
hasItem(SingleValuedDefaultValues.DEFAULT_STRING));
allOf(hasItem(SingleValuedDefaultValues.DEFAULT_STRING),
iterableWithSize(1)));
SingleValuedDefaultValues copy = SingleValuedDefaultValues.builder().from(original).build();
assertThat("Copied value",
copy.strings(),
hasItem(SingleValuedDefaultValues.DEFAULT_STRING));
allOf(hasItem(SingleValuedDefaultValues.DEFAULT_STRING),
iterableWithSize(1)));

}

Expand All @@ -47,7 +53,8 @@ void checkSingleDefaultFromBuilder() {

assertThat("Copied value",
copy.strings(),
hasItem(SingleValuedDefaultValues.DEFAULT_STRING));
allOf(hasItem(SingleValuedDefaultValues.DEFAULT_STRING),
iterableWithSize(1)));

}

Expand All @@ -56,12 +63,14 @@ void checkDualDefaultFromInstance() {
DualValuedDefaultValues original = DualValuedDefaultValues.create();
assertThat("Original values",
original.strings(),
hasItems(DualValuedDefaultValues.DEFAULT_1, DualValuedDefaultValues.DEFAULT_2));
allOf(hasItems(DualValuedDefaultValues.DEFAULT_1, DualValuedDefaultValues.DEFAULT_2),
iterableWithSize(2)));

DualValuedDefaultValues copy = DualValuedDefaultValues.builder().from(original).build();
assertThat("Copied values",
original.strings(),
hasItems(DualValuedDefaultValues.DEFAULT_1, DualValuedDefaultValues.DEFAULT_2));
allOf(hasItems(DualValuedDefaultValues.DEFAULT_1, DualValuedDefaultValues.DEFAULT_2),
iterableWithSize(2)));
}

@Test
Expand All @@ -71,6 +80,64 @@ void checkDualDefaultFromBuilder() {
DualValuedDefaultValues copy = DualValuedDefaultValues.builder().from(builder).build();
assertThat("Copied values",
copy.strings(),
hasItems(DualValuedDefaultValues.DEFAULT_1, DualValuedDefaultValues.DEFAULT_2));
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)));
}
}

0 comments on commit 8a7528c

Please sign in to comment.