This is an automated email from the ASF dual-hosted git repository. granthenke pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 2649865b39723407236b15deddee9d15890f9f95 Author: Grant Henke <[email protected]> AuthorDate: Wed Mar 11 14:29:08 2020 -0500 [java] Fix checkstyle and spotbugs issues This patch fixes remaining checkstyle and spotbugs issues. A follow on patch will adjust the pre-commit build to fail if new issues are introduced. Change-Id: I89c226da7a763991c5adacbc609710450535e26c Reviewed-on: http://gerrit.cloudera.org:8080/15410 Tested-by: Grant Henke <[email protected]> Reviewed-by: Hao Hao <[email protected]> --- java/config/spotbugs/excludeFilter.xml | 19 +++++++++++++++++++ .../org/apache/kudu/client/TestAsyncKuduSession.java | 4 ++-- .../org/apache/kudu/subprocess/MessageParser.java | 2 -- .../org/apache/kudu/subprocess/MessageWriter.java | 2 -- .../apache/kudu/subprocess/SubprocessExecutor.java | 2 +- .../apache/kudu/subprocess/SubprocessMetrics.java | 6 +++--- .../subprocess/ranger/RangerProtocolHandler.java | 9 +++++---- .../kudu/subprocess/ranger/RangerSubprocessMain.java | 3 ++- .../ranger/authorization/RangerKuduAuthorizer.java | 20 +++++++++++--------- .../apache/kudu/subprocess/SubprocessTestUtil.java | 11 +++++------ .../kudu/subprocess/echo/TestEchoSubprocess.java | 6 ++++-- .../kudu/subprocess/ranger/TestRangerSubprocess.java | 15 ++++++++------- .../authorization/TestRangerKuduAuthorizer.java | 15 ++++++++------- 13 files changed, 68 insertions(+), 46 deletions(-) diff --git a/java/config/spotbugs/excludeFilter.xml b/java/config/spotbugs/excludeFilter.xml index 2c785f6..676b885 100644 --- a/java/config/spotbugs/excludeFilter.xml +++ b/java/config/spotbugs/excludeFilter.xml @@ -294,6 +294,25 @@ <Bug pattern="FE_FLOATING_POINT_EQUALITY" /> </Match> + <!-- kudu-subprocess exclusions --> + <Match> + <!-- We want to ignore the exception. --> + <Class name="org.apache.kudu.subprocess.echo.EchoProtocolHandler"/> + <Bug pattern="DE_MIGHT_IGNORE" /> + </Match> + <Match> + <!-- This is not a security concern. --> + <Class name="org.apache.kudu.subprocess.SubprocessTestUtil"/> + <Field name="NO_ARGS" /> + <Bug pattern="MS_MUTABLE_ARRAY" /> + </Match> + <Match> + <!-- This is done to simplify testing. --> + <Class name="org.apache.kudu.subprocess.ranger.TestRangerSubprocess"/> + <Method name="mockAuthorizer" /> + <Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD" /> + </Match> + <!-- kudu-test-utils exclusions --> <Match> <!-- There is nothing useful to do with the File.delete() return value. --> diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java index 700057d..e66789b 100644 --- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java +++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java @@ -34,13 +34,13 @@ import com.stumbleupon.async.Deferred; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.apache.kudu.Schema; import org.apache.kudu.WireProtocol.AppStatusPB; import org.apache.kudu.test.KuduTestHarness; import org.apache.kudu.tserver.Tserver.TabletServerErrorPB; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; public class TestAsyncKuduSession { private static final Logger LOG = LoggerFactory.getLogger(TestAsyncKuduSession.class); diff --git a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageParser.java b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageParser.java index 23f25ea..8f727bc 100644 --- a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageParser.java +++ b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageParser.java @@ -19,10 +19,8 @@ package org.apache.kudu.subprocess; import java.nio.charset.StandardCharsets; import java.util.concurrent.BlockingQueue; -import java.util.concurrent.TimeUnit; import com.google.common.base.Preconditions; -import com.google.common.base.Stopwatch; import com.google.protobuf.InvalidProtocolBufferException; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; diff --git a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageWriter.java b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageWriter.java index 81d7a97..b5b1c77 100644 --- a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageWriter.java +++ b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageWriter.java @@ -23,8 +23,6 @@ import java.util.concurrent.BlockingQueue; import com.google.common.base.Preconditions; import org.apache.yetus.audience.InterfaceAudience; -import org.apache.kudu.subprocess.Subprocess.SubprocessResponsePB; - /** * The {@link MessageWriter} class, * 1. retrieves one message from the outbound queue at a time, diff --git a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessExecutor.java b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessExecutor.java index b56d301..ced6890 100644 --- a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessExecutor.java +++ b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessExecutor.java @@ -181,7 +181,7 @@ public class SubprocessExecutor { throws ExecutionException, InterruptedException { Preconditions.checkArgument(timeoutMs != -1); try { - run(args, handler, timeoutMs); + run(args, handler, timeoutMs); } catch (TimeoutException e) { // no-op } diff --git a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessMetrics.java b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessMetrics.java index 94e82c6..6f2e7e4 100644 --- a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessMetrics.java +++ b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessMetrics.java @@ -17,12 +17,12 @@ package org.apache.kudu.subprocess; -import com.google.common.base.Preconditions; -import com.google.common.base.Stopwatch; - import java.util.concurrent.BlockingQueue; import java.util.concurrent.TimeUnit; +import com.google.common.base.Preconditions; +import com.google.common.base.Stopwatch; + /** * Encapsulates the metrics associated with the subprocess. It is expected that * this is passed around alongside each request/response as it makes its way diff --git a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/RangerProtocolHandler.java b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/RangerProtocolHandler.java index 66a6a47..aad4c50 100644 --- a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/RangerProtocolHandler.java +++ b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/RangerProtocolHandler.java @@ -17,15 +17,16 @@ package org.apache.kudu.subprocess.ranger; +import org.apache.ranger.plugin.policyengine.RangerAccessResult; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import org.apache.kudu.ranger.Ranger.RangerRequestListPB; import org.apache.kudu.ranger.Ranger.RangerResponseListPB; import org.apache.kudu.ranger.Ranger.RangerResponsePB; import org.apache.kudu.subprocess.ProtocolHandler; import org.apache.kudu.subprocess.ranger.authorization.RangerKuduAuthorizer; -import org.apache.ranger.plugin.policyengine.RangerAccessResult; -import org.apache.yetus.audience.InterfaceAudience; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Class that sends requests to Ranger and gets authorization decision diff --git a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/RangerSubprocessMain.java b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/RangerSubprocessMain.java index 167cd08..238c13f 100644 --- a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/RangerSubprocessMain.java +++ b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/RangerSubprocessMain.java @@ -17,9 +17,10 @@ package org.apache.kudu.subprocess.ranger; -import org.apache.kudu.subprocess.SubprocessExecutor; import org.apache.yetus.audience.InterfaceAudience; +import org.apache.kudu.subprocess.SubprocessExecutor; + // The Ranger subprocess that wraps the Kudu Ranger plugin. For the // plugin to successfully connect to the Ranger service, configurations // such as ranger-kudu-security.xml (and ranger-kudu-policymgr-ssl.xml diff --git a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/authorization/RangerKuduAuthorizer.java b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/authorization/RangerKuduAuthorizer.java index 01ebe27..1c0040a 100644 --- a/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/authorization/RangerKuduAuthorizer.java +++ b/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/ranger/authorization/RangerKuduAuthorizer.java @@ -17,11 +17,17 @@ package org.apache.kudu.subprocess.ranger.authorization; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import javax.annotation.Nullable; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import org.apache.hadoop.security.UserGroupInformation; -import org.apache.kudu.ranger.Ranger.RangerRequestListPB; -import org.apache.kudu.ranger.Ranger.RangerRequestPB; import org.apache.ranger.plugin.audit.RangerDefaultAuditHandler; import org.apache.ranger.plugin.model.RangerServiceDef; import org.apache.ranger.plugin.policyengine.RangerAccessRequest; @@ -33,12 +39,8 @@ import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.annotation.Nullable; -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashSet; -import java.util.List; -import java.util.Set; +import org.apache.kudu.ranger.Ranger.RangerRequestListPB; +import org.apache.kudu.ranger.Ranger.RangerRequestPB; public class RangerKuduAuthorizer { private static final Logger LOG = LoggerFactory.getLogger(RangerKuduAuthorizer.class); @@ -157,7 +159,7 @@ public class RangerKuduAuthorizer { Set<String> groups = getUserGroups(user); for (RangerRequestPB request : requests.getRequestsList()) { // Action should be lower case to match the Kudu service def in Ranger. - String action = request.getAction().name().toLowerCase(); + String action = request.getAction().name().toLowerCase(Locale.ENGLISH); String db = request.hasDatabase() ? request.getDatabase() : null; String table = request.hasTable() ? request.getTable() : null; String column = request.hasColumn() ? request.getColumn() : null; diff --git a/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/SubprocessTestUtil.java b/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/SubprocessTestUtil.java index 1abb019..f08a8a4 100644 --- a/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/SubprocessTestUtil.java +++ b/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/SubprocessTestUtil.java @@ -35,11 +35,11 @@ import java.util.function.Function; import com.google.protobuf.Any; import com.google.protobuf.Message; import com.google.protobuf.Parser; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.apache.kudu.subprocess.Subprocess.EchoRequestPB; import org.apache.kudu.subprocess.Subprocess.SubprocessRequestPB; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Utility class of common functions used for testing subprocess. @@ -56,7 +56,7 @@ public class SubprocessTestUtil { fail(); return null; }; - protected final Function<Throwable, Void> HAS_ERR = e -> { + protected static final Function<Throwable, Void> HAS_ERR = e -> { assertTrue(e instanceof KuduSubprocessException); return null; }; @@ -115,10 +115,9 @@ public class SubprocessTestUtil { if (injectIOError) { System.setOut(new PrintStreamWithIOException(outputPipe, /*autoFlush*/false, "UTF-8")); } else { - System.setOut(new PrintStream(outputPipe)); + System.setOut(new PrintStream(outputPipe, /*autoFlush*/false, "UTF-8")); } - SubprocessExecutor subprocessExecutor = new SubprocessExecutor(errorHandler); - return subprocessExecutor; + return new SubprocessExecutor(errorHandler); } /** diff --git a/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java b/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java index 5f0e3c1..fbaaa52 100644 --- a/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java +++ b/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java @@ -113,9 +113,10 @@ public class TestEchoSubprocess extends SubprocessTestUtil { */ @Test public void testSlowExecutionMetrics() throws Exception { - SubprocessExecutor executor = - setUpExecutorIO(NO_ERR, /*injectIOError*/false); final int SLEEP_MS = 200; + // Suppress checkstyle VariableDeclarationUsageDistance warning. + // CHECKSTYLE:OFF + SubprocessExecutor executor = setUpExecutorIO(NO_ERR, /*injectIOError*/false); sendRequestToPipe(createEchoSubprocessRequest(MESSAGE, SLEEP_MS)); sendRequestToPipe(createEchoSubprocessRequest(MESSAGE, SLEEP_MS)); sendRequestToPipe(createEchoSubprocessRequest(MESSAGE, SLEEP_MS)); @@ -123,6 +124,7 @@ public class TestEchoSubprocess extends SubprocessTestUtil { // Run the executor with a single parser thread so we can make stronger // assumptions about timing. executor.runUntilTimeout(new String[]{"-p", "1"}, new EchoProtocolHandler(), TIMEOUT_MS); + // CHECKSTYLE:ON SubprocessMetricsPB m = receiveResponse().getMetrics(); long inboundQueueLength = m.getInboundQueueLength(); diff --git a/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/ranger/TestRangerSubprocess.java b/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/ranger/TestRangerSubprocess.java index 667fa02..d856386 100644 --- a/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/ranger/TestRangerSubprocess.java +++ b/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/ranger/TestRangerSubprocess.java @@ -26,6 +26,14 @@ import java.util.List; import java.util.concurrent.TimeoutException; import com.google.protobuf.Any; +import org.apache.ranger.plugin.model.RangerServiceDef; +import org.apache.ranger.plugin.policyengine.RangerAccessRequestImpl; +import org.apache.ranger.plugin.policyengine.RangerAccessResult; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.mockito.Mockito; + import org.apache.kudu.ranger.Ranger.ActionPB; import org.apache.kudu.ranger.Ranger.RangerRequestListPB; import org.apache.kudu.ranger.Ranger.RangerRequestPB; @@ -35,13 +43,6 @@ import org.apache.kudu.subprocess.SubprocessExecutor; import org.apache.kudu.subprocess.SubprocessTestUtil; import org.apache.kudu.subprocess.ranger.authorization.RangerKuduAuthorizer; import org.apache.kudu.test.junit.RetryRule; -import org.apache.ranger.plugin.model.RangerServiceDef; -import org.apache.ranger.plugin.policyengine.RangerAccessRequestImpl; -import org.apache.ranger.plugin.policyengine.RangerAccessResult; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.mockito.Mockito; /** * Tests for the ranger subprocess. diff --git a/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/ranger/authorization/TestRangerKuduAuthorizer.java b/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/ranger/authorization/TestRangerKuduAuthorizer.java index e90c8ad..faa6f5f 100644 --- a/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/ranger/authorization/TestRangerKuduAuthorizer.java +++ b/java/kudu-subprocess/src/test/java/org/apache/kudu/subprocess/ranger/authorization/TestRangerKuduAuthorizer.java @@ -17,7 +17,13 @@ package org.apache.kudu.subprocess.ranger.authorization; -import org.apache.kudu.test.junit.RetryRule; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; + import org.apache.ranger.plugin.model.RangerServiceDef; import org.apache.ranger.plugin.policyengine.RangerAccessRequest; import org.apache.ranger.plugin.policyengine.RangerAccessRequestImpl; @@ -27,12 +33,7 @@ import org.junit.Rule; import org.junit.Test; import org.mockito.Mockito; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Iterator; - -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import org.apache.kudu.test.junit.RetryRule; /** * Tests for the Ranger authorizer.
