Flatten Get Snapshots Response (#74451)

This PR returns the get snapshots API to the 7.x format (and transport client behavior) and enhances it for requests that ask for multiple repositories. The changes for requests that target multiple repositories are: * Add `repository` field to `SnapshotInfo` and REST response * Add `failures` map alongside `snapshots` list instead of returning just an exception response as done for single repo requests * Pagination now works across repositories instead of being per repository for multi-repository requests closes #69108 closes #43462
elastic · Jun 24, 2021 · cbf48e0 · cbf48e0
1 parent c37184c
commit cbf48e0
Show file tree

Hide file tree

Showing 71 changed files with 640 additions and 757 deletions.
diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java
@@ -35,12 +35,14 @@
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.snapshots.AbstractSnapshotIntegTestCase;
 import org.elasticsearch.snapshots.RestoreInfo;
+import org.elasticsearch.snapshots.SnapshotInfo;
 import org.mockito.internal.util.collections.Sets;
 
 import java.io.IOException;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 import static org.elasticsearch.snapshots.SnapshotsService.NO_FEATURE_STATES_VALUE;
 import static org.elasticsearch.tasks.TaskResultsService.TASKS_FEATURE_NAME;
@@ -209,14 +211,17 @@ public void testGetSnapshots() throws IOException {
         GetSnapshotsResponse response = execute(request, highLevelClient().snapshot()::get, highLevelClient().snapshot()::getAsync);
 
         assertThat(response.isFailed(), is(false));
-        assertThat(response.getRepositories(), equalTo(Sets.newSet(repository1, repository2)));
-
-        assertThat(response.getSnapshots(repository1), hasSize(1));
-        assertThat(response.getSnapshots(repository1).get(0).snapshotId().getName(), equalTo(snapshot1));
+        assertEquals(
+            Sets.newSet(repository1, repository2),
+            response.getSnapshots().stream().map(SnapshotInfo::repository).collect(Collectors.toSet())
+        );
 
-        assertThat(response.getSnapshots(repository2), hasSize(1));
-        assertThat(response.getSnapshots(repository2).get(0).snapshotId().getName(), equalTo(snapshot2));
-        assertThat(response.getSnapshots(repository2).get(0).userMetadata(), equalTo(originalMetadata));
+        assertThat(response.getSnapshots(), hasSize(2));
+        assertThat(response.getSnapshots().get(0).snapshotId().getName(), equalTo(snapshot1));
+        assertThat(response.getSnapshots().get(0).repository(), equalTo(repository1));
+        assertThat(response.getSnapshots().get(1).snapshotId().getName(), equalTo(snapshot2));
+        assertThat(response.getSnapshots().get(1).userMetadata(), equalTo(originalMetadata));
+        assertThat(response.getSnapshots().get(1).repository(), equalTo(repository2));
     }
 
 

diff --git a/...t-high-level/src/test/java/org/elasticsearch/client/documentation/ILMDocumentationIT.java b/...t-high-level/src/test/java/org/elasticsearch/client/documentation/ILMDocumentationIT.java
@@ -1044,7 +1044,7 @@ private void assertSnapshotExists(final RestHighLevelClient client, final String
             GetSnapshotsRequest getSnapshotsRequest = new GetSnapshotsRequest(new String[]{repo}, new String[]{snapshotName});
             try {
                 final GetSnapshotsResponse snaps = client.snapshot().get(getSnapshotsRequest, RequestOptions.DEFAULT);
-                Optional<SnapshotInfo> info = snaps.getSnapshots(repo).stream().findFirst();
+                Optional<SnapshotInfo> info = snaps.getSnapshots().stream().findFirst();
                 if (info.isPresent()) {
                     info.ifPresent(si -> {
                         assertThat(si.snapshotId().getName(), equalTo(snapshotName));

diff --git a/...l/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java b/...l/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java
@@ -617,7 +617,7 @@ public void testSnapshotGetSnapshots() throws IOException {
         // end::get-snapshots-execute
 
         // tag::get-snapshots-response
-        List<SnapshotInfo> snapshotsInfos = response.getSnapshots(repositoryName);
+        List<SnapshotInfo> snapshotsInfos = response.getSnapshots();
         SnapshotInfo snapshotInfo = snapshotsInfos.get(0);
         RestStatus restStatus = snapshotInfo.status(); // <1>
         SnapshotId snapshotId = snapshotInfo.snapshotId(); // <2>

diff --git a/docs/reference/migration/migrate_8_0/snapshots.asciidoc b/docs/reference/migration/migrate_8_0/snapshots.asciidoc
@@ -9,64 +9,6 @@
 
 // end::notable-breaking-changes[]
 
-.The get snapshot API's response format has changed.
-[%collapsible]
-====
-*Details* +
-It's possible to get snapshots from multiple repositories in one go. The response format has changed
-and now contains separate response for each repository.
-
-For example, requesting one snapshot from particular repository
-
-[source,console]
------------------------------------
-GET _snapshot/repo1/snap1
------------------------------------
-// TEST[skip:no repo and snapshots are created]
-
-produces the following response
-
-[source,console-result]
------------------------------------
-{
-  "responses": [
-    {
-      "repository": "repo1",
-      "snapshots": [
-        {
-          "snapshot": "snap1",
-          "uuid": "cEzdqUKxQ5G6MyrJAcYwmA",
-          "version_id": 8000099,
-          "version": "8.0.0",
-          "indices": [],
-          "include_global_state": true,
-          "state": "SUCCESS",
-          "start_time": "2019-05-10T17:01:57.868Z",
-          "start_time_in_millis": 1557507717868,
-          "end_time": "2019-05-10T17:01:57.909Z",
-          "end_time_in_millis": 1557507717909,
-          "duration_in_millis": 41,
-          "failures": [],
-          "shards": {
-            "total": 0,
-            "failed": 0,
-            "successful": 0
-          }
-        }
-      ]
-    }
-  ]
-}
------------------------------------
-// TESTRESPONSE[skip:no repo and snapshots are created]
-
-See <<modules-snapshots>> for more information.
-
-*Impact* +
-Update your workflow and applications to use the get snapshot API's new response
-format.
-====
-
 .The `repositories.fs.compress` node-level setting has been removed.
 [%collapsible]
 ====

diff --git a/docs/reference/snapshot-restore/apis/create-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/create-snapshot-api.asciidoc
@@ -176,6 +176,7 @@ The API returns the following response:
   "snapshot": {
     "snapshot": "snapshot_2",
     "uuid": "vdRctLCxSketdKb54xw67g",
+    "repository": "my_repository",
     "version_id": <version_id>,
     "version": <version>,
     "indices": [],

diff --git a/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc b/docs/reference/snapshot-restore/apis/get-snapshot-api.asciidoc
@@ -284,42 +284,38 @@ The API returns the following response:
 [source,console-result]
 ----
 {
-  "responses": [
+  "snapshots": [
     {
+      "snapshot": "snapshot_2",
+      "uuid": "vdRctLCxSketdKb54xw67g",
       "repository": "my_repository",
-      "snapshots": [
-        {
-          "snapshot": "snapshot_2",
-          "uuid": "vdRctLCxSketdKb54xw67g",
-          "version_id": <version_id>,
-          "version": <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
-          }
-        }
-      ]
+      "version_id": <version_id>,
+      "version": <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": "vdRctLCxSketdKb54xw67g"/"uuid": $body.responses.0.snapshots.0.uuid/]
-// TESTRESPONSE[s/"version_id": <version_id>/"version_id": $body.responses.0.snapshots.0.version_id/]
-// TESTRESPONSE[s/"version": <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>/"version_id": $body.snapshots.0.version_id/]
+// TESTRESPONSE[s/"version": <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/]
diff --git a/docs/reference/snapshot-restore/monitor-snapshot-restore.asciidoc b/docs/reference/snapshot-restore/monitor-snapshot-restore.asciidoc
@@ -31,6 +31,8 @@ PUT /_snapshot/my_fs_backup
 }
 
 PUT /_snapshot/my_backup/snapshot_1?wait_for_completion=true
+
+PUT /_snapshot/my_backup/some_other_snapshot?wait_for_completion=true
 -----------------------------------
 // TESTSETUP
 

diff --git a/...src/internalClusterTest/java/org/elasticsearch/repositories/url/URLSnapshotRestoreIT.java b/...src/internalClusterTest/java/org/elasticsearch/repositories/url/URLSnapshotRestoreIT.java
@@ -28,7 +28,6 @@
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
-import static org.hamcrest.Matchers.notNullValue;
 
 @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST)
 public class URLSnapshotRestoreIT extends ESIntegTestCase {
@@ -77,7 +76,7 @@ public void testUrlRepository() throws Exception {
             .prepareGetSnapshots("test-repo")
             .setSnapshots("test-snap")
             .get()
-            .getSnapshots("test-repo")
+            .getSnapshots()
             .get(0)
             .state();
         assertThat(state, equalTo(SnapshotState.SUCCESS));
@@ -105,16 +104,14 @@ public void testUrlRepository() throws Exception {
 
         logger.info("--> list available shapshots");
         GetSnapshotsResponse getSnapshotsResponse = client.admin().cluster().prepareGetSnapshots("url-repo").get();
-        assertThat(getSnapshotsResponse.getSnapshots("url-repo"), notNullValue());
-        assertThat(getSnapshotsResponse.getSnapshots("url-repo").size(), equalTo(1));
+        assertThat(getSnapshotsResponse.getSnapshots().size(), equalTo(1));
 
         logger.info("--> delete snapshot");
         AcknowledgedResponse deleteSnapshotResponse = client.admin().cluster().prepareDeleteSnapshot("test-repo", "test-snap").get();
         assertAcked(deleteSnapshotResponse);
 
         logger.info("--> list available shapshot again, no snapshots should be returned");
         getSnapshotsResponse = client.admin().cluster().prepareGetSnapshots("url-repo").get();
-        assertThat(getSnapshotsResponse.getSnapshots("url-repo"), notNullValue());
-        assertThat(getSnapshotsResponse.getSnapshots("url-repo").size(), equalTo(0));
+        assertThat(getSnapshotsResponse.getSnapshots().size(), equalTo(0));
     }
 }
diff --git a/.../repository-url/src/yamlRestTest/resources/rest-api-spec/test/repository_url/10_basic.yml b/.../repository-url/src/yamlRestTest/resources/rest-api-spec/test/repository_url/10_basic.yml
@@ -112,9 +112,6 @@ teardown:
 
 ---
 "Restore with repository-url using http://":
-  - skip:
-      version: " - 7.99.99"
-      reason: "8.0 changes get snapshots response format"
 
   # Ensure that the URL repository is registered
   - do:
@@ -129,9 +126,9 @@ teardown:
         repository: repository-url
         snapshot: snapshot-one,snapshot-two
 
-  - is_true: responses.0.snapshots
-  - match: { responses.0.snapshots.0.state : SUCCESS }
-  - match: { responses.0.snapshots.1.state : SUCCESS }
+  - is_true: snapshots
+  - match: { snapshots.0.state : SUCCESS }
+  - match: { snapshots.1.state : SUCCESS }
 
   # Delete the index
   - do:
@@ -177,9 +174,6 @@ teardown:
 
 ---
 "Restore with repository-url using file://":
-  - skip:
-      version: " - 7.99.99"
-      reason: "8.0 changes get snapshots response format"
 
   # Ensure that the URL repository is registered
   - do:
@@ -194,9 +188,9 @@ teardown:
         repository: repository-file
         snapshot: snapshot-one,snapshot-two
 
-  - is_true: responses.0.snapshots
-  - match: { responses.0.snapshots.0.state : SUCCESS }
-  - match: { responses.0.snapshots.1.state : SUCCESS }
+  - is_true: snapshots
+  - match: { snapshots.0.state : SUCCESS }
+  - match: { snapshots.1.state : SUCCESS }
 
   # Delete the index
   - do:
@@ -242,18 +236,13 @@ teardown:
 
 ---
 "Get a non existing snapshot":
-  - skip:
-      version: " - 7.99.99"
-      reason: "8.0 changes get snapshots response format"
 
   - do:
+      catch: /snapshot_missing_exception/
       snapshot.get:
         repository: repository-url
         snapshot: missing
 
-  - is_true: responses.0.error
-  - match: { responses.0.error.type: snapshot_missing_exception }
-
 ---
 "Delete a non existing snapshot":
 

diff --git a/...ry-azure/src/yamlRestTest/resources/rest-api-spec/test/repository_azure/20_repository.yml b/...ry-azure/src/yamlRestTest/resources/rest-api-spec/test/repository_azure/20_repository.yml
@@ -28,9 +28,6 @@ setup:
 
 ---
 "Snapshot/Restore with repository-azure":
-  - skip:
-      version: " - 7.99.99"
-      reason: "8.0 changes get snapshots response format"
 
   # Get repository
   - do:
@@ -130,9 +127,9 @@ setup:
         repository: repository
         snapshot: snapshot-one,snapshot-two
 
-  - is_true: responses.0.snapshots
-  - match: { responses.0.snapshots.0.state: SUCCESS }
-  - match: { responses.0.snapshots.1.state: SUCCESS }
+  - is_true: snapshots
+  - match: { snapshots.0.state : SUCCESS }
+  - match: { snapshots.1.state : SUCCESS }
 
   # Delete the index
   - do:
@@ -212,18 +209,13 @@ setup:
 
 ---
 "Get a non existing snapshot":
-  - skip:
-      version: " - 7.99.99"
-      reason: "8.0 changes get snapshots response format"
 
   - do:
+      catch: /snapshot_missing_exception/
       snapshot.get:
         repository: repository
         snapshot: missing
 
-  - is_true: responses.0.error
-  - match: { responses.0.error.type: snapshot_missing_exception }
-
 ---
 "Delete a non existing snapshot":
 

diff --git a/...sitory-gcs/src/yamlRestTest/resources/rest-api-spec/test/repository_gcs/20_repository.yml b/...sitory-gcs/src/yamlRestTest/resources/rest-api-spec/test/repository_gcs/20_repository.yml
@@ -28,9 +28,6 @@ setup:
 
 ---
 "Snapshot/Restore with repository-gcs":
-  - skip:
-      version: " - 7.99.99"
-      reason: "8.0 changes get snapshots response format"
 
   # Get repository
   - do:
@@ -130,9 +127,9 @@ setup:
         repository: repository
         snapshot: snapshot-one,snapshot-two
 
-  - is_true: responses.0.snapshots
-  - match: { responses.0.snapshots.0.state : SUCCESS }
-  - match: { responses.0.snapshots.1.state : SUCCESS }
+  - is_true: snapshots
+  - match: { snapshots.0.state : SUCCESS }
+  - match: { snapshots.1.state : SUCCESS }
 
   # Delete the index
   - do:
@@ -209,18 +206,13 @@ setup:
 
 ---
 "Get a non existing snapshot":
-  - skip:
-      version: " - 7.99.99"
-      reason: "8.0 changes get snapshots response format"
 
   - do:
+      catch: /snapshot_missing_exception/
       snapshot.get:
         repository: repository
         snapshot: missing
 
-  - is_true: responses.0.error
-  - match: { responses.0.error.type: snapshot_missing_exception }
-
 ---
 "Delete a non existing snapshot":
-Original file line number
+Diff line change
@@ Expand Up / @@ -31,6 +31,8 @@ PUT /_snapshot/my_fs_backup @@
     }
     PUT /_snapshot/my_backup/snapshot_1?wait_for_completion=true
+    PUT /_snapshot/my_backup/some_other_snapshot?wait_for_completion=true
     -----------------------------------
     // TESTSETUP
@@ Expand Down @@