fix: simplify / refine validation methods (#32487)

closes: #32455

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/BootstrapAdminPropertyMappers.java

@@ -35,7 +35,7 @@ public final class BootstrapAdminPropertyMappers {
         return new PropertyMapper[]{
                 fromOption(BootstrapAdminOptions.USERNAME)
                         .paramLabel("username")
-                        .appendValidateEnabled(BootstrapAdminPropertyMappers::isPasswordSet, PASSWORD_SET)
+                        .addValidateEnabled(BootstrapAdminPropertyMappers::isPasswordSet, PASSWORD_SET)
                         .build(),
                 fromOption(BootstrapAdminOptions.PASSWORD)
                         .paramLabel("password")
@@ -47,7 +47,7 @@ public final class BootstrapAdminPropertyMappers {
                         .build(),*/
                 fromOption(BootstrapAdminOptions.CLIENT_ID)
                         .paramLabel("client id")
-                        .appendValidateEnabled(BootstrapAdminPropertyMappers::isClientSecretSet, CLIENT_SECRET_SET)
+                        .addValidateEnabled(BootstrapAdminPropertyMappers::isClientSecretSet, CLIENT_SECRET_SET)
                         .build(),
                 fromOption(BootstrapAdminOptions.CLIENT_SECRET)
                         .paramLabel("client secret")

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ExportPropertyMappers.java

@@ -69,7 +69,7 @@ public final class ExportPropertyMappers {
                         .build(),
                 fromOption(ExportOptions.USERS)
                         .to("kc.spi-export-dir-users-export-strategy")
-                        .validator(ExportPropertyMappers::validateUsersUsage)
+                        .addValidator(ExportPropertyMappers::validateUsersUsage)
                         .paramLabel("strategy")
                         .build(),
                 fromOption(ExportOptions.USERS_PER_FILE)
@@ -81,8 +81,6 @@ public final class ExportPropertyMappers {
     }
 
     private static void validateUsersUsage(PropertyMapper<?> mapper, ConfigValue value) {
-        mapper.validateExpectedValues(value, mapper::validateSingleValue);
-
         if (!isBlank(ExportOptions.FILE) && isBlank(ExportOptions.DIR)) {
             var sameFileIsSpecified = UsersExportStrategy.SAME_FILE.toString().toLowerCase().equals(value.getValue());
 

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/FeaturePropertyMappers.java

@@ -23,8 +23,7 @@ public final class FeaturePropertyMappers {
         return new PropertyMapper[] {
                 fromOption(FeatureOptions.FEATURES)
                         .paramLabel("feature")
-                        .validator((mapper, value) -> mapper.validateExpectedValues(value,
+                        .validator(FeaturePropertyMappers::validateEnabledFeature)
-                                (c, v) -> validateEnabledFeature(v)))
                         .build(),
                 fromOption(FeatureOptions.FEATURES_DISABLED)
                         .paramLabel("feature")

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/LoggingPropertyMappers.java

@@ -94,8 +94,7 @@ public final class LoggingPropertyMappers {
                 fromOption(LoggingOptions.LOG_LEVEL)
                         .to("quarkus.log.level")
                         .transformer(LoggingPropertyMappers::resolveLogLevel)
-                        .validator((mapper, value) -> mapper.validateExpectedValues(value,
+                        .validator(LoggingPropertyMappers::validateLogLevel)
-                                (c, v) -> validateLogLevel(v)))
                         .paramLabel("category:level")
                         .build(),
                 // Syslog

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ManagementPropertyMappers.java

@@ -76,25 +76,25 @@ public class ManagementPropertyMappers {
                 fromOption(ManagementOptions.HTTPS_MANAGEMENT_CERTIFICATE_FILE)
                         .mapFrom(HttpOptions.HTTPS_CERTIFICATE_FILE.getKey())
                         .to("quarkus.management.ssl.certificate.files")
-                        .validator((mapper, value) -> validateTlsProperties())
+                        .validator(value -> validateTlsProperties())
                         .paramLabel("file")
                         .build(),
                 fromOption(ManagementOptions.HTTPS_MANAGEMENT_CERTIFICATE_KEY_FILE)
                         .mapFrom(HttpOptions.HTTPS_CERTIFICATE_KEY_FILE.getKey())
                         .to("quarkus.management.ssl.certificate.key-files")
-                        .validator((mapper, value) -> validateTlsProperties())
+                        .validator(value -> validateTlsProperties())
                         .paramLabel("file")
                         .build(),
                 fromOption(ManagementOptions.HTTPS_MANAGEMENT_KEY_STORE_FILE)
                         .mapFrom(HttpOptions.HTTPS_KEY_STORE_FILE.getKey())
                         .to("quarkus.management.ssl.certificate.key-store-file")
-                        .validator((mapper, value) -> validateTlsProperties())
+                        .validator(value -> validateTlsProperties())
                         .paramLabel("file")
                         .build(),
                 fromOption(ManagementOptions.HTTPS_MANAGEMENT_KEY_STORE_PASSWORD)
                         .mapFrom(HttpOptions.HTTPS_KEY_STORE_PASSWORD.getKey())
                         .to("quarkus.management.ssl.certificate.key-store-password")
-                        .validator((mapper, value) -> validateTlsProperties())
+                        .validator(value -> validateTlsProperties())
                         .paramLabel("password")
                         .isMasked(true)
                         .build(),

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/PropertyMapper.java

@@ -24,10 +24,13 @@ import static org.keycloak.quarkus.runtime.configuration.Configuration.toCliForm
 import static org.keycloak.quarkus.runtime.configuration.Configuration.toEnvVarFormat;
 
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.function.BiConsumer;
 import java.util.function.BiFunction;
 import java.util.function.BooleanSupplier;
+import java.util.function.Consumer;
+import java.util.stream.Stream;
 
 import io.smallrye.config.ConfigSourceInterceptorContext;
 import io.smallrye.config.ConfigValue;
@@ -297,7 +300,7 @@ public class PropertyMapper<T> {
         private BooleanSupplier isEnabled = () -> true;
         private String enabledWhen = "";
         private String paramLabel;
-        private BiConsumer<PropertyMapper<T>, ConfigValue> validator = (mapper, value) -> mapper.validateExpectedValues(value, mapper::validateSingleValue);
+        private BiConsumer<PropertyMapper<T>, ConfigValue> validator = (mapper, value) -> mapper.validateValues(value, mapper::validateExpectedValues);
         private String description;
 
         public Builder(Option<T> option) {
@@ -349,27 +352,41 @@ public class PropertyMapper<T> {
         /**
          * Set the validator, overwriting the current one.
          */
-        public Builder<T> validator(BiConsumer<PropertyMapper<T>, ConfigValue> validator) {
+        public Builder<T> validator(Consumer<String> validator) {
-            this.validator = validator;
+            this.validator = (mapper, value) -> mapper.validateValues(value,
+                    (c, v) -> validator.accept(v));
+            if (!Objects.equals(this.description, this.option.getDescription())) {
+                throw new AssertionError("Overwriting the validator will cause the description modification from addValidateEnabled to be incorrect.");
+            }
             return this;
         }
         
-        public Builder<T> appendValidator(BiConsumer<PropertyMapper<T>, ConfigValue> validator) {
+        public Builder<T> addValidator(BiConsumer<PropertyMapper<T>, ConfigValue> validator) {
             var current = this.validator;
             this.validator = (mapper, value) -> {
-                validator.accept(mapper, value);
+                Stream.of(current, validator).map(v -> {
-                current.accept(mapper, value);
+                    try {
+                        v.accept(mapper, value);
+                        return Optional.<PropertyException>empty();
+                    } catch (PropertyException e) {
+                        return Optional.of(e);
+                    }
+                }).flatMap(Optional::stream)
+                        .reduce((e1, e2) -> new PropertyException(String.format("%s.\n%s", e1.getMessage(), e2.getMessage())))
+                        .ifPresent(e -> {
+                            throw e;
+                        });
             };
             return this;
         }
         
         /**
-         * Similar to {@link #enabledWhen}, but uses the condition as a validator that is appended to the current one. This allows the option
+         * Similar to {@link #enabledWhen}, but uses the condition as a validator that is added to the current one. This allows the option
          * to appear in help. 
          * @return
          */
-        public Builder<T> appendValidateEnabled(BooleanSupplier isEnabled, String enabledWhen) {
+        public Builder<T> addValidateEnabled(BooleanSupplier isEnabled, String enabledWhen) {
-            this.appendValidator((mapper, value) -> {
+            this.addValidator((mapper, value) -> {
                 if (!isEnabled.getAsBoolean()) {
                     throw new PropertyException(mapper.getOption().getKey() + " available only when " + enabledWhen);
                 }
@@ -396,18 +413,34 @@ public class PropertyMapper<T> {
         }
     }
 
-    public void validateExpectedValues(ConfigValue configValue, BiConsumer<ConfigValue, String> singleValidator) {
+    public void validateValues(ConfigValue configValue, BiConsumer<ConfigValue, String> singleValidator) {
         String value = configValue.getValue();
 
         boolean multiValued = getOption().getType() == java.util.List.class;
+        StringBuilder result = new StringBuilder();
 
         String[] values = multiValued ? value.split(",") : new String[] { value };
         for (String v : values) {
             if (multiValued && !v.trim().equals(v)) {
-                throw new PropertyException("Invalid value for multivalued option " + getOptionAndSourceMessage(configValue)
+                if (!result.isEmpty()) {
-                        + ": list value '" + v + "' should not have leading nor trailing whitespace");
+                    result.append(".\n");
                 }
+                result.append("Invalid value for multivalued option " + getOptionAndSourceMessage(configValue)
+                        + ": list value '" + v + "' should not have leading nor trailing whitespace");
+                continue;
+            }
+            try {
                 singleValidator.accept(configValue, v);
+            } catch (PropertyException e) {
+                if (!result.isEmpty()) {
+                    result.append(".\n");
+                }
+                result.append(e.getMessage());
+            }
+        }
+        
+        if (!result.isEmpty()) {
+            throw new PropertyException(result.toString());
         }
     }
 
@@ -419,7 +452,7 @@ public class PropertyMapper<T> {
         return Optional.ofNullable(configValue.getConfigSourceName()).filter(name -> name.contains(KcEnvConfigSource.NAME)).isPresent();
     }
 
-    void validateSingleValue(ConfigValue configValue, String v) {
+    void validateExpectedValues(ConfigValue configValue, String v) {
         List<String> expectedValues = getExpectedValues();
         if (!expectedValues.isEmpty() && !expectedValues.contains(v) && getOption().isStrictExpectedValues()) {
             throw new PropertyException(

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/ProxyPropertyMappers.java

@@ -38,9 +38,8 @@ final class ProxyPropertyMappers {
                         .build(),
                 fromOption(ProxyOptions.PROXY_TRUSTED_ADDRESSES)
                         .to("quarkus.http.proxy.trusted-proxies")
-                        .validator((mapper, value) -> mapper.validateExpectedValues(value,
+                        .validator(ProxyPropertyMappers::validateAddress)
-                                (c, v) -> validateAddress(v)))
+                        .addValidateEnabled(() -> !Configuration.isBlank(ProxyOptions.PROXY_HEADERS), "proxy-headers is set")
-                        .appendValidateEnabled(() -> !Configuration.isBlank(ProxyOptions.PROXY_HEADERS), "proxy-headers is set")
                         .paramLabel("trusted proxies")
                         .build()
         };

quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/configuration/mappers/TracingPropertyMappers.java

@@ -17,7 +17,6 @@
 
 package org.keycloak.quarkus.runtime.configuration.mappers;
 
-import io.smallrye.config.ConfigValue;
 import org.keycloak.common.Profile;
 import org.keycloak.quarkus.runtime.Environment;
 import org.keycloak.quarkus.runtime.cli.PropertyException;
@@ -97,23 +96,23 @@ public class TracingPropertyMappers {
         };
     }
 
-    private static void validateEndpoint(PropertyMapper<String> mapper, ConfigValue value) {
+    private static void validateEndpoint(String value) {
-        if (value == null || StringUtil.isBlank(value.getValue())) {
+        if (StringUtil.isBlank(value)) {
             throw new PropertyException("URL specified in 'tracing-endpoint' option must not be empty.");
         }
 
-        if (!isValidUrl(value.getValue())) {
+        if (!isValidUrl(value)) {
             throw new PropertyException("URL specified in 'tracing-endpoint' option is invalid.");
         }
     }
 
-    private static void validateRatio(PropertyMapper<Double> mapper, ConfigValue value) {
+    private static void validateRatio(String value) {
-        if (value == null || StringUtil.isBlank(value.getValue())) {
+        if (StringUtil.isBlank(value)) {
             throw new PropertyException("Ratio in 'tracing-sampler-ratio' option must not be empty.");
         }
 
         try {
-            var ratio = Double.parseDouble(value.getValue());
+            var ratio = Double.parseDouble(value);
             if (ratio <= 0.0 || ratio > 1.0) {
                 throw new NumberFormatException();
             }