From d64bea14dc13c7bbebab13023c83652788ffc756 Mon Sep 17 00:00:00 2001
From: David Kyle <david.kyle@elastic.co>
Date: Fri, 9 Jun 2017 10:02:51 +0100
Subject: [PATCH] [ML] Closing an unknown job should throw resource not found
 exception (elastic/x-pack-elasticsearch#1673)

Original commit: elastic/x-pack-elasticsearch@c244d2809b510338ad781a731c9d2124e2767710
---
 .../xpack/ml/action/CloseJobAction.java           | 10 +++++-----
 .../ml/action/CloseJobActionRequestTests.java     | 15 +++++++++++++++
 .../resources/rest-api-spec/test/ml/jobs_crud.yml | 12 ++++++++++++
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/CloseJobAction.java b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/CloseJobAction.java
index 018363cc8eb..eaf33628a5c 100644
--- a/plugin/src/main/java/org/elasticsearch/xpack/ml/action/CloseJobAction.java
+++ b/plugin/src/main/java/org/elasticsearch/xpack/ml/action/CloseJobAction.java
@@ -563,12 +563,9 @@ public class CloseJobAction extends Action<CloseJobAction.Request, CloseJobActio
      */
     static void resolveAndValidateJobId(String jobId, ClusterState state, List<String> openJobIds, List<String> closingJobIds,
             boolean allowFailed) {
-        MlMetadata mlMetadata = state.metaData().custom(MlMetadata.TYPE);
         PersistentTasksCustomMetaData tasksMetaData = state.getMetaData().custom(PersistentTasksCustomMetaData.TYPE);
-
-        if (mlMetadata.getJobs().isEmpty()) {
-            return;
-        }
+        MlMetadata maybeNull = state.metaData().custom(MlMetadata.TYPE);
+        final MlMetadata mlMetadata = (maybeNull == null) ? MlMetadata.EMPTY_METADATA : maybeNull;
 
         List<String> failedJobs = new ArrayList<>();
 
@@ -582,6 +579,9 @@ public class CloseJobAction extends Action<CloseJobAction.Request, CloseJobActio
         };
 
         if (!Job.ALL.equals(jobId)) {
+            if (mlMetadata.getJobs().containsKey(jobId) == false) {
+                throw ExceptionsHelper.missingJobException(jobId);
+            }
             jobIdProcessor.accept(jobId);
 
             if (allowFailed == false && failedJobs.size() > 0) {
diff --git a/plugin/src/test/java/org/elasticsearch/xpack/ml/action/CloseJobActionRequestTests.java b/plugin/src/test/java/org/elasticsearch/xpack/ml/action/CloseJobActionRequestTests.java
index 26f752ef958..2d6360a9905 100644
--- a/plugin/src/test/java/org/elasticsearch/xpack/ml/action/CloseJobActionRequestTests.java
+++ b/plugin/src/test/java/org/elasticsearch/xpack/ml/action/CloseJobActionRequestTests.java
@@ -177,6 +177,21 @@ public class CloseJobActionRequestTests extends AbstractStreamableXContentTestCa
         assertEquals(Collections.emptyList(), closingJobs);
     }
 
+    public void testResolve_throwsWithUnknownJobId() {
+        MlMetadata.Builder mlBuilder = new MlMetadata.Builder();
+        mlBuilder.putJob(BaseMlIntegTestCase.createFareQuoteJob("job_id_1").build(new Date()), false);
+
+        ClusterState cs1 = ClusterState.builder(new ClusterName("_name"))
+                .metaData(new MetaData.Builder().putCustom(MlMetadata.TYPE, mlBuilder.build()))
+                .build();
+
+        List<String> openJobs = new ArrayList<>();
+        List<String> closingJobs = new ArrayList<>();
+
+        expectThrows(ResourceNotFoundException.class,
+                () -> CloseJobAction.resolveAndValidateJobId("missing-job", cs1, openJobs, closingJobs, false));
+    }
+
     public void testResolve_givenJobIdFailed() {
         MlMetadata.Builder mlBuilder = new MlMetadata.Builder();
         mlBuilder.putJob(BaseMlIntegTestCase.createFareQuoteJob("job_id_failed").build(new Date()), false);
diff --git a/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml b/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml
index 398b52c7a68..553dd8954f2 100644
--- a/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml
+++ b/plugin/src/test/resources/rest-api-spec/test/ml/jobs_crud.yml
@@ -572,6 +572,18 @@
         force: true
   - match: { closed: true }
 
+---
+"Test open and close an unknown job is resource not found":
+  - do:
+      catch: missing
+      xpack.ml.open_job:
+        job_id: jobs-crud-some-missing-job-i-made-up
+
+  - do:
+      catch: missing
+      xpack.ml.close_job:
+        job_id: jobs-crud-some-missing-job-i-made-up
+
 ---
 "Test cannot create job with existing categorizer state document":