From 9874da150e5003cf1b19bd2ab95f097cf3ffe4f0 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Fri, 17 Feb 2023 11:57:03 -0300 Subject: [PATCH] Avoid resolving expressions twice but rely on MP config expression support Closes #16573 --- .../PropertyMappingInterceptor.java | 37 ++++++---------- .../configuration/mappers/PropertyMapper.java | 21 ++++++--- .../mappers/PropertyMappers.java | 43 +------------------ .../configuration/test/ConfigurationTest.java | 9 ++++ 4 files changed, 37 insertions(+), 73 deletions(-) diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/PropertyMappingInterceptor.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/PropertyMappingInterceptor.java index 3bdfcb1c92..dc2deb585f 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/PropertyMappingInterceptor.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/PropertyMappingInterceptor.java @@ -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; *

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. + * + *

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); } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java index ec40c8672e..fe90b05189 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java @@ -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 { } // 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 { return current; } + Optional 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 getOption() { return this.option; } @@ -213,6 +214,14 @@ public class PropertyMapper { 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 { private final Option option; diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMappers.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMappers.java index 6741d50414..0b94daa07c 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMappers.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMappers.java @@ -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 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 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 getPrefixedMapper(String name) { - return MAPPERS.entrySet().stream().filter( - new Predicate>() { - @Override - public boolean test(Map.Entry 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 { private Map> buildTimeMappers = new EnumMap<>(OptionCategory.class); diff --git a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java index 4d7158d244..9b04dfdc4c 100644 --- a/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java +++ b/quarkus/runtime/src/test/java/org/keycloak/quarkus/runtime/configuration/test/ConfigurationTest.java @@ -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");