Enable PMD rule to prevent == string comparisons (#20080)

airbytehq · Dec 5, 2022 · 201df24 · 201df24
1 parent c471be2
commit 201df24
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 22 deletions.
diff --git a/.../io/airbyte/db/instance/jobs/migrations/V0_35_40_001__MigrateFailureReasonEnumValues.java b/.../io/airbyte/db/instance/jobs/migrations/V0_35_40_001__MigrateFailureReasonEnumValues.java
@@ -12,6 +12,7 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import org.flywaydb.core.api.migration.BaseJavaMigration;
 import org.flywaydb.core.api.migration.Context;
 import org.jooq.DSLContext;
@@ -322,14 +323,14 @@ public boolean equals(final Object other) {
         return false;
       }
       final FailureReasonForMigration rhs = ((FailureReasonForMigration) other);
-      return (((((((((this.retryable == rhs.retryable) || ((this.retryable != null) && this.retryable.equals(rhs.retryable)))
-          && ((this.metadata == rhs.metadata) || ((this.metadata != null) && this.metadata.equals(rhs.metadata))))
-          && ((this.stacktrace == rhs.stacktrace) || ((this.stacktrace != null) && this.stacktrace.equals(rhs.stacktrace))))
-          && ((this.failureOrigin == rhs.failureOrigin) || ((this.failureOrigin != null) && this.failureOrigin.equals(rhs.failureOrigin))))
-          && ((this.failureType == rhs.failureType) || ((this.failureType != null) && this.failureType.equals(rhs.failureType))))
-          && ((this.internalMessage == rhs.internalMessage) || ((this.internalMessage != null) && this.internalMessage.equals(rhs.internalMessage))))
-          && ((this.externalMessage == rhs.externalMessage) || ((this.externalMessage != null) && this.externalMessage.equals(rhs.externalMessage))))
-          && ((this.timestamp == rhs.timestamp) || ((this.timestamp != null) && this.timestamp.equals(rhs.timestamp))));
+      return ((((((((Objects.equals(this.retryable, rhs.retryable))
+          && (Objects.equals(this.metadata, rhs.metadata)))
+          && (Objects.equals(this.stacktrace, rhs.stacktrace)))
+          && (Objects.equals(this.failureOrigin, rhs.failureOrigin)))
+          && (Objects.equals(this.failureType, rhs.failureType)))
+          && (Objects.equals(this.internalMessage, rhs.internalMessage)))
+          && (Objects.equals(this.externalMessage, rhs.externalMessage)))
+          && (Objects.equals(this.timestamp, rhs.timestamp)));
     }
 
   }

