Skip to content

Commit 3fadefd

Browse files
tomas-langertomas-langer
authored
4.x: Fixes for problems discovered in service registry work (helidon-io#8403)
* Fixes for problems discovered in service registry work - handling of Optional<char[]> in builders - handling of generic types built by builders in another builder - missing primitive type mappers in common mappers - support for mappable values in Parameters - small build.sh fix to remove deleted scripts (replaced by another job) * Review fixes.
1 parent 13c7a3b commit 3fadefd

21 files changed

Lines changed: 341 additions & 82 deletions

File tree

‎builder/api/src/main/java/io/helidon/builder/api/GeneratedBuilder.java‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/*
2+
* Copyright (c) 2024 Oracle and/or its affiliates.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.helidon.builder.api;
18+
19+
import java.util.Arrays;
20+
import java.util.Optional;
21+
22+
/**
23+
* Types used from generated code.
24+
*/
25+
public final class GeneratedBuilder {
26+
private GeneratedBuilder() {
27+
}
28+
29+
/**
30+
* Utility methods for equals and hash code of specific cases of field types.
31+
*/
32+
public static class EqualityUtil {
33+
/**
34+
* Equals that uses {@link java.util.Arrays#equals(char[], char[])} in case both optionals have a value.
35+
*
36+
* @param first first optional
37+
* @param second second optional
38+
* @return whether the optionals are equals
39+
*/
40+
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
41+
public static boolean optionalCharArrayEquals(Optional<char[]> first, Optional<char[]> second) {
42+
if (first.isEmpty() && second.isEmpty()) {
43+
return true;
44+
}
45+
if (first.isEmpty() || second.isEmpty()) {
46+
return false;
47+
}
48+
return Arrays.equals(first.get(), second.get());
49+
}
50+
51+
/**
52+
* Hash code that uses {@link java.util.Arrays#hashCode(char[])} in case the optional has a value.
53+
*
54+
* @param instance instance to get hash code for
55+
* @return hash code that honors existence of char array
56+
*/
57+
@SuppressWarnings("OptionalUsedAsFieldOrParameterType")
58+
public static int optionalCharArrayHash(Optional<char[]> instance) {
59+
return instance.map(Arrays::hashCode)
60+
.orElseGet(instance::hashCode);
61+
}
62+
}
63+
}

‎builder/api/src/main/java/io/helidon/builder/api/Prototype.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public interface ConfiguredBuilder<BUILDER, PROTOTYPE> extends Builder<BUILDER,
106106
* @param configType type of the configured service
107107
* @param allFromServiceLoader whether all services from service loader should be used, or only the ones with configured
108108
* node
109-
* @param existingInstances already configured instances
109+
* @param existingInstances already configured instances
110110
* @param <S> type of the expected service
111111
* @param <T> type of the configured service provider that creates instances of S
112112
* @return list of discovered services, ordered by {@link io.helidon.common.Weight} (highest weight is first in the list)
@@ -339,7 +339,7 @@ public interface OptionDecorator<B, T> {
339339
* builder still contains previous value).
340340
* Do not call the same setter again from within this method, as it would end in a stack overflow.
341341
*
342-
* @param builder the target builder being decorated
342+
* @param builder the target builder being decorated
343343
* @param optionValue option value set by the caller of the setter method
344344
*/
345345
void decorate(B builder, T optionValue);
@@ -450,5 +450,6 @@ public interface OptionDecorator<B, T> {
450450
*/
451451
String[] value();
452452
}
453+
453454
}
454455

