From b1c19778e80a6d2cc126ff7a90915ee254025ed7 Mon Sep 17 00:00:00 2001 From: Dimitris Athanasiou Date: Fri, 15 Jun 2018 16:05:22 +0100 Subject: [PATCH] [ML] Implement new rules design (#31110) (#31294) Rules allow users to supply a detector with domain knowledge that can improve the quality of the results. The model detects statistically anomalous results but it has no knowledge of the meaning of the values being modelled. For example, a detector that performs a population analysis over IP addresses could benefit from a list of IP addresses that the user knows to be safe. Then anomalous results for those IP addresses will not be created and will not affect the quantiles either. Another example would be a detector looking for anomalies in the median value of CPU utilization. A user might want to inform the detector that any results where the actual value is less than 5 is not interesting. This commit introduces a `custom_rules` field to the `Detector`. A detector may have multiple rules which are combined with `or`. A rule has 3 fields: `actions`, `scope` and `conditions`. Actions is a list of what should happen when the rule applies. The current options include `skip_result` and `skip_model_update`. The default value for `actions` is the `skip_result` action. Scope is optional and allows for applying filters on any of the partition/over/by field. When not defined the rule applies to all series. The `filter_id` needs to be specified to match the id of the filter to be used. Optionally, the `filter_type` can be specified as either `include` (default) or `exclude`. When set to `include` the rule applies to entities that are in the filter. When set to `exclude` the rule only applies to entities not in the filter. There may be zero or more conditions. A condition requires `applies_to`, `operator` and `value` to be specified. The `applies_to` value can be either `actual`, `typical` or `diff_from_typical` and it specifies the numerical value to which the condition applies. The `operator` (`lt`, `lte`, `gt`, `gte`) and `value` complete the definition. Conditions are combined with `and` and allow to specify numerical conditions for when a rule applies. A rule must either have a scope or one or more conditions. Finally, a rule with scope and conditions applies when all of them apply. --- .../core/ml/calendars/ScheduledEvent.java | 4 +- .../xpack/core/ml/job/config/Condition.java | 132 ---------- .../xpack/core/ml/job/config/Connective.java | 42 ---- .../core/ml/job/config/DetectionRule.java | 181 ++++++------- .../xpack/core/ml/job/config/Detector.java | 158 ++++-------- .../xpack/core/ml/job/config/FilterRef.java | 123 +++++++++ .../xpack/core/ml/job/config/JobUpdate.java | 4 +- .../xpack/core/ml/job/config/Operator.java | 31 +-- .../xpack/core/ml/job/config/RuleAction.java | 4 +- .../core/ml/job/config/RuleCondition.java | 237 +++++------------- .../core/ml/job/config/RuleConditionType.java | 69 ----- .../xpack/core/ml/job/config/RuleScope.java | 143 +++++++++++ .../xpack/core/ml/job/messages/Messages.java | 33 +-- .../ml/calendars/ScheduledEventTests.java | 19 +- .../ml/job/config/AnalysisConfigTests.java | 8 +- .../ml/job/config/DetectionRuleTests.java | 157 +++++------- .../core/ml/job/config/DetectorTests.java | 205 ++++++++------- .../core/ml/job/config/FilterRefTests.java | 30 +++ .../core/ml/job/config/JobUpdateTests.java | 12 +- .../ml/job/config/RuleConditionTests.java | 234 +++-------------- .../core/ml/job/config/RuleScopeTests.java | 81 ++++++ .../ml/action/TransportOpenJobAction.java | 18 +- .../xpack/ml/job/JobManager.java | 1 + .../action/TransportOpenJobActionTests.java | 84 ++++++- .../xpack/ml/integration/JobProviderIT.java | 28 +-- .../xpack/ml/job/JobManagerTests.java | 8 +- .../xpack/ml/job/config/ConditionTests.java | 114 --------- .../xpack/ml/job/config/ConnectiveTests.java | 79 ------ .../xpack/ml/job/config/OperatorTests.java | 76 +----- .../xpack/ml/job/config/RuleActionTests.java | 14 +- .../ml/job/config/RuleConditionTypeTests.java | 140 ----------- .../AutodetectCommunicatorTests.java | 7 +- .../ControlMsgToProcessWriterTests.java | 43 ++-- .../writer/FieldConfigWriterTests.java | 21 +- .../writer/ScheduledEventsWriterTests.java | 12 +- .../rest-api-spec/test/ml/filter_crud.yml | 12 +- .../rest-api-spec/test/ml/jobs_crud.yml | 131 ++++------ .../ml/integration/DetectionRulesIT.java | 148 ++++++----- .../ml/integration/ScheduledEventsIT.java | 3 - .../test/mixed_cluster/30_ml_jobs_crud.yml | 4 +- .../test/old_cluster/30_ml_jobs_crud.yml | 26 +- .../test/upgraded_cluster/30_ml_jobs_crud.yml | 23 +- 42 files changed, 1108 insertions(+), 1791 deletions(-) delete mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Condition.java delete mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/Connective.java create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/FilterRef.java delete mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleConditionType.java create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleScope.java create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/FilterRefTests.java create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/job/config/RuleScopeTests.java delete mode 100644 x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/ConditionTests.java delete mode 100644 x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/ConnectiveTests.java delete mode 100644 x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/config/RuleConditionTypeTests.java 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..32477fdec04e6 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,94 @@ 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); + actions = in.readEnumSet(RuleAction.class); + scope = new RuleScope(in); + conditions = in.readList(RuleCondition::new); + } + + /** + * This is reading the old style rules (before 6.4) off the stream. + */ + public static DetectionRule readOldFormat(StreamInput in) throws IOException { + // Read actions if (in.getVersion().before(Version.V_6_2_0)) { - actions.add(RuleAction.readFromStream(in)); + // used to be a singe enum + in.readVInt(); } else { + // read an enum set int actionsCount = in.readVInt(); for (int i = 0; i < actionsCount; ++i) { - actions.add(RuleAction.readFromStream(in)); + in.readVInt(); } } - conditionsConnective = Connective.readFromStream(in); + // Read conditions connective enum + in.readVInt(); + + // Read conditions int size = in.readVInt(); - conditions = new ArrayList<>(size); for (int i = 0; i < size; i++) { - conditions.add(new RuleCondition(in)); + // read condition type enum + in.readVInt(); + + // read internal condition which is optional + if (in.readBoolean()) { + // operator + in.readVInt(); + // value + in.readOptionalString(); + } + + // field_name + in.readOptionalString(); + // field_value + in.readOptionalString(); + // filter_id + in.readOptionalString(); + } - targetFieldName = in.readOptionalString(); - targetFieldValue = in.readOptionalString(); + + // Read target_field_name + in.readOptionalString(); + // Read target_field_value + in.readOptionalString(); + + return null; } @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 +147,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 +156,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 +171,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 +213,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 +224,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..8a0213f291d5d 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 * ", \ @@ -239,7 +252,12 @@ public Detector(StreamInput in) throws IOException { partitionFieldName = in.readOptionalString(); useNull = in.readBoolean(); excludeFrequent = in.readBoolean() ? ExcludeFrequent.readFromStream(in) : null; - rules = in.readList(DetectionRule::new); + if (in.getVersion().onOrAfter(DetectionRule.VERSION_INTRODUCED)) { + rules = in.readList(DetectionRule::new); + } else { + in.readList(DetectionRule::readOldFormat); + rules = Collections.emptyList(); + } if (in.getVersion().onOrAfter(Version.V_5_5_0)) { detectorIndex = in.readInt(); } else { @@ -263,7 +281,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 +315,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 +489,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 +609,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 +696,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 53603f4dde423..5c4e313bace08 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 @@ -465,7 +465,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; @@ -519,7 +519,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..ee156847d063b 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 @@ -12,19 +12,11 @@ import java.io.IOException; import java.util.Locale; -import java.util.regex.Matcher; -import java.util.regex.Pattern; /** * 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 +40,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 +51,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 0078dd4a05822..780c1a02d841e 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)); } @@ -116,13 +114,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 1ddd19104cd30..4abb626b1d872 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 @@ -55,6 +55,7 @@ import org.elasticsearch.xpack.core.ml.action.PutJobAction; import org.elasticsearch.xpack.core.ml.action.UpdateJobAction; import org.elasticsearch.xpack.core.ml.job.config.AnalysisLimits; +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.JobTaskState; @@ -184,6 +185,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; @@ -357,8 +366,13 @@ private static boolean nodeSupportsJobVersion(Version nodeVersion) { return nodeVersion.onOrAfter(Version.V_5_5_0); } + private static boolean jobHasRules(Job job) { + return job.getAnalysisConfig().getDetectors().stream().anyMatch(d -> d.getRules().isEmpty() == false); + } + public static String[] mappingRequiresUpdate(ClusterState state, String[] concreteIndices, Version minVersion, Logger logger) throws IOException { + List indicesToUpdate = new ArrayList<>(); ImmutableOpenMap> currentMapping = state.metaData().findMappings(concreteIndices, @@ -651,9 +665,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 b4231012177af..85e8eb5759878 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 a52d880a4a3b9..20f89003e4ff7 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 @@ -27,6 +27,8 @@ import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.index.Index; import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.persistent.PersistentTasksCustomMetaData; +import org.elasticsearch.persistent.PersistentTasksCustomMetaData.Assignment; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.VersionUtils; @@ -34,21 +36,26 @@ 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.JobTaskState; +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; import org.elasticsearch.xpack.core.ml.notifications.AuditorField; -import org.elasticsearch.persistent.PersistentTasksCustomMetaData; -import org.elasticsearch.persistent.PersistentTasksCustomMetaData.Assignment; import org.elasticsearch.xpack.ml.MachineLearning; import org.elasticsearch.xpack.ml.support.BaseMlIntegTestCase; 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; @@ -395,6 +402,62 @@ public void testSelectLeastLoadedMlNode_noNodesPriorTo_V_5_5() { 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(); @@ -613,4 +676,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 886f8dcbef3bb..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,7 +20,7 @@ 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; @@ -34,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; @@ -47,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); @@ -90,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) { @@ -114,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()); @@ -148,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)); @@ -168,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); @@ -262,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 d44df6d85a965..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 @@ -41,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); @@ -153,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); @@ -195,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 1551b6cb94e40..2aff6cf55ed3c 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 @@ -111,9 +111,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 b59c0d3ad4c00..d587c1578ffef 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 @@ -138,11 +138,11 @@ - skip: version: "6.2.0 - " - reason: "Rules fields were renamed on 6.2.0" + reason: "Rules were renamed on 6.2.0 and 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 dummy job 6.2 onwards": - skip: version: " - 6.1.99" - reason: "Rules fields were renamed on 6.2.0" + reason: "This creates a job with the same name as the rules job to ensure that job is not missing in the upgraded cluster; also some of the rule fields were deprecated in 6.2 so we using the dummy from 6.2 onwards" - 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 8f52a1afb0943..bf6d3bf6bdef0 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 @@ -95,29 +95,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":