Fix precedence for explicit sys prop and env var sources.

config/config/src/main/java/io/helidon/config/BuilderImpl.java

@@ -47,7 +47,6 @@
 import io.helidon.config.spi.ConfigSource;
 import io.helidon.config.spi.OverrideSource;
 
-import static io.helidon.config.ConfigSources.ENV_VARS_NAME;
 import static io.helidon.config.ConfigSources.classpath;
 
 /**
@@ -97,7 +96,7 @@ public Config.Builder sources(List<Supplier<ConfigSource>> sourceSuppliers) {
         sources = new ArrayList<>(sourceSuppliers.size());
         sourceSuppliers.stream().map(Supplier::get).forEach(source -> {
             sources.add(source);
-            if (source.description().contains(ENV_VARS_NAME)) {
+            if (source instanceof ConfigSources.EnvironmentVariablesConfigSource) {
                 envVarAliasGeneratorEnabled = true;
             }
         });
@@ -312,13 +311,19 @@ private ProviderImpl buildProvider() {
 
     private ConfigSource targetConfigSource(ConfigContext context) {
         List<ConfigSource> targetSources = new LinkedList<>();
-        if (environmentVariablesSourceEnabled) {
+
+        if (hasSourceType(ConfigSources.EnvironmentVariablesConfigSource.class)) {
+            envVarAliasGeneratorEnabled = true;
+        } else if (environmentVariablesSourceEnabled) {
             targetSources.add(ConfigSources.environmentVariables());
             envVarAliasGeneratorEnabled = true;
         }
-        if (systemPropertiesSourceEnabled) {
+
+        if (systemPropertiesSourceEnabled
+            && !hasSourceType(ConfigSources.SystemPropertiesConfigSource.class)) {
             targetSources.add(ConfigSources.systemProperties());
         }
+
         if (sources != null) {
             targetSources.addAll(sources);
         } else {
@@ -335,6 +340,17 @@ private ConfigSource targetConfigSource(ConfigContext context) {
         return targetConfigSource;
     }
 
+    private boolean hasSourceType(Class<?> sourceType) {
+        if (sources != null) {
+            for (ConfigSource source : sources) {
+                if (sourceType.isAssignableFrom(source.getClass())) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
     @SuppressWarnings("ParameterNumber")
     ProviderImpl createProvider(ConfigMapperManager configMapperManager,
                                 ConfigSource targetConfigSource,

config/config/src/main/java/io/helidon/config/ConfigSources.java

@@ -58,8 +58,6 @@
 public final class ConfigSources {
 
     private static final String SOURCES_KEY = "sources";
-    static final String ENV_VARS_NAME = "env-vars";
-    static final String SYS_PROPS_NAME = "sys-props";
     static final String DEFAULT_MAP_NAME = "map";
     static final String DEFAULT_PROPERTIES_NAME = "properties";
 
@@ -221,7 +219,7 @@ public static ConfigSource prefixed(String key, Supplier<ConfigSource> sourceSup
      * @return {@code ConfigSource} for config derived from system properties
      */
     public static ConfigSource systemProperties() {
-        return create(System.getProperties(), SYS_PROPS_NAME).lax().build();
+        return new SystemPropertiesConfigSource();
     }
 
     /**
@@ -231,7 +229,7 @@ public static ConfigSource systemProperties() {
      * @return {@code ConfigSource} for config derived from environment variables
      */
     public static ConfigSource environmentVariables() {
-        return create(EnvironmentVariables.expand(), ENV_VARS_NAME).lax().build();
+        return new EnvironmentVariablesConfigSource();
     }
 
     /**
@@ -766,4 +764,27 @@ public Optional<ConfigNode.ObjectNode> load() throws ConfigException {
         };
     }
 
+    /**
+     * Environment variables config source.
+     */
+    static final class EnvironmentVariablesConfigSource extends MapConfigSource {
+        /**
+         * Constructor.
+         */
+        EnvironmentVariablesConfigSource() {
+            super(EnvironmentVariables.expand(), false, "");
+        }
+    }
+
+    /**
+     * System properties config source.
+     */
+    static final class SystemPropertiesConfigSource extends MapConfigSource {
+        /**
+         * Constructor.
+         */
+        SystemPropertiesConfigSource() {
+            super(ConfigUtils.propertiesToMap(System.getProperties()), false, "");
+        }
+    }
 }

