diff --git a/config/config/src/main/java/io/helidon/config/BuilderImpl.java b/config/config/src/main/java/io/helidon/config/BuilderImpl.java index b2cb66cafd6..5a2d067aff6 100644 --- a/config/config/src/main/java/io/helidon/config/BuilderImpl.java +++ b/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> 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 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, diff --git a/config/config/src/main/java/io/helidon/config/ConfigSources.java b/config/config/src/main/java/io/helidon/config/ConfigSources.java index 68c81687095..a98ac6fbdf3 100644 --- a/config/config/src/main/java/io/helidon/config/ConfigSources.java +++ b/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 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 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, ""); + } + } } diff --git a/config/config/src/main/java/io/helidon/config/internal/MapConfigSource.java b/config/config/src/main/java/io/helidon/config/internal/MapConfigSource.java index b9f95dadcf0..84370f50226 100644 --- a/config/config/src/main/java/io/helidon/config/internal/MapConfigSource.java +++ b/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 map, boolean strict, String mapSource @Override public String description() { - return ConfigSource.super.description() + "[" + mapSourceName + "]"; + return ConfigSource.super.description() + (mapSourceName.isEmpty() ? "" : "[" + mapSourceName + "]"); } @Override public Optional load() { return Optional.of(ConfigUtils.mapToObjectNode(map, strict)); } - } diff --git a/config/config/src/test/java/io/helidon/config/ConfigSourcesTest.java b/config/config/src/test/java/io/helidon/config/ConfigSourcesTest.java index 549c9422aed..e0972228cf9 100644 --- a/config/config/src/test/java/io/helidon/config/ConfigSourcesTest.java +++ b/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 diff --git a/config/config/src/test/java/io/helidon/config/ConfigTest.java b/config/config/src/test/java/io/helidon/config/ConfigTest.java index d8d7f0cd3a1..9f57e7fdd94 100644 --- a/config/config/src/test/java/io/helidon/config/ConfigTest.java +++ b/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)); diff --git a/config/config/src/test/java/io/helidon/config/spi/ConfigSourceTest.java b/config/config/src/test/java/io/helidon/config/spi/ConfigSourceTest.java index 29c642cb6ae..965dce07e2b 100644 --- a/config/config/src/test/java/io/helidon/config/spi/ConfigSourceTest.java +++ b/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