diff --git a/plugin/core/src/main/java/org/elasticsearch/xpack/ml/job/config/Detector.java b/plugin/core/src/main/java/org/elasticsearch/xpack/ml/job/config/Detector.java index 7020dfec7b9..214f55fa584 100644 --- a/plugin/core/src/main/java/org/elasticsearch/xpack/ml/job/config/Detector.java +++ b/plugin/core/src/main/java/org/elasticsearch/xpack/ml/job/config/Detector.java @@ -767,8 +767,7 @@ public class Detector implements ToXContentObject, Writeable { } public List extractAnalysisFields() { - List analysisFields = Arrays.asList(byFieldName, - overFieldName, partitionFieldName); + List analysisFields = Arrays.asList(byFieldName, overFieldName, partitionFieldName); return analysisFields.stream().filter(item -> item != null).collect(Collectors.toList()); } @@ -800,8 +799,21 @@ public class Detector implements ToXContentObject, Writeable { private void checkScoping(DetectionRule rule) throws ElasticsearchParseException { String targetFieldName = rule.getTargetFieldName(); checkTargetFieldNameIsValid(extractAnalysisFields(), targetFieldName); - List validOptions = getValidFieldNameOptions(rule); for (RuleCondition condition : rule.getConditions()) { + List validOptions = Collections.emptyList(); + switch (condition.getType()) { + case CATEGORICAL: + validOptions = extractAnalysisFields(); + break; + case NUMERICAL_ACTUAL: + case NUMERICAL_TYPICAL: + case NUMERICAL_DIFF_ABS: + validOptions = getValidFieldNameOptionsForNumeric(rule); + break; + case TIME: + default: + break; + } if (!validOptions.contains(condition.getFieldName())) { String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_CONDITION_INVALID_FIELD_NAME, validOptions, condition.getFieldName()); @@ -819,7 +831,7 @@ public class Detector implements ToXContentObject, Writeable { } } - private List getValidFieldNameOptions(DetectionRule rule) { + private List getValidFieldNameOptionsForNumeric(DetectionRule rule) { List result = new ArrayList<>(); if (overFieldName != null) { result.add(byFieldName == null ? overFieldName : byFieldName); diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/config/DetectorTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/config/DetectorTests.java index ed47b3df599..af25bbccf0e 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/ml/job/config/DetectorTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/job/config/DetectorTests.java @@ -535,6 +535,38 @@ public class DetectorTests extends AbstractSerializingTestCase { detector.build(); } + public void testVerify_GivenCategoricalRuleOnAllPartitioningFields() { + Detector.Builder detector = new Detector.Builder("count", null); + detector.setPartitionFieldName("my_partition"); + detector.setOverFieldName("my_over"); + detector.setByFieldName("my_by"); + DetectionRule rule = new DetectionRule.Builder(Arrays.asList( + RuleCondition.createCategorical("my_partition", "my_filter_id"), + RuleCondition.createCategorical("my_over", "my_filter_id"), + RuleCondition.createCategorical("my_by", "my_filter_id") + )).build(); + detector.setRules(Collections.singletonList(rule)); + + detector.build(); + } + + public void testVerify_GivenCategoricalRuleOnInvalidField() { + Detector.Builder detector = new Detector.Builder("mean", "my_metric"); + detector.setPartitionFieldName("my_partition"); + detector.setOverFieldName("my_over"); + detector.setByFieldName("my_by"); + DetectionRule rule = new DetectionRule.Builder(Collections.singletonList( + RuleCondition.createCategorical("my_metric", "my_filter_id") + )).build(); + detector.setRules(Collections.singletonList(rule)); + + ElasticsearchException e = ESTestCase.expectThrows(ElasticsearchException.class, detector::build); + + assertEquals(Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_CONDITION_INVALID_FIELD_NAME, + "[my_by, my_over, my_partition]", "my_metric"), + e.getMessage()); + } + public void testVerify_GivenSameByAndPartition() { Detector.Builder detector = new Detector.Builder("count", ""); detector.setByFieldName("x"); diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml b/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml index 28d215d8cd3..5203ead3ce8 100644 --- a/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml +++ b/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml @@ -151,12 +151,13 @@ setup: "description":"Analysis of response time by airline", "analysis_config" : { "bucket_span": "3600s", - "detectors" :[{"function":"mean","field_name":"airline", + "detectors" :[{"function":"mean","field_name":"responsetime", "by_field_name": "airline", "rules": [ { "conditions": [ { "type": "categorical", + "field_name": "airline", "filter_id": "filter-foo" } ]