From f0162db56f4cf7219361467c0de1a325dac9d094 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Barto=C5=A1?= Date: Tue, 13 Aug 2024 09:35:40 +0100 Subject: [PATCH] Cache guide does not properly print cache-stack values (#31943) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Cache guide does not properly print cache-stack values Ability to choose expected values strict Fixes #31941 Signed-off-by: Martin Bartoš * Add Javadoc Signed-off-by: Martin Bartoš * Reflect non-strict values in docs Signed-off-by: Martin Bartoš * Use 'or any' in docs for non-strict expected values Signed-off-by: Martin Bartoš * Edit approved files for HelpCommandDistTest Signed-off-by: Martin Bartoš --------- Signed-off-by: Martin Bartoš --- docs/guides/templates/options.adoc | 11 +-- .../org/keycloak/guides/maven/Options.java | 11 +++ .../org/keycloak/config/CachingOptions.java | 5 +- .../main/java/org/keycloak/config/Option.java | 19 +++++- .../org/keycloak/config/OptionBuilder.java | 68 +++++++++++++++---- .../keycloak/quarkus/runtime/cli/Picocli.java | 6 +- .../configuration/mappers/PropertyMapper.java | 12 +++- ...mandDistTest.testStartDevHelp.approved.txt | 3 +- ...dDistTest.testStartDevHelpAll.approved.txt | 3 +- ...CommandDistTest.testStartHelp.approved.txt | 3 +- ...mandDistTest.testStartHelpAll.approved.txt | 3 +- ...stTest.testStartOptimizedHelp.approved.txt | 3 +- ...est.testStartOptimizedHelpAll.approved.txt | 3 +- 13 files changed, 119 insertions(+), 31 deletions(-) diff --git a/docs/guides/templates/options.adoc b/docs/guides/templates/options.adoc index 75f8db69c0..43d5bf1dad 100644 --- a/docs/guides/templates/options.adoc +++ b/docs/guides/templates/options.adoc @@ -1,7 +1,8 @@ <#macro expectedValues option> -<#list ctx.options.getOption(option).expectedValues as expectedValue> -* ${expectedValue} - + <#assign optionObj = ctx.options.getOption(option) /> + <#list optionObj.expectedValues as expectedValue> + * ${expectedValue} <#if optionObj.defaultValue?has_content && expectedValue == optionObj.defaultValue> (default) + <#macro list options buildIcon=true anchor=true> @@ -39,7 +40,7 @@ ${option.deprecated.note!}<#if option.deprecated.newOptionsKeys?has_content><#if |<#if option.expectedValues?has_content> -<#list option.expectedValues as value>`+${value!}+`<#if option.defaultValue?has_content && value = option.defaultValue> (default)<#if value?has_next>, +<#list option.expectedValues as value>`+${value!}+`<#if option.defaultValue?has_content && value = option.defaultValue> (default)<#if value?has_next>, <#if !option.strictExpectedValues>, or any <#else> <#if option.defaultValue?has_content>[.options-default]#`+${option.defaultValue!}+`# (default)<#if option.type?has_content && option.defaultValue?has_content> or <#if option.type?has_content && !option.expectedValues?has_content>any `+${option.type!}+` @@ -47,4 +48,4 @@ ${option.deprecated.note!}<#if option.deprecated.newOptionsKeys?has_content><#if |=== - \ No newline at end of file + diff --git a/docs/maven-plugin/src/main/java/org/keycloak/guides/maven/Options.java b/docs/maven-plugin/src/main/java/org/keycloak/guides/maven/Options.java index 63c88d58d6..4ac6e5acf0 100644 --- a/docs/maven-plugin/src/main/java/org/keycloak/guides/maven/Options.java +++ b/docs/maven-plugin/src/main/java/org/keycloak/guides/maven/Options.java @@ -50,6 +50,7 @@ public class Options { m.getDescription(), m.getDefaultValue().map(Object::toString).orElse(null), m.getExpectedValues(), + m.isStrictExpectedValues(), m.getEnabledWhen().orElse(""), m.getDeprecatedMetadata().orElse(null))) .forEach(o -> options.computeIfAbsent(o.category, k -> new TreeSet<>(Comparator.comparing(Option::getKey))).add(o)); @@ -76,6 +77,7 @@ public class Options { m.getHelpText(), m.getDefaultValue() == null ? null : m.getDefaultValue().toString(), m.getOptions() == null ? Collections.emptyList() : m.getOptions(), + true, "", null)) .sorted(Comparator.comparing(Option::getKey)).collect(Collectors.toList()); @@ -169,6 +171,9 @@ public class Options { private String description; private final String defaultValue; private List expectedValues; + + private final boolean strictExpectedValues; + private final String enabledWhen; private final DeprecatedMetadata deprecated; @@ -179,6 +184,7 @@ public class Options { String description, String defaultValue, Iterable expectedValues, + boolean strictExpectedValues, String enabledWhen, DeprecatedMetadata deprecatedMetadata) { this.key = key; @@ -188,6 +194,7 @@ public class Options { this.description = description; this.defaultValue = defaultValue; this.expectedValues = StreamSupport.stream(expectedValues.spliterator(), false).collect(Collectors.toList()); + this.strictExpectedValues = strictExpectedValues; this.enabledWhen = enabledWhen; this.deprecated = deprecatedMetadata; } @@ -239,6 +246,10 @@ public class Options { return expectedValues; } + public boolean isStrictExpectedValues() { + return strictExpectedValues; + } + public String getEnabledWhen() { if (StringUtil.isBlank(enabledWhen)) return null; return enabledWhen; diff --git a/quarkus/config-api/src/main/java/org/keycloak/config/CachingOptions.java b/quarkus/config-api/src/main/java/org/keycloak/config/CachingOptions.java index 4bf09e6a30..e84d8974f8 100644 --- a/quarkus/config-api/src/main/java/org/keycloak/config/CachingOptions.java +++ b/quarkus/config-api/src/main/java/org/keycloak/config/CachingOptions.java @@ -50,9 +50,8 @@ public class CachingOptions { public static final Option CACHE_STACK = new OptionBuilder<>("cache-stack", Stack.class) .category(OptionCategory.CACHE) - .expectedValues(List.of()) - .description("Define the default stack to use for cluster communication and node discovery. This option only takes effect " - + "if 'cache' is set to 'ispn'. Default: udp. Built-in values include: " + Stream.of(Stack.values()).map(Stack::name).collect(Collectors.joining(", "))) + .expectedValues(false) + .description("Define the default stack to use for cluster communication and node discovery. This option only takes effect if 'cache' is set to 'ispn'. Default: udp.") .build(); public static final Option CACHE_CONFIG_FILE = new OptionBuilder<>(CACHE_CONFIG_FILE_PROPERTY, File.class) diff --git a/quarkus/config-api/src/main/java/org/keycloak/config/Option.java b/quarkus/config-api/src/main/java/org/keycloak/config/Option.java index cc15b9b702..c38bd7ab55 100644 --- a/quarkus/config-api/src/main/java/org/keycloak/config/Option.java +++ b/quarkus/config-api/src/main/java/org/keycloak/config/Option.java @@ -14,9 +14,10 @@ public class Option { private final String description; private final Optional defaultValue; private final List expectedValues; + private final boolean strictExpectedValues; private final DeprecatedMetadata deprecatedMetadata; - public Option(Class type, String key, OptionCategory category, boolean hidden, boolean buildTime, String description, Optional defaultValue, List expectedValues, DeprecatedMetadata deprecatedMetadata) { + public Option(Class type, String key, OptionCategory category, boolean hidden, boolean buildTime, String description, Optional defaultValue, List expectedValues, boolean strictExpectedValues, DeprecatedMetadata deprecatedMetadata) { this.type = type; this.key = key; this.category = category; @@ -25,6 +26,7 @@ public class Option { this.description = getDescriptionByCategorySupportLevel(description, category); this.defaultValue = defaultValue; this.expectedValues = expectedValues; + this.strictExpectedValues = strictExpectedValues; this.deprecatedMetadata = deprecatedMetadata; } @@ -52,10 +54,24 @@ public class Option { return defaultValue; } + /** + * If {@link #isStrictExpectedValues()} is false, custom values can be provided + * Otherwise, only specified expected values can be used + * + * @return expected values + */ public List getExpectedValues() { return expectedValues; } + /** + * Denotes whether a custom value can be provided among the expected values + * If strict, application fails when some custom value is provided + */ + public boolean isStrictExpectedValues() { + return strictExpectedValues; + } + public Optional getDeprecatedMetadata() { return Optional.ofNullable(deprecatedMetadata); } @@ -70,6 +86,7 @@ public class Option { this.description, Optional.ofNullable(defaultValue), this.expectedValues, + this.strictExpectedValues, this.deprecatedMetadata ); } diff --git a/quarkus/config-api/src/main/java/org/keycloak/config/OptionBuilder.java b/quarkus/config-api/src/main/java/org/keycloak/config/OptionBuilder.java index 2282d88fc0..8cdbaad632 100644 --- a/quarkus/config-api/src/main/java/org/keycloak/config/OptionBuilder.java +++ b/quarkus/config-api/src/main/java/org/keycloak/config/OptionBuilder.java @@ -1,17 +1,21 @@ package org.keycloak.config; +import org.keycloak.common.util.CollectionUtil; + import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; public class OptionBuilder { private static final List BOOLEAN_TYPE_VALUES = List.of(Boolean.TRUE.toString(), Boolean.FALSE.toString()); private final Class type; + private final Class auxiliaryType; private final String key; private OptionCategory category; private boolean hidden; @@ -19,6 +23,8 @@ public class OptionBuilder { private String description; private Optional defaultValue; private List expectedValues = List.of(); + // Denotes whether a custom value can be provided among the expected values + private boolean strictExpectedValues; private DeprecatedMetadata deprecatedMetadata; public static OptionBuilder> listOptionBuilder(String key, Class type) { @@ -31,6 +37,7 @@ public class OptionBuilder { private OptionBuilder(String key, Class type, Class auxiliaryType) { this.type = type; + this.auxiliaryType = auxiliaryType; if (type.isArray() || ((Collection.class.isAssignableFrom(type) || Map.class.isAssignableFrom(type)) && type != java.util.List.class)) { throw new IllegalArgumentException("Non-List multi-valued options are not yet supported"); } @@ -39,17 +46,8 @@ public class OptionBuilder { hidden = false; build = false; description = null; - Class expected = type; - if (auxiliaryType != null) { - expected = auxiliaryType; - } - defaultValue = Boolean.class.equals(expected) ? Optional.of((T) Boolean.FALSE) : Optional.empty(); - if (Boolean.class.equals(expected)) { - expectedValues(BOOLEAN_TYPE_VALUES); - } - if (Enum.class.isAssignableFrom(expected)) { - expectedValues((Class) expected); - } + defaultValue = Optional.empty(); + strictExpectedValues = true; } public OptionBuilder category(OptionCategory category) { @@ -83,17 +81,40 @@ public class OptionBuilder { } public OptionBuilder expectedValues(List expected) { + return expectedValues(true, expected); + } + + /** + * @param strict if only expected values are allowed, or some other custom value can be specified + * @param expected expected values + */ + public OptionBuilder expectedValues(boolean strict, List expected) { + this.strictExpectedValues = strict; this.expectedValues = expected; return this; } public OptionBuilder expectedValues(Class expected) { - this.expectedValues = List.of(expected.getEnumConstants()).stream().map(Object::toString).collect(Collectors.toList()); + return expectedValues(true, expected); + } + + public OptionBuilder expectedValues(boolean strict, Class expected) { + this.strictExpectedValues = strict; + this.expectedValues = Stream.of(expected.getEnumConstants()).map(Object::toString).collect(Collectors.toList()); return this; } public OptionBuilder expectedValues(T ... expected) { - this.expectedValues = List.of(expected).stream().map(v -> v.toString()).collect(Collectors.toList()); + return expectedValues(true, expected); + } + + /** + * @param strict if only expected values are allowed, or some other custom value can be specified + * @param expected expected values - if empty and the {@link #type} or {@link #auxiliaryType} is enum, values are inferred + */ + public OptionBuilder expectedValues(boolean strict, T... expected) { + this.strictExpectedValues = strict; + this.expectedValues = Stream.of(expected).map(Object::toString).collect(Collectors.toList()); return this; } @@ -128,7 +149,26 @@ public class OptionBuilder { deprecated(); } - return new Option(type, key, category, hidden, build, description, defaultValue, expectedValues, deprecatedMetadata); + Class expected = type; + if (auxiliaryType != null) { + expected = auxiliaryType; + } + + if (CollectionUtil.isEmpty(expectedValues)) { + if (Boolean.class.equals(expected)) { + expectedValues(strictExpectedValues, BOOLEAN_TYPE_VALUES); + } + + if (Enum.class.isAssignableFrom(expected)) { + expectedValues(strictExpectedValues, (Class) expected); + } + } + + if (defaultValue.isEmpty() && Boolean.class.equals(expected)) { + defaultValue = Optional.of((T) Boolean.FALSE); + } + + return new Option(type, key, category, hidden, build, description, defaultValue, expectedValues, strictExpectedValues, deprecatedMetadata); } } diff --git a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/Picocli.java b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/Picocli.java index 26c82f58d4..53b87db7b4 100644 --- a/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/Picocli.java +++ b/quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/cli/Picocli.java @@ -831,7 +831,11 @@ public final class Picocli { } return value; }).toList(); - transformedDesc.append(" Possible values are: " + String.join(", ", decoratedExpectedValues) + "."); + + var isStrictExpectedValues = mapper.getOption().isStrictExpectedValues(); + var printableValues = String.join(", ", decoratedExpectedValues) + (!isStrictExpectedValues ? ", or a custom one" : ""); + + transformedDesc.append(String.format(" Possible values are: %s.", printableValues)); } mapper.getDefaultValue() 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 9e16235176..281e07ee8e 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 @@ -193,10 +193,20 @@ public class PropertyMapper { return this.option.getDescription(); } + /** + * If {@link #isStrictExpectedValues()} is false, custom values can be provided + * Otherwise, only specified expected values can be used. + * + * @return expected values + */ public List getExpectedValues() { return this.option.getExpectedValues(); } + public boolean isStrictExpectedValues() { + return this.option.isStrictExpectedValues(); + } + public Optional getDefaultValue() { return this.option.getDefaultValue(); } public OptionCategory getCategory() { @@ -378,7 +388,7 @@ public class PropertyMapper { void validateSingleValue(ConfigValue configValue, String v) { List expectedValues = getExpectedValues(); - if (!expectedValues.isEmpty() && !expectedValues.contains(v)) { + if (!expectedValues.isEmpty() && !expectedValues.contains(v) && getOption().isStrictExpectedValues()) { throw new PropertyException( String.format("Invalid value for option %s: %s.%s", getOptionAndSourceMessage(configValue), v, PropertyMapperParameterConsumer.getExpectedValuesMessage(expectedValues, expectedValues))); diff --git a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartDevHelp.approved.txt b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartDevHelp.approved.txt index 9647dfdaee..f22a75ecaf 100644 --- a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartDevHelp.approved.txt +++ b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartDevHelp.approved.txt @@ -48,7 +48,8 @@ Cache: --cache-stack Define the default stack to use for cluster communication and node discovery. This option only takes effect if 'cache' is set to 'ispn'. Default: udp. - Built-in values include: tcp, udp, kubernetes, ec2, azure, google + Possible values are: tcp, udp, kubernetes, ec2, azure, google, or a custom + one. Config: diff --git a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartDevHelpAll.approved.txt b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartDevHelpAll.approved.txt index cd90f93407..5f4fe9b184 100644 --- a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartDevHelpAll.approved.txt +++ b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartDevHelpAll.approved.txt @@ -74,7 +74,8 @@ Cache: --cache-stack Define the default stack to use for cluster communication and node discovery. This option only takes effect if 'cache' is set to 'ispn'. Default: udp. - Built-in values include: tcp, udp, kubernetes, ec2, azure, google + Possible values are: tcp, udp, kubernetes, ec2, azure, google, or a custom + one. Config: diff --git a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartHelp.approved.txt b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartHelp.approved.txt index 3eb2fd5cb4..057aab2d75 100644 --- a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartHelp.approved.txt +++ b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartHelp.approved.txt @@ -49,7 +49,8 @@ Cache: --cache-stack Define the default stack to use for cluster communication and node discovery. This option only takes effect if 'cache' is set to 'ispn'. Default: udp. - Built-in values include: tcp, udp, kubernetes, ec2, azure, google + Possible values are: tcp, udp, kubernetes, ec2, azure, google, or a custom + one. Config: diff --git a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartHelpAll.approved.txt b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartHelpAll.approved.txt index 0dc1a6d161..7e22edbd08 100644 --- a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartHelpAll.approved.txt +++ b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartHelpAll.approved.txt @@ -75,7 +75,8 @@ Cache: --cache-stack Define the default stack to use for cluster communication and node discovery. This option only takes effect if 'cache' is set to 'ispn'. Default: udp. - Built-in values include: tcp, udp, kubernetes, ec2, azure, google + Possible values are: tcp, udp, kubernetes, ec2, azure, google, or a custom + one. Config: diff --git a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartOptimizedHelp.approved.txt b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartOptimizedHelp.approved.txt index eb5b9fd43a..16d3d9d476 100644 --- a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartOptimizedHelp.approved.txt +++ b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartOptimizedHelp.approved.txt @@ -49,7 +49,8 @@ Cache: --cache-stack Define the default stack to use for cluster communication and node discovery. This option only takes effect if 'cache' is set to 'ispn'. Default: udp. - Built-in values include: tcp, udp, kubernetes, ec2, azure, google + Possible values are: tcp, udp, kubernetes, ec2, azure, google, or a custom + one. Config: diff --git a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartOptimizedHelpAll.approved.txt b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartOptimizedHelpAll.approved.txt index 97533b5a7b..600b4ba02e 100644 --- a/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartOptimizedHelpAll.approved.txt +++ b/quarkus/tests/integration/src/test/resources/org/keycloak/it/cli/dist/approvals/cli/help/HelpCommandDistTest.testStartOptimizedHelpAll.approved.txt @@ -75,7 +75,8 @@ Cache: --cache-stack Define the default stack to use for cluster communication and node discovery. This option only takes effect if 'cache' is set to 'ispn'. Default: udp. - Built-in values include: tcp, udp, kubernetes, ec2, azure, google + Possible values are: tcp, udp, kubernetes, ec2, azure, google, or a custom + one. Config: