Skip to content

Fix ignored default enum value for @ConfigProperties #9459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 20, 2020
Merged

Fix ignored default enum value for @ConfigProperties #9459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 42 additions & 39 deletions ...t/src/main/java/io/quarkus/arc/deployment/configproperties/ClassConfigPropertiesUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,14 +270,9 @@ private static ResultHandle populateConfigObject(ClassLoader classLoader, ClassI
ResultHandle mpConfig = methodCreator.getMethodParam(0);
if (fieldTypeClassInfo != null) {
if (DotNames.ENUM.equals(fieldTypeClassInfo.superName())) {
// just read the value from MP Config normally
ResultHandle value = methodCreator.invokeInterfaceMethod(
MethodDescriptor.ofMethod(Config.class, "getValue", Object.class, String.class, Class.class),
mpConfig,
methodCreator.load(getFullConfigName(prefixStr, namingStrategy, field)),
methodCreator.loadClass(fieldTypeDotName.toString()));

createWriteValue(methodCreator, configObject, field, setter, useFieldAccess, value);
populateTypicalProperty(methodCreator, configObject, configPropertyBuildItemCandidates,
currentClassInHierarchy, field, useFieldAccess, fieldType, setter, mpConfig,
getFullConfigName(prefixStr, namingStrategy, field));
} else {
if (!fieldTypeClassInfo.hasNoArgsConstructor()) {
throw new IllegalArgumentException(
Expand Down Expand Up @@ -328,37 +323,9 @@ private static ResultHandle populateConfigObject(ClassLoader classLoader, ClassI
MethodDescriptor.ofMethod(Optional.class, "empty", Optional.class)));
}
} else {
/*
* We want to support cases where the Config class defines a default value for fields
* by simply specifying the default value in its constructor
* For such cases the strategy we follow is that when a requested property does not exist
* we check the value from the corresponding getter (or read the field value if possible)
* and if the value is not null we don't fail
*/
if (shouldCheckForDefaultValue(currentClassInHierarchy, field)) {
String getterName = JavaBeanUtil.getGetterName(field.name(), fieldTypeDotName.toString());

ReadOptionalResponse readOptionalResponse = createReadOptionalValueAndConvertIfNeeded(
fullConfigName,
fieldType, field.declaringClass().name(), methodCreator, mpConfig);

// call the setter if the optional contained data
createWriteValue(readOptionalResponse.getIsPresentTrue(), configObject, field, setter,
useFieldAccess,
readOptionalResponse.getValue());
} else {
/*
* In this case we want a missing property to cause an exception that we don't handle
* So we call config.getValue making sure to handle collection values
*/
ResultHandle setterValue = createReadMandatoryValueAndConvertIfNeeded(
fullConfigName, fieldType,
field.declaringClass().name(), methodCreator, mpConfig);
createWriteValue(methodCreator, configObject, field, setter, useFieldAccess, setterValue);

}
configPropertyBuildItemCandidates
.add(new ConfigPropertyBuildItemCandidate(field.name(), fullConfigName, fieldType));
populateTypicalProperty(methodCreator, configObject, configPropertyBuildItemCandidates,
currentClassInHierarchy, field, useFieldAccess, fieldType, setter, mpConfig,
fullConfigName);
}
}
}
Expand Down Expand Up @@ -393,6 +360,42 @@ private static ResultHandle populateConfigObject(ClassLoader classLoader, ClassI
return configObject;
}

// creates the bytecode needed to populate anything other than a nested config object or an optional
private static void populateTypicalProperty(MethodCreator methodCreator, ResultHandle configObject,
List<ConfigPropertyBuildItemCandidate> configPropertyBuildItemCandidates, ClassInfo currentClassInHierarchy,
FieldInfo field, boolean useFieldAccess, Type fieldType, MethodInfo setter,
ResultHandle mpConfig, String fullConfigName) {
/*
* We want to support cases where the Config class defines a default value for fields
* by simply specifying the default value in its constructor
* For such cases the strategy we follow is that when a requested property does not exist
* we check the value from the corresponding getter (or read the field value if possible)
* and if the value is not null we don't fail
*/
if (shouldCheckForDefaultValue(currentClassInHierarchy, field)) {
ReadOptionalResponse readOptionalResponse = createReadOptionalValueAndConvertIfNeeded(
fullConfigName,
fieldType, field.declaringClass().name(), methodCreator, mpConfig);

// call the setter if the optional contained data
createWriteValue(readOptionalResponse.getIsPresentTrue(), configObject, field, setter,
useFieldAccess,
readOptionalResponse.getValue());
} else {
/*
* In this case we want a missing property to cause an exception that we don't handle
* So we call config.getValue making sure to handle collection values
*/
ResultHandle setterValue = createReadMandatoryValueAndConvertIfNeeded(
fullConfigName, fieldType,
field.declaringClass().name(), methodCreator, mpConfig);
createWriteValue(methodCreator, configObject, field, setter, useFieldAccess, setterValue);

}
configPropertyBuildItemCandidates
.add(new ConfigPropertyBuildItemCandidate(field.name(), fullConfigName, fieldType));
}

private static String getFullConfigName(String prefixStr, ConfigProperties.NamingStrategy namingStrategy, FieldInfo field) {
return prefixStr + "." + namingStrategy.getName(field.name());
}
Expand Down
6 changes: 5 additions & 1 deletion ...st/java/io/quarkus/arc/test/configproperties/ClassWithoutGettersConfigPropertiesTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class ClassWithoutGettersConfigPropertiesTest {
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(DummyBean.class, DummyProperties.class)
.addAsResource(new StringAsset(
"dummy.name=quarkus\ndummy.my-enum=OPTIONAL\ndummy.numbers=1,2,3,4\ndummy.unused=whatever"),
"dummy.name=quarkus\ndummy.my-enum=OPTIONAL\ndummy.other-enum=Enum_Two\ndummy.numbers=1,2,3,4\ndummy.unused=whatever"),
"application.properties"));

@Inject
Expand All @@ -36,6 +36,8 @@ public void testConfiguredValues() {
assertEquals("quarkus", dummyProperties.name);
assertEquals(Arrays.asList(1, 2, 3, 4), dummyProperties.numbers);
assertEquals(MyEnum.OPTIONAL, dummyProperties.myEnum);
assertEquals(MyEnum.ENUM_ONE, dummyProperties.unsetEnum);
assertEquals(MyEnum.Enum_Two, dummyProperties.otherEnum);
}

@Singleton
Expand All @@ -50,6 +52,8 @@ public static class DummyProperties {
public String name;
public List<Integer> numbers;
public MyEnum myEnum;
public MyEnum otherEnum = MyEnum.ENUM_ONE;
public MyEnum unsetEnum = MyEnum.ENUM_ONE;

public void setName(String name) {
this.name = name;
Expand Down