[ML] Add sort parameter for get buckets (elastic/x-pack-elasticsearch#1464)

* Add sort parameter for get buckets

* Add secondary sort by time

* Use default values from actions in rest requests

Original commit: elastic/x-pack-elasticsearch@a530c0bed6

docs/en/rest-api/ml/get-bucket.asciidoc

@@ -33,6 +33,9 @@ This API presents a chronological view of the records, grouped by bucket.
 `anomaly_score`::
   (double) Returns buckets with anomaly scores higher than this value.
 
+`desc`::
+  (boolean) If true, the buckets are sorted in descending order.
+
 `end`::
   (string) Returns buckets with timestamps earlier than this time.
 
@@ -49,6 +52,10 @@ This API presents a chronological view of the records, grouped by bucket.
 `size`:::
   (integer) Specifies the maximum number of buckets to obtain.
 
+`sort`::
+  (string) Specifies the sort field for the requested buckets.
+  By default, the buckets are sorted by the `timestamp` field.
+
 `start`::
   (string) Returns buckets with timestamps after this time.
 

docs/en/rest-api/ml/get-influencer.asciidoc

@@ -20,7 +20,6 @@ influencers.
 
 `desc`::
   (boolean) If true, the results are sorted in descending order.
-//TBD: Using the "sort" value?
 
 `end`::
   (string) Returns influencers with timestamps earlier than this time.
@@ -40,7 +39,7 @@ influencers.
 
 `sort`::
   (string) Specifies the sort field for the requested influencers.
-//TBD: By default the results are sorted on the influencer score?
+  By default the influencers are sorted by the `influencer_score` value.
 
 `start`::
   (string) Returns influencers with timestamps after this time.

plugin/src/main/java/org/elasticsearch/xpack/ml/action/GetBucketsAction.java

@@ -5,6 +5,7 @@
  */
 package org.elasticsearch.xpack.ml.action;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.Action;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.ActionRequest;
@@ -69,6 +70,8 @@ public class GetBucketsAction extends Action<GetBucketsAction.Request, GetBucket
         public static final ParseField END = new ParseField("end");
         public static final ParseField ANOMALY_SCORE = new ParseField("anomaly_score");
         public static final ParseField TIMESTAMP = new ParseField("timestamp");
+        public static final ParseField SORT = new ParseField("sort");
+        public static final ParseField DESCENDING = new ParseField("desc");
 
         private static final ObjectParser<Request, Void> PARSER = new ObjectParser<>(NAME, Request::new);
 
@@ -81,6 +84,8 @@ public class GetBucketsAction extends Action<GetBucketsAction.Request, GetBucket
             PARSER.declareStringOrNull(Request::setEnd, END);
             PARSER.declareObject(Request::setPageParams, PageParams.PARSER, PageParams.PAGE);
             PARSER.declareDouble(Request::setAnomalyScore, ANOMALY_SCORE);
+            PARSER.declareString(Request::setSort, SORT);
+            PARSER.declareBoolean(Request::setDescending, DESCENDING);
         }
 
         public static Request parseRequest(String jobId, XContentParser parser) {
@@ -99,6 +104,8 @@ public class GetBucketsAction extends Action<GetBucketsAction.Request, GetBucket
         private String end;
         private PageParams pageParams;
         private Double anomalyScore;
+        private String sort = Result.TIMESTAMP.getPreferredName();
+        private boolean descending = false;
 
         Request() {
         }
@@ -179,7 +186,7 @@ public class GetBucketsAction extends Action<GetBucketsAction.Request, GetBucket
             this.pageParams = ExceptionsHelper.requireNonNull(pageParams, PageParams.PAGE.getPreferredName());
         }
 
-        public double getAnomalyScore() {
+        public Double getAnomalyScore() {
             return anomalyScore;
         }
 
@@ -191,6 +198,22 @@ public class GetBucketsAction extends Action<GetBucketsAction.Request, GetBucket
             this.anomalyScore = anomalyScore;
         }
 
