Fix get a single job

davidkyle · Oct 3, 2018 · 168fd12 · 168fd12
1 parent e647611
commit 168fd12
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 6 deletions.
diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java
@@ -170,7 +170,7 @@ private void getJobFromClusterState(String jobId, ActionListener<Job> jobListene
      * @param jobsListener The jobs listener
      */
     public void expandJobs(String expression, boolean allowNoJobs, ActionListener<QueryPage<Job>> jobsListener) {
-        Map<String, Job> clusterStateJobs = expandJobsFromClusterState(expression, allowNoJobs, clusterService.state());
+        Map<String, Job> clusterStateJobs = expandJobsFromClusterState(expression, clusterService.state());
 
         jobConfigProvider.expandJobs(expression, allowNoJobs, ActionListener.wrap(
                 jobBuilders -> {
@@ -197,12 +197,20 @@ public void expandJobs(String expression, boolean allowNoJobs, ActionListener<Qu
         ));
     }
 
-    private Map<String, Job> expandJobsFromClusterState(String expression, boolean allowNoJobs, ClusterState clusterState) {
-        Set<String> expandedJobIds = MlMetadata.getMlMetadata(clusterState).expandJobIds(expression, allowNoJobs);
-        MlMetadata mlMetadata = MlMetadata.getMlMetadata(clusterState);
+    private Map<String, Job> expandJobsFromClusterState(String expression, ClusterState clusterState) {
         Map<String, Job> jobIdToJob = new HashMap<>();
-        for (String expandedJobId : expandedJobIds) {
-            jobIdToJob.put(expandedJobId, mlMetadata.getJobs().get(expandedJobId));
+        MlMetadata mlMetadata = MlMetadata.getMlMetadata(clusterState);
+        try {
+            // This call will throw if the expression is not a wild card
+            // and the job does not exist. This is not the behaviour we
+            // want as the job may exist in the index.
+            // TODO jindex review the use of this function. Can it be changed not to throw in a BWC manner?
+            Set<String> expandedJobIds = mlMetadata.expandJobIds(expression, true);
+            for (String expandedJobId : expandedJobIds) {
+                jobIdToJob.put(expandedJobId, mlMetadata.getJobs().get(expandedJobId));
+            }
+        } catch (ResourceNotFoundException e) {
+            return jobIdToJob;
         }
         return jobIdToJob;
     }

diff --git a/...k/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobConfigProviderIT.java b/...k/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobConfigProviderIT.java
@@ -42,6 +42,7 @@
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
+import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
@@ -342,6 +343,11 @@ public void testExpandJobs_GroupsAndJobIds() throws Exception {
         expandedJobs = expandedJobsBuilders.stream().map(Job.Builder::build).collect(Collectors.toList());
         assertThat(expandedJobs, containsInAnyOrder(tom, harry));
 
+        expandedJobsBuilders = blockingCall(actionListener ->
+                jobConfigProvider.expandJobs("tom", false, actionListener));
+        expandedJobs = expandedJobsBuilders.stream().map(Job.Builder::build).collect(Collectors.toList());
+        assertThat(expandedJobs, contains(tom));
+
         expandedJobsBuilders = blockingCall(actionListener ->
                 jobConfigProvider.expandJobs("", false, actionListener));
         expandedJobs = expandedJobsBuilders.stream().map(Job.Builder::build).collect(Collectors.toList());

diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/job/JobManagerTests.java
@@ -216,6 +216,19 @@ public void testExpandJobsFromClusterStateAndIndex() throws IOException {
         assertThat(jobsHolder.get().results(), hasSize(3));
         jobIds = jobsHolder.get().results().stream().map(Job::getId).collect(Collectors.toList());
         assertThat(jobIds, contains("foo-cs-1", "foo-cs-2", "foo-index"));
+
+
+        jobsHolder.set(null);
+        jobManager.expandJobs("foo-index", true, ActionListener.wrap(
+                jobs -> jobsHolder.set(jobs),
+                e -> fail(e.getMessage())
+        ));
+
+        assertNotNull(jobsHolder.get());
+        assertThat(jobsHolder.get().results(), hasSize(1));
+        jobIds = jobsHolder.get().results().stream().map(Job::getId).collect(Collectors.toList());
+        assertThat(jobIds, contains("foo-index"));
+
     }
 
     @SuppressWarnings("unchecked")