Skip to content

Support multiple repositories in get snapshot request #41799

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 11 commits into from
Closed

Support multiple repositories in get snapshot request #41799

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
26ac824
Support multiple repositories in get snapshot request
May 3, 2019
4bf8e56
Fix checkstyle
May 6, 2019
6ecc6ac
BwC fix
May 6, 2019
0567239
Fix cat docs
May 6, 2019
5691315
BwC fix 2
May 6, 2019
9fb9399
No need to pass repository to finalizeSnapshot
May 6, 2019
c9b3f29
Fix yml cat snapshot tests
May 6, 2019
8b34f65
Revert formatting in finalizeSnapshot
May 6, 2019
f33140e
Skip version
May 6, 2019
527b264
Use completeWith
May 7, 2019
efe95c8
Failed repos
May 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotRequestConverters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ static Request createSnapshot(CreateSnapshotRequest createSnapshotRequest) throw

static Request getSnapshots(GetSnapshotsRequest getSnapshotsRequest) {
RequestConverters.EndpointBuilder endpointBuilder = new RequestConverters.EndpointBuilder().addPathPartAsIs("_snapshot")
.addPathPart(getSnapshotsRequest.repository());
.addCommaSeparatedPathParts(getSnapshotsRequest.repositories());
String endpoint;
if (getSnapshotsRequest.snapshots().length == 0) {
endpoint = endpointBuilder.addPathPart("_all").build();
Expand Down
37 changes: 21 additions & 16 deletions client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@

import java.io.IOException;
import java.util.Collections;
import java.util.stream.Collectors;

import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.is;
Expand Down Expand Up @@ -155,37 +153,44 @@ public void testCreateSnapshot() throws IOException {
}

public void testGetSnapshots() throws IOException {
String repository = "test_repository";
String repository1 = "test_repository1";
String repository2 = "test_repository2";
String snapshot1 = "test_snapshot1";
String snapshot2 = "test_snapshot2";

AcknowledgedResponse putRepositoryResponse = createTestRepository(repository, FsRepository.TYPE, "{\"location\": \".\"}");
AcknowledgedResponse putRepositoryResponse =
createTestRepository(repository1, FsRepository.TYPE, "{\"location\": \"loc1\"}");
assertTrue(putRepositoryResponse.isAcknowledged());

CreateSnapshotRequest createSnapshotRequest1 = new CreateSnapshotRequest(repository, snapshot1);
AcknowledgedResponse putRepositoryResponse2 =
createTestRepository(repository2, FsRepository.TYPE, "{\"location\": \"loc2\"}");
assertTrue(putRepositoryResponse2.isAcknowledged());

CreateSnapshotRequest createSnapshotRequest1 = new CreateSnapshotRequest(repository1, snapshot1);
createSnapshotRequest1.waitForCompletion(true);
CreateSnapshotResponse putSnapshotResponse1 = createTestSnapshot(createSnapshotRequest1);
CreateSnapshotRequest createSnapshotRequest2 = new CreateSnapshotRequest(repository, snapshot2);
CreateSnapshotRequest createSnapshotRequest2 = new CreateSnapshotRequest(repository2, snapshot2);
createSnapshotRequest2.waitForCompletion(true);
CreateSnapshotResponse putSnapshotResponse2 = createTestSnapshot(createSnapshotRequest2);
// check that the request went ok without parsing JSON here. When using the high level client, check acknowledgement instead.
assertEquals(RestStatus.OK, putSnapshotResponse1.status());
assertEquals(RestStatus.OK, putSnapshotResponse2.status());

GetSnapshotsRequest request;
if (randomBoolean()) {
request = new GetSnapshotsRequest(repository);
} else if (randomBoolean()) {
request = new GetSnapshotsRequest(repository, new String[] {"_all"});
GetSnapshotsRequest request = new GetSnapshotsRequest(
randomFrom(new String[]{"_all"}, new String[]{"*"}, new String[]{repository1, repository2}),
randomFrom(new String[]{"_all"}, new String[]{"*"}, new String[]{snapshot1, snapshot2})
);
request.ignoreUnavailable(true);

} else {
request = new GetSnapshotsRequest(repository, new String[] {snapshot1, snapshot2});
}
GetSnapshotsResponse response = execute(request, highLevelClient().snapshot()::get, highLevelClient().snapshot()::getAsync);

logger.info(response.getSnapshots());
assertEquals(2, response.getSnapshots().size());
assertThat(response.getSnapshots().stream().map((s) -> s.snapshotId().getName()).collect(Collectors.toList()),
contains("test_snapshot1", "test_snapshot2"));
assertEquals(repository1, response.getSnapshots().get(0).repository());
assertEquals(snapshot1, response.getSnapshots().get(0).snapshotId().getName());

assertEquals(repository2, response.getSnapshots().get(1).repository());
assertEquals(snapshot2, response.getSnapshots().get(1).snapshotId().getName());
}

public void testSnapshotsStatus() throws IOException {
Expand Down
9 changes: 5 additions & 4 deletions ...est-high-level/src/test/java/org/elasticsearch/client/SnapshotRequestConvertersTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,15 +148,16 @@ public void testCreateSnapshot() throws IOException {

public void testGetSnapshots() {
Map<String, String> expectedParams = new HashMap<>();
String repository = RequestConvertersTests.randomIndicesNames(1, 1)[0];
String repository1 = randomAlphaOfLength(10);
String repository2 = randomAlphaOfLength(10);
String snapshot1 = "snapshot1-" + randomAlphaOfLengthBetween(2, 5).toLowerCase(Locale.ROOT);
String snapshot2 = "snapshot2-" + randomAlphaOfLengthBetween(2, 5).toLowerCase(Locale.ROOT);

String endpoint = String.format(Locale.ROOT, "/_snapshot/%s/%s,%s", repository, snapshot1, snapshot2);
String endpoint = String.format(Locale.ROOT, "/_snapshot/%s,%s/%s,%s", repository1, repository2, snapshot1, snapshot2);

GetSnapshotsRequest getSnapshotsRequest = new GetSnapshotsRequest();
getSnapshotsRequest.repository(repository);
getSnapshotsRequest.snapshots(Arrays.asList(snapshot1, snapshot2).toArray(new String[0]));
getSnapshotsRequest.repositories(repository1, repository2);
getSnapshotsRequest.snapshots(new String[]{snapshot1, snapshot2});
RequestConvertersTests.setRandomMasterTimeout(getSnapshotsRequest, expectedParams);

if (randomBoolean()) {
Expand Down
2 changes: 1 addition & 1 deletion ...l/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ public void testSnapshotGetSnapshots() throws IOException {
// end::get-snapshots-request

// tag::get-snapshots-request-repositoryName
request.repository(repositoryName); // <1>
request.repositories(repositoryName); // <1>
// end::get-snapshots-request-repositoryName

// tag::get-snapshots-request-snapshots
Expand Down
3 changes: 3 additions & 0 deletions ...java/org/elasticsearch/action/admin/cluster/repositories/get/GetRepositoriesResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ public List<RepositoryMetaData> repositories() {
return repositories.repositories();
}

public GetRepositoriesResponse(StreamInput in) throws IOException {
readFrom(in);
}

@Override
public void readFrom(StreamInput in) throws IOException {
Expand Down
51 changes: 30 additions & 21 deletions ...c/main/java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.action.admin.cluster.snapshots.get;

import org.elasticsearch.Version;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.support.master.MasterNodeRequest;
import org.elasticsearch.common.Strings;
Expand All @@ -38,7 +39,7 @@ public class GetSnapshotsRequest extends MasterNodeRequest<GetSnapshotsRequest>
public static final String CURRENT_SNAPSHOT = "_current";
public static final boolean DEFAULT_VERBOSE_MODE = true;

private String repository;
private String[] repositories;

private String[] snapshots = Strings.EMPTY_ARRAY;

Expand All @@ -50,28 +51,32 @@ public GetSnapshotsRequest() {
}

/**
* Constructs a new get snapshots request with given repository name and list of snapshots
* Constructs a new get snapshots request with given repository names and list of snapshots
*
* @param repository repository name
* @param repositories repository names
* @param snapshots list of snapshots
*/
public GetSnapshotsRequest(String repository, String[] snapshots) {
this.repository = repository;
public GetSnapshotsRequest(String[] repositories, String[] snapshots) {
this.repositories = repositories;
this.snapshots = snapshots;
}

/**
* Constructs a new get snapshots request with given repository name
* Constructs a new get snapshots request with given repository names
*
* @param repository repository name
* @param repositories repository names
*/
public GetSnapshotsRequest(String repository) {
this.repository = repository;
public GetSnapshotsRequest(String... repositories) {
this.repositories = repositories;
}

public GetSnapshotsRequest(StreamInput in) throws IOException {
super(in);
repository = in.readString();
if (in.getVersion().onOrAfter(Version.V_7_1_0)) {
repositories = in.readStringArray();
} else {
repositories = new String[]{in.readString()};
}
snapshots = in.readStringArray();
ignoreUnavailable = in.readBoolean();
verbose = in.readBoolean();
Expand All @@ -80,7 +85,11 @@ public GetSnapshotsRequest(StreamInput in) throws IOException {
@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeString(repository);
if (out.getVersion().onOrAfter(Version.V_7_1_0)) {
Copy link
Contributor

@original-brownbear original-brownbear May 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should fail BwC tests shouldn't it? I think we gotta first set it to V8_0_0 and then back port and adjust the constant?

Copy link
Contributor Author

@andrershov andrershov May 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@original-brownbear thanks, I've fixed it in 5691315.
The sequence of BwC story would be the following:

  1. Merge this PR.
  2. Create separate PR to disable snapshot related BwC tests in master and change version to 7.2.0.
  3. Backport the change to 7.2.0.
  4. Enable tests.

out.writeStringArray(repositories);
} else {
out.writeString(repositories[0]);
}
out.writeStringArray(snapshots);
out.writeBoolean(ignoreUnavailable);
out.writeBoolean(verbose);
Expand All @@ -89,30 +98,30 @@ public void writeTo(StreamOutput out) throws IOException {
@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (repository == null) {
validationException = addValidationError("repository is missing", validationException);
if (repositories == null) {
validationException = addValidationError("repositories are missing", validationException);
}
return validationException;
}

/**
* Sets repository name
* Sets repository names
*
* @param repository repository name
* @param repositories repository names
* @return this request
*/
public GetSnapshotsRequest repository(String repository) {
this.repository = repository;
public GetSnapshotsRequest repositories(String... repositories) {
this.repositories = repositories;
return this;
}

/**
* Returns repository name
* Returns repository names
*
* @return repository name
* @return repository names
*/
public String repository() {
return this.repository;
public String[] repositories() {
return this.repositories;
}

/**
Expand Down
19 changes: 6 additions & 13 deletions ...java/org/elasticsearch/action/admin/cluster/snapshots/get/GetSnapshotsRequestBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,28 +29,21 @@
public class GetSnapshotsRequestBuilder extends MasterNodeOperationRequestBuilder<GetSnapshotsRequest,
GetSnapshotsResponse, GetSnapshotsRequestBuilder> {

/**
* Constructs the new get snapshot request
*/
public GetSnapshotsRequestBuilder(ElasticsearchClient client, GetSnapshotsAction action) {
super(client, action, new GetSnapshotsRequest());
}

/**
* Constructs the new get snapshot request with specified repository
*/
public GetSnapshotsRequestBuilder(ElasticsearchClient client, GetSnapshotsAction action, String repository) {
super(client, action, new GetSnapshotsRequest(repository));
public GetSnapshotsRequestBuilder(ElasticsearchClient client, GetSnapshotsAction action, String[] repositories) {
super(client, action, new GetSnapshotsRequest(repositories));
}

/**
* Sets the repository name
* Sets the repository names
*
* @param repository repository name
* @param repositories repository names
* @return this builder
*/
public GetSnapshotsRequestBuilder setRepository(String repository) {
request.repository(repository);
public GetSnapshotsRequestBuilder setRepositories(String... repositories) {
request.repositories(repositories);
return this;
}

Expand Down
Loading