Avoid resolving expressions twice but rely on MP config expression support

Closes #16573
This commit is contained in:
Pedro Igor 2023-02-17 11:57:03 -03:00 committed by Václav Muzikář
parent 137b1fab9e
commit 9874da150e
4 changed files with 37 additions and 73 deletions

View file

@ -16,14 +16,16 @@
*/
package org.keycloak.quarkus.runtime.configuration;
import static org.keycloak.quarkus.runtime.configuration.MicroProfileConfigProvider.NS_QUARKUS;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import javax.annotation.Priority;
import io.quarkus.runtime.configuration.AbstractRawDefaultConfigSource;
import io.smallrye.config.ConfigSourceInterceptor;
import io.smallrye.config.ConfigSourceInterceptorContext;
import io.smallrye.config.ConfigValue;
import org.keycloak.common.util.StringPropertyReplacer;
import org.keycloak.quarkus.runtime.Environment;
import io.smallrye.config.Priorities;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMapper;
import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
@ -36,31 +38,16 @@ import org.keycloak.quarkus.runtime.configuration.mappers.PropertyMappers;
* <p>The {@link PropertyMapper} can either perform a 1:1 mapping where the value of a property from
* Keycloak (e.g.: https.port) is mapped to a single properties in Quarkus, or perform a 1:N mapping where the value of a property
* from Keycloak (e.g.: database) is mapped to multiple properties in Quarkus.
*
* <p>This interceptor should execute between the {@link io.smallrye.config.ExpressionConfigSourceInterceptor} and the {@link io.smallrye.config.ProfileConfigSourceInterceptor}
* so that expressions are properly resolved after executing this interceptor while still able to resolve properties based on the current
* profile.
*/
@Priority(Priorities.LIBRARY + 201)
public class PropertyMappingInterceptor implements ConfigSourceInterceptor {
@Override
public ConfigValue getValue(ConfigSourceInterceptorContext context, String name) {
ConfigValue value = PropertyMappers.getValue(context, name);
if (value == null || value.getValue() == null) {
return null;
}
if (value.getValue().indexOf("${") == -1) {
return value;
}
return value.withValue(
StringPropertyReplacer.replaceProperties(value.getValue(),
property -> {
ConfigValue prop = context.proceed(property);
if (prop == null) {
return null;
}
return prop.getValue();
}));
return PropertyMappers.getValue(context, name);
}
}

View file

@ -27,7 +27,6 @@ import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.function.BiFunction;
import java.util.stream.Collectors;
import io.smallrye.config.ConfigSourceInterceptorContext;
import io.smallrye.config.ConfigValue;
@ -95,13 +94,13 @@ public class PropertyMapper<T> {
}
// try to obtain the value for the property we want to map first
ConfigValue config = context.proceed(from);
ConfigValue config = convertValue(context.proceed(from));
if (config == null) {
if (mapFrom != null) {
// if the property we want to map depends on another one, we use the value from the other property to call the mapper
String parentKey = MicroProfileConfigProvider.NS_KEYCLOAK_PREFIX + mapFrom;
ConfigValue parentValue = context.proceed(parentKey);
ConfigValue parentValue = convertValue(context.proceed(parentKey));
if (parentValue == null) {
// parent value not explicitly set, try to resolve the default value set to the parent property
@ -131,18 +130,20 @@ public class PropertyMapper<T> {
return current;
}
Optional<String> configValue = ofNullable(config.getValue());
if (config.getName().equals(name)) {
return config;
}
ConfigValue value = transformValue(ofNullable(config.getValue()), context);
ConfigValue transformedValue = transformValue(configValue, context);
// we always fallback to the current value from the property we are mapping
if (value == null) {
if (transformedValue == null) {
return context.proceed(name);
}
return value;
return transformedValue;
}
public Option<T> getOption() { return this.option; }
@ -213,6 +214,14 @@ public class PropertyMapper<T> {
return ConfigValue.builder().withName(to).withValue(mappedValue.get()).withRawValue(value.orElse(null)).build();
}
private ConfigValue convertValue(ConfigValue configValue) {
if (configValue == null) {
return null;
}
return configValue.withValue(ofNullable(configValue.getValue()).map(String::trim).orElse(null));
}
public static class Builder<T> {
private final Option<T> option;

View file

@ -14,9 +14,7 @@ import java.util.EnumMap;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Function;
import java.util.function.Predicate;
public final class PropertyMappers {
@ -44,19 +42,7 @@ public final class PropertyMappers {
public static ConfigValue getValue(ConfigSourceInterceptorContext context, String name) {
PropertyMapper mapper = MAPPERS.getOrDefault(name, PropertyMapper.IDENTITY);
ConfigValue configValue = mapper.getConfigValue(name, context);
if (configValue == null) {
Optional<String> prefixedMapper = getPrefixedMapper(name);
if (prefixedMapper.isPresent()) {
return MAPPERS.get(prefixedMapper.get()).getConfigValue(name, context);
}
} else {
configValue.withName(mapper.getTo());
}
return configValue;
return mapper.getConfigValue(name, context);
}
public static boolean isBuildTimeProperty(String name) {
@ -67,14 +53,6 @@ public final class PropertyMappers {
PropertyMapper mapper = MAPPERS.get(name);
boolean isBuildTimeProperty = mapper == null ? false : mapper.isBuildTime();
if (mapper == null && !isBuildTimeProperty) {
Optional<String> prefixedMapper = PropertyMappers.getPrefixedMapper(name);
if (prefixedMapper.isPresent()) {
isBuildTimeProperty = MAPPERS.get(prefixedMapper.get()).isBuildTime();
}
}
return isBuildTimeProperty
&& !"kc.version".equals(name)
&& !ConfigArgsConfigSource.CLI_ARGS.equals(name)
@ -136,25 +114,6 @@ public final class PropertyMappers {
return mapper.getCategory().getSupportLevel().equals(ConfigSupportLevel.SUPPORTED);
}
private static Optional<String> getPrefixedMapper(String name) {
return MAPPERS.entrySet().stream().filter(
new Predicate<Map.Entry<String, PropertyMapper>>() {
@Override
public boolean test(Map.Entry<String, PropertyMapper> entry) {
String key = entry.getKey();
if (!key.endsWith(".")) {
return false;
}
// checks both to and from mapping
return name.startsWith(key) || name.startsWith(entry.getValue().getFrom());
}
})
.map(Map.Entry::getKey)
.findAny();
}
private static class MappersConfig extends HashMap<String, PropertyMapper> {
private Map<OptionCategory, List<PropertyMapper>> buildTimeMappers = new EnumMap<>(OptionCategory.class);

View file

@ -293,6 +293,15 @@ public class ConfigurationTest {
assertEquals("postgresql", config.getConfigValue("quarkus.datasource.db-kind").getValue());
}
@Test
public void testRemoveSpaceFromValue() {
System.setProperty(CLI_ARGS, "--db=postgres ");
SmallRyeConfig config = createConfig();
assertEquals("io.quarkus.hibernate.orm.runtime.dialect.QuarkusPostgreSQL10Dialect",
config.getConfigValue("quarkus.hibernate-orm.dialect").getValue());
assertEquals("postgres", config.getConfigValue("quarkus.datasource.db-kind").getRawValue());
}
@Test
public void testDefaultDbPortGetApplied() {
System.setProperty(CLI_ARGS, "--db=mssql" + ARG_SEPARATOR + "--db-url-host=myhost" + ARG_SEPARATOR + "--db-url-database=kcdb" + ARG_SEPARATOR + "--db-url-port=1234" + ARG_SEPARATOR + "--db-url-properties=?foo=bar");