Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address code smells in DataGenerator #1076

Merged
merged 2 commits into from
Oct 24, 2024
Merged

Address code smells in DataGenerator #1076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
77166dd
Address code smells in DataGenerator
peternied Oct 16, 2024
b110bd8
Remove usage of @UtilityClass annotation
peternied Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion DataGenerator/src/main/java/org/opensearch/migrations/data/FieldBuilders.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion DataGenerator/src/main/java/org/opensearch/migrations/data/IndexOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions DataGenerator/src/main/java/org/opensearch/migrations/data/RandomDataBuilders.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion DataGenerator/src/main/java/org/opensearch/migrations/data/WorkloadOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
11 changes: 7 additions & 4 deletions DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/Geonames.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Comment on lines +12 to +14
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's continue to not allow static imports outside of test code (this is in src, not testFixture)
Adding the classname will give names like "INTEGER" some more context and clarity

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, I don't agree with the style preference. I would prefer to merge this change as is.

I've created MIGRATIONS-2141 [1] to automate enforcement of style issues, lets use tooling to align our preferences.

import static org.opensearch.migrations.data.FieldBuilders.createField;
import static org.opensearch.migrations.data.FieldBuilders.createFieldTextRawKeyword;
import static org.opensearch.migrations.data.RandomDataBuilders.randomDouble;
Expand All @@ -35,7 +38,7 @@ public List<String> 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());
Expand All @@ -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);
Expand Down
18 changes: 11 additions & 7 deletions DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/HttpLogs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -46,10 +50,10 @@ public List<String> 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);
Expand All @@ -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();
Expand Down
20 changes: 12 additions & 8 deletions DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/Nested.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -46,17 +50,17 @@ public List<String> 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();
Expand Down
24 changes: 14 additions & 10 deletions DataGenerator/src/main/java/org/opensearch/migrations/data/workloads/NycTaxis.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -38,28 +42,28 @@ public List<String> 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();
Expand Down