Skip to content

Commit

Permalink
Merge pull request #20590 from radcortez/config-recording
Browse files Browse the repository at this point in the history
Only record config properties coming from ConfigSources
  • Loading branch information
michalszynkiewicz authored Oct 15, 2021
2 parents 09faf5d + a44f19a commit 5bfeb6f
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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<String> getAllProperties(final Set<String> registeredRoots) {
Set<String> properties = new HashSet<>();
Expand All @@ -762,17 +763,21 @@ private Set<String> getAllProperties(final Set<String> 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.
*
* @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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String> 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;
}
}
Original file line number Diff line number Diff line change
@@ -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<ConfigSource> 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"));
}
}
Original file line number Diff line number Diff line change
@@ -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<String> 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;
}
}

0 comments on commit 5bfeb6f

Please sign in to comment.