Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Commit

Permalink
Prevent creating detector with duplicate name. Issue:#118 (#134)
Browse files Browse the repository at this point in the history 
* Prevent creating detector with duplicate name. Issue:#118

* Exclude detector with input detector id when searching detectors with duplicate name
  • Loading branch information
@yizheliu-amazon
yizheliu-amazon authored May 22, 2020
1 parent 713c3af commit 64d7f0e
Show file tree
Hide file tree
Showing 2 changed files with 153 additions and 0 deletions.
38 changes: 38 additions & 0 deletions .../amazon/opendistroforelasticsearch/ad/rest/handler/IndexAnomalyDetectorActionHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.query.BoolQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.rest.BytesRestResponse;
Expand All @@ -50,6 +51,7 @@
import java.time.Instant;
import java.util.Arrays;
import java.util.Locale;
import java.util.stream.Collectors;

import static com.amazon.opendistroforelasticsearch.ad.model.AnomalyDetector.ANOMALY_DETECTORS_INDEX;
import static com.amazon.opendistroforelasticsearch.ad.util.RestHandlerUtils.XCONTENT_WITH_TYPE;
Expand Down Expand Up @@ -221,6 +223,42 @@ private void onSearchAdInputIndicesResponse(SearchResponse response, String dete
+ Arrays.toString(anomalyDetector.getIndices().toArray(new String[0]));
logger.error(errorMsg);
onFailure(new IllegalArgumentException(errorMsg));
} else {
checkADNameExists(detectorId);
}
}

private void checkADNameExists(String detectorId) throws IOException {
BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder();
// src/main/resources/mappings/anomaly-detectors.json#L14
boolQueryBuilder.must(QueryBuilders.termQuery("name.keyword", anomalyDetector.getName()));
if (StringUtils.isNotBlank(detectorId)) {
boolQueryBuilder.mustNot(QueryBuilders.termQuery(RestHandlerUtils._ID, detectorId));
}
SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(boolQueryBuilder).timeout(requestTimeout);
SearchRequest searchRequest = new SearchRequest(ANOMALY_DETECTORS_INDEX).source(searchSourceBuilder);

client
.search(
searchRequest,
ActionListener
.wrap(
searchResponse -> onSearchADNameResponse(searchResponse, detectorId, anomalyDetector.getName()),
exception -> onFailure(exception)
)
);
}

