DRILL-8117: Clean up deprecated Apache code in Drill

apache · Mar 20, 2022 · e996f17 · e996f17
1 parent 7ef203b
commit e996f17
Show file tree

Hide file tree

Showing 11 changed files with 93 additions and 90 deletions.
diff --git a/common/pom.xml b/common/pom.xml
@@ -47,6 +47,11 @@
       <artifactId>commons-lang3</artifactId>
     </dependency>
 
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-text</artifactId>
+    </dependency>
+
     <dependency>
       <groupId>org.msgpack</groupId>
       <artifactId>msgpack</artifactId>

diff --git a/common/src/main/java/org/apache/drill/common/PlanStringBuilder.java b/common/src/main/java/org/apache/drill/common/PlanStringBuilder.java
@@ -17,7 +17,7 @@
  */
 package org.apache.drill.common;
 
-import org.apache.commons.lang3.StringEscapeUtils;
+import org.apache.commons.text.StringEscapeUtils;
 
 /**
  * Builds a string in Drill's "plan string" format: that shown in the

diff --git a/common/src/main/java/org/apache/drill/common/config/DrillConfig.java b/common/src/main/java/org/apache/drill/common/config/DrillConfig.java
@@ -317,7 +317,7 @@ private <T> Class<T> getClassAt(String location, Class<T> clazz) throws DrillCon
   public <T> T getInstanceOf(String location, Class<T> clazz) throws DrillConfigurationException{
     final Class<T> c = getClassAt(location, clazz);
     try {
-      return c.newInstance();
+      return c.getDeclaredConstructor().newInstance();
     } catch (Exception ex) {
       throw new DrillConfigurationException(String.format("Failure while instantiating class [%s] located at '%s.", clazz.getCanonicalName(), location), ex);
     }

diff --git a/common/src/main/java/org/apache/drill/common/util/DrillStringUtils.java b/common/src/main/java/org/apache/drill/common/util/DrillStringUtils.java
@@ -19,7 +19,7 @@
 
 import io.netty.buffer.ByteBuf;
 
-import org.apache.commons.lang3.StringEscapeUtils;
+import org.apache.commons.text.StringEscapeUtils;
 import org.apache.commons.lang3.StringUtils;
 
 import java.util.Arrays;

diff --git a/common/src/test/java/org/apache/drill/common/util/function/TestCheckedFunction.java b/common/src/test/java/org/apache/drill/common/util/function/TestCheckedFunction.java
@@ -18,17 +18,16 @@
 package org.apache.drill.common.util.function;
 
 import org.apache.drill.test.BaseTest;
-import org.junit.Rule;
+import org.hamcrest.MatcherAssert;
+import org.junit.Assert;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
 
 import java.util.HashMap;
 import java.util.Map;
 
-public class TestCheckedFunction extends BaseTest {
+import static org.hamcrest.CoreMatchers.containsString;
 
-  @Rule
-  public ExpectedException thrown = ExpectedException.none();
+public class TestCheckedFunction extends BaseTest {
 
   @Test
   public void testComputeIfAbsentWithCheckedFunction() {
@@ -37,10 +36,8 @@ public void testComputeIfAbsentWithCheckedFunction() {
     String message = "Exception message";
     CheckedFunction<String, String, Exception> function = producer::failWithMessage;
 
-    thrown.expect(Exception.class);
-    thrown.expectMessage(message);
-
-    map.computeIfAbsent(message, function);
+    Exception exception = Assert.assertThrows(Exception.class, () -> map.computeIfAbsent(message, function));
+    MatcherAssert.assertThat(exception.getMessage(), containsString(message));
   }
 
   private class ExceptionProducer {

diff --git a/common/src/test/java/org/apache/drill/test/DrillTest.java b/common/src/test/java/org/apache/drill/test/DrillTest.java
@@ -27,7 +27,6 @@
 import org.junit.BeforeClass;
 import org.junit.Rule;
 import org.junit.rules.DisableOnDebug;
-import org.junit.rules.ExpectedException;
 import org.junit.rules.TestRule;
 import org.junit.rules.TestWatcher;
 import org.junit.runner.Description;
@@ -58,13 +57,6 @@ public class DrillTest extends BaseTest {
 
   @Rule public final TestRule REPEAT_RULE = TestTools.getRepeatRule(false);
 
-  /**
-   * Rule for tests that verify {@link org.apache.drill.common.exceptions.UserException} type and message. See
-   * {@link UserExceptionMatcher} and e.g. apache.drill.exec.server.TestOptions#checkValidationException.
-   * Tests that do not use this rule are not affected.
-   */
-  @Rule public final ExpectedException thrownException = ExpectedException.none();
-
   @BeforeClass
   public static void initDrillTest() {
     memWatcher = new MemWatcher();

diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java b/exec/java-exec/src/test/java/org/apache/drill/exec/TestWindowFunctions.java
@@ -29,6 +29,8 @@
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.hamcrest.MatcherAssert;
+import org.junit.Assert;
 
 import java.nio.file.Paths;
 
@@ -190,54 +192,53 @@ public void testWindowWithAllowDisallow() throws Exception {
 
   @Test // DRILL-3360
   public void testWindowInWindow() throws Exception {
-    thrownException.expect(new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION));
     String query = "select rank() over(order by row_number() over(order by n_nationkey)) \n" +
-        "from cp.`tpch/nation.parquet`";
+      "from cp.`tpch/nation.parquet`";
 
-    test(query);
+    UserException userException = Assert.assertThrows(UserException.class, () -> test(query));
+    MatcherAssert.assertThat(userException, new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION));
   }
 
   @Test // DRILL-3280
   public void testMissingOverWithWindowClause() throws Exception {
-    thrownException.expect(new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION));
     String query = "select rank(), cume_dist() over w \n" +
