diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEvent.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEvent.java index 68e1201816dc4..79e569987fa02 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEvent.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEvent.java @@ -16,7 +16,6 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.xpack.core.ml.MlMetaIndex; -import org.elasticsearch.xpack.core.ml.job.config.Connective; import org.elasticsearch.xpack.core.ml.job.config.DetectionRule; import org.elasticsearch.xpack.core.ml.job.config.Operator; import org.elasticsearch.xpack.core.ml.job.config.RuleAction; @@ -148,8 +147,7 @@ public DetectionRule toDetectionRule(TimeValue bucketSpan) { conditions.add(RuleCondition.createTime(Operator.LT, bucketEndTime)); DetectionRule.Builder builder = new DetectionRule.Builder(conditions); - builder.setActions(RuleAction.FILTER_RESULTS, RuleAction.SKIP_SAMPLING); - builder.setConditionsConnective(Connective.AND); + builder.setActions(RuleAction.SKIP_RESULT, RuleAction.SKIP_MODEL_UPDATE); return builder.build(); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Condition.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Condition.java deleted file mode 100644 index 7d3074df0ae28..0000000000000 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Condition.java +++ /dev/null @@ -1,132 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.core.ml.job.config; - -import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.io.stream.Writeable; -import org.elasticsearch.common.xcontent.ConstructingObjectParser; -import org.elasticsearch.common.xcontent.ObjectParser.ValueType; -import org.elasticsearch.common.xcontent.ToXContentObject; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.xpack.core.ml.job.messages.Messages; -import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; - -import java.io.IOException; -import java.util.Objects; -import java.util.regex.Pattern; -import java.util.regex.PatternSyntaxException; - -/** - * A class that describes a condition. - * The {@linkplain Operator} enum defines the available - * comparisons a condition can use. - */ -public class Condition implements ToXContentObject, Writeable { - public static final ParseField CONDITION_FIELD = new ParseField("condition"); - public static final ParseField VALUE_FIELD = new ParseField("value"); - - public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( - CONDITION_FIELD.getPreferredName(), a -> new Condition((Operator) a[0], (String) a[1])); - - static { - PARSER.declareField(ConstructingObjectParser.constructorArg(), p -> { - if (p.currentToken() == XContentParser.Token.VALUE_STRING) { - return Operator.fromString(p.text()); - } - throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]"); - }, Operator.OPERATOR_FIELD, ValueType.STRING); - PARSER.declareField(ConstructingObjectParser.constructorArg(), p -> { - if (p.currentToken() == XContentParser.Token.VALUE_STRING) { - return p.text(); - } - if (p.currentToken() == XContentParser.Token.VALUE_NULL) { - return null; - } - throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]"); - }, VALUE_FIELD, ValueType.STRING_OR_NULL); - } - - private final Operator op; - private final String value; - - public Condition(StreamInput in) throws IOException { - op = Operator.readFromStream(in); - value = in.readOptionalString(); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - op.writeTo(out); - out.writeOptionalString(value); - } - - public Condition(Operator op, String value) { - if (value == null) { - throw ExceptionsHelper.badRequestException(Messages.getMessage(Messages.JOB_CONFIG_CONDITION_INVALID_VALUE_NULL)); - } - - if (op.expectsANumericArgument()) { - try { - Double.parseDouble(value); - } catch (NumberFormatException nfe) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_CONDITION_INVALID_VALUE_NUMBER, value); - throw ExceptionsHelper.badRequestException(msg); - } - } else { - try { - Pattern.compile(value); - } catch (PatternSyntaxException e) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_CONDITION_INVALID_VALUE_REGEX, value); - throw ExceptionsHelper.badRequestException(msg); - } - } - this.op = op; - this.value = value; - } - - public Operator getOperator() { - return op; - } - - public String getValue() { - return value; - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - builder.field(Operator.OPERATOR_FIELD.getPreferredName(), op); - builder.field(VALUE_FIELD.getPreferredName(), value); - builder.endObject(); - return builder; - } - - @Override - public int hashCode() { - return Objects.hash(op, value); - } - - @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - if (obj == null) { - return false; - } - - if (getClass() != obj.getClass()) { - return false; - } - - Condition other = (Condition) obj; - return Objects.equals(this.op, other.op) && - Objects.equals(this.value, other.value); - } -} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Connective.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Connective.java deleted file mode 100644 index 0b4ad010fdd32..0000000000000 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Connective.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.core.ml.job.config; - -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.io.stream.Writeable; - -import java.io.IOException; -import java.util.Locale; - -public enum Connective implements Writeable { - OR, AND; - - /** - * Case-insensitive from string method. - * - * @param value - * String representation - * @return The connective type - */ - public static Connective fromString(String value) { - return Connective.valueOf(value.toUpperCase(Locale.ROOT)); - } - - public static Connective readFromStream(StreamInput in) throws IOException { - return in.readEnum(Connective.class); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeEnum(this); - } - - @Override - public String toString() { - return name().toLowerCase(Locale.ROOT); - } -} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/DetectionRule.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/DetectionRule.java index 0948e978c886e..fbdb2f6662a77 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/DetectionRule.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/DetectionRule.java @@ -6,22 +6,18 @@ package org.elasticsearch.xpack.core.ml.job.config; import org.elasticsearch.Version; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.ObjectParser; -import org.elasticsearch.common.xcontent.ObjectParser.ValueType; import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.xpack.core.ml.MlParserType; import org.elasticsearch.xpack.core.ml.job.messages.Messages; import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.EnumMap; @@ -30,16 +26,15 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.stream.Collectors; public class DetectionRule implements ToXContentObject, Writeable { + public static final Version VERSION_INTRODUCED = Version.V_6_4_0; + public static final ParseField DETECTION_RULE_FIELD = new ParseField("detection_rule"); - public static final ParseField ACTIONS_FIELD = new ParseField("actions", "rule_action"); - public static final ParseField TARGET_FIELD_NAME_FIELD = new ParseField("target_field_name"); - public static final ParseField TARGET_FIELD_VALUE_FIELD = new ParseField("target_field_value"); - public static final ParseField CONDITIONS_CONNECTIVE_FIELD = new ParseField("conditions_connective"); - public static final ParseField CONDITIONS_FIELD = new ParseField("conditions", "rule_conditions"); + public static final ParseField ACTIONS_FIELD = new ParseField("actions"); + public static final ParseField SCOPE_FIELD = new ParseField("scope"); + public static final ParseField CONDITIONS_FIELD = new ParseField("conditions"); // These parsers follow the pattern that metadata is parsed leniently (to allow for enhancements), whilst config is parsed strictly public static final ObjectParser METADATA_PARSER = @@ -55,87 +50,44 @@ public class DetectionRule implements ToXContentObject, Writeable { ObjectParser parser = PARSERS.get(parserType); assert parser != null; parser.declareStringArray(Builder::setActions, ACTIONS_FIELD); - parser.declareString(Builder::setTargetFieldName, TARGET_FIELD_NAME_FIELD); - parser.declareString(Builder::setTargetFieldValue, TARGET_FIELD_VALUE_FIELD); - parser.declareField(Builder::setConditionsConnective, p -> { - if (p.currentToken() == XContentParser.Token.VALUE_STRING) { - return Connective.fromString(p.text()); - } - throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]"); - }, CONDITIONS_CONNECTIVE_FIELD, ValueType.STRING); + parser.declareObject(Builder::setScope, RuleScope.parser(parserType), SCOPE_FIELD); parser.declareObjectArray(Builder::setConditions, (p, c) -> RuleCondition.PARSERS.get(parserType).apply(p, c), CONDITIONS_FIELD); } } private final EnumSet actions; - private final String targetFieldName; - private final String targetFieldValue; - private final Connective conditionsConnective; + private final RuleScope scope; private final List conditions; - private DetectionRule(EnumSet actions, @Nullable String targetFieldName, @Nullable String targetFieldValue, - Connective conditionsConnective, List conditions) { + private DetectionRule(EnumSet actions, RuleScope scope, List conditions) { this.actions = Objects.requireNonNull(actions); - this.targetFieldName = targetFieldName; - this.targetFieldValue = targetFieldValue; - this.conditionsConnective = Objects.requireNonNull(conditionsConnective); + this.scope = Objects.requireNonNull(scope); this.conditions = Collections.unmodifiableList(conditions); } public DetectionRule(StreamInput in) throws IOException { - actions = EnumSet.noneOf(RuleAction.class); - if (in.getVersion().before(Version.V_6_2_0)) { - actions.add(RuleAction.readFromStream(in)); - } else { - int actionsCount = in.readVInt(); - for (int i = 0; i < actionsCount; ++i) { - actions.add(RuleAction.readFromStream(in)); - } - } - - conditionsConnective = Connective.readFromStream(in); - int size = in.readVInt(); - conditions = new ArrayList<>(size); - for (int i = 0; i < size; i++) { - conditions.add(new RuleCondition(in)); - } - targetFieldName = in.readOptionalString(); - targetFieldValue = in.readOptionalString(); + actions = in.readEnumSet(RuleAction.class); + scope = new RuleScope(in); + conditions = in.readList(RuleCondition::new); } @Override public void writeTo(StreamOutput out) throws IOException { - if (out.getVersion().before(Version.V_6_2_0)) { - // Only filter_results is supported prior to 6.2.0 - RuleAction.FILTER_RESULTS.writeTo(out); - } else { - out.writeVInt(actions.size()); - for (RuleAction action : actions) { - action.writeTo(out); - } - } - - conditionsConnective.writeTo(out); - out.writeVInt(conditions.size()); - for (RuleCondition condition : conditions) { - condition.writeTo(out); - } - out.writeOptionalString(targetFieldName); - out.writeOptionalString(targetFieldValue); + out.writeEnumSet(actions); + scope.writeTo(out); + out.writeList(conditions); } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); builder.field(ACTIONS_FIELD.getPreferredName(), actions); - builder.field(CONDITIONS_CONNECTIVE_FIELD.getPreferredName(), conditionsConnective); - builder.field(CONDITIONS_FIELD.getPreferredName(), conditions); - if (targetFieldName != null) { - builder.field(TARGET_FIELD_NAME_FIELD.getPreferredName(), targetFieldName); + if (scope.isEmpty() == false) { + builder.field(SCOPE_FIELD.getPreferredName(), scope); } - if (targetFieldValue != null) { - builder.field(TARGET_FIELD_VALUE_FIELD.getPreferredName(), targetFieldValue); + if (conditions.isEmpty() == false) { + builder.field(CONDITIONS_FIELD.getPreferredName(), conditions); } builder.endObject(); return builder; @@ -145,18 +97,8 @@ public EnumSet getActions() { return actions; } - @Nullable - public String getTargetFieldName() { - return targetFieldName; - } - - @Nullable - public String getTargetFieldValue() { - return targetFieldValue; - } - - public Connective getConditionsConnective() { - return conditionsConnective; + public RuleScope getScope() { + return scope; } public List getConditions() { @@ -164,7 +106,7 @@ public List getConditions() { } public Set extractReferencedFilters() { - return conditions.stream().map(RuleCondition::getFilterId).filter(Objects::nonNull).collect(Collectors.toSet()); + return scope.getReferencedFilters(); } @Override @@ -179,29 +121,29 @@ public boolean equals(Object obj) { DetectionRule other = (DetectionRule) obj; return Objects.equals(actions, other.actions) - && Objects.equals(targetFieldName, other.targetFieldName) - && Objects.equals(targetFieldValue, other.targetFieldValue) - && Objects.equals(conditionsConnective, other.conditionsConnective) + && Objects.equals(scope, other.scope) && Objects.equals(conditions, other.conditions); } @Override public int hashCode() { - return Objects.hash(actions, targetFieldName, targetFieldValue, conditionsConnective, conditions); + return Objects.hash(actions, scope, conditions); } public static class Builder { - private EnumSet actions = EnumSet.of(RuleAction.FILTER_RESULTS); - private String targetFieldName; - private String targetFieldValue; - private Connective conditionsConnective = Connective.OR; + private EnumSet actions = EnumSet.of(RuleAction.SKIP_RESULT); + private RuleScope scope = new RuleScope(); private List conditions = Collections.emptyList(); + public Builder(RuleScope.Builder scope) { + this.scope = scope.build(); + } + public Builder(List conditions) { this.conditions = ExceptionsHelper.requireNonNull(conditions, CONDITIONS_FIELD.getPreferredName()); } - private Builder() { + Builder() { } public Builder setActions(List actions) { @@ -221,18 +163,8 @@ public Builder setActions(RuleAction... actions) { return this; } - public Builder setTargetFieldName(String targetFieldName) { - this.targetFieldName = targetFieldName; - return this; - } - - public Builder setTargetFieldValue(String targetFieldValue) { - this.targetFieldValue = targetFieldValue; - return this; - } - - public Builder setConditionsConnective(Connective connective) { - this.conditionsConnective = ExceptionsHelper.requireNonNull(connective, CONDITIONS_CONNECTIVE_FIELD.getPreferredName()); + public Builder setScope(RuleScope scope) { + this.scope = Objects.requireNonNull(scope); return this; } @@ -242,22 +174,11 @@ public Builder setConditions(List conditions) { } public DetectionRule build() { - if (targetFieldValue != null && targetFieldName == null) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_MISSING_TARGET_FIELD_NAME, targetFieldValue); + if (scope.isEmpty() && conditions.isEmpty()) { + String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_REQUIRES_SCOPE_OR_CONDITION); throw ExceptionsHelper.badRequestException(msg); } - if (conditions == null || conditions.isEmpty()) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_REQUIRES_AT_LEAST_ONE_CONDITION); - throw ExceptionsHelper.badRequestException(msg); - } - for (RuleCondition condition : conditions) { - if (condition.getType().isCategorical() && targetFieldName != null) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_CONDITION_CATEGORICAL_INVALID_OPTION, - DetectionRule.TARGET_FIELD_NAME_FIELD.getPreferredName()); - throw ExceptionsHelper.badRequestException(msg); - } - } - return new DetectionRule(actions, targetFieldName, targetFieldValue, conditionsConnective, conditions); + return new DetectionRule(actions, scope, conditions); } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Detector.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Detector.java index e5cf4b16f6e73..bae5e654ba4fa 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Detector.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Detector.java @@ -34,6 +34,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.TreeSet; import java.util.stream.Collectors; @@ -84,8 +85,7 @@ public String toString() { public static final ParseField PARTITION_FIELD_NAME_FIELD = new ParseField("partition_field_name"); public static final ParseField USE_NULL_FIELD = new ParseField("use_null"); public static final ParseField EXCLUDE_FREQUENT_FIELD = new ParseField("exclude_frequent"); - // TODO: Remove the deprecated detector_rules setting in 7.0 - public static final ParseField RULES_FIELD = new ParseField("rules", "detector_rules"); + public static final ParseField CUSTOM_RULES_FIELD = new ParseField("custom_rules"); public static final ParseField DETECTOR_INDEX = new ParseField("detector_index"); // These parsers follow the pattern that metadata is parsed leniently (to allow for enhancements), whilst config is parsed strictly @@ -113,7 +113,7 @@ public String toString() { throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]"); }, EXCLUDE_FREQUENT_FIELD, ObjectParser.ValueType.STRING); parser.declareObjectArray(Builder::setRules, (p, c) -> - DetectionRule.PARSERS.get(parserType).apply(p, c).build(), RULES_FIELD); + DetectionRule.PARSERS.get(parserType).apply(p, c).build(), CUSTOM_RULES_FIELD); parser.declareInt(Builder::setDetectorIndex, DETECTOR_INDEX); } } @@ -209,6 +209,19 @@ public String toString() { DetectorFunction.TIME_OF_WEEK ); + /** + * Functions that do not support rule conditions: + *
    + *
  • lat_long - because it is a multivariate feature + *
  • metric - because having the same conditions on min,max,mean is + * error-prone + *
  • rare - because the actual/typical value is not something a user can anticipate + *
  • freq_rare - because the actual/typical value is not something a user can anticipate + *
+ */ + static final EnumSet FUNCTIONS_WITHOUT_RULE_CONDITION_SUPPORT = EnumSet.of( + DetectorFunction.LAT_LONG, DetectorFunction.METRIC, DetectorFunction.RARE, DetectorFunction.FREQ_RARE); + /** * field names cannot contain any of these characters * ", \ @@ -263,7 +276,11 @@ public void writeTo(StreamOutput out) throws IOException { } else { out.writeBoolean(false); } - out.writeList(rules); + if (out.getVersion().onOrAfter(DetectionRule.VERSION_INTRODUCED)) { + out.writeList(rules); + } else { + out.writeList(Collections.emptyList()); + } if (out.getVersion().onOrAfter(Version.V_5_5_0)) { out.writeInt(detectorIndex); } @@ -293,7 +310,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(EXCLUDE_FREQUENT_FIELD.getPreferredName(), excludeFrequent); } if (rules.isEmpty() == false) { - builder.field(RULES_FIELD.getPreferredName(), rules); + builder.field(CUSTOM_RULES_FIELD.getPreferredName(), rules); } // negative means "unknown", which should only happen for a 5.4 job if (detectorIndex >= 0 @@ -467,17 +484,6 @@ public int hashCode() { public static class Builder { - /** - * Functions that do not support rules: - *
    - *
  • lat_long - because it is a multivariate feature - *
  • metric - because having the same conditions on min,max,mean is - * error-prone - *
- */ - static final EnumSet FUNCTIONS_WITHOUT_RULE_SUPPORT = EnumSet.of( - DetectorFunction.LAT_LONG, DetectorFunction.METRIC); - private String detectorDescription; private DetectorFunction function; private String fieldName; @@ -598,14 +604,8 @@ public Detector build() { } DetectorFunction function = this.function == null ? DetectorFunction.METRIC : this.function; - if (rules.isEmpty() == false) { - if (FUNCTIONS_WITHOUT_RULE_SUPPORT.contains(function)) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_NOT_SUPPORTED_BY_FUNCTION, function); - throw ExceptionsHelper.badRequestException(msg); - } - for (DetectionRule rule : rules) { - checkScoping(rule); - } + for (DetectionRule rule : rules) { + validateRule(rule, function); } // partition, by and over field names cannot be duplicates @@ -691,96 +691,37 @@ private static boolean containsInvalidChar(String field) { return field.chars().anyMatch(Character::isISOControl); } - private void checkScoping(DetectionRule rule) throws ElasticsearchParseException { - String targetFieldName = rule.getTargetFieldName(); - checkTargetFieldNameIsValid(extractAnalysisFields(), targetFieldName); - for (RuleCondition condition : rule.getConditions()) { - List validOptions = Collections.emptyList(); - switch (condition.getType()) { - case CATEGORICAL: - case CATEGORICAL_COMPLEMENT: - 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()); - throw ExceptionsHelper.badRequestException(msg); - } - } + private void validateRule(DetectionRule rule, DetectorFunction function) { + checkFunctionHasRuleSupport(rule, function); + checkScoping(rule); } - private void checkTargetFieldNameIsValid(List analysisFields, String targetFieldName) - throws ElasticsearchParseException { - if (targetFieldName != null && !analysisFields.contains(targetFieldName)) { - String msg = - Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_INVALID_TARGET_FIELD_NAME, analysisFields, targetFieldName); + private void checkFunctionHasRuleSupport(DetectionRule rule, DetectorFunction function) { + if (ruleHasConditionOnResultValue(rule) && FUNCTIONS_WITHOUT_RULE_CONDITION_SUPPORT.contains(function)) { + String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_NOT_SUPPORTED_BY_FUNCTION, function); throw ExceptionsHelper.badRequestException(msg); } } - private List getValidFieldNameOptionsForNumeric(DetectionRule rule) { - List result = new ArrayList<>(); - if (overFieldName != null) { - result.add(byFieldName == null ? overFieldName : byFieldName); - } else if (byFieldName != null) { - result.add(byFieldName); - } - - if (rule.getTargetFieldName() != null) { - ScopingLevel targetLevel = ScopingLevel.from(this, rule.getTargetFieldName()); - result = result.stream().filter(field -> targetLevel.isHigherThan(ScopingLevel.from(this, field))) - .collect(Collectors.toList()); - } - - if (isEmptyFieldNameAllowed(rule)) { - result.add(null); - } - return result; - } - - private boolean isEmptyFieldNameAllowed(DetectionRule rule) { - List analysisFields = extractAnalysisFields(); - return analysisFields.isEmpty() || (rule.getTargetFieldName() != null && analysisFields.size() == 1); - } - - enum ScopingLevel { - PARTITION(3), - OVER(2), - BY(1); - - int level; - - ScopingLevel(int level) { - this.level = level; - } - - boolean isHigherThan(ScopingLevel other) { - return level > other.level; - } - - static ScopingLevel from(Detector.Builder detector, String fieldName) { - if (fieldName.equals(detector.partitionFieldName)) { - return ScopingLevel.PARTITION; - } - if (fieldName.equals(detector.overFieldName)) { - return ScopingLevel.OVER; - } - if (fieldName.equals(detector.byFieldName)) { - return ScopingLevel.BY; + private static boolean ruleHasConditionOnResultValue(DetectionRule rule) { + for (RuleCondition condition : rule.getConditions()) { + switch (condition.getAppliesTo()) { + case ACTUAL: + case TYPICAL: + case DIFF_FROM_TYPICAL: + return true; + case TIME: + return false; + default: + throw new IllegalStateException("Unknown applies_to value [" + condition.getAppliesTo() + "]"); } - throw ExceptionsHelper.badRequestException( - "fieldName '" + fieldName + "' does not match an analysis field"); } + return false; } + private void checkScoping(DetectionRule rule) { + Set analysisFields = new TreeSet<>(extractAnalysisFields()); + rule.getScope().validate(analysisFields); + } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/FilterRef.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/FilterRef.java new file mode 100644 index 0000000000000..7f3fb56287965 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/FilterRef.java @@ -0,0 +1,123 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.ml.job.config; + +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ObjectParser; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.xpack.core.ml.MlParserType; + +import java.io.IOException; +import java.util.EnumMap; +import java.util.Locale; +import java.util.Map; +import java.util.Objects; + +public class FilterRef implements ToXContentObject, Writeable { + + public static final ParseField FILTER_REF_FIELD = new ParseField("filter_ref"); + public static final ParseField FILTER_ID = new ParseField("filter_id"); + public static final ParseField FILTER_TYPE = new ParseField("filter_type"); + + public enum FilterType { + INCLUDE, EXCLUDE; + + public static FilterType fromString(String value) { + return valueOf(value.toUpperCase(Locale.ROOT)); + } + + @Override + public String toString() { + return name().toLowerCase(Locale.ROOT); + } + } + + // These parsers follow the pattern that metadata is parsed leniently (to allow for enhancements), whilst config is parsed strictly + public static final ConstructingObjectParser METADATA_PARSER = + new ConstructingObjectParser<>(FILTER_REF_FIELD.getPreferredName(), true, + a -> new FilterRef((String) a[0], (FilterType) a[1])); + public static final ConstructingObjectParser CONFIG_PARSER = + new ConstructingObjectParser<>(FILTER_REF_FIELD.getPreferredName(), false, + a -> new FilterRef((String) a[0], (FilterType) a[1])); + public static final Map> PARSERS = new EnumMap<>(MlParserType.class); + + static { + PARSERS.put(MlParserType.METADATA, METADATA_PARSER); + PARSERS.put(MlParserType.CONFIG, CONFIG_PARSER); + for (MlParserType parserType : MlParserType.values()) { + ConstructingObjectParser parser = PARSERS.get(parserType); + assert parser != null; + parser.declareString(ConstructingObjectParser.constructorArg(), FILTER_ID); + parser.declareField(ConstructingObjectParser.optionalConstructorArg(), p -> { + if (p.currentToken() == XContentParser.Token.VALUE_STRING) { + return FilterType.fromString(p.text()); + } + throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]"); + }, FILTER_TYPE, ObjectParser.ValueType.STRING); + } + } + + private final String filterId; + private final FilterType filterType; + + public FilterRef(String filterId, FilterType filterType) { + this.filterId = Objects.requireNonNull(filterId); + this.filterType = filterType == null ? FilterType.INCLUDE : filterType; + } + + public FilterRef(StreamInput in) throws IOException { + filterId = in.readString(); + filterType = in.readEnum(FilterType.class); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(filterId); + out.writeEnum(filterType); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.field(FILTER_ID.getPreferredName(), filterId); + builder.field(FILTER_TYPE.getPreferredName(), filterType); + builder.endObject(); + return builder; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + + if (obj instanceof FilterRef == false) { + return false; + } + + FilterRef other = (FilterRef) obj; + return Objects.equals(filterId, other.filterId) && Objects.equals(filterType, other.filterType); + } + + @Override + public int hashCode() { + return Objects.hash(filterId, filterType); + } + + public String getFilterId() { + return filterId; + } + + public FilterType getFilterType() { + return filterType; + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdate.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdate.java index 2c7ee538485b9..16243ed16edd4 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdate.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdate.java @@ -496,7 +496,7 @@ public static class DetectorUpdate implements Writeable, ToXContentObject { PARSER.declareInt(ConstructingObjectParser.optionalConstructorArg(), Detector.DETECTOR_INDEX); PARSER.declareStringOrNull(ConstructingObjectParser.optionalConstructorArg(), Job.DESCRIPTION); PARSER.declareObjectArray(ConstructingObjectParser.optionalConstructorArg(), (parser, parseFieldMatcher) -> - DetectionRule.CONFIG_PARSER.apply(parser, parseFieldMatcher).build(), Detector.RULES_FIELD); + DetectionRule.CONFIG_PARSER.apply(parser, parseFieldMatcher).build(), Detector.CUSTOM_RULES_FIELD); } private int detectorIndex; @@ -550,7 +550,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field(Job.DESCRIPTION.getPreferredName(), description); } if (rules != null) { - builder.field(Detector.RULES_FIELD.getPreferredName(), rules); + builder.field(Detector.CUSTOM_RULES_FIELD.getPreferredName(), rules); } builder.endObject(); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Operator.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Operator.java index 5813a10c93bb3..bfe9b0e3589ba 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Operator.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Operator.java @@ -19,12 +19,6 @@ * Enum representing logical comparisons on doubles */ public enum Operator implements Writeable { - EQ { - @Override - public boolean test(double lhs, double rhs) { - return Double.compare(lhs, rhs) == 0; - } - }, GT { @Override public boolean test(double lhs, double rhs) { @@ -48,19 +42,10 @@ public boolean test(double lhs, double rhs) { public boolean test(double lhs, double rhs) { return Double.compare(lhs, rhs) <= 0; } - }, - MATCH { - @Override - public boolean match(Pattern pattern, String field) { - Matcher match = pattern.matcher(field); - return match.matches(); - } - - @Override - public boolean expectsANumericArgument() { - return false; - } }; + // EQ was considered but given the oddity of such a + // condition and the fact that it would be a numerically + // unstable condition, it was rejected. public static final ParseField OPERATOR_FIELD = new ParseField("operator"); @@ -68,14 +53,6 @@ public boolean test(double lhs, double rhs) { return false; } - public boolean match(Pattern pattern, String field) { - return false; - } - - public boolean expectsANumericArgument() { - return true; - } - public static Operator fromString(String name) { return valueOf(name.trim().toUpperCase(Locale.ROOT)); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleAction.java index 607961140be4e..499f3a47f0e06 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleAction.java @@ -13,8 +13,8 @@ import java.util.Locale; public enum RuleAction implements Writeable { - FILTER_RESULTS, - SKIP_SAMPLING; + SKIP_RESULT, + SKIP_MODEL_UPDATE; /** * Case-insensitive from string method. diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleCondition.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleCondition.java index 6ca24c518d8fa..378ceaca6c476 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleCondition.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleCondition.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.xpack.core.ml.job.config; -import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -16,29 +15,27 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.xpack.core.ml.MlParserType; -import org.elasticsearch.xpack.core.ml.job.messages.Messages; -import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; import java.io.IOException; import java.util.EnumMap; -import java.util.EnumSet; +import java.util.Locale; import java.util.Map; import java.util.Objects; public class RuleCondition implements ToXContentObject, Writeable { - public static final ParseField TYPE_FIELD = new ParseField("type", "condition_type"); + public static final ParseField RULE_CONDITION_FIELD = new ParseField("rule_condition"); - public static final ParseField FIELD_NAME_FIELD = new ParseField("field_name"); - public static final ParseField FIELD_VALUE_FIELD = new ParseField("field_value"); - public static final ParseField FILTER_ID_FIELD = new ParseField(MlFilter.ID.getPreferredName(), "value_filter"); + + public static final ParseField APPLIES_TO_FIELD = new ParseField("applies_to"); + public static final ParseField VALUE_FIELD = new ParseField("value"); // These parsers follow the pattern that metadata is parsed leniently (to allow for enhancements), whilst config is parsed strictly public static final ConstructingObjectParser METADATA_PARSER = new ConstructingObjectParser<>(RULE_CONDITION_FIELD.getPreferredName(), true, - a -> new RuleCondition((RuleConditionType) a[0], (String) a[1], (String) a[2], (Condition) a[3], (String) a[4])); + a -> new RuleCondition((AppliesTo) a[0], (Operator) a[1], (double) a[2])); public static final ConstructingObjectParser CONFIG_PARSER = new ConstructingObjectParser<>(RULE_CONDITION_FIELD.getPreferredName(), false, - a -> new RuleCondition((RuleConditionType) a[0], (String) a[1], (String) a[2], (Condition) a[3], (String) a[4])); + a -> new RuleCondition((AppliesTo) a[0], (Operator) a[1], (double) a[2])); public static final Map> PARSERS = new EnumMap<>(MlParserType.class); @@ -50,111 +47,63 @@ public class RuleCondition implements ToXContentObject, Writeable { assert parser != null; parser.declareField(ConstructingObjectParser.constructorArg(), p -> { if (p.currentToken() == XContentParser.Token.VALUE_STRING) { - return RuleConditionType.fromString(p.text()); + return AppliesTo.fromString(p.text()); + } + throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]"); + }, APPLIES_TO_FIELD, ValueType.STRING); + parser.declareField(ConstructingObjectParser.constructorArg(), p -> { + if (p.currentToken() == XContentParser.Token.VALUE_STRING) { + return Operator.fromString(p.text()); } throw new IllegalArgumentException("Unsupported token [" + p.currentToken() + "]"); - }, TYPE_FIELD, ValueType.STRING); - parser.declareStringOrNull(ConstructingObjectParser.optionalConstructorArg(), FIELD_NAME_FIELD); - parser.declareStringOrNull(ConstructingObjectParser.optionalConstructorArg(), FIELD_VALUE_FIELD); - parser.declareObject(ConstructingObjectParser.optionalConstructorArg(), Condition.PARSER, Condition.CONDITION_FIELD); - parser.declareStringOrNull(ConstructingObjectParser.optionalConstructorArg(), FILTER_ID_FIELD); + }, Operator.OPERATOR_FIELD, ValueType.STRING); + parser.declareDouble(ConstructingObjectParser.constructorArg(), VALUE_FIELD); } } - private final RuleConditionType type; - private final String fieldName; - private final String fieldValue; - private final Condition condition; - private final String filterId; + private final AppliesTo appliesTo; + private final Operator operator; + private final double value; public RuleCondition(StreamInput in) throws IOException { - type = RuleConditionType.readFromStream(in); - condition = in.readOptionalWriteable(Condition::new); - fieldName = in.readOptionalString(); - fieldValue = in.readOptionalString(); - filterId = in.readOptionalString(); + appliesTo = AppliesTo.readFromStream(in); + operator = Operator.readFromStream(in); + value = in.readDouble(); } @Override public void writeTo(StreamOutput out) throws IOException { - type.writeTo(out); - out.writeOptionalWriteable(condition); - out.writeOptionalString(fieldName); - out.writeOptionalString(fieldValue); - out.writeOptionalString(filterId); + appliesTo.writeTo(out); + operator.writeTo(out); + out.writeDouble(value); } - RuleCondition(RuleConditionType type, String fieldName, String fieldValue, Condition condition, String filterId) { - this.type = type; - this.fieldName = fieldName; - this.fieldValue = fieldValue; - this.condition = condition; - this.filterId = filterId; - - verifyFieldsBoundToType(this); - verifyFieldValueRequiresFieldName(this); - } - - public RuleCondition(RuleCondition ruleCondition) { - this.type = ruleCondition.type; - this.fieldName = ruleCondition.fieldName; - this.fieldValue = ruleCondition.fieldValue; - this.condition = ruleCondition.condition; - this.filterId = ruleCondition.filterId; + public RuleCondition(AppliesTo appliesTo, Operator operator, double value) { + this.appliesTo = appliesTo; + this.operator = operator; + this.value = value; } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); - builder.field(TYPE_FIELD.getPreferredName(), type); - if (condition != null) { - builder.field(Condition.CONDITION_FIELD.getPreferredName(), condition); - } - if (fieldName != null) { - builder.field(FIELD_NAME_FIELD.getPreferredName(), fieldName); - } - if (fieldValue != null) { - builder.field(FIELD_VALUE_FIELD.getPreferredName(), fieldValue); - } - if (filterId != null) { - builder.field(FILTER_ID_FIELD.getPreferredName(), filterId); - } + builder.field(APPLIES_TO_FIELD.getPreferredName(), appliesTo); + builder.field(Operator.OPERATOR_FIELD.getPreferredName(), operator); + builder.field(VALUE_FIELD.getPreferredName(), value); builder.endObject(); return builder; } - public RuleConditionType getType() { - return type; - } - - /** - * The field name for which the rule applies. Can be null, meaning rule - * applies to all results. - */ - public String getFieldName() { - return fieldName; - } - - /** - * The value of the field name for which the rule applies. When set, the - * rule applies only to the results that have the fieldName/fieldValue pair. - * When null, the rule applies to all values for of the specified field - * name. Only applicable when fieldName is not null. - */ - public String getFieldValue() { - return fieldValue; + public AppliesTo getAppliesTo() { + return appliesTo; } - public Condition getCondition() { - return condition; + public Operator getOperator() { + return operator; } - /** - * The unique identifier of a filter. Required when the rule type is - * categorical. Should be null for all other types. - */ - public String getFilterId() { - return filterId; + public double getValue() { + return value; } @Override @@ -168,114 +117,40 @@ public boolean equals(Object obj) { } RuleCondition other = (RuleCondition) obj; - return Objects.equals(type, other.type) && Objects.equals(fieldName, other.fieldName) - && Objects.equals(fieldValue, other.fieldValue) && Objects.equals(condition, other.condition) - && Objects.equals(filterId, other.filterId); + return appliesTo == other.appliesTo && operator == other.operator && value == other.value; } @Override public int hashCode() { - return Objects.hash(type, fieldName, fieldValue, condition, filterId); - } - - public static RuleCondition createCategorical(String fieldName, String filterId) { - return new RuleCondition(RuleConditionType.CATEGORICAL, fieldName, null, null, filterId); - } - - public static RuleCondition createNumerical(RuleConditionType conditionType, String fieldName, String fieldValue, - Condition condition ) { - if (conditionType.isNumerical() == false) { - throw new IllegalStateException("Rule condition type [" + conditionType + "] not valid for a numerical condition"); - } - return new RuleCondition(conditionType, fieldName, fieldValue, condition, null); + return Objects.hash(appliesTo, operator, value); } public static RuleCondition createTime(Operator operator, long epochSeconds) { - return new RuleCondition(RuleConditionType.TIME, null, null, new Condition(operator, Long.toString(epochSeconds)), null); - } - - private static void verifyFieldsBoundToType(RuleCondition ruleCondition) throws ElasticsearchParseException { - switch (ruleCondition.getType()) { - case CATEGORICAL: - case CATEGORICAL_COMPLEMENT: - verifyCategorical(ruleCondition); - break; - case NUMERICAL_ACTUAL: - case NUMERICAL_TYPICAL: - case NUMERICAL_DIFF_ABS: - verifyNumerical(ruleCondition); - break; - case TIME: - verifyTimeRule(ruleCondition); - break; - default: - throw new IllegalStateException(); - } - } - - private static void verifyCategorical(RuleCondition ruleCondition) throws ElasticsearchParseException { - checkCategoricalHasNoField(Condition.CONDITION_FIELD.getPreferredName(), ruleCondition.getCondition()); - checkCategoricalHasNoField(RuleCondition.FIELD_VALUE_FIELD.getPreferredName(), ruleCondition.getFieldValue()); - checkCategoricalHasField(FILTER_ID_FIELD.getPreferredName(), ruleCondition.getFilterId()); - } - - private static void checkCategoricalHasNoField(String fieldName, Object fieldValue) throws ElasticsearchParseException { - if (fieldValue != null) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_CONDITION_CATEGORICAL_INVALID_OPTION, fieldName); - throw ExceptionsHelper.badRequestException(msg); - } - } - - private static void checkCategoricalHasField(String fieldName, Object fieldValue) throws ElasticsearchParseException { - if (fieldValue == null) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_CONDITION_CATEGORICAL_MISSING_OPTION, fieldName); - throw ExceptionsHelper.badRequestException(msg); - } + return new RuleCondition(AppliesTo.TIME, operator, epochSeconds); } - private static void verifyNumerical(RuleCondition ruleCondition) throws ElasticsearchParseException { - checkNumericalHasNoField(FILTER_ID_FIELD.getPreferredName(), ruleCondition.getFilterId()); - checkNumericalHasField(Condition.CONDITION_FIELD.getPreferredName(), ruleCondition.getCondition()); - if (ruleCondition.getFieldName() != null && ruleCondition.getFieldValue() == null) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_CONDITION_NUMERICAL_WITH_FIELD_NAME_REQUIRES_FIELD_VALUE); - throw ExceptionsHelper.badRequestException(msg); - } - checkNumericalConditionOparatorsAreValid(ruleCondition); - } + public enum AppliesTo implements Writeable { + ACTUAL, + TYPICAL, + DIFF_FROM_TYPICAL, + TIME; - private static void checkNumericalHasNoField(String fieldName, Object fieldValue) throws ElasticsearchParseException { - if (fieldValue != null) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_CONDITION_NUMERICAL_INVALID_OPTION, fieldName); - throw ExceptionsHelper.badRequestException(msg); + public static AppliesTo fromString(String value) { + return valueOf(value.toUpperCase(Locale.ROOT)); } - } - private static void checkNumericalHasField(String fieldName, Object fieldValue) throws ElasticsearchParseException { - if (fieldValue == null) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_CONDITION_NUMERICAL_MISSING_OPTION, fieldName); - throw ExceptionsHelper.badRequestException(msg); + public static AppliesTo readFromStream(StreamInput in) throws IOException { + return in.readEnum(AppliesTo.class); } - } - private static void verifyFieldValueRequiresFieldName(RuleCondition ruleCondition) throws ElasticsearchParseException { - if (ruleCondition.getFieldValue() != null && ruleCondition.getFieldName() == null) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_CONDITION_MISSING_FIELD_NAME, - ruleCondition.getFieldValue()); - throw ExceptionsHelper.badRequestException(msg); + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeEnum(this); } - } - - static EnumSet VALID_CONDITION_OPERATORS = EnumSet.of(Operator.LT, Operator.LTE, Operator.GT, Operator.GTE); - private static void checkNumericalConditionOparatorsAreValid(RuleCondition ruleCondition) throws ElasticsearchParseException { - Operator operator = ruleCondition.getCondition().getOperator(); - if (!VALID_CONDITION_OPERATORS.contains(operator)) { - String msg = Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_CONDITION_NUMERICAL_INVALID_OPERATOR, operator); - throw ExceptionsHelper.badRequestException(msg); + @Override + public String toString() { + return name().toLowerCase(Locale.ROOT); } } - - private static void verifyTimeRule(RuleCondition ruleCondition) { - checkNumericalConditionOparatorsAreValid(ruleCondition); - } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleConditionType.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleConditionType.java deleted file mode 100644 index aa563d001b5ca..0000000000000 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleConditionType.java +++ /dev/null @@ -1,69 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.core.ml.job.config; - - -import org.elasticsearch.Version; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.io.stream.Writeable; - -import java.io.IOException; -import java.util.Locale; - -public enum RuleConditionType implements Writeable { - CATEGORICAL(false, true), - NUMERICAL_ACTUAL(true, false), - NUMERICAL_TYPICAL(true, false), - NUMERICAL_DIFF_ABS(true, false), - TIME(false, false), - CATEGORICAL_COMPLEMENT(false, true); - - private final boolean isNumerical; - private final boolean isCategorical; - - RuleConditionType(boolean isNumerical, boolean isCategorical) { - this.isNumerical = isNumerical; - this.isCategorical = isCategorical; - } - - public boolean isNumerical() { - return isNumerical; - } - - public boolean isCategorical() { - return isCategorical; - } - - /** - * Case-insensitive from string method. - * - * @param value - * String representation - * @return The condition type - */ - public static RuleConditionType fromString(String value) { - return RuleConditionType.valueOf(value.toUpperCase(Locale.ROOT)); - } - - public static RuleConditionType readFromStream(StreamInput in) throws IOException { - return in.readEnum(RuleConditionType.class); - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - if (this == CATEGORICAL_COMPLEMENT && out.getVersion().before(Version.V_6_3_0)) { - out.writeEnum(CATEGORICAL); - } else { - out.writeEnum(this); - } - } - - @Override - public String toString() { - return name().toLowerCase(Locale.ROOT); - } -} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleScope.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleScope.java new file mode 100644 index 0000000000000..b6b3b4e061bdd --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleScope.java @@ -0,0 +1,143 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.ml.job.config; + +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.ConstructingObjectParser; +import org.elasticsearch.common.xcontent.ContextParser; +import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; +import org.elasticsearch.common.xcontent.NamedXContentRegistry; +import org.elasticsearch.common.xcontent.ToXContentObject; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.xpack.core.ml.MlParserType; +import org.elasticsearch.xpack.core.ml.job.messages.Messages; +import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper; + +import java.io.IOException; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; + +public class RuleScope implements ToXContentObject, Writeable { + + public static ContextParser parser(MlParserType parserType) { + return (p, c) -> { + Map unparsedScope = p.map(); + if (unparsedScope.isEmpty()) { + return new RuleScope(); + } + ConstructingObjectParser filterRefParser = FilterRef.PARSERS.get(parserType); + Map scope = new HashMap<>(); + for (Map.Entry entry : unparsedScope.entrySet()) { + try (XContentBuilder builder = XContentFactory.jsonBuilder()) { + builder.map((Map) entry.getValue()); + try (XContentParser scopeParser = XContentFactory.xContent(builder.contentType()).createParser( + NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, Strings.toString(builder))) { + scope.put(entry.getKey(), filterRefParser.parse(scopeParser, null)); + } + } + } + return new RuleScope(scope); + }; + } + + private final Map scope; + + public RuleScope() { + scope = Collections.emptyMap(); + } + + public RuleScope(Map scope) { + this.scope = Objects.requireNonNull(scope); + } + + public RuleScope(StreamInput in) throws IOException { + scope = in.readMap(StreamInput::readString, FilterRef::new); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeMap(scope, StreamOutput::writeString, (out1, value) -> value.writeTo(out1)); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + return builder.map(scope); + } + + public boolean isEmpty() { + return scope.isEmpty(); + } + + public void validate(Set validKeys) { + Optional invalidKey = scope.keySet().stream().filter(k -> !validKeys.contains(k)).findFirst(); + if (invalidKey.isPresent()) { + throw ExceptionsHelper.badRequestException(Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_SCOPE_HAS_INVALID_FIELD, + invalidKey.get(), validKeys)); + } + } + + public Set getReferencedFilters() { + return scope.values().stream().map(FilterRef::getFilterId).collect(Collectors.toSet()); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + + if (obj instanceof RuleScope == false) { + return false; + } + + RuleScope other = (RuleScope) obj; + return Objects.equals(scope, other.scope); + } + + @Override + public int hashCode() { + return Objects.hash(scope); + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + + private Map scope = new HashMap<>(); + + public Builder() {} + + public Builder(RuleScope otherScope) { + scope = new HashMap<>(otherScope.scope); + } + + public Builder exclude(String field, String filterId) { + scope.put(field, new FilterRef(filterId, FilterRef.FilterType.EXCLUDE)); + return this; + } + + public Builder include(String field, String filterId) { + scope.put(field, new FilterRef(filterId, FilterRef.FilterType.INCLUDE)); + return this; + } + + public RuleScope build() { + return new RuleScope(scope); + } + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/messages/Messages.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/messages/Messages.java index 7e5dc231e057a..79d8f068d91f8 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/messages/Messages.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/messages/Messages.java @@ -88,35 +88,12 @@ public final class Messages { "categorization_filters require setting categorization_field_name"; public static final String JOB_CONFIG_CATEGORIZATION_ANALYZER_REQUIRES_CATEGORIZATION_FIELD_NAME = "categorization_analyzer requires setting categorization_field_name"; - public static final String JOB_CONFIG_CONDITION_INVALID_VALUE_NULL = "Invalid condition: the value field cannot be null"; - public static final String JOB_CONFIG_CONDITION_INVALID_VALUE_NUMBER = - "Invalid condition value: cannot parse a double from string ''{0}''"; - public static final String JOB_CONFIG_CONDITION_INVALID_VALUE_REGEX = - "Invalid condition value: ''{0}'' is not a valid regular expression"; - public static final String JOB_CONFIG_DETECTION_RULE_CONDITION_CATEGORICAL_INVALID_OPTION = - "Invalid detector rule: a categorical rule_condition does not support {0}"; - public static final String JOB_CONFIG_DETECTION_RULE_CONDITION_CATEGORICAL_MISSING_OPTION = - "Invalid detector rule: a categorical rule_condition requires {0} to be set"; - public static final String JOB_CONFIG_DETECTION_RULE_CONDITION_INVALID_FIELD_NAME = - "Invalid detector rule: field_name has to be one of {0}; actual was ''{1}''"; - public static final String JOB_CONFIG_DETECTION_RULE_CONDITION_MISSING_FIELD_NAME = - "Invalid detector rule: missing field_name in rule_condition where field_value ''{0}'' is set"; - public static final String JOB_CONFIG_DETECTION_RULE_CONDITION_NUMERICAL_INVALID_OPERATOR = - "Invalid detector rule: operator ''{0}'' is not allowed"; - public static final String JOB_CONFIG_DETECTION_RULE_CONDITION_NUMERICAL_INVALID_OPTION = - "Invalid detector rule: a numerical rule_condition does not support {0}"; - public static final String JOB_CONFIG_DETECTION_RULE_CONDITION_NUMERICAL_MISSING_OPTION = - "Invalid detector rule: a numerical rule_condition requires {0} to be set"; - public static final String JOB_CONFIG_DETECTION_RULE_CONDITION_NUMERICAL_WITH_FIELD_NAME_REQUIRES_FIELD_VALUE = - "Invalid detector rule: a numerical rule_condition with field_name requires that field_value is set"; - public static final String JOB_CONFIG_DETECTION_RULE_INVALID_TARGET_FIELD_NAME = - "Invalid detector rule: target_field_name has to be one of {0}; actual was ''{1}''"; - public static final String JOB_CONFIG_DETECTION_RULE_MISSING_TARGET_FIELD_NAME = - "Invalid detector rule: missing target_field_name where target_field_value ''{0}'' is set"; public static final String JOB_CONFIG_DETECTION_RULE_NOT_SUPPORTED_BY_FUNCTION = - "Invalid detector rule: function {0} does not support rules"; - public static final String JOB_CONFIG_DETECTION_RULE_REQUIRES_AT_LEAST_ONE_CONDITION = - "Invalid detector rule: at least one rule_condition is required"; + "Invalid detector rule: function {0} does not support rules with conditions"; + public static final String JOB_CONFIG_DETECTION_RULE_REQUIRES_SCOPE_OR_CONDITION = + "Invalid detector rule: at least scope or a condition is required"; + public static final String JOB_CONFIG_DETECTION_RULE_SCOPE_HAS_INVALID_FIELD = + "Invalid detector rule: scope field ''{0}'' is invalid; select from {1}"; public static final String JOB_CONFIG_FIELDNAME_INCOMPATIBLE_FUNCTION = "field_name cannot be used with function ''{0}''"; public static final String JOB_CONFIG_FIELD_VALUE_TOO_LOW = "{0} cannot be less than {1,number}. Value = {2,number}"; public static final String JOB_CONFIG_MODEL_MEMORY_LIMIT_TOO_LOW = "model_memory_limit must be at least 1 MiB. Value = {0,number}"; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEventTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEventTests.java index f98eb9d5dcecc..3d6c317542279 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEventTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/calendars/ScheduledEventTests.java @@ -11,12 +11,10 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.AbstractSerializingTestCase; -import org.elasticsearch.xpack.core.ml.job.config.Connective; import org.elasticsearch.xpack.core.ml.job.config.DetectionRule; import org.elasticsearch.xpack.core.ml.job.config.Operator; import org.elasticsearch.xpack.core.ml.job.config.RuleAction; import org.elasticsearch.xpack.core.ml.job.config.RuleCondition; -import org.elasticsearch.xpack.core.ml.job.config.RuleConditionType; import org.joda.time.DateTime; import java.io.IOException; @@ -56,25 +54,22 @@ public void testToDetectionRule() { ScheduledEvent event = createTestInstance(); DetectionRule rule = event.toDetectionRule(TimeValue.timeValueSeconds(bucketSpanSecs)); - assertEquals(Connective.AND, rule.getConditionsConnective()); - assertEquals(rule.getActions(), EnumSet.of(RuleAction.FILTER_RESULTS, RuleAction.SKIP_SAMPLING)); - assertNull(rule.getTargetFieldName()); - assertNull(rule.getTargetFieldValue()); + assertEquals(rule.getActions(), EnumSet.of(RuleAction.SKIP_RESULT, RuleAction.SKIP_MODEL_UPDATE)); List conditions = rule.getConditions(); assertEquals(2, conditions.size()); - assertEquals(RuleConditionType.TIME, conditions.get(0).getType()); - assertEquals(RuleConditionType.TIME, conditions.get(1).getType()); - assertEquals(Operator.GTE, conditions.get(0).getCondition().getOperator()); - assertEquals(Operator.LT, conditions.get(1).getCondition().getOperator()); + assertEquals(RuleCondition.AppliesTo.TIME, conditions.get(0).getAppliesTo()); + assertEquals(RuleCondition.AppliesTo.TIME, conditions.get(1).getAppliesTo()); + assertEquals(Operator.GTE, conditions.get(0).getOperator()); + assertEquals(Operator.LT, conditions.get(1).getOperator()); // Check times are aligned with the bucket - long conditionStartTime = Long.parseLong(conditions.get(0).getCondition().getValue()); + long conditionStartTime = (long) conditions.get(0).getValue(); assertEquals(0, conditionStartTime % bucketSpanSecs); long bucketCount = conditionStartTime / bucketSpanSecs; assertEquals(bucketSpanSecs * bucketCount, conditionStartTime); - long conditionEndTime = Long.parseLong(conditions.get(1).getCondition().getValue()); + long conditionEndTime = (long) conditions.get(1).getValue(); assertEquals(0, conditionEndTime % bucketSpanSecs); long eventTime = event.getEndTime().toEpochSecond() - conditionStartTime; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/AnalysisConfigTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/AnalysisConfigTests.java index e64c01d7d0c3b..6c54eb78189a4 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/AnalysisConfigTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/AnalysisConfigTests.java @@ -486,11 +486,9 @@ public void testEquals_GivenDifferentCategorizationFilters() { assertFalse(config2.equals(config1)); } - public void testExtractReferencedLists() { - DetectionRule rule1 = new DetectionRule.Builder(Collections.singletonList(RuleCondition.createCategorical("foo", - "filter1"))).build(); - DetectionRule rule2 = new DetectionRule.Builder(Collections.singletonList(RuleCondition.createCategorical("foo", - "filter2"))).build(); + public void testExtractReferencedFilters() { + DetectionRule rule1 = new DetectionRule.Builder(RuleScope.builder().exclude("foo", "filter1")).build(); + DetectionRule rule2 = new DetectionRule.Builder(RuleScope.builder().exclude("foo", "filter2")).build(); Detector.Builder detector1 = new Detector.Builder("count", null); detector1.setByFieldName("foo"); detector1.setRules(Collections.singletonList(rule1)); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/DetectionRuleTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/DetectionRuleTests.java index 3aaf99ab730f8..a57bd24eda0e1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/DetectionRuleTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/DetectionRuleTests.java @@ -5,100 +5,40 @@ */ package org.elasticsearch.xpack.core.ml.job.config; +import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.common.io.stream.Writeable.Reader; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.AbstractSerializingTestCase; -import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; -public class DetectionRuleTests extends AbstractSerializingTestCase { - - public void testExtractReferencedLists() { - RuleCondition numericalCondition = - new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "field", "value", new Condition(Operator.GT, "5"), null); - List conditions = Arrays.asList( - numericalCondition, - RuleCondition.createCategorical("foo", "filter1"), - RuleCondition.createCategorical("bar", "filter2")); - - DetectionRule rule = new DetectionRule.Builder(conditions).build(); - - assertEquals(new HashSet<>(Arrays.asList("filter1", "filter2")), rule.extractReferencedFilters()); - } - - public void testEqualsGivenSameObject() { - DetectionRule rule = createFullyPopulated().build(); - assertTrue(rule.equals(rule)); - } - - public void testEqualsGivenString() { - assertFalse(createFullyPopulated().build().equals("a string")); - } +import static org.hamcrest.Matchers.equalTo; - public void testEqualsGivenDifferentTargetFieldName() { - DetectionRule rule1 = createFullyPopulated().build(); - DetectionRule rule2 = createFullyPopulated().setTargetFieldName("targetField2").build(); - assertFalse(rule1.equals(rule2)); - assertFalse(rule2.equals(rule1)); - } - - public void testEqualsGivenDifferentTargetFieldValue() { - DetectionRule rule1 = createFullyPopulated().build(); - DetectionRule rule2 = createFullyPopulated().setTargetFieldValue("targetValue2").build(); - assertFalse(rule1.equals(rule2)); - assertFalse(rule2.equals(rule1)); - } - - public void testEqualsGivenDifferentConnective() { - DetectionRule rule1 = createFullyPopulated().build(); - DetectionRule rule2 = createFullyPopulated().setConditionsConnective(Connective.OR).build(); - assertFalse(rule1.equals(rule2)); - assertFalse(rule2.equals(rule1)); - } - - public void testEqualsGivenRules() { - DetectionRule rule1 = createFullyPopulated().build(); - DetectionRule rule2 = createFullyPopulated().setConditions(createRule("10")).build(); - assertFalse(rule1.equals(rule2)); - assertFalse(rule2.equals(rule1)); - } +public class DetectionRuleTests extends AbstractSerializingTestCase { - public void testEqualsGivenEqual() { - DetectionRule rule1 = createFullyPopulated().build(); - DetectionRule rule2 = createFullyPopulated().build(); - assertTrue(rule1.equals(rule2)); - assertTrue(rule2.equals(rule1)); - assertEquals(rule1.hashCode(), rule2.hashCode()); + public void testBuildWithNeitherScopeNorCondition() { + ElasticsearchStatusException e = expectThrows(ElasticsearchStatusException.class, () -> new DetectionRule.Builder().build()); + assertThat(e.getMessage(), equalTo("Invalid detector rule: at least scope or a condition is required")); } - private static DetectionRule.Builder createFullyPopulated() { - return new DetectionRule.Builder(createRule("5")) - .setActions(EnumSet.of(RuleAction.FILTER_RESULTS, RuleAction.SKIP_SAMPLING)) - .setTargetFieldName("targetField") - .setTargetFieldValue("targetValue") - .setConditionsConnective(Connective.AND); - } + public void testExtractReferencedLists() { + DetectionRule rule = new DetectionRule.Builder(RuleScope.builder() + .exclude("foo", "filter1").include("bar", "filter2")) + .build(); - private static List createRule(String value) { - Condition condition = new Condition(Operator.GT, value); - return Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, condition, null)); + assertEquals(new HashSet<>(Arrays.asList("filter1", "filter2")), rule.extractReferencedFilters()); } @Override protected DetectionRule createTestInstance() { - int size = 1 + randomInt(20); - List ruleConditions = new ArrayList<>(size); - for (int i = 0; i < size; i++) { - // no need for random condition (it is already tested) - ruleConditions.addAll(createRule(Double.toString(randomDouble()))); - } - DetectionRule.Builder builder = new DetectionRule.Builder(ruleConditions); + DetectionRule.Builder builder = new DetectionRule.Builder(); if (randomBoolean()) { EnumSet actions = EnumSet.noneOf(RuleAction.class); @@ -109,13 +49,35 @@ protected DetectionRule createTestInstance() { builder.setActions(actions); } - if (randomBoolean()) { - builder.setConditionsConnective(randomFrom(Connective.values())); + boolean hasScope = randomBoolean(); + boolean hasConditions = randomBoolean(); + + if (!hasScope && !hasConditions) { + // at least one of the two should be present + if (randomBoolean()) { + hasScope = true; + } else { + hasConditions = true; + } } - if (randomBoolean()) { - builder.setTargetFieldName(randomAlphaOfLengthBetween(1, 20)); - builder.setTargetFieldValue(randomAlphaOfLengthBetween(1, 20)); + if (hasScope) { + Map scope = new HashMap<>(); + int scopeSize = randomIntBetween(1, 3); + for (int i = 0; i < scopeSize; i++) { + scope.put(randomAlphaOfLength(20), new FilterRef(randomAlphaOfLength(20), randomFrom(FilterRef.FilterType.values()))); + } + builder.setScope(new RuleScope(scope)); + } + + if (hasConditions) { + int size = randomIntBetween(1, 5); + List ruleConditions = new ArrayList<>(size); + for (int i = 0; i < size; i++) { + // no need for random condition (it is already tested) + ruleConditions.addAll(createCondition(randomDouble())); + } + builder.setConditions(ruleConditions); } return builder.build(); @@ -132,39 +94,34 @@ protected DetectionRule doParseInstance(XContentParser parser) { } @Override - protected DetectionRule mutateInstance(DetectionRule instance) throws IOException { + protected DetectionRule mutateInstance(DetectionRule instance) { List conditions = instance.getConditions(); + RuleScope scope = instance.getScope(); EnumSet actions = instance.getActions(); - String targetFieldName = instance.getTargetFieldName(); - String targetFieldValue = instance.getTargetFieldValue(); - Connective connective = instance.getConditionsConnective(); - switch (between(0, 3)) { + switch (between(0, 2)) { case 0: - conditions = new ArrayList<>(conditions); - conditions.addAll(createRule(Double.toString(randomDouble()))); + if (actions.size() == RuleAction.values().length) { + actions = EnumSet.of(randomFrom(RuleAction.values())); + } else { + actions = EnumSet.allOf(RuleAction.class); + } break; case 1: - targetFieldName = randomAlphaOfLengthBetween(5, 10); + conditions = new ArrayList<>(conditions); + conditions.addAll(createCondition(randomDouble())); break; case 2: - targetFieldValue = randomAlphaOfLengthBetween(5, 10); - if (targetFieldName == null) { - targetFieldName = randomAlphaOfLengthBetween(5, 10); - } - break; - case 3: - if (connective == Connective.AND) { - connective = Connective.OR; - } else { - connective = Connective.AND; - } + scope = new RuleScope.Builder(scope).include("another_field", "another_filter").build(); break; default: throw new AssertionError("Illegal randomisation branch"); } - return new DetectionRule.Builder(conditions).setActions(actions).setTargetFieldName(targetFieldName) - .setTargetFieldValue(targetFieldValue).setConditionsConnective(connective).build(); + return new DetectionRule.Builder(conditions).setActions(actions).setScope(scope).build(); + } + + private static List createCondition(double value) { + return Collections.singletonList(new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.GT, value)); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/DetectorTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/DetectorTests.java index 1296928d68478..c9be0d3b89248 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/DetectorTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/DetectorTests.java @@ -63,55 +63,46 @@ public void testEquals_GivenDifferentByFieldName() { Detector.Builder builder = new Detector.Builder(detector2); builder.setByFieldName("by2"); - Condition condition = new Condition(Operator.GT, "5"); - DetectionRule rule = new DetectionRule.Builder( - Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "by2", "val", condition, null))) - .setActions(RuleAction.FILTER_RESULTS).setTargetFieldName("over_field") - .setTargetFieldValue("targetValue") - .setConditionsConnective(Connective.AND) - .build(); - builder.setRules(Collections.singletonList(rule)); detector2 = builder.build(); assertFalse(detector1.equals(detector2)); } public void testExtractAnalysisFields() { - Detector detector = createDetector().build(); + DetectionRule rule = new DetectionRule.Builder( + Collections.singletonList(new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.GT, 5))) + .setActions(RuleAction.SKIP_RESULT) + .build(); + Detector.Builder builder = createDetector(); + builder.setRules(Collections.singletonList(rule)); + Detector detector = builder.build(); assertEquals(Arrays.asList("by_field", "over_field", "partition"), detector.extractAnalysisFields()); - Detector.Builder builder = new Detector.Builder(detector); + builder.setPartitionFieldName(null); + detector = builder.build(); + assertEquals(Arrays.asList("by_field", "over_field"), detector.extractAnalysisFields()); + builder = new Detector.Builder(detector); - Condition condition = new Condition(Operator.GT, "5"); - DetectionRule rule = new DetectionRule.Builder( - Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, condition, null))) - .setActions(RuleAction.FILTER_RESULTS) - .setTargetFieldName("over_field") - .setTargetFieldValue("targetValue") - .setConditionsConnective(Connective.AND) - .build(); - builder.setRules(Collections.singletonList(rule)); builder.setByFieldName(null); + detector = builder.build(); + assertEquals(Collections.singletonList("over_field"), detector.extractAnalysisFields()); + builder = new Detector.Builder(detector); - rule = new DetectionRule.Builder( - Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, condition, null))) - .setActions(RuleAction.FILTER_RESULTS) - .setConditionsConnective(Connective.AND) - .build(); - builder.setRules(Collections.singletonList(rule)); builder.setOverFieldName(null); + detector = builder.build(); + assertTrue(detector.extractAnalysisFields().isEmpty()); } public void testExtractReferencedLists() { Detector.Builder builder = createDetector(); builder.setRules(Arrays.asList( - new DetectionRule.Builder(Collections.singletonList(RuleCondition.createCategorical("by_field", "list1"))).build(), - new DetectionRule.Builder(Collections.singletonList(RuleCondition.createCategorical("by_field", "list2"))).build())); + new DetectionRule.Builder(RuleScope.builder().exclude("by_field", "list1")).build(), + new DetectionRule.Builder(RuleScope.builder().exclude("by_field", "list2")).build())); Detector detector = builder.build(); assertEquals(new HashSet<>(Arrays.asList("list1", "list2")), detector.extractReferencedFilters()); @@ -139,13 +130,8 @@ private Detector.Builder createDetector() { detector.setOverFieldName("over_field"); detector.setPartitionFieldName("partition"); detector.setUseNull(true); - Condition condition = new Condition(Operator.GT, "5"); - DetectionRule rule = new DetectionRule.Builder( - Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "by_field", "val", condition, null))) - .setActions(RuleAction.FILTER_RESULTS) - .setTargetFieldName("over_field") - .setTargetFieldValue("targetValue") - .setConditionsConnective(Connective.AND) + DetectionRule rule = new DetectionRule.Builder(RuleScope.builder().exclude("partition", "partition_filter")) + .setActions(RuleAction.SKIP_RESULT) .build(); detector.setRules(Collections.singletonList(rule)); return detector; @@ -159,32 +145,27 @@ protected Detector createTestInstance() { detector = new Detector.Builder(function = randomFrom(Detector.COUNT_WITHOUT_FIELD_FUNCTIONS), null); } else { EnumSet functions = EnumSet.copyOf(Detector.FIELD_NAME_FUNCTIONS); - functions.removeAll(Detector.Builder.FUNCTIONS_WITHOUT_RULE_SUPPORT); detector = new Detector.Builder(function = randomFrom(functions), randomAlphaOfLengthBetween(1, 20)); } if (randomBoolean()) { detector.setDetectorDescription(randomAlphaOfLengthBetween(1, 20)); } - String fieldName = null; if (randomBoolean()) { - detector.setPartitionFieldName(fieldName = randomAlphaOfLengthBetween(6, 20)); + detector.setPartitionFieldName(randomAlphaOfLengthBetween(6, 20)); } else if (randomBoolean() && Detector.NO_OVER_FIELD_NAME_FUNCTIONS.contains(function) == false) { - detector.setOverFieldName(fieldName = randomAlphaOfLengthBetween(6, 20)); + detector.setOverFieldName(randomAlphaOfLengthBetween(6, 20)); } else if (randomBoolean()) { - detector.setByFieldName(fieldName = randomAlphaOfLengthBetween(6, 20)); + detector.setByFieldName(randomAlphaOfLengthBetween(6, 20)); } if (randomBoolean()) { detector.setExcludeFrequent(randomFrom(Detector.ExcludeFrequent.values())); } - if (randomBoolean()) { + if (Detector.FUNCTIONS_WITHOUT_RULE_CONDITION_SUPPORT.contains(function) == false && randomBoolean()) { int size = randomInt(10); List rules = new ArrayList<>(size); for (int i = 0; i < size; i++) { // no need for random DetectionRule (it is already tested) - Condition condition = new Condition(Operator.GT, "5"); - rules.add(new DetectionRule.Builder( - Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, condition, null))) - .setTargetFieldName(fieldName).build()); + rules.add(new DetectionRule.Builder(Collections.singletonList(RuleConditionTests.createRandom())).build()); } detector.setRules(rules); } @@ -462,63 +443,20 @@ public void testVerify_GivenFunctionsThatCanHaveByField() { } } - public void testVerify_GivenInvalidRuleTargetFieldName() { - Detector.Builder detector = new Detector.Builder("mean", "metricVale"); - detector.setByFieldName("metricName"); - detector.setPartitionFieldName("instance"); - RuleCondition ruleCondition = - new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "metricName", "metricVale", new Condition(Operator.LT, "5"), null); - DetectionRule rule = new DetectionRule.Builder(Collections.singletonList(ruleCondition)).setTargetFieldName("instancE").build(); - detector.setRules(Collections.singletonList(rule)); - - ElasticsearchException e = ESTestCase.expectThrows(ElasticsearchException.class, detector::build); - - assertEquals(Messages.getMessage(Messages.JOB_CONFIG_DETECTION_RULE_INVALID_TARGET_FIELD_NAME, - "[metricName, instance]", "instancE"), - e.getMessage()); - } - - public void testVerify_GivenValidRule() { - Detector.Builder detector = new Detector.Builder("mean", "metricVale"); - detector.setByFieldName("metricName"); - detector.setPartitionFieldName("instance"); - RuleCondition ruleCondition = - new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "metricName", "CPU", new Condition(Operator.LT, "5"), null); - DetectionRule rule = new DetectionRule.Builder(Collections.singletonList(ruleCondition)).setTargetFieldName("instance").build(); - detector.setRules(Collections.singletonList(rule)); - detector.build(); - } - - public void testVerify_GivenCategoricalRuleOnAllPartitioningFields() { + public void testVerify_GivenAllPartitioningFieldsAreScoped() { 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(); + DetectionRule rule = new DetectionRule.Builder(RuleScope.builder() + .exclude("my_partition", "my_filter_id") + .exclude("my_over", "my_filter_id") + .exclude("my_by", "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()); + detector.build(); } public void testVerify_GivenSameByAndPartition() { @@ -596,6 +534,85 @@ public void testVerify_GivenOverIsOver() { assertEquals("'over' is not a permitted value for over_field_name", e.getMessage()); } + public void testVerify_GivenRulesAndFunctionIsLatLong() { + Detector.Builder detector = new Detector.Builder("lat_long", "geo"); + detector.setRules(Collections.singletonList(new DetectionRule.Builder(Collections.singletonList( + new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.GT, 42.0))).build())); + + ElasticsearchException e = ESTestCase.expectThrows(ElasticsearchException.class, detector::build); + + assertThat(e.getMessage(), equalTo("Invalid detector rule: function lat_long does not support rules with conditions")); + } + + public void testVerify_GivenRulesAndFunctionIsMetric() { + Detector.Builder detector = new Detector.Builder("metric", "some_metric"); + detector.setRules(Collections.singletonList(new DetectionRule.Builder(Collections.singletonList( + new RuleCondition(RuleCondition.AppliesTo.TYPICAL, Operator.GT, 42.0))).build())); + + ElasticsearchException e = ESTestCase.expectThrows(ElasticsearchException.class, detector::build); + + assertThat(e.getMessage(), equalTo("Invalid detector rule: function metric does not support rules with conditions")); + } + + public void testVerify_GivenRulesAndFunctionIsRare() { + Detector.Builder detector = new Detector.Builder("rare", null); + detector.setByFieldName("some_field"); + detector.setRules(Collections.singletonList(new DetectionRule.Builder(Collections.singletonList( + new RuleCondition(RuleCondition.AppliesTo.DIFF_FROM_TYPICAL, Operator.GT, 42.0))).build())); + + ElasticsearchException e = ESTestCase.expectThrows(ElasticsearchException.class, detector::build); + + assertThat(e.getMessage(), equalTo("Invalid detector rule: function rare does not support rules with conditions")); + } + + public void testVerify_GivenRulesAndFunctionIsFreqRare() { + Detector.Builder detector = new Detector.Builder("freq_rare", null); + detector.setByFieldName("some_field"); + detector.setOverFieldName("some_field2"); + detector.setRules(Collections.singletonList(new DetectionRule.Builder(Collections.singletonList( + new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.GT, 42.0))).build())); + + ElasticsearchException e = ESTestCase.expectThrows(ElasticsearchException.class, detector::build); + + assertThat(e.getMessage(), equalTo("Invalid detector rule: function freq_rare does not support rules with conditions")); + } + + public void testVerify_GivenTimeConditionRuleAndFunctionIsLatLong() { + Detector.Builder detector = new Detector.Builder("lat_long", "geo"); + detector.setRules(Collections.singletonList(new DetectionRule.Builder(Collections.singletonList( + new RuleCondition(RuleCondition.AppliesTo.TIME, Operator.GT, 42.0))).build())); + detector.build(); + } + + public void testVerify_GivenScopeRuleOnInvalidField() { + 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(RuleScope.builder().exclude("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_SCOPE_HAS_INVALID_FIELD, + "my_metric", "[my_by, my_over, my_partition]"), e.getMessage()); + } + + public void testVerify_GivenValidRule() { + Detector.Builder detector = new Detector.Builder("mean", "metricVale"); + detector.setByFieldName("metricName"); + detector.setPartitionFieldName("instance"); + DetectionRule rule = new DetectionRule.Builder(Collections.singletonList(RuleConditionTests.createRandom())) + .setScope(RuleScope.builder() + .include("metricName", "f1") + .exclude("instance", "f2") + .build()) + .build(); + detector.setRules(Collections.singletonList(rule)); + detector.build(); + } + public void testExcludeFrequentForString() { assertEquals(Detector.ExcludeFrequent.ALL, Detector.ExcludeFrequent.forString("all")); assertEquals(Detector.ExcludeFrequent.ALL, Detector.ExcludeFrequent.forString("ALL")); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/FilterRefTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/FilterRefTests.java new file mode 100644 index 0000000000000..241bf6593320c --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/FilterRefTests.java @@ -0,0 +1,30 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.ml.job.config; + +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.test.AbstractSerializingTestCase; + +import java.io.IOException; + +public class FilterRefTests extends AbstractSerializingTestCase { + + @Override + protected FilterRef createTestInstance() { + return new FilterRef(randomAlphaOfLength(20), randomFrom(FilterRef.FilterType.values())); + } + + @Override + protected FilterRef doParseInstance(XContentParser parser) throws IOException { + return FilterRef.CONFIG_PARSER.parse(parser, null); + } + + @Override + protected Writeable.Reader instanceReader() { + return FilterRef::new; + } +} \ No newline at end of file diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdateTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdateTests.java index 3663ff14e6302..c529d6ebfb368 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdateTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/JobUpdateTests.java @@ -53,10 +53,8 @@ protected JobUpdate createTestInstance() { List detectionRules = null; if (randomBoolean()) { detectionRules = new ArrayList<>(); - Condition condition = new Condition(Operator.GT, "5"); detectionRules.add(new DetectionRule.Builder( - Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, condition, null))) - .setTargetFieldName("foo").build()); + Collections.singletonList(new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.GT, 5))).build()); } detectorUpdates.add(new JobUpdate.DetectorUpdate(i, detectorDescription, detectionRules)); } @@ -119,13 +117,11 @@ protected JobUpdate doParseInstance(XContentParser parser) { public void testMergeWithJob() { List detectorUpdates = new ArrayList<>(); List detectionRules1 = Collections.singletonList(new DetectionRule.Builder( - Collections.singletonList(new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, new Condition(Operator.GT, "5") - , null))) - .setTargetFieldName("mlcategory").build()); + Collections.singletonList(new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.GT, 5))) + .build()); detectorUpdates.add(new JobUpdate.DetectorUpdate(0, "description-1", detectionRules1)); List detectionRules2 = Collections.singletonList(new DetectionRule.Builder(Collections.singletonList( - new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, new Condition(Operator.GT, "5"), null))) - .setTargetFieldName("host").build()); + new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.GT, 5))).build()); detectorUpdates.add(new JobUpdate.DetectorUpdate(1, "description-2", detectionRules2)); ModelPlotConfig modelPlotConfig = new ModelPlotConfig(randomBoolean(), randomAlphaOfLength(10)); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/RuleConditionTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/RuleConditionTests.java index 882c590983aae..07122818d5506 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/RuleConditionTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/RuleConditionTests.java @@ -5,36 +5,21 @@ */ package org.elasticsearch.xpack.core.ml.job.config; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.io.stream.Writeable.Reader; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.AbstractSerializingTestCase; -import org.elasticsearch.xpack.core.ml.job.messages.Messages; public class RuleConditionTests extends AbstractSerializingTestCase { @Override protected RuleCondition createTestInstance() { - Condition condition = null; - String fieldName = null; - String valueFilter = null; - String fieldValue = null; - RuleConditionType type = randomFrom(RuleConditionType.values()); - if (type.isCategorical()) { - valueFilter = randomAlphaOfLengthBetween(1, 20); - if (randomBoolean()) { - fieldName = randomAlphaOfLengthBetween(1, 20); - } - } else { - // no need to randomize, it is properly randomly tested in - // ConditionTest - condition = new Condition(Operator.LT, Long.toString(randomLong())); - if (randomBoolean()) { - fieldName = randomAlphaOfLengthBetween(1, 20); - fieldValue = randomAlphaOfLengthBetween(1, 20); - } - } - return new RuleCondition(type, fieldName, fieldValue, condition, valueFilter); + return createRandom(); + } + + public static RuleCondition createRandom() { + RuleCondition.AppliesTo appliesTo = randomFrom(RuleCondition.AppliesTo.values()); + Operator operator = randomFrom(Operator.LT, Operator.LTE, Operator.GT, Operator.GTE); + return new RuleCondition(appliesTo, operator, randomDouble()); } @Override @@ -47,199 +32,52 @@ protected RuleCondition doParseInstance(XContentParser parser) { return RuleCondition.CONFIG_PARSER.apply(parser, null); } - public void testConstructor() { - RuleCondition condition = new RuleCondition(RuleConditionType.CATEGORICAL, null, null, null, "valueFilter"); - assertEquals(RuleConditionType.CATEGORICAL, condition.getType()); - assertNull(condition.getFieldName()); - assertNull(condition.getFieldValue()); - assertNull(condition.getCondition()); - } - public void testEqualsGivenSameObject() { - RuleCondition condition = new RuleCondition(RuleConditionType.CATEGORICAL, null, null, null, "valueFilter"); + RuleCondition condition = createRandom(); assertTrue(condition.equals(condition)); } public void testEqualsGivenString() { - assertFalse(new RuleCondition(RuleConditionType.CATEGORICAL, null, null, null, "filter").equals("a string")); - } - - public void testEqualsGivenDifferentType() { - RuleCondition condition1 = createFullyPopulated(); - RuleCondition condition2 = new RuleCondition(RuleConditionType.CATEGORICAL, null, null, null, "valueFilter"); - assertFalse(condition1.equals(condition2)); - assertFalse(condition2.equals(condition1)); - } - - public void testEqualsGivenDifferentFieldName() { - RuleCondition condition1 = createFullyPopulated(); - RuleCondition condition2 = new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "metricNameaaa", "cpu", - new Condition(Operator.LT, "5"), null); - assertFalse(condition1.equals(condition2)); - assertFalse(condition2.equals(condition1)); - } - - public void testEqualsGivenDifferentFieldValue() { - RuleCondition condition1 = createFullyPopulated(); - RuleCondition condition2 = new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "metricName", "cpuaaa", - new Condition(Operator.LT, "5"), null); - assertFalse(condition1.equals(condition2)); - assertFalse(condition2.equals(condition1)); - } - - public void testEqualsGivenDifferentCondition() { - RuleCondition condition1 = createFullyPopulated(); - RuleCondition condition2 = new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "metricName", "cpu", - new Condition(Operator.GT, "5"), null); - assertFalse(condition1.equals(condition2)); - assertFalse(condition2.equals(condition1)); - } - - public void testEqualsGivenDifferentValueFilter() { - RuleCondition condition1 = new RuleCondition(RuleConditionType.CATEGORICAL, null, null, null, "myFilter"); - RuleCondition condition2 = new RuleCondition(RuleConditionType.CATEGORICAL, null, null, null, "myFilteraaa"); - assertFalse(condition1.equals(condition2)); - assertFalse(condition2.equals(condition1)); - } - - private static RuleCondition createFullyPopulated() { - return new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "metricName", "cpu", new Condition(Operator.LT, "5"), null); - } - - public void testVerify_GivenCategoricalWithCondition() { - Condition condition = new Condition(Operator.MATCH, "text"); - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.CATEGORICAL, null, null, condition, null)); - assertEquals("Invalid detector rule: a categorical rule_condition does not support condition", e.getMessage()); - } - - public void testVerify_GivenCategoricalWithFieldValue() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.CATEGORICAL, "metric", "CPU", null, null)); - assertEquals("Invalid detector rule: a categorical rule_condition does not support field_value", e.getMessage()); - } - - public void testVerify_GivenCategoricalWithoutFilterId() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.CATEGORICAL, null, null, null, null)); - assertEquals("Invalid detector rule: a categorical rule_condition requires filter_id to be set", e.getMessage()); - } - - public void testVerify_GivenNumericalActualWithFilterId() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, null, "myFilter")); - assertEquals("Invalid detector rule: a numerical rule_condition does not support filter_id", e.getMessage()); - } - - public void testVerify_GivenNumericalActualWithoutCondition() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, null, null)); - assertEquals("Invalid detector rule: a numerical rule_condition requires condition to be set", e.getMessage()); + assertFalse(createRandom().equals("a string")); } - public void testVerify_GivenNumericalActualWithFieldNameButNoFieldValue() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "metric", null, new Condition(Operator.LT, "5"), null)); - assertEquals("Invalid detector rule: a numerical rule_condition with field_name requires that field_value is set", e.getMessage()); - } - - public void testVerify_GivenNumericalTypicalWithFilterId() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, null, "myFilter")); - assertEquals("Invalid detector rule: a numerical rule_condition does not support filter_id", e.getMessage()); - } - - public void testVerify_GivenNumericalTypicalWithoutCondition() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, null, null)); - assertEquals("Invalid detector rule: a numerical rule_condition requires condition to be set", e.getMessage()); - } - - public void testVerify_GivenNumericalDiffAbsWithFilterId() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.NUMERICAL_DIFF_ABS, null, null, null, "myFilter")); - assertEquals("Invalid detector rule: a numerical rule_condition does not support filter_id", e.getMessage()); - } - - public void testVerify_GivenNumericalDiffAbsWithoutCondition() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.NUMERICAL_DIFF_ABS, null, null, null, null)); - assertEquals("Invalid detector rule: a numerical rule_condition requires condition to be set", e.getMessage()); - } - - public void testVerify_GivenFieldValueWithoutFieldName() { - Condition condition = new Condition(Operator.LTE, "5"); - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.NUMERICAL_DIFF_ABS, null, "foo", condition, null)); - assertEquals("Invalid detector rule: missing field_name in rule_condition where field_value 'foo' is set", e.getMessage()); - } - - public void testVerify_GivenNumericalAndOperatorEquals() { - Condition condition = new Condition(Operator.EQ, "5"); - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, condition, null)); - assertEquals("Invalid detector rule: operator 'eq' is not allowed", e.getMessage()); - } - - public void testVerify_GivenNumericalAndOperatorMatch() { - Condition condition = new Condition(Operator.MATCH, "aaa"); - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, null, null, condition, null)); - assertEquals("Invalid detector rule: operator 'match' is not allowed", e.getMessage()); - } - - public void testVerify_GivenDetectionRuleWithInvalidCondition() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "metricName", "CPU", new Condition(Operator.LT, "invalid"), - null)); - assertEquals(Messages.getMessage(Messages.JOB_CONFIG_CONDITION_INVALID_VALUE_NUMBER, "invalid"), e.getMessage()); - } - - public void testVerify_GivenValidCategorical() { - // no validation error: - new RuleCondition(RuleConditionType.CATEGORICAL, "metric", null, null, "myFilter"); - new RuleCondition(RuleConditionType.CATEGORICAL_COMPLEMENT, "metric", null, null, "myFilter"); - } - - public void testVerify_GivenValidNumericalActual() { + public void testVerify_GivenValidActual() { // no validation error: - new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "metric", "cpu", new Condition(Operator.GT, "5"), null); + new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.GT, 5.0); } - public void testVerify_GivenValidNumericalTypical() { + public void testVerify_GivenValidTypical() { // no validation error: - new RuleCondition(RuleConditionType.NUMERICAL_ACTUAL, "metric", "cpu", new Condition(Operator.GTE, "5"), null); + new RuleCondition(RuleCondition.AppliesTo.TYPICAL, Operator.GTE, 5.0); } - public void testVerify_GivenValidNumericalDiffAbs() { + public void testVerify_GivenValidDiffFromTypical() { // no validation error: - new RuleCondition(RuleConditionType.NUMERICAL_DIFF_ABS, "metric", "cpu", new Condition(Operator.LT, "5"), null); + new RuleCondition(RuleCondition.AppliesTo.DIFF_FROM_TYPICAL, Operator.LT, 5.0); } public void testCreateTimeBased() { RuleCondition timeBased = RuleCondition.createTime(Operator.GTE, 100L); - assertEquals(RuleConditionType.TIME, timeBased.getType()); - assertEquals(Operator.GTE, timeBased.getCondition().getOperator()); - assertEquals("100", timeBased.getCondition().getValue()); - assertNull(timeBased.getFieldName()); - assertNull(timeBased.getFieldValue()); - assertNull(timeBased.getFilterId()); - } - - public void testCreateTimeBased_GivenOperatorMatch() { - ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> RuleCondition.createTime(Operator.MATCH, 100L)); - assertEquals("Invalid detector rule: operator 'match' is not allowed", e.getMessage()); - } - - public void testCreateNumerical() { - RuleCondition ruleCondition = RuleCondition.createNumerical(RuleConditionType.NUMERICAL_ACTUAL, "foo", "bar", - new Condition(Operator.GTE, "100")); - assertEquals(RuleConditionType.NUMERICAL_ACTUAL, ruleCondition.getType()); - assertEquals(Operator.GTE, ruleCondition.getCondition().getOperator()); - assertEquals("100", ruleCondition.getCondition().getValue()); - assertEquals("foo", ruleCondition.getFieldName()); - assertEquals("bar", ruleCondition.getFieldValue()); - assertNull(ruleCondition.getFilterId()); + assertEquals(RuleCondition.AppliesTo.TIME, timeBased.getAppliesTo()); + assertEquals(Operator.GTE, timeBased.getOperator()); + assertEquals(100.0, timeBased.getValue(), 0.000001); + } + + public void testAppliesToFromString() { + assertEquals(RuleCondition.AppliesTo.ACTUAL, RuleCondition.AppliesTo.fromString("actual")); + assertEquals(RuleCondition.AppliesTo.ACTUAL, RuleCondition.AppliesTo.fromString("ACTUAL")); + assertEquals(RuleCondition.AppliesTo.TYPICAL, RuleCondition.AppliesTo.fromString("typical")); + assertEquals(RuleCondition.AppliesTo.TYPICAL, RuleCondition.AppliesTo.fromString("TYPICAL")); + assertEquals(RuleCondition.AppliesTo.DIFF_FROM_TYPICAL, RuleCondition.AppliesTo.fromString("diff_from_typical")); + assertEquals(RuleCondition.AppliesTo.DIFF_FROM_TYPICAL, RuleCondition.AppliesTo.fromString("DIFF_FROM_TYPICAL")); + assertEquals(RuleCondition.AppliesTo.TIME, RuleCondition.AppliesTo.fromString("time")); + assertEquals(RuleCondition.AppliesTo.TIME, RuleCondition.AppliesTo.fromString("TIME")); + } + + public void testAppliesToToString() { + assertEquals("actual", RuleCondition.AppliesTo.ACTUAL.toString()); + assertEquals("typical", RuleCondition.AppliesTo.TYPICAL.toString()); + assertEquals("diff_from_typical", RuleCondition.AppliesTo.DIFF_FROM_TYPICAL.toString()); + assertEquals("time", RuleCondition.AppliesTo.TIME.toString()); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/RuleScopeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/RuleScopeTests.java new file mode 100644 index 0000000000000..10b9c29aba7ef --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/RuleScopeTests.java @@ -0,0 +1,81 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core.ml.job.config; + +import org.elasticsearch.ElasticsearchStatusException; +import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.util.set.Sets; +import org.elasticsearch.test.AbstractWireSerializingTestCase; + +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +public class RuleScopeTests extends AbstractWireSerializingTestCase { + + @Override + protected RuleScope createTestInstance() { + RuleScope.Builder scope = RuleScope.builder(); + int count = randomIntBetween(0, 3); + for (int i = 0; i < count; ++i) { + if (randomBoolean()) { + scope.include(randomAlphaOfLength(20), randomAlphaOfLength(20)); + } else { + scope.exclude(randomAlphaOfLength(20), randomAlphaOfLength(20)); + } + } + return scope.build(); + } + + @Override + protected Writeable.Reader instanceReader() { + return RuleScope::new; + } + + public void testValidate_GivenEmpty() { + RuleScope scope = RuleScope.builder().build(); + assertThat(scope.isEmpty(), is(true)); + + scope.validate(Sets.newHashSet("a", "b")); + } + + public void testValidate_GivenMultipleValidFields() { + RuleScope scope = RuleScope.builder() + .include("foo", "filter1") + .exclude("bar", "filter2") + .include("foobar", "filter3") + .build(); + assertThat(scope.isEmpty(), is(false)); + + scope.validate(Sets.newHashSet("foo", "bar", "foobar")); + } + + public void testValidate_GivenMultipleFieldsIncludingInvalid() { + RuleScope scope = RuleScope.builder() + .include("foo", "filter1") + .exclude("bar", "filter2") + .include("foobar", "filter3") + .build(); + assertThat(scope.isEmpty(), is(false)); + + ElasticsearchStatusException e = expectThrows(ElasticsearchStatusException.class, + () -> scope.validate(Sets.newHashSet("foo", "foobar"))); + assertThat(e.getMessage(), equalTo("Invalid detector rule: scope field 'bar' is invalid; select from [foo, foobar]")); + } + + public void testGetReferencedFilters_GivenEmpty() { + assertThat(RuleScope.builder().build().getReferencedFilters().isEmpty(), is(true)); + } + + public void testGetReferencedFilters_GivenMultipleFields() { + RuleScope scope = RuleScope.builder() + .include("foo", "filter1") + .exclude("bar", "filter2") + .include("foobar", "filter3") + .build(); + assertThat(scope.getReferencedFilters(), contains("filter1", "filter2", "filter3")); + } +} \ No newline at end of file diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java index f783dfbddc35a..5de7962169279 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportOpenJobAction.java @@ -54,6 +54,7 @@ import org.elasticsearch.xpack.core.ml.action.OpenJobAction; import org.elasticsearch.xpack.core.ml.action.PutJobAction; import org.elasticsearch.xpack.core.ml.action.UpdateJobAction; +import org.elasticsearch.xpack.core.ml.job.config.DetectionRule; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.job.config.JobState; import org.elasticsearch.xpack.core.ml.job.config.JobTaskStatus; @@ -190,6 +191,14 @@ static PersistentTasksCustomMetaData.Assignment selectLeastLoadedMlNode(String j continue; } + if (jobHasRules(job) && node.getVersion().before(DetectionRule.VERSION_INTRODUCED)) { + String reason = "Not opening job [" + jobId + "] on node [" + nodeNameAndVersion(node) + "], because jobs using " + + "custom_rules require a node of version [" + DetectionRule.VERSION_INTRODUCED + "] or higher"; + logger.trace(reason); + reasons.add(reason); + continue; + } + long numberOfAssignedJobs = 0; int numberOfAllocatingJobs = 0; long assignedJobMemory = 0; @@ -373,6 +382,10 @@ private static boolean nodeSupportsModelSnapshotVersion(DiscoveryNode node, Job return node.getVersion().onOrAfter(job.getModelSnapshotMinVersion()); } + private static boolean jobHasRules(Job job) { + return job.getAnalysisConfig().getDetectors().stream().anyMatch(d -> d.getRules().isEmpty() == false); + } + static String[] mappingRequiresUpdate(ClusterState state, String[] concreteIndices, Version minVersion, Logger logger) throws IOException { List indicesToUpdate = new ArrayList<>(); @@ -646,9 +659,11 @@ public PersistentTasksCustomMetaData.Assignment getAssignment(OpenJobAction.JobP @Override public void validate(OpenJobAction.JobParams params, ClusterState clusterState) { + + TransportOpenJobAction.validate(params.getJobId(), MlMetadata.getMlMetadata(clusterState)); + // If we already know that we can't find an ml node because all ml nodes are running at capacity or // simply because there are no ml nodes in the cluster then we fail quickly here: - TransportOpenJobAction.validate(params.getJobId(), MlMetadata.getMlMetadata(clusterState)); PersistentTasksCustomMetaData.Assignment assignment = selectLeastLoadedMlNode(params.getJobId(), clusterState, maxConcurrentJobAllocations, fallbackMaxNumberOfOpenJobs, maxMachineMemoryPercent, logger); if (assignment.getExecutorNode() == null) { diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java index 2042d917f6057..1fd73b96667b2 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java @@ -181,6 +181,7 @@ public void putJob(PutJobAction.Request request, AnalysisRegistry analysisRegist request.getJobBuilder().validateCategorizationAnalyzer(analysisRegistry, environment); Job job = request.getJobBuilder().build(new Date()); + if (job.getDataDescription() != null && job.getDataDescription().getFormat() == DataDescription.DataFormat.DELIMITED) { DEPRECATION_LOGGER.deprecated("Creating jobs with delimited data format is deprecated. Please use xcontent instead."); } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportOpenJobActionTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportOpenJobActionTests.java index 5ee07d63a968b..6ef2d92d9c7c6 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportOpenJobActionTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/action/TransportOpenJobActionTests.java @@ -36,9 +36,15 @@ import org.elasticsearch.xpack.core.ml.MlMetaIndex; import org.elasticsearch.xpack.core.ml.MlMetadata; import org.elasticsearch.xpack.core.ml.action.OpenJobAction; +import org.elasticsearch.xpack.core.ml.job.config.AnalysisConfig; +import org.elasticsearch.xpack.core.ml.job.config.DataDescription; +import org.elasticsearch.xpack.core.ml.job.config.DetectionRule; +import org.elasticsearch.xpack.core.ml.job.config.Detector; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.job.config.JobState; import org.elasticsearch.xpack.core.ml.job.config.JobTaskStatus; +import org.elasticsearch.xpack.core.ml.job.config.Operator; +import org.elasticsearch.xpack.core.ml.job.config.RuleCondition; import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndex; import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndexFields; import org.elasticsearch.xpack.core.ml.job.persistence.ElasticsearchMappings; @@ -49,6 +55,7 @@ import java.io.IOException; import java.net.InetAddress; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Date; import java.util.HashMap; @@ -427,6 +434,62 @@ public void testSelectLeastLoadedMlNode_noNodesMatchingModelSnapshotMinVersion() assertNull(result.getExecutorNode()); } + public void testSelectLeastLoadedMlNode_jobWithRulesButNoNodeMeetsRequiredVersion() { + Map nodeAttr = new HashMap<>(); + nodeAttr.put(MachineLearning.ML_ENABLED_NODE_ATTR, "true"); + DiscoveryNodes nodes = DiscoveryNodes.builder() + .add(new DiscoveryNode("_node_name1", "_node_id1", new TransportAddress(InetAddress.getLoopbackAddress(), 9300), + nodeAttr, Collections.emptySet(), Version.V_6_2_0)) + .add(new DiscoveryNode("_node_name2", "_node_id2", new TransportAddress(InetAddress.getLoopbackAddress(), 9301), + nodeAttr, Collections.emptySet(), Version.V_6_3_0)) + .build(); + + PersistentTasksCustomMetaData.Builder tasksBuilder = PersistentTasksCustomMetaData.builder(); + addJobTask("job_with_rules", "_node_id1", null, tasksBuilder); + PersistentTasksCustomMetaData tasks = tasksBuilder.build(); + + ClusterState.Builder cs = ClusterState.builder(new ClusterName("_name")); + MetaData.Builder metaData = MetaData.builder(); + RoutingTable.Builder routingTable = RoutingTable.builder(); + addJobAndIndices(metaData, routingTable, jobWithRulesCreator(), "job_with_rules"); + cs.nodes(nodes); + metaData.putCustom(PersistentTasksCustomMetaData.TYPE, tasks); + cs.metaData(metaData); + cs.routingTable(routingTable.build()); + Assignment result = TransportOpenJobAction.selectLeastLoadedMlNode("job_with_rules", cs.build(), + 2, 10, 30, logger); + assertThat(result.getExplanation(), containsString( + "because jobs using custom_rules require a node of version [6.4.0] or higher")); + assertNull(result.getExecutorNode()); + } + + public void testSelectLeastLoadedMlNode_jobWithRulesAndNodeMeetsRequiredVersion() { + Map nodeAttr = new HashMap<>(); + nodeAttr.put(MachineLearning.ML_ENABLED_NODE_ATTR, "true"); + DiscoveryNodes nodes = DiscoveryNodes.builder() + .add(new DiscoveryNode("_node_name1", "_node_id1", new TransportAddress(InetAddress.getLoopbackAddress(), 9300), + nodeAttr, Collections.emptySet(), Version.V_6_2_0)) + .add(new DiscoveryNode("_node_name2", "_node_id2", new TransportAddress(InetAddress.getLoopbackAddress(), 9301), + nodeAttr, Collections.emptySet(), Version.V_6_4_0)) + .build(); + + PersistentTasksCustomMetaData.Builder tasksBuilder = PersistentTasksCustomMetaData.builder(); + addJobTask("job_with_rules", "_node_id1", null, tasksBuilder); + PersistentTasksCustomMetaData tasks = tasksBuilder.build(); + + ClusterState.Builder cs = ClusterState.builder(new ClusterName("_name")); + MetaData.Builder metaData = MetaData.builder(); + RoutingTable.Builder routingTable = RoutingTable.builder(); + addJobAndIndices(metaData, routingTable, jobWithRulesCreator(), "job_with_rules"); + cs.nodes(nodes); + metaData.putCustom(PersistentTasksCustomMetaData.TYPE, tasks); + cs.metaData(metaData); + cs.routingTable(routingTable.build()); + Assignment result = TransportOpenJobAction.selectLeastLoadedMlNode("job_with_rules", cs.build(), + 2, 10, 30, logger); + assertNotNull(result.getExecutorNode()); + } + public void testVerifyIndicesPrimaryShardsAreActive() { MetaData.Builder metaData = MetaData.builder(); RoutingTable.Builder routingTable = RoutingTable.builder(); @@ -645,4 +708,21 @@ private ClusterState getClusterStateWithMappingsWithMetaData(Map return csBuilder.build(); } + private static Function jobWithRulesCreator() { + return jobId -> { + DetectionRule rule = new DetectionRule.Builder(Arrays.asList( + new RuleCondition(RuleCondition.AppliesTo.TYPICAL, Operator.LT, 100.0) + )).build(); + + Detector.Builder detector = new Detector.Builder("count", null); + detector.setRules(Arrays.asList(rule)); + AnalysisConfig.Builder analysisConfig = new AnalysisConfig.Builder(Arrays.asList(detector.build())); + DataDescription.Builder dataDescription = new DataDescription.Builder(); + Job.Builder job = new Job.Builder(jobId); + job.setAnalysisConfig(analysisConfig); + job.setDataDescription(dataDescription); + return job.build(new Date()); + }; + } + } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobProviderIT.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobProviderIT.java index 120f04e95e70b..7e0dc453f07ee 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobProviderIT.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobProviderIT.java @@ -27,29 +27,28 @@ import org.elasticsearch.xpack.core.ml.calendars.Calendar; import org.elasticsearch.xpack.core.ml.calendars.ScheduledEvent; import org.elasticsearch.xpack.core.ml.job.config.AnalysisConfig; -import org.elasticsearch.xpack.core.ml.job.config.Connective; import org.elasticsearch.xpack.core.ml.job.config.DataDescription; import org.elasticsearch.xpack.core.ml.job.config.DetectionRule; import org.elasticsearch.xpack.core.ml.job.config.Detector; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.job.config.MlFilter; import org.elasticsearch.xpack.core.ml.job.config.RuleAction; -import org.elasticsearch.xpack.core.ml.job.config.RuleCondition; +import org.elasticsearch.xpack.core.ml.job.config.RuleScope; import org.elasticsearch.xpack.core.ml.job.persistence.AnomalyDetectorsIndex; -import org.elasticsearch.xpack.ml.job.persistence.CalendarQueryBuilder; -import org.elasticsearch.xpack.ml.job.persistence.ScheduledEventsQueryBuilder; -import org.elasticsearch.xpack.ml.job.process.autodetect.params.AutodetectParams; import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.DataCounts; +import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.DataCountsTests; import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.ModelSizeStats; import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.ModelSnapshot; import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.Quantiles; import org.elasticsearch.xpack.ml.LocalStateMachineLearning; import org.elasticsearch.xpack.ml.MachineLearning; import org.elasticsearch.xpack.ml.MlSingleNodeTestCase; +import org.elasticsearch.xpack.ml.job.persistence.CalendarQueryBuilder; import org.elasticsearch.xpack.ml.job.persistence.JobDataCountsPersister; import org.elasticsearch.xpack.ml.job.persistence.JobProvider; import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister; -import org.elasticsearch.xpack.core.ml.job.process.autodetect.state.DataCountsTests; +import org.elasticsearch.xpack.ml.job.persistence.ScheduledEventsQueryBuilder; +import org.elasticsearch.xpack.ml.job.process.autodetect.params.AutodetectParams; import org.junit.Before; import java.io.IOException; @@ -511,20 +510,15 @@ private Job.Builder createJob(String jobId, List filterIds, List private AnalysisConfig.Builder createAnalysisConfig(List filterIds) { Detector.Builder detector = new Detector.Builder("mean", "field"); detector.setByFieldName("by_field"); + List rules = new ArrayList<>(); - if (!filterIds.isEmpty()) { - List conditions = new ArrayList<>(); - - for (String filterId : filterIds) { - conditions.add(RuleCondition.createCategorical("by_field", filterId)); - } - - DetectionRule.Builder rule = new DetectionRule.Builder(conditions) - .setActions(RuleAction.FILTER_RESULTS) - .setConditionsConnective(Connective.OR); + for (String filterId : filterIds) { + RuleScope.Builder ruleScope = RuleScope.builder(); + ruleScope.include("by_field", filterId); - detector.setRules(Collections.singletonList(rule.build())); + rules.add(new DetectionRule.Builder(ruleScope).setActions(RuleAction.SKIP_RESULT).build()); } + detector.setRules(rules); return new AnalysisConfig.Builder(Collections.singletonList(detector.build())); } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java index 64b3bfbab45c8..454f941d6c8b0 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java @@ -19,6 +19,7 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.env.TestEnvironment; import org.elasticsearch.index.analysis.AnalysisRegistry; +import org.elasticsearch.persistent.PersistentTasksCustomMetaData; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.ml.MLMetadataField; import org.elasticsearch.xpack.core.ml.MachineLearningField; @@ -32,8 +33,7 @@ import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.job.config.JobState; import org.elasticsearch.xpack.core.ml.job.config.MlFilter; -import org.elasticsearch.xpack.core.ml.job.config.RuleCondition; -import org.elasticsearch.persistent.PersistentTasksCustomMetaData; +import org.elasticsearch.xpack.core.ml.job.config.RuleScope; import org.elasticsearch.xpack.ml.job.categorization.CategorizationAnalyzerTests; import org.elasticsearch.xpack.ml.job.persistence.JobProvider; import org.elasticsearch.xpack.ml.job.process.autodetect.UpdateParams; @@ -177,9 +177,7 @@ public void onFailure(Exception e) { public void testUpdateProcessOnFilterChanged() { Detector.Builder detectorReferencingFilter = new Detector.Builder("count", null); detectorReferencingFilter.setByFieldName("foo"); - RuleCondition.createCategorical("foo", "foo_filter"); - DetectionRule filterRule = new DetectionRule.Builder(Collections.singletonList( - RuleCondition.createCategorical("foo", "foo_filter"))).build(); + DetectionRule filterRule = new DetectionRule.Builder(RuleScope.builder().exclude("foo", "foo_filter")).build(); detectorReferencingFilter.setRules(Collections.singletonList(filterRule)); AnalysisConfig.Builder filterAnalysisConfig = new AnalysisConfig.Builder(Collections.singletonList( detectorReferencingFilter.build())); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/ConditionTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/ConditionTests.java deleted file mode 100644 index 09bae9fb8cb9b..0000000000000 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/ConditionTests.java +++ /dev/null @@ -1,114 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.ml.job.config; - -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.common.io.stream.Writeable.Reader; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.test.AbstractSerializingTestCase; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.core.ml.job.config.Condition; -import org.elasticsearch.xpack.core.ml.job.config.Operator; -import org.elasticsearch.xpack.core.ml.job.messages.Messages; - -import java.io.IOException; - -public class ConditionTests extends AbstractSerializingTestCase { - - public void testSetValues() { - Condition cond = new Condition(Operator.EQ, "5"); - assertEquals(Operator.EQ, cond.getOperator()); - assertEquals("5", cond.getValue()); - } - - public void testHashCodeAndEquals() { - Condition cond1 = new Condition(Operator.MATCH, "regex"); - Condition cond2 = new Condition(Operator.MATCH, "regex"); - - assertEquals(cond1, cond2); - assertEquals(cond1.hashCode(), cond2.hashCode()); - - Condition cond3 = new Condition(Operator.EQ, "5"); - assertFalse(cond1.equals(cond3)); - assertFalse(cond1.hashCode() == cond3.hashCode()); - } - - @Override - protected Condition createTestInstance() { - Operator op = randomFrom(Operator.values()); - Condition condition; - switch (op) { - case EQ: - case GT: - case GTE: - case LT: - case LTE: - condition = new Condition(op, Double.toString(randomDouble())); - break; - case MATCH: - condition = new Condition(op, randomAlphaOfLengthBetween(1, 20)); - break; - default: - throw new AssertionError("Unknown operator selected: " + op); - } - return condition; - } - - @Override - protected Reader instanceReader() { - return Condition::new; - } - - @Override - protected Condition doParseInstance(XContentParser parser) { - return Condition.PARSER.apply(parser, null); - } - - public void testVerifyArgsNumericArgs() { - new Condition(Operator.LTE, "100"); - new Condition(Operator.GT, "10.0"); - } - - public void testVerify_GivenEmptyValue() { - ElasticsearchException e = ESTestCase.expectThrows(ElasticsearchException.class, () -> new Condition(Operator.LT, "")); - assertEquals(Messages.getMessage(Messages.JOB_CONFIG_CONDITION_INVALID_VALUE_NUMBER, ""), e.getMessage()); - } - - public void testVerify_GivenInvalidRegex() { - ElasticsearchException e = ESTestCase.expectThrows(ElasticsearchException.class, () -> new Condition(Operator.MATCH, "[*")); - assertEquals(Messages.getMessage(Messages.JOB_CONFIG_CONDITION_INVALID_VALUE_REGEX, "[*"), e.getMessage()); - } - - public void testVerify_GivenNullRegex() { - ElasticsearchException e = ESTestCase.expectThrows(ElasticsearchException.class, - () -> new Condition(Operator.MATCH, null)); - assertEquals(Messages.getMessage(Messages.JOB_CONFIG_CONDITION_INVALID_VALUE_NULL, "[*"), e.getMessage()); - } - - @Override - protected Condition mutateInstance(Condition instance) throws IOException { - Operator op = instance.getOperator(); - String value = instance.getValue(); - switch (between(0, 1)) { - case 0: - Operator newOp = op; - while (newOp == op) { - newOp = randomFrom(Operator.values()); - } - if (op == Operator.MATCH && newOp != Operator.MATCH) { - value = Double.toString(randomDouble()); - } - op = newOp; - break; - case 1: - value = Double.toString(randomDouble()); - break; - default: - throw new AssertionError("Illegal randomisation branch"); - } - return new Condition(op, value); - } -} diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/ConnectiveTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/ConnectiveTests.java deleted file mode 100644 index c8f9b6d36cbca..0000000000000 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/ConnectiveTests.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.ml.job.config; - -import org.elasticsearch.common.io.stream.BytesStreamOutput; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.core.ml.job.config.Connective; - -import java.io.IOException; - -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; - -public class ConnectiveTests extends ESTestCase { - - public void testForString() { - assertEquals(Connective.OR, Connective.fromString("or")); - assertEquals(Connective.OR, Connective.fromString("OR")); - assertEquals(Connective.AND, Connective.fromString("and")); - assertEquals(Connective.AND, Connective.fromString("AND")); - } - - public void testToString() { - assertEquals("or", Connective.OR.toString()); - assertEquals("and", Connective.AND.toString()); - } - - public void testValidOrdinals() { - assertThat(Connective.OR.ordinal(), equalTo(0)); - assertThat(Connective.AND.ordinal(), equalTo(1)); - } - - public void testwriteTo() throws Exception { - try (BytesStreamOutput out = new BytesStreamOutput()) { - Connective.OR.writeTo(out); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(in.readVInt(), equalTo(0)); - } - } - - try (BytesStreamOutput out = new BytesStreamOutput()) { - Connective.AND.writeTo(out); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(in.readVInt(), equalTo(1)); - } - } - } - - public void testReadFrom() throws Exception { - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(0); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(Connective.readFromStream(in), equalTo(Connective.OR)); - } - } - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(1); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(Connective.readFromStream(in), equalTo(Connective.AND)); - } - } - } - - public void testInvalidReadFrom() throws Exception { - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(randomIntBetween(2, Integer.MAX_VALUE)); - try (StreamInput in = out.bytes().streamInput()) { - Connective.readFromStream(in); - fail("Expected IOException"); - } catch (IOException e) { - assertThat(e.getMessage(), containsString("Unknown Connective ordinal [")); - } - } - } -} diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/OperatorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/OperatorTests.java index 07f4d4c5edfa2..469dbf822e005 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/OperatorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/OperatorTests.java @@ -11,7 +11,6 @@ import org.elasticsearch.xpack.core.ml.job.config.Operator; import java.io.IOException; -import java.util.regex.Pattern; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -19,110 +18,63 @@ public class OperatorTests extends ESTestCase { public void testFromString() { - assertEquals(Operator.fromString("eq"), Operator.EQ); assertEquals(Operator.fromString("gt"), Operator.GT); assertEquals(Operator.fromString("gte"), Operator.GTE); assertEquals(Operator.fromString("lte"), Operator.LTE); assertEquals(Operator.fromString("lt"), Operator.LT); - assertEquals(Operator.fromString("match"), Operator.MATCH); assertEquals(Operator.fromString("Gt"), Operator.GT); - assertEquals(Operator.fromString("EQ"), Operator.EQ); assertEquals(Operator.fromString("GTE"), Operator.GTE); - assertEquals(Operator.fromString("Match"), Operator.MATCH); } public void testToString() { - assertEquals("eq", Operator.EQ.toString()); assertEquals("gt", Operator.GT.toString()); assertEquals("gte", Operator.GTE.toString()); assertEquals("lte", Operator.LTE.toString()); assertEquals("lt", Operator.LT.toString()); - assertEquals("match", Operator.MATCH.toString()); } public void testTest() { - assertTrue(Operator.GT.expectsANumericArgument()); assertTrue(Operator.GT.test(1.0, 0.0)); assertFalse(Operator.GT.test(0.0, 1.0)); - assertTrue(Operator.GTE.expectsANumericArgument()); assertTrue(Operator.GTE.test(1.0, 0.0)); assertTrue(Operator.GTE.test(1.0, 1.0)); assertFalse(Operator.GTE.test(0.0, 1.0)); - assertTrue(Operator.EQ.expectsANumericArgument()); - assertTrue(Operator.EQ.test(0.0, 0.0)); - assertFalse(Operator.EQ.test(1.0, 0.0)); - - assertTrue(Operator.LT.expectsANumericArgument()); assertTrue(Operator.LT.test(0.0, 1.0)); assertFalse(Operator.LT.test(0.0, 0.0)); - assertTrue(Operator.LTE.expectsANumericArgument()); assertTrue(Operator.LTE.test(0.0, 1.0)); assertTrue(Operator.LTE.test(1.0, 1.0)); assertFalse(Operator.LTE.test(1.0, 0.0)); } - public void testMatch() { - assertFalse(Operator.MATCH.expectsANumericArgument()); - assertFalse(Operator.MATCH.test(0.0, 1.0)); - - Pattern pattern = Pattern.compile("^aa.*"); - - assertTrue(Operator.MATCH.match(pattern, "aaaaa")); - assertFalse(Operator.MATCH.match(pattern, "bbaaa")); - } - - public void testValidOrdinals() { - assertThat(Operator.EQ.ordinal(), equalTo(0)); - assertThat(Operator.GT.ordinal(), equalTo(1)); - assertThat(Operator.GTE.ordinal(), equalTo(2)); - assertThat(Operator.LT.ordinal(), equalTo(3)); - assertThat(Operator.LTE.ordinal(), equalTo(4)); - assertThat(Operator.MATCH.ordinal(), equalTo(5)); - } - - public void testwriteTo() throws Exception { - try (BytesStreamOutput out = new BytesStreamOutput()) { - Operator.EQ.writeTo(out); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(in.readVInt(), equalTo(0)); - } - } - + public void testWriteTo() throws Exception { try (BytesStreamOutput out = new BytesStreamOutput()) { Operator.GT.writeTo(out); try (StreamInput in = out.bytes().streamInput()) { - assertThat(in.readVInt(), equalTo(1)); + assertThat(in.readVInt(), equalTo(0)); } } try (BytesStreamOutput out = new BytesStreamOutput()) { Operator.GTE.writeTo(out); try (StreamInput in = out.bytes().streamInput()) { - assertThat(in.readVInt(), equalTo(2)); + assertThat(in.readVInt(), equalTo(1)); } } try (BytesStreamOutput out = new BytesStreamOutput()) { Operator.LT.writeTo(out); try (StreamInput in = out.bytes().streamInput()) { - assertThat(in.readVInt(), equalTo(3)); + assertThat(in.readVInt(), equalTo(2)); } } try (BytesStreamOutput out = new BytesStreamOutput()) { Operator.LTE.writeTo(out); try (StreamInput in = out.bytes().streamInput()) { - assertThat(in.readVInt(), equalTo(4)); - } - } - - try (BytesStreamOutput out = new BytesStreamOutput()) { - Operator.MATCH.writeTo(out); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(in.readVInt(), equalTo(5)); + assertThat(in.readVInt(), equalTo(3)); } } } @@ -130,40 +82,28 @@ public void testwriteTo() throws Exception { public void testReadFrom() throws Exception { try (BytesStreamOutput out = new BytesStreamOutput()) { out.writeVInt(0); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(Operator.readFromStream(in), equalTo(Operator.EQ)); - } - } - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(1); try (StreamInput in = out.bytes().streamInput()) { assertThat(Operator.readFromStream(in), equalTo(Operator.GT)); } } try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(2); + out.writeVInt(1); try (StreamInput in = out.bytes().streamInput()) { assertThat(Operator.readFromStream(in), equalTo(Operator.GTE)); } } try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(3); + out.writeVInt(2); try (StreamInput in = out.bytes().streamInput()) { assertThat(Operator.readFromStream(in), equalTo(Operator.LT)); } } try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(4); + out.writeVInt(3); try (StreamInput in = out.bytes().streamInput()) { assertThat(Operator.readFromStream(in), equalTo(Operator.LTE)); } } - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(5); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(Operator.readFromStream(in), equalTo(Operator.MATCH)); - } - } } public void testInvalidReadFrom() throws Exception { diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/RuleActionTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/RuleActionTests.java index 1aeefc47ac61c..fadf81693934b 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/RuleActionTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/RuleActionTests.java @@ -15,27 +15,29 @@ public class RuleActionTests extends ESTestCase { public void testForString() { - assertEquals(RuleAction.FILTER_RESULTS, RuleAction.fromString("filter_results")); - assertEquals(RuleAction.FILTER_RESULTS, RuleAction.fromString("FILTER_RESULTS")); - assertEquals(RuleAction.SKIP_SAMPLING, RuleAction.fromString("SKip_sampLing")); + assertEquals(RuleAction.SKIP_RESULT, RuleAction.fromString("skip_result")); + assertEquals(RuleAction.SKIP_RESULT, RuleAction.fromString("SKIP_RESULT")); + assertEquals(RuleAction.SKIP_MODEL_UPDATE, RuleAction.fromString("skip_model_update")); + assertEquals(RuleAction.SKIP_MODEL_UPDATE, RuleAction.fromString("SKIP_MODEL_UPDATE")); } public void testToString() { - assertEquals("filter_results", RuleAction.FILTER_RESULTS.toString()); + assertEquals("skip_result", RuleAction.SKIP_RESULT.toString()); + assertEquals("skip_model_update", RuleAction.SKIP_MODEL_UPDATE.toString()); } public void testReadFrom() throws Exception { try (BytesStreamOutput out = new BytesStreamOutput()) { out.writeVInt(0); try (StreamInput in = out.bytes().streamInput()) { - assertThat(RuleAction.readFromStream(in), equalTo(RuleAction.FILTER_RESULTS)); + assertThat(RuleAction.readFromStream(in), equalTo(RuleAction.SKIP_RESULT)); } } try (BytesStreamOutput out = new BytesStreamOutput()) { out.writeVInt(1); try (StreamInput in = out.bytes().streamInput()) { - assertThat(RuleAction.readFromStream(in), equalTo(RuleAction.SKIP_SAMPLING)); + assertThat(RuleAction.readFromStream(in), equalTo(RuleAction.SKIP_MODEL_UPDATE)); } } } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/RuleConditionTypeTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/RuleConditionTypeTests.java deleted file mode 100644 index bec7ca24fbe68..0000000000000 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/RuleConditionTypeTests.java +++ /dev/null @@ -1,140 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ -package org.elasticsearch.xpack.ml.job.config; - -import org.elasticsearch.common.io.stream.BytesStreamOutput; -import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.core.ml.job.config.RuleConditionType; - -import java.io.IOException; -import java.util.EnumSet; - -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; - -public class RuleConditionTypeTests extends ESTestCase { - - public void testFromString() { - assertEquals(RuleConditionType.CATEGORICAL, RuleConditionType.fromString("categorical")); - assertEquals(RuleConditionType.CATEGORICAL, RuleConditionType.fromString("CATEGORICAL")); - assertEquals(RuleConditionType.NUMERICAL_ACTUAL, RuleConditionType.fromString("numerical_actual")); - assertEquals(RuleConditionType.NUMERICAL_ACTUAL, RuleConditionType.fromString("NUMERICAL_ACTUAL")); - assertEquals(RuleConditionType.NUMERICAL_TYPICAL, RuleConditionType.fromString("numerical_typical")); - assertEquals(RuleConditionType.NUMERICAL_TYPICAL, RuleConditionType.fromString("NUMERICAL_TYPICAL")); - assertEquals(RuleConditionType.NUMERICAL_DIFF_ABS, RuleConditionType.fromString("numerical_diff_abs")); - assertEquals(RuleConditionType.NUMERICAL_DIFF_ABS, RuleConditionType.fromString("NUMERICAL_DIFF_ABS")); - } - - public void testToString() { - assertEquals("categorical", RuleConditionType.CATEGORICAL.toString()); - assertEquals("categorical_complement", RuleConditionType.CATEGORICAL_COMPLEMENT.toString()); - assertEquals("numerical_actual", RuleConditionType.NUMERICAL_ACTUAL.toString()); - assertEquals("numerical_typical", RuleConditionType.NUMERICAL_TYPICAL.toString()); - assertEquals("numerical_diff_abs", RuleConditionType.NUMERICAL_DIFF_ABS.toString()); - } - - public void testValidOrdinals() { - assertThat(RuleConditionType.CATEGORICAL.ordinal(), equalTo(0)); - assertThat(RuleConditionType.NUMERICAL_ACTUAL.ordinal(), equalTo(1)); - assertThat(RuleConditionType.NUMERICAL_TYPICAL.ordinal(), equalTo(2)); - assertThat(RuleConditionType.NUMERICAL_DIFF_ABS.ordinal(), equalTo(3)); - } - - public void testwriteTo() throws Exception { - try (BytesStreamOutput out = new BytesStreamOutput()) { - RuleConditionType.CATEGORICAL.writeTo(out); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(in.readVInt(), equalTo(0)); - } - } - - try (BytesStreamOutput out = new BytesStreamOutput()) { - RuleConditionType.NUMERICAL_ACTUAL.writeTo(out); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(in.readVInt(), equalTo(1)); - } - } - - try (BytesStreamOutput out = new BytesStreamOutput()) { - RuleConditionType.NUMERICAL_TYPICAL.writeTo(out); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(in.readVInt(), equalTo(2)); - } - } - - try (BytesStreamOutput out = new BytesStreamOutput()) { - RuleConditionType.NUMERICAL_DIFF_ABS.writeTo(out); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(in.readVInt(), equalTo(3)); - } - } - } - - public void testReadFrom() throws Exception { - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(0); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(RuleConditionType.readFromStream(in), equalTo(RuleConditionType.CATEGORICAL)); - } - } - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(1); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(RuleConditionType.readFromStream(in), equalTo(RuleConditionType.NUMERICAL_ACTUAL)); - } - } - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(2); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(RuleConditionType.readFromStream(in), equalTo(RuleConditionType.NUMERICAL_TYPICAL)); - } - } - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(3); - try (StreamInput in = out.bytes().streamInput()) { - assertThat(RuleConditionType.readFromStream(in), equalTo(RuleConditionType.NUMERICAL_DIFF_ABS)); - } - } - } - - public void testInvalidReadFrom() throws Exception { - try (BytesStreamOutput out = new BytesStreamOutput()) { - out.writeVInt(randomIntBetween(4, Integer.MAX_VALUE)); - try (StreamInput in = out.bytes().streamInput()) { - RuleConditionType.readFromStream(in); - fail("Expected IOException"); - } catch (IOException e) { - assertThat(e.getMessage(), containsString("Unknown RuleConditionType ordinal [")); - } - } - } - - public void testIsNumerical() { - for (RuleConditionType type : EnumSet.allOf(RuleConditionType.class)) { - boolean isNumerical = type.isNumerical(); - if (type == RuleConditionType.NUMERICAL_ACTUAL || - type == RuleConditionType.NUMERICAL_DIFF_ABS || - type == RuleConditionType.NUMERICAL_TYPICAL) { - assertTrue(isNumerical); - } else { - assertFalse(isNumerical); - } - } - } - - public void testIsCategorical() { - for (RuleConditionType type : EnumSet.allOf(RuleConditionType.class)) { - boolean isCategorical = type.isCategorical(); - if (type == RuleConditionType.CATEGORICAL || - type == RuleConditionType.CATEGORICAL_COMPLEMENT) { - assertTrue(isCategorical); - } else { - assertFalse(isCategorical); - } - } - } -} diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectCommunicatorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectCommunicatorTests.java index 0aecad2b3a634..cedc65c2ee225 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectCommunicatorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/AutodetectCommunicatorTests.java @@ -22,7 +22,7 @@ import org.elasticsearch.xpack.core.ml.job.config.Detector; import org.elasticsearch.xpack.core.ml.job.config.Job; import org.elasticsearch.xpack.core.ml.job.config.JobUpdate; -import org.elasticsearch.xpack.core.ml.job.config.RuleCondition; +import org.elasticsearch.xpack.core.ml.job.config.RuleScope; import org.elasticsearch.xpack.core.ml.job.process.autodetect.output.FlushAcknowledgement; import org.elasticsearch.xpack.ml.job.categorization.CategorizationAnalyzerTests; import org.elasticsearch.xpack.ml.job.persistence.StateStreamer; @@ -91,10 +91,7 @@ public void testWriteUpdateProcessMessage() throws IOException { when(process.isReady()).thenReturn(true); AutodetectCommunicator communicator = createAutodetectCommunicator(process, mock(AutoDetectResultProcessor.class)); - List conditions = Collections.singletonList( - RuleCondition.createCategorical("foo", "bar")); - - DetectionRule updatedRule = new DetectionRule.Builder(conditions).build(); + DetectionRule updatedRule = new DetectionRule.Builder(RuleScope.builder().exclude("foo", "bar")).build(); List detectorUpdates = Collections.singletonList( new JobUpdate.DetectorUpdate(0, "updated description", Collections.singletonList(updatedRule))); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ControlMsgToProcessWriterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ControlMsgToProcessWriterTests.java index 130476951abbd..8c32a5bb40d46 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ControlMsgToProcessWriterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ControlMsgToProcessWriterTests.java @@ -8,18 +8,16 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.ml.calendars.ScheduledEvent; -import org.elasticsearch.xpack.core.ml.job.config.Condition; -import org.elasticsearch.xpack.core.ml.job.config.Connective; import org.elasticsearch.xpack.core.ml.job.config.DetectionRule; import org.elasticsearch.xpack.core.ml.job.config.MlFilter; import org.elasticsearch.xpack.core.ml.job.config.ModelPlotConfig; import org.elasticsearch.xpack.core.ml.job.config.Operator; import org.elasticsearch.xpack.core.ml.job.config.RuleCondition; -import org.elasticsearch.xpack.core.ml.job.config.RuleConditionType; import org.elasticsearch.xpack.ml.job.process.autodetect.params.DataLoadParams; import org.elasticsearch.xpack.ml.job.process.autodetect.params.FlushJobParams; import org.elasticsearch.xpack.ml.job.process.autodetect.params.TimeRange; import org.junit.Before; +import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.mockito.Mockito; @@ -31,6 +29,7 @@ import java.util.Optional; import java.util.stream.IntStream; +import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -190,22 +189,18 @@ public void testWriteUpdateModelPlotMessage() throws IOException { public void testWriteUpdateDetectorRulesMessage() throws IOException { ControlMsgToProcessWriter writer = new ControlMsgToProcessWriter(lengthEncodedWriter, 4); - DetectionRule rule1 = new DetectionRule.Builder(createRule("5")).setTargetFieldName("targetField1") - .setTargetFieldValue("targetValue").setConditionsConnective(Connective.AND).build(); - DetectionRule rule2 = new DetectionRule.Builder(createRule("5")).setTargetFieldName("targetField2") - .setTargetFieldValue("targetValue").setConditionsConnective(Connective.AND).build(); + DetectionRule rule1 = new DetectionRule.Builder(createRule(5)).build(); + DetectionRule rule2 = new DetectionRule.Builder(createRule(5)).build(); writer.writeUpdateDetectorRulesMessage(2, Arrays.asList(rule1, rule2)); InOrder inOrder = inOrder(lengthEncodedWriter); inOrder.verify(lengthEncodedWriter).writeNumFields(4); inOrder.verify(lengthEncodedWriter, times(3)).writeField(""); inOrder.verify(lengthEncodedWriter).writeField("u[detectorRules]\ndetectorIndex=2\n" + - "rulesJson=[{\"actions\":[\"filter_results\"],\"conditions_connective\":\"and\",\"conditions\":" + - "[{\"type\":\"numerical_actual\",\"condition\":{\"operator\":\"gt\",\"value\":\"5\"}}]," + - "\"target_field_name\":\"targetField1\",\"target_field_value\":\"targetValue\"}," + - "{\"actions\":[\"filter_results\"],\"conditions_connective\":\"and\",\"conditions\":[" + - "{\"type\":\"numerical_actual\",\"condition\":{\"operator\":\"gt\",\"value\":\"5\"}}]," + - "\"target_field_name\":\"targetField2\",\"target_field_value\":\"targetValue\"}]"); + "rulesJson=[{\"actions\":[\"skip_result\"],\"conditions\":" + + "[{\"applies_to\":\"actual\",\"operator\":\"gt\",\"value\":5.0}]}," + + "{\"actions\":[\"skip_result\"],\"conditions\":[" + + "{\"applies_to\":\"actual\",\"operator\":\"gt\",\"value\":5.0}]}]"); verifyNoMoreInteractions(lengthEncodedWriter); } @@ -244,16 +239,17 @@ public void testWriteUpdateScheduledEventsMessage() throws IOException { InOrder inOrder = inOrder(lengthEncodedWriter); inOrder.verify(lengthEncodedWriter).writeNumFields(2); inOrder.verify(lengthEncodedWriter, times(1)).writeField(""); - inOrder.verify(lengthEncodedWriter).writeField("u[scheduledEvents]\n" + ArgumentCaptor capturedMessage = ArgumentCaptor.forClass(String.class); + inOrder.verify(lengthEncodedWriter).writeField(capturedMessage.capture()); + assertThat(capturedMessage.getValue(), equalTo("u[scheduledEvents]\n" + "scheduledevent.0.description = new year\n" - + "scheduledevent.0.rules = [{\"actions\":[\"filter_results\",\"skip_sampling\"],\"conditions_connective\":\"and\"," - + "\"conditions\":[{\"type\":\"time\",\"condition\":{\"operator\":\"gte\",\"value\":\"1514764800\"}}," - + "{\"type\":\"time\",\"condition\":{\"operator\":\"lt\",\"value\":\"1514851200\"}}]}]\n" + + "scheduledevent.0.rules = [{\"actions\":[\"skip_result\",\"skip_model_update\"]," + + "\"conditions\":[{\"applies_to\":\"time\",\"operator\":\"gte\",\"value\":1.5147648E9}," + + "{\"applies_to\":\"time\",\"operator\":\"lt\",\"value\":1.5148512E9}]}]\n" + "scheduledevent.1.description = Jan maintenance day\n" - + "scheduledevent.1.rules = [{\"actions\":[\"filter_results\",\"skip_sampling\"],\"conditions_connective\":\"and\"," - + "\"conditions\":[{\"type\":\"time\",\"condition\":{\"operator\":\"gte\",\"value\":\"1515196800\"}}," - + "{\"type\":\"time\",\"condition\":{\"operator\":\"lt\",\"value\":\"1515283200\"}}]}]\n" - ); + + "scheduledevent.1.rules = [{\"actions\":[\"skip_result\",\"skip_model_update\"]," + + "\"conditions\":[{\"applies_to\":\"time\",\"operator\":\"gte\",\"value\":1.5151968E9}," + + "{\"applies_to\":\"time\",\"operator\":\"lt\",\"value\":1.5152832E9}]}]\n")); verifyNoMoreInteractions(lengthEncodedWriter); } @@ -288,8 +284,7 @@ public void testWriteStartBackgroundPersistMessage() throws IOException { verifyNoMoreInteractions(lengthEncodedWriter); } - private static List createRule(String value) { - Condition condition = new Condition(Operator.GT, value); - return Collections.singletonList(RuleCondition.createNumerical(RuleConditionType.NUMERICAL_ACTUAL, null, null, condition)); + private static List createRule(double value) { + return Collections.singletonList(new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.GT, value)); } } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/FieldConfigWriterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/FieldConfigWriterTests.java index 57917f6761bb7..bf08d09bf090c 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/FieldConfigWriterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/FieldConfigWriterTests.java @@ -10,16 +10,14 @@ import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.test.ESTestCase; -import org.elasticsearch.xpack.ml.MachineLearning; import org.elasticsearch.xpack.core.ml.calendars.ScheduledEvent; import org.elasticsearch.xpack.core.ml.job.config.AnalysisConfig; -import org.elasticsearch.xpack.core.ml.job.config.Condition; import org.elasticsearch.xpack.core.ml.job.config.DetectionRule; import org.elasticsearch.xpack.core.ml.job.config.Detector; import org.elasticsearch.xpack.core.ml.job.config.MlFilter; import org.elasticsearch.xpack.core.ml.job.config.Operator; import org.elasticsearch.xpack.core.ml.job.config.RuleCondition; -import org.elasticsearch.xpack.core.ml.job.config.RuleConditionType; +import org.elasticsearch.xpack.ml.MachineLearning; import org.ini4j.Config; import org.ini4j.Ini; import org.ini4j.Profile.Section; @@ -193,9 +191,8 @@ public void testWrite_GivenDetectorWithRules() throws IOException { Detector.Builder detector = new Detector.Builder("mean", "metricValue"); detector.setByFieldName("metricName"); detector.setPartitionFieldName("instance"); - RuleCondition ruleCondition = RuleCondition.createNumerical - (RuleConditionType.NUMERICAL_ACTUAL, "metricName", "metricValue", new Condition(Operator.LT, "5")); - DetectionRule rule = new DetectionRule.Builder(Collections.singletonList(ruleCondition)).setTargetFieldName("instance").build(); + RuleCondition ruleCondition = new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.LT, 5); + DetectionRule rule = new DetectionRule.Builder(Collections.singletonList(ruleCondition)).build(); detector.setRules(Collections.singletonList(rule)); AnalysisConfig.Builder builder = new AnalysisConfig.Builder(Collections.singletonList(detector.build())); @@ -255,14 +252,12 @@ public void testWrite_GivenScheduledEvents() throws IOException { verify(writer).write("detector.0.clause = count\n" + "scheduledevent.0.description = The Ashes\n" + - "scheduledevent.0.rules = [{\"actions\":[\"filter_results\",\"skip_sampling\"],\"conditions_connective\":\"and\"," + - "\"conditions\":[{\"type\":\"time\",\"condition\":{\"operator\":\"gte\",\"value\":\"1511395200\"}}," + - "{\"type\":\"time\",\"condition\":{\"operator\":\"lt\",\"value\":\"1515369600\"}}]}]\n" + + "scheduledevent.0.rules = [{\"actions\":[\"skip_result\",\"skip_model_update\"],\"conditions\":[{\"applies_to\":\"time\"," + + "\"operator\":\"gte\",\"value\":1.5113952E9},{\"applies_to\":\"time\",\"operator\":\"lt\",\"value\":1.5153696E9}]}]\n" + "scheduledevent.1.description = elasticon\n" + - "scheduledevent.1.rules = [{\"actions\":[\"filter_results\",\"skip_sampling\"],\"conditions_connective\":\"and\"," + - "\"conditions\":[{\"type\":\"time\",\"condition\":{\"operator\":\"gte\",\"value\":\"1519603200\"}}," + - "{\"type\":\"time\",\"condition\":{\"operator\":\"lt\",\"value\":\"1519862400\"}}]}]" + - "\n"); + "scheduledevent.1.rules = [{\"actions\":[\"skip_result\",\"skip_model_update\"]," + + "\"conditions\":[{\"applies_to\":\"time\",\"operator\":\"gte\",\"value\":1.5196032E9}," + + "{\"applies_to\":\"time\",\"operator\":\"lt\",\"value\":1.5198624E9}]}]\n"); verifyNoMoreInteractions(writer); } diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ScheduledEventsWriterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ScheduledEventsWriterTests.java index ae1b77b34089c..2806aef128575 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ScheduledEventsWriterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/process/autodetect/writer/ScheduledEventsWriterTests.java @@ -44,13 +44,13 @@ public void testWrite() throws IOException { new ScheduledEventsWriter(events, TimeValue.timeValueHours(1), buffer).write(); String expectedString = "scheduledevent.0.description = Black Friday\n" + - "scheduledevent.0.rules = [{\"actions\":[\"filter_results\",\"skip_sampling\"],\"conditions_connective\":\"and\"," + - "\"conditions\":[{\"type\":\"time\",\"condition\":{\"operator\":\"gte\",\"value\":\"1511395200\"}}," + - "{\"type\":\"time\",\"condition\":{\"operator\":\"lt\",\"value\":\"1515369600\"}}]}]\n" + + "scheduledevent.0.rules = [{\"actions\":[\"skip_result\",\"skip_model_update\"]," + + "\"conditions\":[{\"applies_to\":\"time\",\"operator\":\"gte\",\"value\":1.5113952E9}," + + "{\"applies_to\":\"time\",\"operator\":\"lt\",\"value\":1.5153696E9}]}]\n" + "scheduledevent.1.description = Blue Monday\n" + - "scheduledevent.1.rules = [{\"actions\":[\"filter_results\",\"skip_sampling\"],\"conditions_connective\":\"and\"," + - "\"conditions\":[{\"type\":\"time\",\"condition\":{\"operator\":\"gte\",\"value\":\"1519603200\"}}," + - "{\"type\":\"time\",\"condition\":{\"operator\":\"lt\",\"value\":\"1519862400\"}}]}]" + + "scheduledevent.1.rules = [{\"actions\":[\"skip_result\",\"skip_model_update\"]," + + "\"conditions\":[{\"applies_to\":\"time\",\"operator\":\"gte\",\"value\":1.5196032E9}," + + "{\"applies_to\":\"time\",\"operator\":\"lt\",\"value\":1.5198624E9}]}]" + "\n"; assertThat(buffer.toString(), equalTo(expectedString)); } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml index 5203ead3ce8a9..d3165260f4b95 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/filter_crud.yml @@ -152,15 +152,11 @@ setup: "analysis_config" : { "bucket_span": "3600s", "detectors" :[{"function":"mean","field_name":"responsetime", "by_field_name": "airline", - "rules": [ + "custom_rules": [ { - "conditions": [ - { - "type": "categorical", - "field_name": "airline", - "filter_id": "filter-foo" - } - ] + "scope": { + "airline": {"filter_id": "filter-foo"} + } } ]}] }, diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml index ed6b9ead1a3b1..df505176ae739 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml @@ -301,10 +301,26 @@ { "groups": ["group-1", "group-2"], "description":"Post update description", - "detectors": [{"detector_index": 0, "rules": {"target_field_name": "airline", - "conditions": [ { "type": "numerical_actual", - "condition": {"operator": "gt", "value": "10" } } ] } }, - {"detector_index": 1, "description": "updated description"}], + "detectors": [ + { + "detector_index": 0, + "custom_rules":[ + { + "conditions": [ + { + "applies_to": "actual", + "operator": "gt", + "value": 10 + } + ] + } + ] + }, + { + "detector_index": 1, + "description": "updated description" + } + ], "model_plot_config": { "enabled": false, "terms": "foobar" @@ -324,7 +340,8 @@ - match: { model_plot_config.enabled: false } - match: { model_plot_config.terms: "foobar" } - match: { analysis_config.categorization_filters: ["cat3.*"] } - - match: { analysis_config.detectors.0.rules.0.target_field_name: "airline" } + - match: { analysis_config.detectors.0.custom_rules.0.actions: ["skip_result"] } + - length: { analysis_config.detectors.0.custom_rules.0.conditions: 1 } - match: { analysis_config.detectors.0.detector_index: 0 } - match: { analysis_config.detectors.1.detector_description: "updated description" } - match: { analysis_config.detectors.1.detector_index: 1 } @@ -1223,17 +1240,23 @@ { "function": "count", "by_field_name": "country", - "rules": [ + "custom_rules": [ { - "actions": ["filter_results", "skip_sampling"], + "actions": ["skip_result", "skip_model_update"], + "scope": { + "country": {"filter_id": "safe_countries"} + }, "conditions": [ { - "type":"numerical_actual", - "field_name":"country", - "field_value": "uk", - "condition": {"operator":"lt","value":"33.3"} + "applies_to":"actual", + "operator":"lt", + "value": 33.3 }, - {"type":"categorical", "field_name":"country", "filter_id": "foo"} + { + "applies_to":"typical", + "operator":"lte", + "value": 42.0 + } ] } ] @@ -1248,83 +1271,23 @@ job_id: jobs-crud-rules - match: { count: 1 } - match: { - jobs.0.analysis_config.detectors.0.rules: [ + jobs.0.analysis_config.detectors.0.custom_rules: [ { - "actions": ["filter_results", "skip_sampling"], - "conditions_connective": "or", + "actions": ["skip_result", "skip_model_update"], + "scope": { + "country": {"filter_id": "safe_countries", "filter_type": "include"} + }, "conditions": [ { - "type":"numerical_actual", - "field_name":"country", - "field_value": "uk", - "condition": {"operator":"lt","value":"33.3"} - }, - {"type":"categorical", "field_name":"country", "filter_id": "foo"} - ] - } - ] - } - ---- -"Test job with pre 6.2 rules": - - - skip: - features: "warnings" - reason: certain rule fields were renamed in 6.2.0 - - - do: - warnings: - - Deprecated field [detector_rules] used, expected [rules] instead - - Deprecated field [rule_action] used, expected [actions] instead - - Deprecated field [rule_conditions] used, expected [conditions] instead - - Deprecated field [condition_type] used, expected [type] instead - - Deprecated field [value_filter] used, expected [filter_id] instead - xpack.ml.put_job: - job_id: jobs-crud-pre-6-2-rules - body: > - { - "analysis_config": { - "detectors": [ - { - "function": "count", - "by_field_name": "country", - "detector_rules": [ - { - "rule_action": "filter_results", - "rule_conditions": [ - { - "condition_type":"numerical_actual", - "field_name":"country", - "field_value": "uk", - "condition": {"operator":"lt","value":"33.3"} - }, - {"type":"categorical", "field_name":"country", "value_filter": "foo"} - ] - } - ] - } - ] + "applies_to":"actual", + "operator":"lt", + "value": 33.3 }, - "data_description" : {} - } - - - do: - xpack.ml.get_jobs: - job_id: jobs-crud-pre-6-2-rules - - match: { count: 1 } - - match: { - jobs.0.analysis_config.detectors.0.rules: [ - { - "actions": ["filter_results"], - "conditions_connective": "or", - "conditions": [ { - "type":"numerical_actual", - "field_name":"country", - "field_value": "uk", - "condition": {"operator":"lt","value":"33.3"} - }, - {"type":"categorical", "field_name":"country", "filter_id": "foo"} + "applies_to":"typical", + "operator":"lte", + "value": 42.0 + } ] } ] diff --git a/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DetectionRulesIT.java b/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DetectionRulesIT.java index 47ced4a96dde8..aa53d6255cb8e 100644 --- a/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DetectionRulesIT.java +++ b/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/DetectionRulesIT.java @@ -12,7 +12,6 @@ import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.xpack.core.ml.action.GetRecordsAction; import org.elasticsearch.xpack.core.ml.job.config.AnalysisConfig; -import org.elasticsearch.xpack.core.ml.job.config.Condition; import org.elasticsearch.xpack.core.ml.job.config.DataDescription; import org.elasticsearch.xpack.core.ml.job.config.DetectionRule; import org.elasticsearch.xpack.core.ml.job.config.Detector; @@ -21,10 +20,9 @@ import org.elasticsearch.xpack.core.ml.job.config.MlFilter; import org.elasticsearch.xpack.core.ml.job.config.Operator; import org.elasticsearch.xpack.core.ml.job.config.RuleCondition; -import org.elasticsearch.xpack.core.ml.job.config.RuleConditionType; +import org.elasticsearch.xpack.core.ml.job.config.RuleScope; import org.elasticsearch.xpack.core.ml.job.results.AnomalyRecord; import org.junit.After; -import org.junit.Ignore; import java.io.IOException; import java.util.ArrayList; @@ -35,9 +33,7 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Collectors; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.isOneOf; @@ -48,38 +44,22 @@ public class DetectionRulesIT extends MlNativeAutodetectIntegTestCase { @After - public void cleanUpTest() throws Exception { + public void cleanUpTest() { cleanUp(); } - @AwaitsFix(bugUrl = "this test is muted temporarily until the new rules implementation is merged in") - public void testNumericalRule() throws Exception { - RuleCondition condition1 = RuleCondition.createNumerical( - RuleConditionType.NUMERICAL_ACTUAL, - "by_field", - "by_field_value_1", - new Condition(Operator.LT, "1000")); - RuleCondition condition2 = RuleCondition.createNumerical( - RuleConditionType.NUMERICAL_ACTUAL, - "by_field", - "by_field_value_2", - new Condition(Operator.LT, "500")); - RuleCondition condition3 = RuleCondition.createNumerical( - RuleConditionType.NUMERICAL_ACTUAL, - "by_field", - "by_field_value_3", - new Condition(Operator.LT, "100")); - DetectionRule rule = new DetectionRule.Builder(Arrays.asList(condition1, condition2, condition3)).build(); - - Detector.Builder detector = new Detector.Builder("max", "value"); - detector.setRules(Arrays.asList(rule)); - detector.setByFieldName("by_field"); + public void testCondition() throws Exception { + DetectionRule rule = new DetectionRule.Builder(Arrays.asList( + new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.LT, 100.0) + )).build(); - AnalysisConfig.Builder analysisConfig = new AnalysisConfig.Builder( - Arrays.asList(detector.build())); + Detector.Builder detector = new Detector.Builder("mean", "value"); + detector.setByFieldName("by_field"); + detector.setRules(Arrays.asList(rule)); + AnalysisConfig.Builder analysisConfig = new AnalysisConfig.Builder(Arrays.asList(detector.build())); analysisConfig.setBucketSpan(TimeValue.timeValueHours(1)); DataDescription.Builder dataDescription = new DataDescription.Builder(); - Job.Builder job = new Job.Builder("detection-rule-numeric-test"); + Job.Builder job = new Job.Builder("detection-rules-it-test-condition"); job.setAnalysisConfig(analysisConfig); job.setDataDescription(dataDescription); @@ -91,12 +71,11 @@ public void testNumericalRule() throws Exception { int totalBuckets = 2 * 24; // each half of the buckets contains one anomaly for each by field value Set anomalousBuckets = new HashSet<>(Arrays.asList(20, 44)); - List byFieldValues = Arrays.asList("by_field_value_1", "by_field_value_2", "by_field_value_3"); + List byFieldValues = Arrays.asList("low", "high"); Map anomalousValues = new HashMap<>(); - anomalousValues.put("by_field_value_1", 800); - anomalousValues.put("by_field_value_2", 400); - anomalousValues.put("by_field_value_3", 400); - int normalValue = 1; + anomalousValues.put("low", 99); + anomalousValues.put("high", 701); + int normalValue = 400; List data = new ArrayList<>(); for (int bucket = 0; bucket < totalBuckets; bucket++) { for (String byFieldValue : byFieldValues) { @@ -115,27 +94,14 @@ public void testNumericalRule() throws Exception { List records = getRecords(job.getId()); assertThat(records.size(), equalTo(1)); - assertThat(records.get(0).getByFieldValue(), equalTo("by_field_value_3")); + assertThat(records.get(0).getByFieldValue(), equalTo("high")); long firstRecordTimestamp = records.get(0).getTimestamp().getTime(); { // Update rules so that the anomalies suppression is inverted - RuleCondition newCondition1 = RuleCondition.createNumerical( - RuleConditionType.NUMERICAL_ACTUAL, - "by_field", - "by_field_value_1", - new Condition(Operator.GT, "1000")); - RuleCondition newCondition2 = RuleCondition.createNumerical( - RuleConditionType.NUMERICAL_ACTUAL, - "by_field", - "by_field_value_2", - new Condition(Operator.GT, "500")); - RuleCondition newCondition3 = RuleCondition.createNumerical( - RuleConditionType.NUMERICAL_ACTUAL, - "by_field", - "by_field_value_3", - new Condition(Operator.GT, "0")); - DetectionRule newRule = new DetectionRule.Builder(Arrays.asList(newCondition1, newCondition2, newCondition3)).build(); + DetectionRule newRule = new DetectionRule.Builder(Arrays.asList( + new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.GT, 700.0) + )).build(); JobUpdate.Builder update = new JobUpdate.Builder(job.getId()); update.setDetectorUpdates(Arrays.asList(new JobUpdate.DetectorUpdate(0, null, Arrays.asList(newRule)))); updateJob(job.getId(), update.build()); @@ -149,18 +115,15 @@ public void testNumericalRule() throws Exception { GetRecordsAction.Request recordsAfterFirstHalf = new GetRecordsAction.Request(job.getId()); recordsAfterFirstHalf.setStart(String.valueOf(firstRecordTimestamp + 1)); records = getRecords(recordsAfterFirstHalf); - assertThat(records.size(), equalTo(2)); - Set secondHaldRecordByFieldValues = records.stream().map(AnomalyRecord::getByFieldValue).collect(Collectors.toSet()); - assertThat(secondHaldRecordByFieldValues, contains("by_field_value_1", "by_field_value_2")); + assertThat(records.size(), equalTo(1)); + assertThat(records.get(0).getByFieldValue(), equalTo("low")); } - @AwaitsFix(bugUrl = "this test is muted temporarily until the new rules implementation is merged in") - public void testCategoricalRule() throws Exception { + public void testScope() throws Exception { MlFilter safeIps = new MlFilter("safe_ips", Arrays.asList("111.111.111.111", "222.222.222.222")); assertThat(putMlFilter(safeIps), is(true)); - RuleCondition condition = RuleCondition.createCategorical("ip", safeIps.getId()); - DetectionRule rule = new DetectionRule.Builder(Collections.singletonList(condition)).build(); + DetectionRule rule = new DetectionRule.Builder(RuleScope.builder().include("ip", "safe_ips")).build(); Detector.Builder detector = new Detector.Builder("count", null); detector.setRules(Arrays.asList(rule)); @@ -169,7 +132,7 @@ public void testCategoricalRule() throws Exception { AnalysisConfig.Builder analysisConfig = new AnalysisConfig.Builder(Collections.singletonList(detector.build())); analysisConfig.setBucketSpan(TimeValue.timeValueHours(1)); DataDescription.Builder dataDescription = new DataDescription.Builder(); - Job.Builder job = new Job.Builder("detection-rule-categorical-test"); + Job.Builder job = new Job.Builder("detection-rules-it-test-scope"); job.setAnalysisConfig(analysisConfig); job.setDataDescription(dataDescription); @@ -263,6 +226,70 @@ public void testCategoricalRule() throws Exception { closeJob(job.getId()); } + public void testScopeAndCondition() throws IOException { + // We have 2 IPs and they're both safe-listed. + List ips = Arrays.asList("111.111.111.111", "222.222.222.222"); + MlFilter safeIps = new MlFilter("safe_ips", ips); + assertThat(putMlFilter(safeIps), is(true)); + + // Ignore if ip in safe list AND actual < 10. + DetectionRule rule = new DetectionRule.Builder(RuleScope.builder().include("ip", "safe_ips")) + .setConditions(Arrays.asList(new RuleCondition(RuleCondition.AppliesTo.ACTUAL, Operator.LT, 10.0))) + .build(); + + Detector.Builder detector = new Detector.Builder("count", null); + detector.setRules(Arrays.asList(rule)); + detector.setOverFieldName("ip"); + + AnalysisConfig.Builder analysisConfig = new AnalysisConfig.Builder(Collections.singletonList(detector.build())); + analysisConfig.setBucketSpan(TimeValue.timeValueHours(1)); + DataDescription.Builder dataDescription = new DataDescription.Builder(); + Job.Builder job = new Job.Builder("detection-rules-it-test-scope-and-condition"); + job.setAnalysisConfig(analysisConfig); + job.setDataDescription(dataDescription); + + registerJob(job); + putJob(job); + openJob(job.getId()); + + long timestamp = 1509062400000L; + List data = new ArrayList<>(); + + // First, 20 buckets with a count of 1 for both IPs + for (int bucket = 0; bucket < 20; bucket++) { + for (String ip : ips) { + data.add(createIpRecord(timestamp, ip)); + } + timestamp += TimeValue.timeValueHours(1).getMillis(); + } + + // Now send anomalous count of 9 for 111.111.111.111 + for (int i = 0; i < 9; i++) { + data.add(createIpRecord(timestamp, "111.111.111.111")); + } + + // and 10 for 222.222.222.222 + for (int i = 0; i < 10; i++) { + data.add(createIpRecord(timestamp, "222.222.222.222")); + } + timestamp += TimeValue.timeValueHours(1).getMillis(); + + // Some more normal buckets + for (int bucket = 0; bucket < 3; bucket++) { + for (String ip : ips) { + data.add(createIpRecord(timestamp, ip)); + } + timestamp += TimeValue.timeValueHours(1).getMillis(); + } + + postData(job.getId(), joinBetween(0, data.size(), data)); + closeJob(job.getId()); + + List records = getRecords(job.getId()); + assertThat(records.size(), equalTo(1)); + assertThat(records.get(0).getOverFieldValue(), equalTo("222.222.222.222")); + } + private String createIpRecord(long timestamp, String ip) throws IOException { Map record = new HashMap<>(); record.put("time", timestamp); diff --git a/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ScheduledEventsIT.java b/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ScheduledEventsIT.java index 8410075ff5e27..6703e4ef2365b 100644 --- a/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ScheduledEventsIT.java +++ b/x-pack/qa/ml-native-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ScheduledEventsIT.java @@ -20,7 +20,6 @@ import org.elasticsearch.xpack.core.ml.job.results.AnomalyRecord; import org.elasticsearch.xpack.core.ml.job.results.Bucket; import org.junit.After; -import org.junit.Ignore; import java.io.IOException; import java.time.Instant; @@ -42,7 +41,6 @@ public void cleanUpTest() { cleanUp(); } - @AwaitsFix(bugUrl = "this test is muted temporarily until the new rules implementation is merged in") public void testScheduledEvents() throws IOException { TimeValue bucketSpan = TimeValue.timeValueMinutes(30); @@ -154,7 +152,6 @@ public void testScheduledEvents() throws IOException { assertThat(records, is(empty())); } - @AwaitsFix(bugUrl = "this test is muted temporarily until the new rules implementation is merged in") public void testScheduledEventWithInterimResults() throws IOException { TimeValue bucketSpan = TimeValue.timeValueMinutes(30); Job.Builder job = createJob("scheduled-events-interim-results", bucketSpan); @@ -196,7 +193,6 @@ public void testScheduledEventWithInterimResults() throws IOException { /** * Test an open job picks up changes to scheduled events/calendars */ - @AwaitsFix(bugUrl = "this test is muted temporarily until the new rules implementation is merged in") public void testOnlineUpdate() throws Exception { TimeValue bucketSpan = TimeValue.timeValueMinutes(30); Job.Builder job = createJob("scheduled-events-online-update", bucketSpan); diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/30_ml_jobs_crud.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/30_ml_jobs_crud.yml index 6ea8771c2374e..c1b238422e92e 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/30_ml_jobs_crud.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/30_ml_jobs_crud.yml @@ -91,9 +91,9 @@ wait_for_status: green --- -"Test get job with rules": +"Test job with pre 6.4 rules": - do: xpack.ml.get_jobs: - job_id: old-cluster-job-with-rules + job_id: job-with-old-rules - match: { count: 1 } diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/30_ml_jobs_crud.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/30_ml_jobs_crud.yml index da36da301c1f8..3a3334f6907e9 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/30_ml_jobs_crud.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/30_ml_jobs_crud.yml @@ -134,15 +134,15 @@ } --- -"Test job with pre 6.2 rules": +"Test job with pre 6.4 rules": - skip: - version: "6.2.0 - " - reason: "Rules fields were renamed on 6.2.0" + version: "6.4.0 - " + reason: "Rules were replaced by custom_rules on 6.4.0" - do: xpack.ml.put_job: - job_id: old-cluster-job-with-rules + job_id: job-with-old-rules body: > { "analysis_config": { @@ -171,36 +171,22 @@ } --- -"Test job with post 6.2 rules": +"Test job with pre 6.4 rules - dummy job 6.4 onwards": - skip: - version: " - 6.1.99" - reason: "Rules fields were renamed on 6.2.0" + version: " - 6.3.99" + reason: "Rules replaced by custom_rules on 6.4.0" - do: xpack.ml.put_job: - job_id: old-cluster-job-with-rules + job_id: job-with-old-rules body: > { "analysis_config": { "detectors": [ { "function": "count", - "by_field_name": "country", - "rules": [ - { - "actions": ["filter_results"], - "conditions": [ - { - "type":"numerical_actual", - "field_name":"country", - "field_value": "uk", - "condition": {"operator":"lt","value":"33.3"} - }, - {"type":"categorical", "field_name":"country", "filter_id": "foo"} - ] - } - ] + "by_field_name": "country" } ] }, diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/30_ml_jobs_crud.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/30_ml_jobs_crud.yml index 91d294572894d..6634722fac977 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/30_ml_jobs_crud.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/30_ml_jobs_crud.yml @@ -100,29 +100,14 @@ setup: - match: { acknowledged: true } --- -"Test get job with rules": +"Test job with pre 6.4 rules": - do: xpack.ml.get_jobs: - job_id: old-cluster-job-with-rules + job_id: job-with-old-rules - match: { count: 1 } - - match: { - jobs.0.analysis_config.detectors.0.rules: [ - { - "actions": ["filter_results"], - "conditions_connective": "or", - "conditions": [ - { - "type":"numerical_actual", - "field_name":"country", - "field_value": "uk", - "condition": {"operator":"lt","value":"33.3"} - }, - {"type":"categorical", "field_name":"country", "filter_id": "foo"} - ] - } - ] - } + - is_false: jobs.0.analysis_config.detectors.0.rules + - is_false: jobs.0.analysis_config.detectors.0.custom_rules --- "Test get job with function shortcut should expand":