Adding more unit tests for validation API and some model classes (#335)

Signed-off-by: Amit Galitzky <[email protected]>

src/main/java/org/opensearch/ad/constant/CommonErrorMessages.java

@@ -13,6 +13,7 @@
 
 import static org.opensearch.ad.constant.CommonName.CUSTOM_RESULT_INDEX_PREFIX;
 import static org.opensearch.ad.model.AnomalyDetector.MAX_RESULT_INDEX_NAME_SIZE;
+import static org.opensearch.ad.rest.handler.AbstractAnomalyDetectorActionHandler.MAX_DETECTOR_NAME_SIZE;
 
 import java.util.Locale;
 
@@ -51,6 +52,7 @@ public class CommonErrorMessages {
     public static final String BUG_RESPONSE = "We might have bugs.";
     public static final String INDEX_NOT_FOUND = "index does not exist";
     public static final String NOT_EXISTENT_VALIDATION_TYPE = "The given validation type doesn't exist";
+    public static final String UNSUPPORTED_PROFILE_TYPE = "Unsupported profile types";
 
     private static final String TOO_MANY_CATEGORICAL_FIELD_ERR_MSG_FORMAT = "We can have only %d categorical field/s.";
 
@@ -73,8 +75,10 @@ public static String getTooManyCategoricalFieldErr(int limit) {
     public static String HISTORICAL_ANALYSIS_CANCELLED = "Historical analysis cancelled by user";
     public static String HC_DETECTOR_TASK_IS_UPDATING = "HC detector task is updating";
     public static String NEGATIVE_TIME_CONFIGURATION = "should be non-negative";
+    public static String INVALID_TIME_CONFIGURATION_UNITS = "Time unit %s is not supported";
     public static String INVALID_DETECTOR_NAME =
         "Valid characters for detector name are a-z, A-Z, 0-9, -(hyphen), _(underscore) and .(period)";
+    public static String DUPLICATE_FEATURE_AGGREGATION_NAMES = "Detector has duplicate feature aggregation query names: ";
 
     public static String FAIL_TO_GET_DETECTOR = "Fail to get detector";
     public static String FAIL_TO_GET_DETECTOR_INFO = "Fail to get detector info";
@@ -96,4 +100,7 @@ public static String getTooManyCategoricalFieldErr(int limit) {
     public static String INVALID_CHAR_IN_RESULT_INDEX_NAME =
         "Result index name has invalid character. Valid characters are a-z, 0-9, -(hyphen) and _(underscore)";
     public static String INVALID_RESULT_INDEX_MAPPING = "Result index mapping is not correct for index: ";
+    public static String INVALID_DETECTOR_NAME_SIZE = "Name should be shortened. The maximum limit is "
+        + MAX_DETECTOR_NAME_SIZE
+        + " characters.";
 }

src/main/java/org/opensearch/ad/model/ADEntityTaskProfile.java

@@ -14,6 +14,7 @@
 import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken;
 
 import java.io.IOException;
+import java.util.Objects;
 
 import org.opensearch.common.io.stream.StreamInput;
 import org.opensearch.common.io.stream.StreamOutput;
@@ -269,4 +270,38 @@ public String getAdTaskType() {
     public void setAdTaskType(String adTaskType) {
         this.adTaskType = adTaskType;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o)
+            return true;
+        if (o == null || getClass() != o.getClass())
+            return false;
+        ADEntityTaskProfile that = (ADEntityTaskProfile) o;
+        return Objects.equals(shingleSize, that.shingleSize)
+            && Objects.equals(rcfTotalUpdates, that.rcfTotalUpdates)
+            && Objects.equals(thresholdModelTrained, that.thresholdModelTrained)
+            && Objects.equals(thresholdModelTrainingDataSize, that.thresholdModelTrainingDataSize)
+            && Objects.equals(modelSizeInBytes, that.modelSizeInBytes)
+            && Objects.equals(nodeId, that.nodeId)
+            && Objects.equals(taskId, that.taskId)
+            && Objects.equals(adTaskType, that.adTaskType)
+            && Objects.equals(entity, that.entity);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects
+            .hash(
+                shingleSize,
+                rcfTotalUpdates,
+                thresholdModelTrained,
+                thresholdModelTrainingDataSize,
+                modelSizeInBytes,
+                nodeId,
+                entity,
+                taskId,
+                adTaskType
+            );
+    }
 }

src/main/java/org/opensearch/ad/model/DetectorProfileName.java

@@ -15,6 +15,7 @@
 import java.util.Set;
 
 import org.opensearch.ad.Name;
+import org.opensearch.ad.constant.CommonErrorMessages;
 import org.opensearch.ad.constant.CommonName;
 
 public enum DetectorProfileName implements Name {
@@ -68,7 +69,7 @@ public static DetectorProfileName getName(String name) {
             case CommonName.AD_TASK:
                 return AD_TASK;
             default:
-                throw new IllegalArgumentException("Unsupported profile types");
+                throw new IllegalArgumentException(CommonErrorMessages.UNSUPPORTED_PROFILE_TYPE);
         }
     }
 

src/main/java/org/opensearch/ad/model/IntervalTimeConfiguration.java

@@ -46,7 +46,7 @@ public IntervalTimeConfiguration(long interval, ChronoUnit unit) {
             );
         }
         if (!SUPPORTED_UNITS.contains(unit)) {
-            throw new IllegalArgumentException(String.format(Locale.ROOT, "Timezone %s is not supported", unit));
+            throw new IllegalArgumentException(String.format(Locale.ROOT, CommonErrorMessages.INVALID_TIME_CONFIGURATION_UNITS, unit));
         }
         this.interval = interval;
         this.unit = unit;

src/main/java/org/opensearch/ad/rest/handler/AbstractAnomalyDetectorActionHandler.java

@@ -119,7 +119,7 @@ public abstract class AbstractAnomalyDetectorActionHandler<T extends ActionRespo
     public static final String FEATURE_WITH_INVALID_QUERY_MSG = FEATURE_INVALID_MSG_PREFIX + " causing a runtime exception: ";
     public static final String UNKNOWN_SEARCH_QUERY_EXCEPTION_MSG =
         "Feature has an unknown exception caught while executing the feature query: ";
-
+    public static final String VALIDATION_FEATURE_FAILURE = "Validation failed for feature(s) of detector %s";
     public static final String NAME_REGEX = "[a-zA-Z0-9._-]+";
     public static final Integer MAX_DETECTOR_NAME_SIZE = 64;
 
@@ -303,7 +303,7 @@ protected void validateDetectorName(boolean indexingDryRun) {
             listener
                 .onFailure(
                     new ADValidationException(
-                        "Name should be shortened. The maximum limit is " + MAX_DETECTOR_NAME_SIZE + " characters.",
+                        CommonErrorMessages.INVALID_DETECTOR_NAME_SIZE,
                         DetectorValidationIssueType.NAME,
                         ValidationAspect.DETECTOR
                     )
@@ -405,7 +405,6 @@ protected void validateAgainstExistingMultiEntityAnomalyDetector(String detector
             SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(query).size(0).timeout(requestTimeout);
 
             SearchRequest searchRequest = new SearchRequest(ANOMALY_DETECTORS_INDEX).source(searchSourceBuilder);
-
             client
                 .search(
                     searchRequest,
@@ -817,7 +816,7 @@ protected void validateAnomalyDetectorFeatures(String detectorId, boolean indexi
             new MultiResponsesDelegateActionListener<MergeableList<Optional<double[]>>>(
                 validateFeatureQueriesListener,
                 anomalyDetector.getFeatureAttributes().size(),
-                String.format(Locale.ROOT, "Validation failed for feature(s) of detector %s", anomalyDetector.getName()),
+                String.format(Locale.ROOT, VALIDATION_FEATURE_FAILURE, anomalyDetector.getName()),
                 false
             );
 

src/main/java/org/opensearch/ad/util/RestHandlerUtils.java

@@ -27,6 +27,7 @@
 import org.opensearch.action.search.ShardSearchFailure;
 import org.opensearch.ad.common.exception.AnomalyDetectionException;
 import org.opensearch.ad.common.exception.ResourceNotFoundException;
+import org.opensearch.ad.constant.CommonErrorMessages;
 import org.opensearch.ad.model.AnomalyDetector;
 import org.opensearch.ad.model.Feature;
 import org.opensearch.common.Strings;
@@ -151,7 +152,7 @@ private static String validateFeaturesConfig(List<Feature> features) {
             errorMsgBuilder.append(". ");
         }
         if (duplicateFeatureAggNames.size() > 0) {
-            errorMsgBuilder.append("Detector has duplicate feature aggregation query names: ");
+            errorMsgBuilder.append(CommonErrorMessages.DUPLICATE_FEATURE_AGGREGATION_NAMES);
             errorMsgBuilder.append(String.join(", ", duplicateFeatureAggNames));
         }
         return errorMsgBuilder.toString();

src/test/java/org/opensearch/action/admin/indices/mapping/get/IndexAnomalyDetectorActionHandlerTests.java

@@ -32,6 +32,7 @@
 import org.junit.BeforeClass;
 import org.junit.Ignore;
 import org.mockito.ArgumentCaptor;
+import org.opensearch.OpenSearchStatusException;
 import org.opensearch.action.ActionListener;
 import org.opensearch.action.ActionRequest;
 import org.opensearch.action.ActionResponse;
@@ -46,10 +47,12 @@
 import org.opensearch.ad.AbstractADTest;
 import org.opensearch.ad.TestHelpers;
 import org.opensearch.ad.common.exception.ADValidationException;
+import org.opensearch.ad.constant.CommonErrorMessages;
 import org.opensearch.ad.constant.CommonName;
 import org.opensearch.ad.feature.SearchFeatureDao;
 import org.opensearch.ad.indices.AnomalyDetectionIndices;
 import org.opensearch.ad.model.AnomalyDetector;
+import org.opensearch.ad.model.Feature;
 import org.opensearch.ad.rest.handler.IndexAnomalyDetectorActionHandler;
 import org.opensearch.ad.task.ADTaskManager;
 import org.opensearch.ad.transport.IndexAnomalyDetectorResponse;
@@ -66,6 +69,8 @@
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.TransportService;
 
+import com.google.common.collect.ImmutableList;
+
 /**
  *
  * we need to put the test in the same package of GetFieldMappingsResponse
@@ -703,4 +708,50 @@ public void testTenMultiEntityDetectorsUpdateExistingMultiEntityAd() throws IOEx
         assertTrue(value instanceof IllegalStateException);
         assertTrue(value.getMessage().contains("NodeClient has not been initialized"));
     }
+
+    @SuppressWarnings("unchecked")
+    public void testCreateAnomalyDetectorWithDuplicateFeatureAggregationNames() throws IOException {
+        Feature featureOne = TestHelpers.randomFeature("featureName", "test-1");
+        Feature featureTwo = TestHelpers.randomFeature("featureNameTwo", "test-1");
+        AnomalyDetector anomalyDetector = TestHelpers.randomAnomalyDetector(ImmutableList.of(featureOne, featureTwo));
+        SearchResponse mockResponse = mock(SearchResponse.class);
+        when(mockResponse.getHits()).thenReturn(TestHelpers.createSearchHits(9));
+        doAnswer(invocation -> {
+            Object[] args = invocation.getArguments();
+            assertTrue(String.format("The size of args is %d.  Its content is %s", args.length, Arrays.toString(args)), args.length == 2);
+            assertTrue(args[0] instanceof SearchRequest);
+            assertTrue(args[1] instanceof ActionListener);
+            ActionListener<SearchResponse> listener = (ActionListener<SearchResponse>) args[1];
+            listener.onResponse(mockResponse);
+            return null;
+        }).when(clientMock).search(any(SearchRequest.class), any());
+
+        handler = new IndexAnomalyDetectorActionHandler(
+            clusterService,
+            clientMock,
+            transportService,
+            channel,
+            anomalyDetectionIndices,
+            detectorId,
+            seqNo,
+            primaryTerm,
+            refreshPolicy,
+            anomalyDetector,
+            requestTimeout,
+            maxSingleEntityAnomalyDetectors,
+            maxMultiEntityAnomalyDetectors,
+            maxAnomalyFeatures,
+            method,
+            xContentRegistry(),
+            null,
+            adTaskManager,
+            searchFeatureDao
+        );
+        ArgumentCaptor<Exception> response = ArgumentCaptor.forClass(Exception.class);
+        handler.start();
+        verify(channel).onFailure(response.capture());
+        Exception value = response.getValue();
+        assertTrue(value instanceof OpenSearchStatusException);
+        assertTrue(value.getMessage().contains(CommonErrorMessages.DUPLICATE_FEATURE_AGGREGATION_NAMES));
+    }
 }

src/test/java/org/opensearch/action/admin/indices/mapping/get/ValidateAnomalyDetectorActionHandlerTests.java

@@ -121,7 +121,7 @@ public void setUp() throws Exception {
     @SuppressWarnings("unchecked")
     public void testValidateMoreThanThousandSingleEntityDetectorLimit() throws IOException {
         SearchResponse mockResponse = mock(SearchResponse.class);
-        int totalHits = 1001;
+        int totalHits = maxSingleEntityAnomalyDetectors + 1;
         when(mockResponse.getHits()).thenReturn(TestHelpers.createSearchHits(totalHits));
         doAnswer(invocation -> {
             Object[] args = invocation.getArguments();
@@ -152,7 +152,6 @@ public void testValidateMoreThanThousandSingleEntityDetectorLimit() throws IOExc
             searchFeatureDao,
             ValidationAspect.DETECTOR.getName()
         );
-
         handler.start();
         ArgumentCaptor<Exception> response = ArgumentCaptor.forClass(Exception.class);
         verify(clientMock, never()).execute(eq(GetMappingsAction.INSTANCE), any(), any());
@@ -172,7 +171,7 @@ public void testValidateMoreThanThousandSingleEntityDetectorLimit() throws IOExc
     public void testValidateMoreThanTenMultiEntityDetectorsLimit() throws IOException {
         SearchResponse mockResponse = mock(SearchResponse.class);
 
-        int totalHits = 11;
+        int totalHits = maxMultiEntityAnomalyDetectors + 1;
 
         when(mockResponse.getHits()).thenReturn(TestHelpers.createSearchHits(totalHits));
 
@@ -204,7 +203,7 @@ public void testValidateMoreThanTenMultiEntityDetectorsLimit() throws IOExceptio
             xContentRegistry(),
             null,
             searchFeatureDao,
-            ValidationAspect.DETECTOR.getName()
+            ""
         );
         handler.start();
 

src/test/java/org/opensearch/ad/ADIntegTestCase.java

@@ -136,6 +136,10 @@ public void createADResultIndex() throws IOException {
         createIndex(CommonName.ANOMALY_RESULT_INDEX_ALIAS, AnomalyDetectionIndices.getAnomalyResultMappings());
     }
 
+    public void createCustomADResultIndex(String indexName) throws IOException {
+        createIndex(indexName, AnomalyDetectionIndices.getAnomalyResultMappings());
+    }
+
     public void createDetectionStateIndex() throws IOException {
         createIndex(CommonName.DETECTION_STATE_INDEX, AnomalyDetectionIndices.getDetectionStateMappings());
     }

src/test/java/org/opensearch/ad/TestHelpers.java

@@ -1440,11 +1440,20 @@ public static Map<String, Object> parseStatsResult(String statsResult) throws IO
     }
 
     public static DetectorValidationIssue randomDetectorValidationIssue() {
+        DetectorValidationIssue issue = new DetectorValidationIssue(
+            ValidationAspect.DETECTOR,
+            DetectorValidationIssueType.NAME,
+            randomAlphaOfLength(5)
+        );
+        return issue;
+    }
+
+    public static DetectorValidationIssue randomDetectorValidationIssueWithSubIssues(Map<String, String> subIssues) {
         DetectorValidationIssue issue = new DetectorValidationIssue(
             ValidationAspect.DETECTOR,
             DetectorValidationIssueType.NAME,
             randomAlphaOfLength(5),
-            null,
+            subIssues,
             null
         );
         return issue;