private void onSearchADNameResponse(SearchResponse response, String detectorId, String name) throws IOException {
if (response.getHits().getTotalHits().value > 0) {
String errorMsg = String
.format(
"Cannot create anomaly detector with name [%s] as it's already used by detector %s",
name,
Arrays.stream(response.getHits().getHits()).map(hit -> hit.getId()).collect(Collectors.toList())
);
logger.warn(errorMsg);
onFailure(new IllegalArgumentException(errorMsg));
} else {
indexAnomalyDetector(detectorId);
}
Expand Down
115 changes: 115 additions & 0 deletions src/test/java/com/amazon/opendistroforelasticsearch/ad/rest/AnomalyDetectorRestApiIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@

import static com.amazon.opendistroforelasticsearch.ad.TestHelpers.AD_BASE_PREVIEW_URI;
import static com.amazon.opendistroforelasticsearch.ad.TestHelpers.randomAnomalyDetectorWithEmptyFeature;
import static com.amazon.opendistroforelasticsearch.ad.TestHelpers.randomFeature;
import static com.amazon.opendistroforelasticsearch.ad.TestHelpers.randomIntervalTimeConfiguration;
import static com.amazon.opendistroforelasticsearch.ad.TestHelpers.randomQuery;
import static com.amazon.opendistroforelasticsearch.ad.TestHelpers.randomUiMetadata;
import static org.hamcrest.Matchers.containsString;

public class AnomalyDetectorRestApiIT extends AnomalyDetectorRestTestCase {
Expand Down Expand Up @@ -78,6 +82,41 @@ public void testCreateAnomalyDetectorWithEmptyIndices() throws Exception {
);
}

public void testCreateAnomalyDetectorWithDuplicateName() throws Exception {
AnomalyDetector detector = createRandomAnomalyDetector(true, true);

AnomalyDetector detectorDuplicateName = new AnomalyDetector(
AnomalyDetector.NO_ID,
randomLong(),
detector.getName(),
randomAlphaOfLength(5),
randomAlphaOfLength(5),
detector.getIndices(),
ImmutableList.of(randomFeature()),
randomQuery(),
randomIntervalTimeConfiguration(),
randomIntervalTimeConfiguration(),
randomUiMetadata(),
randomInt(),
null
);

TestHelpers
.assertFailWith(
ResponseException.class,
"Cannot create anomaly detector with name",
() -> TestHelpers
.makeRequest(
client(),
"POST",
TestHelpers.AD_BASE_DETECTORS_URI,
ImmutableMap.of(),
toHttpEntity(detectorDuplicateName),
null
)
);
}

public void testCreateAnomalyDetector() throws Exception {
AnomalyDetector detector = TestHelpers.randomAnomalyDetector(TestHelpers.randomUiMetadata(), null);
String indexName = detector.getIndices().get(0);
Expand Down Expand Up @@ -181,6 +220,82 @@ public void testUpdateAnomalyDetectorA() throws Exception {
assertEquals("Anomaly detector description not updated", newDescription, updatedDetector.getDescription());
}

public void testUpdateAnomalyDetectorNameToExisting() throws Exception {
AnomalyDetector detector1 = createRandomAnomalyDetector(true, true);

AnomalyDetector detector2 = createRandomAnomalyDetector(true, true);

AnomalyDetector newDetector1WithDetector2Name = new AnomalyDetector(
detector1.getDetectorId(),
detector1.getVersion(),
detector2.getName(),
detector1.getDescription(),
detector1.getTimeField(),
detector1.getIndices(),
detector1.getFeatureAttributes(),
detector1.getFilterQuery(),
detector1.getDetectionInterval(),
detector1.getWindowDelay(),
detector1.getUiMetadata(),
detector1.getSchemaVersion(),
detector1.getLastUpdateTime()
);

TestHelpers
.assertFailWith(
ResponseException.class,
"Cannot create anomaly detector with name",
() -> TestHelpers
.makeRequest(
client(),
"POST",
TestHelpers.AD_BASE_DETECTORS_URI,
ImmutableMap.of(),
toHttpEntity(newDetector1WithDetector2Name),
null
)
);
}

public void testUpdateAnomalyDetectorNameToNew() throws Exception {
AnomalyDetector detector = createRandomAnomalyDetector(true, true);

AnomalyDetector detectorWithNewName = new AnomalyDetector(
detector.getDetectorId(),
detector.getVersion(),
randomAlphaOfLength(5),
detector.getDescription(),
detector.getTimeField(),
detector.getIndices(),
detector.getFeatureAttributes(),
detector.getFilterQuery(),
detector.getDetectionInterval(),
detector.getWindowDelay(),
detector.getUiMetadata(),
detector.getSchemaVersion(),
Instant.now()
);

TestHelpers
.makeRequest(
client(),
"PUT",
TestHelpers.AD_BASE_DETECTORS_URI + "/" + detector.getDetectorId() + "?refresh=true",
ImmutableMap.of(),
toHttpEntity(detectorWithNewName),
null
);

AnomalyDetector resultDetector = getAnomalyDetector(detectorWithNewName.getDetectorId());
assertEquals("Detector name updating failed", detectorWithNewName.getName(), resultDetector.getName());
assertEquals("Updated anomaly detector id doesn't match", detectorWithNewName.getDetectorId(), resultDetector.getDetectorId());
assertNotEquals(
"Anomaly detector last update time not changed",
detectorWithNewName.getLastUpdateTime(),
resultDetector.getLastUpdateTime()
);
}

public void testUpdateAnomalyDetectorWithNotExistingIndex() throws Exception {
AnomalyDetector detector = createRandomAnomalyDetector(true, true);

Expand Down

0 comments on commit 64d7f0e

Please sign in to comment.