diff --git a/...yte-persistence/job-persistence/src/main/java/io/airbyte/persistence/job/JobNotifier.java b/...yte-persistence/job-persistence/src/main/java/io/airbyte/persistence/job/JobNotifier.java
@@ -88,11 +88,11 @@ private void notifyJob(final String reason,
         try {
           final Builder<String, Object> notificationMetadata = ImmutableMap.builder();
           notificationMetadata.put("connection_id", connectionId);
-          if (notification.getNotificationType().equals(NotificationType.SLACK) &&
+          if (NotificationType.SLACK.equals(notification.getNotificationType()) &&
               notification.getSlackConfiguration().getWebhook().contains("hooks.slack.com")) {
             // flag as slack if the webhook URL is also pointing to slack
             notificationMetadata.put("notification_type", NotificationType.SLACK);
-          } else if (notification.getNotificationType().equals(NotificationType.CUSTOMERIO)) {
+          } else if (NotificationType.CUSTOMERIO.equals(notification.getNotificationType())) {
             notificationMetadata.put("notification_type", NotificationType.CUSTOMERIO);
           } else {
             // Slack Notification type could be "hacked" and re-used for custom webhooks
@@ -103,23 +103,23 @@ private void notifyJob(final String reason,
               action,
               MoreMaps.merge(jobMetadata, sourceMetadata, destinationMetadata, notificationMetadata.build()));
 
-          if (FAILURE_NOTIFICATION == action) {
+          if (FAILURE_NOTIFICATION.equalsIgnoreCase(action)) {
             if (!notificationClient.notifyJobFailure(sourceConnector, destinationConnector, jobDescription, logUrl, job.getId())) {
               LOGGER.warn("Failed to successfully notify failure: {}", notification);
             }
             break;
-          } else if (SUCCESS_NOTIFICATION == action) {
+          } else if (SUCCESS_NOTIFICATION.equalsIgnoreCase(action)) {
             if (!notificationClient.notifyJobSuccess(sourceConnector, destinationConnector, jobDescription, logUrl, job.getId())) {
               LOGGER.warn("Failed to successfully notify success: {}", notification);
             }
             break;
-          } else if (CONNECTION_DISABLED_NOTIFICATION == action) {
+          } else if (CONNECTION_DISABLED_NOTIFICATION.equalsIgnoreCase(action)) {
             if (!notificationClient.notifyConnectionDisabled(workspace.getEmail(), sourceConnector, destinationConnector, jobDescription,
                 workspaceId, connectionId)) {
               LOGGER.warn("Failed to successfully notify auto-disable connection: {}", notification);
             }
             break;
-          } else if (CONNECTION_DISABLED_WARNING_NOTIFICATION == action) {
+          } else if (CONNECTION_DISABLED_WARNING_NOTIFICATION.equalsIgnoreCase(action)) {
             if (!notificationClient.notifyConnectionDisableWarning(workspace.getEmail(), sourceConnector, destinationConnector, jobDescription,
                 workspaceId, connectionId)) {
               LOGGER.warn("Failed to successfully notify auto-disable connection warning: {}", notification);

diff --git a/...yte-server/src/test/java/io/airbyte/server/handlers/WebBackendConnectionsHandlerTest.java b/...yte-server/src/test/java/io/airbyte/server/handlers/WebBackendConnectionsHandlerTest.java
@@ -418,7 +418,7 @@ void testWebBackendGetConnectionWithDiscoveryAndNewSchema() throws ConfigNotFoun
     when(configRepository.getMostRecentActorCatalogFetchEventForSource(any()))
         .thenReturn(Optional.of(new ActorCatalogFetchEvent().withActorCatalogId(newCatalogId)));
     when(configRepository.getActorCatalogById(any())).thenReturn(new ActorCatalog().withId(UUID.randomUUID()));
-    SourceDiscoverSchemaRead schemaRead =
+    final SourceDiscoverSchemaRead schemaRead =
         new SourceDiscoverSchemaRead().catalogDiff(expectedWithNewSchema.getCatalogDiff()).catalog(expectedWithNewSchema.getSyncCatalog())
             .breakingChange(false).connectionStatus(ConnectionStatus.ACTIVE);
     when(schedulerHandler.discoverSchemaForSourceFromSourceId(any())).thenReturn(schemaRead);
@@ -435,7 +435,7 @@ void testWebBackendGetConnectionWithDiscoveryAndNewSchemaBreakingChange() throws
     when(configRepository.getMostRecentActorCatalogFetchEventForSource(any()))
         .thenReturn(Optional.of(new ActorCatalogFetchEvent().withActorCatalogId(newCatalogId)));
     when(configRepository.getActorCatalogById(any())).thenReturn(new ActorCatalog().withId(UUID.randomUUID()));
-    SourceDiscoverSchemaRead schemaRead =
+    final SourceDiscoverSchemaRead schemaRead =
         new SourceDiscoverSchemaRead().catalogDiff(expectedWithNewSchema.getCatalogDiff()).catalog(expectedWithNewSchema.getSyncCatalog())
             .breakingChange(true).connectionStatus(ConnectionStatus.INACTIVE);
     when(schedulerHandler.discoverSchemaForSourceFromSourceId(any())).thenReturn(schemaRead);
@@ -1163,13 +1163,13 @@ void testGetStreamsToReset() {
     final List<StreamDescriptor> resultList = WebBackendConnectionsHandler.getStreamsToReset(catalogDiff);
     assertTrue(
         resultList.stream().anyMatch(
-            streamDescriptor -> streamDescriptor.getName() == "added_stream"));
+            streamDescriptor -> "added_stream".equalsIgnoreCase(streamDescriptor.getName())));
     assertTrue(
         resultList.stream().anyMatch(
-            streamDescriptor -> streamDescriptor.getName() == "removed_stream"));
+            streamDescriptor -> "removed_stream".equalsIgnoreCase(streamDescriptor.getName())));
     assertTrue(
         resultList.stream().anyMatch(
-            streamDescriptor -> streamDescriptor.getName() == "updated_stream"));
+            streamDescriptor -> "updated_stream".equalsIgnoreCase(streamDescriptor.getName())));
   }
 
   @Test

diff --git a/...acceptanceTests/java/io/airbyte/test/acceptance/ContainerOrchestratorAcceptanceTests.java b/...acceptanceTests/java/io/airbyte/test/acceptance/ContainerOrchestratorAcceptanceTests.java
@@ -38,7 +38,6 @@
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.condition.EnabledIfEnvironmentVariable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -101,7 +100,8 @@ static void end() {
   void setup() throws URISyntaxException, IOException, SQLException {
     testHarness.setup();
   }
-  // This test is flaky.  Warnings are suppressed until that condition us understood
+
+  // This test is flaky. Warnings are suppressed until that condition us understood
   // See: https://github.com/airbytehq/airbyte/issues/19948
   @Test
   @SuppressWarnings("PMD.JUnitTestsShouldIncludeAssert")

diff --git a/tools/gradle/pmd/rules.xml b/tools/gradle/pmd/rules.xml
@@ -107,7 +107,6 @@
         <exclude name="ReturnEmptyCollectionRatherThanNull"/>
         <exclude name="SimpleDateFormatNeedsLocale"/>
         <exclude name="SingletonClassReturningNewInstance"/>
-        <exclude name="UseEqualsToCompareStrings"/>
         <exclude name="UseProperClassLoader"/>
     </rule>
     <rule ref="category/java/errorprone.xml/AssignmentInOperand">