Skip to content
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.

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

Clean Up Snapshot Create Rest API #31779

Merged
merged 7 commits into from
Jul 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public void verifyRepositoryAsync(VerifyRepositoryRequest verifyRepositoryReques
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-snapshots.html"> Snapshot and Restore
* API on elastic.co</a>
*/
public CreateSnapshotResponse createSnapshot(CreateSnapshotRequest createSnapshotRequest, RequestOptions options)
public CreateSnapshotResponse create(CreateSnapshotRequest createSnapshotRequest, RequestOptions options)
throws IOException {
return restHighLevelClient.performRequestAndParseEntity(createSnapshotRequest, RequestConverters::createSnapshot, options,
CreateSnapshotResponse::fromXContent, emptySet());
Expand All @@ -188,7 +188,7 @@ public CreateSnapshotResponse createSnapshot(CreateSnapshotRequest createSnapsho
* See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-snapshots.html"> Snapshot and Restore
* API on elastic.co</a>
*/
public void createSnapshotAsync(CreateSnapshotRequest createSnapshotRequest, RequestOptions options,
public void createAsync(CreateSnapshotRequest createSnapshotRequest, RequestOptions options,
ActionListener<CreateSnapshotResponse> listener) {
restHighLevelClient.performRequestAsyncAndParseEntity(createSnapshotRequest, RequestConverters::createSnapshot, options,
CreateSnapshotResponse::fromXContent, listener, emptySet());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ private PutRepositoryResponse createTestRepository(String repository, String typ
private CreateSnapshotResponse createTestSnapshot(CreateSnapshotRequest createSnapshotRequest) throws IOException {
// assumes the repository already exists

return execute(createSnapshotRequest, highLevelClient().snapshot()::createSnapshot,
highLevelClient().snapshot()::createSnapshotAsync);
return execute(createSnapshotRequest, highLevelClient().snapshot()::create,
highLevelClient().snapshot()::createAsync);
}

public void testCreateRepository() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -425,14 +425,20 @@ public void testSnapshotCreate() throws IOException {
// end::create-snapshot-request-waitForCompletion

// tag::create-snapshot-execute
CreateSnapshotResponse response = client.snapshot().createSnapshot(request, RequestOptions.DEFAULT);
CreateSnapshotResponse response = client.snapshot().create(request, RequestOptions.DEFAULT);
// end::create-snapshot-execute

// tag::create-snapshot-response
RestStatus status = response.status(); // <1>
// end::create-snapshot-response

assertEquals(RestStatus.OK, status);

// tag::create-snapshot-response-snapshot-info
SnapshotInfo snapshotInfo = response.getSnapshotInfo(); // <1>
// end::create-snapshot-response-snapshot-info

assertNotNull(snapshotInfo);
}

public void testSnapshotCreateAsync() throws InterruptedException {
Expand Down Expand Up @@ -460,7 +466,7 @@ public void onFailure(Exception exception) {
listener = new LatchedActionListener<>(listener, latch);

// tag::create-snapshot-execute-async
client.snapshot().createSnapshotAsync(request, RequestOptions.DEFAULT, listener); // <1>
client.snapshot().createAsync(request, RequestOptions.DEFAULT, listener); // <1>
// end::create-snapshot-execute-async

assertTrue(latch.await(30L, TimeUnit.SECONDS));
Expand Down
11 changes: 11 additions & 0 deletions docs/java-rest/high-level/snapshot/create_snapshot.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,22 @@ include-tagged::{doc-tests}/SnapshotClientDocumentationIT.java[create-snapshot-r
[[java-rest-high-snapshot-create-snapshot-sync]]
==== Synchronous Execution

Execute a `CreateSnapshotRequest` synchronously to receive a `CreateSnapshotResponse`.

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/SnapshotClientDocumentationIT.java[create-snapshot-execute]
--------------------------------------------------

Retrieve the `SnapshotInfo` from a `CreateSnapshotResponse` when the snapshot is fully created.
(The `waitForCompletion` parameter is `true`).

["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests}/SnapshotClientDocumentationIT.java[create-snapshot-response-snapshot-info]
--------------------------------------------------
<1> The `SnapshotInfo` object.

[[java-rest-high-snapshot-create-snapshot-async]]
==== Asynchronous Execution

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@

import static org.elasticsearch.action.ValidateActions.addValidationError;
import static org.elasticsearch.common.Strings.EMPTY_ARRAY;
import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;
import static org.elasticsearch.common.settings.Settings.readSettingsFromStream;
import static org.elasticsearch.common.settings.Settings.writeSettingsToStream;
import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;
import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBooleanValue;

/**
Expand Down Expand Up @@ -433,8 +433,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
if (indicesOptions != null) {
indicesOptions.toXContent(builder, params);
}
builder.field("wait_for_completion", waitForCompletion);
builder.field("master_node_timeout", masterNodeTimeout.toString());
Copy link
Member

Choose a reason for hiding this comment

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

good catch , these should not be printed out, nor parsed back, they are query_string parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Credit to @vladimirdolzhenko for pointing this out.

builder.endObject();
return builder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@

import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ObjectParser;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.common.xcontent.XContentParser.Token;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.snapshots.SnapshotInfo;
import org.elasticsearch.snapshots.SnapshotInfo.SnapshotInfoBuilder;

import java.io.IOException;
import java.util.Objects;
Expand All @@ -38,6 +40,14 @@
*/
public class CreateSnapshotResponse extends ActionResponse implements ToXContentObject {

private static final ObjectParser<CreateSnapshotResponse, Void> PARSER =
new ObjectParser<>(CreateSnapshotResponse.class.getName(), true, CreateSnapshotResponse::new);

static {
PARSER.declareObject(CreateSnapshotResponse::setSnapshotInfoFromBuilder,
SnapshotInfo.SNAPSHOT_INFO_PARSER, new ParseField("snapshot"));
}

@Nullable
private SnapshotInfo snapshotInfo;

Expand All @@ -48,8 +58,8 @@ public class CreateSnapshotResponse extends ActionResponse implements ToXContent
CreateSnapshotResponse() {
}

void setSnapshotInfo(SnapshotInfo snapshotInfo) {
this.snapshotInfo = snapshotInfo;
private void setSnapshotInfoFromBuilder(SnapshotInfoBuilder snapshotInfoBuilder) {
this.snapshotInfo = snapshotInfoBuilder.build();
}

/**
Expand Down Expand Up @@ -101,38 +111,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
return builder;
}

public static CreateSnapshotResponse fromXContent(XContentParser parser) throws IOException {
CreateSnapshotResponse createSnapshotResponse = new CreateSnapshotResponse();

parser.nextToken(); // move to '{'

if (parser.currentToken() != Token.START_OBJECT) {
throw new IllegalArgumentException("unexpected token [" + parser.currentToken() + "], expected ['{']");
}

parser.nextToken(); // move to 'snapshot' || 'accepted'

if ("snapshot".equals(parser.currentName())) {
createSnapshotResponse.snapshotInfo = SnapshotInfo.fromXContent(parser);
} else if ("accepted".equals(parser.currentName())) {
parser.nextToken(); // move to 'accepted' field value

if (parser.booleanValue()) {
// ensure accepted is a boolean value
}

parser.nextToken(); // move past 'true'/'false'
} else {
throw new IllegalArgumentException("unexpected token [" + parser.currentToken() + "] expected ['snapshot', 'accepted']");
}

if (parser.currentToken() != Token.END_OBJECT) {
throw new IllegalArgumentException("unexpected token [" + parser.currentToken() + "], expected ['}']");
}

parser.nextToken(); // move past '}'

return createSnapshotResponse;
public static CreateSnapshotResponse fromXContent(XContentParser parser) {
return PARSER.apply(parser, null);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.elasticsearch.Version;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.rest.RestRequest;
Expand Down Expand Up @@ -316,21 +317,6 @@ public static IndicesOptions fromMap(Map<String, Object> map, IndicesOptions def
defaultSettings);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startArray("expand_wildcards");
for (WildcardStates expandWildcard : expandWildcards) {
builder.value(expandWildcard.toString().toLowerCase(Locale.ROOT));
}
builder.endArray();
builder.field("ignore_unavailable", ignoreUnavailable());
builder.field("allow_no_indices", allowNoIndices());
builder.field("forbid_aliases_to_multiple_indices", allowAliasesToMultipleIndices() == false);
builder.field("forbid_closed_indices", forbidClosedIndices());
builder.field("ignore_aliases", ignoreAliases());
return builder;
}

/**
* Returns true if the name represents a valid name for one of the indices option
* false otherwise
Expand Down Expand Up @@ -360,6 +346,18 @@ public static IndicesOptions fromParameters(Object wildcardsString, Object ignor
);
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
builder.startArray("expand_wildcards");
for (WildcardStates expandWildcard : expandWildcards) {
builder.value(expandWildcard.toString().toLowerCase(Locale.ROOT));
}
builder.endArray();
builder.field("ignore_unavailable", ignoreUnavailable());
builder.field("allow_no_indices", allowNoIndices());
return builder;
}

/**
* @return indices options that requires every specified index to exist, expands wildcards only to open indices and
* allows that no indices are resolved from wildcard expressions (not returning an error).
Expand Down
25 changes: 0 additions & 25 deletions server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java
Original file line number Diff line number Diff line change
Expand Up @@ -140,22 +140,6 @@ private void setShardFailures(List<SnapshotShardFailure> shardFailures) {
this.shardFailures = shardFailures;
}

private void ignoreVersion(String version) {
// ignore extra field
}

private void ignoreStartTime(String startTime) {
// ignore extra field
}

private void ignoreEndTime(String endTime) {
// ignore extra field
}

private void ignoreDurationInMillis(long durationInMillis) {
// ignore extra field
}

public SnapshotInfo build() {
SnapshotId snapshotId = new SnapshotId(snapshotName, snapshotUUID);

Expand Down Expand Up @@ -197,10 +181,6 @@ private void setSuccessfulShards(int successfulShards) {
int getSuccessfulShards() {
return successfulShards;
}

private void ignoreFailedShards(int failedShards) {
// ignore extra field
}
}

public static final ObjectParser<SnapshotInfoBuilder, Void> SNAPSHOT_INFO_PARSER =
Expand All @@ -222,14 +202,9 @@ private void ignoreFailedShards(int failedShards) {
SNAPSHOT_INFO_PARSER.declareInt(SnapshotInfoBuilder::setVersion, new ParseField(VERSION_ID));
SNAPSHOT_INFO_PARSER.declareObjectArray(SnapshotInfoBuilder::setShardFailures, SnapshotShardFailure.SNAPSHOT_SHARD_FAILURE_PARSER,
new ParseField(FAILURES));
SNAPSHOT_INFO_PARSER.declareString(SnapshotInfoBuilder::ignoreVersion, new ParseField(VERSION));
SNAPSHOT_INFO_PARSER.declareString(SnapshotInfoBuilder::ignoreStartTime, new ParseField(START_TIME));
SNAPSHOT_INFO_PARSER.declareString(SnapshotInfoBuilder::ignoreEndTime, new ParseField(END_TIME));
SNAPSHOT_INFO_PARSER.declareLong(SnapshotInfoBuilder::ignoreDurationInMillis, new ParseField(DURATION_IN_MILLIS));

SHARD_STATS_PARSER.declareInt(ShardStatsBuilder::setTotalShards, new ParseField(TOTAL));
SHARD_STATS_PARSER.declareInt(ShardStatsBuilder::setSuccessfulShards, new ParseField(SUCCESSFUL));
SHARD_STATS_PARSER.declareInt(ShardStatsBuilder::ignoreFailedShards, new ParseField(FAILED));
}

private final SnapshotId snapshotId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ public void testToXContent() throws IOException {
NamedXContentRegistry.EMPTY, null, BytesReference.bytes(builder).streamInput());
Map<String, Object> map = parser.mapOrdered();
CreateSnapshotRequest processed = new CreateSnapshotRequest((String)map.get("repository"), (String)map.get("snapshot"));
processed.waitForCompletion((boolean)map.getOrDefault("wait_for_completion", false));
processed.masterNodeTimeout((String)map.getOrDefault("master_node_timeout", "30s"));
processed.waitForCompletion(original.waitForCompletion());
processed.masterNodeTimeout(original.masterNodeTimeout());
processed.source(map);

assertEquals(original, processed);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ protected CreateSnapshotResponse doParseInstance(XContentParser parser) throws I

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

@Override
Expand All @@ -63,9 +63,7 @@ protected CreateSnapshotResponse createTestInstance() {

boolean globalState = randomBoolean();

CreateSnapshotResponse response = new CreateSnapshotResponse();
response.setSnapshotInfo(
return new CreateSnapshotResponse(
new SnapshotInfo(snapshotId, indices, startTime, reason, endTime, totalShards, shardFailures, globalState));
return response;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,5 @@ public void testToXContent() throws IOException {
}
assertEquals(map.get("ignore_unavailable"), options.contains(Option.IGNORE_UNAVAILABLE));
assertEquals(map.get("allow_no_indices"), options.contains(Option.ALLOW_NO_INDICES));
assertEquals(map.get("forbid_aliases_to_multiple_indices"), options.contains(Option.FORBID_ALIASES_TO_MULTIPLE_INDICES));
assertEquals(map.get("forbid_closed_indices"), options.contains(Option.FORBID_CLOSED_INDICES));
assertEquals(map.get("ignore_aliases"), options.contains(Option.IGNORE_ALIASES));
}
}