Skip to content

Commit 24775a9

Browse files
davidkyledavidkyle
committed
Return missing job error when .ml-config is does not exist
1 parent 2ca9d11 commit 24775a9

File tree

5 files changed

+87
-28
lines changed

5 files changed

+87
-28
lines changed

‎x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/persistence/DatafeedConfigProvider.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.elasticsearch.common.xcontent.XContentFactory;
3535
import org.elasticsearch.common.xcontent.XContentParser;
3636
import org.elasticsearch.common.xcontent.XContentType;
37+
import org.elasticsearch.index.IndexNotFoundException;
3738
import org.elasticsearch.index.engine.VersionConflictEngineException;
3839
import org.elasticsearch.index.query.BoolQueryBuilder;
3940
import org.elasticsearch.index.query.QueryBuilder;
@@ -136,6 +137,9 @@ public void putDatafeedConfig(DatafeedConfig config, Map<String, String> headers
136137
* If the datafeed document is missing a {@code ResourceNotFoundException}
137138
* is returned via the listener.
138139
*
140+
* If the .ml-config index does not exist it is treated as a missing datafeed
141+
* error.
142+
*
139143
* @param datafeedId The datafeed ID
140144
* @param datafeedConfigListener The config listener
141145
*/
@@ -154,7 +158,11 @@ public void onResponse(GetResponse getResponse) {
154158
}
155159
@Override
156160
public void onFailure(Exception e) {
157-
datafeedConfigListener.onFailure(e);
161+
if (e.getClass() == IndexNotFoundException.class) {
162+
datafeedConfigListener.onFailure(ExceptionsHelper.missingDatafeedException(datafeedId));
163+
} else {
164+
datafeedConfigListener.onFailure(e);
165+
}
158166
}
159167
});
160168
}

‎x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ private void setMaxModelMemoryLimit(ByteSizeValue maxModelMemoryLimit) {
120120
}
121121

122122
public void jobExists(String jobId, ActionListener<Boolean> listener) {
123-
jobConfigProvider.checkJobExists(jobId, listener);
123+
jobConfigProvider.jobExists(jobId, true, listener);
124124
}
125125

