Error Prone PMD rules

airbyte-analytics/src/test/java/io/airbyte/analytics/SegmentTrackingClientTest.java

@@ -35,6 +35,8 @@ class SegmentTrackingClientTest {
   private static final TrackingIdentity IDENTITY = new TrackingIdentity(AIRBYTE_VERSION, UUID.randomUUID(), EMAIL, false, false, true);
   private static final UUID WORKSPACE_ID = UUID.randomUUID();
   private static final Function<UUID, TrackingIdentity> MOCK_TRACKING_IDENTITY = (workspaceId) -> IDENTITY;
+  private static final String AIRBYTE_VERSION_KEY = "airbyte_version";
+  private static final String JUMP = "jump";
 
   private Analytics analytics;
   private SegmentTrackingClient segmentTrackingClient;
@@ -61,7 +63,7 @@ void testIdentify() {
     final IdentifyMessage actual = mockBuilder.getValue().build();
     final Map<String, Object> expectedTraits = ImmutableMap.<String, Object>builder()
         .put("anonymized", IDENTITY.isAnonymousDataCollection())
-        .put("airbyte_version", AIRBYTE_VERSION.serialize())
+        .put(AIRBYTE_VERSION_KEY, AIRBYTE_VERSION.serialize())
         .put("deployment_env", DEPLOYMENT.getDeploymentEnv())
         .put("deployment_mode", DEPLOYMENT.getDeploymentMode())
         .put("deployment_id", DEPLOYMENT.getDeploymentId())
@@ -87,7 +89,7 @@ void testIdentifyWithRole() {
     final IdentifyMessage actual = mockBuilder.getValue().build();
     final Map<String, Object> expectedTraits = ImmutableMap.<String, Object>builder()
         .put("airbyte_role", "role")
-        .put("airbyte_version", AIRBYTE_VERSION.serialize())
+        .put(AIRBYTE_VERSION_KEY, AIRBYTE_VERSION.serialize())
         .put("anonymized", IDENTITY.isAnonymousDataCollection())
         .put("deployment_env", DEPLOYMENT.getDeploymentEnv())
         .put("deployment_mode", DEPLOYMENT.getDeploymentMode())
@@ -104,13 +106,13 @@ void testIdentifyWithRole() {
   void testTrack() {
     final ArgumentCaptor<TrackMessage.Builder> mockBuilder = ArgumentCaptor.forClass(TrackMessage.Builder.class);
     final ImmutableMap<String, Object> metadata =
-        ImmutableMap.of("airbyte_version", AIRBYTE_VERSION.serialize(), "user_id", IDENTITY.getCustomerId());
+        ImmutableMap.of(AIRBYTE_VERSION_KEY, AIRBYTE_VERSION.serialize(), "user_id", IDENTITY.getCustomerId());
 
-    segmentTrackingClient.track(WORKSPACE_ID, "jump");
+    segmentTrackingClient.track(WORKSPACE_ID, JUMP);
 
     verify(analytics).enqueue(mockBuilder.capture());
     final TrackMessage actual = mockBuilder.getValue().build();
-    assertEquals("jump", actual.event());
+    assertEquals(JUMP, actual.event());
     assertEquals(IDENTITY.getCustomerId().toString(), actual.userId());
     assertEquals(metadata, filterTrackedAtProperty(Objects.requireNonNull(actual.properties())));
   }
@@ -119,16 +121,16 @@ void testTrack() {
   void testTrackWithMetadata() {
     final ArgumentCaptor<TrackMessage.Builder> mockBuilder = ArgumentCaptor.forClass(TrackMessage.Builder.class);
     final ImmutableMap<String, Object> metadata = ImmutableMap.of(
-        "airbyte_version", AIRBYTE_VERSION.serialize(),
+        AIRBYTE_VERSION_KEY, AIRBYTE_VERSION.serialize(),
         "email", EMAIL,
         "height", "80 meters",
         "user_id", IDENTITY.getCustomerId());
 
-    segmentTrackingClient.track(WORKSPACE_ID, "jump", metadata);
+    segmentTrackingClient.track(WORKSPACE_ID, JUMP, metadata);
 
     verify(analytics).enqueue(mockBuilder.capture());
     final TrackMessage actual = mockBuilder.getValue().build();
-    assertEquals("jump", actual.event());
+    assertEquals(JUMP, actual.event());
     assertEquals(IDENTITY.getCustomerId().toString(), actual.userId());
     assertEquals(metadata, filterTrackedAtProperty(Objects.requireNonNull(actual.properties())));
   }

airbyte-bootloader/src/test/java/io/airbyte/bootloader/BootloaderAppTest.java

@@ -59,13 +59,18 @@ class BootloaderAppTest {
   private PostgreSQLContainer container;
   private DataSource configsDataSource;
   private DataSource jobsDataSource;
+  private static final String DOCKER = "docker";
+  private static final String VERSION_0330_ALPHA = "0.33.0-alpha";
+  private static final String VERSION_0320_ALPHA = "0.32.0-alpha";
+  private static final String VERSION_0321_ALPHA = "0.32.1-alpha";
+  private static final String VERSION_0170_ALPHA = "0.17.0-alpha";
 
   @BeforeEach
   void setup() {
     container = new PostgreSQLContainer<>("postgres:13-alpine")
         .withDatabaseName("public")
-        .withUsername("docker")
-        .withPassword("docker");
+        .withUsername(DOCKER)
+        .withPassword(DOCKER);
     container.start();
 
     configsDataSource =
@@ -86,16 +91,14 @@ void cleanup() throws Exception {
 
   @Test
   void testBootloaderAppBlankDb() throws Exception {
-    val version = "0.33.0-alpha";
-
     val mockedConfigs = mock(Configs.class);
     when(mockedConfigs.getConfigDatabaseUrl()).thenReturn(container.getJdbcUrl());
     when(mockedConfigs.getConfigDatabaseUser()).thenReturn(container.getUsername());
     when(mockedConfigs.getConfigDatabasePassword()).thenReturn(container.getPassword());
     when(mockedConfigs.getDatabaseUrl()).thenReturn(container.getJdbcUrl());
     when(mockedConfigs.getDatabaseUser()).thenReturn(container.getUsername());
     when(mockedConfigs.getDatabasePassword()).thenReturn(container.getPassword());
-    when(mockedConfigs.getAirbyteVersion()).thenReturn(new AirbyteVersion(version));
+    when(mockedConfigs.getAirbyteVersion()).thenReturn(new AirbyteVersion(VERSION_0330_ALPHA));
     when(mockedConfigs.runDatabaseMigrationOnStartup()).thenReturn(true);
     when(mockedConfigs.getConfigsDatabaseInitializationTimeoutMs()).thenReturn(60000L);
     when(mockedConfigs.getJobsDatabaseInitializationTimeoutMs()).thenReturn(60000L);
@@ -107,8 +110,8 @@ void testBootloaderAppBlankDb() throws Exception {
     // Although we are able to inject mocked configs into the Bootloader, a particular migration in the
     // configs database
     // requires the env var to be set. Flyway prevents injection, so we dynamically set this instead.
-    environmentVariables.set("DATABASE_USER", "docker");
-    environmentVariables.set("DATABASE_PASSWORD", "docker");
+    environmentVariables.set("DATABASE_USER", DOCKER);
+    environmentVariables.set("DATABASE_PASSWORD", DOCKER);
     environmentVariables.set("DATABASE_URL", container.getJdbcUrl());
 
     try (val configsDslContext = DSLContextFactory.create(configsDataSource, SQLDialect.POSTGRES);
@@ -133,24 +136,22 @@ void testBootloaderAppBlankDb() throws Exception {
       assertEquals("0.39.17.001", configsMigrator.getLatestMigration().getVersion().getVersion());
 
       val jobsPersistence = new DefaultJobPersistence(jobDatabase);
-      assertEquals(version, jobsPersistence.getVersion().get());
+      assertEquals(VERSION_0330_ALPHA, jobsPersistence.getVersion().get());
 
       assertNotEquals(Optional.empty(), jobsPersistence.getDeployment().get());
     }
   }
 
   @Test
   void testBootloaderAppRunSecretMigration() throws Exception {
-    val version = "0.33.0-alpha";
-
     val mockedConfigs = mock(Configs.class);
     when(mockedConfigs.getConfigDatabaseUrl()).thenReturn(container.getJdbcUrl());
     when(mockedConfigs.getConfigDatabaseUser()).thenReturn(container.getUsername());
     when(mockedConfigs.getConfigDatabasePassword()).thenReturn(container.getPassword());
     when(mockedConfigs.getDatabaseUrl()).thenReturn(container.getJdbcUrl());
     when(mockedConfigs.getDatabaseUser()).thenReturn(container.getUsername());
     when(mockedConfigs.getDatabasePassword()).thenReturn(container.getPassword());
-    when(mockedConfigs.getAirbyteVersion()).thenReturn(new AirbyteVersion(version));
+    when(mockedConfigs.getAirbyteVersion()).thenReturn(new AirbyteVersion(VERSION_0330_ALPHA));
     when(mockedConfigs.runDatabaseMigrationOnStartup()).thenReturn(true);
     when(mockedConfigs.getSecretPersistenceType()).thenReturn(TESTING_CONFIG_DB_TABLE);
     when(mockedConfigs.getConfigsDatabaseInitializationTimeoutMs()).thenReturn(60000L);
@@ -181,8 +182,8 @@ void testBootloaderAppRunSecretMigration() throws Exception {
       // Although we are able to inject mocked configs into the Bootloader, a particular migration in the
       // configs database requires the env var to be set. Flyway prevents injection, so we dynamically set
       // this instead.
-      environmentVariables.set("DATABASE_USER", "docker");
-      environmentVariables.set("DATABASE_PASSWORD", "docker");
+      environmentVariables.set("DATABASE_USER", DOCKER);
+      environmentVariables.set("DATABASE_PASSWORD", DOCKER);
       environmentVariables.set("DATABASE_URL", container.getJdbcUrl());
 
       // Bootstrap the database for the test
@@ -265,37 +266,35 @@ void testBootloaderAppRunSecretMigration() throws Exception {
   void testIsLegalUpgradePredicate() {
     // starting from no previous version is always legal.
     assertTrue(BootloaderApp.isLegalUpgrade(null, new AirbyteVersion("0.17.1-alpha")));
-    assertTrue(BootloaderApp.isLegalUpgrade(null, new AirbyteVersion("0.32.0-alpha")));
-    assertTrue(BootloaderApp.isLegalUpgrade(null, new AirbyteVersion("0.32.1-alpha")));
+    assertTrue(BootloaderApp.isLegalUpgrade(null, new AirbyteVersion(VERSION_0320_ALPHA)));
+    assertTrue(BootloaderApp.isLegalUpgrade(null, new AirbyteVersion(VERSION_0321_ALPHA)));
     assertTrue(BootloaderApp.isLegalUpgrade(null, new AirbyteVersion("0.33.1-alpha")));
     // starting from a version that is pre-breaking migration cannot go past the breaking migration.
-    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion("0.17.0-alpha"), new AirbyteVersion("0.17.1-alpha")));
-    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion("0.17.0-alpha"), new AirbyteVersion("0.18.0-alpha")));
-    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion("0.17.0-alpha"), new AirbyteVersion("0.32.0-alpha")));
-    assertFalse(BootloaderApp.isLegalUpgrade(new AirbyteVersion("0.17.0-alpha"), new AirbyteVersion("0.32.1-alpha")));
-    assertFalse(BootloaderApp.isLegalUpgrade(new AirbyteVersion("0.17.0-alpha"), new AirbyteVersion("0.33.0-alpha")));
+    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion(VERSION_0170_ALPHA), new AirbyteVersion("0.17.1-alpha")));
+    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion(VERSION_0170_ALPHA), new AirbyteVersion("0.18.0-alpha")));
+    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion(VERSION_0170_ALPHA), new AirbyteVersion(VERSION_0320_ALPHA)));
+    assertFalse(BootloaderApp.isLegalUpgrade(new AirbyteVersion(VERSION_0170_ALPHA), new AirbyteVersion(VERSION_0321_ALPHA)));
+    assertFalse(BootloaderApp.isLegalUpgrade(new AirbyteVersion(VERSION_0170_ALPHA), new AirbyteVersion(VERSION_0330_ALPHA)));
     // any migration starting at the breaking migration or after it can upgrade to anything.
-    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion("0.32.0-alpha"), new AirbyteVersion("0.32.1-alpha")));
-    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion("0.32.0-alpha"), new AirbyteVersion("0.33.0-alpha")));
-    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion("0.32.1-alpha"), new AirbyteVersion("0.32.1-alpha")));
-    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion("0.32.1-alpha"), new AirbyteVersion("0.33.0-alpha")));
-    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion("0.33.0-alpha"), new AirbyteVersion("0.33.1-alpha")));
-    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion("0.33.0-alpha"), new AirbyteVersion("0.34.0-alpha")));
+    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion(VERSION_0320_ALPHA), new AirbyteVersion(VERSION_0321_ALPHA)));
+    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion(VERSION_0320_ALPHA), new AirbyteVersion(VERSION_0330_ALPHA)));
+    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion(VERSION_0321_ALPHA), new AirbyteVersion(VERSION_0321_ALPHA)));
+    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion(VERSION_0321_ALPHA), new AirbyteVersion(VERSION_0330_ALPHA)));
+    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion(VERSION_0330_ALPHA), new AirbyteVersion("0.33.1-alpha")));
+    assertTrue(BootloaderApp.isLegalUpgrade(new AirbyteVersion(VERSION_0330_ALPHA), new AirbyteVersion("0.34.0-alpha")));
   }
 
   @Test
   void testPostLoadExecutionExecutes() throws Exception {
     final var testTriggered = new AtomicBoolean();
-    val version = "0.33.0-alpha";
-
     val mockedConfigs = mock(Configs.class);
     when(mockedConfigs.getConfigDatabaseUrl()).thenReturn(container.getJdbcUrl());
     when(mockedConfigs.getConfigDatabaseUser()).thenReturn(container.getUsername());
     when(mockedConfigs.getConfigDatabasePassword()).thenReturn(container.getPassword());
     when(mockedConfigs.getDatabaseUrl()).thenReturn(container.getJdbcUrl());
     when(mockedConfigs.getDatabaseUser()).thenReturn(container.getUsername());
     when(mockedConfigs.getDatabasePassword()).thenReturn(container.getPassword());
-    when(mockedConfigs.getAirbyteVersion()).thenReturn(new AirbyteVersion(version));
+    when(mockedConfigs.getAirbyteVersion()).thenReturn(new AirbyteVersion(VERSION_0330_ALPHA));
     when(mockedConfigs.runDatabaseMigrationOnStartup()).thenReturn(true);
     when(mockedConfigs.getConfigsDatabaseInitializationTimeoutMs()).thenReturn(60000L);
     when(mockedConfigs.getJobsDatabaseInitializationTimeoutMs()).thenReturn(60000L);

airbyte-commons-cli/src/test/java/io/airbyte/commons/cli/ClisTest.java

@@ -16,10 +16,13 @@
 
 class ClisTest {
 
+  private static final String ALPHA = "alpha";
+  private static final String BETA = "beta";
+
   @Test
   void testCreateOptionGroup() {
-    final Option optionA = new Option("a", "alpha");
-    final Option optionB = new Option("b", "beta");
+    final Option optionA = new Option("a", ALPHA);
+    final Option optionB = new Option("b", BETA);
     final OptionGroup optionGroupExpected = new OptionGroup();
     optionGroupExpected.addOption(optionA);
     optionGroupExpected.addOption(optionB);
@@ -38,18 +41,18 @@ void testParse() {
     final Option optionA = Option.builder("a").required(true).hasArg(true).build();
     final Option optionB = Option.builder("b").required(true).hasArg(true).build();
     final Options options = new Options().addOption(optionA).addOption(optionB);
-    final String[] args = {"-a", "alpha", "-b", "beta"};
+    final String[] args = {"-a", ALPHA, "-b", BETA};
     final CommandLine parsed = Clis.parse(args, options, new DefaultParser());
-    assertEquals("alpha", parsed.getOptions()[0].getValue());
-    assertEquals("beta", parsed.getOptions()[1].getValue());
+    assertEquals(ALPHA, parsed.getOptions()[0].getValue());
+    assertEquals(BETA, parsed.getOptions()[1].getValue());
   }
 
   @Test
   void testParseNonConforming() {
     final Option optionA = Option.builder("a").required(true).hasArg(true).build();
     final Option optionB = Option.builder("b").required(true).hasArg(true).build();
     final Options options = new Options().addOption(optionA).addOption(optionB);
-    final String[] args = {"-a", "alpha", "-b", "beta", "-c", "charlie"};
+    final String[] args = {"-a", ALPHA, "-b", BETA, "-c", "charlie"};
     assertThrows(IllegalArgumentException.class, () -> Clis.parse(args, options, new DefaultParser()));
   }
 
@@ -58,7 +61,7 @@ void testParseNonConformingWithSyntax() {
     final Option optionA = Option.builder("a").required(true).hasArg(true).build();
     final Option optionB = Option.builder("b").required(true).hasArg(true).build();
     final Options options = new Options().addOption(optionA).addOption(optionB);
-    final String[] args = {"-a", "alpha", "-b", "beta", "-c", "charlie"};
+    final String[] args = {"-a", ALPHA, "-b", BETA, "-c", "charlie"};
     assertThrows(IllegalArgumentException.class, () -> Clis.parse(args, options, new DefaultParser(), "search"));
   }
 
@@ -67,10 +70,10 @@ void testRelaxedParser() {
     final Option optionA = Option.builder("a").required(true).hasArg(true).build();
     final Option optionB = Option.builder("b").required(true).hasArg(true).build();
     final Options options = new Options().addOption(optionA).addOption(optionB);
-    final String[] args = {"-a", "alpha", "-b", "beta", "-c", "charlie"};
+    final String[] args = {"-a", ALPHA, "-b", BETA, "-c", "charlie"};
     final CommandLine parsed = Clis.parse(args, options, Clis.getRelaxedParser());
-    assertEquals("alpha", parsed.getOptions()[0].getValue());
-    assertEquals("beta", parsed.getOptions()[1].getValue());
+    assertEquals(ALPHA, parsed.getOptions()[0].getValue());
+    assertEquals(BETA, parsed.getOptions()[1].getValue());
   }
 
 }

airbyte-commons/src/main/java/io/airbyte/commons/io/Archives.java

@@ -49,8 +49,8 @@ private static void compressFile(final Path file, final Path filename, final Tar
   public static void extractArchive(final Path archiveFile, final Path destinationFolder) throws IOException {
     final TarArchiveInputStream archive =
         new TarArchiveInputStream(new GzipCompressorInputStream(new BufferedInputStream(Files.newInputStream(archiveFile))));
-    ArchiveEntry entry;
-    while ((entry = archive.getNextEntry()) != null) {
+    ArchiveEntry entry = archive.getNextEntry();
+    while (entry != null) {
       final Path newPath = zipSlipProtect(entry, destinationFolder);
       if (entry.isDirectory()) {
         Files.createDirectories(newPath);
@@ -63,6 +63,7 @@ public static void extractArchive(final Path archiveFile, final Path destination
         }
         Files.copy(archive, newPath, StandardCopyOption.REPLACE_EXISTING);
       }
+      entry = archive.getNextEntry();
     }
   }
 

airbyte-commons/src/main/java/io/airbyte/commons/io/IOs.java

@@ -88,9 +88,10 @@ public static List<String> getTail(final int numLines, final Path path) throws I
     try (final ReversedLinesFileReader fileReader = new ReversedLinesFileReader(file, Charsets.UTF_8)) {
       final List<String> lines = new ArrayList<>();
 
-      String line;
-      while ((line = fileReader.readLine()) != null && lines.size() < numLines) {
+      String line = fileReader.readLine();
+      while (line != null && lines.size() < numLines) {
         lines.add(line);
+        line = fileReader.readLine();
       }
 
       Collections.reverse(lines);

airbyte-commons/src/main/java/io/airbyte/commons/io/LineGobbler.java

@@ -20,13 +20,14 @@
 public class LineGobbler implements VoidCallable {
 
   private final static Logger LOGGER = LoggerFactory.getLogger(LineGobbler.class);
+  private final static String GENERIC = "generic";
 
   public static void gobble(final InputStream is, final Consumer<String> consumer) {
-    gobble(is, consumer, "generic", MdcScope.DEFAULT_BUILDER);
+    gobble(is, consumer, GENERIC, MdcScope.DEFAULT_BUILDER);
   }
 
   public static void gobble(final InputStream is, final Consumer<String> consumer, final MdcScope.Builder mdcScopeBuilder) {
-    gobble(is, consumer, "generic", mdcScopeBuilder);
+    gobble(is, consumer, GENERIC, mdcScopeBuilder);
   }
 
   public static void gobble(final InputStream is, final Consumer<String> consumer, final String caller, final MdcScope.Builder mdcScopeBuilder) {
@@ -47,15 +48,15 @@ public static void gobble(final InputStream is, final Consumer<String> consumer,
               final Consumer<String> consumer,
               final ExecutorService executor,
               final Map<String, String> mdc) {
-    this(is, consumer, executor, mdc, "generic", MdcScope.DEFAULT_BUILDER);
+    this(is, consumer, executor, mdc, GENERIC, MdcScope.DEFAULT_BUILDER);
   }
 
   LineGobbler(final InputStream is,
               final Consumer<String> consumer,
               final ExecutorService executor,
               final Map<String, String> mdc,
               final MdcScope.Builder mdcScopeBuilder) {
-    this(is, consumer, executor, mdc, "generic", mdcScopeBuilder);
+    this(is, consumer, executor, mdc, GENERIC, mdcScopeBuilder);
   }
 
   LineGobbler(final InputStream is,
@@ -76,11 +77,12 @@ public static void gobble(final InputStream is, final Consumer<String> consumer,
   public void voidCall() {
     MDC.setContextMap(mdc);
     try {
-      String line;
-      while ((line = is.readLine()) != null) {
+      String line = is.readLine();
+      while (line != null) {
         try (final var mdcScope = containerLogMdcBuilder.build()) {
           consumer.accept(line);
         }
+        line = is.readLine();
       }
     } catch (final IOException i) {
       LOGGER.warn("{} gobbler IOException: {}. Typically happens when cancelling a job.", caller, i.getMessage());