diff --git a/flink-docs/src/main/java/org/apache/flink/docs/configuration/ConfigOptionsDocGenerator.java b/flink-docs/src/main/java/org/apache/flink/docs/configuration/ConfigOptionsDocGenerator.java index 68795826a5bd6..3ceef55e4fc83 100644 --- a/flink-docs/src/main/java/org/apache/flink/docs/configuration/ConfigOptionsDocGenerator.java +++ b/flink-docs/src/main/java/org/apache/flink/docs/configuration/ConfigOptionsDocGenerator.java @@ -26,15 +26,18 @@ import org.apache.flink.configuration.ConfigOption; import org.apache.flink.configuration.description.Formatter; import org.apache.flink.configuration.description.HtmlFormatter; +import org.apache.flink.util.TimeUtils; import org.apache.flink.util.function.ThrowingConsumer; import java.io.IOException; import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.nio.charset.StandardCharsets; import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -47,6 +50,7 @@ import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import static org.apache.flink.docs.util.Utils.escapeCharacters; @@ -158,6 +162,7 @@ private static void createTable(String rootDir, String module, String packageNam }); } + @VisibleForTesting static void processConfigOptions(String rootDir, String module, String packageName, String pathPrefix, ThrowingConsumer, IOException> classConsumer) throws IOException, ClassNotFoundException { Path configDir = Paths.get(rootDir, module, pathPrefix, packageName.replaceAll("\\.", "/")); @@ -202,6 +207,7 @@ static List> generateTablesForClass(Class options return tables; } + @VisibleForTesting static List extractConfigOptions(Class clazz) { try { List configOptions = new ArrayList<>(8); @@ -241,7 +247,8 @@ private static String toHtmlTable(final List options) { htmlTable.append(" \n"); htmlTable.append(" Key\n"); htmlTable.append(" Default\n"); - htmlTable.append(" Description\n"); + htmlTable.append(" Type\n"); + htmlTable.append(" Description\n"); htmlTable.append(" \n"); htmlTable.append(" \n"); htmlTable.append(" \n"); @@ -265,6 +272,7 @@ private static String toHtmlTable(final List options) { private static String toHtmlString(final OptionWithMetaInfo optionWithMetaInfo) { ConfigOption option = optionWithMetaInfo.option; String defaultValue = stringifyDefault(optionWithMetaInfo); + String type = typeToHtml(optionWithMetaInfo); Documentation.TableOption tableOption = optionWithMetaInfo.field.getAnnotation(Documentation.TableOption.class); StringBuilder execModeStringBuilder = new StringBuilder(); if (tableOption != null) { @@ -286,10 +294,76 @@ private static String toHtmlString(final OptionWithMetaInfo optionWithMetaInfo) " \n" + "
" + escapeCharacters(option.key()) + "
" + execModeStringBuilder.toString() + "\n" + " " + escapeCharacters(addWordBreakOpportunities(defaultValue)) + "\n" + + " " + type + "\n" + " " + formatter.format(option.description()) + "\n" + " \n"; } + private static Class getClazz(ConfigOption option) { + try { + Method getClazzMethod = ConfigOption.class.getDeclaredMethod("getClazz"); + getClazzMethod.setAccessible(true); + Class clazz = (Class) getClazzMethod.invoke(option); + getClazzMethod.setAccessible(false); + return clazz; + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + private static boolean isList(ConfigOption option) { + try { + Method getClazzMethod = ConfigOption.class.getDeclaredMethod("isList"); + getClazzMethod.setAccessible(true); + boolean isList = (boolean) getClazzMethod.invoke(option); + getClazzMethod.setAccessible(false); + return isList; + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + @VisibleForTesting + static String typeToHtml(OptionWithMetaInfo optionWithMetaInfo) { + ConfigOption option = optionWithMetaInfo.option; + Class clazz = getClazz(option); + boolean isList = isList(option); + + if (clazz.isEnum()) { + return enumTypeToHtml(clazz, isList); + } + + return atomicTypeToHtml(clazz, isList); + } + + private static String atomicTypeToHtml(Class clazz, boolean isList) { + String typeName = clazz.getSimpleName(); + + final String type; + if (isList) { + type = String.format("List<%s>", typeName); + } else { + type = typeName; + } + + return escapeCharacters(type); + } + + private static String enumTypeToHtml(Class enumClazz, boolean isList) { + final String type; + if (isList) { + type = "List"; + } else { + type = "Enum"; + } + + return String.format( + "

%s

Possible values: %s", + escapeCharacters(type), + escapeCharacters(Arrays.toString(enumClazz.getEnumConstants()))); + } + + @VisibleForTesting static String stringifyDefault(OptionWithMetaInfo optionWithMetaInfo) { ConfigOption option = optionWithMetaInfo.option; Documentation.OverrideDefault overrideDocumentedDefault = optionWithMetaInfo.field.getAnnotation(Documentation.OverrideDefault.class); @@ -297,14 +371,31 @@ static String stringifyDefault(OptionWithMetaInfo optionWithMetaInfo) { return overrideDocumentedDefault.value(); } else { Object value = option.defaultValue(); - if (value instanceof String) { - if (((String) value).isEmpty()) { - return "(none)"; - } - return "\"" + value + "\""; + return stringifyObject(value); + } + } + + @SuppressWarnings("unchecked") + private static String stringifyObject(Object value) { + if (value instanceof String) { + if (((String) value).isEmpty()) { + return "(none)"; } - return value == null ? "(none)" : value.toString(); + return "\"" + value + "\""; + } else if (value instanceof Duration) { + return TimeUtils.getStringInMillis((Duration) value); + } else if (value instanceof List) { + return ((List) value).stream() + .map(ConfigOptionsDocGenerator::stringifyObject) + .collect(Collectors.joining(";")); + } else if (value instanceof Map) { + return ((Map) value) + .entrySet() + .stream() + .map(e -> String.format("%s:%s", e.getKey(), e.getValue())) + .collect(Collectors.joining(",")); } + return value == null ? "(none)" : value.toString(); } private static String addWordBreakOpportunities(String value) { diff --git a/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocGeneratorTest.java b/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocGeneratorTest.java index 438c10848b2b5..46eb0eed2ad88 100644 --- a/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocGeneratorTest.java +++ b/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocGeneratorTest.java @@ -24,6 +24,7 @@ import org.apache.flink.api.java.tuple.Tuple2; import org.apache.flink.configuration.ConfigOption; import org.apache.flink.configuration.ConfigOptions; +import org.apache.flink.configuration.MemorySize; import org.apache.flink.configuration.description.Formatter; import org.apache.flink.configuration.description.HtmlFormatter; import org.apache.flink.docs.configuration.data.TestCommonOptions; @@ -36,11 +37,14 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Paths; +import java.time.Duration; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; @@ -64,6 +68,108 @@ static class TestConfigGroup { .withDescription("This is long example description for the second option."); } + private enum TestEnum { + VALUE_1, + VALUE_2, + VALUE_3; + } + + static class TypeTestConfigGroup { + public static ConfigOption enumOption = ConfigOptions + .key("option.enum") + .enumType(TestEnum.class) + .defaultValue(TestEnum.VALUE_1) + .withDescription("Description"); + + public static ConfigOption> enumListOption = ConfigOptions + .key("option.enum.list") + .enumType(TestEnum.class) + .asList() + .defaultValues(TestEnum.VALUE_1, TestEnum.VALUE_2) + .withDescription("Description"); + + public static ConfigOption memoryOption = ConfigOptions + .key("option.memory") + .memoryType() + .defaultValue(new MemorySize(1024)) + .withDescription("Description"); + + public static ConfigOption> mapOption = ConfigOptions + .key("option.map") + .mapType() + .defaultValue(Collections.singletonMap("key1", "value1")) + .withDescription("Description"); + + public static ConfigOption>> mapListOption = ConfigOptions + .key("option.map.list") + .mapType() + .asList() + .defaultValues(Collections.singletonMap("key1", "value1"), Collections.singletonMap("key2", "value2")) + .withDescription("Description"); + + public static ConfigOption durationOption = ConfigOptions + .key("option.duration") + .durationType() + .defaultValue(Duration.ofMinutes(1)) + .withDescription("Description"); + } + + @Test + public void testCreatingTypes() { + final String expectedTable = + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + "
KeyDefaultTypeDescription
option.duration
60000msDurationDescription
option.enum
VALUE_1

Enum

Possible values: [VALUE_1, VALUE_2, VALUE_3]		Description
option.enum.list
VALUE_1;VALUE_2

List<Enum>

Possible values: [VALUE_1, VALUE_2, VALUE_3]		Description
option.map
key1:value1MapDescription
option.map.list
key1:value1;key2:value2List<Map>Description
option.memory
1024 bytesMemorySizeDescription
\n"; + final String htmlTable = ConfigOptionsDocGenerator.generateTablesForClass(TypeTestConfigGroup.class).get(0).f1; + + assertEquals(expectedTable, htmlTable); + } + @Test public void testCreatingDescription() { final String expectedTable = @@ -72,18 +178,21 @@ public void testCreatingDescription() { " \n" + " Key\n" + " Default\n" + - " Description\n" + + " Type\n" + + " Description\n" + " \n" + " \n" + " \n" + " \n" + "
first.option.a
\n" + " 2\n" + + " Integer\n" + " This is example description for the first option.\n" + " \n" + " \n" + "
second.option.a
\n" + " (none)\n" + + " String\n" + " This is long example description for the second option.\n" + " \n" + " \n" + @@ -123,6 +232,12 @@ static class TestConfigPrefix { public static ConfigOption option5 = ConfigOptions .key("a.c.b.option") .noDefaultValue(); + + // should end up in group2, full key-prefix match for group 2 + // checks that the longest matching group is assigned + public static ConfigOption option6 = ConfigOptions + .key("a.b.c.d.option") + .defaultValue(2); } @Test @@ -139,6 +254,8 @@ public void testLongestPrefixMatching() { assertThat(tablesConverted.get("group1"), containsString("a.b.option")); assertThat(tablesConverted.get("group1"), containsString("a.b.c.option")); assertThat(tablesConverted.get("group1"), containsString("a.b.c.e.option")); + assertThat(tablesConverted.get("group2"), containsString("a.b.c.d.option")); + assertThat(tablesConverted.get("group1"), not(containsString("a.b.c.d.option"))); assertThat(tablesConverted.get("default"), containsString("a.option")); assertThat(tablesConverted.get("default"), containsString("a.c.b.option")); } @@ -185,13 +302,15 @@ public void testCreatingMultipleGroups() { " \n" + " Key\n" + " Default\n" + - " Description\n" + + " Type\n" + + " Description\n" + " \n" + " \n" + " \n" + " \n" + "
first.option.a
\n" + " 2\n" + + " Integer\n" + " This is example description for the first option.\n" + " \n" + " \n" + @@ -202,13 +321,15 @@ public void testCreatingMultipleGroups() { " \n" + " Key\n" + " Default\n" + - " Description\n" + + " Type\n" + + " Description\n" + " \n" + " \n" + " \n" + " \n" + "
second.option.a
\n" + " (none)\n" + + " String\n" + " This is long example description for the second option.\n" + " \n" + " \n" + @@ -219,18 +340,21 @@ public void testCreatingMultipleGroups() { " \n" + " Key\n" + " Default\n" + - " Description\n" + + " Type\n" + + " Description\n" + " \n" + " \n" + " \n" + " \n" + "
fourth.option.a
\n" + " (none)\n" + + " String\n" + " This is long example description for the fourth option.\n" + " \n" + " \n" + "
third.option.a
\n" + " 2\n" + + " Integer\n" + " This is example description for the third option.\n" + " \n" + " \n" + @@ -259,18 +383,21 @@ public void testOverrideDefault() { " \n" + " Key\n" + " Default\n" + - " Description\n" + + " Type\n" + + " Description\n" + " \n" + " \n" + " \n" + " \n" + "
first.option.a
\n" + " default_1\n" + + " Integer\n" + " This is example description for the first option.\n" + " \n" + " \n" + "
second.option.a
\n" + " default_2\n" + + " String\n" + " This is long example description for the second option.\n" + " \n" + " \n" + @@ -298,18 +425,21 @@ public void testCommonOptions() throws IOException, ClassNotFoundException { " \n" + " Key\n" + " Default\n" + - " Description\n" + + " Type\n" + + " Description\n" + " \n" + " \n" + " \n" + " \n" + "
" + TestCommonOptions.COMMON_POSITIONED_OPTION.key() + "
\n" + " " + TestCommonOptions.COMMON_POSITIONED_OPTION.defaultValue() + "\n" + + " Integer\n" + " " + formatter.format(TestCommonOptions.COMMON_POSITIONED_OPTION.description()) + "\n" + " \n" + " \n" + "
" + TestCommonOptions.COMMON_OPTION.key() + "
\n" + " " + TestCommonOptions.COMMON_OPTION.defaultValue() + "\n" + + " Integer\n" + " " + formatter.format(TestCommonOptions.COMMON_OPTION.description()) + "\n" + " \n" + " \n" + @@ -345,13 +475,15 @@ public void testConfigOptionExclusion() { " \n" + " Key\n" + " Default\n" + - " Description\n" + + " Type\n" + + " Description\n" + " \n" + " \n" + " \n" + " \n" + "
first.option.a
\n" + " 2\n" + + " Integer\n" + " This is example description for the first option.\n" + " \n" + " \n" + diff --git a/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocsCompletenessITCase.java b/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocsCompletenessITCase.java index 9ae2595357558..44890ef56fdad 100644 --- a/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocsCompletenessITCase.java +++ b/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocsCompletenessITCase.java @@ -38,6 +38,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -48,6 +49,7 @@ import static org.apache.flink.docs.configuration.ConfigOptionsDocGenerator.extractConfigOptions; import static org.apache.flink.docs.configuration.ConfigOptionsDocGenerator.processConfigOptions; import static org.apache.flink.docs.configuration.ConfigOptionsDocGenerator.stringifyDefault; +import static org.apache.flink.docs.configuration.ConfigOptionsDocGenerator.typeToHtml; /** * This test verifies that all {@link ConfigOption ConfigOptions} in the configured @@ -180,12 +182,18 @@ private static Collection parseDocumentedOptionsFromFile(Path // Use split to exclude document key tag. String key = tableRow.child(0).text().split(" ")[0]; String defaultValue = tableRow.child(1).text(); - String description = tableRow.child(2) + String typeValue = tableRow.child(2).text(); + String description = tableRow.child(3) .childNodes() .stream() .map(Object::toString) .collect(Collectors.joining()); - return new DocumentedOption(key, defaultValue, description, file.getName(file.getNameCount() - 1)); + return new DocumentedOption( + key, + defaultValue, + typeValue, + description, + file.getName(file.getNameCount() - 1)); }) .collect(Collectors.toList()); } @@ -200,8 +208,11 @@ private static Map findExistingOptions(Predicate containingClass; - private ExistingOption(String key, String defaultValue, String description, Class containingClass) { - super(key, defaultValue, description); + private ExistingOption( + String key, + String defaultValue, + String typeValue, + String description, + Class containingClass) { + super(key, defaultValue, typeValue, description); this.containingClass = containingClass; } } @@ -233,8 +249,13 @@ private static final class DocumentedOption extends Option { private final Path containingFile; - private DocumentedOption(String key, String defaultValue, String description, Path containingFile) { - super(key, defaultValue, description); + private DocumentedOption( + String key, + String defaultValue, + String typeValue, + String description, + Path containingFile) { + super(key, defaultValue, typeValue, description); this.containingFile = containingFile; } } @@ -242,35 +263,44 @@ private DocumentedOption(String key, String defaultValue, String description, Pa private abstract static class Option { protected final String key; protected final String defaultValue; + protected final String typeValue; protected final String description; - private Option(String key, String defaultValue, String description) { + private Option(String key, String defaultValue, String typeValue, String description) { this.key = key; this.defaultValue = defaultValue; + this.typeValue = typeValue; this.description = description; } @Override - public int hashCode() { - return key.hashCode() + defaultValue.hashCode() + description.hashCode(); - } - - @Override - public boolean equals(Object obj) { - if (!(obj instanceof Option)) { + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { return false; } + Option option = (Option) o; + return Objects.equals(key, option.key) && + Objects.equals(defaultValue, option.defaultValue) && + Objects.equals(typeValue, option.typeValue) && + Objects.equals(description, option.description); + } - Option other = (Option) obj; - - return this.key.equals(other.key) - && this.defaultValue.equals(other.defaultValue) - && this.description.equals(other.description); + @Override + public int hashCode() { + return Objects.hash(key, defaultValue, typeValue, description); } @Override public String toString() { - return "Option(key=" + key + ", default=" + defaultValue + ", description=" + description + ')'; + return "Option{" + + "key='" + key + '\'' + + ", defaultValue='" + defaultValue + '\'' + + ", typeValue='" + typeValue + '\'' + + ", description='" + description + '\'' + + '}'; } } }