[hotfix][table-planner] Remove org.reflections usage and dependency

liuyongvs · Feb 2, 2022 · d9d72ef · d9d72ef
1 parent 05eecb0
commit d9d72ef
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 281 deletions.
diff --git a/...ble-planner-loader/src/main/java/org/apache/flink/table/planner/loader/PlannerModule.java b/...ble-planner-loader/src/main/java/org/apache/flink/table/planner/loader/PlannerModule.java
@@ -68,9 +68,7 @@ class PlannerModule {
                                     // flink-table-runtime or flink-dist itself
                                     "org.codehaus.janino",
                                     "org.codehaus.commons",
-                                    "org.apache.commons.lang3",
-                                    // Used by org.reflections
-                                    "javassist"))
+                                    "org.apache.commons.lang3"))
                     .toArray(String[]::new);
 
     private static final String[] COMPONENT_CLASSPATH = new String[] {"org.apache.flink"};

diff --git a/flink-table/flink-table-planner/pom.xml b/flink-table/flink-table-planner/pom.xml
@@ -189,24 +189,6 @@ under the License.
 			<artifactId>scala-parser-combinators_${scala.binary.version}</artifactId>
 		</dependency>
 
-		<dependency>
-			<!-- Utility to scan classpaths -->
-			<groupId>org.reflections</groupId>
-			<artifactId>reflections</artifactId>
-			<version>0.9.10</version>
-			<scope>compile</scope>
-			<exclusions>
-				<exclusion>
-					<groupId>com.google.code.findbugs</groupId>
-					<artifactId>annotations</artifactId>
-				</exclusion>
-				<exclusion>
-					<groupId>com.google.guava</groupId>
-					<artifactId>guava</artifactId>
-				</exclusion>
-			</exclusions>
-		</dependency>
-
 		<!-- Test dependencies -->
 
 		<dependency>
@@ -372,9 +354,6 @@ under the License.
 
 							<!-- For legacy string expressions in Table API -->
 							<include>org.scala-lang.modules:scala-parser-combinators_${scala.binary.version}</include>
-
-							<!-- ReflectionsUtil -->
-							<include>org.reflections:reflections</include>
 						</includes>
 					</artifactSet>
 					<relocations>
@@ -410,11 +389,6 @@ under the License.
 							<shadedPattern>org.apache.flink.table.shaded.com.jayway</shadedPattern>
 						</relocation>
 
-						<!-- Other table dependencies -->
-						<relocation>
-							<pattern>org.reflections</pattern>
-							<shadedPattern>org.apache.flink.table.shaded.org.reflections</shadedPattern>
-						</relocation>
 						<relocation>
 							<!-- icu4j's dependencies -->
 							<pattern>com.ibm.icu</pattern>

diff --git a/...able-planner/src/main/java/org/apache/flink/table/planner/plan/utils/ReflectionsUtil.java b/...able-planner/src/main/java/org/apache/flink/table/planner/plan/utils/ReflectionsUtil.java
diff --git a/...est/java/org/apache/flink/table/planner/plan/nodes/exec/stream/JsonSerdeCoverageTest.java b/...est/java/org/apache/flink/table/planner/plan/nodes/exec/stream/JsonSerdeCoverageTest.java
diff --git a/...ner/src/test/java/org/apache/flink/table/planner/plan/utils/ExecNodeMetadataUtilTest.java b/...ner/src/test/java/org/apache/flink/table/planner/plan/utils/ExecNodeMetadataUtilTest.java
@@ -22,11 +22,13 @@
 import org.apache.flink.api.dag.Transformation;
 import org.apache.flink.table.data.RowData;
 import org.apache.flink.table.planner.delegation.PlannerBase;
+import org.apache.flink.table.planner.plan.nodes.exec.ExecNode;
 import org.apache.flink.table.planner.plan.nodes.exec.ExecNodeBase;
 import org.apache.flink.table.planner.plan.nodes.exec.ExecNodeContext;
 import org.apache.flink.table.planner.plan.nodes.exec.ExecNodeMetadata;
 import org.apache.flink.table.planner.plan.nodes.exec.InputProperty;
 import org.apache.flink.table.planner.plan.nodes.exec.MultipleExecNodeMetadata;
+import org.apache.flink.table.planner.plan.nodes.exec.serde.JsonSerdeUtil;
 import org.apache.flink.table.planner.plan.nodes.exec.stream.StreamExecNode;
 import org.apache.flink.table.types.logical.LogicalType;
 
@@ -35,8 +37,12 @@
 import org.assertj.core.api.Condition;
 import org.junit.Test;
 
+import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
 
+import static org.apache.flink.table.planner.plan.utils.ExecNodeMetadataUtil.UNSUPPORTED_JSON_SERDE_CLASSES;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
@@ -60,8 +66,7 @@ public void testNoAnnotation() {
                 .hasMessage(
                         "ExecNode: org.apache.flink.table.planner.plan.utils."
                                 + "ExecNodeMetadataUtilTest.DummyNodeNoAnnotation is missing "
-                                + "ExecNodeMetadata annotation. This is a bug, please contact "
-                                + "developers.");
+                                + "ExecNodeMetadata annotation.");
     }
 
     @Test
@@ -133,6 +138,41 @@ public void testNewContext() {
                                 + "classes and yet is annotated with: ExecNodeMetadata.");
     }
 
+    @Test
+    public void testStreamExecNodeJsonSerdeCoverage() {
+        Set<Class<? extends ExecNode<?>>> subClasses = ExecNodeMetadataUtil.execNodes();
+        List<Class<? extends ExecNode<?>>> classesWithoutJsonCreator = new ArrayList<>();
+        List<Class<? extends ExecNode<?>>> classesWithJsonCreatorInUnsupportedList =
+                new ArrayList<>();
+        for (Class<? extends ExecNode<?>> clazz : subClasses) {
+            boolean hasJsonCreator = JsonSerdeUtil.hasJsonCreatorAnnotation(clazz);
+            if (hasJsonCreator && UNSUPPORTED_JSON_SERDE_CLASSES.contains(clazz)) {
+                classesWithJsonCreatorInUnsupportedList.add(clazz);
+            }
+            if (!hasJsonCreator && !UNSUPPORTED_JSON_SERDE_CLASSES.contains(clazz)) {
+                classesWithoutJsonCreator.add(clazz);
+            }
+        }
+
+        assertThat(classesWithoutJsonCreator)
+                .as(
+                        "%s do not support json serialization/deserialization, "
+                                + "please refer the implementation of the other StreamExecNodes.",
+                        classesWithoutJsonCreator.stream()
+                                .map(Class::getSimpleName)
+                                .collect(Collectors.joining(",")))
+                .isEmpty();
+        assertThat(classesWithJsonCreatorInUnsupportedList)
+                .as(
+                        "%s have support for json serialization/deserialization, "
+                                + "but still in UNSUPPORTED_JSON_SERDE_CLASSES list. "
+                                + "please move them from UNSUPPORTED_JSON_SERDE_CLASSES.",
+                        classesWithJsonCreatorInUnsupportedList.stream()
+                                .map(Class::getSimpleName)
+                                .collect(Collectors.joining(",")))
+                .isEmpty();
+    }
+
     @MultipleExecNodeMetadata({
         @ExecNodeMetadata(
                 name = "dummy-node",

diff --git a/...-planner/src/test/java/org/apache/flink/table/planner/plan/utils/ReflectionsUtilTest.java b/...-planner/src/test/java/org/apache/flink/table/planner/plan/utils/ReflectionsUtilTest.java