Address a number of sonarqube java issues (opensearch-project#1181)

* Address a number of sonarqube java issues Fixed issues: - Remove this use of "put"; it is deprecated. - Remove this unnecessary cast to "Map". - Rename field "TYPE" to prevent any misunderstanding/clash with field "type" defined in superclass "org.opensearch.migrations.bulkload.models.MigrationItem". - Reorder the modifiers to comply with the Java Language Specification. - Change the visibility of this constructor to "protected". - Replace this lambda with method reference '*Template.class::isInstance'. - Replace this lambda with method reference '*Template.class::cast'. - Remove this unused method parameter "templateType". - Remove the parentheses around the "kvp" parameter Exclusions for false positives: - Set the credentials provider explicitly on this builder - Set the region explicitly on this builder Signed-off-by: Peter Nied <[email protected]>
AndreKurait · Dec 11, 2024 · 3eb8c08 · 3eb8c08
1 parent 0a329c0
commit 3eb8c08
Show file tree

Hide file tree

Showing 15 changed files with 72 additions and 55 deletions.
diff --git a/DataGenerator/src/main/java/org/opensearch/migrations/data/IFieldCreator.java b/DataGenerator/src/main/java/org/opensearch/migrations/data/IFieldCreator.java
@@ -10,7 +10,8 @@ public interface IFieldCreator {
     ObjectMapper mapper = new ObjectMapper();
 
     default ObjectNode createField(ElasticsearchType type) {
-        return mapper.createObjectNode().put("type", type.getValue());
+        String typeString = type.getValue();
+        return mapper.createObjectNode().put("type", typeString);
     }
 
     default ObjectNode fieldGeoPoint() { return createField(ElasticsearchType.GEO_POINT); }

diff --git a/DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/HttpLogs.java b/DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/HttpLogs.java
@@ -83,8 +83,9 @@ public Stream<ObjectNode> createDocs(int numDocs) {
         return IntStream.range(0, numDocs)
             .mapToObj(i -> {
                 var random = new Random(i);
+                long randomTime = randomTime(currentTime, random);
                 return mapper.createObjectNode()
-                    .put("@timestamp", randomTime(currentTime, random))
+                    .put("@timestamp", randomTime)
                     .put("clientip", randomIpAddress(random))
                     .put("request", randomRequest(random))
                     .put("status", randomStatus(random))

diff --git a/DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/Nested.java b/DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/Nested.java
@@ -104,8 +104,9 @@ private ArrayNode randomAnswers(ObjectMapper mapper, long timeFrom, Random rando
         var numAnswers = random.nextInt(5) + 1;
 
         for (int i = 0; i < numAnswers; i++) {
+            long randomTime = randomTime(timeFrom, random);
             var answer = mapper.createObjectNode()
-                .put("date", randomTime(timeFrom, random))
+                .put("date", randomTime)
                 .put("user", randomUser(random));
             answers.add(answer);
         }

diff --git a/DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/NycTaxis.java b/DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/NycTaxis.java
@@ -100,21 +100,29 @@ public Stream<ObjectNode> createDocs(int numDocs) {
         return IntStream.range(0, numDocs)
             .mapToObj(i -> {
                 var random = new Random(i);
+                double totalAmount = randomDouble(random, 5.0, 50.0);
+                String pickupTime = randomTimeISOString(currentTime, random);
+                String dropOffTime = randomTimeISOString(currentTime, random);
+                double tolls = randomDouble(random, 0.0, 5.0);
+                double fare = randomDouble(random, 5.0, 50.0);
+                double extra = randomDouble(random, 0.0, 1.0);
+                double tripDistance = randomDouble(random, 0.5, 20.0);
+                double tip = randomDouble(random, 0.0, 15.0);
                 return mapper.createObjectNode()
-                .<ObjectNode>put("total_amount", randomDouble(random, 5.0, 50.0))
+                .<ObjectNode>put("total_amount", totalAmount)
                 .<ObjectNode>put("improvement_surcharge", 0.3)
                 .<ObjectNode>set("pickup_location", randomLocationInNyc(random))
-                .<ObjectNode>put("pickup_datetime", randomTimeISOString(currentTime, random))
+                .<ObjectNode>put("pickup_datetime", pickupTime)
                 .<ObjectNode>put("trip_type", randomTripType(random))
-                .<ObjectNode>put("dropoff_datetime", randomTimeISOString(currentTime, random))
+                .<ObjectNode>put("dropoff_datetime", dropOffTime)
                 .<ObjectNode>put("rate_code_id", "1")
-                .<ObjectNode>put("tolls_amount", randomDouble(random, 0.0, 5.0))
+                .<ObjectNode>put("tolls_amount", tolls)
                 .<ObjectNode>set("dropoff_location", randomLocationInNyc(random))
                 .<ObjectNode>put("passenger_count", random.nextInt(4) + 1)
-                .<ObjectNode>put("fare_amount", randomDouble(random, 5.0, 50.0))
-                .<ObjectNode>put("extra", randomDouble(random, 0.0, 1.0))
-                .<ObjectNode>put("trip_distance", randomDouble(random, 0.5, 20.0))
-                .<ObjectNode>put("tip_amount", randomDouble(random, 0.0, 15.0))
+                .<ObjectNode>put("fare_amount", fare)
+                .<ObjectNode>put("extra", extra)
+                .<ObjectNode>put("trip_distance", tripDistance)
+                .<ObjectNode>put("tip_amount", tip)
                 .<ObjectNode>put("store_and_fwd_flag", randomStoreAndFwdFlag(random))
                 .<ObjectNode>put("payment_type", randomPaymentType(random))
                 .<ObjectNode>put("mta_tax", 0.5)

diff --git a/RFS/src/main/java/org/opensearch/migrations/bulkload/common/BulkDocSection.java b/RFS/src/main/java/org/opensearch/migrations/bulkload/common/BulkDocSection.java
@@ -102,7 +102,7 @@ public String asBulkIndexString() {
 
     @SuppressWarnings("unchecked")
     public Map<String, Object> toMap() {
-        return (Map<String, Object>) OBJECT_MAPPER.convertValue(bulkIndex, Map.class);
+        return OBJECT_MAPPER.convertValue(bulkIndex, Map.class);
     }
 
     @NoArgsConstructor(force = true) // For Jackson

diff --git a/RFS/src/main/java/org/opensearch/migrations/bulkload/models/ComponentTemplate.java b/RFS/src/main/java/org/opensearch/migrations/bulkload/models/ComponentTemplate.java
@@ -6,8 +6,8 @@
 
 @NoArgsConstructor(force = true, access = AccessLevel.PRIVATE) // For Jackson
 public class ComponentTemplate extends MigrationItem {
-    public static final String TYPE = "component_template";
+    public static final String TYPE_NAME = "component_template";
     public ComponentTemplate(final String name, final ObjectNode body) {
-        super(TYPE, name, body);
+        super(TYPE_NAME, name, body);
     }
 }
diff --git a/RFS/src/main/java/org/opensearch/migrations/bulkload/models/Index.java b/RFS/src/main/java/org/opensearch/migrations/bulkload/models/Index.java
@@ -6,8 +6,8 @@
 
 @NoArgsConstructor(force = true, access = AccessLevel.PRIVATE) // For Jackson
 public class Index extends MigrationItem {
-    public final static String TYPE = "index";
+    public static final String TYPE_NAME = "index";
     public Index(String name, ObjectNode body) {
-        super(TYPE, name, body);
+        super(TYPE_NAME, name, body);
     }
 }
diff --git a/RFS/src/main/java/org/opensearch/migrations/bulkload/models/IndexTemplate.java b/RFS/src/main/java/org/opensearch/migrations/bulkload/models/IndexTemplate.java
@@ -6,8 +6,8 @@
 
 @NoArgsConstructor(force = true, access = AccessLevel.PRIVATE) // For Jackson
 public class IndexTemplate extends MigrationItem {
-    public static final String TYPE = "index_template";
+    public static final String TYPE_NAME = "index_template";
     public IndexTemplate(final String name, final ObjectNode body) {
-        super(TYPE, name, body);
+        super(TYPE_NAME, name, body);
     }
 }
diff --git a/RFS/src/main/java/org/opensearch/migrations/bulkload/models/LegacyTemplate.java b/RFS/src/main/java/org/opensearch/migrations/bulkload/models/LegacyTemplate.java
@@ -6,8 +6,8 @@
 
 @NoArgsConstructor(force = true, access = AccessLevel.PRIVATE) // For Jackson
 public class LegacyTemplate extends MigrationItem {
-    public final static String TYPE = "template";
+    public static final String TYPE_NAME = "template";
     public LegacyTemplate(final String name, final ObjectNode body) {
-        super(TYPE, name, body);
+        super(TYPE_NAME, name, body);
     }
 }
diff --git a/RFS/src/main/java/org/opensearch/migrations/bulkload/models/MigrationItem.java b/RFS/src/main/java/org/opensearch/migrations/bulkload/models/MigrationItem.java
@@ -9,17 +9,17 @@
 @NoArgsConstructor(force = true, access = AccessLevel.PROTECTED) // For Jackson
 @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "type")
 @JsonSubTypes({
-        @JsonSubTypes.Type(value = Index.class, name = Index.TYPE),
-        @JsonSubTypes.Type(value = LegacyTemplate.class, name = LegacyTemplate.TYPE),
-        @JsonSubTypes.Type(value = IndexTemplate.class, name = IndexTemplate.TYPE),
-        @JsonSubTypes.Type(value = ComponentTemplate.class, name = ComponentTemplate.TYPE)
+        @JsonSubTypes.Type(value = Index.class, name = Index.TYPE_NAME),
+        @JsonSubTypes.Type(value = LegacyTemplate.class, name = LegacyTemplate.TYPE_NAME),
+        @JsonSubTypes.Type(value = IndexTemplate.class, name = IndexTemplate.TYPE_NAME),
+        @JsonSubTypes.Type(value = ComponentTemplate.class, name = ComponentTemplate.TYPE_NAME)
 })
 public abstract class MigrationItem {
     public final String type;
     public final String name;
     public final ObjectNode body;
 
-    public MigrationItem(final String type, final String name, final ObjectNode body) {
+    protected MigrationItem(final String type, final String name, final ObjectNode body) {
         this.type = type;
         this.name = name;
         this.body = body;

diff --git a/...org/opensearch/migrations/bulkload/transformers/TransformerToIJsonTransformerAdapter.java b/...org/opensearch/migrations/bulkload/transformers/TransformerToIJsonTransformerAdapter.java
@@ -28,7 +28,7 @@
 @Slf4j
 public class TransformerToIJsonTransformerAdapter implements Transformer {
     public static final String OUTPUT_TRANSFORMATION_JSON_LOGGER = "OutputTransformationJsonLogger";
-    private final static ObjectMapper MAPPER = new ObjectMapper();
+    private static final ObjectMapper MAPPER = new ObjectMapper();
     private final IJsonTransformer transformer;
     private final Logger transformerLogger;
 
@@ -65,7 +65,7 @@ private Map<String, Object> toTransformationMap(Map<String, Object> before, Map<
 
     @SuppressWarnings("unchecked")
     private static Map<String, Object> objectNodeToMap(Object node) {
-        return (Map<String, Object>) MAPPER.convertValue(node, Map.class);
+        return MAPPER.convertValue(node, Map.class);
     }
 
     @SneakyThrows
@@ -124,22 +124,19 @@ public GlobalMetadata transformGlobalMetadata(GlobalMetadata globalData) {
                 )
                 .map(this::transformMigrationItem).collect(Collectors.toList());
 
-        var transformedLegacy = transformedTemplates.stream().filter(
-                item -> item instanceof LegacyTemplate
-        )
-                .map(item -> (LegacyTemplate) item)
+        var transformedLegacy = transformedTemplates.stream()
+                .filter(LegacyTemplate.class::isInstance)
+                .map(LegacyTemplate.class::cast)
                 .collect(Collectors.toList());
 
-        var transformedIndex = transformedTemplates.stream().filter(
-                        item -> item instanceof IndexTemplate
-                )
-                .map(item -> (IndexTemplate) item)
+        var transformedIndex = transformedTemplates.stream()
+                .filter(IndexTemplate.class::isInstance)
+                .map(IndexTemplate.class::cast)
                 .collect(Collectors.toList());
 
-        var transformedComponent = transformedTemplates.stream().filter(
-                        item -> item instanceof ComponentTemplate
-                )
-                .map(item -> (ComponentTemplate) item)
+        var transformedComponent = transformedTemplates.stream()
+                .filter(ComponentTemplate.class::isInstance)
+                .map(ComponentTemplate.class::cast)
                 .collect(Collectors.toList());
 
         assert transformedLegacy.size() + transformedIndex.size() + transformedComponent.size() == transformedTemplates.size();

diff --git a/...ava/org/opensearch/migrations/bulkload/version_os_2_11/GlobalMetadataCreator_OS_2_11.java b/...ava/org/opensearch/migrations/bulkload/version_os_2_11/GlobalMetadataCreator_OS_2_11.java
@@ -118,12 +118,12 @@ private List<CreationResult> createTemplates(
             return List.of();
         }
 
-        var templatesToCreate = getAllTemplates(templates, templateType);
+        var templatesToCreate = getAllTemplates(templates);
 
         return processTemplateCreation(templatesToCreate, templateType, templateAllowlist, mode, context);
     }
 
-    Map<String, ObjectNode> getAllTemplates(ObjectNode templates, TemplateTypes templateType) {
+    Map<String, ObjectNode> getAllTemplates(ObjectNode templates) {
         var templatesToCreate = new HashMap<String, ObjectNode>();
 
         templates.fieldNames().forEachRemaining(templateName -> {
@@ -143,7 +143,7 @@ private List<CreationResult> processTemplateCreation(
         ) {
         var skipCreation = FilterScheme.filterByAllowList(templateAllowList).negate();
 
-        return templatesToCreate.entrySet().stream().map((kvp) -> {
+        return templatesToCreate.entrySet().stream().map(kvp -> {
             var templateName = kvp.getKey();
             var templateBody = kvp.getValue();
             var creationResult = CreationResult.builder().name(templateName);

diff --git a/RFS/src/main/java/org/opensearch/migrations/bulkload/worker/IndexRunner.java b/RFS/src/main/java/org/opensearch/migrations/bulkload/worker/IndexRunner.java
@@ -65,10 +65,10 @@ private CreationResult createIndex(String indexName, MigrationMode mode, ICreate
         try {
             indexMetadata = transformer.transformIndexMetadata(indexMetadata);
             return indexCreator.create(indexMetadata, mode, context);
-        } catch (Throwable t) {
+        } catch (Exception e) {
             return CreationResult.builder()
                 .name(indexName)
-                .exception(new IndexTransformationException(indexName, t))
+                .exception(new IndexTransformationException(indexName, e))
                 .failureType(CreationFailureType.UNABLE_TO_TRANSFORM_FAILURE)
                 .build();
         }

diff --git a/...org/opensearch/migrations/bulkload/version_os_2_11/GlobalMetadataCreator_OS_2_11Test.java b/...org/opensearch/migrations/bulkload/version_os_2_11/GlobalMetadataCreator_OS_2_11Test.java
@@ -7,7 +7,6 @@
 import org.opensearch.migrations.MigrationMode;
 import org.opensearch.migrations.bulkload.common.OpenSearchClient;
 import org.opensearch.migrations.bulkload.models.GlobalMetadata;
-import org.opensearch.migrations.bulkload.version_os_2_11.GlobalMetadataCreator_OS_2_11.TemplateTypes;
 import org.opensearch.migrations.metadata.CreationResult;
 import org.opensearch.migrations.metadata.CreationResult.CreationFailureType;
 import org.opensearch.migrations.metadata.tracing.IMetadataMigrationContexts.IClusterMetadataContext;
@@ -22,7 +21,6 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
@@ -46,15 +44,18 @@ void testCreate() {
         doReturn(filledOptional).when(client).createIndexTemplate(any(), any(), any());
         doReturn(filledOptional).when(client).createLegacyTemplate(any(), any(), any());
 
-        var creator = spy(new GlobalMetadataCreator_OS_2_11(client, List.of("lit1"), List.of(), null));
-        doReturn(Map.of("lit1", obj, "lit2", obj, ".lits", obj)).when(creator).getAllTemplates(any(), eq(TemplateTypes.LEGACY_INDEX_TEMPLATE));
-        doReturn(Map.of("it1", obj, ".its", obj)).when(creator).getAllTemplates(any(), eq(TemplateTypes.INDEX_TEMPLATE));
-        doReturn(Map.of("ct1", obj, ".cts", obj)).when(creator).getAllTemplates(any(), eq(TemplateTypes.COMPONENT_TEMPLATE));
-
         var globalMetadata = mock(GlobalMetadata.class);
-        doReturn(obj).when(globalMetadata).getComponentTemplates();
-        doReturn(obj).when(globalMetadata).getIndexTemplates();
-        doReturn(obj).when(globalMetadata).getTemplates();
+        var componentTemplates = mapper.createObjectNode().put("type", "component");
+        var indexTemplates = mapper.createObjectNode().put("type", "index");
+        var legacyTemplates = mapper.createObjectNode().put("type", "legacy");
+        doReturn(componentTemplates).when(globalMetadata).getComponentTemplates();
+        doReturn(indexTemplates).when(globalMetadata).getIndexTemplates();
+        doReturn(legacyTemplates).when(globalMetadata).getTemplates();
+
+        var creator = spy(new GlobalMetadataCreator_OS_2_11(client, List.of("lit1"), List.of(), null));
+        doReturn(Map.of("lit1", obj, "lit2", obj, ".lits", obj)).when(creator).getAllTemplates(legacyTemplates);
+        doReturn(Map.of("it1", obj, ".its", obj)).when(creator).getAllTemplates(indexTemplates);
+        doReturn(Map.of("ct1", obj, ".cts", obj)).when(creator).getAllTemplates(componentTemplates);
 
         var results = creator.create(globalMetadata, MigrationMode.PERFORM, context);
         assertThat(results.fatalIssueCount(), equalTo(0L));

diff --git a/sonar-project.properties b/sonar-project.properties
@@ -58,7 +58,7 @@ sonar.sourceEncoding=UTF-8
 sonar.issue.ignore.multicriteria = \
   p1, \
   ts1, ts2, ts3, ts4, ts5, ts6, ts7, ts8, ts9, ts10, \
-  j1, j2, j3, j4, j5, j6, j7, j8, j9, j10, j11, j12, \
+  j1, j2, j3, j4, j5, j6, j7, j8, j9, j10, j11, j12, j13, j14, \
   autoclose, \
   comp1, comp2, comp3, comp4, \
   loop1, loop2, loop3, loop4, loop5, \
@@ -168,6 +168,14 @@ sonar.issue.ignore.multicriteria.j11.resourceKey = **/*.java
 sonar.issue.ignore.multicriteria.j12.ruleKey = java:S1452
 sonar.issue.ignore.multicriteria.j12.resourceKey = **/*.java
 
+# Ignore Set the credentials provider explicitly on this builder, false positive.
+sonar.issue.ignore.multicriteria.j13.ruleKey = java:S6242
+sonar.issue.ignore.multicriteria.j13.resourceKey = **/*.java 
+
+# Ignore Set the region explicitly on this builder, false positive.
+sonar.issue.ignore.multicriteria.j14.ruleKey = java:S6241
+sonar.issue.ignore.multicriteria.j14.resourceKey = **/*.java
+
 
 
 # "Use try-with-resources or close this"