config/config/src/main/java/io/helidon/config/internal/MapConfigSource.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2018 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2019 Oracle and/or its affiliates. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -55,12 +55,11 @@ public MapConfigSource(Map<String, String> map, boolean strict, String mapSource
 
     @Override
     public String description() {
-        return ConfigSource.super.description() + "[" + mapSourceName + "]";
+        return ConfigSource.super.description() + (mapSourceName.isEmpty() ? "" : "[" + mapSourceName + "]");
     }
 
     @Override
     public Optional<ObjectNode> load() {
         return Optional.of(ConfigUtils.mapToObjectNode(map, strict));
     }
-
 }

config/config/src/test/java/io/helidon/config/ConfigSourcesTest.java

@@ -32,13 +32,12 @@
 import static io.helidon.common.CollectionsHelper.mapOf;
 import static io.helidon.config.ConfigSources.DEFAULT_MAP_NAME;
 import static io.helidon.config.ConfigSources.DEFAULT_PROPERTIES_NAME;
-import static io.helidon.config.ConfigSources.ENV_VARS_NAME;
-import static io.helidon.config.ConfigSources.SYS_PROPS_NAME;
 import static io.helidon.config.ConfigSources.prefixed;
 import static io.helidon.config.ValueNodeMatcher.valueNode;
 import static java.util.Collections.emptyMap;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
 import static org.hamcrest.Matchers.sameInstance;
@@ -186,13 +185,17 @@ public void testLoadMultipleSource() {
     }
 
     @Test
