Avoid throwing Exception for Optional types and ConfigValue if expres…

…sion cannot be expanded (#701)
eclipse · Feb 28, 2022 · 11f6443 · 11f6443
1 parent fdc012e
commit 11f6443
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 1 deletion.
diff --git a/spec/src/main/asciidoc/property-expressions.asciidoc b/spec/src/main/asciidoc/property-expressions.asciidoc
@@ -55,7 +55,8 @@ server.endpoint.path.bar=foo
 The property `server.url` is expanded to `http://example.org:8080/foo`.
 
 If an expression cannot be expanded and does not have a default value, a `NoSuchElementException` is thrown. In the
-Optional case, an empty Optional will be returned.
+Optional case, an empty Optional will be returned. In the `ConfigValue` case, an `ConfigValue` with only
+the name of the property will be returned.
 
 The number of recursion lookups is not infinite, but a limited number for composed expressions. Implementations are
 encouraged to limit the number to `5`, but they can use higher limits if they wish to. When the number of allowed

diff --git a/tck/src/main/java/org/eclipse/microprofile/config/tck/CDIPropertyExpressionsTest.java b/tck/src/main/java/org/eclipse/microprofile/config/tck/CDIPropertyExpressionsTest.java
@@ -19,9 +19,16 @@
 package org.eclipse.microprofile.config.tck;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Optional;
+import java.util.OptionalDouble;
+import java.util.OptionalInt;
+import java.util.OptionalLong;
 import java.util.Set;
 
 import org.eclipse.microprofile.config.ConfigValue;
@@ -64,6 +71,20 @@ public void expressionNoDefault() {
         assertEquals(propertyExpressionBean.expressionDefault, "${expression}");
     }
 
+    @Test
+    public void badExpansion() {
+        assertFalse(propertyExpressionBean.badExpansion.isPresent());
+        assertFalse(propertyExpressionBean.badExpansionInt.isPresent());
+        assertFalse(propertyExpressionBean.badExpansionDouble.isPresent());
+        assertFalse(propertyExpressionBean.badExpansionLong.isPresent());
+
+        assertNotNull(propertyExpressionBean.badExpansionConfigValue);
+        assertEquals(propertyExpressionBean.badExpansionConfigValue.getName(), "expression");
+        assertNull(propertyExpressionBean.badExpansionConfigValue.getValue());
+        assertNull(propertyExpressionBean.badExpansionConfigValue.getSourceName());
+        assertEquals(propertyExpressionBean.badExpansionConfigValue.getSourceOrdinal(), 0);
+    }
+
     @Dependent
     public static class PropertyExpressionBean {
         @Inject
@@ -75,6 +96,19 @@ public static class PropertyExpressionBean {
         @Inject
         @ConfigProperty(name = "another.prop", defaultValue = "${expression}")
         String expressionDefault;
+        @ConfigProperty(name = "bad.property.expression.prop")
+        Optional<String> badExpansion;
+        @Inject
+        @ConfigProperty(name = "bad.property.expression.prop")
+        OptionalInt badExpansionInt;
+        @Inject
+        @ConfigProperty(name = "bad.property.expression.prop")
+        OptionalDouble badExpansionDouble;
+        @Inject
+        @ConfigProperty(name = "bad.property.expression.prop")
+        OptionalLong badExpansionLong;
+        @ConfigProperty(name = "bad.property.expression.prop")
+        ConfigValue badExpansionConfigValue;
     }
 
     public static class PropertyExpressionConfigSource implements ConfigSource {
@@ -83,6 +117,7 @@ public static class PropertyExpressionConfigSource implements ConfigSource {
         public PropertyExpressionConfigSource() {
             properties.put("my.prop", "${expression}");
             properties.put("expression", "1234");
+            properties.put("bad.property.expression.prop", "${missing.prop}");
         }
 
         @Override

diff --git a/tck/src/main/java/org/eclipse/microprofile/config/tck/PropertyExpressionsTest.java b/tck/src/main/java/org/eclipse/microprofile/config/tck/PropertyExpressionsTest.java
@@ -21,17 +21,21 @@
 import static java.util.stream.Collectors.toList;
 import static org.eclipse.microprofile.config.Config.PROPERTY_EXPRESSIONS_ENABLED;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
 import static org.testng.Assert.assertThrows;
 
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Stream;
 
 import org.eclipse.microprofile.config.Config;
+import org.eclipse.microprofile.config.ConfigValue;
 import org.eclipse.microprofile.config.spi.ConfigProviderResolver;
 import org.eclipse.microprofile.config.spi.ConfigSource;
 import org.jboss.arquillian.container.test.api.Deployment;
@@ -123,13 +127,51 @@ public void noExpression() {
         assertThrows(NoSuchElementException.class, () -> config.getValue("expression", String.class));
     }
 
+    @Test
+    void noExpressionButOptional() {
+        Config config = buildConfig("expression", "${my.prop}");
+
+        assertEquals(Optional.empty(), config.getOptionalValue("expression", String.class));
+    }
+
+    @Test
+    void noExpressionButConfigValue() {
+        Config config = buildConfig("expression", "${my.prop}");
+
+        ConfigValue configValue = config.getConfigValue("expression");
+        assertNotNull(configValue);
+        assertEquals(configValue.getName(), "expression");
+        assertNull(configValue.getValue());
+        assertNull(configValue.getSourceName());
+        assertEquals(configValue.getSourceOrdinal(), 0);
+    }
+
     @Test
     public void noExpressionComposed() {
         Config config = buildConfig("expression", "${my.prop${compose}}");
 
         assertThrows(NoSuchElementException.class, () -> config.getValue("expression", String.class));
     }
 
+    @Test
+    void noExpressionComposedButOptional() {
+        Config config = buildConfig("expression", "${my.prop${compose}}");
+
+        assertEquals(Optional.empty(), config.getOptionalValue("expression", String.class));
+    }
+
+    @Test
+    void noExpressionComposedButConfigValue() {
+        Config config = buildConfig("expression", "${my.prop${compose}}");
+
+        ConfigValue configValue = config.getConfigValue("expression");
+        assertNotNull(configValue);
+        assertEquals(configValue.getName(), "expression");
+        assertNull(configValue.getValue());
+        assertNull(configValue.getSourceName());
+        assertEquals(configValue.getSourceOrdinal(), 0);
+    }
+
     @Test
     public void multipleExpansions() {
         Config config = buildConfig("my.prop", "1234", "my.prop.two", "${my.prop}", "my.prop.three",