[ML] Ensure BWC between old and new rules syntax (elastic/x-pack-elasticsearch#3738)

relates elastic/x-pack-elasticsearch#3325

Original commit: elastic/x-pack-elasticsearch@74cccbccf0

plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/DetectionRule.java

@@ -5,6 +5,7 @@
  */
 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;
@@ -34,11 +35,11 @@ import java.util.stream.Collectors;
 public class DetectionRule implements ToXContentObject, Writeable {
 
     public static final ParseField DETECTION_RULE_FIELD = new ParseField("detection_rule");
-    public static final ParseField ACTIONS_FIELD = new ParseField("actions");
+    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");
+    public static final ParseField CONDITIONS_FIELD = new ParseField("conditions", "rule_conditions");
 
     // These parsers follow the pattern that metadata is parsed leniently (to allow for enhancements), whilst config is parsed strictly
     public static final ObjectParser<Builder, Void> METADATA_PARSER =
@@ -83,10 +84,14 @@ public class DetectionRule implements ToXContentObject, Writeable {
     }
 
     public DetectionRule(StreamInput in) throws IOException {
-        int actionsCount = in.readVInt();
         actions = EnumSet.noneOf(RuleAction.class);
-        for (int i = 0; i < actionsCount; ++i) {
+        if (in.getVersion().before(Version.V_6_2_0)) {
             actions.add(RuleAction.readFromStream(in));
+        } else {
+            int actionsCount = in.readVInt();
+            for (int i = 0; i < actionsCount; ++i) {
+                actions.add(RuleAction.readFromStream(in));
+            }
         }
 
         conditionsConnective = Connective.readFromStream(in);
@@ -101,9 +106,14 @@ public class DetectionRule implements ToXContentObject, Writeable {
 
     @Override
     public void writeTo(StreamOutput out) throws IOException {
-        out.writeVInt(actions.size());
-        for (RuleAction action : actions) {
-            action.writeTo(out);
+        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);

plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/config/RuleCondition.java

@@ -26,10 +26,11 @@ import java.util.Map;
 import java.util.Objects;
 
 public class RuleCondition implements ToXContentObject, Writeable {
-    public static final ParseField TYPE_FIELD = new ParseField("type");
+    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");
 
     // These parsers follow the pattern that metadata is parsed leniently (to allow for enhancements), whilst config is parsed strictly
     public static final ConstructingObjectParser<RuleCondition, Void> METADATA_PARSER =
@@ -56,7 +57,7 @@ public class RuleCondition implements ToXContentObject, Writeable {
             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(), MlFilter.ID);
+            parser.declareStringOrNull(ConstructingObjectParser.optionalConstructorArg(), FILTER_ID_FIELD);
         }
     }
 
@@ -116,7 +117,7 @@ public class RuleCondition implements ToXContentObject, Writeable {
             builder.field(FIELD_VALUE_FIELD.getPreferredName(), fieldValue);
         }
         if (filterId != null) {
-            builder.field(MlFilter.ID.getPreferredName(), filterId);
+            builder.field(FILTER_ID_FIELD.getPreferredName(), filterId);
         }
         builder.endObject();
         return builder;
@@ -214,7 +215,7 @@ public class RuleCondition implements ToXContentObject, Writeable {
     private static void verifyCategorical(RuleCondition ruleCondition) throws ElasticsearchParseException {
         checkCategoricalHasNoField(Condition.CONDITION_FIELD.getPreferredName(), ruleCondition.getCondition());
         checkCategoricalHasNoField(RuleCondition.FIELD_VALUE_FIELD.getPreferredName(), ruleCondition.getFieldValue());
-        checkCategoricalHasField(MlFilter.ID.getPreferredName(), ruleCondition.getFilterId());
+        checkCategoricalHasField(FILTER_ID_FIELD.getPreferredName(), ruleCondition.getFilterId());
     }
 
     private static void checkCategoricalHasNoField(String fieldName, Object fieldValue) throws ElasticsearchParseException {
@@ -232,7 +233,7 @@ public class RuleCondition implements ToXContentObject, Writeable {
     }
 
     private static void verifyNumerical(RuleCondition ruleCondition) throws ElasticsearchParseException {
-        checkNumericalHasNoField(MlFilter.ID.getPreferredName(), ruleCondition.getFilterId());
+        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);

plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml

@@ -1135,3 +1135,122 @@
             }
           }
 
+---
+"Test job with rules":
+
+  - do:
+      xpack.ml.put_job:
+        job_id: jobs-crud-rules
+        body:  >
+          {
+            "analysis_config": {
+              "detectors": [
+                {
+                  "function": "count",
+                  "by_field_name": "country",
+                  "rules": [
+                    {
+                      "actions": ["filter_results", "skip_sampling"],
+                      "conditions": [
+                        {
+                          "type":"numerical_actual",
+                          "field_name":"country",
+                          "field_value": "uk",
+                          "condition": {"operator":"lt","value":"33.3"}
+                        },
+                        {"type":"categorical", "field_name":"country", "filter_id": "foo"}
+                      ]
+                    }
+                  ]
+                }
+              ]
+            },
+            "data_description" : {}
+          }
+
+  - do:
+      xpack.ml.get_jobs:
+        job_id: jobs-crud-rules
+  - match: { count: 1 }
+  - match: {
+      jobs.0.analysis_config.detectors.0.rules: [
+        {
+          "actions": ["filter_results", "skip_sampling"],
+          "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"}
+          ]
+        }
+      ]
+    }
+
+---
+"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"}
+                      ]
+                    }
+                  ]
+                }
+              ]
+            },
+            "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"}
+          ]
+        }
+      ]
+    }