+        public String getSort() {
+            return sort;
+        }
+
+        public void setSort(String sort) {
+            this.sort = sort;
+        }
+
+        public boolean isDescending() {
+            return descending;
+        }
+
+        public void setDescending(boolean descending) {
+            this.descending = descending;
+        }
+
         @Override
         public ActionRequestValidationException validate() {
             return null;
@@ -207,6 +230,10 @@ public class GetBucketsAction extends Action<GetBucketsAction.Request, GetBucket
             end = in.readOptionalString();
             anomalyScore = in.readOptionalDouble();
             pageParams = in.readOptionalWriteable(PageParams::new);
+            if (in.getVersion().after(Version.V_5_4_0)) {
+                sort = in.readString();
+                descending = in.readBoolean();
+            }
         }
 
         @Override
@@ -220,6 +247,10 @@ public class GetBucketsAction extends Action<GetBucketsAction.Request, GetBucket
             out.writeOptionalString(end);
             out.writeOptionalDouble(anomalyScore);
             out.writeOptionalWriteable(pageParams);
+            if (out.getVersion().after(Version.V_5_4_0)) {
+                out.writeString(sort);
+                out.writeBoolean(descending);
+            }
         }
 
         @Override
@@ -243,13 +274,15 @@ public class GetBucketsAction extends Action<GetBucketsAction.Request, GetBucket
             if (anomalyScore != null) {
                 builder.field(ANOMALY_SCORE.getPreferredName(), anomalyScore);
             }
+            builder.field(SORT.getPreferredName(), sort);
+            builder.field(DESCENDING.getPreferredName(), descending);
             builder.endObject();
             return builder;
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(jobId, timestamp, expand, excludeInterim, anomalyScore, pageParams, start, end);
+            return Objects.hash(jobId, timestamp, expand, excludeInterim, anomalyScore, pageParams, start, end, sort, descending);
         }
 
         @Override
@@ -268,7 +301,9 @@ public class GetBucketsAction extends Action<GetBucketsAction.Request, GetBucket
                     Objects.equals(anomalyScore, other.anomalyScore) &&
                     Objects.equals(pageParams, other.pageParams) &&
                     Objects.equals(start, other.start) &&
-                    Objects.equals(end, other.end);
+                    Objects.equals(end, other.end) &&
+                    Objects.equals(sort, other.sort) &&
+                    Objects.equals(descending, other.descending);
         }
     }
 
@@ -362,7 +397,9 @@ public class GetBucketsAction extends Action<GetBucketsAction.Request, GetBucket
                             .includeInterim(request.excludeInterim == false)
                             .start(request.start)
                             .end(request.end)