126126
/**
@@ -281,7 +281,16 @@ public void onFailure(Exception e) {
281281
actionListener::onFailure
282282
);
283283

284-
jobResultsProvider.checkForLeftOverDocuments(job, checkForLeftOverDocs);
284+
jobConfigProvider.jobExists(job.getId(), false, ActionListener.wrap(
285+
jobExists -> {
286+
if (jobExists) {
287+
actionListener.onFailure(ExceptionsHelper.jobAlreadyExists(job.getId()));
288+
} else {
289+
jobResultsProvider.checkForLeftOverDocuments(job, checkForLeftOverDocs);
290+
}
291+
},
292+
actionListener::onFailure
293+
));
285294
}
286295

287296
public void updateJob(UpdateJobAction.Request request, ActionListener<PutJobAction.Response> actionListener) {

‎x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/persistence/JobConfigProvider.java‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.elasticsearch.common.xcontent.XContentFactory;
4040
import org.elasticsearch.common.xcontent.XContentParser;
4141
import org.elasticsearch.common.xcontent.XContentType;
42+
import org.elasticsearch.index.IndexNotFoundException;
4243
import org.elasticsearch.index.engine.VersionConflictEngineException;
4344
import org.elasticsearch.index.query.BoolQueryBuilder;
4445
import org.elasticsearch.index.query.QueryBuilder;
@@ -122,6 +123,9 @@ public void putJob(Job job, ActionListener<IndexResponse> listener) {
122123
* If the job is missing a {@code ResourceNotFoundException} is returned
123124
* via the listener.
124125
*
126+
* If the .ml-config index does not exist it is treated as a missing job
127+
* error.
128+
*
125129
* @param jobId The job ID
126130
* @param jobListener Job listener
127131
*/
@@ -143,7 +147,11 @@ public void onResponse(GetResponse getResponse) {
143147

144148
@Override
145149
public void onFailure(Exception e) {
146-
jobListener.onFailure(e);
150+
if (e.getClass() == IndexNotFoundException.class) {
151+
jobListener.onFailure(ExceptionsHelper.missingJobException(jobId));
152+
} else {
153+
jobListener.onFailure(e);
154+
}
147155
}
148156
}, client::get);
149157
}
@@ -368,14 +376,19 @@ private void indexUpdatedJob(Job updatedJob, long version, ActionListener<Job> u
368376

369377
/**
370378
* Check a job exists. A job exists if it has a configuration document.
379+
* If the .ml-config index does not exist it is treated as a missing job
380+
* error.
371381
*
372-
* If the job does not exist a ResourceNotFoundException is returned to the listener,
373-
* FALSE will never be returned only TRUE or ResourceNotFoundException
382+
* Depending on the value of {@code errorIfMissing} if the job does not
383+
* exist a ResourceNotFoundException is returned to the listener,
384+
* otherwise false is returned in the response.
374385
*
375-
* @param jobId The jobId to check
376-
* @param listener Exists listener
386+
* @param jobId The jobId to check
387+
* @param errorIfMissing If true and the job is missing the listener fails with
388+
* a ResourceNotFoundException else false is returned.
389+
* @param listener Exists listener
377390
*/
378-
public void checkJobExists(String jobId, ActionListener<Boolean> listener) {
391+
public void jobExists(String jobId, boolean errorIfMissing, ActionListener<Boolean> listener) {
379392
GetRequest getRequest = new GetRequest(AnomalyDetectorsIndex.configIndexName(),
380393
ElasticsearchMappings.DOC_TYPE, Job.documentId(jobId));
381394
getRequest.fetchSourceContext(FetchSourceContext.DO_NOT_FETCH_SOURCE);
@@ -384,15 +397,27 @@ public void checkJobExists(String jobId, ActionListener<Boolean> listener) {
384397
@Override
385398
public void onResponse(GetResponse getResponse) {
386399
if (getResponse.isExists() == false) {
387-
listener.onFailure(ExceptionsHelper.missingJobException(jobId));
400+
if (errorIfMissing) {
401+
listener.onFailure(ExceptionsHelper.missingJobException(jobId));
402+
} else {
403+
listener.onResponse(Boolean.FALSE);
404+
}
388405
} else {
389406
listener.onResponse(Boolean.TRUE);
390407
}
391408
}
392409

393410
@Override
394411
public void onFailure(Exception e) {
395-
listener.onFailure(e);
412+
if (e.getClass() == IndexNotFoundException.class) {
413+
if (errorIfMissing) {
414+
listener.onFailure(ExceptionsHelper.missingJobException(jobId));
415+
} else {
416+
listener.onResponse(Boolean.FALSE);
417+
}
418+
} else {
419+
listener.onFailure(e);
420+
}
396421
}
397422
});
398423
}

‎x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/DatafeedConfigProviderIT.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,15 @@ public void testCrud() throws InterruptedException {
108108
assertEquals(DocWriteResponse.Result.DELETED, deleteResponseHolder.get().getResult());
109109
}
110110

111+
public void testGetDatafeedConfig_missing() throws InterruptedException {
112+
AtomicReference<Exception> exceptionHolder = new AtomicReference<>();
113+
AtomicReference<DatafeedConfig.Builder> configBuilderHolder = new AtomicReference<>();
114+
blockingCall(actionListener -> datafeedConfigProvider.getDatafeedConfig("missing", actionListener),
115+
configBuilderHolder, exceptionHolder);
116+
assertNull(configBuilderHolder.get());
117+
assertEquals(ResourceNotFoundException.class, exceptionHolder.get().getClass());
118+
}
119+
111120
public void testMultipleCreateAndDeletes() throws InterruptedException {
112121
String datafeedId = "df2";
113122

@@ -127,7 +136,7 @@ public void testMultipleCreateAndDeletes() throws InterruptedException {
127136
indexResponseHolder, exceptionHolder);
128137
assertNull(indexResponseHolder.get());
129138
assertThat(exceptionHolder.get(), instanceOf(ResourceAlreadyExistsException.class));
130-
assertEquals("A datafeed with Id [df2] already exists", exceptionHolder.get().getMessage());
139+
assertEquals("A datafeed with id [df2] already exists", exceptionHolder.get().getMessage());
131140

132141
// delete
133142
exceptionHolder.set(null);
@@ -142,7 +151,7 @@ public void testMultipleCreateAndDeletes() throws InterruptedException {
142151
blockingCall(actionListener -> datafeedConfigProvider.deleteDatafeedConfig(datafeedId, actionListener),
143152
deleteResponseHolder, exceptionHolder);
144153
assertNull(deleteResponseHolder.get());
145-
assertThat(exceptionHolder.get(), instanceOf(ResourceNotFoundException.class));
154+
assertEquals(ResourceNotFoundException.class, exceptionHolder.get().getClass());
146155
}
147156

148157
public void testUpdateWhenApplyingTheUpdateThrows() throws Exception {
@@ -202,7 +211,7 @@ public void testAllowNoDatafeeds() throws InterruptedException {
202211

203212
assertNull(datafeedIdsHolder.get());
204213
assertNotNull(exceptionHolder.get());
205-
assertThat(exceptionHolder.get(), IsInstanceOf.instanceOf(ResourceNotFoundException.class));
214+
assertEquals(ResourceNotFoundException.class, exceptionHolder.get().getClass());
206215
assertThat(exceptionHolder.get().getMessage(), containsString("No datafeed with id [*] exists"));
207216

208217
exceptionHolder.set(null);
@@ -217,7 +226,7 @@ public void testAllowNoDatafeeds() throws InterruptedException {
217226

218227
assertNull(datafeedsHolder.get());
219228
assertNotNull(exceptionHolder.get());
220-
assertThat(exceptionHolder.get(), IsInstanceOf.instanceOf(ResourceNotFoundException.class));
229+
assertEquals(ResourceNotFoundException.class, exceptionHolder.get().getClass());
221230
assertThat(exceptionHolder.get().getMessage(), containsString("No datafeed with id [*] exists"));
222231

223232
exceptionHolder.set(null);

‎x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/integration/JobConfigProviderIT.java‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,18 +68,25 @@ public void testGetMissingJob() throws InterruptedException {
6868

6969
assertNull(jobHolder.get());
7070
assertNotNull(exceptionHolder.get());
71-
assertThat(exceptionHolder.get(), instanceOf(ResourceNotFoundException.class));
71+
assertEquals(ResourceNotFoundException.class, exceptionHolder.get().getClass());
7272
}
7373

7474
public void testCheckJobExists() throws InterruptedException {
7575
AtomicReference<Boolean> jobExistsHolder = new AtomicReference<>();
7676
AtomicReference<Exception> exceptionHolder = new AtomicReference<>();
7777

78-
blockingCall(actionListener -> jobConfigProvider.checkJobExists("missing", actionListener), jobExistsHolder, exceptionHolder);
79-
80-
assertNull(jobExistsHolder.get());
81-
assertNotNull(exceptionHolder.get());
82-
assertThat(exceptionHolder.get(), instanceOf(ResourceNotFoundException.class));
78+
boolean throwIfMissing = randomBoolean();
79+
blockingCall(actionListener ->
80+
jobConfigProvider.jobExists("missing", throwIfMissing, actionListener), jobExistsHolder, exceptionHolder);
81+
82+
if (throwIfMissing) {
83+
assertNull(jobExistsHolder.get());
84+
assertNotNull(exceptionHolder.get());
85+
assertEquals(ResourceNotFoundException.class, exceptionHolder.get().getClass());
86+
} else {
87+
assertFalse(jobExistsHolder.get());
88+
assertNull(exceptionHolder.get());
89+
}
8390

8491
AtomicReference<IndexResponse> indexResponseHolder = new AtomicReference<>();
8592

@@ -88,7 +95,8 @@ public void testCheckJobExists() throws InterruptedException {
8895
blockingCall(actionListener -> jobConfigProvider.putJob(job, actionListener), indexResponseHolder, exceptionHolder);
8996

9097
exceptionHolder.set(null);
91-
blockingCall(actionListener -> jobConfigProvider.checkJobExists("existing-job", actionListener), jobExistsHolder, exceptionHolder);
98+
blockingCall(actionListener ->
99+
jobConfigProvider.jobExists("existing-job", throwIfMissing, actionListener), jobExistsHolder, exceptionHolder);
92100
assertNull(exceptionHolder.get());
93101
assertNotNull(jobExistsHolder.get());
94102
assertTrue(jobExistsHolder.get());
@@ -159,15 +167,15 @@ public void testCrud() throws InterruptedException {
159167
getJobResponseHolder.set(null);
160168
blockingCall(actionListener -> jobConfigProvider.getJob(jobId, actionListener), getJobResponseHolder, exceptionHolder);
161169
assertNull(getJobResponseHolder.get());
162-
assertThat(exceptionHolder.get(), instanceOf(ResourceNotFoundException.class));
170+
assertEquals(ResourceNotFoundException.class, exceptionHolder.get().getClass());
163171

164172
// Delete deleted job
165173
deleteJobResponseHolder.set(null);
166174
exceptionHolder.set(null);
167175
blockingCall(actionListener -> jobConfigProvider.deleteJob(jobId, actionListener),
168176
deleteJobResponseHolder, exceptionHolder);
169177
assertNull(deleteJobResponseHolder.get());
170-
assertThat(exceptionHolder.get(), instanceOf(ResourceNotFoundException.class));
178+
assertEquals(ResourceNotFoundException.class, exceptionHolder.get().getClass());
171179
}
172180

173181
public void testGetJobs() throws Exception {
@@ -263,7 +271,7 @@ public void testAllowNoJobs() throws InterruptedException {
263271

264272
assertNull(jobIdsHolder.get());
265273
assertNotNull(exceptionHolder.get());
266-
assertThat(exceptionHolder.get(), instanceOf(ResourceNotFoundException.class));
274+
assertEquals(ResourceNotFoundException.class, exceptionHolder.get().getClass());
267275
assertThat(exceptionHolder.get().getMessage(), containsString("No known job with id"));
268276

269277
exceptionHolder.set(null);
@@ -278,7 +286,7 @@ public void testAllowNoJobs() throws InterruptedException {
278286

279287
assertNull(jobsHolder.get());
280288
assertNotNull(exceptionHolder.get());
281-
assertThat(exceptionHolder.get(), instanceOf(ResourceNotFoundException.class));
289+
assertEquals(ResourceNotFoundException.class, exceptionHolder.get().getClass());
282290
assertThat(exceptionHolder.get().getMessage(), containsString("No known job with id"));
283291

284292
exceptionHolder.set(null);
@@ -315,7 +323,7 @@ public void testExpandJobs_GroupsAndJobIds() throws Exception {
315323
jobIdsHolder, exceptionHolder);
316324
assertNull(jobIdsHolder.get());
317325
assertNotNull(exceptionHolder.get());
318-
assertThat(exceptionHolder.get(), instanceOf(ResourceNotFoundException.class));
326+
assertEquals(ResourceNotFoundException.class, exceptionHolder.get().getClass());
319327
assertThat(exceptionHolder.get().getMessage(), equalTo("No known job with id 'missing1,missing2'"));
320328

321329
// Job builders
@@ -344,7 +352,7 @@ public void testExpandJobs_GroupsAndJobIds() throws Exception {
344352
jobsHolder, exceptionHolder);
345353
assertNull(jobsHolder.get());
346354
assertNotNull(exceptionHolder.get());
347-
assertThat(exceptionHolder.get(), instanceOf(ResourceNotFoundException.class));
355+
assertEquals(ResourceNotFoundException.class, exceptionHolder.get().getClass());
348356
assertThat(exceptionHolder.get().getMessage(), equalTo("No known job with id 'missing1,missing2'"));
349357
}
350358

0 commit comments

Comments
 (0)