-    public void testSystemPropertiesSourceName() {
-        assertThat(ConfigSources.systemProperties().get().description(), containsString(SYS_PROPS_NAME));
+    public void testSystemPropertiesSourceType() {
+        ConfigSource source = ConfigSources.systemProperties();
+        assertThat(source, is(instanceOf(ConfigSources.SystemPropertiesConfigSource.class)));
+        assertThat(source.description(), is("SystemPropertiesConfig"));
     }
 
     @Test
-    public void testEnvironmentVariablesSourceName() {
-        assertThat(ConfigSources.environmentVariables().get().description(), containsString(ENV_VARS_NAME));
+    public void testEnvironmentVariablesSourceType() {
+        ConfigSource source = ConfigSources.environmentVariables();
+        assertThat(source, is(instanceOf(ConfigSources.EnvironmentVariablesConfigSource.class)));
+        assertThat(source.description(), is("EnvironmentVariablesConfig"));
     }
 
     @Test

config/config/src/test/java/io/helidon/config/ConfigTest.java

@@ -27,6 +27,7 @@
 import io.helidon.config.Config.Key;
 import io.helidon.config.spi.ConfigNode.ListNode;
 import io.helidon.config.spi.ConfigNode.ObjectNode;
+import io.helidon.config.spi.ConfigSourceTest;
 import io.helidon.config.test.infra.RestoreSystemPropertiesExt;
 
 import org.hamcrest.Matcher;
@@ -36,8 +37,6 @@
 import static io.helidon.config.Config.Type.LIST;
 import static io.helidon.config.Config.Type.OBJECT;
 import static io.helidon.config.Config.Type.VALUE;
-import static io.helidon.config.spi.ConfigSourceTest.TEST_ENV_VAR_NAME;
-import static io.helidon.config.spi.ConfigSourceTest.TEST_ENV_VAR_VALUE;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
@@ -61,12 +60,19 @@ public class ConfigTest {
     private static final String TEST_SYS_PROP_NAME = "this_is_my_property-ConfigTest";
     private static final String TEST_SYS_PROP_VALUE = "This Is My SYS PROPS Value.";
 
+    private static final String TEST_ENV_VAR_NAME = "FOO_BAR";
+    private static final String TEST_ENV_VAR_VALUE = "mapped-env-value";
+
+    private static final String OVERRIDE_NAME = "simple";
+    private static final String OVERRIDE_ENV_VAR_VALUE = "unmapped-env-value";
+    private static final String OVERRIDE_SYS_PROP_VALUE = "unmapped-sys-prop-value";
+
     private static final boolean LOG = false;
 
     private static final String OBJECT_VALUE_PREFIX = "object-";
     private static final String LIST_VALUE_PREFIX = "list-";
 
-    private void testKeyNotSet(Config config) {
+    private static void testKeyNotSet(Config config) {
         assertThat(config, not(nullValue()));
         assertThat(config.traverse().collect(Collectors.toList()), not(empty()));
         assertThat(config.get(TEST_SYS_PROP_NAME).type(), is(Config.Type.MISSING));
@@ -105,19 +111,19 @@ public void testBuilderDefaultConfigSourceKeyFromSysProps() {
     private void testKeyFromEnvVars(Config config) {
         assertThat(config, not(nullValue()));
         assertThat(config.traverse().collect(Collectors.toList()), not(empty()));
-        assertThat(config.get(TEST_ENV_VAR_NAME).asString(), is(ConfigValues.simpleValue(TEST_ENV_VAR_VALUE)));
+        assertThat(config.get(ConfigSourceTest.TEST_ENV_VAR_NAME).asString(), is(ConfigValues.simpleValue(ConfigSourceTest.TEST_ENV_VAR_VALUE)));
     }
 
     @Test
     public void testCreateKeyFromEnvVars() {
-        System.setProperty(TEST_ENV_VAR_NAME, "This value is not used, but from Env Vars, see pom.xml!");
+        System.setProperty(ConfigSourceTest.TEST_ENV_VAR_NAME, "This value is not used, but from Env Vars, see pom.xml!");
 
         testKeyFromEnvVars(Config.create());
     }
 
     @Test
     public void testBuilderDefaultConfigSourceKeyFromEnvVars() {
-        System.setProperty(TEST_ENV_VAR_NAME, "This value is not used, but from Env Vars, see pom.xml!");
+        System.setProperty(ConfigSourceTest.TEST_ENV_VAR_NAME, "This value is not used, but from Env Vars, see pom.xml!");
 
         testKeyFromEnvVars(Config.builder().build());
     }
@@ -616,6 +622,77 @@ objects to assign values to the complex nodes (object- and list-type)
         assertThat(config.get("list-1").asString(), is(ConfigValues.simpleValue(LIST_VALUE_PREFIX + valueQual + "-2")));
     }
 
+    @Test
+    void testImplicitSysPropAndEnvVarPrecedence() {
+        System.setProperty(OVERRIDE_NAME, OVERRIDE_SYS_PROP_VALUE);
+        System.setProperty(TEST_SYS_PROP_NAME, TEST_SYS_PROP_VALUE);
+
+        Config config = Config.create();
+
+        assertThat(config.get(TEST_SYS_PROP_NAME).asString().get(), is(TEST_SYS_PROP_VALUE));
+        assertThat(config.get(TEST_ENV_VAR_NAME).asString().get(), is(TEST_ENV_VAR_VALUE));
+
+        assertThat(config.get(OVERRIDE_NAME).asString().get(), is(OVERRIDE_ENV_VAR_VALUE));
+    }
+
+    @Test
+    void testExplicitSysPropAndEnvVarPrecedence() {
+        System.setProperty(OVERRIDE_NAME, OVERRIDE_SYS_PROP_VALUE);
+        System.setProperty(TEST_SYS_PROP_NAME, TEST_SYS_PROP_VALUE);
+
+        Config config = Config.builder()
+                              .sources(ConfigSources.systemProperties(),
+                                       ConfigSources.environmentVariables())
+                              .build();
+
+        assertThat(config.get(TEST_SYS_PROP_NAME).asString().get(), is(TEST_SYS_PROP_VALUE));
+        assertThat(config.get(TEST_ENV_VAR_NAME).asString().get(), is(TEST_ENV_VAR_VALUE));
+
+        assertThat(config.get(OVERRIDE_NAME).asString().get(), is(OVERRIDE_SYS_PROP_VALUE));
+
+        config = Config.builder()
+                       .sources(ConfigSources.environmentVariables(),
+                                ConfigSources.systemProperties())
+                       .build();
+
+        assertThat(config.get(TEST_SYS_PROP_NAME).asString().get(), is(TEST_SYS_PROP_VALUE));
+        assertThat(config.get(TEST_ENV_VAR_NAME).asString().get(), is(TEST_ENV_VAR_VALUE));
+
+        assertThat(config.get(OVERRIDE_NAME).asString().get(), is(OVERRIDE_ENV_VAR_VALUE));
+    }
+
+    @Test
+    void testExplicitEnvVarSourceAndImplicitSysPropSourcePrecedence() {
+        System.setProperty(OVERRIDE_NAME, OVERRIDE_SYS_PROP_VALUE);
+        System.setProperty(TEST_SYS_PROP_NAME, TEST_SYS_PROP_VALUE);
+
+        Config config = Config.builder()
+                              .sources(ConfigSources.environmentVariables())
+                              .build();
+
+        assertThat(config.get(TEST_SYS_PROP_NAME).asString().get(), is(TEST_SYS_PROP_VALUE));
+        assertThat(config.get(TEST_ENV_VAR_NAME).asString().get(), is(TEST_ENV_VAR_VALUE));
+
+        // Implicit sources always take precedence! (??)
+        assertThat(config.get(OVERRIDE_NAME).asString().get(), is(OVERRIDE_SYS_PROP_VALUE));
+    }
+
+    @Test
+    void testExplicitSysPropSourceAndImplicitEnvVarSourcePrecedence() {
+        System.setProperty(OVERRIDE_NAME, OVERRIDE_SYS_PROP_VALUE);
+        System.setProperty(TEST_SYS_PROP_NAME, TEST_SYS_PROP_VALUE);
+
+        Config config = Config.builder()
+                              .sources(ConfigSources.systemProperties())
+                              .build();
+
+        assertThat(config.get(TEST_SYS_PROP_NAME).asString().get(), is(TEST_SYS_PROP_VALUE));
+        assertThat(config.get(TEST_ENV_VAR_NAME).asString().get(), is(TEST_ENV_VAR_VALUE));
+
+        // Implicit sources always take precedence! (??)
+        assertThat(config.get(OVERRIDE_NAME).asString().get(), is(OVERRIDE_ENV_VAR_VALUE));
+    }
+
     private void testConfigKeyEscapeUnescapeName(String name, String escapedName) {
         assertThat(Key.escapeName(name), is(escapedName));
         assertThat(Key.unescapeName(escapedName), is(name));

config/config/src/test/java/io/helidon/config/spi/ConfigSourceTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2018 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2019 Oracle and/or its affiliates. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -28,15 +28,15 @@
 import io.helidon.config.internal.PropertiesConfigParser;
 import io.helidon.config.spi.ConfigNode.ObjectNode;
 import io.helidon.config.test.infra.RestoreSystemPropertiesExt;
-import static org.hamcrest.MatcherAssert.assertThat;
 
 import org.hamcrest.core.Is;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.is;
 import static org.junit.jupiter.api.Assertions.fail;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.Mockito.mock;
@@ -113,7 +113,7 @@ public void testFromTextLoad() {
     public void testFromSystemPropertiesDescription() {
         ConfigSource configSource = ConfigSources.systemProperties();
 
-        assertThat(configSource.description(), is("MapConfig[sys-props]"));
+        assertThat(configSource.description(), is("SystemPropertiesConfig"));
     }
 
     @Test
@@ -131,7 +131,7 @@ public void testFromSystemProperties() {
     public void testFromEnvironmentVariablesDescription() {
         ConfigSource configSource = ConfigSources.environmentVariables();
 
-        assertThat(configSource.description(), is("MapConfig[env-vars]"));
+        assertThat(configSource.description(), is("EnvironmentVariablesConfig"));
     }
 
     @Test