Convert RevertModel response from SingleDoc to simple, inlined model (elastic/elasticsearch#423)

Convert response from SingleDoc to simple, inlined model

Original commit: elastic/x-pack-elasticsearch@6231195b47

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

@@ -228,49 +228,53 @@ extends Action<RevertModelSnapshotAction.Request, RevertModelSnapshotAction.Resp
 
     public static class Response extends AcknowledgedResponse implements StatusToXContent {
 
-        private SingleDocument<ModelSnapshot> response;
+        private static final ParseField ACKNOWLEDGED = new ParseField("acknowledged");
+        private static final ParseField MODEL = new ParseField("model");
+        private ModelSnapshot model;
+
+        Response() {
 
-        public Response() {
-            super(false);
-            response = SingleDocument.empty(ModelSnapshot.TYPE.getPreferredName());
         }
 
         public Response(ModelSnapshot modelSnapshot) {
             super(true);
-            response = new SingleDocument<>(ModelSnapshot.TYPE.getPreferredName(), modelSnapshot);
+            model = modelSnapshot;
         }
 
-        public SingleDocument<ModelSnapshot> getResponse() {
+        public ModelSnapshot getModel() {
-            return response;
+            return model;
         }
 
         @Override
         public void readFrom(StreamInput in) throws IOException {
             super.readFrom(in);
             readAcknowledged(in);
-            response = new SingleDocument<>(in, ModelSnapshot::new);
+            model = new ModelSnapshot(in);
         }
 
         @Override
         public void writeTo(StreamOutput out) throws IOException {
             super.writeTo(out);
             writeAcknowledged(out);
-            response.writeTo(out);
+            model.writeTo(out);
         }
 
         @Override
         public RestStatus status() {
-            return response.status();
+            return RestStatus.OK;
         }
 
         @Override
         public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-            return response.toXContent(builder, params);
+            builder.field(ACKNOWLEDGED.getPreferredName(), true);
+            builder.field(MODEL.getPreferredName());
+            builder = model.toXContent(builder, params);
+            return builder;
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(response);
+            return Objects.hash(model);
         }
 
         @Override
@@ -282,7 +286,7 @@ extends Action<RevertModelSnapshotAction.Request, RevertModelSnapshotAction.Resp
                 return false;
             }
             Response other = (Response) obj;
-            return Objects.equals(response, other.response);
+            return Objects.equals(model, other.model);
         }
 
         @SuppressWarnings("deprecation")

elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/job/manager/JobManager.java

@@ -435,10 +435,8 @@ public class JobManager extends AbstractComponent {
 
             @Override
             protected RevertModelSnapshotAction.Response newResponse(boolean acknowledged) {
-                RevertModelSnapshotAction.Response response;
-
                 if (acknowledged) {
-                    response = new RevertModelSnapshotAction.Response(modelSnapshot);
+                    return new RevertModelSnapshotAction.Response(modelSnapshot);
 
                     // NORELEASE: This is not the place the audit log
                     // (indexes a document), because this method is
@@ -448,10 +446,9 @@ public class JobManager extends AbstractComponent {
                     // audit(jobId).info(Messages.getMessage(Messages.JOB_AUDIT_REVERTED,
                     // modelSnapshot.getDescription()));
 
-                } else {
-                    response = new RevertModelSnapshotAction.Response();
                 }
-                return response;
+                throw new IllegalStateException("Could not revert modelSnapshot on job ["
+                        + request.getJobId() + "], not acknowledged by master.");
             }
 
             @Override

elasticsearch/src/test/java/org/elasticsearch/xpack/prelert/action/RevertModelSnapshotActionResponseTests.java

@@ -13,13 +13,9 @@ public class RevertModelSnapshotActionResponseTests extends AbstractStreamableTe
 
     @Override
     protected Response createTestInstance() {
-        if (randomBoolean()) {
+        ModelSnapshot modelSnapshot = new ModelSnapshot(randomAsciiOfLengthBetween(1, 20));
-            return new Response();
+        modelSnapshot.setDescription(randomAsciiOfLengthBetween(1, 20));
-        } else {
+        return new RevertModelSnapshotAction.Response(modelSnapshot);
-            ModelSnapshot modelSnapshot = new ModelSnapshot(randomAsciiOfLengthBetween(1, 20));
-            modelSnapshot.setDescription(randomAsciiOfLengthBetween(1, 20));
-            return new RevertModelSnapshotAction.Response(modelSnapshot);
-        }
     }
 
     @Override

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

@@ -110,26 +110,24 @@ setup:
         job_id: "foo"
         snapshotId: "foo1"
 
-  - match: { exists: true }
+  - match: { acknowledged: true }
-  - match: { type: "modelSnapshot" }
+  - match: { model.jobId: "foo" }
-  - match: { document.jobId: "foo" }
+  - match: { model.timestamp: 1464825600000 }
-  - match: { document.timestamp: 1464825600000 }
+  - match: { model.restorePriority: 1 }
-  - match: { document.restorePriority: 1 }
+  - match: { model.snapshotId: "foo1" }
-  - match: { document.snapshotId: "foo1" }
+  - match: { model.snapshotDocCount: 0 }
-  - match: { document.snapshotDocCount: 0 }
 
   - do:
       xpack.prelert.revert_model_snapshot:
         job_id: "foo"
         snapshotId: "foo2"
 
-  - match: { exists: true }
+  - match: { acknowledged: true }
-  - match: { type: "modelSnapshot" }
+  - match: { model.jobId: "foo" }
-  - match: { document.jobId: "foo" }
+  - match: { model.timestamp: 1464739200000 }
-  - match: { document.timestamp: 1464739200000 }
+  - match: { model.restorePriority: 2 }
-  - match: { document.restorePriority: 2 }
+  - match: { model.snapshotId: "foo2" }
-  - match: { document.snapshotId: "foo2" }
+  - match: { model.snapshotDocCount: 0 }
-  - match: { document.snapshotDocCount: 0 }
 
 ---
 "Test revert model with valid time":
@@ -138,26 +136,24 @@ setup:
         job_id: "foo"
         time: "2016-06-02T01:00:00Z"
 
-  - match: { exists: true }
+  - match: { acknowledged: true }
-  - match: { type: "modelSnapshot" }
+  - match: { model.jobId: "foo" }
-  - match: { document.jobId: "foo" }
+  - match: { model.timestamp: 1464825600000 }
-  - match: { document.timestamp: 1464825600000 }
+  - match: { model.restorePriority: 1 }
-  - match: { document.restorePriority: 1 }
+  - match: { model.snapshotId: "foo1" }
-  - match: { document.snapshotId: "foo1" }
+  - match: { model.snapshotDocCount: 0 }
-  - match: { document.snapshotDocCount: 0 }
 
   - do:
         xpack.prelert.revert_model_snapshot:
           job_id: "foo"
           time: "2016-06-01T01:00:00Z"
 
-  - match: { exists: true }
+  - match: { acknowledged: true }
-  - match: { type: "modelSnapshot" }
+  - match: { model.jobId: "foo" }
-  - match: { document.jobId: "foo" }
+  - match: { model.timestamp: 1464739200000 }
-  - match: { document.timestamp: 1464739200000 }
+  - match: { model.restorePriority: 2 }
-  - match: { document.restorePriority: 2 }
+  - match: { model.snapshotId: "foo2" }
-  - match: { document.snapshotId: "foo2" }
+  - match: { model.snapshotDocCount: 0 }
-  - match: { document.snapshotDocCount: 0 }
 
 ---
 "Test revert model with valid description":
@@ -166,26 +162,24 @@ setup:
         job_id: "foo"
         description: "first"
 
-  - match: { exists: true }
+  - match: { acknowledged: true }
-  - match: { type: "modelSnapshot" }
+  - match: { model.jobId: "foo" }
-  - match: { document.jobId: "foo" }
+  - match: { model.timestamp: 1464825600000 }
-  - match: { document.timestamp: 1464825600000 }
+  - match: { model.restorePriority: 1 }
-  - match: { document.restorePriority: 1 }
+  - match: { model.snapshotId: "foo1" }
-  - match: { document.snapshotId: "foo1" }
+  - match: { model.snapshotDocCount: 0 }
-  - match: { document.snapshotDocCount: 0 }
 
   - do:
         xpack.prelert.revert_model_snapshot:
           job_id: "foo"
           description: "second"
 
-  - match: { exists: true }
+  - match: { acknowledged: true }
-  - match: { type: "modelSnapshot" }
+  - match: { model.jobId: "foo" }
-  - match: { document.jobId: "foo" }
+  - match: { model.timestamp: 1464739200000 }
-  - match: { document.timestamp: 1464739200000 }
+  - match: { model.restorePriority: 2 }
-  - match: { document.restorePriority: 2 }
+  - match: { model.snapshotId: "foo2" }
-  - match: { document.snapshotId: "foo2" }
+  - match: { model.snapshotDocCount: 0 }
-  - match: { document.snapshotDocCount: 0 }
 
 ---
 "Test revert model with deleteInterveningResults":