-        "from cp.`tpch/nation.parquet` \n" +
-        "window w as (partition by n_name order by n_nationkey)";
+      "from cp.`tpch/nation.parquet` \n" +
+      "window w as (partition by n_name order by n_nationkey)";
 
-    test(query);
+    UserException userException = Assert.assertThrows(UserException.class, () -> test(query));
+    MatcherAssert.assertThat(userException, new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION));
   }
 
   @Test // DRILL-3601
   public void testLeadMissingOver() throws Exception {
-    thrownException.expect(new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION));
     String query = "select lead(n_nationkey) from cp.`tpch/nation.parquet`";
 
-    test(query);
+    UserException userException = Assert.assertThrows(UserException.class, () -> test(query));
+    MatcherAssert.assertThat(userException, new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION));
   }
 
   @Test // DRILL-3649
   public void testMissingOverWithConstant() throws Exception {
-    thrownException.expect(new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION));
     String query = "select NTILE(1) from cp.`tpch/nation.parquet`";
 
-    test(query);
+    UserException userException = Assert.assertThrows(UserException.class, () -> test(query));
+    MatcherAssert.assertThat(userException, new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION));
   }
 
   @Test // DRILL-3344
   public void testWindowGroupBy() throws Exception {
-    thrownException.expect(new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION));
     String query = "explain plan for SELECT max(n_nationkey) OVER (), n_name as col2 \n" +
         "from cp.`tpch/nation.parquet` \n" +
         "group by n_name";
 
-    test(query);
+    UserException userException = Assert.assertThrows(UserException.class, () -> test(query));
+    MatcherAssert.assertThat(userException, new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION));
   }
 
   @Test // DRILL-3346
   @Category(UnlikelyTest.class)
   public void testWindowGroupByOnView() throws Exception {
     try {
-      thrownException.expect(new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION));
       String createView = "create view testWindowGroupByOnView(a, b) as \n" +
           "select n_nationkey, n_name from cp.`tpch/nation.parquet`";
       String query = "explain plan for SELECT max(a) OVER (), b as col2 \n" +
@@ -247,6 +248,8 @@ public void testWindowGroupByOnView() throws Exception {
       test("use dfs.tmp");
       test(createView);
       test(query);
+      UserException userException = Assert.assertThrows(UserException.class, () -> test(query));
+      MatcherAssert.assertThat(userException, new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION));
     } finally {
       test("drop view testWindowGroupByOnView");
     }