-                            .anomalyScoreThreshold(request.anomalyScore);
+                            .anomalyScoreThreshold(request.anomalyScore)
+                            .sortField(request.sort)
+                            .sortDescending(request.descending);
 
             if (request.pageParams != null) {
                 query.from(request.pageParams.getFrom())

plugin/src/main/java/org/elasticsearch/xpack/ml/action/GetRecordsAction.java

@@ -98,7 +98,7 @@ public class GetRecordsAction extends Action<GetRecordsAction.Request, GetRecord
         private PageParams pageParams = new PageParams();
         private double recordScoreFilter = 0.0;
         private String sort = Influencer.INFLUENCER_SCORE.getPreferredName();
-        private boolean descending = false;
+        private boolean descending = true;
 
         Request() {
         }

plugin/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobProvider.java

@@ -384,8 +384,13 @@ public class JobProvider {
         searchSourceBuilder.query(boolQuery);
         searchSourceBuilder.from(query.getFrom());
         searchSourceBuilder.size(query.getSize());
+        // If not using the default sort field (timestamp) add it as a secondary sort
+        if (Result.TIMESTAMP.getPreferredName().equals(query.getSortField()) == false) {
+            searchSourceBuilder.sort(Result.TIMESTAMP.getPreferredName(), query.isSortDescending() ? SortOrder.DESC : SortOrder.ASC);
+        }
         searchRequest.source(searchSourceBuilder);
 
+
         MultiSearchRequest mrequest = new MultiSearchRequest();
         mrequest.indicesOptions(addIgnoreUnavailable(mrequest.indicesOptions()));
         mrequest.add(searchRequest);

plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetBucketsAction.java

@@ -71,6 +71,11 @@ public class RestGetBucketsAction extends BaseRestHandler {
                 request.setAnomalyScore(
                         Double.parseDouble(restRequest.param(GetBucketsAction.Request.ANOMALY_SCORE.getPreferredName(), "0.0")));
             }
+            if (restRequest.hasParam(GetBucketsAction.Request.SORT.getPreferredName())) {
+                request.setSort(restRequest.param(GetBucketsAction.Request.SORT.getPreferredName()));
+            }
+            request.setDescending(restRequest.paramAsBoolean(GetBucketsAction.Request.DESCENDING.getPreferredName(),
+                    request.isDescending()));
 
             // single and multiple bucket options
             request.setExpand(restRequest.paramAsBoolean(GetBucketsAction.Request.EXPAND.getPreferredName(), false));

plugin/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestGetRecordsAction.java

@@ -42,14 +42,16 @@ public class RestGetRecordsAction extends BaseRestHandler {
             request = new GetRecordsAction.Request(jobId);
             request.setStart(restRequest.param(GetRecordsAction.Request.START.getPreferredName()));
             request.setEnd(restRequest.param(GetRecordsAction.Request.END.getPreferredName()));
-            request.setExcludeInterim(restRequest.paramAsBoolean(GetRecordsAction.Request.EXCLUDE_INTERIM.getPreferredName(), false));
+            request.setExcludeInterim(restRequest.paramAsBoolean(GetRecordsAction.Request.EXCLUDE_INTERIM.getPreferredName(),
+                    request.isExcludeInterim()));
             request.setPageParams(new PageParams(restRequest.paramAsInt(PageParams.FROM.getPreferredName(), PageParams.DEFAULT_FROM),
                     restRequest.paramAsInt(PageParams.SIZE.getPreferredName(), PageParams.DEFAULT_SIZE)));
             request.setRecordScore(
                     Double.parseDouble(restRequest.param(GetRecordsAction.Request.RECORD_SCORE_FILTER.getPreferredName(), "0.0")));
             request.setSort(restRequest.param(GetRecordsAction.Request.SORT.getPreferredName(),
                     AnomalyRecord.RECORD_SCORE.getPreferredName()));
-            request.setDescending(restRequest.paramAsBoolean(GetRecordsAction.Request.DESCENDING.getPreferredName(), true));
+            request.setDescending(restRequest.paramAsBoolean(GetRecordsAction.Request.DESCENDING.getPreferredName(),
+                    request.isDescending()));
         }
 
         return channel -> client.execute(GetRecordsAction.INSTANCE, request, new RestToXContentListener<>(channel));

plugin/src/test/java/org/elasticsearch/xpack/ml/action/GetBucketActionRequestTests.java

@@ -37,6 +37,10 @@ public class GetBucketActionRequestTests extends AbstractStreamableXContentTestC
                 int size = randomInt(maxSize);
                 request.setPageParams(new PageParams(from, size));
             }
+            if (randomBoolean()) {
+                request.setSort("anomaly_score");
+            }
+            request.setDescending(randomBoolean());
         }
         if (randomBoolean()) {
             request.setExpand(randomBoolean());

plugin/src/test/resources/rest-api-spec/api/xpack.ml.get_buckets.json

@@ -46,6 +46,14 @@
         "anomaly_score": {
           "type": "double",
           "description": "Filter for the most anomalous buckets"
+        },
+        "sort": {
+          "type": "string",
+          "description": "Sort buckets by a particular field"
+        },
+        "desc": {
+          "type": "boolean",
+          "description": "Set the sort direction"
         }
       }
     },

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

@@ -23,6 +23,7 @@ setup:
             "job_id": "jobs-get-result-buckets",
             "result_type": "bucket",
             "timestamp": "2016-06-01T00:00:00Z",
+            "anomaly_score": 90.0,
             "bucket_span":1
           }
 
