From eb6fd0cf32056e6df2a76d5064ca60848e2aa5f4 Mon Sep 17 00:00:00 2001
From: stair <123031771+stair-aws@users.noreply.github.com>
Date: Fri, 23 Jun 2023 16:15:33 -0400
Subject: [PATCH] Bound Checkstyle to `validate` goal for automated
 enforcement. (#1149)

---
 .../lifecycle/ShardConsumerSubscriber.java    |  2 +
 .../fanout/FanOutRecordsPublisher.java        | 14 +++++++
 .../retrieval/polling/KinesisDataFetcher.java |  2 +
 checkstyle/checkstyle-suppressions.xml        |  8 ++++
 checkstyle/checkstyle.xml                     | 39 +++++++++++++++++++
 pom.xml                                       | 21 ++++++++++
 6 files changed, 86 insertions(+)
 create mode 100644 checkstyle/checkstyle-suppressions.xml
 create mode 100644 checkstyle/checkstyle.xml

diff --git a/amazon-kinesis-client/src/main/java/software/amazon/kinesis/lifecycle/ShardConsumerSubscriber.java b/amazon-kinesis-client/src/main/java/software/amazon/kinesis/lifecycle/ShardConsumerSubscriber.java
index 7a034000c..81528c4e3 100644
--- a/amazon-kinesis-client/src/main/java/software/amazon/kinesis/lifecycle/ShardConsumerSubscriber.java
+++ b/amazon-kinesis-client/src/main/java/software/amazon/kinesis/lifecycle/ShardConsumerSubscriber.java
@@ -130,7 +130,9 @@ private void restartIfRequestTimerExpired(long maxTimeBetweenRequests) {
                 Duration timeSinceLastResponse = Duration.between(lastRequestTime, now);
                 if (timeSinceLastResponse.toMillis() > maxTimeBetweenRequests) {
                     log.error(
+                            // CHECKSTYLE.OFF: LineLength
                             "{}: Last request was dispatched at {}, but no response as of {} ({}).  Cancelling subscription, and restarting. Last successful request details -- {}",
+                            // CHECKSTYLE.ON: LineLength
                             shardInfoId, lastRequestTime, now, timeSinceLastResponse, recordsPublisher.getLastSuccessfulRequestDetails());
                     cancel();
 
diff --git a/amazon-kinesis-client/src/main/java/software/amazon/kinesis/retrieval/fanout/FanOutRecordsPublisher.java b/amazon-kinesis-client/src/main/java/software/amazon/kinesis/retrieval/fanout/FanOutRecordsPublisher.java
index 3a8af8c2d..ca4ce12d9 100644
--- a/amazon-kinesis-client/src/main/java/software/amazon/kinesis/retrieval/fanout/FanOutRecordsPublisher.java
+++ b/amazon-kinesis-client/src/main/java/software/amazon/kinesis/retrieval/fanout/FanOutRecordsPublisher.java
@@ -369,7 +369,9 @@ private void errorOccurred(RecordFlow triggeringFlow, Throwable t) {
             } else {
                 if (triggeringFlow != null) {
                     log.debug(
+                            // CHECKSTYLE.OFF: LineLength
                             "{}: [SubscriptionLifetime] - (FanOutRecordsPublisher#errorOccurred) @ {} id: {} -- {} -> triggeringFlow wasn't the active flow.  Didn't dispatch error",
+                            // CHECKSTYLE.ON: LineLength
                             streamAndShardId, triggeringFlow.connectionStartedAt, triggeringFlow.subscribeToShardId,
                             category.throwableTypeString);
                     triggeringFlow.cancel();
@@ -605,7 +607,9 @@ public void request(long n) {
                     synchronized (lockObject) {
                         if (subscriber != s) {
                             log.warn(
+                                    // CHECKSTYLE.OFF: LineLength
                                     "{}: (FanOutRecordsPublisher/Subscription#request) - Rejected an attempt to request({}), because subscribers don't match. Last successful request details -- {}",
+                                    // CHECKSTYLE.ON: LineLength
                                     streamAndShardId, n, lastSuccessfulRequestDetails);
                             return;
                         }
@@ -632,13 +636,17 @@ public void cancel() {
                     synchronized (lockObject) {
                         if (subscriber != s) {
                             log.warn(
+                                    // CHECKSTYLE.OFF: LineLength
                                     "{}: (FanOutRecordsPublisher/Subscription#cancel) - Rejected attempt to cancel subscription, because subscribers don't match. Last successful request details -- {}",
+                                    // CHECKSTYLE.ON: LineLength
                                     streamAndShardId, lastSuccessfulRequestDetails);
                             return;
                         }
                         if (!hasValidSubscriber()) {
                             log.warn(
+                                    // CHECKSTYLE.OFF: LineLength
                                     "{}: (FanOutRecordsPublisher/Subscription#cancel) - Cancelled called even with an invalid subscriber. Last successful request details -- {}",
+                                    // CHECKSTYLE.ON: LineLength
                                     streamAndShardId, lastSuccessfulRequestDetails);
                         }
                         subscriber = null;
@@ -808,7 +816,9 @@ private void executeExceptionOccurred(Throwable throwable) {
                     isErrorDispatched = true;
                 } else {
                     log.debug(
+                            // CHECKSTYLE.OFF: LineLength
                             "{}: [SubscriptionLifetime]: (RecordFlow#exceptionOccurred) @ {} id: {} -- An error has previously been dispatched, not dispatching this error {}: {}",
+                            // CHECKSTYLE.OFF: LineLength
                             parent.streamAndShardId, connectionStartedAt, subscribeToShardId, throwable.getClass().getName(),
                             throwable.getMessage());
                 }
@@ -839,7 +849,9 @@ private void tryEnqueueSubscriptionShutdownEvent(SubscriptionShutdownEvent subsc
                         .add(new RecordsRetrievedContext(Either.right(subscriptionShutdownEvent), this, Instant.now()));
             } catch (Exception e) {
                 log.warn(
+                        // CHECKSTYLE.OFF: LineLength
                         "{}: Unable to enqueue the {} shutdown event due to capacity restrictions in delivery queue with remaining capacity {}. Ignoring. Last successful request details -- {}",
+                        // CHECKSTYLE.ON: LineLength
                         parent.streamAndShardId, subscriptionShutdownEvent.getEventIdentifier(), parent.recordsDeliveryQueue.remainingCapacity(),
                         parent.lastSuccessfulRequestDetails, subscriptionShutdownEvent.getShutdownEventThrowableOptional());
             }
@@ -863,7 +875,9 @@ private void executeComplete() {
                 }
                 if (this.isDisposed) {
                     log.warn(
+                            // CHECKSTYLE.OFF: LineLength
                             "{}: [SubscriptionLifetime]: (RecordFlow#complete) @ {} id: {} -- This flow has been disposed not dispatching completion. Last successful request details -- {}",
+                            // CHECKSTYLE.ON: LineLength
                             parent.streamAndShardId, connectionStartedAt, subscribeToShardId, parent.lastSuccessfulRequestDetails);
                     return;
                 }
diff --git a/amazon-kinesis-client/src/main/java/software/amazon/kinesis/retrieval/polling/KinesisDataFetcher.java b/amazon-kinesis-client/src/main/java/software/amazon/kinesis/retrieval/polling/KinesisDataFetcher.java
index 65da2b329..495dcfb1b 100644
--- a/amazon-kinesis-client/src/main/java/software/amazon/kinesis/retrieval/polling/KinesisDataFetcher.java
+++ b/amazon-kinesis-client/src/main/java/software/amazon/kinesis/retrieval/polling/KinesisDataFetcher.java
@@ -145,7 +145,9 @@ public DataFetcherResult getRecords() {
         }
     }
 
+    // CHECKSTYLE.OFF: MemberName
     final DataFetcherResult TERMINAL_RESULT = new DataFetcherResult() {
+    // CHECKSTYLE.ON: MemberName
         @Override
         public GetRecordsResponse getResult() {
             return GetRecordsResponse.builder()
diff --git a/checkstyle/checkstyle-suppressions.xml b/checkstyle/checkstyle-suppressions.xml
new file mode 100644
index 000000000..91be49c89
--- /dev/null
+++ b/checkstyle/checkstyle-suppressions.xml
@@ -0,0 +1,8 @@
+<!DOCTYPE suppressions PUBLIC
+        "-//Puppy Crawl//DTD Suppressions 1.1//EN"
+        "http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">
+
+<suppressions>
+    <!-- Disable all checks for protobuf-generated files. -->
+    <suppress files=".*/kpl/Messages.java" checks="[a-zA-Z0-9]*"/>
+</suppressions>
\ No newline at end of file
diff --git a/checkstyle/checkstyle.xml b/checkstyle/checkstyle.xml
new file mode 100644
index 000000000..07932723f
--- /dev/null
+++ b/checkstyle/checkstyle.xml
@@ -0,0 +1,39 @@
+<?xml version="1.0"?>
+<!DOCTYPE module PUBLIC
+        "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
+        "https://checkstyle.org/dtds/configuration_1_3.dtd">
+
+<module name="Checker">
+    <module name="FileTabCharacter">
+        <property name="eachLine" value="true"/>
+    </module>
+
+    <module name="LineLength">
+        <property name="fileExtensions" value="java"/>
+        <property name="max" value="170"/>
+        <property name="ignorePattern" value="^package.*|^import.*|a href|href|http://|https://|ftp://"/>
+    </module>
+
+    <module name="SuppressWithPlainTextCommentFilter">
+        <property name="offCommentFormat" value="CHECKSTYLE.OFF\: ([\w\|]+)"/>
+        <property name="onCommentFormat" value="CHECKSTYLE.ON\: ([\w\|]+)"/>
+        <!-- $1 refers to the first match group in the regex defined in commentFormat -->
+        <property name="checkFormat" value="$1"/>
+    </module>
+
+    <module name="TreeWalker">
+        <module name="AvoidStarImport"/>
+        <module name="ConstantName"/>
+        <module name="InvalidJavadocPosition"/>
+        <module name="LocalVariableName"/>
+        <module name="MemberName"/>
+        <module name="MethodName"/>
+        <module name="NeedBraces"/>
+        <module name="OneStatementPerLine"/>
+        <module name="OneTopLevelClass"/>
+        <module name="OuterTypeFilename"/>
+        <module name="ParameterName"/>
+        <module name="WhitespaceAfter"/>
+    </module>
+
+</module>
diff --git a/pom.xml b/pom.xml
index 7ebd6e54e..82d7857d0 100644
--- a/pom.xml
+++ b/pom.xml
@@ -72,6 +72,27 @@
   </distributionManagement>
 
   <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-checkstyle-plugin</artifactId>
+        <version>3.3.0</version>
+        <configuration>
+          <configLocation>checkstyle/checkstyle.xml</configLocation>
+          <consoleOutput>true</consoleOutput>
+          <failOnViolation>true</failOnViolation>
+          <suppressionsLocation>checkstyle/checkstyle-suppressions.xml</suppressionsLocation>
+        </configuration>
+        <executions>
+          <execution>
+            <phase>validate</phase>
+            <goals>
+              <goal>check</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
     <pluginManagement>
       <plugins>
         <plugin>