diff --git a/...java-exec/src/test/java/org/apache/drill/exec/impersonation/TestInboundImpersonation.java b/...java-exec/src/test/java/org/apache/drill/exec/impersonation/TestInboundImpersonation.java
@@ -17,6 +17,7 @@
  */
 package org.apache.drill.exec.impersonation;
 
+import org.apache.drill.common.exceptions.UserException;
 import org.apache.drill.shaded.guava.com.google.common.collect.Maps;
 import com.typesafe.config.ConfigValueFactory;
 import org.apache.drill.categories.SecurityTest;
@@ -33,7 +34,9 @@
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.permission.FsPermission;
+import org.hamcrest.MatcherAssert;
 import org.junit.AfterClass;
+import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -156,22 +159,25 @@ public void unauthorizedTarget() throws Exception {
 
   @Test
   public void invalidPolicy() throws Exception {
-    thrownException.expect(new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION,
-        "Invalid impersonation policies."));
+    String query = "ALTER SYSTEM SET `%s`='%s'";
+    String arg = "[ invalid json ]";
+
     updateClient(UserAuthenticatorTestImpl.PROCESS_USER,
-        UserAuthenticatorTestImpl.PROCESS_USER_PASSWORD);
-    test("ALTER SYSTEM SET `%s`='%s'", ExecConstants.IMPERSONATION_POLICIES_KEY,
-        "[ invalid json ]");
+      UserAuthenticatorTestImpl.PROCESS_USER_PASSWORD);
+
+    UserException userException = Assert.assertThrows(UserException.class, () -> test(query, ExecConstants.IMPERSONATION_POLICIES_KEY, arg));
+    MatcherAssert.assertThat(userException, new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION, "Invalid impersonation policies."));
   }
 
   @Test
   public void invalidProxy() throws Exception {
-    thrownException.expect(new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION,
-        "Proxy principals cannot have a wildcard entry."));
+    String query = "ALTER SYSTEM SET `%s`='%s'";
+    String arg = "[ { proxy_principals : { users: [\"*\" ] }," + "target_principals : { users : [\"" + TARGET_NAME + "\"] } } ]";
+
     updateClient(UserAuthenticatorTestImpl.PROCESS_USER,
-        UserAuthenticatorTestImpl.PROCESS_USER_PASSWORD);
-    test("ALTER SYSTEM SET `%s`='%s'", ExecConstants.IMPERSONATION_POLICIES_KEY,
-        "[ { proxy_principals : { users: [\"*\" ] },"
-            + "target_principals : { users : [\"" + TARGET_NAME + "\"] } } ]");
+      UserAuthenticatorTestImpl.PROCESS_USER_PASSWORD);
+
+    UserException userException = Assert.assertThrows(UserException.class, () -> test(query, ExecConstants.IMPERSONATION_POLICIES_KEY, arg));
+    MatcherAssert.assertThat(userException, new UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION, "Proxy principals cannot have a wildcard entry."));
   }
 }
diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/planner/sql/CrossJoinTest.java b/exec/java-exec/src/test/java/org/apache/drill/exec/planner/sql/CrossJoinTest.java
@@ -18,16 +18,20 @@
 package org.apache.drill.exec.planner.sql;
 
 import org.apache.drill.categories.SqlTest;
+import org.apache.drill.common.exceptions.UserException;
 import org.apache.drill.common.exceptions.UserRemoteException;
 import org.apache.drill.exec.planner.physical.PlannerSettings;
 import org.apache.drill.test.ClusterFixture;
 import org.apache.drill.test.ClusterTest;
+import org.hamcrest.MatcherAssert;
 import org.junit.After;
+import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
 import static org.apache.drill.exec.physical.impl.join.JoinUtils.FAILED_TO_PLAN_CARTESIAN_JOIN;
