Skip to content

Commit

Permalink
[FLINK-35359][config] ConfigOptionsDocGenerator check the annotation …
Browse files Browse the repository at this point in the history
…of the Options class
  • Loading branch information
Sxnan authored and xintongsong committed May 23, 2024
1 parent 51da2d0 commit 63304de
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 0 deletions.
4 changes: 4 additions & 0 deletions flink-docs/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ under the License.
<version>${project.version}</version>
</dependency>

<dependency>
<groupId>org.apache.flink</groupId>
<artifactId>flink-shaded-asm-9</artifactId>
</dependency>
<dependency>
<groupId>org.apache.flink</groupId>
<artifactId>flink-shaded-netty</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@

package org.apache.flink.docs.configuration;

import org.apache.flink.annotation.Experimental;
import org.apache.flink.annotation.Public;
import org.apache.flink.annotation.PublicEvolving;
import org.apache.flink.annotation.VisibleForTesting;
import org.apache.flink.annotation.docs.ConfigGroup;
import org.apache.flink.annotation.docs.ConfigGroups;
Expand All @@ -36,6 +39,12 @@
import org.apache.flink.util.ExceptionUtils;
import org.apache.flink.util.TimeUtils;

import org.apache.flink.shaded.asm9.org.objectweb.asm.AnnotationVisitor;
import org.apache.flink.shaded.asm9.org.objectweb.asm.ClassReader;
import org.apache.flink.shaded.asm9.org.objectweb.asm.ClassVisitor;
import org.apache.flink.shaded.asm9.org.objectweb.asm.Opcodes;
import org.apache.flink.shaded.asm9.org.objectweb.asm.Type;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -103,6 +112,29 @@ public static void main(String[] args) throws Exception {
generateCommonSection(rootDir, new ConfigurationOptionLocator(), outputDirectory);
}

@VisibleForTesting
static void verifyClassAnnotation(Class<?> optionsClass) throws Exception {
final ClassReader classReader = new ClassReader(optionsClass.getName());
List<String> annotationDescriptors = new ArrayList<>();
classReader.accept(
new ClassVisitor(Opcodes.ASM9) {
@Override
public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) {
annotationDescriptors.add(descriptor);
return super.visitAnnotation(descriptor, visible);
}
},
0);
if (!annotationDescriptors.contains(Type.getDescriptor(Public.class))
&& !annotationDescriptors.contains(Type.getDescriptor(PublicEvolving.class))
&& !annotationDescriptors.contains(Type.getDescriptor(Experimental.class))) {
throw new RuntimeException(
String.format(
"The ConfigOption class %s must be annotated with Public, PublicEvolving, or Experimental.",
optionsClass.getName()));
}
}

@VisibleForTesting
static void generateCommonSection(
String rootDir, ConfigurationOptionLocator locator, String outputDirectory)
Expand Down Expand Up @@ -197,6 +229,9 @@ private static void createTables(String rootDir, String outputDirectory) throws
(optionsClass, optionWithMetaInfos) -> {
List<Tuple2<ConfigGroup, String>> tables =
generateTablesForClass(optionsClass, optionWithMetaInfos);
if (!tables.isEmpty()) {
verifyClassAnnotation(optionsClass);
}
for (Tuple2<ConfigGroup, String> group : tables) {
String name;
if (group.f0 == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@

package org.apache.flink.docs.configuration;

import org.apache.flink.annotation.Experimental;
import org.apache.flink.annotation.Internal;
import org.apache.flink.annotation.Public;
import org.apache.flink.annotation.PublicEvolving;
import org.apache.flink.annotation.docs.ConfigGroup;
import org.apache.flink.annotation.docs.ConfigGroups;
import org.apache.flink.annotation.docs.Documentation;
Expand Down Expand Up @@ -46,6 +50,8 @@

import static org.apache.flink.configuration.description.TextElement.text;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatException;
import static org.assertj.core.api.Assertions.assertThatNoException;

/** Tests for the {@link ConfigOptionsDocGenerator}. */
@SuppressWarnings("unused")
Expand Down Expand Up @@ -726,6 +732,47 @@ void testSnakeCaseConversion() {
assertThat(ConfigOptionsDocGenerator.toSnakeCase("DBOptions")).isEqualTo("db_options");
}

@Public
static class PublicOptions {}

@PublicEvolving
static class PublicEvolvingOptions {}

@Experimental
static class ExperimentalOptions {}

@Internal
static class InternalOptions {}

static class OptionsWithoutAnnotation {}

@Test
void testVerifyClassAnnotation() {
assertThatNoException()
.isThrownBy(
() -> ConfigOptionsDocGenerator.verifyClassAnnotation(PublicOptions.class));
assertThatNoException()
.isThrownBy(
() ->
ConfigOptionsDocGenerator.verifyClassAnnotation(
PublicEvolvingOptions.class));
assertThatNoException()
.isThrownBy(
() ->
ConfigOptionsDocGenerator.verifyClassAnnotation(
ExperimentalOptions.class));
assertThatException()
.isThrownBy(
() ->
ConfigOptionsDocGenerator.verifyClassAnnotation(
OptionsWithoutAnnotation.class));
assertThatException()
.isThrownBy(
() ->
ConfigOptionsDocGenerator.verifyClassAnnotation(
InternalOptions.class));
}

static String getProjectRootDir() {
final String dirFromProperty = System.getProperty("rootDir");
if (dirFromProperty != null) {
Expand Down

0 comments on commit 63304de

Please sign in to comment.