diff --git a/implementation/src/main/java/io/smallrye/config/ConfigMessages.java b/implementation/src/main/java/io/smallrye/config/ConfigMessages.java index 5f880db56..f2d01b1ac 100644 --- a/implementation/src/main/java/io/smallrye/config/ConfigMessages.java +++ b/implementation/src/main/java/io/smallrye/config/ConfigMessages.java @@ -57,7 +57,8 @@ public interface ConfigMessages { @Message(id = 13, value = "No Converter registered for %s") IllegalArgumentException noRegisteredConverter(Class type); - @Message(id = 14, value = "Property %s is required but the value was not found or is empty") + // Returns a String rather than a NoSuchElementException for a slight performance improvement as throwing this exception could be quite common. + @Message(id = 14, value = "The config property %s is required but it could not be found in any config source") String propertyNotFound(String name); @Message(id = 15, value = "No configuration is available for this class loader") @@ -131,4 +132,14 @@ public interface ConfigMessages { @Message(id = 38, value = "Type has no raw type class: %s") IllegalArgumentException noRawType(Type type); + + @Message(id = 39, value = "The config property %s with the config value \"%s\" threw an Exception whilst being converted") + IllegalArgumentException converterException(@Cause Throwable converterException, String configProperty, String configValue); + + @Message(id = 40, value = "The config property %s is defined as the empty String (\"\") which the following Converter considered to be null: %s") + NoSuchElementException propertyEmptyString(String configPropertyName, String converter); + + @Message(id = 41, value = "The config property %s with the config value \"%s\" was converted to null from the following Converter: %s") + NoSuchElementException converterReturnedNull(String configPropertyName, String configValue, String converter); + } diff --git a/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java b/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java index 8610c8c1e..7e0a9f023 100644 --- a/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java +++ b/implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java @@ -186,6 +186,23 @@ public T getValue(String name, Class aClass) { return getValue(name, requireConverter(aClass)); } + /** + * + * This method handles calls from both {@link Config#getValue} and {@link Config#getOptionalValue}.
+ *
+ * + * Calls from {@link Config#getValue} should throw an {@link Exception} for each of the following:
+ * + * 1. {@link IllegalArgumentException} - if the property cannot be converted by the {@link Converter} to the specified type + *
+ * 2. {@link NoSuchElementException} - if the property is not defined
+ * 3. {@link NoSuchElementException} - if the property is defined as an empty string
+ * 4. {@link NoSuchElementException} - if the {@link Converter} returns {@code null}
+ *
+ * + * Calls from {@link Config#getOptionalValue} should only throw an {@link Exception} for #1 + * ({@link IllegalArgumentException} when the property cannot be converted to the specified type). + */ @SuppressWarnings("unchecked") public T getValue(String name, Converter converter) { ConfigValue configValue = getConfigValue(name); @@ -200,20 +217,35 @@ public T getValue(String name, Converter converter) { } } - String value = configValue.getValue(); + String value = configValue.getValue(); // Can return the empty String (which is not considered as null) + final T converted; + if (value != null) { - converted = converter.convert(value); + try { + converted = converter.convert(value); + } catch (IllegalArgumentException e) { + throw ConfigMessages.msg.converterException(e, name, value); // 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)); + throw new NoSuchElementException(ConfigMessages.msg.propertyNotFound(name)); // 2 } } + if (converted == null) { - throw new NoSuchElementException(ConfigMessages.msg.propertyNotFound(name)); + 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 + } else { + throw ConfigMessages.msg.converterReturnedNull(name, value, converter.getClass().getTypeName()); // 4 + } } + return converted; } diff --git a/implementation/src/test/java/io/smallrye/config/ConfigValueConversionRulesExceptionsTest.java b/implementation/src/test/java/io/smallrye/config/ConfigValueConversionRulesExceptionsTest.java new file mode 100644 index 000000000..c6f193384 --- /dev/null +++ b/implementation/src/test/java/io/smallrye/config/ConfigValueConversionRulesExceptionsTest.java @@ -0,0 +1,102 @@ +package io.smallrye.config; + +import static io.smallrye.config.KeyValuesConfigSource.config; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.NoSuchElementException; + +import org.junit.jupiter.api.Test; + +/** + * + * Test sensible error messages are output when edge case config value conversion Exceptions occur + * + */ +class ConfigValueConversionRulesExceptionsTest { + + @Test + void missingString() { + final SmallRyeConfig config = new SmallRyeConfigBuilder().build(); + final NoSuchElementException exception = assertThrows(NoSuchElementException.class, + () -> config.getValue("none.existing.prop", String.class)); + assertEquals( + "SRCFG00014: The config property none.existing.prop is required but it could not be found in any config source", + exception.getMessage()); + } + + @Test + void missingStringArray() { + final SmallRyeConfig config = new SmallRyeConfigBuilder().build(); + final NoSuchElementException exception = assertThrows(NoSuchElementException.class, + () -> config.getValue("none.existing.array.prop", String[].class)); + assertEquals( + "SRCFG00014: The config property none.existing.array.prop is required but it could not be found in any config source", + exception.getMessage()); + } + + @Test + void emptyString() { + final SmallRyeConfig config = new SmallRyeConfigBuilder() + .withSources(config("empty.string", "")).build(); + final NoSuchElementException exception = assertThrows(NoSuchElementException.class, + () -> config.getValue("empty.string", String.class)); + assertEquals( + "SRCFG00040: The config property empty.string is defined as the empty String (\"\") which the following Converter considered to be null: io.smallrye.config.Converters$BuiltInConverter", + exception.getMessage()); + } + + @Test + void emptyStringArray() { + final SmallRyeConfig config = new SmallRyeConfigBuilder() + .withSources(config("empty.string.array", "")).build(); + final NoSuchElementException exception = assertThrows(NoSuchElementException.class, + () -> config.getValue("empty.string.array", String[].class)); + assertEquals( + "SRCFG00040: The config property empty.string.array is defined as the empty String (\"\") which the following Converter considered to be null: io.smallrye.config.Converters$ArrayConverter", + exception.getMessage()); + } + + @Test + void commaStringArray() { + final SmallRyeConfig config = new SmallRyeConfigBuilder() + .withSources(config("comma.string.array", ",")).build(); + final NoSuchElementException exception = assertThrows(NoSuchElementException.class, + () -> config.getValue("comma.string.array", String[].class)); + assertEquals( + "SRCFG00041: The config property comma.string.array with the config value \",\" was converted to null from the following Converter: io.smallrye.config.Converters$ArrayConverter", + exception.getMessage()); + } + + @Test + void doubleCommaStringArray() { + final SmallRyeConfig config = new SmallRyeConfigBuilder() + .withSources(config("double.comma.string.array", ",,")).build(); + final NoSuchElementException exception = assertThrows(NoSuchElementException.class, + () -> config.getValue("double.comma.string.array", String[].class)); + assertEquals( + "SRCFG00041: The config property double.comma.string.array with the config value \",,\" was converted to null from the following Converter: io.smallrye.config.Converters$ArrayConverter", + exception.getMessage()); + } + + @Test + void missingStringWithBadDefault() { + final SmallRyeConfig config = new SmallRyeConfigBuilder().withDefaultValue("bad.default.value", "").build(); + final NoSuchElementException exception = assertThrows(NoSuchElementException.class, + () -> config.getValue("bad.default.value", String.class)); + assertEquals( + "SRCFG00040: The config property bad.default.value is defined as the empty String (\"\") which the following Converter considered to be null: io.smallrye.config.Converters$BuiltInConverter", + exception.getMessage()); + } + + @Test + void badConversion() throws Exception { + final SmallRyeConfig config = new SmallRyeConfigBuilder().withDefaultValue("not.an.Integer", "notInt").build(); + final Exception exception = assertThrows(IllegalArgumentException.class, + () -> config.getValue("not.an.Integer", Integer.class)); + assertEquals( + "SRCFG00039: The config property not.an.Integer with the config value \"notInt\" threw an Exception whilst being converted", + exception.getMessage()); + assertEquals("SRCFG00029: Expected an integer value, got \"notInt\"", exception.getCause().getMessage()); + } +} diff --git a/implementation/src/test/java/io/smallrye/config/CustomConverterTest.java b/implementation/src/test/java/io/smallrye/config/CustomConverterTest.java index 23d1df204..6f619fd74 100644 --- a/implementation/src/test/java/io/smallrye/config/CustomConverterTest.java +++ b/implementation/src/test/java/io/smallrye/config/CustomConverterTest.java @@ -122,9 +122,10 @@ void malformedUUID() { IllegalArgumentException thrownException = assertThrows(IllegalArgumentException.class, () -> { config.getValue("uuid.invalid", UUID.class); }, "Malformed UUID should throw exception"); - assertEquals("SRCFG00026: notauuid cannot be converted into a UUID", thrownException.getMessage(), - "Exception message is incorrect"); - + assertEquals( + "SRCFG00039: The config property uuid.invalid with the config value \"notauuid\" threw an Exception whilst being converted", + thrownException.getMessage()); + assertEquals("SRCFG00026: notauuid cannot be converted into a UUID", thrownException.getCause().getMessage()); } private static Config buildConfig(String... keyValues) {