Improve EnvConfigSource

common/src/main/java/io/smallrye/config/common/utils/StringUtil.java

@@ -139,8 +139,11 @@ public static boolean equalsIgnoreCaseReplacingNonAlphanumericByUnderscores(fina
     }
 
     public static String replaceNonAlphanumericByUnderscores(final String name) {
+        return replaceNonAlphanumericByUnderscores(name, new StringBuilder(name.length()));
+    }
+
+    public static String replaceNonAlphanumericByUnderscores(final String name, final StringBuilder sb) {
         int length = name.length();
-        StringBuilder sb = new StringBuilder(length);
         for (int i = 0; i < length; i++) {
             char c = name.charAt(i);
             if (isAsciiLetterOrDigit(c)) {
@@ -156,10 +159,13 @@ public static String replaceNonAlphanumericByUnderscores(final String name) {
     }
 
     public static String toLowerCaseAndDotted(final String name) {
+        return toLowerCaseAndDotted(name, new StringBuilder(name.length()));
+    }
+
+    public static String toLowerCaseAndDotted(final String name, final StringBuilder sb) {
         int length = name.length();
         int beginSegment = 0;
         boolean quotesOpen = false;
-        StringBuilder sb = new StringBuilder();
         for (int i = 0; i < length; i++) {
             char c = name.charAt(i);
             if ('_' == c) {

common/src/test/java/io/smallrye/config/common/utils/StringUtilTest.java

@@ -120,11 +120,38 @@ void replaceNonAlphanumericByUnderscores() {
                 StringUtil.replaceNonAlphanumericByUnderscores("test.language.\"de.etr\"".toUpperCase()));
     }
 
+    @Test
+    void replaceNonAlphanumericByUnderscoresWithStringBuilder() {
+        StringBuilder builder = new StringBuilder();
+        builder.setLength(0);
+        assertEquals("FOO_BAR", StringUtil.replaceNonAlphanumericByUnderscores("foo.bar", builder).toUpperCase());
+        builder.setLength(0);
+        assertEquals("FOO_BAR_BAZ", StringUtil.replaceNonAlphanumericByUnderscores("foo.bar.baz", builder).toUpperCase());
+        builder.setLength(0);
+        assertEquals("FOO", StringUtil.replaceNonAlphanumericByUnderscores("foo", builder).toUpperCase());
+        builder.setLength(0);
+        assertEquals("TEST_LANGUAGE__DE_ETR__",
+                StringUtil.replaceNonAlphanumericByUnderscores("test.language.\"de.etr\"".toUpperCase()));
+    }
+
     @Test
     void toLowerCaseAndDotted() {
         assertEquals("test.language.\"de.etr\"", StringUtil.toLowerCaseAndDotted("TEST_LANGUAGE__DE_ETR__"));
     }
 
+    @Test
+    void toLowerCaseAndDottedWithStringBuilder() {
+        StringBuilder builder = new StringBuilder();
+        builder.setLength(0);
+        assertEquals("foo.bar", StringUtil.toLowerCaseAndDotted("FOO_BAR", builder));
+        builder.setLength(0);
+        assertEquals("foo.bar.baz", StringUtil.toLowerCaseAndDotted("FOO_BAR_BAZ", builder));
+        builder.setLength(0);
+        assertEquals("foo", StringUtil.toLowerCaseAndDotted("FOO", builder));
+        builder.setLength(0);
+        assertEquals("test.language.\"de.etr\"", StringUtil.toLowerCaseAndDotted("TEST_LANGUAGE__DE_ETR__"));
+    }
+
     @Test
     void isNumeric() {
         assertTrue(StringUtil.isNumeric("0"));

implementation/src/main/java/io/smallrye/config/EnvConfigSource.java

@@ -18,23 +18,68 @@
 import static io.smallrye.config.common.utils.ConfigSourceUtil.CONFIG_ORDINAL_KEY;
 import static io.smallrye.config.common.utils.StringUtil.replaceNonAlphanumericByUnderscores;
 import static java.security.AccessController.doPrivileged;
-import static java.util.Collections.unmodifiableMap;
+import static java.util.Collections.emptySet;
 
 import java.io.Serializable;
 import java.security.PrivilegedAction;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
-import io.smallrye.config.common.MapBackedConfigSource;
+import io.smallrye.config.common.AbstractConfigSource;
+import io.smallrye.config.common.utils.StringUtil;
 
 /**
- * @author <a href="http://jmesnil.net/">Jeff Mesnil</a> (c) 2017 Red Hat inc.
+ * A {@link org.eclipse.microprofile.config.spi.ConfigSource} to access Environment Variables following the mapping
+ * rules defined by the MicroProfile Config specification.
+ * <p>
+ *
+ * For a given property name <code>foo.bar.baz</code>, is matched to an environment variable with the following rules:
+ *
+ * <ol>
+ * <li>Exact match (<code>foo.bar.baz</code>)</li>
+ * <li>Replace each character that is neither alphanumeric nor <code>_</code> with <code>_</code>
+ * (<code>foo_bar_baz</code>)</li>
+ * <li>Replace each character that is neither alphanumeric nor <code>_</code> with <code>_</code>; then convert the name to
+ * upper case (<code>FOO_BAR_BAZ</code>)</li>
+ * </ol>
+ * <p>
+ *
+ * Additionally, this implementation provides candidate matching dotted property name from the Environment
+ * Variable name. These are required when a consumer relies on the list of properties to find additional
+ * configurations. 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 it is unclear about property names.
+ * <br>
+ * 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, so consumers must be aware of such
+ * limitations.
+ * <p>
+ *
+ * Due to its high ordinality (<code>300</code>), the {@link EnvConfigSource} may be queried on every property name
+ * lookup, just to not find a value and proceed to the next source. The conversion rules make such lookups inefficient
+ * and unnecessary. In order to reduce lookups, this implementation provides the following mechanisms:
+ * <p>
+ * Keeps three forms of the environment variables names (original, lowercased and uppercased), to avoid having to
+ * transform the property name on each lookup.
+ * <br>
+ * A dotted property name lookup is first matched to the existing names to avoid mapping and replacing rules in the
+ * name. Property names with other special characters always require replacing rules:
+ *
+ * <ol>
+ * <li>A lookup to <code>foo.bar</code> requires <code>FOO_BAR</code> converted to the dotted name <code>foo.bar</code></li>
+ * <li>A lookup to <code>foo-bar</code> requires <code>FOO_BAR</code> no match, mapping applied</li>
+ * <li>A lookup to <code>foo.baz</code> no match</li>
+ * </ol>
  */
-public class EnvConfigSource extends MapBackedConfigSource {
+public class EnvConfigSource extends AbstractConfigSource {
     private static final long serialVersionUID = -4525015934376795496L;
 
     private static final int DEFAULT_ORDINAL = 300;
 
+    private final Map<String, String> properties;
+    private final Set<String> names;
+
     protected EnvConfigSource() {
         this(DEFAULT_ORDINAL);
     }
@@ -43,35 +88,52 @@ protected EnvConfigSource(final int ordinal) {
         this(getEnvProperties(), ordinal);
     }
 
-    public EnvConfigSource(final Map<String, String> propertyMap, final int ordinal) {
-        super("EnvConfigSource", propertyMap, getEnvOrdinal(propertyMap, ordinal));
+    public EnvConfigSource(final Map<String, String> properties, final int ordinal) {
+        super("EnvConfigSource", getEnvOrdinal(properties, ordinal));
+        this.properties = new HashMap<>(properties.size() * 3);
+        this.names = new HashSet<>(properties.size() * 2);
+        StringBuilder builder = new StringBuilder();
+        for (Map.Entry<String, String> entry : properties.entrySet()) {
+            this.properties.put(entry.getKey(), entry.getValue());
+            this.properties.put(entry.getKey().toLowerCase(), entry.getValue());
+            this.properties.put(entry.getKey().toUpperCase(), entry.getValue());
+            this.names.add(entry.getKey());
+            this.names.add(StringUtil.toLowerCaseAndDotted(entry.getKey(), builder));
+            builder.setLength(0);
+        }
+    }
+
+    @Override
+    public Map<String, String> getProperties() {
+        return this.properties;
+    }
+
+    @Override
+    public Set<String> getPropertyNames() {
+        return this.names;
     }
 
     @Override
     public String getValue(final String propertyName) {
-        return getValue(propertyName, getProperties());
+        return getValue(propertyName, getProperties(), names);
     }
 
-    private static String getValue(final String name, final Map<String, String> properties) {
-        if (name == null) {
+    private static String getValue(final String propertyName, final Map<String, String> properties, final Set<String> names) {
+        if (propertyName == null) {
             return null;
         }
 
         // exact match
-        String value = properties.get(name);
+        String value = properties.get(propertyName);
         if (value != null) {
             return value;
         }
 
-        // replace non-alphanumeric characters by underscores
-        String sanitizedName = replaceNonAlphanumericByUnderscores(name);
-        value = properties.get(sanitizedName);
-        if (value != null) {
-            return value;
+        if (isDottedFormat(propertyName) && !names.contains(propertyName)) {
+            return null;
         }
 
-        // replace non-alphanumeric characters by underscores and convert to uppercase
-        return properties.get(sanitizedName.toUpperCase());
+        return properties.get(replaceNonAlphanumericByUnderscores(propertyName));
     }
 
     /**
@@ -80,17 +142,30 @@ private static String getValue(final String name, final Map<String, String> prop
      * instantiated in the heap.
      */
     private static Map<String, String> getEnvProperties() {
-        return unmodifiableMap(doPrivileged((PrivilegedAction<Map<String, String>>) () -> new HashMap<>(System.getenv())));
+        return doPrivileged((PrivilegedAction<Map<String, String>>) () -> new HashMap<>(System.getenv()));
     }
 
     private static int getEnvOrdinal(final Map<String, String> properties, final int ordinal) {
-        final String value = getValue(CONFIG_ORDINAL_KEY, properties);
+        String value = getValue(CONFIG_ORDINAL_KEY, properties, emptySet());
+        if (value == null) {
+            value = getValue(CONFIG_ORDINAL_KEY.toUpperCase(), properties, emptySet());
+        }
         if (value != null) {
             return Converters.INTEGER_CONVERTER.convert(value);
         }
         return ordinal;
     }
 
+    private static boolean isDottedFormat(final String propertyName) {
+        for (int i = 0; i < propertyName.length(); i++) {
+            char c = propertyName.charAt(i);
+            if (!('a' <= c && c <= 'z') && !('0' <= c && c <= '9') && c != '.') {
+                return false;
+            }
+        }
+        return true;
+    }
+
     Object writeReplace() {
         return new Ser();
     }

implementation/src/main/java/io/smallrye/config/PropertyNamesConfigSourceInterceptor.java

@@ -1,15 +1,9 @@
 package io.smallrye.config;
 
-import static io.smallrye.config.common.utils.StringUtil.replaceNonAlphanumericByUnderscores;
-import static io.smallrye.config.common.utils.StringUtil.toLowerCaseAndDotted;
-
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Set;
 
-import org.eclipse.microprofile.config.spi.ConfigSource;
-
 /**
  * This interceptor adds additional entries to {@link org.eclipse.microprofile.config.Config#getPropertyNames}.
  */
@@ -18,11 +12,6 @@ class PropertyNamesConfigSourceInterceptor implements ConfigSourceInterceptor {
 
     private final Set<String> properties = new HashSet<>();
 
-    public PropertyNamesConfigSourceInterceptor(final ConfigSourceInterceptorContext context,
-            final List<ConfigSource> sources) {
-        this.properties.addAll(generateDottedProperties(context, sources));
-    }
-
     @Override
     public ConfigValue getValue(final ConfigSourceInterceptorContext context, final String name) {
         return context.proceed(name);
@@ -42,61 +31,4 @@ public Iterator<String> iterateNames(final ConfigSourceInterceptorContext contex
     void addProperties(final Set<String> properties) {
         this.properties.addAll(properties);
     }
-
-    /**
-     * Generate dotted properties from Env properties.
-     * <br>
-     * 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.
-     * <br>
-     * 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:
-     * <br>
-     * If <code>foo.bar</code> is present and <code>FOO_BAR</code> is also present, no property is required.
-     * If <code>foo-bar</code> is present and <code>FOO_BAR</code> is also present, no property is required.
-     * If <code>FOO_BAR</code> is present a property <code>foo.bar</code> is required.
-     */
-    private static Set<String> generateDottedProperties(final ConfigSourceInterceptorContext current,
-            final List<ConfigSource> sources) {
-        // Collect all known properties
-        Set<String> properties = new HashSet<>();
-        Iterator<String> iterateNames = current.iterateNames();
-        while (iterateNames.hasNext()) {
-            properties.add(iterateNames.next());
-        }
-
-        // Collect only properties from the EnvSources
-        Set<String> envProperties = new HashSet<>();
-        for (ConfigSource source : sources) {
-            if (source instanceof EnvConfigSource) {
-                envProperties.addAll(source.getPropertyNames());
-            }
-        }
-        properties.removeAll(envProperties);
-
-        // Collect properties that have the same semantic meaning
-        Set<String> overrides = new HashSet<>();
-        for (String property : properties) {
-            String semanticProperty = replaceNonAlphanumericByUnderscores(property);
-            for (String envProperty : envProperties) {
-                if (envProperty.equalsIgnoreCase(semanticProperty)) {
-                    overrides.add(envProperty);
-                    break;
-                }
-            }
-        }
-
-        // Remove them - Remaining properties can only be found in the EnvSource - generate a dotted version
-        envProperties.removeAll(overrides);
-        Set<String> dottedProperties = new HashSet<>();
-        for (String envProperty : envProperties) {
-            dottedProperties.add(toLowerCaseAndDotted(envProperty));
-        }
-        return dottedProperties;
-    }
 }

implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java

@@ -566,8 +566,7 @@ private static class ConfigSources implements Serializable {
             // the resolved final source or interceptor to use.
             current = new SmallRyeConfigSourceInterceptorContext(EMPTY, null);
             current = new SmallRyeConfigSourceInterceptorContext(new SmallRyeConfigSources(sourcesWithPriorities), current);
-            PropertyNamesConfigSourceInterceptor propertyNamesInterceptor = new PropertyNamesConfigSourceInterceptor(current,
-                    configSources);
+            PropertyNamesConfigSourceInterceptor propertyNamesInterceptor = new PropertyNamesConfigSourceInterceptor();
             current = new SmallRyeConfigSourceInterceptorContext(propertyNamesInterceptor, current);
             for (ConfigSourceInterceptor interceptor : interceptors) {
                 current = new SmallRyeConfigSourceInterceptorContext(interceptor, current);

implementation/src/test/java/io/smallrye/config/EnvConfigSourceTest.java

@@ -54,7 +54,7 @@ void conversionOfEnvVariableNames() {
         assertFalse(cs.getPropertyNames().contains("smallrye_mp_config_prop"));
 
         assertEquals(envProp, cs.getValue("smallrye.mp.config.prop"));
-        assertFalse(cs.getPropertyNames().contains("smallrye.mp.config.prop"));
+        assertTrue(cs.getPropertyNames().contains("smallrye.mp.config.prop"));
 
         assertEquals(envProp, cs.getValue("SMALLRYE.MP.CONFIG.PROP"));
         assertFalse(cs.getPropertyNames().contains("SMALLRYE.MP.CONFIG.PROP"));
@@ -100,7 +100,7 @@ void ordinal() {
         ConfigSource configSource = config.getConfigSources().iterator().next();
 
         assertTrue(configSource instanceof EnvConfigSource);
-        assertEquals(configSource.getOrdinal(), 301);
+        assertEquals(301, configSource.getOrdinal());
     }
 
     @Test

implementation/src/test/java/io/smallrye/config/SmallRyeConfigTest.java

@@ -313,7 +313,7 @@ void getPropertyNames() {
         assertEquals("1234", config.getRawValue("smallrye.mp-config.prop"));
         assertTrue(((Set<String>) config.getPropertyNames()).contains("SMALLRYE_MP_CONFIG_PROP"));
         assertTrue(((Set<String>) config.getPropertyNames()).contains("smallrye.mp-config.prop"));
-        assertFalse(((Set<String>) config.getPropertyNames()).contains("smallrye.mp.config.prop"));
+        assertTrue(((Set<String>) config.getPropertyNames()).contains("smallrye.mp.config.prop"));
     }
 
     @Test