‎builder/codegen/src/main/java/io/helidon/builder/codegen/FactoryMethods.java‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,10 @@ private static Optional<FactoryMethod> builder(CodegenContext ctx,
121121
.filter(ElementInfoPredicates.elementName("builder"))
122122
.filter(ElementInfoPredicates::hasNoArgs)
123123
.findFirst()
124-
.map(it -> new FactoryMethod(builderCandidate, it.typeName(), "builder", null))
124+
.map(it -> new FactoryMethod(builderCandidate,
125+
copyGenericTypes(builderCandidate, it.typeName()),
126+
"builder",
127+
null))
125128
.orElse(null);
126129
if (found != null) {
127130
break;
@@ -132,6 +135,15 @@ private static Optional<FactoryMethod> builder(CodegenContext ctx,
132135
return Optional.ofNullable(found).or(() -> Optional.ofNullable(secondaryMethod));
133136
}
134137

138+
private static TypeName copyGenericTypes(TypeName from, TypeName to) {
139+
if (from.typeArguments().isEmpty()) {
140+
return to;
141+
}
142+
return TypeName.builder(to)
143+
.typeArguments(from.typeArguments())
144+
.build();
145+
}
146+
135147
private static Optional<FactoryMethod> createFromConfigMethod(CodegenContext ctx,
136148
TypeInfo blueprint,
137149
TypeHandler typeHandler,
@@ -278,7 +290,18 @@ private static Optional<FactoryMethod> targetTypeMethod(CodegenContext ctx,
278290
// MyTargetType MyTargetType.create(ConfigObject object)
279291
factoryMethodReturnType = typeHandler.actualType();
280292
typeWithFactoryMethod = factoryMethodReturnType;
281-
argumentType = Optional.of(configuredTypeInterface.get().typeName().typeArguments().get(0));
293+
TypeName argumentWithGenerics = configuredTypeInterface.get().typeName().typeArguments().get(0);
294+
if (!argumentWithGenerics.typeParameters().isEmpty() || !argumentWithGenerics.typeArguments().isEmpty()) {
295+
if (!factoryMethodReturnType.typeArguments().isEmpty()) {
296+
TypeName finalFactoryMethodReturnType = factoryMethodReturnType;
297+
argumentWithGenerics = TypeName.builder(argumentWithGenerics)
298+
.typeArguments(List.of())
299+
.typeParameters(List.of())
300+
.update(it -> finalFactoryMethodReturnType.typeArguments().forEach(it::addTypeArgument))
301+
.build();
302+
}
303+
}
304+
argumentType = Optional.of(argumentWithGenerics);
282305

283306
return Optional.of(new FactoryMethod(typeWithFactoryMethod,
284307
factoryMethodReturnType,

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

Lines changed: 57 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616

1717
package io.helidon.builder.codegen;
1818

19+
import java.util.Arrays;
1920
import java.util.Collections;
21+
import java.util.Iterator;
2022
import java.util.LinkedHashMap;
2123
import java.util.LinkedHashSet;
2224
import java.util.List;
@@ -202,7 +204,7 @@ private static void createConstructor(Constructor.Builder constructor, TypeConte
202204
for (var prop : typeContext.propertyData().overridingProperties()) {
203205
if (prop.configuredOption().hasDefault()) {
204206
constructor.addContent(prop.setterName())
205-
.addContent("(");
207+
.addContent("(");
206208
prop.configuredOption().defaultValue().accept(constructor);
207209
constructor.addContent(");");
208210
}
@@ -476,7 +478,7 @@ private static void preBuildPrototypeMethod(InnerClass.Builder classBuilder,
476478
boolean configured = typeContext.configuredData().configured();
477479
if (configured) {
478480
// need to have a non-null config instance
479-
preBuildBuilder.addContentLine("this.config = config == null ? Config.empty() : config;");
481+
preBuildBuilder.addContentLine("var config = this.config == null ? Config.empty() : this.config;");
480482
}
481483
for (PrototypeProperty property : typeContext.propertyData().properties()) {
482484
AnnotationDataOption configuredOption = property.configuredOption();
@@ -754,7 +756,8 @@ private static void equalsMethod(InnerClass.Builder classBuilder,
754756
.addContentLine("return false;")
755757
.addContentLine("}");
756758
// compare fields
757-
method.addContent("return ");
759+
method.addContent("return ")
760+
.increaseContentPadding();
758761
if (hasSuper) {
759762
method.addContent("super.equals(other)");
760763
if (!equalityFields.isEmpty()) {
@@ -764,18 +767,42 @@ private static void equalsMethod(InnerClass.Builder classBuilder,
764767
if (!hasSuper && equalityFields.isEmpty()) {
765768
method.addContent("true");
766769
} else {
767-
method.addContent(equalityFields.stream()
768-
.map(field -> {
769-
if (field.typeName().array()) {
770-
return "java.util.Arrays.equals(" + field.name() + ", other."
771-
+ field.getterName() + "())";
772-
}
773-
if (field.typeName().primitive()) {
774-
return field.name() + " == other." + field.getterName() + "()";
775-
}
776-
return "Objects.equals(" + field.name() + ", other." + field.getterName() + "())";
777-
})
778-
.collect(Collectors.joining(newLine)));
770+
Iterator<PrototypeProperty> equalIterator = equalityFields.iterator();
771+
while (equalIterator.hasNext()) {
772+
PrototypeProperty field = equalIterator.next();
773+
if (field.typeName().array()) {
774+
method.addContent(Arrays.class)
775+
.addContent(".equals(")
776+
.addContent(field.name())
777+
.addContent(", other.")
778+
.addContent(field.getterName())
779+
.addContent("())");
780+
} else if (field.typeName().primitive()) {
781+
method.addContent(field.name())
782+
.addContent(" == other.")
783+
.addContent(field.getterName())
784+
.addContent("()");
785+
} else if (field.typeName().isOptional() && field.typeHandler().actualType().equals(Types.CHAR_ARRAY)) {
786+
method.addContent(Types.GENERATED_EQUALITY_UTIL)
787+
.addContent(".optionalCharArrayEquals(")
788+
.addContent(field.name())
789+
.addContent(", other.")
790+
.addContent(field.getterName())
791+
.addContent("())");
792+
} else {
793+
method.addContent(Objects.class)
794+
.addContent(".equals(")
795+
.addContent(field.name())
796+
.addContent(", other.")
797+
.addContent(field.getterName())
798+
.addContent("())");
799+
}
800+
801+
if (equalIterator.hasNext()) {
802+
method.addContentLine("")
803+
.addContent("&& ");
804+
}
805+
}
779806
}
780807
method.addContentLine(";");
781808
classBuilder.addMethod(method);
@@ -808,9 +835,23 @@ private static void hashCodeMethod(InnerClass.Builder classBuilder,
808835
}
809836

810837
method.addContent(equalityFields.stream()
838+
.filter(it -> !(it.typeName().isOptional()
839+
&& it.typeHandler().actualType().equals(Types.CHAR_ARRAY)))
811840
.map(PrototypeProperty::name)
812841
.collect(Collectors.joining(", ")))
813-
.addContentLine(");");
842+
.addContent(")");
843+
844+
for (PrototypeProperty field : equalityFields) {
845+
if (field.typeName().isOptional() && field.typeHandler().actualType().equals(Types.CHAR_ARRAY)) {
846+
// Optional<char[]>
847+
method.addContent(" + 31 * ")
848+
.addContent(Types.GENERATED_EQUALITY_UTIL)
849+
.addContent(".optionalCharArrayHash(")
850+
.addContent(field.name())
851+
.addContent(")");
852+
}
853+
}
854+
method.addContent(";");
814855
}
815856

816857
classBuilder.addMethod(method);

‎builder/codegen/src/main/java/io/helidon/builder/codegen/TypeHandler.java‎

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.time.Duration;
2020
import java.util.ArrayList;
21+
import java.util.Iterator;
2122
import java.util.List;
2223
import java.util.Objects;
2324
import java.util.Optional;
@@ -292,36 +293,38 @@ String configGet(AnnotationDataOption configured) {
292293
return "config.get(\"" + configured.configKey() + "\")";
293294
}
294295

295-
String generateFromConfig(FactoryMethods factoryMethods) {
296+
void generateFromConfig(ContentBuilder<?> content, FactoryMethods factoryMethods) {
296297
if (actualType().fqName().equals("char[]")) {
297-
return ".asString().as(String::toCharArray)";
298-
}
299-
300-
TypeName boxed = actualType().boxed();
301-
return factoryMethods.createFromConfig()
302-
.map(it -> ".map(" + it.typeWithFactoryMethod().genericTypeName().fqName() + "::" + it.createMethodName() + ")")
303-
.orElseGet(() -> ".as(" + boxed.fqName() + ".class)");
304-
305-
}
306-
307-
void generateFromConfig(Method.Builder method, FactoryMethods factoryMethods) {
308-
if (actualType().fqName().equals("char[]")) {
309-
method.addContent(".asString().as(")
298+
content.addContent(".asString().as(")
310299
.addContent(String.class)
311300
.addContent("::toCharArray)");
312301
return;
313302
}
314303

315-
Optional<FactoryMethods.FactoryMethod> fromConfig = factoryMethods.createFromConfig();
316-
if (fromConfig.isPresent()) {
317-
FactoryMethods.FactoryMethod factoryMethod = fromConfig.get();
318-
method.addContent(".map(")
319-
.addContent(factoryMethod.typeWithFactoryMethod().genericTypeName())
320-
.addContent("::" + factoryMethod.createMethodName() + ")");
304+
Optional<FactoryMethods.FactoryMethod> factoryMethod = factoryMethods.createFromConfig();
305+
306+
TypeName boxed = actualType().boxed();
307+
if (factoryMethod.isPresent()) {
308+
FactoryMethods.FactoryMethod fm = factoryMethod.get();
309+
content.addContent(".map(")
310+
.addContent(fm.typeWithFactoryMethod().genericTypeName())
311+
.addContent("::");
312+
if (!actualType().typeArguments().isEmpty()) {
313+
content.addContent("<");
314+
Iterator<TypeName> iterator = actualType().typeArguments().iterator();
315+
while (iterator.hasNext()) {
316+
content.addContent(iterator.next());
317+
if (iterator.hasNext()) {
318+
content.addContent(", ");
319+
}
320+
}
321+
content.addContent(">");
322+
}
323+
content.addContent(fm.createMethodName())
324+
.addContent(")");
321325
} else {
322-
TypeName boxed = actualType().boxed();
323-
method.addContent(".as(")
324-
.addContent(boxed)
326+
content.addContent(".as(")
327+
.addContent(boxed.genericTypeName())
325328
.addContent(".class)");
326329
}
327330
}

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -198,9 +198,9 @@ void generateFromConfig(Method.Builder method,
198198
+ generateMapListFromConfig(factoryMethods)
199199
+ ").ifPresent(this::" + setterName() + ");");
200200
} else {
201-
method.addContentLine(configGet(configured)
202-
+ generateFromConfig(factoryMethods)
203-
+ ".ifPresent(this::" + setterName() + ");");
201+
method.addContent(configGet(configured));
202+
generateFromConfig(method, factoryMethods);
203+
method.addContentLine(".ifPresent(this::" + setterName() + ");");
204204
}
205205
} else if (BUILT_IN_MAPPERS.contains(actualType)) {
206206
// types we support in config can be simplified,
@@ -215,12 +215,12 @@ void generateFromConfig(Method.Builder method,
215215
.addContent(setterName())
216216
.addContentLine(");");
217217
} else {
218-
method.addContentLine(configGet(configured)
219-
+ ".asNodeList()"
220-
+ ".map(nodeList -> nodeList.stream()"
221-
+ ".map(cfg -> cfg"
222-
+ generateFromConfig(factoryMethods)
223-
+ ".get())"
218+
method.addContent(configGet(configured)
219+
+ ".asNodeList()"
220+
+ ".map(nodeList -> nodeList.stream()"
221+
+ ".map(cfg -> cfg");
222+
generateFromConfig(method, factoryMethods);
223+
method.addContentLine(".get())"
224224
+ "." + collector + ")"
225225
+ ".ifPresent(this::" + setterName() + ");");
226226
}

‎builder/codegen/src/main/java/io/helidon/builder/codegen/TypeHandlerMap.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,12 @@ void generateFromConfig(Method.Builder method,
120120
// the special case of Map<String, String>
121121
method.addContentLine(configGet(configured) + ".detach().asMap().ifPresent(this::" + name() + ");");
122122
} else {
123-
method.addContentLine(configGet(configured)
124-
+ ".asNodeList().ifPresent(nodes -> nodes.forEach"
125-
+ "(node -> "
126-
+ name() + ".put(node.get(\"name\").asString().orElse(node.name()), node"
127-
+ generateFromConfig(factoryMethods)
128-
+ ".get())));");
123+
method.addContent(configGet(configured)
124+
+ ".asNodeList().ifPresent(nodes -> nodes.forEach"
125+
+ "(node -> "
126+
+ name() + ".put(node.get(\"name\").asString().orElse(node.name()), node");
127+
generateFromConfig(method, factoryMethods);
128+
method.addContentLine(".get())));");
129129
}
130130
}
131131

0 commit comments

Comments
 (0)