From d90a44804d08b663d7e46f0f143e8e0f2e1d3661 Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Thu, 24 Oct 2024 07:45:13 -0500 Subject: [PATCH] Address code smells in DataGenerator (#1076) * Address code smells in DataGenerator - Defined constants for reused literals - Mark unmodified fields final - Add a private constructor to hide the implicit public constructor - Remove usage of @UtilityClass annotation Signed-off-by: Peter Nied --- .../migrations/data/FieldBuilders.java | 12 +++++++++- .../migrations/data/IndexOptions.java | 2 +- .../migrations/data/RandomDataBuilders.java | 6 ++--- .../migrations/data/WorkloadOptions.java | 2 +- .../migrations/data/workloads/Geonames.java | 11 +++++---- .../migrations/data/workloads/HttpLogs.java | 18 ++++++++------ .../migrations/data/workloads/Nested.java | 20 +++++++++------- .../migrations/data/workloads/NycTaxis.java | 24 +++++++++++-------- 8 files changed, 60 insertions(+), 35 deletions(-) diff --git a/DataGenerator/src/main/java/org/opensearch/migrations/data/FieldBuilders.java b/DataGenerator/src/main/java/org/opensearch/migrations/data/FieldBuilders.java index b2a922232..a0517da64 100644 --- a/DataGenerator/src/main/java/org/opensearch/migrations/data/FieldBuilders.java +++ b/DataGenerator/src/main/java/org/opensearch/migrations/data/FieldBuilders.java @@ -5,7 +5,17 @@ /** Shared ways to build fields for index mappings */ public class FieldBuilders { - private static final ObjectMapper mapper = new ObjectMapper(); + + private FieldBuilders() {} + + public static final String DATE = "date"; + public static final String GEO_POINT = "geo_point"; + public static final String INTEGER = "integer"; + public static final String KEYWORD = "keyword"; + public static final String LONG = "long"; + public static final String TEXT = "text"; + + public static final ObjectMapper mapper = new ObjectMapper(); public static ObjectNode createField(String type) { var field = mapper.createObjectNode(); diff --git a/DataGenerator/src/main/java/org/opensearch/migrations/data/IndexOptions.java b/DataGenerator/src/main/java/org/opensearch/migrations/data/IndexOptions.java index 9866cca92..910a483dc 100644 --- a/DataGenerator/src/main/java/org/opensearch/migrations/data/IndexOptions.java +++ b/DataGenerator/src/main/java/org/opensearch/migrations/data/IndexOptions.java @@ -8,7 +8,7 @@ public class IndexOptions { private static final ObjectMapper mapper = new ObjectMapper(); /** Improvement to add more flexibility with these values */ - public ObjectNode indexSettings = mapper.createObjectNode() + public final ObjectNode indexSettings = mapper.createObjectNode() .put("index.number_of_shards", 5) .put("index.number_of_replicas", 0) .put("index.queries.cache.enabled", false) diff --git a/DataGenerator/src/main/java/org/opensearch/migrations/data/RandomDataBuilders.java b/DataGenerator/src/main/java/org/opensearch/migrations/data/RandomDataBuilders.java index 6ccfe5ea3..bd8c69fab 100644 --- a/DataGenerator/src/main/java/org/opensearch/migrations/data/RandomDataBuilders.java +++ b/DataGenerator/src/main/java/org/opensearch/migrations/data/RandomDataBuilders.java @@ -5,15 +5,15 @@ import java.time.format.DateTimeFormatter; import java.util.Random; -import lombok.experimental.UtilityClass; - /** Shared ways to build random data */ -@UtilityClass public class RandomDataBuilders { + private static final ZoneId UTC_ZONE = ZoneId.of("UTC"); private static final DateTimeFormatter SIMPLE_DATE_PATTERN = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"); private static final int ONE_DAY_IN_MILLIS = 24 * 60 * 60 * 1000; + private RandomDataBuilders() {} + public static long randomTime(long timeFrom, Random random) { return timeFrom - random.nextInt(ONE_DAY_IN_MILLIS); } diff --git a/DataGenerator/src/main/java/org/opensearch/migrations/data/WorkloadOptions.java b/DataGenerator/src/main/java/org/opensearch/migrations/data/WorkloadOptions.java index 6335d8a26..241818853 100644 --- a/DataGenerator/src/main/java/org/opensearch/migrations/data/WorkloadOptions.java +++ b/DataGenerator/src/main/java/org/opensearch/migrations/data/WorkloadOptions.java @@ -17,5 +17,5 @@ public class WorkloadOptions { @Parameter(names = { "--max-bulk-request-batch-count" }, description = "The maximum batch count for bulk requests") public int maxBulkBatchSize = 50; - public IndexOptions index = new IndexOptions(); + public final IndexOptions index = new IndexOptions(); } diff --git a/DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/Geonames.java b/DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/Geonames.java index 1728849dc..a393729e4 100644 --- a/DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/Geonames.java +++ b/DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/Geonames.java @@ -9,6 +9,9 @@ import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; +import static org.opensearch.migrations.data.FieldBuilders.GEO_POINT; +import static org.opensearch.migrations.data.FieldBuilders.INTEGER; +import static org.opensearch.migrations.data.FieldBuilders.LONG; import static org.opensearch.migrations.data.FieldBuilders.createField; import static org.opensearch.migrations.data.FieldBuilders.createFieldTextRawKeyword; import static org.opensearch.migrations.data.RandomDataBuilders.randomDouble; @@ -35,7 +38,7 @@ public List indexNames() { @Override public ObjectNode createIndex(ObjectNode defaultSettings) { var properties = mapper.createObjectNode(); - properties.set("geonameid", createField("long")); + properties.set("geonameid", createField(LONG)); properties.set("name", createFieldTextRawKeyword()); properties.set("asciiname", createFieldTextRawKeyword()); properties.set("alternatenames", createFieldTextRawKeyword()); @@ -46,11 +49,11 @@ public ObjectNode createIndex(ObjectNode defaultSettings) { properties.set("admin2_code", createFieldTextRawKeyword()); properties.set("admin3_code", createFieldTextRawKeyword()); properties.set("admin4_code", createFieldTextRawKeyword()); - properties.set("elevation", createField("integer")); - properties.set("population", createField("long")); + properties.set("elevation", createField(INTEGER)); + properties.set("population", createField(LONG)); properties.set("dem", createFieldTextRawKeyword()); properties.set("timezone", createFieldTextRawKeyword()); - properties.set("location", createField("geo_point")); + properties.set("location", createField(GEO_POINT)); var countryCodeField = createFieldTextRawKeyword(); countryCodeField.put("fielddata", true); 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 182a6287e..d4fb5a1d8 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 @@ -8,6 +8,10 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; +import static org.opensearch.migrations.data.FieldBuilders.DATE; +import static org.opensearch.migrations.data.FieldBuilders.GEO_POINT; +import static org.opensearch.migrations.data.FieldBuilders.INTEGER; +import static org.opensearch.migrations.data.FieldBuilders.KEYWORD; import static org.opensearch.migrations.data.FieldBuilders.createField; import static org.opensearch.migrations.data.FieldBuilders.createFieldTextRawKeyword; import static org.opensearch.migrations.data.RandomDataBuilders.randomElement; @@ -46,10 +50,10 @@ public List indexNames() { @Override public ObjectNode createIndex(ObjectNode defaultSettings) { var properties = mapper.createObjectNode(); - var timestamp = createField("date"); + var timestamp = createField(DATE); timestamp.put("format", "strict_date_optional_time||epoch_second"); properties.set("@timestamp", timestamp); - var message = createField("keyword"); + var message = createField(KEYWORD); message.put("index", false); message.put("doc_values", false); properties.set("message", message); @@ -58,14 +62,14 @@ public ObjectNode createIndex(ObjectNode defaultSettings) { var requestRaw = (ObjectNode) request.get("fields").get("raw"); requestRaw.put("ignore_above", 256); properties.set("request", request); - properties.set("status", createField("integer")); - properties.set("size", createField("integer")); + properties.set("status", createField(INTEGER)); + properties.set("size", createField(INTEGER)); var geoip = mapper.createObjectNode(); var geoipProps = mapper.createObjectNode(); geoip.set("properties", geoipProps); - geoipProps.set("country_name", createField("keyword")); - geoipProps.set("city_name", createField("keyword")); - geoipProps.set("location", createField("geo_point")); + geoipProps.set("country_name", createField(KEYWORD)); + geoipProps.set("city_name", createField(KEYWORD)); + geoipProps.set("location", createField(GEO_POINT)); properties.set("geoip", geoip); var mappings = mapper.createObjectNode(); 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 f8ba62869..8756bec08 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 @@ -11,6 +11,10 @@ import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; +import static org.opensearch.migrations.data.FieldBuilders.DATE; +import static org.opensearch.migrations.data.FieldBuilders.INTEGER; +import static org.opensearch.migrations.data.FieldBuilders.KEYWORD; +import static org.opensearch.migrations.data.FieldBuilders.TEXT; import static org.opensearch.migrations.data.FieldBuilders.createField; import static org.opensearch.migrations.data.RandomDataBuilders.randomElement; import static org.opensearch.migrations.data.RandomDataBuilders.randomTime; @@ -46,17 +50,17 @@ public List indexNames() { @Override public ObjectNode createIndex(ObjectNode defaultSettings) { var properties = mapper.createObjectNode(); - properties.set("user", createField("keyword")); - properties.set("creationDate", createField("date")); - properties.set("title", createField("text")); - properties.set("qid", createField("keyword")); - properties.set("tag", createField("keyword")); - properties.set("answer_count", createField("integer")); + properties.set("user", createField(KEYWORD)); + properties.set("creationDate", createField(DATE)); + properties.set("title", createField(TEXT)); + properties.set("qid", createField(KEYWORD)); + properties.set("tag", createField(KEYWORD)); + properties.set("answer_count", createField(INTEGER)); var answers = createField("nested"); var answersProps = mapper.createObjectNode(); answers.set("properties", answersProps); - answersProps.set("user", createField("keyword")); - answersProps.set("date", createField("date")); + answersProps.set("user", createField(KEYWORD)); + answersProps.set("date", createField(DATE)); properties.set("answers", answers); var mappings = mapper.createObjectNode(); 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 26fa6da49..710d86bca 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 @@ -9,6 +9,10 @@ import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; +import static org.opensearch.migrations.data.FieldBuilders.GEO_POINT; +import static org.opensearch.migrations.data.FieldBuilders.INTEGER; +import static org.opensearch.migrations.data.FieldBuilders.KEYWORD; +import static org.opensearch.migrations.data.FieldBuilders.TEXT; import static org.opensearch.migrations.data.FieldBuilders.createField; import static org.opensearch.migrations.data.RandomDataBuilders.randomDouble; import static org.opensearch.migrations.data.RandomDataBuilders.randomElement; @@ -38,28 +42,28 @@ public List indexNames() { @Override public ObjectNode createIndex(ObjectNode defaultSettings) { var properties = mapper.createObjectNode(); - properties.set("cab_color", createField("keyword")); + properties.set("cab_color", createField(KEYWORD)); properties.set("dropoff_datetime", createDateField()); - properties.set("dropoff_location", createField("geo_point")); + properties.set("dropoff_location", createField(GEO_POINT)); properties.set("ehail_fee", createScaledFloatField()); properties.set("extra", createScaledFloatField()); properties.set("fare_amount", createScaledFloatField()); properties.set("improvement_surcharge", createScaledFloatField()); properties.set("mta_tax", createScaledFloatField()); - properties.set("passenger_count", createField("integer")); - properties.set("payment_type", createField("keyword")); + properties.set("passenger_count", createField(INTEGER)); + properties.set("payment_type", createField(KEYWORD)); properties.set("pickup_datetime", createDateField()); - properties.set("pickup_location", createField("geo_point")); - properties.set("rate_code_id", createField("keyword")); - properties.set("store_and_fwd_flag", createField("keyword")); + properties.set("pickup_location", createField(GEO_POINT)); + properties.set("rate_code_id", createField(KEYWORD)); + properties.set("store_and_fwd_flag", createField(KEYWORD)); properties.set("surcharge", createScaledFloatField()); properties.set("tip_amount", createScaledFloatField()); properties.set("tolls_amount", createScaledFloatField()); properties.set("total_amount", createScaledFloatField()); properties.set("trip_distance", createScaledFloatField()); - properties.set("trip_type", createField("keyword")); - properties.set("vendor_id", createField("keyword")); - properties.set("vendor_name", createField("text")); + properties.set("trip_type", createField(KEYWORD)); + properties.set("vendor_id", createField(KEYWORD)); + properties.set("vendor_name", createField(TEXT)); var mappings = mapper.createObjectNode();