Apply more Best Practices PMD rules (#14772)

* implement PMD rules: * AbstractClassWithoutAbstractMethod * ArrayIsStoredDirectly * AvoidPrintStackTrace, AvoidReassigningLoopVariables, AvoidReassigningParameters, AvoidUsingHardCodedIP and CheckResultSet * DoubleBraceInitialization * MissingOverride * ForLoopCanBeForeach *JUnitTest rules
airbytehq · Jul 21, 2022 · bdd307c · bdd307c
1 parent a3131e8
commit bdd307c
Show file tree

Hide file tree

Showing 148 changed files with 825 additions and 752 deletions.
diff --git a/airbyte-bootloader/src/test/java/io/airbyte/bootloader/BootloaderAppTest.java b/airbyte-bootloader/src/test/java/io/airbyte/bootloader/BootloaderAppTest.java
@@ -52,8 +52,9 @@
 import uk.org.webcompere.systemstubs.jupiter.SystemStub;
 import uk.org.webcompere.systemstubs.jupiter.SystemStubsExtension;
 
+@SuppressWarnings("PMD.AvoidUsingHardCodedIP")
 @ExtendWith(SystemStubsExtension.class)
-public class BootloaderAppTest {
+class BootloaderAppTest {
 
   private PostgreSQLContainer container;
   private DataSource configsDataSource;

diff --git a/airbyte-bootloader/src/test/java/io/airbyte/bootloader/SecretMigratorTest.java b/airbyte-bootloader/src/test/java/io/airbyte/bootloader/SecretMigratorTest.java
@@ -34,7 +34,7 @@
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @ExtendWith(MockitoExtension.class)
-public class SecretMigratorTest {
+class SecretMigratorTest {
 
   private final UUID workspaceId = UUID.randomUUID();
 
@@ -55,7 +55,7 @@ void setup() {
   }
 
   @Test
-  public void testMigrateSecret() throws JsonValidationException, IOException {
+  void testMigrateSecret() throws JsonValidationException, IOException {
     final JsonNode sourceSpec = Jsons.jsonNode("sourceSpec");
     final UUID sourceDefinitionId = UUID.randomUUID();
     final StandardSourceDefinition standardSourceDefinition = new StandardSourceDefinition()

diff --git a/airbyte-commons/src/main/java/io/airbyte/commons/concurrency/GracefulShutdownHandler.java b/airbyte-commons/src/main/java/io/airbyte/commons/concurrency/GracefulShutdownHandler.java
@@ -18,7 +18,7 @@ public class GracefulShutdownHandler extends Thread {
 
   public GracefulShutdownHandler(final Duration terminateWaitDuration, final ExecutorService... threadPools) {
     this.terminateWaitDuration = terminateWaitDuration;
-    this.threadPools = threadPools;
+    this.threadPools = threadPools.clone();
   }
 
   @Override

diff --git a/airbyte-commons/src/main/java/io/airbyte/commons/json/JsonSchemas.java b/airbyte-commons/src/main/java/io/airbyte/commons/json/JsonSchemas.java
@@ -167,6 +167,7 @@ public static List<List<FieldNameOrList>> collectPathsThatMeetCondition(final Js
    *        the node from the root of the object passed at the root level invocation
    *
    */
+  @SuppressWarnings("PMD.ForLoopCanBeForeach")
   private static void traverseJsonSchemaInternal(final JsonNode jsonSchemaNode,
                                                  final List<FieldNameOrList> path,
                                                  final BiConsumer<JsonNode, List<FieldNameOrList>> consumer) {

diff --git a/airbyte-commons/src/main/java/io/airbyte/commons/json/Jsons.java b/airbyte-commons/src/main/java/io/airbyte/commons/json/Jsons.java
@@ -34,6 +34,7 @@
 import java.util.function.BiConsumer;
 import java.util.stream.Collectors;
 
+@SuppressWarnings("PMD.AvoidReassigningParameters")
 public class Jsons {
 
   // Object Mapper is thread-safe
@@ -222,6 +223,7 @@ public static int getIntOrZero(final JsonNode json, final List<String> keys) {
   /**
    * Flattens an ObjectNode, or dumps it into a {null: value} map if it's not an object.
    */
+  @SuppressWarnings("PMD.ForLoopCanBeForeach")
   public static Map<String, Object> flatten(final JsonNode node) {
     if (node.isObject()) {
       final Map<String, Object> output = new HashMap<>();

diff --git a/...yte-commons/src/test/java/io/airbyte/commons/concurrency/GracefulShutdownHandlerTest.java b/...yte-commons/src/test/java/io/airbyte/commons/concurrency/GracefulShutdownHandlerTest.java
@@ -14,7 +14,7 @@
 class GracefulShutdownHandlerTest {
 
   @Test
-  public void testRun() throws InterruptedException {
+  void testRun() throws InterruptedException {
     final ExecutorService executorService = mock(ExecutorService.class);
     final GracefulShutdownHandler gracefulShutdownHandler = new GracefulShutdownHandler(Duration.ofSeconds(30), executorService);
     gracefulShutdownHandler.start();

diff --git a/airbyte-commons/src/test/java/io/airbyte/commons/enums/EnumsTest.java b/airbyte-commons/src/test/java/io/airbyte/commons/enums/EnumsTest.java
@@ -34,12 +34,12 @@ enum E4 {
   }
 
   @Test
-  public void testConversion() {
+  void testConversion() {
     Assertions.assertEquals(E2.TEST, convertTo(E1.TEST, E2.class));
   }
 
   @Test
-  public void testConversionFails() {
+  void testConversionFails() {
     Assertions.assertThrows(IllegalArgumentException.class, () -> convertTo(E1.TEST2, E2.class));
   }
 

diff --git a/airbyte-commons/src/test/java/io/airbyte/commons/io/IOsTest.java b/airbyte-commons/src/test/java/io/airbyte/commons/io/IOsTest.java
@@ -47,20 +47,20 @@ void testWriteBytes() throws IOException {
   }
 
   @Test
-  public void testWriteFileToRandomDir() throws IOException {
+  void testWriteFileToRandomDir() throws IOException {
     final String contents = "something to remember";
     final String tmpFilePath = IOs.writeFileToRandomTmpDir("file.txt", contents);
     assertEquals(contents, Files.readString(Path.of(tmpFilePath)));
   }
 
   @Test
-  public void testGetTailDoesNotExist() throws IOException {
+  void testGetTailDoesNotExist() throws IOException {
     final List<String> tail = IOs.getTail(100, Path.of(RandomStringUtils.randomAlphanumeric(100)));
     assertEquals(Collections.emptyList(), tail);
   }
 
   @Test
-  public void testGetTailExists() throws IOException {
+  void testGetTailExists() throws IOException {
     final Path stdoutFile = Files.createTempFile("job-history-handler-test", "stdout");
 
     final List<String> head = List.of(

diff --git a/airbyte-commons/src/test/java/io/airbyte/commons/json/JsonSchemasTest.java b/airbyte-commons/src/test/java/io/airbyte/commons/json/JsonSchemasTest.java
@@ -21,6 +21,7 @@
 import org.mockito.InOrder;
 import org.mockito.Mockito;
 
+@SuppressWarnings("PMD.JUnitTestsShouldIncludeAssert")
 class JsonSchemasTest {
 
   @Test

diff --git a/airbyte-commons/src/test/java/io/airbyte/commons/lang/CloseableShutdownHookTest.java b/airbyte-commons/src/test/java/io/airbyte/commons/lang/CloseableShutdownHookTest.java
@@ -11,7 +11,7 @@
 import java.io.InputStream;
 import org.junit.jupiter.api.Test;
 
-public class CloseableShutdownHookTest {
+class CloseableShutdownHookTest {
 
   @Test
   void testRegisteringShutdownHook() throws Exception {

diff --git a/airbyte-commons/src/test/java/io/airbyte/commons/lang/ExceptionsTest.java b/airbyte-commons/src/test/java/io/airbyte/commons/lang/ExceptionsTest.java
@@ -12,6 +12,7 @@
 import java.util.List;
 import org.junit.jupiter.api.Test;
 
+@SuppressWarnings("PMD.JUnitTestsShouldIncludeAssert")
 class ExceptionsTest {
 
   @Test

diff --git a/airbyte-commons/src/test/java/io/airbyte/commons/logging/Log4j2ConfigTest.java b/airbyte-commons/src/test/java/io/airbyte/commons/logging/Log4j2ConfigTest.java
@@ -20,7 +20,7 @@
 import org.slf4j.LoggerFactory;
 import org.slf4j.MDC;
 
-public class Log4j2ConfigTest {
+class Log4j2ConfigTest {
 
   private static final Path TEST_ROOT = Path.of("/tmp/airbyte_tests");
   private Path root;

diff --git a/airbyte-commons/src/test/java/io/airbyte/commons/logging/MdcScopeTest.java b/airbyte-commons/src/test/java/io/airbyte/commons/logging/MdcScopeTest.java
@@ -4,62 +4,38 @@
 
 package io.airbyte.commons.logging;
 
-import java.util.HashMap;
 import java.util.Map;
 import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 import org.slf4j.MDC;
 
-public class MdcScopeTest {
+class MdcScopeTest {
 
-  private static final Map<String, String> originalMap = new HashMap<>() {
+  private static final Map<String, String> originalMap = Map.of("test", "entry", "testOverride", "should be overrided");
 
-    {
-      put("test", "entry");
-      put("testOverride", "should be overrided");
-    }
-
-  };
-
-  private static final Map<String, String> modificationInMDC = new HashMap<>() {
-
-    {
-      put("new", "will be added");
-      put("testOverride", "will override");
-    }
-
-  };
+  private static final Map<String, String> modificationInMDC = Map.of("new", "will be added", "testOverride", "will override");
 
   @BeforeEach
-  public void init() {
+  void init() {
     MDC.setContextMap(originalMap);
   }
 
   @Test
   @DisplayName("The MDC context is properly overrided")
-  public void testMDCModified() {
+  void testMDCModified() {
     try (final MdcScope mdcScope = new MdcScope(modificationInMDC)) {
       final Map<String, String> mdcState = MDC.getCopyOfContextMap();
 
       Assertions.assertThat(mdcState).containsExactlyInAnyOrderEntriesOf(
-          new HashMap<String, String>() {
-
-            {
-              put("test", "entry");
-              put("new", "will be added");
-              put("testOverride", "will override");
-            }
-
-          });
-
+          Map.of("test", "entry", "new", "will be added", "testOverride", "will override"));
     }
   }
 
   @Test
   @DisplayName("The MDC context is properly restored")
-  public void testMDCRestore() {
+  void testMDCRestore() {
     try (final MdcScope mdcScope = new MdcScope(modificationInMDC)) {}
 
     final Map<String, String> mdcState = MDC.getCopyOfContextMap();

diff --git a/airbyte-commons/src/test/java/io/airbyte/commons/text/NamesTest.java b/airbyte-commons/src/test/java/io/airbyte/commons/text/NamesTest.java
@@ -9,7 +9,7 @@
 
 import org.junit.jupiter.api.Test;
 
-public class NamesTest {
+class NamesTest {
 
   @Test
   void testToAlphanumericAndUnderscore() {

diff --git a/airbyte-commons/src/test/java/io/airbyte/commons/version/AirbyteVersionTest.java b/airbyte-commons/src/test/java/io/airbyte/commons/version/AirbyteVersionTest.java
@@ -11,26 +11,26 @@
 
 import org.junit.jupiter.api.Test;
 
-public class AirbyteVersionTest {
+class AirbyteVersionTest {
 
   @Test
-  public void testParseVersion() {
+  void testParseVersion() {
     final AirbyteVersion version = new AirbyteVersion("6.7.8");
     assertEquals("6", version.getMajorVersion());
     assertEquals("7", version.getMinorVersion());
     assertEquals("8", version.getPatchVersion());
   }
 
   @Test
-  public void testParseVersionWithLabel() {
+  void testParseVersionWithLabel() {
     final AirbyteVersion version = new AirbyteVersion("6.7.8-omega");
     assertEquals("6", version.getMajorVersion());
     assertEquals("7", version.getMinorVersion());
     assertEquals("8", version.getPatchVersion());
   }
 
   @Test
-  public void testCompatibleVersionCompareTo() {
+  void testCompatibleVersionCompareTo() {
     assertEquals(0, new AirbyteVersion("6.7.8-omega").compatibleVersionCompareTo(new AirbyteVersion("6.7.8-gamma")));
     assertEquals(0, new AirbyteVersion("6.7.8-alpha").compatibleVersionCompareTo(new AirbyteVersion("6.7.9-alpha")));
     assertTrue(0 < new AirbyteVersion("6.8.0-alpha").compatibleVersionCompareTo(new AirbyteVersion("6.7.8-alpha")));
@@ -42,7 +42,7 @@ public void testCompatibleVersionCompareTo() {
   }
 
   @Test
-  public void testPatchVersionCompareTo() {
+  void testPatchVersionCompareTo() {
     assertEquals(0, new AirbyteVersion("6.7.8-omega").patchVersionCompareTo(new AirbyteVersion("6.7.8-gamma")));
     assertTrue(0 > new AirbyteVersion("6.7.8-alpha").patchVersionCompareTo(new AirbyteVersion("6.7.9-alpha")));
     assertTrue(0 > new AirbyteVersion("6.7.8-alpha").patchVersionCompareTo(new AirbyteVersion("6.7.11-alpha")));
@@ -55,7 +55,7 @@ public void testPatchVersionCompareTo() {
   }
 
   @Test
-  public void testGreaterThan() {
+  void testGreaterThan() {
     assertFalse(new AirbyteVersion("6.7.8-omega").greaterThan(new AirbyteVersion("6.7.8-gamma")));
     assertFalse(new AirbyteVersion("6.7.8-alpha").greaterThan(new AirbyteVersion("6.7.9-alpha")));
     assertFalse(new AirbyteVersion("6.7.8-alpha").greaterThan(new AirbyteVersion("6.7.11-alpha")));
@@ -68,7 +68,7 @@ public void testGreaterThan() {
   }
 
   @Test
-  public void testLessThan() {
+  void testLessThan() {
     assertFalse(new AirbyteVersion("6.7.8-omega").lessThan(new AirbyteVersion("6.7.8-gamma")));
     assertTrue(new AirbyteVersion("6.7.8-alpha").lessThan(new AirbyteVersion("6.7.9-alpha")));
     assertTrue(new AirbyteVersion("6.7.8-alpha").lessThan(new AirbyteVersion("6.7.11-alpha")));
@@ -81,13 +81,13 @@ public void testLessThan() {
   }
 
   @Test
-  public void testInvalidVersions() {
+  void testInvalidVersions() {
     assertThrows(NullPointerException.class, () -> new AirbyteVersion(null));
     assertThrows(IllegalArgumentException.class, () -> new AirbyteVersion("0.6"));
   }
 
   @Test
-  public void testSerialize() {
+  void testSerialize() {
     final var devVersion = "dev";
     assertEquals(devVersion, new AirbyteVersion(devVersion).serialize());
 
@@ -96,13 +96,13 @@ public void testSerialize() {
   }
 
   @Test
-  public void testCheckVersion() {
+  void testCheckVersion() {
     AirbyteVersion.assertIsCompatible(new AirbyteVersion("3.2.1"), new AirbyteVersion("3.2.1"));
     assertThrows(IllegalStateException.class, () -> AirbyteVersion.assertIsCompatible(new AirbyteVersion("1.2.3"), new AirbyteVersion("3.2.1")));
   }
 
   @Test
-  public void testCheckOnlyPatchVersion() {
+  void testCheckOnlyPatchVersion() {
     assertFalse(new AirbyteVersion("6.7.8").checkOnlyPatchVersionIsUpdatedComparedTo(new AirbyteVersion("6.7.8")));
     assertFalse(new AirbyteVersion("6.9.8").checkOnlyPatchVersionIsUpdatedComparedTo(new AirbyteVersion("6.8.9")));
     assertFalse(new AirbyteVersion("7.7.8").checkOnlyPatchVersionIsUpdatedComparedTo(new AirbyteVersion("6.7.11")));

diff --git a/airbyte-config/config-models/src/main/java/io/airbyte/config/EnvConfigs.java b/airbyte-config/config-models/src/main/java/io/airbyte/config/EnvConfigs.java
@@ -710,6 +710,7 @@ public String getMetricClient() {
     return getEnvOrDefault(METRIC_CLIENT, "");
   }
 
+  @Override
   public String getOtelCollectorEndpoint() {
     return getEnvOrDefault(OTEL_COLLECTOR_ENDPOINT, "");
   }