This is an automated email from the ASF dual-hosted git repository. twalthr pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/flink.git
commit d9d72ef142a2343f37b8b10ca6e04dc7f6ca086e Author: Marios Trivyzas <[email protected]> AuthorDate: Mon Jan 31 12:55:58 2022 +0200 [hotfix][table-planner] Remove org.reflections usage and dependency --- .../flink/table/planner/loader/PlannerModule.java | 4 +- flink-table/flink-table-planner/pom.xml | 26 ------ .../table/planner/plan/utils/ReflectionsUtil.java | 56 ------------- .../nodes/exec/stream/JsonSerdeCoverageTest.java | 93 ---------------------- .../plan/utils/ExecNodeMetadataUtilTest.java | 44 +++++++++- .../planner/plan/utils/ReflectionsUtilTest.java | 87 -------------------- .../metadata/MetadataHandlerConsistencyTest.scala | 36 +++++---- 7 files changed, 65 insertions(+), 281 deletions(-) diff --git a/flink-table/flink-table-planner-loader/src/main/java/org/apache/flink/table/planner/loader/PlannerModule.java b/flink-table/flink-table-planner-loader/src/main/java/org/apache/flink/table/planner/loader/PlannerModule.java index 7d72fdc..0a698b2 100644 --- a/flink-table/flink-table-planner-loader/src/main/java/org/apache/flink/table/planner/loader/PlannerModule.java +++ b/flink-table/flink-table-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 index 3d986ef..ce559ac 100644 --- 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/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/utils/ReflectionsUtil.java b/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/utils/ReflectionsUtil.java deleted file mode 100644 index 7ea1409..0000000 --- a/flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/utils/ReflectionsUtil.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.flink.table.planner.plan.utils; - -import org.reflections.Reflections; - -import java.lang.reflect.Modifier; -import java.util.Set; -import java.util.stream.Collectors; - -/** An utility class for reflection operations on classes. */ -public class ReflectionsUtil { - - public static <T> Set<Class<? extends T>> scanSubClasses( - String packageName, Class<T> targetClass) { - return scanSubClasses(packageName, targetClass, false, false); - } - - public static <T> Set<Class<? extends T>> scanSubClasses( - String packageName, - Class<T> targetClass, - boolean includingInterface, - boolean includingAbstractClass) { - Reflections reflections = new Reflections(packageName); - return reflections.getSubTypesOf(targetClass).stream() - .filter( - c -> { - if (c.isInterface()) { - return includingInterface; - } else if (Modifier.isAbstract(c.getModifiers())) { - return includingAbstractClass; - } else { - return true; - } - }) - .collect(Collectors.toSet()); - } - - private ReflectionsUtil() {} -} diff --git a/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/stream/JsonSerdeCoverageTest.java b/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/stream/JsonSerdeCoverageTest.java deleted file mode 100644 index d87ecb0..0000000 --- a/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/stream/JsonSerdeCoverageTest.java +++ /dev/null @@ -1,93 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.flink.table.planner.plan.nodes.exec.stream; - -import org.apache.flink.table.planner.plan.nodes.exec.serde.JsonSerdeUtil; -import org.apache.flink.table.planner.plan.utils.ReflectionsUtil; - -import org.junit.Test; - -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Set; -import java.util.stream.Collectors; - -import static org.junit.Assert.assertTrue; - -/** - * Test to check whether all {@link StreamExecNode}s have been implemented json - * serialization/deserialization. - */ -public class JsonSerdeCoverageTest { - - private static final List<String> UNSUPPORTED_JSON_SERDE_CLASSES = - Arrays.asList( - "StreamExecDataStreamScan", - "StreamExecLegacyTableSourceScan", - "StreamExecLegacySink", - "StreamExecGroupTableAggregate", - "StreamExecPythonGroupTableAggregate", - "StreamExecSort", - "StreamExecMultipleInput"); - - @SuppressWarnings({"rawtypes"}) - @Test - public void testStreamExecNodeJsonSerdeCoverage() { - Set<Class<? extends StreamExecNode>> subClasses = - ReflectionsUtil.scanSubClasses("org.apache.flink", StreamExecNode.class); - - List<String> classes = new ArrayList<>(); - List<String> classesWithoutJsonCreator = new ArrayList<>(); - List<String> classesWithJsonCreatorInUnsupportedList = new ArrayList<>(); - for (Class<? extends StreamExecNode> clazz : subClasses) { - String className = clazz.getSimpleName(); - classes.add(className); - boolean hasJsonCreator = JsonSerdeUtil.hasJsonCreatorAnnotation(clazz); - if (hasJsonCreator && UNSUPPORTED_JSON_SERDE_CLASSES.contains(className)) { - classesWithJsonCreatorInUnsupportedList.add(className); - } - if (!hasJsonCreator && !UNSUPPORTED_JSON_SERDE_CLASSES.contains(className)) { - classesWithoutJsonCreator.add(className); - } - } - assertTrue( - String.format( - "%s do not support json serialization/deserialization, " - + "please refer the implementation of the other StreamExecNodes.", - String.join(",", classesWithoutJsonCreator)), - classesWithoutJsonCreator.isEmpty()); - assertTrue( - String.format( - "%s have support json serialization/deserialization, " - + "but still in UNSUPPORTED_JSON_SERDE_CLASSES list. " - + "please move them from UNSUPPORTED_JSON_SERDE_CLASSES.", - String.join(",", classesWithJsonCreatorInUnsupportedList)), - classesWithJsonCreatorInUnsupportedList.isEmpty()); - List<String> notExistingClasses = - UNSUPPORTED_JSON_SERDE_CLASSES.stream() - .filter(c -> !classes.contains(c)) - .collect(Collectors.toList()); - assertTrue( - String.format( - "%s do not exist any more, please remove them from UNSUPPORTED_JSON_SERDE_CLASSES.", - String.join(",", notExistingClasses)), - notExistingClasses.isEmpty()); - } -} diff --git a/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/utils/ExecNodeMetadataUtilTest.java b/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/utils/ExecNodeMetadataUtilTest.java index 7fd6a00..a6b8642 100644 --- a/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/utils/ExecNodeMetadataUtilTest.java +++ b/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/utils/ExecNodeMetadataUtilTest.java @@ -22,11 +22,13 @@ import org.apache.flink.FlinkVersion; 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.apache.flink.shaded.jackson2.com.fasterxml.jackson.annotation.JsonCre 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 class ExecNodeMetadataUtilTest { .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 class ExecNodeMetadataUtilTest { + "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/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/utils/ReflectionsUtilTest.java b/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/utils/ReflectionsUtilTest.java deleted file mode 100644 index 822442f..0000000 --- a/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/utils/ReflectionsUtilTest.java +++ /dev/null @@ -1,87 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.flink.table.planner.plan.utils; - -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; - -import java.util.HashSet; -import java.util.Set; - -import static org.junit.Assert.assertEquals; - -/** Test for {@link ReflectionsUtil}. */ -@RunWith(Parameterized.class) -public class ReflectionsUtilTest { - - @Parameterized.Parameter public boolean includingInterface; - - @Parameterized.Parameter(1) - public boolean includingAbstractClass; - - @Test - public void testScanSubClasses() { - Set<Class<? extends TestInterface>> actual = - ReflectionsUtil.scanSubClasses( - ReflectionsUtilTest.class.getPackage().getName(), - TestInterface.class, - includingInterface, - includingAbstractClass); - Set<Class<? extends TestInterface>> expected = new HashSet<>(); - expected.add(TestClass1.class); - expected.add(TestClass2.class); - expected.add(TestClass3.class); - if (includingInterface) { - expected.add(TestSubInterface.class); - } - if (includingAbstractClass) { - expected.add(TestAbstractClass.class); - } - assertEquals(expected, actual); - } - - @Parameterized.Parameters(name = "includingInterface={0}, includingAbstractClass={1}") - public static Object[][] testData() { - return new Object[][] { - new Object[] {false, false}, - new Object[] {true, false}, - new Object[] {false, true}, - new Object[] {true, true} - }; - } - - /** Testing interface. */ - public interface TestInterface {} - - /** Testing interface. */ - public interface TestSubInterface extends TestInterface {} - - /** Testing abstract class. */ - public abstract static class TestAbstractClass implements TestSubInterface {} - - /** Testing class. */ - public static class TestClass1 implements TestInterface {} - - /** Testing class. */ - public static class TestClass2 implements TestSubInterface {} - - /** Testing class. */ - public static class TestClass3 extends TestAbstractClass {} -} diff --git a/flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/plan/metadata/MetadataHandlerConsistencyTest.scala b/flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/plan/metadata/MetadataHandlerConsistencyTest.scala index b1e48d9..ebed9ff 100644 --- a/flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/plan/metadata/MetadataHandlerConsistencyTest.scala +++ b/flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/plan/metadata/MetadataHandlerConsistencyTest.scala @@ -19,7 +19,6 @@ package org.apache.flink.table.planner.plan.metadata import org.apache.flink.table.planner.plan.nodes.physical.batch.{BatchPhysicalCorrelate, BatchPhysicalGroupAggregateBase} -import org.apache.flink.table.planner.plan.utils.ReflectionsUtil import org.apache.calcite.rel.RelNode import org.apache.calcite.rel.core.{Aggregate, Correlate} @@ -56,8 +55,28 @@ class MetadataHandlerConsistencyTest( logicalNodeClass: Class[_ <: RelNode], physicalNodeClass: Class[_ <: RelNode]) { - // get all subclasses of [[MetadataHandler]] - private val allMdHandlerClazz = fetchAllExtendedMetadataHandlers + // all subclasses of [[MetadataHandler]] + private val allMdHandlerClazz = List( + classOf[FlinkRelMdPercentageOriginalRows], + classOf[FlinkRelMdColumnNullCount], + classOf[FlinkRelMdColumnInterval], + classOf[FlinkRelMdCumulativeCost], + classOf[FlinkRelMdModifiedMonotonicity], + classOf[FlinkRelMdWindowProperties], + classOf[FlinkRelMdUniqueKeys], + classOf[FlinkRelMdDistribution], + classOf[FlinkRelMdSize], + classOf[FlinkRelMdCollation], + classOf[FlinkRelMdUniqueGroups], + classOf[FlinkRelMdPopulationSize], + classOf[FlinkRelMdFilteredColumnInterval], + classOf[FlinkRelMdUpsertKeys], + classOf[FlinkRelMdColumnUniqueness], + classOf[FlinkRelMdColumnOriginNullCount], + classOf[FlinkRelMdRowCount], + classOf[FlinkRelMdSelectivity], + classOf[FlinkRelMdNonCumulativeCost], + classOf[FlinkRelMdDistinctRowCount]) // initiate each subclasses of [[MetadataHandler]] private val mdHandlerInstances = allMdHandlerClazz map { mdhClass => @@ -99,17 +118,6 @@ class MetadataHandlerConsistencyTest( } /** - * Scan packages to find out all subclasses of [[MetadataHandler]] in flink. - * - * @return A list contains all subclasses of [[MetadataHandler]] in flink. - */ - private def fetchAllExtendedMetadataHandlers: Seq[Class[_ <: MetadataHandler[_]]] = { - ReflectionsUtil.scanSubClasses( - "org.apache.flink.table.planner.plan.cost", - classOf[MetadataHandler[_]]).toSeq - } - - /** * Gets whether the given metadataHandler contains explicit metadata estimation for the given * RelNode class. *
