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.

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 a number of sonarqube java issues #1181

Merged
merged 8 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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); }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
peternied marked this conversation as resolved.
Show resolved Hide resolved
.put("clientip", randomIpAddress(random))
.put("request", randomRequest(random))
.put("status", randomStatus(random))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 -> {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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));
Expand Down
10 changes: 9 additions & 1 deletion sonar-project.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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, \
Expand Down Expand Up @@ -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"
Expand Down
Loading