From a9b0d00852208ef2fac587633c58d561c30e1672 Mon Sep 17 00:00:00 2001 From: stair <123031771+stair-aws@users.noreply.github.com> Date: Tue, 27 Jun 2023 15:24:03 -0400 Subject: [PATCH] Checkstyle: added additional checks to, primarily, safeguard against bugs. (#1154) --- .../multilang/messages/MessageTest.java | 2 +- .../kinesis/retrieval/RetrievalConfig.java | 2 +- ...rdShardRecordProcessorCheckpointerTest.java | 11 ++++------- .../metrics/EndingMetricsScopeTest.java | 1 - .../retrieval/AWSExceptionManagerTest.java | 18 ++++++------------ .../retrieval/ThrottlingReporterTest.java | 3 --- checkstyle/checkstyle.xml | 7 +++++++ 7 files changed, 19 insertions(+), 25 deletions(-) diff --git a/amazon-kinesis-client-multilang/src/test/java/software/amazon/kinesis/multilang/messages/MessageTest.java b/amazon-kinesis-client-multilang/src/test/java/software/amazon/kinesis/multilang/messages/MessageTest.java index 86798080..62e5a741 100644 --- a/amazon-kinesis-client-multilang/src/test/java/software/amazon/kinesis/multilang/messages/MessageTest.java +++ b/amazon-kinesis-client-multilang/src/test/java/software/amazon/kinesis/multilang/messages/MessageTest.java @@ -49,7 +49,7 @@ public class MessageTest { new ProcessRecordsMessage(), new ShutdownRequestedMessage(), new LeaseLostMessage(), - new ShardEndedMessage() + new ShardEndedMessage(), }; // TODO: fix this diff --git a/amazon-kinesis-client/src/main/java/software/amazon/kinesis/retrieval/RetrievalConfig.java b/amazon-kinesis-client/src/main/java/software/amazon/kinesis/retrieval/RetrievalConfig.java index ea302b19..3cb4a0d9 100644 --- a/amazon-kinesis-client/src/main/java/software/amazon/kinesis/retrieval/RetrievalConfig.java +++ b/amazon-kinesis-client/src/main/java/software/amazon/kinesis/retrieval/RetrievalConfig.java @@ -152,7 +152,7 @@ public class RetrievalConfig { if (streamTracker().isMultiStream()) { throw new IllegalArgumentException( "Cannot set initialPositionInStreamExtended when multiStreamTracker is set"); - }; + } final StreamIdentifier streamIdentifier = getSingleStreamIdentifier(); final StreamConfig updatedConfig = new StreamConfig(streamIdentifier, initialPositionInStreamExtended); diff --git a/amazon-kinesis-client/src/test/java/software/amazon/kinesis/checkpoint/ShardShardRecordProcessorCheckpointerTest.java b/amazon-kinesis-client/src/test/java/software/amazon/kinesis/checkpoint/ShardShardRecordProcessorCheckpointerTest.java index 37a40b6b..98ce1dc5 100644 --- a/amazon-kinesis-client/src/test/java/software/amazon/kinesis/checkpoint/ShardShardRecordProcessorCheckpointerTest.java +++ b/amazon-kinesis-client/src/test/java/software/amazon/kinesis/checkpoint/ShardShardRecordProcessorCheckpointerTest.java @@ -397,7 +397,7 @@ public class ShardShardRecordProcessorCheckpointerTest { assertThat(checkpointer.largestPermittedCheckpointValue(), equalTo(sequenceNumber)); } - /* + /** * This test is a mixed test of checking some basic functionality of checkpointing at a sequence number and making * sure certain bounds checks and validations are being performed inside the checkpointer to prevent clients from * checkpointing out of order/too big/non-numeric values that aren't valid strings for them to be checkpointing @@ -444,7 +444,7 @@ public class ShardShardRecordProcessorCheckpointerTest { new ExtendedSequenceNumber("bogus-checkpoint-value"), // Can't checkpoint at non-numeric string ExtendedSequenceNumber.SHARD_END, // Can't go to the end unless it is set as the max ExtendedSequenceNumber.TRIM_HORIZON, // Can't go back to an initial sentinel value - ExtendedSequenceNumber.LATEST // Can't go back to an initial sentinel value + ExtendedSequenceNumber.LATEST, // Can't go back to an initial sentinel value }; for (ExtendedSequenceNumber badCheckpointValue : valuesWeShouldNotBeAbleToCheckpointAt) { try { @@ -477,7 +477,7 @@ public class ShardShardRecordProcessorCheckpointerTest { processingCheckpointer.lastCheckpointValue(), equalTo(ExtendedSequenceNumber.SHARD_END)); } - /* + /** * This test is a mixed test of checking some basic functionality of two phase checkpointing at a sequence number * and making sure certain bounds checks and validations are being performed inside the checkpointer to prevent * clients from checkpointing out of order/too big/non-numeric values that aren't valid strings for them to be @@ -548,7 +548,7 @@ public class ShardShardRecordProcessorCheckpointerTest { new ExtendedSequenceNumber("bogus-checkpoint-value"), // Can't checkpoint at non-numeric string ExtendedSequenceNumber.SHARD_END, // Can't go to the end unless it is set as the max ExtendedSequenceNumber.TRIM_HORIZON, // Can't go back to an initial sentinel value - ExtendedSequenceNumber.LATEST // Can't go back to an initial sentinel value + ExtendedSequenceNumber.LATEST, // Can't go back to an initial sentinel value }; for (ExtendedSequenceNumber badCheckpointValue : valuesWeShouldNotBeAbleToCheckpointAt) { try { @@ -566,7 +566,6 @@ public class ShardShardRecordProcessorCheckpointerTest { assertThat("Largest sequence number should not have changed", processingCheckpointer.largestPermittedCheckpointValue(), equalTo(thirdSequenceNumber)); assertThat(checkpoint.getCheckpointObject(shardId).pendingCheckpoint(), nullValue()); - } // advance to third number @@ -601,7 +600,6 @@ public class ShardShardRecordProcessorCheckpointerTest { * * @throws Exception */ - @SuppressWarnings("serial") @Test public final void testMixedCheckpointCalls() throws Exception { for (LinkedHashMap testPlan : getMixedCallsTestPlan()) { @@ -617,7 +615,6 @@ public class ShardShardRecordProcessorCheckpointerTest { * * @throws Exception */ - @SuppressWarnings("serial") @Test public final void testMixedTwoPhaseCheckpointCalls() throws Exception { for (LinkedHashMap testPlan : getMixedCallsTestPlan()) { diff --git a/amazon-kinesis-client/src/test/java/software/amazon/kinesis/metrics/EndingMetricsScopeTest.java b/amazon-kinesis-client/src/test/java/software/amazon/kinesis/metrics/EndingMetricsScopeTest.java index 2a32764d..a3d792ae 100644 --- a/amazon-kinesis-client/src/test/java/software/amazon/kinesis/metrics/EndingMetricsScopeTest.java +++ b/amazon-kinesis-client/src/test/java/software/amazon/kinesis/metrics/EndingMetricsScopeTest.java @@ -17,7 +17,6 @@ package software.amazon.kinesis.metrics; import org.junit.Test; import software.amazon.awssdk.services.cloudwatch.model.StandardUnit; -import software.amazon.kinesis.metrics.EndingMetricsScope; public class EndingMetricsScopeTest { diff --git a/amazon-kinesis-client/src/test/java/software/amazon/kinesis/retrieval/AWSExceptionManagerTest.java b/amazon-kinesis-client/src/test/java/software/amazon/kinesis/retrieval/AWSExceptionManagerTest.java index 8319a0ac..030979df 100644 --- a/amazon-kinesis-client/src/test/java/software/amazon/kinesis/retrieval/AWSExceptionManagerTest.java +++ b/amazon-kinesis-client/src/test/java/software/amazon/kinesis/retrieval/AWSExceptionManagerTest.java @@ -27,31 +27,28 @@ import static org.junit.Assert.assertThat; @Slf4j public class AWSExceptionManagerTest { + private static final String EXPECTED_HANDLING_MARKER = AWSExceptionManagerTest.class.getSimpleName(); + + private final AWSExceptionManager manager = new AWSExceptionManager(); + @Test public void testSpecificException() { - AWSExceptionManager manager = new AWSExceptionManager(); - final String EXPECTED_HANDLING_MARKER = "Handled-TestException"; - manager.add(TestException.class, t -> { log.info("Handling test exception: {} -> {}", t.getMessage(), t.getAdditionalMessage()); return new RuntimeException(EXPECTED_HANDLING_MARKER, t); }); - TestException te = new TestException("Main Mesage", "Sub Message"); - + TestException te = new TestException("Main Message", "Sub Message"); RuntimeException converted = manager.apply(te); assertThat(converted, isA(RuntimeException.class)); assertThat(converted.getMessage(), equalTo(EXPECTED_HANDLING_MARKER)); assertThat(converted.getCause(), equalTo(te)); - } @Test public void testParentException() { - AWSExceptionManager manager = new AWSExceptionManager(); - final String EXPECTED_HANDLING_MARKER = "Handled-IllegalStateException"; manager.add(IllegalArgumentException.class, i -> new RuntimeException("IllegalArgument", i)); manager.add(Exception.class, i -> new RuntimeException("RawException", i)); manager.add(IllegalStateException.class, i -> new RuntimeException(EXPECTED_HANDLING_MARKER, i)); @@ -66,8 +63,7 @@ public class AWSExceptionManagerTest { @Test public void testDefaultHandler() { - final String EXPECTED_HANDLING_MARKER = "Handled-Default"; - AWSExceptionManager manager = new AWSExceptionManager().defaultFunction(i -> new RuntimeException(EXPECTED_HANDLING_MARKER, i)); + manager.defaultFunction(i -> new RuntimeException(EXPECTED_HANDLING_MARKER, i)); manager.add(IllegalArgumentException.class, i -> new RuntimeException("IllegalArgument", i)); manager.add(Exception.class, i -> new RuntimeException("RawException", i)); @@ -83,8 +79,6 @@ public class AWSExceptionManagerTest { @Test public void testIdHandler() { - AWSExceptionManager manager = new AWSExceptionManager(); - manager.add(IllegalArgumentException.class, i -> new RuntimeException("IllegalArgument", i)); manager.add(Exception.class, i -> new RuntimeException("RawException", i)); manager.add(IllegalStateException.class, i -> i); diff --git a/amazon-kinesis-client/src/test/java/software/amazon/kinesis/retrieval/ThrottlingReporterTest.java b/amazon-kinesis-client/src/test/java/software/amazon/kinesis/retrieval/ThrottlingReporterTest.java index eec5ea9e..f13f0ad0 100644 --- a/amazon-kinesis-client/src/test/java/software/amazon/kinesis/retrieval/ThrottlingReporterTest.java +++ b/amazon-kinesis-client/src/test/java/software/amazon/kinesis/retrieval/ThrottlingReporterTest.java @@ -24,7 +24,6 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.slf4j.Logger; -import software.amazon.kinesis.retrieval.ThrottlingReporter; @RunWith(MockitoJUnitRunner.class) public class ThrottlingReporterTest { @@ -40,7 +39,6 @@ public class ThrottlingReporterTest { reporter.throttled(); verify(throttleLog).warn(anyString()); verify(throttleLog, never()).error(anyString()); - } @Test @@ -63,7 +61,6 @@ public class ThrottlingReporterTest { reporter.throttled(); verify(throttleLog, times(2)).warn(anyString()); verify(throttleLog, times(3)).error(anyString()); - } private class LogTestingThrottingReporter extends ThrottlingReporter { diff --git a/checkstyle/checkstyle.xml b/checkstyle/checkstyle.xml index 10031e23..76c4b330 100644 --- a/checkstyle/checkstyle.xml +++ b/checkstyle/checkstyle.xml @@ -23,8 +23,13 @@ + + + + + @@ -36,7 +41,9 @@ + +