From fed18fb5ed8156954f1b9005a284ecdf2cecffe9 Mon Sep 17 00:00:00 2001 From: Roberto Cortez Date: Mon, 8 Aug 2022 21:34:58 +0100 Subject: [PATCH] Do not fail with NoSuchElement when expanding Optionals --- .../config/inject/ConfigExtension.java | 14 ++-- .../config/inject/ConfigProducer.java | 4 +- .../config/inject/ConfigProducerUtil.java | 68 ++++++++----------- .../config/inject/ConfigInjectionTest.java | 27 +++++++- .../config/ConfigValidationException.java | 18 ++++- .../java/io/smallrye/config/ConfigValue.java | 42 +++++++++++- .../java/io/smallrye/config/Converters.java | 31 +++++---- .../ExpressionConfigSourceInterceptor.java | 22 +++--- .../io/smallrye/config/SmallRyeConfig.java | 62 ++++++++++------- .../config/ConfigMappingInterfaceTest.java | 24 ++++++- ...ExpressionConfigSourceInterceptorTest.java | 35 ++++++++-- 11 files changed, 238 insertions(+), 109 deletions(-) diff --git a/cdi/src/main/java/io/smallrye/config/inject/ConfigExtension.java b/cdi/src/main/java/io/smallrye/config/inject/ConfigExtension.java index 8d53e7a78..f1755629a 100644 --- a/cdi/src/main/java/io/smallrye/config/inject/ConfigExtension.java +++ b/cdi/src/main/java/io/smallrye/config/inject/ConfigExtension.java @@ -52,7 +52,6 @@ import javax.enterprise.inject.spi.WithAnnotations; import javax.inject.Provider; -import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.config.ConfigProvider; import org.eclipse.microprofile.config.inject.ConfigProperties; import org.eclipse.microprofile.config.inject.ConfigProperty; @@ -144,7 +143,7 @@ protected void registerCustomBeans(@Observes AfterBeanDiscovery abd, BeanManager } protected void validate(@Observes AfterDeploymentValidation adv) { - Config config = ConfigProvider.getConfig(getContextClassLoader()); + SmallRyeConfig config = ConfigProvider.getConfig(getContextClassLoader()).unwrap(SmallRyeConfig.class); Set configNames = StreamSupport.stream(config.getPropertyNames().spliterator(), false).collect(toSet()); for (InjectionPoint injectionPoint : getConfigPropertyInjectionPoints()) { Type type = injectionPoint.getType(); @@ -176,7 +175,7 @@ protected void validate(@Observes AfterDeploymentValidation adv) { // Since properties can be a subset, then search for the actual property for a value. // Check if it is a map // Finally also check if the property is indexed (might be a Collection with indexed properties). - if ((!configNames.contains(name) && ConfigProducerUtil.getRawValue(name, config) == null) + if ((!configNames.contains(name) && ConfigProducerUtil.getConfigValue(name, config).getValue() == null) && !isMap(type) && !isIndexed(type, name, config)) { if (configProperty.defaultValue().equals(ConfigProperty.UNCONFIGURED_VALUE)) { adv.addDeploymentProblem( @@ -200,8 +199,8 @@ protected void validate(@Observes AfterDeploymentValidation adv) { configPropertiesInjectionPoints); try { - registerConfigMappings(config.unwrap(SmallRyeConfig.class), configMappingsWithPrefix); - registerConfigProperties(config.unwrap(SmallRyeConfig.class), configPropertiesWithPrefix); + registerConfigMappings(config, configMappingsWithPrefix); + registerConfigProperties(config, configPropertiesWithPrefix); } catch (ConfigValidationException e) { adv.addDeploymentProblem(e); } @@ -227,12 +226,11 @@ private static Set mapToConfigObjectWithPrefix( return configMappingsWithPrefix; } - private static boolean isIndexed(Type type, String name, Config config) { + private static boolean isIndexed(Type type, String name, SmallRyeConfig config) { return type instanceof ParameterizedType && (List.class.isAssignableFrom((Class) ((ParameterizedType) type).getRawType()) || Set.class.isAssignableFrom((Class) ((ParameterizedType) type).getRawType())) - && - !((SmallRyeConfig) config).getIndexedPropertiesIndexes(name).isEmpty(); + && !config.getIndexedPropertiesIndexes(name).isEmpty(); } /** diff --git a/cdi/src/main/java/io/smallrye/config/inject/ConfigProducer.java b/cdi/src/main/java/io/smallrye/config/inject/ConfigProducer.java index 69cfbf2a3..cb454d75f 100644 --- a/cdi/src/main/java/io/smallrye/config/inject/ConfigProducer.java +++ b/cdi/src/main/java/io/smallrye/config/inject/ConfigProducer.java @@ -111,14 +111,14 @@ protected Character produceCharacterConfigProperty(InjectionPoint ip) { @Dependent @Produces @ConfigProperty - protected Optional produceOptionalConfigValue(InjectionPoint ip) { + protected Optional produceOptionalConfigProperty(InjectionPoint ip) { return ConfigProducerUtil.getValue(ip, getConfig()); } @Dependent @Produces @ConfigProperty - protected Supplier produceSupplierConfigValue(InjectionPoint ip) { + protected Supplier produceSupplierConfigProperty(InjectionPoint ip) { return () -> ConfigProducerUtil.getValue(ip, getConfig()); } diff --git a/cdi/src/main/java/io/smallrye/config/inject/ConfigProducerUtil.java b/cdi/src/main/java/io/smallrye/config/inject/ConfigProducerUtil.java index 53da59352..1946288b2 100644 --- a/cdi/src/main/java/io/smallrye/config/inject/ConfigProducerUtil.java +++ b/cdi/src/main/java/io/smallrye/config/inject/ConfigProducerUtil.java @@ -72,38 +72,29 @@ public static T getValue(String name, Type type, String defaultValue, Config if (name == null) { return null; } + + final SmallRyeConfig smallRyeConfig = config.unwrap(SmallRyeConfig.class); + final io.smallrye.config.ConfigValue configValue = getConfigValue(name, smallRyeConfig); + final io.smallrye.config.ConfigValue configValueWithDefault = configValue + .withValue(resolveDefault(configValue.getValue(), defaultValue)); + if (hasCollection(type)) { - return convertValues(name, type, getRawValue(name, config), defaultValue, config); + return convertValues(configValueWithDefault, type, smallRyeConfig); } else if (hasMap(type)) { - return convertValues(name, type, defaultValue, config); + return smallRyeConfig.convertValue(configValueWithDefault, + resolveConverter(type, smallRyeConfig, + (kC, vC) -> new StaticMapConverter<>(configValueWithDefault.getName(), + configValueWithDefault.getValue(), config, kC, vC))); } - return ((SmallRyeConfig) config).convertValue(name, resolveDefault(getRawValue(name, config), defaultValue), - resolveConverter(type, config)); + return smallRyeConfig.convertValue(configValueWithDefault, resolveConverter(type, smallRyeConfig)); } - /** - * Converts the direct sub properties of the given parent property as a Map. - * - * @param name the name of the parent property for which we want the direct sub properties as a Map. - * @param type the {@link Type} of the configuration value to convert. - * @param defaultValue the default value to convert in case no sub properties could be found. - * @param config the configuration from which the values are retrieved. - * @param the expected type of the configuration value to convert. - * - * @return the converted configuration value. - */ - private static T convertValues(String name, Type type, String defaultValue, Config config) { - return ((SmallRyeConfig) config).convertValue(name, null, - resolveConverter(type, config, (kC, vC) -> new StaticMapConverter<>(name, defaultValue, config, kC, vC))); - } - - private static T convertValues(String name, Type type, String rawValue, String defaultValue, Config config) { - List indexedProperties = ((SmallRyeConfig) config).getIndexedProperties(name); + private static T convertValues(io.smallrye.config.ConfigValue configValue, Type type, SmallRyeConfig config) { + List indexedProperties = config.getIndexedProperties(configValue.getName()); // If converting a config property which exists (i.e. myProp[1] = aValue) or no indexed properties exist for the config property - if (rawValue != null || indexedProperties.isEmpty()) { - return ((SmallRyeConfig) config).convertValue(name, resolveDefault(rawValue, defaultValue), - resolveConverter(type, config)); + if (configValue.getRawValue() != null || indexedProperties.isEmpty()) { + return config.convertValue(configValue, resolveConverter(type, config)); } BiFunction, IntFunction>, Collection> indexedConverter = (itemConverter, @@ -111,9 +102,7 @@ private static T convertValues(String name, Type type, String rawValue, Stri Collection collection = collectionFactory.apply(indexedProperties.size()); for (String indexedProperty : indexedProperties) { // Never null by the rules of converValue - collection.add( - ((SmallRyeConfig) config).convertValue(indexedProperty, getRawValue(indexedProperty, config), - itemConverter)); + collection.add(config.convertValue(getConfigValue(indexedProperty, config), itemConverter)); } return collection; }; @@ -121,37 +110,38 @@ private static T convertValues(String name, Type type, String rawValue, Stri return resolveConverterForIndexed(type, config, indexedConverter).convert(" "); } - static ConfigValue getConfigValue(InjectionPoint injectionPoint, Config config) { + static ConfigValue getConfigValue(InjectionPoint injectionPoint, SmallRyeConfig config) { String name = getName(injectionPoint); if (name == null) { return null; } - ConfigValue configValue = config.getConfigValue(name); + io.smallrye.config.ConfigValue configValue = config.getConfigValue(name); if (configValue.getRawValue() == null) { - if (configValue instanceof io.smallrye.config.ConfigValue) { - configValue = ((io.smallrye.config.ConfigValue) configValue).withValue(getDefaultValue(injectionPoint)); - } + configValue = configValue.withValue(getDefaultValue(injectionPoint)); } return configValue; } - static String getRawValue(String name, Config config) { - return SecretKeys.doUnlocked(() -> config.getConfigValue(name).getValue()); + static io.smallrye.config.ConfigValue getConfigValue(String name, SmallRyeConfig config) { + return SecretKeys.doUnlocked(() -> config.getConfigValue(name)); } private static String resolveDefault(String rawValue, String defaultValue) { return rawValue != null ? rawValue : defaultValue; } - private static Converter resolveConverter(final Type type, final Config config) { + private static Converter resolveConverter(final Type type, final SmallRyeConfig config) { return resolveConverter(type, config, Converters::newMapConverter); } @SuppressWarnings("unchecked") - private static Converter resolveConverter(final Type type, final Config config, + private static Converter resolveConverter( + final Type type, + final SmallRyeConfig config, final BiFunction, Converter, Converter>> mapConverterFactory) { + Class rawType = rawTypeOf(type); if (type instanceof ParameterizedType) { ParameterizedType paramType = (ParameterizedType) type; @@ -179,7 +169,7 @@ private static Converter resolveConverter(final Type type, final Config c /** * We need to handle indexed properties in a special way, since a Collection may be wrapped in other converters. - * The issue is that in the original code the value was retrieve by calling the first converter that will delegate + * The issue is that in the original code the value was retrieved by calling the first converter that will delegate * to all the wrapped types until it finally gets the result. For indexed properties, because it requires * additional key lookups, a special converter is added to perform the work. This is mostly a workaround, since * converters do not have the proper API, and probably should not have to handle this type of logic. @@ -189,7 +179,7 @@ private static Converter resolveConverter(final Type type, final Config c @SuppressWarnings("unchecked") private static Converter resolveConverterForIndexed( final Type type, - final Config config, + final SmallRyeConfig config, final BiFunction, IntFunction>, Collection> indexedConverter) { Class rawType = rawTypeOf(type); diff --git a/cdi/src/test/java/io/smallrye/config/inject/ConfigInjectionTest.java b/cdi/src/test/java/io/smallrye/config/inject/ConfigInjectionTest.java index 423fcae61..88bc6e859 100644 --- a/cdi/src/test/java/io/smallrye/config/inject/ConfigInjectionTest.java +++ b/cdi/src/test/java/io/smallrye/config/inject/ConfigInjectionTest.java @@ -90,10 +90,12 @@ void inject() { assertEquals("5678", configBean.getMyPropProfile()); assertThrows(SecurityException.class, () -> configBean.getConfig().getValue("secret", String.class), "Not allowed to access secret key secret"); - assertEquals("1234", configBean.getConfig().getValue("my.prop", String.class)); assertEquals("1234", configBean.getSmallRyeConfig().getRawValue("my.prop")); assertEquals(HyphenatedEnum.A_B, configBean.getHyphenatedEnum()); + assertNull(configBean.getMissingExpression().getValue()); + assertFalse(configBean.getMissingExpressionOptional().isPresent()); + assertFalse(configBean.getMissingExpressionOptionalInt().isPresent()); } @Test @@ -195,6 +197,15 @@ static class ConfigBean { @Inject @ConfigProperty(name = "hyphenated.enum") HyphenatedEnum hyphenatedEnum; + @Inject + @ConfigProperty(name = "missing.expression") + ConfigValue missingExpression; + @Inject + @ConfigProperty(name = "missing.expression") + Optional missingExpressionOptional; + @Inject + @ConfigProperty(name = "missing.expression") + Optional missingExpressionOptionalInt; Optional> getReasonsOptional() { return reasonsOptional; @@ -283,6 +294,18 @@ Optional getConvertedValueOptional() { HyphenatedEnum getHyphenatedEnum() { return hyphenatedEnum; } + + ConfigValue getMissingExpression() { + return missingExpression; + } + + Optional getMissingExpressionOptional() { + return missingExpressionOptional; + } + + Optional getMissingExpressionOptionalInt() { + return missingExpressionOptionalInt; + } } @BeforeAll @@ -290,7 +313,7 @@ static void beforeAll() { SmallRyeConfig config = new SmallRyeConfigBuilder() .withSources(config("my.prop", "1234", "expansion", "${my.prop}", "secret", "12345678", "empty", "", "mp.config.profile", "prof", "my.prop.profile", "1234", "%prof.my.prop.profile", "5678", - "bad.property.expression.prop", "${missing.prop}", "reasons.200", "OK", "reasons.201", "Created", + "missing.expression", "${missing.prop}", "reasons.200", "OK", "reasons.201", "Created", "versions.v1", "1.The version 1.2.3", "versions.v1.2", "1.The version 1.2.0", "versions.v2", "2.The version 2.0.0", "lnums.even", "2,4,6,8", "lnums.odd", "1,3,5,7,9", diff --git a/implementation/src/main/java/io/smallrye/config/ConfigValidationException.java b/implementation/src/main/java/io/smallrye/config/ConfigValidationException.java index bcd8957fb..07c0a94b7 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigValidationException.java +++ b/implementation/src/main/java/io/smallrye/config/ConfigValidationException.java @@ -1,6 +1,7 @@ package io.smallrye.config; import java.io.Serializable; +import java.util.Optional; import io.smallrye.common.constraint.Assert; @@ -18,13 +19,13 @@ public class ConfigValidationException extends RuntimeException { * @param problems the reported problems */ public ConfigValidationException(final Problem[] problems) { - super(list("Configuration validation failed", problems)); + super(list(problems)); this.problems = problems; } - private static String list(String msg, Problem[] problems) { + private static String list(Problem[] problems) { StringBuilder b = new StringBuilder(); - b.append(msg).append(':'); + b.append("Configuration validation failed").append(':'); for (int i = 0; i < problems.length; i++) { Problem problem = problems[i]; Assert.checkNotNullArrayParam("problems", i, problem); @@ -48,13 +49,24 @@ public static final class Problem implements Serializable { private static final long serialVersionUID = 5984436393578154541L; private final String message; + transient private final RuntimeException exception; public Problem(final String message) { this.message = message; + this.exception = null; + } + + Problem(final RuntimeException exception) { + this.message = exception.getMessage(); + this.exception = exception; } public String getMessage() { return message; } + + Optional getException() { + return Optional.ofNullable(exception); + } } } diff --git a/implementation/src/main/java/io/smallrye/config/ConfigValue.java b/implementation/src/main/java/io/smallrye/config/ConfigValue.java index a94a369a1..7a8fcbc53 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigValue.java +++ b/implementation/src/main/java/io/smallrye/config/ConfigValue.java @@ -1,9 +1,13 @@ package io.smallrye.config; +import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; +import java.util.List; import java.util.Objects; import io.smallrye.common.annotation.Experimental; +import io.smallrye.config.ConfigValidationException.Problem; /** * The ConfigValue is a metadata object that holds additional information after the lookup of a configuration. @@ -28,6 +32,8 @@ public class ConfigValue implements org.eclipse.microprofile.config.ConfigValue private final int configSourcePosition; private final int lineNumber; + private final List problems; + private ConfigValue(final ConfigValueBuilder builder) { this.name = builder.name; this.value = builder.value; @@ -37,6 +43,7 @@ private ConfigValue(final ConfigValueBuilder builder) { this.configSourceOrdinal = builder.configSourceOrdinal; this.configSourcePosition = builder.configSourcePosition; this.lineNumber = builder.lineNumber; + this.problems = builder.problems; } @Override @@ -92,6 +99,10 @@ public String getLocation() { return lineNumber != -1 ? configSourceName + ":" + lineNumber : configSourceName; } + List getProblems() { + return Collections.unmodifiableList(problems); + } + public ConfigValue withName(final String name) { return from().withName(name).build(); } @@ -120,6 +131,18 @@ public ConfigValue withLineNumber(final int lineNumber) { return from().withLineNumber(lineNumber).build(); } + public ConfigValue noProblems() { + return from().noProblems().build(); + } + + public ConfigValue withProblems(final List problems) { + return from().withProblems(problems).build(); + } + + public ConfigValue withProblem(final Problem problem) { + return from().addProblem(problem).build(); + } + @Override public boolean equals(final Object o) { if (this == o) { @@ -166,7 +189,8 @@ ConfigValueBuilder from() { .withConfigSourceName(configSourceName) .withConfigSourceOrdinal(configSourceOrdinal) .withConfigSourcePosition(configSourcePosition) - .withLineNumber(lineNumber); + .withLineNumber(lineNumber) + .withProblems(problems); } public static ConfigValueBuilder builder() { @@ -182,6 +206,7 @@ public static class ConfigValueBuilder { private int configSourceOrdinal; private int configSourcePosition; private int lineNumber = -1; + private final List problems = new ArrayList<>(); public ConfigValueBuilder withName(final String name) { this.name = name; @@ -223,6 +248,21 @@ public ConfigValueBuilder withLineNumber(final int lineNumber) { return this; } + public ConfigValueBuilder noProblems() { + this.problems.clear(); + return this; + } + + public ConfigValueBuilder withProblems(final List problems) { + this.problems.addAll(problems); + return this; + } + + public ConfigValueBuilder addProblem(final Problem problem) { + this.problems.add(problem); + return this; + } + public ConfigValue build() { return new ConfigValue(this); } diff --git a/implementation/src/main/java/io/smallrye/config/Converters.java b/implementation/src/main/java/io/smallrye/config/Converters.java index 9c3e2c639..12946de29 100644 --- a/implementation/src/main/java/io/smallrye/config/Converters.java +++ b/implementation/src/main/java/io/smallrye/config/Converters.java @@ -617,6 +617,13 @@ public static Converter patternValidatingConverter(Converter return patternValidatingConverter(delegate, Pattern.compile(pattern)); } + static boolean isOptionalConverter(Converter converter) { + return converter instanceof Converters.OptionalConverter || + converter.equals(Converters.OPTIONAL_INT_CONVERTER) || + converter.equals(Converters.OPTIONAL_LONG_CONVERTER) || + converter.equals(Converters.OPTIONAL_DOUBLE_CONVERTER); + } + static final class PatternCheckConverter implements Converter, Serializable { private static final long serialVersionUID = 358813973126582008L; @@ -709,7 +716,7 @@ static final class CollectionConverter> extends Abstr this.collectionFactory = collectionFactory; } - protected Converter create(final Converter newDelegate) { + Converter create(final Converter newDelegate) { return new CollectionConverter<>(newDelegate, collectionFactory); } @@ -742,7 +749,7 @@ static final class ArrayConverter extends AbstractDelegatingConverter create(final Converter newDelegate) { + ArrayConverter create(final Converter newDelegate) { return new ArrayConverter<>(newDelegate, arrayType); } @@ -797,8 +804,8 @@ static final class OptionalConverter extends AbstractDelegatingConverter create(final Converter newDelegate) { - return new OptionalConverter(newDelegate); + OptionalConverter create(final Converter newDelegate) { + return new OptionalConverter<>(newDelegate); } public Optional convert(final String value) { @@ -817,11 +824,11 @@ public Optional convert(final String value) { static final class OptionalIntConverter extends AbstractDelegatingConverter { private static final long serialVersionUID = 4331039532024222756L; - protected OptionalIntConverter(final Converter delegate) { + OptionalIntConverter(final Converter delegate) { super(delegate); } - protected OptionalIntConverter create(final Converter newDelegate) { + OptionalIntConverter create(final Converter newDelegate) { return new OptionalIntConverter(newDelegate); } @@ -830,7 +837,7 @@ public OptionalInt convert(final String value) { return OptionalInt.empty(); } else { final Integer converted = getDelegate().convert(value); - return converted == null ? OptionalInt.empty() : OptionalInt.of(converted.intValue()); + return converted == null ? OptionalInt.empty() : OptionalInt.of(converted); } } } @@ -838,11 +845,11 @@ public OptionalInt convert(final String value) { static final class OptionalLongConverter extends AbstractDelegatingConverter { private static final long serialVersionUID = 140937551800590852L; - protected OptionalLongConverter(final Converter delegate) { + OptionalLongConverter(final Converter delegate) { super(delegate); } - protected OptionalLongConverter create(final Converter newDelegate) { + OptionalLongConverter create(final Converter newDelegate) { return new OptionalLongConverter(newDelegate); } @@ -851,7 +858,7 @@ public OptionalLong convert(final String value) { return OptionalLong.empty(); } else { final Long converted = getDelegate().convert(value); - return converted == null ? OptionalLong.empty() : OptionalLong.of(converted.longValue()); + return converted == null ? OptionalLong.empty() : OptionalLong.of(converted); } } } @@ -863,7 +870,7 @@ static final class OptionalDoubleConverter extends AbstractDelegatingConverter newDelegate) { + OptionalDoubleConverter create(final Converter newDelegate) { return new OptionalDoubleConverter(newDelegate); } @@ -872,7 +879,7 @@ public OptionalDouble convert(final String value) { return OptionalDouble.empty(); } else { final Double converted = getDelegate().convert(value); - return converted == null ? OptionalDouble.empty() : OptionalDouble.of(converted.doubleValue()); + return converted == null ? OptionalDouble.empty() : OptionalDouble.of(converted); } } } diff --git a/implementation/src/main/java/io/smallrye/config/ExpressionConfigSourceInterceptor.java b/implementation/src/main/java/io/smallrye/config/ExpressionConfigSourceInterceptor.java index 8372d0e07..22bc4b018 100644 --- a/implementation/src/main/java/io/smallrye/config/ExpressionConfigSourceInterceptor.java +++ b/implementation/src/main/java/io/smallrye/config/ExpressionConfigSourceInterceptor.java @@ -3,7 +3,10 @@ import static io.smallrye.common.expression.Expression.Flag.LENIENT_SYNTAX; import static io.smallrye.common.expression.Expression.Flag.NO_SMART_BRACES; import static io.smallrye.common.expression.Expression.Flag.NO_TRIM; +import static io.smallrye.config.ConfigMessages.msg; +import java.util.ArrayList; +import java.util.List; import java.util.Optional; import java.util.function.BiConsumer; @@ -13,6 +16,7 @@ import io.smallrye.common.expression.Expression; import io.smallrye.common.expression.ResolveContext; +import io.smallrye.config.ConfigValidationException.Problem; @Priority(Priorities.LIBRARY + 300) public class ExpressionConfigSourceInterceptor implements ConfigSourceInterceptor { @@ -40,10 +44,10 @@ public ConfigValue getValue(final ConfigSourceInterceptorContext context, final private ConfigValue getValue(final ConfigSourceInterceptorContext context, final String name, final int depth) { if (depth == MAX_DEPTH) { - throw ConfigMessages.msg.expressionExpansionTooDepth(name); + throw msg.expressionExpansionTooDepth(name); } - final ConfigValue configValue = context.proceed(name); + ConfigValue configValue = context.proceed(name); if (!Expressions.isEnabled() || !enabled) { return configValue; @@ -53,30 +57,30 @@ private ConfigValue getValue(final ConfigSourceInterceptorContext context, final return null; } - final Expression expression = Expression.compile(escapeDollarIfExists(configValue.getValue()), LENIENT_SYNTAX, NO_TRIM, + List problems = new ArrayList<>(); + Expression expression = Expression.compile(escapeDollarIfExists(configValue.getValue()), LENIENT_SYNTAX, NO_TRIM, NO_SMART_BRACES); - final String expanded = expression.evaluate(new BiConsumer, StringBuilder>() { + String expanded = expression.evaluate(new BiConsumer, StringBuilder>() { @Override - public void accept(ResolveContext resolveContext, - StringBuilder stringBuilder) { + public void accept(ResolveContext resolveContext, StringBuilder stringBuilder) { final ConfigValue resolve = getValue(context, resolveContext.getKey(), depth + 1); if (resolve != null) { stringBuilder.append(resolve.getValue()); } else if (resolveContext.hasDefault()) { resolveContext.expandDefault(); } else { - throw ConfigMessages.msg.expandingElementNotFound(resolveContext.getKey(), configValue.getName()); + problems.add(new Problem(msg.expandingElementNotFound(resolveContext.getKey(), configValue.getName()))); } } }); - return configValue.withValue(expanded); + return problems.isEmpty() ? configValue.withValue(expanded) : configValue.withValue(null).withProblems(problems); } /** * MicroProfile Config defines the backslash escape for dollar to retrieve the raw expression. We don't want to * turn {@link Expression.Flag#ESCAPES} on because it may break working configurations. - * + *
* This will replace the expected escape in MicroProfile Config by the escape used in {@link Expression}, a double * dollar. */ diff --git a/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java b/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java index 82ec9a0ef..51f64f316 100644 --- a/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java +++ b/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java @@ -209,8 +209,9 @@ public Map getValuesAsMap(String name, Converter keyConverter, C // Ignore sub namespaces continue; } - result.put(convertValue(propertyName + "#key", key, keyConverter), - convertValue(propertyName + "#value", getRawValue(propertyName), valueConverter)); + ConfigValue configValueKey = ConfigValue.builder().withName(propertyName + "#key").withValue(key).build(); + ConfigValue configValue = getConfigValue(propertyName).withName(propertyName + "#value"); + result.put(convertValue(configValueKey, keyConverter), convertValue(configValue, valueConverter)); } } return result.isEmpty() ? null : result; @@ -222,25 +223,22 @@ public Map getValuesAsMap(String name, Converter keyConverter, C */ @SuppressWarnings("unchecked") public T getValue(String name, Converter converter) { - final ConfigValue configValue = getConfigValue(name); + ConfigValue configValue = getConfigValue(name); if (ConfigValueConverter.CONFIG_VALUE_CONVERTER.equals(converter)) { - return (T) configValue; + return (T) configValue.noProblems(); } if (converter instanceof Converters.OptionalConverter) { if (ConfigValueConverter.CONFIG_VALUE_CONVERTER.equals( ((Converters.OptionalConverter) converter).getDelegate())) { - return (T) Optional.of(configValue); + return (T) Optional.of(configValue.noProblems()); } } - final String value = configValue.getValue(); // Can return the empty String (which is not considered as null) - - return convertValue(name, value, converter); + return convertValue(configValue, converter); } /** - * * This method handles converting values for both CDI injections and programatical calls.
*
* @@ -258,32 +256,46 @@ public T getValue(String name, Converter converter) { * should only throw an {@link Exception} for #1 ({@link IllegalArgumentException} when the property cannot be converted to * the specified type). */ - public T convertValue(String name, String value, Converter converter) { + public T convertValue(ConfigValue configValue, Converter converter) { + List problems = configValue.getProblems(); + if (!problems.isEmpty()) { + // TODO - Maybe it will depend on the problem, but we only get the expression NoSuchElement here for now + if (Converters.isOptionalConverter(converter)) { + configValue = configValue.noProblems(); + } else { + ConfigValidationException.Problem problem = problems.get(0); + if (problem.getException().isPresent()) { + throw problem.getException().get(); + } + } + } final T converted; - if (value != null) { + if (configValue.getValue() != null) { try { - converted = converter.convert(value); + converted = converter.convert(configValue.getValue()); } catch (IllegalArgumentException e) { - throw ConfigMessages.msg.converterException(e, name, value, e.getLocalizedMessage()); // 1 + throw ConfigMessages.msg.converterException(e, configValue.getName(), configValue.getValue(), + e.getLocalizedMessage()); // 1 } } else { try { // See if the Converter is designed to handle a missing (null) value i.e. Optional Converters converted = converter.convert(""); } catch (IllegalArgumentException ignored) { - throw new NoSuchElementException(ConfigMessages.msg.propertyNotFound(name)); // 2 + throw new NoSuchElementException(ConfigMessages.msg.propertyNotFound(configValue.getName())); // 2 } } if (converted == null) { - if (value == null) { - throw new NoSuchElementException(ConfigMessages.msg.propertyNotFound(name)); // 2 - } else if (value.length() == 0) { - throw ConfigMessages.msg.propertyEmptyString(name, converter.getClass().getTypeName()); // 3 + if (configValue.getValue() == null) { + throw new NoSuchElementException(ConfigMessages.msg.propertyNotFound(configValue.getName())); // 2 + } else if (configValue.getValue().length() == 0) { + throw ConfigMessages.msg.propertyEmptyString(configValue.getName(), converter.getClass().getTypeName()); // 3 } else { - throw ConfigMessages.msg.converterReturnedNull(name, value, converter.getClass().getTypeName()); // 4 + throw ConfigMessages.msg.converterReturnedNull(configValue.getName(), configValue.getValue(), + converter.getClass().getTypeName()); // 4 } } @@ -405,11 +417,11 @@ public Iterable getPropertyNames() { /** * Checks if a property is present in the {@link Config} instance. - * + *
* Because {@link ConfigSource#getPropertyNames()} may not include all available properties, it is not possible to - * reliable determine if the property is present in the properties list. The property needs to be retrieved to make + * reliably determine if the property is present in the properties list. The property needs to be retrieved to make * sure it exists. The lookup is done without expression expansion, because the expansion value may not be - * available and it not relevant for the final check. + * available, and it is not relevant for the final check. * * @param name the property name. * @return true if the property is present or false otherwise. @@ -697,18 +709,18 @@ private static List getConfigurableSources(final List< /** * Generate dotted properties from Env properties. - * + *
* These are required when a consumer relies on the list of properties to find additional * configurations. The list of properties is not normalized due to environment variables, which follow specific * naming rules. The MicroProfile Config specification defines a set of conversion rules to look up and find * values from environment variables even when using their dotted version, but this does not apply to the * properties list. - * + *
* Because an environment variable name may only be represented by a subset of characters, it is not possible * to represent exactly a dotted version name from an environment variable name. Additional dotted properties * mapped from environment variables are only added if a relationship cannot be found between all properties * using the conversions look up rules of the MicroProfile Config specification. Example: - * + *
* If foo.bar is present and FOO_BAR is also present, no property is required. * If foo-bar is present and FOO_BAR is also present, no property is required. * If FOO_BAR is present a property foo.bar is required. diff --git a/implementation/src/test/java/io/smallrye/config/ConfigMappingInterfaceTest.java b/implementation/src/test/java/io/smallrye/config/ConfigMappingInterfaceTest.java index cdab95343..9488e5c98 100644 --- a/implementation/src/test/java/io/smallrye/config/ConfigMappingInterfaceTest.java +++ b/implementation/src/test/java/io/smallrye/config/ConfigMappingInterfaceTest.java @@ -1490,7 +1490,7 @@ void nestedOptionalsGroupMap() { } @ConfigMapping(prefix = "optional-map") - public interface NestedOptionalMapGroup { + interface NestedOptionalMapGroup { Optional enable(); Map> map(); @@ -1531,4 +1531,26 @@ void nestedOptionalAndMaps() { assertTrue(mapping.map().get("client").get("setup-api").enable().isPresent()); assertTrue(mapping.map().get("client").get("setup-api").enable().get()); } + + @ConfigMapping(prefix = "optional") + interface OptionalExpressions { + Optional expression(); + + OptionalInt expressionInt(); + } + + @Test + void optionalExpressions() { + SmallRyeConfig config = new SmallRyeConfigBuilder() + .addDefaultInterceptors() + .withMapping(OptionalExpressions.class) + .withSources(config("optional.expression", "${expression}")) + .withSources(config("optional.expression-int", "${expression}")) + .build(); + + OptionalExpressions mapping = config.getConfigMapping(OptionalExpressions.class); + + assertFalse(mapping.expression().isPresent()); + assertFalse(mapping.expressionInt().isPresent()); + } } diff --git a/implementation/src/test/java/io/smallrye/config/ExpressionConfigSourceInterceptorTest.java b/implementation/src/test/java/io/smallrye/config/ExpressionConfigSourceInterceptorTest.java index 325c49919..51f909693 100644 --- a/implementation/src/test/java/io/smallrye/config/ExpressionConfigSourceInterceptorTest.java +++ b/implementation/src/test/java/io/smallrye/config/ExpressionConfigSourceInterceptorTest.java @@ -2,11 +2,14 @@ import static java.util.stream.Collectors.toList; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import java.util.ArrayList; import java.util.List; import java.util.NoSuchElementException; +import java.util.Optional; import java.util.stream.Stream; import org.junit.jupiter.api.Test; @@ -77,7 +80,7 @@ void noExpression() { void noExpressionComposed() { SmallRyeConfig config = buildConfig("expression", "${my.prop${compose}}"); - final NoSuchElementException exception = assertThrows(NoSuchElementException.class, + NoSuchElementException exception = assertThrows(NoSuchElementException.class, () -> config.getValue("expression", String.class)); assertEquals("SRCFG00011: Could not expand value compose in property expression", exception.getMessage()); } @@ -132,35 +135,53 @@ void escape() { @Test void expressionMissing() { - final SmallRyeConfig config = buildConfig("my.prop", "${expression}", "my.prop.partial", "${expression}partial"); + SmallRyeConfig config = buildConfig("my.prop", "${expression}", "my.prop.partial", "${expression}partial"); assertThrows(Exception.class, () -> config.getValue("my.prop", String.class)); assertThrows(Exception.class, () -> config.getValue("my.prop.partial", String.class)); } + @Test + void expressionMissingOptional() { + SmallRyeConfig config = buildConfig("my.prop", "${expression}", "my.prop.partial", "${expression}partial"); + + assertEquals(Optional.empty(), config.getOptionalValue("my.prop", String.class)); + assertEquals(Optional.empty(), config.getOptionalValue("my.prop.partial", String.class)); + + ConfigValue noExpression = config.getConfigValue("my.prop"); + assertNotNull(noExpression); + assertEquals(noExpression.getName(), "my.prop"); + assertNull(noExpression.getValue()); + + ConfigValue noExpressionPartial = config.getConfigValue("my.prop.partial"); + assertNotNull(noExpressionPartial); + assertEquals(noExpressionPartial.getName(), "my.prop.partial"); + assertNull(noExpressionPartial.getValue()); + } + @Test void arrayEscapes() { - final SmallRyeConfig config = buildConfig("list", "cat,dog,${mouse},sea\\,turtle", "mouse", "mouse"); - final List list = config.getValues("list", String.class, ArrayList::new); + SmallRyeConfig config = buildConfig("list", "cat,dog,${mouse},sea\\,turtle", "mouse", "mouse"); + List list = config.getValues("list", String.class, ArrayList::new); assertEquals(4, list.size()); assertEquals(list, Stream.of("cat", "dog", "mouse", "sea,turtle").collect(toList())); } @Test void escapeDollar() { - final SmallRyeConfig config = buildConfig("my.prop", "\\${value\\${another}end:value}"); + SmallRyeConfig config = buildConfig("my.prop", "\\${value\\${another}end:value}"); assertEquals("${value${another}end:value}", config.getRawValue("my.prop")); } @Test void escapeBraces() { - final SmallRyeConfig config = buildConfig("my.prop", "${value:111{111}"); + SmallRyeConfig config = buildConfig("my.prop", "${value:111{111}"); assertEquals("111{111", config.getRawValue("my.prop")); } @Test void windowPath() { - final SmallRyeConfig config = buildConfig("window.path", "C:\\Some\\Path"); + SmallRyeConfig config = buildConfig("window.path", "C:\\Some\\Path"); assertEquals("C:\\Some\\Path", config.getRawValue("window.path")); }