From b4a36baeabb4bbbe500aaf895a21527871b1d5ce Mon Sep 17 00:00:00 2001
From: Peter Nied <peternied@hotmail.com>
Date: Fri, 22 Nov 2024 23:59:03 +0000
Subject: [PATCH] Fix several minor issues in RFS codebase

- Fix keyword ordering of DOC_CONFIG_PARAMETER_ARG_PREFIX
- Created exception for Unexpected status code to remove duplicated
  error strings
- Make check for hasCompatibilityModeEnabled null safe
- Use isEmpty instead of size() == 0
- Remove redundant static modifier on CreationFailureType

Signed-off-by: Peter Nied <peternied@hotmail.com>
---
 .../migrations/RfsMigrateDocuments.java         |  2 +-
 .../bulkload/common/OpenSearchClient.java       | 17 +++++++++++------
 .../migrations/metadata/CreationResult.java     |  2 +-
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/DocumentsFromSnapshotMigration/src/main/java/org/opensearch/migrations/RfsMigrateDocuments.java b/DocumentsFromSnapshotMigration/src/main/java/org/opensearch/migrations/RfsMigrateDocuments.java
index 569e8b4a6..203d10d1d 100644
--- a/DocumentsFromSnapshotMigration/src/main/java/org/opensearch/migrations/RfsMigrateDocuments.java
+++ b/DocumentsFromSnapshotMigration/src/main/java/org/opensearch/migrations/RfsMigrateDocuments.java
@@ -176,7 +176,7 @@ public static class DocParams implements TransformerParams {
         public String getTransformerConfigParameterArgPrefix() {
             return DOC_CONFIG_PARAMETER_ARG_PREFIX;
         }
-        final static String DOC_CONFIG_PARAMETER_ARG_PREFIX = "doc-";
+        private static final String DOC_CONFIG_PARAMETER_ARG_PREFIX = "doc-";
 
         @Parameter(
                 required = false,
diff --git a/RFS/src/main/java/org/opensearch/migrations/bulkload/common/OpenSearchClient.java b/RFS/src/main/java/org/opensearch/migrations/bulkload/common/OpenSearchClient.java
index 9b7dbeda9..b906f9601 100644
--- a/RFS/src/main/java/org/opensearch/migrations/bulkload/common/OpenSearchClient.java
+++ b/RFS/src/main/java/org/opensearch/migrations/bulkload/common/OpenSearchClient.java
@@ -15,7 +15,6 @@
 import org.opensearch.migrations.Flavor;
 import org.opensearch.migrations.Version;
 import org.opensearch.migrations.VersionMatchers;
-import org.opensearch.migrations.bulkload.common.OpenSearchClient.OperationFailed;
 import org.opensearch.migrations.bulkload.common.http.ConnectionContext;
 import org.opensearch.migrations.bulkload.common.http.HttpResponse;
 import org.opensearch.migrations.bulkload.tracing.IRfsContexts;
@@ -89,7 +88,7 @@ public Version getClusterVersion() {
                 if (resp.statusCode == 404) {
                     return Mono.just(AMAZON_SERVERLESS_VERSION);
                 }
-                return Mono.error(new OperationFailed("Unexpected status code " + resp.statusCode, resp));
+                return Mono.error(new UnexpectedStatusCode(resp));
             })
             .doOnError(e -> log.error(e.getMessage()))
             .retryWhen(CHECK_IF_ITEM_EXISTS_RETRY_STRATEGY)
@@ -105,7 +104,7 @@ public Version getClusterVersion() {
             .retryWhen(CHECK_IF_ITEM_EXISTS_RETRY_STRATEGY)
             .flatMap(hasCompatibilityModeEnabled -> {
                 log.atInfo().setMessage("Checking CompatibilityMode, was enabled? {}").addArgument(hasCompatibilityModeEnabled).log();
-                if (!hasCompatibilityModeEnabled) {
+                if (Boolean.FALSE.equals(hasCompatibilityModeEnabled)) {
                     return Mono.just(versionFromRootApi);
                 }
                 return client.getAsync("_nodes/_all/nodes,version?format=json", null)
@@ -150,7 +149,7 @@ private Mono<Version> versionFromResponse(HttpResponse resp) {
 
     Mono<Boolean> checkCompatibilityModeFromResponse(HttpResponse resp) {
         if (resp.statusCode != 200) {
-            return Mono.error(new OperationFailed("Unexpected status code " + resp.statusCode, resp));
+            return Mono.error(new UnexpectedStatusCode(resp));
         }
         try {
             var body = Optional.of(objectMapper.readTree(resp.body));
@@ -175,7 +174,7 @@ private boolean inCompatibilityMode(Optional<JsonNode> node) {
 
     private Mono<Version> getVersionFromNodes(HttpResponse resp) {
         if (resp.statusCode != 200) {
-            return Mono.error(new OperationFailed("Unexpected status code " + resp.statusCode, resp));
+            return Mono.error(new UnexpectedStatusCode(resp));
         }
         var foundVersions = new HashSet<Version>();
         try {
@@ -188,7 +187,7 @@ private Mono<Version> getVersionFromNodes(HttpResponse resp) {
                 foundVersions.add(nodeVersion);
             });
 
-            if (foundVersions.size() == 0) {
+            if (foundVersions.isEmpty()) {
                 return Mono.error(new OperationFailed("Unable to find any version numbers", resp));
             }
 
@@ -556,4 +555,10 @@ public OperationFailed(String message, HttpResponse response) {
             this.response = response;
         }
     }
+
+    public static class UnexpectedStatusCode extends OperationFailed {
+        public UnexpectedStatusCode(HttpResponse response) {
+            super("Unexpected status code " + response.statusCode, response);
+        }
+    }
 }
diff --git a/RFS/src/main/java/org/opensearch/migrations/metadata/CreationResult.java b/RFS/src/main/java/org/opensearch/migrations/metadata/CreationResult.java
index 08e2ae2ed..63fe9da41 100644
--- a/RFS/src/main/java/org/opensearch/migrations/metadata/CreationResult.java
+++ b/RFS/src/main/java/org/opensearch/migrations/metadata/CreationResult.java
@@ -26,7 +26,7 @@ public boolean wasFatal() {
 
     @AllArgsConstructor
     @Getter
-    public static enum CreationFailureType {
+    public enum CreationFailureType {
         ALREADY_EXISTS(false, "already exists"),
         UNABLE_TO_TRANSFORM_FAILURE(true, "failed to transform to the target version"),
         TARGET_CLUSTER_FAILURE(true, "failed on target cluster");