Skip to content

Commit

Permalink
DRILL-8117: Clean up deprecated Apache code in Drill
Browse files Browse the repository at this point in the history
  • Loading branch information
kingswanwho authored and kingswanwho committed Dec 28, 2022
1 parent 88bb1c8 commit dac8dff
Show file tree
Hide file tree
Showing 12 changed files with 111 additions and 101 deletions.
5 changes: 5 additions & 0 deletions common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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 {
Expand Down
8 changes: 0 additions & 8 deletions common/src/test/java/org/apache/drill/test/DrillTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
*/
package org.apache.drill.exec.store.log;

import static org.junit.Assert.assertEquals;
import static org.hamcrest.CoreMatchers.containsString;

import java.io.File;
import java.util.HashMap;
Expand All @@ -32,11 +32,13 @@
import org.apache.drill.exec.physical.rowSet.RowSetBuilder;
import org.apache.drill.exec.record.metadata.SchemaBuilder;
import org.apache.drill.exec.record.metadata.TupleMetadata;
import org.apache.drill.exec.rpc.RpcException;
import org.apache.drill.exec.store.easy.text.compliant.BaseCsvTest;
import org.apache.drill.shaded.guava.com.google.common.collect.Lists;
import org.apache.drill.test.QueryBuilder;
import org.apache.drill.test.QueryBuilder.QuerySummary;
import org.apache.drill.test.rowSet.RowSetComparison;
import org.hamcrest.MatcherAssert;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.experimental.categories.Category;
Expand Down Expand Up @@ -106,10 +108,12 @@ public void testIssue7853UseValidDatetimeFormat() throws Exception {

@Test
public void testIssue7853() throws Exception {
thrownException.expect(UserRemoteException.class);
thrownException.expectMessage("is not valid for type TIMESTAMP");
String sql = "SELECT type, `time` FROM `dfs.data`.`root/issue7853.log2`";
QuerySummary result = client.queryBuilder().sql(sql).run();
assertEquals(2, result.recordCount());
UserRemoteException userRemoteException = Assert.assertThrows(UserRemoteException.class, () -> client.queryBuilder().sql(sql).run());
MatcherAssert.assertThat(userRemoteException.getMessage(), containsString("is not valid for type TIMESTAMP"));

try {
client.testBuilder().sqlQuery(sql).expectsNumRecords(2).go();
} catch (UserRemoteException | RpcException ignored) {}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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" +
Expand All @@ -246,7 +247,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");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,29 @@
import org.apache.drill.exec.ExecConstants;
import org.apache.drill.exec.TestWithZookeeper;
import org.apache.drill.exec.coord.zk.ZKClusterCoordinator;
import org.hamcrest.MatcherAssert;
import org.junit.Assert;
import org.junit.Test;

import static org.hamcrest.core.Is.is;

public class DrillClientStateTest extends TestWithZookeeper {

@Test
public void testNotExistZkRoot() throws Exception {
// There is no drillbit startup, therefore the root path is not saved in ZK.
thrownException.expect(NullPointerException.class);
thrownException.expectMessage("root path does not exist");
DrillConfig config = DrillConfig.create();
String connString = zkHelper.getConnectionString();
String zkRoot = config.getString(ExecConstants.ZK_ROOT); // does not exist
connString = connString + ZKPaths.PATH_SEPARATOR + zkRoot;
try (ZKClusterCoordinator coordinator = new ZKClusterCoordinator(config, connString)) {
coordinator.start(10000);
}
String connStringWithZKRoot = connString + ZKPaths.PATH_SEPARATOR + zkRoot;

NullPointerException nullPointerException = Assert.assertThrows(NullPointerException.class, () -> {
try (ZKClusterCoordinator coordinator = new ZKClusterCoordinator(config, connStringWithZKRoot)) {
coordinator.start(10000);
}
});

MatcherAssert.assertThat(nullPointerException.getMessage(), is("The root path does not exist in the Zookeeper."));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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."));
}
}

0 comments on commit dac8dff

Please sign in to comment.