Add mutually-exclusive param validation to GetCategories (elastic/elasticsearch#512)

Also adds body parsing to the REST action, which seemed to be missing (but supported because of the POST)

Closes elastic/elasticsearch#430

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

elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/GetCategoriesDefinitionAction.java

@@ -37,6 +37,8 @@ import org.elasticsearch.xpack.prelert.utils.ExceptionsHelper;
 import java.io.IOException;
 import java.util.Objects;
 
+import static org.elasticsearch.action.ValidateActions.addValidationError;
+
 public class GetCategoriesDefinitionAction extends
 Action<GetCategoriesDefinitionAction.Request, GetCategoriesDefinitionAction.Response, GetCategoriesDefinitionAction.RequestBuilder> {
 
@@ -81,7 +83,7 @@ Action<GetCategoriesDefinitionAction.Request, GetCategoriesDefinitionAction.Resp
 
         private String jobId;
         private String categoryId;
-        private PageParams pageParams = new PageParams();
+        private PageParams pageParams;
 
         public Request(String jobId) {
             this.jobId = ExceptionsHelper.requireNonNull(jobId, Job.ID.getPreferredName());
@@ -95,6 +97,10 @@ Action<GetCategoriesDefinitionAction.Request, GetCategoriesDefinitionAction.Resp
         }
 
         public void setCategoryId(String categoryId) {
+            if (pageParams != null) {
+                throw new IllegalArgumentException("Param [" + CATEGORY_ID.getPreferredName() + "] is incompatible with ["
+                        + PageParams.FROM.getPreferredName() + ", " + PageParams.SIZE.getPreferredName() + "].");
+            }
             this.categoryId = ExceptionsHelper.requireNonNull(categoryId, CATEGORY_ID.getPreferredName());
         }
 
@@ -103,12 +109,22 @@ Action<GetCategoriesDefinitionAction.Request, GetCategoriesDefinitionAction.Resp
         }
 
         public void setPageParams(PageParams pageParams) {
+            if (categoryId != null) {
+                throw new IllegalArgumentException("Param [" + PageParams.FROM.getPreferredName() + ", "
+                        + PageParams.SIZE.getPreferredName() + "] is incompatible with [" + CATEGORY_ID.getPreferredName() + "].");
+            }
             this.pageParams = pageParams;
         }
 
         @Override
         public ActionRequestValidationException validate() {
-            return null;
+            ActionRequestValidationException validationException = null;
+            if (pageParams == null && categoryId == null) {
+                validationException = addValidationError("Both [" + CATEGORY_ID.getPreferredName() + "] and ["
+                        + PageParams.FROM.getPreferredName() + ", " + PageParams.SIZE.getPreferredName() + "] "
+                        + "cannot be null" , validationException);
+            }
+            return validationException;
         }
 
         @Override
@@ -134,7 +150,9 @@ Action<GetCategoriesDefinitionAction.Request, GetCategoriesDefinitionAction.Resp
             if (categoryId != null) {
                 builder.field(CATEGORY_ID.getPreferredName(), categoryId);
             }
-            builder.field(PageParams.PAGE.getPreferredName(), pageParams);
+            if (pageParams != null) {
+                builder.field(PageParams.PAGE.getPreferredName(), pageParams);
+            }
             builder.endObject();
             return builder;
         }
@@ -229,9 +247,10 @@ Action<GetCategoriesDefinitionAction.Request, GetCategoriesDefinitionAction.Resp
             QueryPage<CategoryDefinition> result;
             if (request.categoryId != null ) {
                 result = jobProvider.categoryDefinition(request.jobId, request.categoryId);
+            } else if (request.pageParams != null) {
+                result = jobProvider.categoryDefinitions(request.jobId, request.pageParams.getFrom(), request.pageParams.getSize());
             } else {
-                result = jobProvider.categoryDefinitions(request.jobId, request.pageParams.getFrom(),
+                throw new IllegalStateException("Both categoryId and pageParams are null");
-                        request.pageParams.getSize());
             }
             listener.onResponse(new Response(result));
         }

elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/rest/results/RestGetCategoriesAction.java

@@ -6,8 +6,12 @@
 package org.elasticsearch.xpack.prelert.rest.results;
 
 import org.elasticsearch.client.node.NodeClient;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
@@ -44,17 +48,30 @@ public class RestGetCategoriesAction extends BaseRestHandler {
 
     @Override
     protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
-        Request request = new Request(restRequest.param(Job.ID.getPreferredName()));
+        Request request;
-
+        String jobId = restRequest.param(Job.ID.getPreferredName());
         String categoryId = restRequest.param(Request.CATEGORY_ID.getPreferredName());
-        if (categoryId != null && !categoryId.isEmpty()) {
+        BytesReference bodyBytes = restRequest.content();
+
+        if (bodyBytes != null && bodyBytes.length() > 0) {
+            XContentParser parser = XContentFactory.xContent(bodyBytes).createParser(bodyBytes);
+            request = GetCategoriesDefinitionAction.Request.parseRequest(jobId, parser, () -> parseFieldMatcher);
             request.setCategoryId(categoryId);
         } else {
-            PageParams pageParams = new PageParams(
+
-                    restRequest.paramAsInt(Request.FROM.getPreferredName(), 0),
+            request = new Request(jobId);
-                    restRequest.paramAsInt(Request.SIZE.getPreferredName(), 100)
+            if (!Strings.isNullOrEmpty(categoryId)) {
-            );
+                request.setCategoryId(categoryId);
-            request.setPageParams(pageParams);
+            }
+            if (restRequest.hasParam(Request.FROM.getPreferredName())
+                    || restRequest.hasParam(Request.SIZE.getPreferredName())
+                    || Strings.isNullOrEmpty(categoryId)){
+
+                request.setPageParams(new PageParams(
+                        restRequest.paramAsInt(Request.FROM.getPreferredName(), 0),
+                        restRequest.paramAsInt(Request.SIZE.getPreferredName(), 100)
+                ));
+            }
         }
 
         return channel -> transportAction.execute(request, new RestToXContentListener<>(channel));

elasticsearch/src/test/resources/rest-api-spec/test/jobs_get_result_categories.yaml

@@ -43,3 +43,54 @@ setup:
 
   - match: { categories.0.job_id: farequote }
   - match: { categories.0.category_id: 1 }
+
+---
+"Test with invalid param combinations":
+  - do:
+      catch: request
+      xpack.prelert.get_categories:
+        job_id: "farequote"
+        category_id: "1"
+        from: 0
+
+  - do:
+      catch: request
+      xpack.prelert.get_categories:
+        job_id: "farequote"
+        category_id: "1"
+        size: 1
+
+  - do:
+      catch: request
+      xpack.prelert.get_categories:
+        job_id: "farequote"
+        category_id: "1"
+        from: 0
+        size: 1
+
+---
+"Test with invalid param combinations (via body)":
+  - do:
+      catch: request
+      xpack.prelert.get_categories:
+        job_id: "farequote"
+        category_id: "1"
+        body:
+          from: 0
+
+  - do:
+      catch: request
+      xpack.prelert.get_categories:
+        job_id: "farequote"
+        category_id: "1"
+        body:
+          size: 1
+
+  - do:
+      catch: request
+      xpack.prelert.get_categories:
+        job_id: "farequote"
+        category_id: "1"
+        body:
+          from: 0
+          size: 1