From a44f19ac91a18c2fd83b15a0c4ce80a135386798 Mon Sep 17 00:00:00 2001 From: Roberto Cortez Date: Thu, 7 Oct 2021 14:23:15 +0100 Subject: [PATCH] Only record config properties coming from ConfigSources --- .../BuildTimeConfigurationReader.java | 23 +++++--- .../RestClientBuildTimeConfigSource.java | 48 ++++++++++++++++ .../RestClientOverrideRuntimeConfigTest.java | 57 +++++++++++++++++++ .../RestClientRunTimeConfigSource.java | 50 ++++++++++++++++ 4 files changed, 169 insertions(+), 9 deletions(-) create mode 100644 extensions/resteasy-classic/rest-client/deployment/src/test/java/io/quarkus/restclient/configuration/RestClientBuildTimeConfigSource.java create mode 100644 extensions/resteasy-classic/rest-client/deployment/src/test/java/io/quarkus/restclient/configuration/RestClientOverrideRuntimeConfigTest.java create mode 100644 extensions/resteasy-classic/rest-client/deployment/src/test/java/io/quarkus/restclient/configuration/RestClientRunTimeConfigSource.java diff --git a/core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java b/core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java index 1fd5ba7bea997..f7024f020f180 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java @@ -6,6 +6,7 @@ import static io.quarkus.deployment.util.ReflectUtil.toError; import static io.quarkus.deployment.util.ReflectUtil.typeOfParameter; import static io.quarkus.deployment.util.ReflectUtil.unwrapInvocationTargetException; +import static io.smallrye.config.SmallRyeConfig.SMALLRYE_CONFIG_PROFILE; import static java.util.stream.Collectors.toSet; import java.lang.reflect.Constructor; @@ -375,7 +376,7 @@ ReadResult run() { if (matched != null) { // it's a specified run-time default (record for later) specifiedRunTimeDefaultValues.put(propertyName, Expressions.withoutExpansion( - () -> config.getOptionalValue(propertyName, String.class).orElse(""))); + () -> runtimeDefaultsConfig.getOptionalValue(propertyName, String.class).orElse(""))); continue; } @@ -733,12 +734,12 @@ private Converter getConverter(SmallRyeConfig config, Field field, ConverterT } /** - * We collect all properties from ConfigSources, because Config#getPropertyNames exclude the active profiled - * properties, meaning that the property is written in the default config source unprofiled. This may cause - * issues if we run with a different profile and fallback to defaults. + * We collect all properties from eligible ConfigSources, because Config#getPropertyNames exclude the active + * profiled properties, meaning that the property is written in the default config source without the profile + * prefix. This may cause issues if we run with a different profile and fallback to defaults. * * We also filter the properties coming from the System with the registered roots, because we don't want to - * record properties set by the compiling JVM (or other properties set that are only related to the build). + * record properties set by the compiling JVM (or other properties that are only related to the build). */ private Set getAllProperties(final Set registeredRoots) { Set properties = new HashSet<>(); @@ -762,9 +763,11 @@ private Set getAllProperties(final Set registeredRoots) { /** * Use this Config instance to record the specified runtime default values. We cannot use the main Config - * instance because it may record values coming from the EnvSource in build time. We do exclude the properties - * coming from the EnvSource, but a call to getValue may still provide values coming from the EnvSource, so we - * need to exclude it from sources when recoding values. + * instance because it may record values coming from the EnvSource in build time. Environment variable values + * may be completely different between build and runtime, so it doesn't make sense to record these. + * + * We do exclude the properties coming from the EnvSource, but a call to getValue may still provide a result + * coming from the EnvSource, so we need to exclude it from the sources when recording values for runtime. * * We also do not want to completely exclude the EnvSource, because it may provide values for the build. This * is only specific when recording the defaults. @@ -772,7 +775,9 @@ private Set getAllProperties(final Set registeredRoots) { * @return a new SmallRye instance without the EnvSources. */ private SmallRyeConfig getConfigForRuntimeDefaults() { - SmallRyeConfigBuilder builder = ConfigUtils.emptyConfigBuilder(); + SmallRyeConfigBuilder builder = new SmallRyeConfigBuilder(); + builder.withDefaultValue(SMALLRYE_CONFIG_PROFILE, ProfileManager.getActiveProfile()); + builder.addDefaultInterceptors(); for (ConfigSource configSource : config.getConfigSources()) { if (configSource instanceof EnvConfigSource) { continue; diff --git a/extensions/resteasy-classic/rest-client/deployment/src/test/java/io/quarkus/restclient/configuration/RestClientBuildTimeConfigSource.java b/extensions/resteasy-classic/rest-client/deployment/src/test/java/io/quarkus/restclient/configuration/RestClientBuildTimeConfigSource.java new file mode 100644 index 0000000000000..94552c9519660 --- /dev/null +++ b/extensions/resteasy-classic/rest-client/deployment/src/test/java/io/quarkus/restclient/configuration/RestClientBuildTimeConfigSource.java @@ -0,0 +1,48 @@ +package io.quarkus.restclient.configuration; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Set; + +import org.eclipse.microprofile.config.ConfigProvider; +import org.eclipse.microprofile.config.spi.ConfigSource; + +import io.smallrye.config.common.MapBackedConfigSource; + +public class RestClientBuildTimeConfigSource extends MapBackedConfigSource { + public RestClientBuildTimeConfigSource() { + super(RestClientBuildTimeConfigSource.class.getName(), new HashMap<>()); + } + + @Override + public String getValue(final String propertyName) { + if (!propertyName.equals("io.quarkus.restclient.configuration.EchoClient/mp-rest/url")) { + return null; + } + + if (isBuildTime()) { + return "http://nohost:${quarkus.http.test-port:8081}"; + } + + return null; + } + + @Override + public Set getPropertyNames() { + return Collections.singleton("io.quarkus.restclient.configuration.EchoClient/mp-rest/url"); + } + + @Override + public int getOrdinal() { + return Integer.MAX_VALUE; + } + + private static boolean isBuildTime() { + for (ConfigSource configSource : ConfigProvider.getConfig().getConfigSources()) { + if (configSource.getClass().getSimpleName().equals("BuildTimeEnvConfigSource")) { + return true; + } + } + return false; + } +} diff --git a/extensions/resteasy-classic/rest-client/deployment/src/test/java/io/quarkus/restclient/configuration/RestClientOverrideRuntimeConfigTest.java b/extensions/resteasy-classic/rest-client/deployment/src/test/java/io/quarkus/restclient/configuration/RestClientOverrideRuntimeConfigTest.java new file mode 100644 index 0000000000000..ed09ebb2dc1d9 --- /dev/null +++ b/extensions/resteasy-classic/rest-client/deployment/src/test/java/io/quarkus/restclient/configuration/RestClientOverrideRuntimeConfigTest.java @@ -0,0 +1,57 @@ +package io.quarkus.restclient.configuration; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.Optional; + +import javax.inject.Inject; + +import org.eclipse.microprofile.config.spi.ConfigSource; +import org.eclipse.microprofile.rest.client.inject.RestClient; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusUnitTest; +import io.smallrye.config.ConfigValue; +import io.smallrye.config.SmallRyeConfig; + +public class RestClientOverrideRuntimeConfigTest { + @RegisterExtension + static final QuarkusUnitTest TEST = new QuarkusUnitTest().setArchiveProducer( + () -> ShrinkWrap.create(JavaArchive.class) + .addClasses(EchoResource.class, EchoClient.class, RestClientBuildTimeConfigSource.class, + RestClientRunTimeConfigSource.class) + .addAsServiceProvider("org.eclipse.microprofile.config.spi.ConfigSource", + "io.quarkus.restclient.configuration.RestClientBuildTimeConfigSource", + "io.quarkus.restclient.configuration.RestClientRunTimeConfigSource")); + + @Inject + @RestClient + EchoClient echoClient; + @Inject + SmallRyeConfig config; + + @Test + void overrideConfig() { + Optional specifiedDefaultValues = config + .getConfigSource("PropertiesConfigSource[source=Specified default values]"); + assertTrue(specifiedDefaultValues.isPresent()); + assertTrue(specifiedDefaultValues.get().getPropertyNames() + .contains("io.quarkus.restclient.configuration.EchoClient/mp-rest/url")); + // This config key comes from the interceptor and it is recorded with an empty value. This allow the user to still override using the original key + assertEquals("", specifiedDefaultValues.get() + .getValue("quarkus.rest-client.\"io.quarkus.restclient.configuration.EchoClient\".url")); + + ConfigValue mpValue = config.getConfigValue("io.quarkus.restclient.configuration.EchoClient/mp-rest/url"); + // Fallbacks from runtime to the override build time value + ConfigValue quarkusValue = config + .getConfigValue("quarkus.rest-client.\"io.quarkus.restclient.configuration.EchoClient\".url"); + assertEquals(mpValue.getValue(), quarkusValue.getValue()); + assertEquals(RestClientRunTimeConfigSource.class.getName(), quarkusValue.getConfigSourceName()); + + assertEquals("Hi", echoClient.echo("Hi")); + } +} diff --git a/extensions/resteasy-classic/rest-client/deployment/src/test/java/io/quarkus/restclient/configuration/RestClientRunTimeConfigSource.java b/extensions/resteasy-classic/rest-client/deployment/src/test/java/io/quarkus/restclient/configuration/RestClientRunTimeConfigSource.java new file mode 100644 index 0000000000000..92f01d7b5950f --- /dev/null +++ b/extensions/resteasy-classic/rest-client/deployment/src/test/java/io/quarkus/restclient/configuration/RestClientRunTimeConfigSource.java @@ -0,0 +1,50 @@ +package io.quarkus.restclient.configuration; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Set; + +import org.eclipse.microprofile.config.ConfigProvider; +import org.eclipse.microprofile.config.spi.ConfigSource; + +import io.quarkus.runtime.annotations.StaticInitSafe; +import io.smallrye.config.common.MapBackedConfigSource; + +@StaticInitSafe +public class RestClientRunTimeConfigSource extends MapBackedConfigSource { + public RestClientRunTimeConfigSource() { + super(RestClientRunTimeConfigSource.class.getName(), new HashMap<>()); + } + + @Override + public String getValue(final String propertyName) { + if (!propertyName.equals("io.quarkus.restclient.configuration.EchoClient/mp-rest/url")) { + return null; + } + + if (isRuntime()) { + return "http://localhost:${quarkus.http.test-port:8081}"; + } + + return null; + } + + @Override + public Set getPropertyNames() { + return Collections.singleton("io.quarkus.restclient.configuration.EchoClient/mp-rest/url"); + } + + @Override + public int getOrdinal() { + return Integer.MAX_VALUE; + } + + private static boolean isRuntime() { + for (ConfigSource configSource : ConfigProvider.getConfig().getConfigSources()) { + if (configSource.getName().equals("PropertiesConfigSource[source=Specified default values]")) { + return true; + } + } + return false; + } +}