@@ -30,15 +31,29 @@ setup:
       index:
         index:  .ml-anomalies-jobs-get-result-buckets
         type:   result
-        id:     "jobs-get-result-buckets_1464739200000_2"
+        id:     "jobs-get-result-buckets_1470009600000_2"
         body:
           {
             "job_id": "jobs-get-result-buckets",
             "result_type": "bucket",
             "timestamp": "2016-08-01T00:00:00Z",
+            "anomaly_score": 60.0,
             "bucket_span":1,
             "is_interim": true
           }
+  - do:
+      index:
+        index:  .ml-anomalies-jobs-get-result-buckets
+        type:   result
+        id:     "jobs-get-result-buckets_1470096000000_3"
+        body:
+          {
+            "job_id": "jobs-get-result-buckets",
+            "result_type": "bucket",
+            "timestamp": "2016-08-02T00:00:00Z",
+            "anomaly_score": 60.0,
+            "bucket_span":1,
+          }
 
   - do:
       indices.refresh:
@@ -63,15 +78,19 @@ setup:
       xpack.ml.get_buckets:
         job_id: "jobs-get-result-buckets"
 
-  - match: { count: 2 }
+  - match: { count: 3 }
   - match: { buckets.0.timestamp: 1464739200000 }
   - match: { buckets.0.job_id: jobs-get-result-buckets}
   - match: { buckets.0.result_type: bucket}
   - match: { buckets.1.timestamp: 1470009600000 }
   - match: { buckets.1.job_id: jobs-get-result-buckets}
   - match: { buckets.1.result_type: bucket}
+  - match: { buckets.2.timestamp: 1470096000000 }
+  - match: { buckets.2.job_id: jobs-get-result-buckets}
+  - match: { buckets.2.result_type: bucket}
   - is_false: buckets.0.partition_scores
   - is_false: buckets.1.partition_scores
+  - is_false: buckets.2.partition_scores
 
 ---
 "Test get buckets given exclude_interim is false":
@@ -80,7 +99,7 @@ setup:
         job_id: "jobs-get-result-buckets"
         exclude_interim: false
 
-  - match: { count: 2 }
+  - match: { count: 3 }
 
 ---
 "Test get buckets given exclude_interim is true":
@@ -89,9 +108,11 @@ setup:
         job_id: "jobs-get-result-buckets"
         exclude_interim: true
 
-  - match: { count: 1 }
+  - match: { count: 2 }
   - match: { buckets.0.timestamp: 1464739200000 }
   - match: { buckets.0.is_interim: false }
+  - match: { buckets.1.timestamp: 1470096000000 }
+  - match: { buckets.1.is_interim: false }
 
 ---
 "Test result single bucket api":
@@ -189,3 +210,33 @@ setup:
       catch: missing
       xpack.ml.get_buckets:
         job_id: "non-existent-job"
+
+---
+"Test get buckets with sort field and secondary sort by time":
+  - do:
+      xpack.ml.get_buckets:
+        job_id: "jobs-get-result-buckets"
+        sort: anomaly_score
+
+  - match: { count: 3 }
+  - match: { buckets.0.anomaly_score: 60.0 }
+  - match: { buckets.0.timestamp: 1470009600000 }
+  - match: { buckets.1.anomaly_score: 60.0 }
+  - match: { buckets.1.timestamp: 1470096000000 }
+  - match: { buckets.2.anomaly_score: 90.0}
+  - match: { buckets.2.timestamp: 1464739200000 }
+
+  - do:
+      xpack.ml.get_buckets:
+        job_id: "jobs-get-result-buckets"
+        sort: anomaly_score
+        desc: true
+
+  - match: { count: 3 }
+  - match: { buckets.0.anomaly_score: 90.0 }
+  - match: { buckets.0.timestamp: 1464739200000 }
+  - match: { buckets.1.anomaly_score: 60.0}
+  - match: { buckets.1.timestamp: 1470096000000 }
+  - match: { buckets.2.anomaly_score: 60.0}
+  - match: { buckets.2.timestamp: 1470009600000 }
+