From 9f73f047ebed106cbc5085a7b97dd8292fd48cd8 Mon Sep 17 00:00:00 2001
From: David Kyle <david.kyle@elastic.co>
Date: Tue, 13 Dec 2016 09:24:16 +0000
Subject: [PATCH] Test for job existence before updating its state
 (elastic/elasticsearch#532)

* Test for job existence before updating its state

* Add unit tests covering expected missing job exceptions

Original commit: elastic/x-pack-elasticsearch@bcd270dafdff4a18c86d5f4a334bbe7d2d437f63
---
 .../xpack/prelert/action/CloseJobAction.java             | 2 --
 .../xpack/prelert/action/OpenJobAction.java              | 2 --
 .../xpack/prelert/job/metadata/PrelertMetadata.java      | 8 ++++++++
 .../xpack/prelert/job/metadata/PrelertMetadataTests.java | 9 +++++++++
 4 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/CloseJobAction.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/CloseJobAction.java
index beefdf797d3..3d085d5b86d 100644
--- a/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/CloseJobAction.java
+++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/CloseJobAction.java
@@ -172,8 +172,6 @@ public class CloseJobAction extends Action<CloseJobAction.Request, CloseJobActio
 
         @Override
         protected void masterOperation(Request request, ClusterState state, ActionListener<Response> listener) throws Exception {
-            jobManager.getJobOrThrowIfUnknown(request.getJobId());
-
             UpdateJobStatusAction.Request updateStatusRequest = new UpdateJobStatusAction.Request(request.getJobId(), JobStatus.CLOSING);
             ActionListener<UpdateJobStatusAction.Response> delegateListener = ActionListener.wrap(
                     response -> respondWhenJobIsClosed(request.getJobId(), listener), listener::onFailure);
diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/OpenJobAction.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/OpenJobAction.java
index 825008ca6cf..9dd162b14c1 100644
--- a/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/OpenJobAction.java
+++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/action/OpenJobAction.java
@@ -192,8 +192,6 @@ public class OpenJobAction extends Action<OpenJobAction.Request, OpenJobAction.R
 
         @Override
         protected void masterOperation(Request request, ClusterState state, ActionListener<Response> listener) throws Exception {
-            jobManager.getJobOrThrowIfUnknown(request.getJobId());
-
             ActionListener<Response> delegateListener = ActionListener.wrap(response -> respondWhenJobIsOpened(request, listener),
                     listener::onFailure);
             jobManager.openJob(request, delegateListener);
diff --git a/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/job/metadata/PrelertMetadata.java b/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/job/metadata/PrelertMetadata.java
index 12b7700fa44..661587b299c 100644
--- a/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/job/metadata/PrelertMetadata.java
+++ b/elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/job/metadata/PrelertMetadata.java
@@ -348,6 +348,10 @@ public class PrelertMetadata implements MetaData.Custom {
         }
 
         public Builder updateStatus(String jobId, JobStatus jobStatus, @Nullable String reason) {
+            if (jobs.containsKey(jobId) == false) {
+                throw ExceptionsHelper.missingJobException(jobId);
+            }
+
             Allocation previous = allocations.get(jobId);
             if (previous == null) {
                 throw new IllegalStateException("[" + jobId + "] no allocation exist to update the status to [" + jobStatus + "]");
@@ -367,6 +371,10 @@ public class PrelertMetadata implements MetaData.Custom {
         }
 
         public Builder setIgnoreDowntime(String jobId) {
+            if (jobs.containsKey(jobId) == false) {
+                throw ExceptionsHelper.missingJobException(jobId);
+            }
+
             Allocation allocation = allocations.get(jobId);
             if (allocation == null) {
                 throw new IllegalStateException("[" + jobId + "] no allocation to ignore downtime");
diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/prelert/job/metadata/PrelertMetadataTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/prelert/job/metadata/PrelertMetadataTests.java
index 63e74ccf6f4..0f9276f3cd3 100644
--- a/elasticsearch/src/test/java/org/elasticsearch/xpack/prelert/job/metadata/PrelertMetadataTests.java
+++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/prelert/job/metadata/PrelertMetadataTests.java
@@ -201,4 +201,13 @@ public class PrelertMetadataTests extends AbstractSerializingTestCase<PrelertMet
         assertThat(prelertMetadata.getJobs().get("_job_id").getFinishedTime(), notNullValue());
     }
 
+    public void testUpdateStatus_failBecauseJobDoesNotExist() {
+        PrelertMetadata.Builder builder = new PrelertMetadata.Builder();
+        expectThrows(ResourceNotFoundException.class, () -> builder.updateStatus("missing-job", JobStatus.CLOSED, "for testting"));
+    }
+
+    public void testSetIgnoreDowntime_failBecauseJobDoesNotExist() {
+        PrelertMetadata.Builder builder = new PrelertMetadata.Builder();
+        expectThrows(ResourceNotFoundException.class, () -> builder.setIgnoreDowntime("missing-job"));
+    }
 }