Skip to content

Commit d4b9262

Browse files
jackiehanyangjackiehanyang
committed
address comments
1 parent c844922 commit d4b9262

File tree

6 files changed

+76
-13
lines changed

6 files changed

+76
-13
lines changed

‎src/main/java/org/opensearch/ad/indices/ADIndexManagement.java

-2
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ public class ADIndexManagement extends IndexManagement<ADIndex> {
6060
// The index name pattern to query all AD result, history and current AD result
6161
public static final String ALL_AD_RESULTS_INDEX_PATTERN = ".opendistro-anomaly-results*";
6262

63-
// private static final ObjectMapper objectMapper = new ObjectMapper();
64-
6563
/**
6664
* Constructor function
6765
*

‎src/main/java/org/opensearch/ad/model/AnomalyDetector.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,7 @@ public static AnomalyDetector parse(
592592
case RESULT_INDEX_FIELD_TTL:
593593
customResultIndexTTL = onlyParseNumberValue(parser);
594594
break;
595-
case FLATTEN_RESULT_INDEX_MAPPING:
595+
case FLATTEN_CUSTOM_RESULT_INDEX:
596596
flattenResultIndexMapping = onlyParseBooleanValue(parser);
597597
break;
598598
case BREAKING_UI_CHANGE_TIME:

‎src/main/java/org/opensearch/forecast/model/Forecaster.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ public static Forecaster parse(
437437
case RESULT_INDEX_FIELD_TTL:
438438
customResultIndexTTL = parser.intValue();
439439
break;
440-
case FLATTEN_RESULT_INDEX_MAPPING:
440+
case FLATTEN_CUSTOM_RESULT_INDEX:
441441
flattenResultIndexMapping = parser.booleanValue();
442442
break;
443443
case BREAKING_UI_CHANGE_TIME:

‎src/main/java/org/opensearch/timeseries/model/Config.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public abstract class Config implements Writeable, ToXContentObject {
7979
public static final String RESULT_INDEX_FIELD_MIN_SIZE = "result_index_min_size";
8080
public static final String RESULT_INDEX_FIELD_MIN_AGE = "result_index_min_age";
8181
public static final String RESULT_INDEX_FIELD_TTL = "result_index_ttl";
82-
public static final String FLATTEN_RESULT_INDEX_MAPPING = "flatten_custom_result_index";
82+
public static final String FLATTEN_CUSTOM_RESULT_INDEX = "flatten_custom_result_index";
8383
// Changing categorical field, feature attributes, interval, windowDelay, time field, horizon, indices,
8484
// result index would force us to display results only from the most recent update. Otherwise,
8585
// the UI appear cluttered and unclear.
@@ -533,7 +533,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
533533
builder.field(RESULT_INDEX_FIELD_TTL, customResultIndexTTL);
534534
}
535535
if (flattenResultIndexMapping != null) {
536-
builder.field(FLATTEN_RESULT_INDEX_MAPPING, flattenResultIndexMapping);
536+
builder.field(FLATTEN_CUSTOM_RESULT_INDEX, flattenResultIndexMapping);
537537
}
538538
if (lastUIBreakingChangeTime != null) {
539539
builder.field(BREAKING_UI_CHANGE_TIME, lastUIBreakingChangeTime.toEpochMilli());
@@ -746,8 +746,8 @@ public Integer getCustomResultIndexTTL() {
746746
return customResultIndexTTL;
747747
}
748748

749-
public Boolean getFlattenResultIndexMapping() {
750-
return flattenResultIndexMapping;
749+
public boolean getFlattenResultIndexMapping() {
750+
return flattenResultIndexMapping != null ? flattenResultIndexMapping : false;
751751
}
752752

753753
public Instant getLastBreakingUIChangeTime() {

‎src/main/java/org/opensearch/timeseries/rest/handler/AbstractTimeSeriesActionHandler.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,7 @@ private BytesReference createPipelineDefinition(String indexName) throws IOExcep
529529
XContentBuilder pipelineBuilder = XContentFactory.jsonBuilder();
530530
pipelineBuilder.startObject();
531531
{
532-
pipelineBuilder.field("description", "Ingest pipeline for anomaly detector with result index: " + indexName);
532+
pipelineBuilder.field("description", "Ingest pipeline for flattening result index: " + indexName);
533533
pipelineBuilder.startArray("processors");
534534
{
535535
pipelineBuilder.startObject();
@@ -578,8 +578,8 @@ private void handleFlattenResultIndexMappingUpdate(Config existingConfig, Action
578578
if (config.getCustomResultIndexOrAlias() == null) {
579579
return;
580580
}
581-
if (Boolean.TRUE.equals(existingConfig.getFlattenResultIndexMapping())
582-
&& Boolean.FALSE.equals(config.getFlattenResultIndexMapping())
581+
if (existingConfig.getFlattenResultIndexMapping()
582+
&& !config.getFlattenResultIndexMapping()
583583
&& existingConfig.getCustomResultIndexOrAlias() != null) {
584584
String pipelineId = timeSeriesIndices.getFlattenResultIndexIngestPipelineId(config.getId());
585585
client.admin().cluster().deletePipeline(new DeletePipelineRequest(pipelineId), new ActionListener<AcknowledgedResponse>() {
@@ -611,8 +611,8 @@ public void onFailure(Exception e) {
611611
}
612612
}
613613
});
614-
} else if (Boolean.FALSE.equals(existingConfig.getFlattenResultIndexMapping())
615-
&& Boolean.TRUE.equals(config.getFlattenResultIndexMapping())
614+
} else if (!existingConfig.getFlattenResultIndexMapping()
615+
&& config.getFlattenResultIndexMapping()
616616
&& existingConfig.getCustomResultIndexOrAlias() != null) {
617617
listener.onFailure(new OpenSearchStatusException(CommonMessages.CAN_NOT_CHANGE_FLATTEN_RESULT_INDEX, RestStatus.BAD_REQUEST));
618618
return;

‎src/test/java/org/opensearch/ad/rest/AnomalyDetectorRestApiIT.java

+65
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,71 @@ public void testUpdateAnomalyDetector_disableFlattenResultIndex_shouldDeletePipe
331331
assertEquals("Expected 404 response but got: " + statusCode, 404, statusCode);
332332
}
333333

334+
public void testUpdateAnomalyDetectorFlattenResultIndexField() throws Exception {
335+
TestHelpers.createIndexWithTimeField(client(), INDEX_NAME, TIME_FIELD, false);
336+
String testIndexData = "{\"keyword-field\": \"field-1\", \"ip-field\": \"1.2.3.4\", \"timestamp\": 1}";
337+
TestHelpers.ingestDataToIndex(client(), INDEX_NAME, TestHelpers.toHttpEntity(testIndexData));
338+
AnomalyDetector detector = TestHelpers
339+
.randomDetector(
340+
ImmutableList.of(TestHelpers.randomFeature("feature_bytes", "agg", true)),
341+
INDEX_NAME,
342+
5,
343+
TIME_FIELD,
344+
null,
345+
ADCommonName.CUSTOM_RESULT_INDEX_PREFIX + "test"
346+
);
347+
Response response = TestHelpers
348+
.makeRequest(client(), "POST", TestHelpers.AD_BASE_DETECTORS_URI, ImmutableMap.of(), TestHelpers.toHttpEntity(detector), null);
349+
assertEquals("Create anomaly detector failed", RestStatus.CREATED, TestHelpers.restStatus(response));
350+
Map<String, Object> responseMap = entityAsMap(response);
351+
String id = (String) responseMap.get("_id");
352+
List<Feature> features = detector.getFeatureAttributes();
353+
long expectedFeatures = features.stream().filter(Feature::getEnabled).count();
354+
AnomalyDetector newDetector = new AnomalyDetector(
355+
id,
356+
null,
357+
detector.getName(),
358+
detector.getDescription(),
359+
detector.getTimeField(),
360+
detector.getIndices(),
361+
features,
362+
detector.getFilterQuery(),
363+
detector.getInterval(),
364+
detector.getWindowDelay(),
365+
detector.getShingleSize(),
366+
detector.getUiMetadata(),
367+
detector.getSchemaVersion(),
368+
detector.getLastUpdateTime(),
369+
detector.getCategoryFields(),
370+
detector.getUser(),
371+
detector.getCustomResultIndexOrAlias(),
372+
TestHelpers.randomImputationOption(features),
373+
randomIntBetween(1, 10000),
374+
randomInt(TimeSeriesSettings.MAX_SHINGLE_SIZE / 2),
375+
randomIntBetween(1, 1000),
376+
detector.getRules(),
377+
null,
378+
null,
379+
null,
380+
true,
381+
detector.getLastBreakingUIChangeTime()
382+
);
383+
384+
Exception ex = expectThrows(
385+
ResponseException.class,
386+
() -> TestHelpers
387+
.makeRequest(
388+
client(),
389+
"PUT",
390+
TestHelpers.AD_BASE_DETECTORS_URI + "/" + id + "?refresh=true",
391+
ImmutableMap.of(),
392+
TestHelpers.toHttpEntity(newDetector),
393+
null
394+
)
395+
);
396+
assertThat(ex.getMessage(), containsString(CommonMessages.CAN_NOT_CHANGE_FLATTEN_RESULT_INDEX));
397+
}
398+
334399
public void testCreateAnomalyDetector() throws Exception {
335400
AnomalyDetector detector = createIndexAndGetAnomalyDetector(INDEX_NAME);
336401
updateClusterSettings(ADEnabledSetting.AD_ENABLED, false);

0 commit comments

Comments
 (0)