From 00921971855c2369b146498c29a9fc1047e78493 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 17 Jun 2021 14:25:45 +0200 Subject: [PATCH 01/10] Introduce Next Field in Paginated GetSnapshots Response Follow up to #73952 adding documentation for the `after` query parameter and the related `next` response field. --- .../apis/get-snapshot-api.asciidoc | 146 +++++++++++++++++- .../http/snapshots/RestGetSnapshotsIT.java | 80 +++++----- .../snapshots/GetSnapshotsIT.java | 59 ++++--- .../snapshots/get/GetSnapshotsRequest.java | 14 ++ .../get/GetSnapshotsRequestBuilder.java | 5 +- .../snapshots/get/GetSnapshotsResponse.java | 52 ++++++- .../get/TransportGetSnapshotsAction.java | 11 +- .../get/GetSnapshotsResponseTests.java | 2 +- .../xpack/slm/SnapshotRetentionTaskTests.java | 2 +- 9 files changed, 304 insertions(+), 67 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index b19e922137815..5ddd35b14ae60 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -19,7 +19,9 @@ PUT /_snapshot/my_repository PUT /_snapshot/my_repository/my_snapshot?wait_for_completion=true +PUT /_snapshot/my_repository/snapshot_1?wait_for_completion=true PUT /_snapshot/my_repository/snapshot_2?wait_for_completion=true +PUT /_snapshot/my_repository/snapshot_3?wait_for_completion=true ---- // TESTSETUP //// @@ -128,7 +130,11 @@ Allows setting a sort order for the result. Defaults to `start_time`, i.e. sorti (Optional, string) Sort order. Valid values are `asc` for ascending and `desc` for descending order. Defaults to `asc`, meaning ascending order. -NOTE: The pagination parameters `size`, `order`, and `sort` are not supported when using `verbose=false` and the sort order for +`after`:: +(Optional, string) +Offset identifier to start pagination from as returned by the `next` field in the response body. + +NOTE: The pagination parameters `size`, `order`, `after` and `sort` are not supported when using `verbose=false` and the sort order for requests with `verbose=false` is undefined. [role="child_attributes"] @@ -268,6 +274,10 @@ The snapshot `state` can be one of the following values: that were not processed correctly. ==== -- +`next`:: +(string) +If the request contained a size limit and there might be more results a `next` field will be added to the response and can be used as the +`after` query parameter to fetch additional results. [[get-snapshot-api-example]] ==== {api-examples-title} @@ -323,3 +333,137 @@ The API returns the following response: // TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.129Z"/"end_time": $body.responses.0.snapshots.0.end_time/] // TESTRESPONSE[s/"end_time_in_millis": 1593094752018/"end_time_in_millis": $body.responses.0.snapshots.0.end_time_in_millis/] // TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.responses.0.snapshots.0.duration_in_millis/] + +The following request returns information for all snapshots with prefix `snapshot` in the `my_repository` repository, +limiting the response size to 2 and sorting by snapshot name. + +[source,console] +---- +GET /_snapshot/my_repository/snapshot*?size=2&sort=name +---- + +The API returns the following response: + +[source,console-result] +---- +{ + "responses": [ + { + "repository": "my_repository", + "snapshots": [ + { + "snapshot": "snapshot_1", + "uuid": "dKb54xw67gvdRctLCxSket", + "version_id": , + "version": , + "indices": [], + "data_streams": [], + "feature_states": [], + "include_global_state": true, + "state": "SUCCESS", + "start_time": "2020-07-06T21:55:18.129Z", + "start_time_in_millis": 1593093628850, + "end_time": "2020-07-06T21:55:18.129Z", + "end_time_in_millis": 1593094752018, + "duration_in_millis": 0, + "failures": [], + "shards": { + "total": 0, + "failed": 0, + "successful": 0 + } + }, + { + "snapshot": "snapshot_2", + "uuid": "vdRctLCxSketdKb54xw67g", + "version_id": , + "version": , + "indices": [], + "data_streams": [], + "feature_states": [], + "include_global_state": true, + "state": "SUCCESS", + "start_time": "2020-07-06T21:55:18.130Z", + "start_time_in_millis": 1593093628851, + "end_time": "2020-07-06T21:55:18.130Z", + "end_time_in_millis": 1593094752019, + "duration_in_millis": 0, + "failures": [], + "shards": { + "total": 0, + "failed": 0, + "successful": 0 + } + } + ], + "next": "snapshot_2,snapshot_2" + } + ] +} +---- +// TESTRESPONSE[s/"uuid": "dKb54xw67gvdRctLCxSket"/"uuid": $body.responses.0.snapshots.0.uuid/] +// TESTRESPONSE[s/"uuid": "vdRctLCxSketdKb54xw67g"/"uuid": $body.responses.0.snapshots.1.uuid/] +// TESTRESPONSE[s/"version_id": /"version_id": $body.responses.0.snapshots.0.version_id/] +// TESTRESPONSE[s/"version": /"version": $body.responses.0.snapshots.0.version/] +// TESTRESPONSE[s/"start_time": "2020-07-06T21:55:18.129Z"/"start_time": $body.responses.0.snapshots.0.start_time/] +// TESTRESPONSE[s/"start_time": "2020-07-06T21:55:18.130Z"/"start_time": $body.responses.0.snapshots.1.start_time/] +// TESTRESPONSE[s/"start_time_in_millis": 1593093628850/"start_time_in_millis": $body.responses.0.snapshots.0.start_time_in_millis/] +// TESTRESPONSE[s/"start_time_in_millis": 1593093628851/"start_time_in_millis": $body.responses.0.snapshots.1.start_time_in_millis/] +// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.129Z"/"end_time": $body.responses.0.snapshots.0.end_time/] +// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.130Z"/"end_time": $body.responses.0.snapshots.1.end_time/] +// TESTRESPONSE[s/"end_time_in_millis": 1593094752018/"end_time_in_millis": $body.responses.0.snapshots.0.end_time_in_millis/] +// TESTRESPONSE[s/"end_time_in_millis": 1593094752019/"end_time_in_millis": $body.responses.0.snapshots.1.end_time_in_millis/] +// TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.responses.0.snapshots.0.duration_in_millis/] +// TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.responses.0.snapshots.1.duration_in_millis/] + +A subsequent request for the remaining snapshots can then be made. + +[source,console] +---- +GET /_snapshot/my_repository/snapshot*?size=2&sort=name&after=snapshot_2,snapshot_2 +---- + +The API returns the following response: + +[source,console-result] +---- +{ + "responses": [ + { + "repository": "my_repository", + "snapshots": [ + { + "snapshot": "snapshot_3", + "uuid": "dRctdKb54xw67gvLCxSket", + "version_id": , + "version": , + "indices": [], + "data_streams": [], + "feature_states": [], + "include_global_state": true, + "state": "SUCCESS", + "start_time": "2020-07-06T21:55:18.129Z", + "start_time_in_millis": 1593093628850, + "end_time": "2020-07-06T21:55:18.129Z", + "end_time_in_millis": 1593094752018, + "duration_in_millis": 0, + "failures": [], + "shards": { + "total": 0, + "failed": 0, + "successful": 0 + } + } + ] + } + ] +} +---- +// TESTRESPONSE[s/"uuid": "dRctdKb54xw67gvLCxSket"/"uuid": $body.responses.0.snapshots.0.uuid/] +// TESTRESPONSE[s/"version_id": /"version_id": $body.responses.0.snapshots.0.version_id/] +// TESTRESPONSE[s/"version": /"version": $body.responses.0.snapshots.0.version/] +// TESTRESPONSE[s/"start_time": "2020-07-06T21:55:18.129Z"/"start_time": $body.responses.0.snapshots.0.start_time/] +// TESTRESPONSE[s/"start_time_in_millis": 1593093628850/"start_time_in_millis": $body.responses.0.snapshots.0.start_time_in_millis/] +// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.129Z"/"end_time": $body.responses.0.snapshots.0.end_time/] +// TESTRESPONSE[s/"end_time_in_millis": 1593094752018/"end_time_in_millis": $body.responses.0.snapshots.0.end_time_in_millis/] +// TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.responses.0.snapshots.0.duration_in_millis/] diff --git a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java index 72514beed8504..f1a31224e879a 100644 --- a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java +++ b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java @@ -20,6 +20,7 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.core.Tuple; import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase; import org.elasticsearch.snapshots.SnapshotInfo; @@ -106,24 +107,31 @@ private void doTestPagination(String repoName, GetSnapshotsRequest.SortBy sort, SortOrder order) throws IOException { final List allSnapshotsSorted = allSnapshotsSorted(names, repoName, sort, order); - final List batch1 = sortedWithLimit(repoName, sort, 2, order); - assertEquals(batch1, allSnapshotsSorted.subList(0, 2)); - final List batch2 = sortedWithLimit(repoName, sort, batch1.get(1), 2, order); - assertEquals(batch2, allSnapshotsSorted.subList(2, 4)); - final int lastBatch = names.size() - batch1.size() - batch2.size(); - final List batch3 = sortedWithLimit(repoName, sort, batch2.get(1), lastBatch, order); - assertEquals(batch3, allSnapshotsSorted.subList(batch1.size() + batch2.size(), names.size())); - final List batch3NoLimit = - sortedWithLimit(repoName, sort, batch2.get(1), GetSnapshotsRequest.NO_LIMIT, order); - assertEquals(batch3, batch3NoLimit); - final List batch3LargeLimit = sortedWithLimit( + final Tuple> batch1 = sortedWithLimit(repoName, sort, null, 2, order); + assertEquals(allSnapshotsSorted.subList(0, 2), batch1.v2()); + final Tuple> batch2 = sortedWithLimit(repoName, sort, batch1.v1(), 2, order); + assertEquals(allSnapshotsSorted.subList(2, 4), batch2.v2()); + final int lastBatch = names.size() - batch1.v2().size() - batch2.v2().size(); + final Tuple> batch3 = sortedWithLimit(repoName, sort, batch2.v1(), lastBatch, order); + assertEquals(batch3.v2(), allSnapshotsSorted.subList(batch1.v2().size() + batch2.v2().size(), names.size())); + final Tuple> batch3NoLimit = sortedWithLimit( repoName, sort, - batch2.get(1), + batch2.v1(), + GetSnapshotsRequest.NO_LIMIT, + order + ); + assertNull(batch3NoLimit.v1()); + assertEquals(batch3.v2(), batch3NoLimit.v2()); + final Tuple> batch3LargeLimit = sortedWithLimit( + repoName, + sort, + batch2.v1(), lastBatch + randomIntBetween(1, 100), order ); - assertEquals(batch3, batch3LargeLimit); + assertEquals(batch3.v2(), batch3LargeLimit.v2()); + assertNull(batch3LargeLimit.v1()); } public void testSortAndPaginateWithInProgress() throws Exception { @@ -173,14 +181,15 @@ private static void assertStablePagination(String repoName, final List allSorted = allSnapshotsSorted(allSnapshotNames, repoName, sort, order); for (int i = 1; i <= allSnapshotNames.size(); i++) { - final List subsetSorted = sortedWithLimit(repoName, sort, i, order); + final List subsetSorted = sortedWithLimit(repoName, sort, i, order).v2(); assertEquals(subsetSorted, allSorted.subList(0, i)); } for (int j = 0; j < allSnapshotNames.size(); j++) { final SnapshotInfo after = allSorted.get(j); for (int i = 1; i < allSnapshotNames.size() - j; i++) { - final List subsetSorted = sortedWithLimit(repoName, sort, after, i, order); + final List subsetSorted = sortedWithLimit( + repoName, sort, GetSnapshotsRequest.After.from(after, sort).asQueryParam(), i, order).v2(); assertEquals(subsetSorted, allSorted.subList(j + 1, j + i + 1)); } } @@ -196,7 +205,7 @@ private static List allSnapshotsSorted(Collection allSnaps request.addParameter("order", order.toString()); } final Response response = getRestClient().performRequest(request); - final List snapshotInfos = readSnapshotInfos(repoName, response); + final List snapshotInfos = readSnapshotInfos(repoName, response).v2(); assertEquals(snapshotInfos.size(), allSnapshotNames.size()); for (SnapshotInfo snapshotInfo : snapshotInfos) { assertThat(snapshotInfo.snapshotId().getName(), is(in(allSnapshotNames))); @@ -208,10 +217,19 @@ private static Request baseGetSnapshotsRequest(String repoName) { return new Request(HttpGet.METHOD_NAME, "/_snapshot/" + repoName + "/*"); } - private static List sortedWithLimit(String repoName, - GetSnapshotsRequest.SortBy sortBy, - int size, - SortOrder order) throws IOException { + private static Tuple> readSnapshotInfos(String repoName, Response response) throws IOException { + try (InputStream input = response.getEntity().getContent(); + XContentParser parser = JsonXContent.jsonXContent.createParser( + NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, input)) { + final GetSnapshotsResponse getSnapshotsResponse = GetSnapshotsResponse.fromXContent(parser); + return Tuple.tuple(getSnapshotsResponse.getNext(repoName), getSnapshotsResponse.getSnapshots(repoName)); + } + } + + private static Tuple> sortedWithLimit(String repoName, + GetSnapshotsRequest.SortBy sortBy, + int size, + SortOrder order) throws IOException { final Request request = baseGetSnapshotsRequest(repoName); request.addParameter("sort", sortBy.toString()); if (order == SortOrder.DESC || randomBoolean()) { @@ -222,28 +240,18 @@ private static List sortedWithLimit(String repoName, return readSnapshotInfos(repoName, response); } - private static List readSnapshotInfos(String repoName, Response response) throws IOException { - final List snapshotInfos; - try (InputStream input = response.getEntity().getContent(); - XContentParser parser = JsonXContent.jsonXContent.createParser( - NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, input)) { - snapshotInfos = GetSnapshotsResponse.fromXContent(parser).getSnapshots(repoName); - } - return snapshotInfos; - } - - private static List sortedWithLimit(String repoName, - GetSnapshotsRequest.SortBy sortBy, - SnapshotInfo after, - int size, - SortOrder order) throws IOException { + private static Tuple> sortedWithLimit(String repoName, + GetSnapshotsRequest.SortBy sortBy, + String after, + int size, + SortOrder order) throws IOException { final Request request = baseGetSnapshotsRequest(repoName); request.addParameter("sort", sortBy.toString()); if (size != GetSnapshotsRequest.NO_LIMIT || randomBoolean()) { request.addParameter("size", String.valueOf(size)); } if (after != null) { - request.addParameter("after", GetSnapshotsRequest.After.from(after, sortBy).value() + "," + after.snapshotId().getName()); + request.addParameter("after", after); } if (order == SortOrder.DESC || randomBoolean()) { request.addParameter("order", order.toString()); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java index 2c12224472a73..adca2be03e5aa 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/snapshots/GetSnapshotsIT.java @@ -13,7 +13,9 @@ import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotResponse; import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsRequest; import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsRequestBuilder; +import org.elasticsearch.action.admin.cluster.snapshots.get.GetSnapshotsResponse; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.Tuple; import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.threadpool.ThreadPool; @@ -95,23 +97,31 @@ public void testResponseSizeLimit() throws Exception { private void doTestPagination(String repoName, List names, GetSnapshotsRequest.SortBy sort, SortOrder order) { final List allSnapshotsSorted = allSnapshotsSorted(names, repoName, sort, order); - final List batch1 = sortedWithLimit(repoName, sort, null, 2, order); - assertEquals(batch1, allSnapshotsSorted.subList(0, 2)); - final List batch2 = sortedWithLimit(repoName, sort, batch1.get(1), 2, order); - assertEquals(batch2, allSnapshotsSorted.subList(2, 4)); - final int lastBatch = names.size() - batch1.size() - batch2.size(); - final List batch3 = sortedWithLimit(repoName, sort, batch2.get(1), lastBatch, order); - assertEquals(batch3, allSnapshotsSorted.subList(batch1.size() + batch2.size(), names.size())); - final List batch3NoLimit = sortedWithLimit(repoName, sort, batch2.get(1), GetSnapshotsRequest.NO_LIMIT, order); - assertEquals(batch3, batch3NoLimit); - final List batch3LargeLimit = sortedWithLimit( + final Tuple> batch1 = sortedWithLimit(repoName, sort, null, 2, order); + assertEquals(allSnapshotsSorted.subList(0, 2), batch1.v2()); + final Tuple> batch2 = sortedWithLimit(repoName, sort, batch1.v1(), 2, order); + assertEquals(allSnapshotsSorted.subList(2, 4), batch2.v2()); + final int lastBatch = names.size() - batch1.v2().size() - batch2.v2().size(); + final Tuple> batch3 = sortedWithLimit(repoName, sort, batch2.v1(), lastBatch, order); + assertEquals(batch3.v2(), allSnapshotsSorted.subList(batch1.v2().size() + batch2.v2().size(), names.size())); + final Tuple> batch3NoLimit = sortedWithLimit( repoName, sort, - batch2.get(1), + batch2.v1(), + GetSnapshotsRequest.NO_LIMIT, + order + ); + assertNull(batch3NoLimit.v1()); + assertEquals(batch3.v2(), batch3NoLimit.v2()); + final Tuple> batch3LargeLimit = sortedWithLimit( + repoName, + sort, + batch2.v1(), lastBatch + randomIntBetween(1, 100), order ); - assertEquals(batch3, batch3LargeLimit); + assertEquals(batch3.v2(), batch3LargeLimit.v2()); + assertNull(batch3LargeLimit.v1()); } public void testSortAndPaginateWithInProgress() throws Exception { @@ -177,14 +187,20 @@ private static void assertStablePagination(String repoName, Collection a final List allSorted = allSnapshotsSorted(allSnapshotNames, repoName, sort, order); for (int i = 1; i <= allSnapshotNames.size(); i++) { - final List subsetSorted = sortedWithLimit(repoName, sort, null, i, order); - assertEquals(subsetSorted, allSorted.subList(0, i)); + final Tuple> subsetSorted = sortedWithLimit(repoName, sort, null, i, order); + assertEquals(allSorted.subList(0, i), subsetSorted.v2()); } for (int j = 0; j < allSnapshotNames.size(); j++) { final SnapshotInfo after = allSorted.get(j); for (int i = 1; i < allSnapshotNames.size() - j; i++) { - final List subsetSorted = sortedWithLimit(repoName, sort, after, i, order); + final List subsetSorted = sortedWithLimit( + repoName, + sort, + GetSnapshotsRequest.After.from(after, sort).asQueryParam(), + i, + order + ).v2(); assertEquals(subsetSorted, allSorted.subList(j + 1, j + i + 1)); } } @@ -196,7 +212,7 @@ private static List allSnapshotsSorted( GetSnapshotsRequest.SortBy sortBy, SortOrder order ) { - final List snapshotInfos = sortedWithLimit(repoName, sortBy, null, GetSnapshotsRequest.NO_LIMIT, order); + final List snapshotInfos = sortedWithLimit(repoName, sortBy, null, GetSnapshotsRequest.NO_LIMIT, order).v2(); assertEquals(snapshotInfos.size(), allSnapshotNames.size()); for (SnapshotInfo snapshotInfo : snapshotInfos) { assertThat(snapshotInfo.snapshotId().getName(), is(in(allSnapshotNames))); @@ -204,14 +220,19 @@ private static List allSnapshotsSorted( return snapshotInfos; } - private static List sortedWithLimit( + private static Tuple> sortedWithLimit( String repoName, GetSnapshotsRequest.SortBy sortBy, - SnapshotInfo after, + String after, int size, SortOrder order ) { - return baseGetSnapshotsRequest(repoName).setAfter(after, sortBy).setSize(size).setOrder(order).get().getSnapshots(repoName); + final GetSnapshotsResponse response = baseGetSnapshotsRequest(repoName).setAfter(after) + .setSort(sortBy) + .setSize(size) + .setOrder(order) + .get(); + return Tuple.tuple(response.getNext(repoName), response.getSnapshots(repoName)); } private static GetSnapshotsRequestBuilder baseGetSnapshotsRequest(String repoName) { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java index 4f1abfe5b2b7c..b5dc2fa5b21d4 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java @@ -317,6 +317,16 @@ public static final class After implements Writeable { this(in.readString(), in.readString()); } + public static After fromParam(String param) { + final String[] parts = param.split(","); + if (parts.length != 2) { + throw new IllegalArgumentException( + "after param must be of the form ${sort_value},${snapshot_name} but was [" + param + "]" + ); + } + return new After(parts[0], parts[1]); + } + @Nullable public static After from(@Nullable SnapshotInfo snapshotInfo, SortBy sortBy) { if (snapshotInfo == null) { @@ -355,6 +365,10 @@ public String snapshotName() { return snapshotName; } + public String asQueryParam() { + return value + "," + snapshotName; + } + @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(value); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java index 2a4c9ee61bec1..3de6a763e0712 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java @@ -13,7 +13,6 @@ import org.elasticsearch.common.util.ArrayUtils; import org.elasticsearch.core.Nullable; import org.elasticsearch.search.sort.SortOrder; -import org.elasticsearch.snapshots.SnapshotInfo; /** * Get snapshots request builder @@ -98,8 +97,8 @@ public GetSnapshotsRequestBuilder setVerbose(boolean verbose) { return this; } - public GetSnapshotsRequestBuilder setAfter(@Nullable SnapshotInfo after, GetSnapshotsRequest.SortBy sortBy) { - return setAfter(GetSnapshotsRequest.After.from(after, sortBy)).setSort(sortBy); + public GetSnapshotsRequestBuilder setAfter(String after) { + return setAfter(after == null ? null : GetSnapshotsRequest.After.fromParam(after)); } public GetSnapshotsRequestBuilder setAfter(@Nullable GetSnapshotsRequest.After after) { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java index df7755acc9082..295c145410d7d 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java @@ -19,6 +19,7 @@ import org.elasticsearch.common.xcontent.ToXContentObject; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; +import org.elasticsearch.core.Nullable; import org.elasticsearch.snapshots.SnapshotInfo; import java.io.IOException; @@ -58,17 +59,23 @@ public GetSnapshotsResponse(StreamInput in) throws IOException { this.successfulResponses = Collections.singletonMap("unknown", in.readList(SnapshotInfo::readFrom)); this.failedResponses = Collections.emptyMap(); } + if (in.getVersion().onOrAfter(GetSnapshotsRequest.PAGINATED_GET_SNAPSHOTS_VERSION)) { + this.next = Collections.unmodifiableMap(in.readMap(StreamInput::readString, StreamInput::readString)); + } else { + this.next = Collections.emptyMap(); + } } public static class Response { private final String repository; private final List snapshots; + private final String next; private final ElasticsearchException error; private static final ConstructingObjectParser RESPONSE_PARSER = new ConstructingObjectParser<>( Response.class.getName(), true, - (args) -> new Response((String) args[0], (List) args[1], (ElasticsearchException) args[2]) + (args) -> new Response((String) args[0], (List) args[1], (ElasticsearchException) args[2], (String) args[3]) ); static { @@ -83,20 +90,22 @@ public static class Response { (p, c) -> ElasticsearchException.fromXContent(p), new ParseField("error") ); + RESPONSE_PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("next")); } - private Response(String repository, List snapshots, ElasticsearchException error) { + private Response(String repository, List snapshots, @Nullable ElasticsearchException error, @Nullable String next) { this.repository = repository; this.snapshots = snapshots; this.error = error; + this.next = next; } - public static Response snapshots(String repository, List snapshots) { - return new Response(repository, snapshots, null); + public static Response snapshots(String repository, List snapshots, @Nullable String next) { + return new Response(repository, snapshots, null, next); } public static Response error(String repository, ElasticsearchException error) { - return new Response(repository, null, error); + return new Response(repository, null, error, null); } private static Response fromXContent(XContentParser parser) throws IOException { @@ -105,21 +114,28 @@ private static Response fromXContent(XContentParser parser) throws IOException { } private final Map> successfulResponses; + private final Map next; private final Map failedResponses; public GetSnapshotsResponse(Collection responses) { Map> successfulResponses = new HashMap<>(); + Map next = new HashMap<>(); Map failedResponses = new HashMap<>(); for (Response response : responses) { if (response.snapshots != null) { assert response.error == null; successfulResponses.put(response.repository, response.snapshots); + if (response.next != null) { + next.put(response.repository, response.next); + } } else { assert response.snapshots == null; + assert response.next == null; failedResponses.put(response.repository, response.error); } } this.successfulResponses = Collections.unmodifiableMap(successfulResponses); + this.next = Map.copyOf(next); this.failedResponses = Collections.unmodifiableMap(failedResponses); } @@ -142,6 +158,18 @@ public List getSnapshots(String repo) { throw error; } + /** + * Pagination offset for the next request if the request for this response used a size limit and there may be additional snapshots to + * fetch. + * + * @param repo repository name + * @return pagination offset for more results if there might be any or {@code null} otherwise + */ + @Nullable + public String getNext(String repo) { + return next.get(repo); + } + /** * Returns list of repositories for both successful and unsuccessful responses. */ @@ -183,6 +211,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws snapshot.toXContent(builder, params); } builder.endArray(); + final String nextVal = next.get(snapshots.getKey()); + if (nextVal != null) { + builder.field("next", nextVal); + } builder.endObject(); } @@ -220,6 +252,16 @@ public void writeTo(StreamOutput out) throws IOException { throw failedResponses.values().iterator().next(); } } + if (out.getVersion().onOrAfter(GetSnapshotsRequest.PAGINATED_GET_SNAPSHOTS_VERSION)) { + out.writeMap(next, StreamOutput::writeString, StreamOutput::writeString); + } else { + if (next.isEmpty() == false) { + throw new IllegalArgumentException( + "Requesting paginated results is not supported in versions prior to " + + GetSnapshotsRequest.PAGINATED_GET_SNAPSHOTS_VERSION + ); + } + } } public static GetSnapshotsResponse fromXContent(XContentParser parser) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java index 7a605f21776bb..d73213b8c351e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java @@ -165,7 +165,16 @@ private void getMultipleReposSnapshotInfo( } else { groupedListener.onFailure(e); } - }).map(snInfos -> GetSnapshotsResponse.Response.snapshots(repoName, snInfos)) + }) + .map( + snInfos -> GetSnapshotsResponse.Response.snapshots( + repoName, + snInfos, + size == GetSnapshotsRequest.NO_LIMIT || snInfos.size() < size + ? null + : GetSnapshotsRequest.After.from(snInfos.get(snInfos.size() - 1), sortBy).asQueryParam() + ) + ) ); } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java index 4e81a52e0851a..167532a63ed67 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java @@ -107,7 +107,7 @@ private GetSnapshotsResponse createTestInstance() { for (int i = 0; i < randomIntBetween(0, 5); i++) { String repository = randomValueOtherThanMany(repositories::contains, () -> randomAlphaOfLength(10)); repositories.add(repository); - responses.add(GetSnapshotsResponse.Response.snapshots(repository, createSnapshotInfos())); + responses.add(GetSnapshotsResponse.Response.snapshots(repository, createSnapshotInfos(), null)); } for (int i = 0; i < randomIntBetween(0, 5); i++) { diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java index 5a397fcab44b3..c8293f898ff97 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/slm/SnapshotRetentionTaskTests.java @@ -314,7 +314,7 @@ void doExecute(ActionType action, Request request, ActionListener called"); listener.onResponse((Response) new GetSnapshotsResponse( - Collections.singleton(GetSnapshotsResponse.Response.snapshots(repoId, Collections.emptyList())))); + Collections.singleton(GetSnapshotsResponse.Response.snapshots(repoId, Collections.emptyList(), null)))); } else { super.doExecute(action, request, listener); } From 6f41fa652349a03320a6d941f97e2971670050f8 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 17 Jun 2021 14:55:31 +0200 Subject: [PATCH 02/10] cleanup docs --- .../reference/snapshot-restore/apis/get-snapshot-api.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index 5ddd35b14ae60..07273224bac05 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -276,7 +276,7 @@ The snapshot `state` can be one of the following values: -- `next`:: (string) -If the request contained a size limit and there might be more results a `next` field will be added to the response and can be used as the +If the request contained a size limit and there might be more results, a `next` field will be added to the response and can be used as the `after` query parameter to fetch additional results. [[get-snapshot-api-example]] @@ -416,7 +416,7 @@ The API returns the following response: // TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.responses.0.snapshots.0.duration_in_millis/] // TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.responses.0.snapshots.1.duration_in_millis/] -A subsequent request for the remaining snapshots can then be made. +A subsequent request for the remaining snapshots can then be made using the `next` value from the previous response as `after` parameter. [source,console] ---- From 622b25b5414851b84c8d364d1a9e0cb5e9b4c351 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 17 Jun 2021 15:23:41 +0200 Subject: [PATCH 03/10] duration isn't always 0 on ci --- .../reference/snapshot-restore/apis/get-snapshot-api.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index 07273224bac05..3d20bd0a93bad 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -387,7 +387,7 @@ The API returns the following response: "start_time_in_millis": 1593093628851, "end_time": "2020-07-06T21:55:18.130Z", "end_time_in_millis": 1593094752019, - "duration_in_millis": 0, + "duration_in_millis": 1, "failures": [], "shards": { "total": 0, @@ -414,7 +414,7 @@ The API returns the following response: // TESTRESPONSE[s/"end_time_in_millis": 1593094752018/"end_time_in_millis": $body.responses.0.snapshots.0.end_time_in_millis/] // TESTRESPONSE[s/"end_time_in_millis": 1593094752019/"end_time_in_millis": $body.responses.0.snapshots.1.end_time_in_millis/] // TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.responses.0.snapshots.0.duration_in_millis/] -// TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.responses.0.snapshots.1.duration_in_millis/] +// TESTRESPONSE[s/"duration_in_millis": 1/"duration_in_millis": $body.responses.0.snapshots.1.duration_in_millis/] A subsequent request for the remaining snapshots can then be made using the `next` value from the previous response as `after` parameter. From d3b362b18a4f6136f220ab528ef4f4a4e15da563 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Sun, 27 Jun 2021 12:51:23 +0200 Subject: [PATCH 04/10] fix test --- .../apis/get-snapshot-api.asciidoc | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index dfcf526e11671..dd9816fd16aa7 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -321,14 +321,14 @@ The API returns the following response: ] } ---- -// TESTRESPONSE[s/"uuid": "vdRctLCxSketdKb54xw67g"/"uuid": $body.responses.0.snapshots.0.uuid/] -// TESTRESPONSE[s/"version_id": /"version_id": $body.responses.0.snapshots.0.version_id/] -// TESTRESPONSE[s/"version": /"version": $body.responses.0.snapshots.0.version/] -// TESTRESPONSE[s/"start_time": "2020-07-06T21:55:18.129Z"/"start_time": $body.responses.0.snapshots.0.start_time/] -// TESTRESPONSE[s/"start_time_in_millis": 1593093628850/"start_time_in_millis": $body.responses.0.snapshots.0.start_time_in_millis/] -// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.129Z"/"end_time": $body.responses.0.snapshots.0.end_time/] -// TESTRESPONSE[s/"end_time_in_millis": 1593094752018/"end_time_in_millis": $body.responses.0.snapshots.0.end_time_in_millis/] -// TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.responses.0.snapshots.0.duration_in_millis/] +// TESTRESPONSE[s/"uuid": "vdRctLCxSketdKb54xw67g"/"uuid": $body.snapshots.0.uuid/] +// TESTRESPONSE[s/"version_id": /"version_id": $body.snapshots.0.version_id/] +// TESTRESPONSE[s/"version": /"version": $body.snapshots.0.version/] +// TESTRESPONSE[s/"start_time": "2020-07-06T21:55:18.129Z"/"start_time": $body.snapshots.0.start_time/] +// TESTRESPONSE[s/"start_time_in_millis": 1593093628850/"start_time_in_millis": $body.snapshots.0.start_time_in_millis/] +// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.129Z"/"end_time": $body.snapshots.0.end_time/] +// TESTRESPONSE[s/"end_time_in_millis": 1593094752018/"end_time_in_millis": $body.snapshots.0.end_time_in_millis/] +// TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.snapshots.0.duration_in_millis/] The following request returns information for all snapshots with prefix `snapshot` in the `my_repository` repository, limiting the response size to 2 and sorting by snapshot name. @@ -370,6 +370,7 @@ The API returns the following response: { "snapshot": "snapshot_2", "uuid": "vdRctLCxSketdKb54xw67g", + "repository": "my_repository", "version_id": , "version": , "indices": [], @@ -393,20 +394,20 @@ The API returns the following response: "next": "snapshot_2,snapshot_2" } ---- -// TESTRESPONSE[s/"uuid": "dKb54xw67gvdRctLCxSket"/"uuid": $body.responses.0.snapshots.0.uuid/] -// TESTRESPONSE[s/"uuid": "vdRctLCxSketdKb54xw67g"/"uuid": $body.responses.0.snapshots.1.uuid/] -// TESTRESPONSE[s/"version_id": /"version_id": $body.responses.0.snapshots.0.version_id/] -// TESTRESPONSE[s/"version": /"version": $body.responses.0.snapshots.0.version/] -// TESTRESPONSE[s/"start_time": "2020-07-06T21:55:18.129Z"/"start_time": $body.responses.0.snapshots.0.start_time/] -// TESTRESPONSE[s/"start_time": "2020-07-06T21:55:18.130Z"/"start_time": $body.responses.0.snapshots.1.start_time/] -// TESTRESPONSE[s/"start_time_in_millis": 1593093628850/"start_time_in_millis": $body.responses.0.snapshots.0.start_time_in_millis/] -// TESTRESPONSE[s/"start_time_in_millis": 1593093628851/"start_time_in_millis": $body.responses.0.snapshots.1.start_time_in_millis/] -// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.129Z"/"end_time": $body.responses.0.snapshots.0.end_time/] -// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.130Z"/"end_time": $body.responses.0.snapshots.1.end_time/] -// TESTRESPONSE[s/"end_time_in_millis": 1593094752018/"end_time_in_millis": $body.responses.0.snapshots.0.end_time_in_millis/] -// TESTRESPONSE[s/"end_time_in_millis": 1593094752019/"end_time_in_millis": $body.responses.0.snapshots.1.end_time_in_millis/] -// TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.responses.0.snapshots.0.duration_in_millis/] -// TESTRESPONSE[s/"duration_in_millis": 1/"duration_in_millis": $body.responses.0.snapshots.1.duration_in_millis/] +// TESTRESPONSE[s/"uuid": "dKb54xw67gvdRctLCxSket"/"uuid": $body.snapshots.0.uuid/] +// TESTRESPONSE[s/"uuid": "vdRctLCxSketdKb54xw67g"/"uuid": $body.snapshots.1.uuid/] +// TESTRESPONSE[s/"version_id": /"version_id": $body.snapshots.0.version_id/] +// TESTRESPONSE[s/"version": /"version": $body.snapshots.0.version/] +// TESTRESPONSE[s/"start_time": "2020-07-06T21:55:18.129Z"/"start_time": $body.snapshots.0.start_time/] +// TESTRESPONSE[s/"start_time": "2020-07-06T21:55:18.130Z"/"start_time": $body.snapshots.1.start_time/] +// TESTRESPONSE[s/"start_time_in_millis": 1593093628850/"start_time_in_millis": $body.snapshots.0.start_time_in_millis/] +// TESTRESPONSE[s/"start_time_in_millis": 1593093628851/"start_time_in_millis": $body.snapshots.1.start_time_in_millis/] +// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.129Z"/"end_time": $body.snapshots.0.end_time/] +// TESTRESPONSE[s/"end_time": "2020-07-06T21:55:18.130Z"/"end_time": $body.snapshots.1.end_time/] +// TESTRESPONSE[s/"end_time_in_millis": 1593094752018/"end_time_in_millis": $body.snapshots.0.end_time_in_millis/] +// TESTRESPONSE[s/"end_time_in_millis": 1593094752019/"end_time_in_millis": $body.snapshots.1.end_time_in_millis/] +// TESTRESPONSE[s/"duration_in_millis": 0/"duration_in_millis": $body.snapshots.0.duration_in_millis/] +// TESTRESPONSE[s/"duration_in_millis": 1/"duration_in_millis": $body.snapshots.1.duration_in_millis/] A subsequent request for the remaining snapshots can then be made using the `next` value from the previous response as `after` parameter. From 332cbfbc037f00a7df3a2051f01134b19261ccf9 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 28 Jun 2021 09:24:21 +0200 Subject: [PATCH 05/10] next field correct in all cases --- .../get/TransportGetSnapshotsAction.java | 49 ++++++++++++------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java index 2842ce4b2d5f7..adc7712f84467 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java @@ -143,25 +143,26 @@ private void getMultipleReposSnapshotInfo( listener.onResponse(new GetSnapshotsResponse(Collections.emptyList(), Collections.emptyMap(), null)); return; } - final GroupedActionListener, List>> groupedActionListener = + final GroupedActionListener, SnapshotsInRepo>> groupedActionListener = new GroupedActionListener<>(listener.map(responses -> { assert repos.size() == responses.size(); final List allSnapshots = responses.stream() .map(Tuple::v2) .filter(Objects::nonNull) - .flatMap(Collection::stream) + .flatMap(snapshotsInRepo -> snapshotsInRepo.snapshotInfos.stream()) .collect(Collectors.toUnmodifiableList()); final Map failures = responses.stream() .map(Tuple::v1) .filter(Objects::nonNull) .collect(Collectors.toMap(Tuple::v1, Tuple::v2)); - final List snInfos = sortSnapshots(allSnapshots, sortBy, after, size, order); + final SnapshotsInRepo snInfos = sortSnapshots(allSnapshots, sortBy, after, size, order); + final List snapshotInfos = snInfos.snapshotInfos; return new GetSnapshotsResponse( - snInfos, + snapshotInfos, failures, - size == GetSnapshotsRequest.NO_LIMIT || snInfos.size() < size - ? null - : GetSnapshotsRequest.After.from(snInfos.get(snInfos.size() - 1), sortBy).asQueryParam() + snInfos.hasMore || responses.stream().anyMatch(r -> r.v2() != null && r.v2().hasMore) + ? GetSnapshotsRequest.After.from(snapshotInfos.get(snapshotInfos.size() - 1), sortBy).asQueryParam() + : null ); }), repos.size()); @@ -200,11 +201,11 @@ private void getSingleRepoSnapshotInfo( @Nullable final GetSnapshotsRequest.After after, int size, SortOrder order, - ActionListener> listener + ActionListener listener ) { final Map allSnapshotIds = new HashMap<>(); final List currentSnapshots = new ArrayList<>(); - for (SnapshotInfo snapshotInfo : sortedCurrentSnapshots(snapshotsInProgress, repo, sortBy, after, size, order)) { + for (SnapshotInfo snapshotInfo : sortedCurrentSnapshots(snapshotsInProgress, repo, sortBy, after, size, order).snapshotInfos) { Snapshot snapshot = snapshotInfo.snapshot(); allSnapshotIds.put(snapshot.getSnapshotId().getName(), snapshot); currentSnapshots.add(snapshotInfo); @@ -245,7 +246,7 @@ private void getSingleRepoSnapshotInfo( * @param repositoryName repository name * @return list of snapshots */ - private static List sortedCurrentSnapshots( + private static SnapshotsInRepo sortedCurrentSnapshots( SnapshotsInProgress snapshotsInProgress, String repositoryName, GetSnapshotsRequest.SortBy sortBy, @@ -279,7 +280,7 @@ private void loadSnapshotInfos( @Nullable final GetSnapshotsRequest.After after, int size, SortOrder order, - ActionListener> listener + ActionListener listener ) { if (task.isCancelled()) { listener.onFailure(new TaskCancelledException("task cancelled")); @@ -333,7 +334,7 @@ private void loadSnapshotInfos( listener ); } else { - final List snapshotInfos; + final SnapshotsInRepo snapshotInfos; if (repositoryData != null) { // want non-current snapshots as well, which are found in the repository data snapshotInfos = buildSimpleSnapshotInfos(toResolve, repo, repositoryData, currentSnapshots, sortBy, after, size, order); @@ -368,7 +369,7 @@ private void snapshots( @Nullable GetSnapshotsRequest.After after, int size, SortOrder order, - ActionListener> listener + ActionListener listener ) { if (task.isCancelled()) { listener.onFailure(new TaskCancelledException("task cancelled")); @@ -440,7 +441,7 @@ private boolean isCurrentSnapshotsOnly(String[] snapshots) { return (snapshots.length == 1 && GetSnapshotsRequest.CURRENT_SNAPSHOT.equalsIgnoreCase(snapshots[0])); } - private static List buildSimpleSnapshotInfos( + private static SnapshotsInRepo buildSimpleSnapshotInfos( final Set toResolve, final String repoName, final RepositoryData repositoryData, @@ -492,7 +493,7 @@ private static List buildSimpleSnapshotInfos( private static final Comparator BY_NAME = Comparator.comparing(sni -> sni.snapshotId().getName()); - private static List sortSnapshots( + private static SnapshotsInRepo sortSnapshots( List snapshotInfos, GetSnapshotsRequest.SortBy sortBy, @Nullable GetSnapshotsRequest.After after, @@ -542,9 +543,11 @@ private static List sortSnapshots( } infos = infos.sorted(order == SortOrder.DESC ? comparator.reversed() : comparator); if (size != GetSnapshotsRequest.NO_LIMIT) { - infos = infos.limit(size); + infos = infos.limit(size + 1); } - return infos.collect(Collectors.toUnmodifiableList()); + final List snapshots = infos.collect(Collectors.toUnmodifiableList()); + boolean hasMore = size != GetSnapshotsRequest.NO_LIMIT && size < snapshots.size(); + return new SnapshotsInRepo(hasMore ? snapshots.subList(0, size) : snapshots, hasMore); } private static Predicate filterByLongOffset( @@ -565,4 +568,16 @@ private static Predicate filterByLongOffset( private static int compareName(String name, SnapshotInfo info) { return name.compareTo(info.snapshotId().getName()); } + + private static final class SnapshotsInRepo { + + private final boolean hasMore; + + private final List snapshotInfos; + + SnapshotsInRepo(List snapshotInfos, boolean hasMore) { + this.hasMore = hasMore; + this.snapshotInfos = snapshotInfos; + } + } } From 8a829d171408956404f07fd728ddb9abd03a521d Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 28 Jun 2021 15:20:05 +0200 Subject: [PATCH 06/10] base64 encode param --- .../snapshot-restore/apis/get-snapshot-api.asciidoc | 4 ++-- .../cluster/snapshots/get/GetSnapshotsRequest.java | 8 +++++--- .../admin/cluster/RestGetSnapshotsAction.java | 13 +++---------- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index dd9816fd16aa7..0acdcb45abd56 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -391,7 +391,7 @@ The API returns the following response: } } ], - "next": "snapshot_2,snapshot_2" + "next": "c25hcHNob3RfMixzbmFwc2hvdF8y" } ---- // TESTRESPONSE[s/"uuid": "dKb54xw67gvdRctLCxSket"/"uuid": $body.snapshots.0.uuid/] @@ -413,7 +413,7 @@ A subsequent request for the remaining snapshots can then be made using the `nex [source,console] ---- -GET /_snapshot/my_repository/snapshot*?size=2&sort=name&after=snapshot_2,snapshot_2 +GET /_snapshot/my_repository/snapshot*?size=2&sort=name&after=c25hcHNob3RfMixzbmFwc2hvdF8y ---- The API returns the following response: diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java index 3207457d7ee71..97be29314ee94 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java @@ -24,6 +24,8 @@ import org.elasticsearch.tasks.TaskId; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Base64; import java.util.Map; import static org.elasticsearch.action.ValidateActions.addValidationError; @@ -326,10 +328,10 @@ public static final class After implements Writeable { } public static After fromParam(String param) { - final String[] parts = param.split(","); + final String[] parts = new String(Base64.getDecoder().decode(param), StandardCharsets.UTF_8).split(","); if (parts.length != 2) { throw new IllegalArgumentException( - "after param must be of the form ${sort_value},${snapshot_name} but was [" + param + "]" + "after param must be base64 encoded and of the form ${sort_value},${snapshot_name} but was [" + param + "]" ); } return new After(parts[0], parts[1]); @@ -374,7 +376,7 @@ public String snapshotName() { } public String asQueryParam() { - return value + "," + snapshotName; + return Base64.getEncoder().encodeToString((value + "," + snapshotName).getBytes(StandardCharsets.UTF_8)); } @Override diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java index e5548d80d072d..1c2a46aecb9aa 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java @@ -58,17 +58,10 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC getSnapshotsRequest.sort(sort); final int size = request.paramAsInt("size", getSnapshotsRequest.size()); getSnapshotsRequest.size(size); - final String[] afterString = request.paramAsStringArray("after", Strings.EMPTY_ARRAY); - final GetSnapshotsRequest.After after; - if (afterString.length == 0) { - after = null; - } else if (afterString.length == 2) { - after = new GetSnapshotsRequest.After(afterString[0], afterString[1]); - } else { - throw new IllegalArgumentException("illegal ?after value [" + Strings.arrayToCommaDelimitedString(afterString) + - "] must be of the form '${sort_value},${snapshot_name}'"); + final String afterString = request.param("after"); + if (afterString != null) { + getSnapshotsRequest.after(GetSnapshotsRequest.After.fromParam(afterString)); } - getSnapshotsRequest.after(after); final SortOrder order = SortOrder.fromString(request.param("order", getSnapshotsRequest.order().toString())); getSnapshotsRequest.order(order); getSnapshotsRequest.masterNodeTimeout(request.paramAsTime("master_timeout", getSnapshotsRequest.masterNodeTimeout())); From 59ac817aded53af9c43e94dbdd3975c96823a7ba Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 28 Jun 2021 16:03:05 +0200 Subject: [PATCH 07/10] docs + repo tiebraker --- .../apis/get-snapshot-api.asciidoc | 4 ++ .../http/snapshots/RestGetSnapshotsIT.java | 24 +++-------- .../snapshots/get/GetSnapshotsRequest.java | 24 +++++++---- .../snapshots/get/GetSnapshotsResponse.java | 4 +- .../get/TransportGetSnapshotsAction.java | 41 ++++++++++++++----- .../get/GetSnapshotsRequestTests.java | 2 +- .../get/GetSnapshotsResponseTests.java | 15 ++++++- 7 files changed, 74 insertions(+), 40 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index 0acdcb45abd56..91c1dbca92d25 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -134,6 +134,10 @@ Sort order. Valid values are `asc` for ascending and `desc` for descending order (Optional, string) Offset identifier to start pagination from as returned by the `next` field in the response body. +NOTE: The `after` parameter and `next` field allow for iterating through snapshots with some consistency guarantees regarding concurrent +creation or deletion of snapshots. It is guaranteed that any snapshot that exists at the beginning of the iteration and not concurrently +deleted will be seen during the iteration. Snapshots concurrently created may be seen during an iteration. + NOTE: The pagination parameters `size`, `order`, `after` and `sort` are not supported when using `verbose=false` and the sort order for requests with `verbose=false` is undefined. diff --git a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java index f293305b886ff..95496e3e537c6 100644 --- a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java +++ b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/snapshots/RestGetSnapshotsIT.java @@ -180,7 +180,7 @@ private static void assertStablePagination(String repoName, final List allSorted = allSnapshotsSorted(allSnapshotNames, repoName, sort, order); for (int i = 1; i <= allSnapshotNames.size(); i++) { - final List subsetSorted = sortedWithLimit(repoName, sort, i, order).v2(); + final List subsetSorted = sortedWithLimit(repoName, sort, null, i, order).v2(); assertEquals(subsetSorted, allSorted.subList(0, i)); } @@ -216,20 +216,6 @@ private static Request baseGetSnapshotsRequest(String repoName) { return new Request(HttpGet.METHOD_NAME, "/_snapshot/" + repoName + "/*"); } - private static Tuple> sortedWithLimit(String repoName, - GetSnapshotsRequest.SortBy sortBy, - int size, - SortOrder order) throws IOException { - final Request request = baseGetSnapshotsRequest(repoName); - request.addParameter("sort", sortBy.toString()); - if (order == SortOrder.DESC || randomBoolean()) { - request.addParameter("order", order.toString()); - } - request.addParameter("size", String.valueOf(size)); - final Response response = getRestClient().performRequest(request); - return readSnapshotInfos(response); - } - private static Tuple> readSnapshotInfos(Response response) throws IOException { try (InputStream input = response.getEntity().getContent(); XContentParser parser = JsonXContent.jsonXContent.createParser( @@ -240,10 +226,10 @@ private static Tuple> readSnapshotInfos(Response resp } private static Tuple> sortedWithLimit(String repoName, - GetSnapshotsRequest.SortBy sortBy, - String after, - int size, - SortOrder order) throws IOException { + GetSnapshotsRequest.SortBy sortBy, + String after, + int size, + SortOrder order) throws IOException { final Request request = baseGetSnapshotsRequest(repoName); request.addParameter("sort", sortBy.toString()); if (size != GetSnapshotsRequest.NO_LIMIT || randomBoolean()) { diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java index 97be29314ee94..59a9670abedb6 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java @@ -321,20 +321,24 @@ public static final class After implements Writeable { private final String value; + private final String repoName; + private final String snapshotName; After(StreamInput in) throws IOException { - this(in.readString(), in.readString()); + this(in.readString(), in.readString(), in.readString()); } public static After fromParam(String param) { final String[] parts = new String(Base64.getDecoder().decode(param), StandardCharsets.UTF_8).split(","); - if (parts.length != 2) { + if (parts.length != 3) { throw new IllegalArgumentException( - "after param must be base64 encoded and of the form ${sort_value},${snapshot_name} but was [" + param + "]" + "after param must be base64 encoded and of the form ${sort_value},${repository_name},${snapshot_name} but was [" + + param + + "]" ); } - return new After(parts[0], parts[1]); + return new After(parts[0], parts[1], parts[2]); } @Nullable @@ -359,11 +363,12 @@ public static After from(@Nullable SnapshotInfo snapshotInfo, SortBy sortBy) { default: throw new AssertionError("unknown sort column [" + sortBy + "]"); } - return new After(afterValue, snapshotInfo.snapshotId().getName()); + return new After(afterValue, snapshotInfo.repository(), snapshotInfo.snapshotId().getName()); } - public After(String value, String snapshotName) { + public After(String value, String repoName, String snapshotName) { this.value = value; + this.repoName = repoName; this.snapshotName = snapshotName; } @@ -375,13 +380,18 @@ public String snapshotName() { return snapshotName; } + public String repoName() { + return repoName; + } + public String asQueryParam() { - return Base64.getEncoder().encodeToString((value + "," + snapshotName).getBytes(StandardCharsets.UTF_8)); + return Base64.getEncoder().encodeToString((value + "," + repoName + "," + snapshotName).getBytes(StandardCharsets.UTF_8)); } @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(value); + out.writeString(repoName); out.writeString(snapshotName); } } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java index 91fa38932f383..247a1c878258e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponse.java @@ -158,12 +158,12 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; GetSnapshotsResponse that = (GetSnapshotsResponse) o; - return Objects.equals(snapshots, that.snapshots) && Objects.equals(failures, that.failures); + return Objects.equals(snapshots, that.snapshots) && Objects.equals(failures, that.failures) && Objects.equals(next, that.next); } @Override public int hashCode() { - return Objects.hash(snapshots, failures); + return Objects.hash(snapshots, failures, next); } @Override diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java index adc7712f84467..2bddc90a881a2 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/TransportGetSnapshotsAction.java @@ -522,19 +522,34 @@ private static SnapshotsInRepo sortSnapshots( if (after != null) { final Predicate isAfter; - final String name = after.snapshotName(); + final String snapshotName = after.snapshotName(); + final String repoName = after.repoName(); switch (sortBy) { case START_TIME: - isAfter = filterByLongOffset(SnapshotInfo::startTime, Long.parseLong(after.value()), name, order); + isAfter = filterByLongOffset(SnapshotInfo::startTime, Long.parseLong(after.value()), snapshotName, repoName, order); break; case NAME: - isAfter = order == SortOrder.ASC ? (info -> compareName(name, info) < 0) : (info -> compareName(name, info) > 0); + isAfter = order == SortOrder.ASC + ? (info -> compareName(snapshotName, repoName, info) < 0) + : (info -> compareName(snapshotName, repoName, info) > 0); break; case DURATION: - isAfter = filterByLongOffset(info -> info.endTime() - info.startTime(), Long.parseLong(after.value()), name, order); + isAfter = filterByLongOffset( + info -> info.endTime() - info.startTime(), + Long.parseLong(after.value()), + snapshotName, + repoName, + order + ); break; case INDICES: - isAfter = filterByLongOffset(info -> info.indices().size(), Integer.parseInt(after.value()), name, order); + isAfter = filterByLongOffset( + info -> info.indices().size(), + Integer.parseInt(after.value()), + snapshotName, + repoName, + order + ); break; default: throw new AssertionError("unexpected sort column [" + sortBy + "]"); @@ -553,20 +568,26 @@ private static SnapshotsInRepo sortSnapshots( private static Predicate filterByLongOffset( ToLongFunction extractor, long after, - String name, + String snapshotName, + String repoName, SortOrder order ) { return order == SortOrder.ASC ? info -> { final long val = extractor.applyAsLong(info); - return after < val || (after == val && compareName(name, info) < 0); + + return after < val || (after == val && compareName(snapshotName, repoName, info) < 0); } : info -> { final long val = extractor.applyAsLong(info); - return after > val || (after == val && compareName(name, info) > 0); + return after > val || (after == val && compareName(snapshotName, repoName, info) > 0); }; } - private static int compareName(String name, SnapshotInfo info) { - return name.compareTo(info.snapshotId().getName()); + private static int compareName(String name, String repoName, SnapshotInfo info) { + final int res = name.compareTo(info.snapshotId().getName()); + if (res != 0) { + return res; + } + return repoName.compareTo(info.repository()); } private static final class SnapshotsInRepo { diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java index 4bed70c153079..03fe362f2ee39 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestTests.java @@ -45,7 +45,7 @@ public void testValidateParameters() { } { final GetSnapshotsRequest request = new GetSnapshotsRequest("repo", "snapshot").verbose(false) - .after(new GetSnapshotsRequest.After("foo", "bar")); + .after(new GetSnapshotsRequest.After("foo", "repo", "bar")); final ActionRequestValidationException e = request.validate(); assertThat(e.getMessage(), containsString("can't use after with verbose=false")); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java index 66d172a1ba4c1..474f16a546da4 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java @@ -26,8 +26,10 @@ import org.elasticsearch.test.ESTestCase; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; +import java.util.Base64; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -65,6 +67,7 @@ private GetSnapshotsResponse copyInstance(GetSnapshotsResponse instance) throws private void assertEqualInstances(GetSnapshotsResponse expectedInstance, GetSnapshotsResponse newInstance) { assertEquals(expectedInstance.getSnapshots(), newInstance.getSnapshots()); + assertEquals(expectedInstance.next(), newInstance.next()); assertEquals(expectedInstance.getFailures().keySet(), newInstance.getFailures().keySet()); for (Map.Entry expectedEntry : expectedInstance.getFailures().entrySet()) { ElasticsearchException expectedException = expectedEntry.getValue(); @@ -119,7 +122,17 @@ private GetSnapshotsResponse createTestInstance() { failures.put(repository, new ElasticsearchException(randomAlphaOfLength(10))); } - return new GetSnapshotsResponse(responses, failures, null); + return new GetSnapshotsResponse( + responses, + failures, + randomBoolean() + ? Base64.getEncoder() + .encodeToString( + (randomAlphaOfLengthBetween(1, 5) + "," + randomAlphaOfLengthBetween(1, 5) + "," + randomAlphaOfLengthBetween(1, 5)) + .getBytes(StandardCharsets.UTF_8) + ) + : null + ); } public void testSerialization() throws IOException { From 27cf07eaba176d998a68f24df62bcfd006482c02 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 28 Jun 2021 16:25:55 +0200 Subject: [PATCH 08/10] fix param doc stest --- .../reference/snapshot-restore/apis/get-snapshot-api.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc index 91c1dbca92d25..4ed67e3cc14d6 100644 --- a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc +++ b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc @@ -395,7 +395,7 @@ The API returns the following response: } } ], - "next": "c25hcHNob3RfMixzbmFwc2hvdF8y" + "next": "c25hcHNob3RfMixteV9yZXBvc2l0b3J5LHNuYXBzaG90XzI=" } ---- // TESTRESPONSE[s/"uuid": "dKb54xw67gvdRctLCxSket"/"uuid": $body.snapshots.0.uuid/] @@ -417,7 +417,7 @@ A subsequent request for the remaining snapshots can then be made using the `nex [source,console] ---- -GET /_snapshot/my_repository/snapshot*?size=2&sort=name&after=c25hcHNob3RfMixzbmFwc2hvdF8y +GET /_snapshot/my_repository/snapshot*?size=2&sort=name&after=c25hcHNob3RfMixteV9yZXBvc2l0b3J5LHNuYXBzaG90XzI= ---- The API returns the following response: From 72f29fedd9a6d70c21a2df33d8aff3b76acae303 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 28 Jun 2021 17:16:20 +0200 Subject: [PATCH 09/10] URL-encode base 64 --- .../cluster/snapshots/get/GetSnapshotsRequest.java | 10 +++------- .../snapshots/get/GetSnapshotsResponseTests.java | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java index 59a9670abedb6..07f580599bce6 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java @@ -330,13 +330,9 @@ public static final class After implements Writeable { } public static After fromParam(String param) { - final String[] parts = new String(Base64.getDecoder().decode(param), StandardCharsets.UTF_8).split(","); + final String[] parts = new String(Base64.getUrlDecoder().decode(param), StandardCharsets.UTF_8).split(","); if (parts.length != 3) { - throw new IllegalArgumentException( - "after param must be base64 encoded and of the form ${sort_value},${repository_name},${snapshot_name} but was [" - + param - + "]" - ); + throw new IllegalArgumentException("invalid ?after parameter [" + param + "]"); } return new After(parts[0], parts[1], parts[2]); } @@ -385,7 +381,7 @@ public String repoName() { } public String asQueryParam() { - return Base64.getEncoder().encodeToString((value + "," + repoName + "," + snapshotName).getBytes(StandardCharsets.UTF_8)); + return Base64.getUrlEncoder().encodeToString((value + "," + repoName + "," + snapshotName).getBytes(StandardCharsets.UTF_8)); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java index 474f16a546da4..97a49ec939ac9 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsResponseTests.java @@ -126,7 +126,7 @@ private GetSnapshotsResponse createTestInstance() { responses, failures, randomBoolean() - ? Base64.getEncoder() + ? Base64.getUrlEncoder() .encodeToString( (randomAlphaOfLengthBetween(1, 5) + "," + randomAlphaOfLengthBetween(1, 5) + "," + randomAlphaOfLengthBetween(1, 5)) .getBytes(StandardCharsets.UTF_8) From 497333c427be597337a5f837b11823c4a21003a5 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 28 Jun 2021 21:23:20 +0200 Subject: [PATCH 10/10] rename --- .../action/admin/cluster/snapshots/get/GetSnapshotsRequest.java | 2 +- .../admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java | 2 +- .../rest/action/admin/cluster/RestGetSnapshotsAction.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java index 07f580599bce6..e0b51ea27cbc2 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java @@ -329,7 +329,7 @@ public static final class After implements Writeable { this(in.readString(), in.readString(), in.readString()); } - public static After fromParam(String param) { + public static After fromQueryParam(String param) { final String[] parts = new String(Base64.getUrlDecoder().decode(param), StandardCharsets.UTF_8).split(","); if (parts.length != 3) { throw new IllegalArgumentException("invalid ?after parameter [" + param + "]"); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java index 3de6a763e0712..f6b988de4504b 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java @@ -98,7 +98,7 @@ public GetSnapshotsRequestBuilder setVerbose(boolean verbose) { } public GetSnapshotsRequestBuilder setAfter(String after) { - return setAfter(after == null ? null : GetSnapshotsRequest.After.fromParam(after)); + return setAfter(after == null ? null : GetSnapshotsRequest.After.fromQueryParam(after)); } public GetSnapshotsRequestBuilder setAfter(@Nullable GetSnapshotsRequest.After after) { diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java index 1c2a46aecb9aa..f63b75915f41b 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestGetSnapshotsAction.java @@ -60,7 +60,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC getSnapshotsRequest.size(size); final String afterString = request.param("after"); if (afterString != null) { - getSnapshotsRequest.after(GetSnapshotsRequest.After.fromParam(afterString)); + getSnapshotsRequest.after(GetSnapshotsRequest.After.fromQueryParam(afterString)); } final SortOrder order = SortOrder.fromString(request.param("order", getSnapshotsRequest.order().toString())); getSnapshotsRequest.order(order);