From 3eb8c087157ebdc17993fb04942778d45beeb96d Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Wed, 11 Dec 2024 16:36:32 -0600 Subject: [PATCH] Address a number of sonarqube java issues (#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 --- .../migrations/data/IFieldCreator.java | 3 ++- .../migrations/data/workloads/HttpLogs.java | 3 ++- .../migrations/data/workloads/Nested.java | 3 ++- .../migrations/data/workloads/NycTaxis.java | 24 ++++++++++++------ .../bulkload/common/BulkDocSection.java | 2 +- .../bulkload/models/ComponentTemplate.java | 4 +-- .../migrations/bulkload/models/Index.java | 4 +-- .../bulkload/models/IndexTemplate.java | 4 +-- .../bulkload/models/LegacyTemplate.java | 4 +-- .../bulkload/models/MigrationItem.java | 10 ++++---- .../TransformerToIJsonTransformerAdapter.java | 25 ++++++++----------- .../GlobalMetadataCreator_OS_2_11.java | 6 ++--- .../bulkload/worker/IndexRunner.java | 4 +-- .../GlobalMetadataCreator_OS_2_11Test.java | 21 ++++++++-------- sonar-project.properties | 10 +++++++- 15 files changed, 72 insertions(+), 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 index 8ee1fb9c9..862ff1bfb 100644 --- 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 index c20543040..6f2cf6d64 100644 --- 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 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 index 038202a43..3655c3818 100644 --- 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 index 61ddbb5fd..3aa06fb21 100644 --- 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 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() - .put("total_amount", randomDouble(random, 5.0, 50.0)) + .put("total_amount", totalAmount) .put("improvement_surcharge", 0.3) .set("pickup_location", randomLocationInNyc(random)) - .put("pickup_datetime", randomTimeISOString(currentTime, random)) + .put("pickup_datetime", pickupTime) .put("trip_type", randomTripType(random)) - .put("dropoff_datetime", randomTimeISOString(currentTime, random)) + .put("dropoff_datetime", dropOffTime) .put("rate_code_id", "1") - .put("tolls_amount", randomDouble(random, 0.0, 5.0)) + .put("tolls_amount", tolls) .set("dropoff_location", randomLocationInNyc(random)) .put("passenger_count", random.nextInt(4) + 1) - .put("fare_amount", randomDouble(random, 5.0, 50.0)) - .put("extra", randomDouble(random, 0.0, 1.0)) - .put("trip_distance", randomDouble(random, 0.5, 20.0)) - .put("tip_amount", randomDouble(random, 0.0, 15.0)) + .put("fare_amount", fare) + .put("extra", extra) + .put("trip_distance", tripDistance) + .put("tip_amount", tip) .put("store_and_fwd_flag", randomStoreAndFwdFlag(random)) .put("payment_type", randomPaymentType(random)) .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 index 02ccfbde8..60ae69877 100644 --- 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 toMap() { - return (Map) 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 index a019b871c..8b8898c36 100644 --- 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 index cef345ad0..1620c2a81 100644 --- 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 index 792e9d53e..9327b2225 100644 --- 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 index 7edfa763a..0a392c2a1 100644 --- 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 index dccd2cea1..e4a77aa16 100644 --- 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/RFS/src/main/java/org/opensearch/migrations/bulkload/transformers/TransformerToIJsonTransformerAdapter.java b/RFS/src/main/java/org/opensearch/migrations/bulkload/transformers/TransformerToIJsonTransformerAdapter.java index eaf08cada..841fe356d 100644 --- a/RFS/src/main/java/org/opensearch/migrations/bulkload/transformers/TransformerToIJsonTransformerAdapter.java +++ b/RFS/src/main/java/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 toTransformationMap(Map before, Map< @SuppressWarnings("unchecked") private static Map objectNodeToMap(Object node) { - return (Map) 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/RFS/src/main/java/org/opensearch/migrations/bulkload/version_os_2_11/GlobalMetadataCreator_OS_2_11.java b/RFS/src/main/java/org/opensearch/migrations/bulkload/version_os_2_11/GlobalMetadataCreator_OS_2_11.java index 2bd08a023..bdb3efc39 100644 --- a/RFS/src/main/java/org/opensearch/migrations/bulkload/version_os_2_11/GlobalMetadataCreator_OS_2_11.java +++ b/RFS/src/main/java/org/opensearch/migrations/bulkload/version_os_2_11/GlobalMetadataCreator_OS_2_11.java @@ -118,12 +118,12 @@ private List createTemplates( return List.of(); } - var templatesToCreate = getAllTemplates(templates, templateType); + var templatesToCreate = getAllTemplates(templates); return processTemplateCreation(templatesToCreate, templateType, templateAllowlist, mode, context); } - Map getAllTemplates(ObjectNode templates, TemplateTypes templateType) { + Map getAllTemplates(ObjectNode templates) { var templatesToCreate = new HashMap(); templates.fieldNames().forEachRemaining(templateName -> { @@ -143,7 +143,7 @@ private List 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 index 832da1243..8dd42d968 100644 --- 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/RFS/src/test/java/org/opensearch/migrations/bulkload/version_os_2_11/GlobalMetadataCreator_OS_2_11Test.java b/RFS/src/test/java/org/opensearch/migrations/bulkload/version_os_2_11/GlobalMetadataCreator_OS_2_11Test.java index 93eab276b..1a606bde9 100644 --- a/RFS/src/test/java/org/opensearch/migrations/bulkload/version_os_2_11/GlobalMetadataCreator_OS_2_11Test.java +++ b/RFS/src/test/java/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 index 501b15e64..d2116e118 100644 --- 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"