+import static org.hamcrest.CoreMatchers.containsString;
 
 @Category(SqlTest.class)
 public class CrossJoinTest extends ClusterTest {
@@ -49,15 +53,12 @@ public void tearDown() {
   @Test
   public void testCrossJoinFailsForEnabledOption() throws Exception {
     enableNlJoinForScalarOnly();
+    String sql = "SELECT l.n_name, r.n_name " +
+      "FROM cp.`tpch/nation.parquet` l " +
+      "CROSS JOIN cp.`tpch/nation.parquet` r";
 
-    thrownException.expect(UserRemoteException.class);
-    thrownException.expectMessage(FAILED_TO_PLAN_CARTESIAN_JOIN);
-
-    queryBuilder().sql(
-        "SELECT l.n_name, r.n_name " +
-            "FROM cp.`tpch/nation.parquet` l " +
-            "CROSS JOIN cp.`tpch/nation.parquet` r")
-        .run();
+    UserRemoteException UserRemoteException = Assert.assertThrows(UserRemoteException.class, () ->  queryBuilder().sql(sql).run());
+    MatcherAssert.assertThat(UserRemoteException.getMessage(), containsString(FAILED_TO_PLAN_CARTESIAN_JOIN));
   }
 
   @Test
@@ -75,13 +76,11 @@ public void testCrossJoinSucceedsForDisabledOption() throws Exception {
   public void testCommaJoinFailsForEnabledOption() throws Exception {
     enableNlJoinForScalarOnly();
 
-    thrownException.expect(UserRemoteException.class);
-    thrownException.expectMessage(FAILED_TO_PLAN_CARTESIAN_JOIN);
+    String sql = "SELECT l.n_name,r.n_name " +
+      "FROM cp.`tpch/nation.parquet` l, cp.`tpch/nation.parquet` r";
 
-    queryBuilder().sql(
-        "SELECT l.n_name,r.n_name " +
-            "FROM cp.`tpch/nation.parquet` l, cp.`tpch/nation.parquet` r")
-        .run();
+    UserException UserRemoteException = Assert.assertThrows(UserRemoteException.class, () -> queryBuilder().sql(sql).run());
+    MatcherAssert.assertThat(UserRemoteException.getMessage(), containsString(FAILED_TO_PLAN_CARTESIAN_JOIN));
   }
 
   @Test
@@ -98,17 +97,15 @@ public void testCommaJoinSucceedsForDisabledOption() throws Exception {
   public void testSubSelectCrossJoinFailsForEnabledOption() throws Exception {
     enableNlJoinForScalarOnly();
 
-    thrownException.expect(UserRemoteException.class);
-    thrownException.expectMessage(FAILED_TO_PLAN_CARTESIAN_JOIN);
+    String sql = "SELECT COUNT(*) c " +
+      "FROM (" +
+      "SELECT l.n_name,r.n_name " +
+      "FROM cp.`tpch/nation.parquet` l " +
+      "CROSS JOIN cp.`tpch/nation.parquet` r" +
+      ")";
 
-    queryBuilder().sql(
-        "SELECT COUNT(*) c " +
-            "FROM (" +
-            "SELECT l.n_name,r.n_name " +
-            "FROM cp.`tpch/nation.parquet` l " +
-            "CROSS JOIN cp.`tpch/nation.parquet` r" +
-            ")")
-        .run();
+    UserException UserRemoteException = Assert.assertThrows(UserRemoteException.class, () -> queryBuilder().sql(sql).run());
+    MatcherAssert.assertThat(UserRemoteException.getMessage(), containsString(FAILED_TO_PLAN_CARTESIAN_JOIN));
   }
 
   @Test
@@ -131,14 +128,12 @@ public void testSubSelectCrossJoinSucceedsForDisabledOption() throws Exception {
   public void textCrossAndCommaJoinFailsForEnabledOption() throws Exception {
     enableNlJoinForScalarOnly();
 
-    thrownException.expect(UserRemoteException.class);
-    thrownException.expectMessage(FAILED_TO_PLAN_CARTESIAN_JOIN);
+    String sql = "SELECT * " +
+      "FROM cp.`tpch/nation.parquet` a, cp.`tpch/nation.parquet` b " +
+      "CROSS JOIN cp.`tpch/nation.parquet` c";
 
-    queryBuilder().sql(
-        "SELECT * " +
-            "FROM cp.`tpch/nation.parquet` a, cp.`tpch/nation.parquet` b " +
-            "CROSS JOIN cp.`tpch/nation.parquet` c")
-        .run();
+    UserException UserRemoteException = Assert.assertThrows(UserRemoteException.class, () -> queryBuilder().sql(sql).run());
+    MatcherAssert.assertThat(UserRemoteException.getMessage(), containsString(FAILED_TO_PLAN_CARTESIAN_JOIN));
   }
 
   @Test
@@ -157,14 +152,12 @@ public void textCrossAndCommaJoinSucceedsForDisabledOption() throws Exception {
   public void testCrossApplyFailsForEnabledOption() throws Exception {
     enableNlJoinForScalarOnly();
 
-    thrownException.expect(UserRemoteException.class);
-    thrownException.expectMessage(FAILED_TO_PLAN_CARTESIAN_JOIN);
+    String sql =  "SELECT * " +
+      "FROM cp.`tpch/nation.parquet` l " +
+      "CROSS APPLY cp.`tpch/nation.parquet` r";
 
-    queryBuilder().sql(
-        "SELECT * " +
-            "FROM cp.`tpch/nation.parquet` l " +
-            "CROSS APPLY cp.`tpch/nation.parquet` r")
-        .run();
+    UserException UserRemoteException = Assert.assertThrows(UserRemoteException.class, () -> queryBuilder().sql(sql).run());
+    MatcherAssert.assertThat(UserRemoteException.getMessage(), containsString(FAILED_TO_PLAN_CARTESIAN_JOIN));
   }
 
   @Test

diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptions.java b/exec/java-exec/src/test/java/org/apache/drill/exec/server/TestOptions.java
@@ -22,9 +22,12 @@
 import static org.apache.drill.exec.proto.UserBitShared.DrillPBError.ErrorType.VALIDATION;
 
 import org.apache.drill.categories.OptionsTest;
+import org.apache.drill.common.exceptions.UserException;
 import org.apache.drill.exec.ExecConstants;
 import org.apache.drill.test.BaseTestQuery;
 import org.apache.drill.test.UserExceptionMatcher;
+import org.hamcrest.MatcherAssert;
+import org.junit.Assert;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
@@ -50,8 +53,8 @@ public void testOptions() throws Exception{
 
   @Test
   public void checkValidationException() throws Exception {
-    thrownException.expect(new UserExceptionMatcher(VALIDATION));
-    test("ALTER session SET %s = '%s';", SLICE_TARGET, "fail");
+    UserException userException = Assert.assertThrows(UserException.class, () -> test("ALTER session SET %s = '%s';", SLICE_TARGET, "fail"));
+    MatcherAssert.assertThat(userException, new UserExceptionMatcher(VALIDATION));
   }
 
   @Test // DRILL-3122
@@ -311,8 +314,7 @@ public void changeSystemAndNotSession() throws Exception {
 
   @Test
   public void unsupportedLiteralValidation() throws Exception {
-    thrownException.expect(new UserExceptionMatcher(VALIDATION,
-      "Drill doesn't support assigning literals of type"));
-    test("ALTER session SET `%s` = DATE '1995-01-01';", ENABLE_VERBOSE_ERRORS_KEY);
+    UserException userException = Assert.assertThrows(UserException.class, () -> test("ALTER session SET `%s` = DATE '1995-01-01';", ENABLE_VERBOSE_ERRORS_KEY));
+    MatcherAssert.assertThat(userException, new UserExceptionMatcher(VALIDATION, "Drill doesn't support assigning literals of type"));
   }
 }