Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only record config properties coming from ConfigSources #20590

Merged
merged 1 commit into from
Oct 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
}