Return 200 on GET requests for all resources when none exists (elastic/elasticsearch#694)

When a user makes a GET request to retrieve all resources of a type
(e.g. anomaly_detectors) and none exists, the response should be an
empty array with 200 status code. This commit fixes this issue for:

* anomaly_detectors and _stats
* schedulers and _stats
* lists
* buckets

All other GETs work fine already.

Original commit: elastic/x-pack-elasticsearch@4daaa91aa4
This commit is contained in:
Dimitris Athanasiou 2017-01-12 17:46:39 +00:00 committed by GitHub
parent c4d5cf660d
commit c1ee50f238
16 changed files with 87 additions and 69 deletions

View File

@ -22,12 +22,11 @@ import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.ml.job.Job;
@ -326,7 +325,7 @@ public class GetBucketsAction extends Action<GetBucketsAction.Request, GetBucket
}
}
public static class Response extends ActionResponse implements StatusToXContentObject {
public static class Response extends ActionResponse implements ToXContentObject {
private QueryPage<Bucket> buckets;
@ -361,11 +360,6 @@ public class GetBucketsAction extends Action<GetBucketsAction.Request, GetBucket
return builder;
}
@Override
public RestStatus status() {
return buckets.count() == 0 ? RestStatus.NOT_FOUND : RestStatus.OK;
}
@Override
public int hashCode() {
return Objects.hash(buckets);

View File

@ -23,10 +23,9 @@ import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.ml.job.Job;
@ -112,7 +111,7 @@ public class GetJobsAction extends Action<GetJobsAction.Request, GetJobsAction.R
}
}
public static class Response extends ActionResponse implements StatusToXContentObject {
public static class Response extends ActionResponse implements ToXContentObject {
private QueryPage<Job> jobs;
@ -146,11 +145,6 @@ public class GetJobsAction extends Action<GetJobsAction.Request, GetJobsAction.R
return builder;
}
@Override
public RestStatus status() {
return jobs.count() == 0 ? RestStatus.NOT_FOUND : RestStatus.OK;
}
@Override
public int hashCode() {
return Objects.hash(jobs);

View File

@ -23,11 +23,10 @@ import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.AtomicArray;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.ml.job.DataCounts;
@ -42,6 +41,7 @@ import org.elasticsearch.xpack.ml.job.persistence.QueryPage;
import org.elasticsearch.xpack.ml.utils.ExceptionsHelper;
import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
@ -129,7 +129,7 @@ public class GetJobsStatsAction extends Action<GetJobsStatsAction.Request, GetJo
}
}
public static class Response extends ActionResponse implements StatusToXContentObject {
public static class Response extends ActionResponse implements ToXContentObject {
public static class JobStats implements ToXContent, Writeable {
private final String jobId;
@ -235,11 +235,6 @@ public class GetJobsStatsAction extends Action<GetJobsStatsAction.Request, GetJo
jobsStats.writeTo(out);
}
@Override
public RestStatus status() {
return jobsStats.count() == 0 ? RestStatus.NOT_FOUND : RestStatus.OK;
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();;
@ -305,6 +300,9 @@ public class GetJobsStatsAction extends Action<GetJobsStatsAction.Request, GetJo
protected void doExecute(Request request, ActionListener<Response> listener) {
logger.debug("Get stats for job '{}'", request.getJobId());
QueryPage<Job> jobs = jobManager.getJob(request.getJobId(), clusterService.state());
if (jobs.count() == 0) {
listener.onResponse(new GetJobsStatsAction.Response(new QueryPage<>(Collections.emptyList(), 0, Job.RESULTS_FIELD)));
}
MlMetadata mlMetadata = clusterService.state().metaData().custom(MlMetadata.TYPE);
AtomicInteger counter = new AtomicInteger(0);

View File

@ -183,7 +183,7 @@ public class GetListAction extends Action<GetListAction.Request, GetListAction.R
@Override
public RestStatus status() {
return lists.count() == 0 ? RestStatus.NOT_FOUND : RestStatus.OK;
return RestStatus.OK;
}
@Override
@ -319,24 +319,17 @@ public class GetListAction extends Action<GetListAction.Request, GetListAction.R
public void onResponse(SearchResponse response) {
try {
QueryPage<ListDocument> responseBody;
if (response.getHits().hits().length > 0) {
List<ListDocument> docs = new ArrayList<>(response.getHits().hits().length);
for (SearchHit hit : response.getHits().getHits()) {
BytesReference docSource = hit.sourceRef();
XContentParser parser =
XContentFactory.xContent(docSource).createParser(NamedXContentRegistry.EMPTY, docSource);
docs.add(ListDocument.PARSER.apply(parser, () -> parseFieldMatcher));
}
responseBody = new QueryPage<>(docs, docs.size(), ListDocument.RESULTS_FIELD);
Response listResponse = new Response(responseBody);
listener.onResponse(listResponse);
} else {
this.onFailure(QueryPage.emptyQueryPage(ListDocument.RESULTS_FIELD));
List<ListDocument> docs = new ArrayList<>();
for (SearchHit hit : response.getHits().getHits()) {
BytesReference docSource = hit.sourceRef();
XContentParser parser =
XContentFactory.xContent(docSource).createParser(NamedXContentRegistry.EMPTY, docSource);
docs.add(ListDocument.PARSER.apply(parser, () -> parseFieldMatcher));
}
Response listResponse = new Response(new QueryPage<>(docs, docs.size(), ListDocument.RESULTS_FIELD));
listener.onResponse(listResponse);
} catch (Exception e) {
this.onFailure(e);
}

View File

@ -23,10 +23,9 @@ import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.ml.job.metadata.MlMetadata;
@ -119,7 +118,7 @@ public class GetSchedulersAction extends Action<GetSchedulersAction.Request, Get
}
}
public static class Response extends ActionResponse implements StatusToXContentObject {
public static class Response extends ActionResponse implements ToXContentObject {
private QueryPage<SchedulerConfig> schedulers;
@ -145,11 +144,6 @@ public class GetSchedulersAction extends Action<GetSchedulersAction.Request, Get
schedulers.writeTo(out);
}
@Override
public RestStatus status() {
return schedulers.count() == 0 ? RestStatus.NOT_FOUND : RestStatus.OK;
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();

View File

@ -24,11 +24,10 @@ import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
import org.elasticsearch.xpack.ml.job.metadata.MlMetadata;
@ -123,7 +122,7 @@ public class GetSchedulersStatsAction extends Action<GetSchedulersStatsAction.Re
}
}
public static class Response extends ActionResponse implements StatusToXContentObject {
public static class Response extends ActionResponse implements ToXContentObject {
public static class SchedulerStats implements ToXContent, Writeable {
@ -206,11 +205,6 @@ public class GetSchedulersStatsAction extends Action<GetSchedulersStatsAction.Re
schedulersStats.writeTo(out);
}
@Override
public RestStatus status() {
return schedulersStats.count() == 0 ? RestStatus.NOT_FOUND : RestStatus.OK;
}
@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject();

View File

@ -101,7 +101,7 @@ public class JobManager extends AbstractComponent {
Job job = mlMetadata.getJobs().get(jobId);
if (job == null) {
logger.debug(String.format(Locale.ROOT, "Cannot find job '%s'", jobId));
throw QueryPage.emptyQueryPage(Job.RESULTS_FIELD);
throw ExceptionsHelper.missingJobException(jobId);
}
logger.debug("Returning job [" + jobId + "]");

View File

@ -11,7 +11,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.RestStatusToXContentListener;
import org.elasticsearch.rest.action.RestToXContentListener;
import org.elasticsearch.xpack.ml.MlPlugin;
import org.elasticsearch.xpack.ml.action.GetJobsAction;
import org.elasticsearch.xpack.ml.job.Job;
@ -34,6 +34,6 @@ public class RestGetJobsAction extends BaseRestHandler {
@Override
protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
GetJobsAction.Request request = new GetJobsAction.Request(restRequest.param(Job.ID.getPreferredName()));
return channel -> transportGetJobAction.execute(request, new RestStatusToXContentListener<>(channel));
return channel -> transportGetJobAction.execute(request, new RestToXContentListener<>(channel));
}
}

View File

@ -11,7 +11,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.RestStatusToXContentListener;
import org.elasticsearch.rest.action.RestToXContentListener;
import org.elasticsearch.xpack.ml.MlPlugin;
import org.elasticsearch.xpack.ml.action.GetJobsStatsAction;
import org.elasticsearch.xpack.ml.job.Job;
@ -35,6 +35,6 @@ public class RestGetJobsStatsAction extends BaseRestHandler {
@Override
protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
GetJobsStatsAction.Request request = new GetJobsStatsAction.Request(restRequest.param(Job.ID.getPreferredName()));
return channel -> transportGetJobsStatsAction.execute(request, new RestStatusToXContentListener<>(channel));
return channel -> transportGetJobsStatsAction.execute(request, new RestToXContentListener<>(channel));
}
}

View File

@ -12,7 +12,7 @@ import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.RestStatusToXContentListener;
import org.elasticsearch.rest.action.RestToXContentListener;
import org.elasticsearch.xpack.ml.MlPlugin;
import org.elasticsearch.xpack.ml.action.GetBucketsAction;
import org.elasticsearch.xpack.ml.job.Job;
@ -90,6 +90,6 @@ public class RestGetBucketsAction extends BaseRestHandler {
request.setIncludeInterim(restRequest.paramAsBoolean(GetBucketsAction.Request.INCLUDE_INTERIM.getPreferredName(), false));
}
return channel -> transportAction.execute(request, new RestStatusToXContentListener<>(channel));
return channel -> transportAction.execute(request, new RestToXContentListener<>(channel));
}
}

View File

@ -11,7 +11,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.RestStatusToXContentListener;
import org.elasticsearch.rest.action.RestToXContentListener;
import org.elasticsearch.xpack.ml.MlPlugin;
import org.elasticsearch.xpack.ml.action.GetSchedulersAction;
import org.elasticsearch.xpack.ml.scheduler.SchedulerConfig;
@ -35,6 +35,6 @@ public class RestGetSchedulersAction extends BaseRestHandler {
@Override
protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
GetSchedulersAction.Request request = new GetSchedulersAction.Request(restRequest.param(SchedulerConfig.ID.getPreferredName()));
return channel -> transportGetSchedulersAction.execute(request, new RestStatusToXContentListener<>(channel));
return channel -> transportGetSchedulersAction.execute(request, new RestToXContentListener<>(channel));
}
}

View File

@ -11,7 +11,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.RestStatusToXContentListener;
import org.elasticsearch.rest.action.RestToXContentListener;
import org.elasticsearch.xpack.ml.MlPlugin;
import org.elasticsearch.xpack.ml.action.GetSchedulersStatsAction;
import org.elasticsearch.xpack.ml.scheduler.SchedulerConfig;
@ -36,6 +36,6 @@ public class RestGetSchedulersStatsAction extends BaseRestHandler {
protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException {
GetSchedulersStatsAction.Request request = new GetSchedulersStatsAction.Request(
restRequest.param(SchedulerConfig.ID.getPreferredName()));
return channel -> transportGetSchedulersStatsAction.execute(request, new RestStatusToXContentListener<>(channel));
return channel -> transportGetSchedulersStatsAction.execute(request, new RestToXContentListener<>(channel));
}
}

View File

@ -55,7 +55,7 @@ public class MlJobIT extends ESRestTestCase {
() -> client().performRequest("get", MlPlugin.BASE_PATH + "anomaly_detectors/non-existing-job/_stats"));
assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(404));
assertThat(e.getMessage(), containsString("Could not find requested jobs"));
assertThat(e.getMessage(), containsString("No known job with id 'non-existing-job'"));
}
public void testGetJob_GivenJobExists() throws Exception {

View File

@ -0,0 +1,19 @@
---
"Test get all lists given no list exists":
- do:
xpack.ml.put_list:
body: >
{
"id": "foo",
"items": ["abc", "xyz"]
}
- do:
xpack.ml.delete_list:
list_id: "foo"
- do:
xpack.ml.get_lists: {}
- match: { count: 0 }
- match: { lists: [] }

View File

@ -1,5 +1,21 @@
---
"Test get all jobs and stats given no job exists":
- do:
xpack.ml.get_jobs:
job_id: "_all"
- match: { count: 0 }
- match: { jobs: [] }
- do:
xpack.ml.get_jobs_stats:
job_id: "_all"
- match: { count: 0 }
- match: { jobs: [] }
---
"Test job crud apis":
- do:
xpack.ml.put_job:
job_id: farequote

View File

@ -31,6 +31,22 @@ setup:
"time_format":"epoch"
}
}
---
"Test get all schedulers and stats given no scheduler exists":
- do:
xpack.ml.get_schedulers:
scheduler_id: "_all"
- match: { count: 0 }
- match: { schedulers: [] }
- do:
xpack.ml.get_schedulers_stats:
scheduler_id: "_all"
- match: { count: 0 }
- match: { schedulers: [] }
---
"Test put scheduler referring to missing job_id":
- do: