From ddace8f7bb1395b5a3e2b4188f2a3fb50fbb839e Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Tue, 3 Jul 2018 11:06:32 -0700 Subject: [PATCH 1/5] Follow up for SnapshotCreate rest client code based on PR feedback. --- .../create/CreateSnapshotResponse.java | 50 ++++++------------- .../elasticsearch/snapshots/SnapshotInfo.java | 25 ---------- .../create/CreateSnapshotResponseTests.java | 6 +-- 3 files changed, 17 insertions(+), 64 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotResponse.java index a2dc02c5c8299..3a6106770db4e 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotResponse.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotResponse.java @@ -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; @@ -38,6 +40,14 @@ */ public class CreateSnapshotResponse extends ActionResponse implements ToXContentObject { + private static final ObjectParser CREATE_SNAPSHOT_RESPONSE_PARSER = + new ObjectParser<>(CreateSnapshotResponse.class.getName(), true, CreateSnapshotResponse::new); + + static { + CREATE_SNAPSHOT_RESPONSE_PARSER.declareObject(CreateSnapshotResponse::setSnapshotInfoFromBuilder, + SnapshotInfo.SNAPSHOT_INFO_PARSER, new ParseField("snapshot")); + } + @Nullable private SnapshotInfo snapshotInfo; @@ -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(); } /** @@ -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 CREATE_SNAPSHOT_RESPONSE_PARSER.apply(parser, null); } @Override diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java index a1f56a1e47376..bf3d337c49ec5 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotInfo.java @@ -138,22 +138,6 @@ private void setShardFailures(List 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); @@ -195,10 +179,6 @@ private void setSuccessfulShards(int successfulShards) { int getSuccessfulShards() { return successfulShards; } - - private void ignoreFailedShards(int failedShards) { - // ignore extra field - } } public static final ObjectParser SNAPSHOT_INFO_PARSER = @@ -220,14 +200,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; diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotResponseTests.java index bbfc9755bf215..bbb11fc6feef0 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotResponseTests.java @@ -40,7 +40,7 @@ protected CreateSnapshotResponse doParseInstance(XContentParser parser) throws I @Override protected boolean supportsUnknownFields() { - return false; + return true; } @Override @@ -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; } } From 1b903ce68c3e1ee2403aff61223f6f929a594833 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Tue, 3 Jul 2018 12:23:56 -0700 Subject: [PATCH 2/5] Fix parameters for indicesOptions. --- .../client/RequestConverters.java | 1 + .../elasticsearch/client/SnapshotClient.java | 4 +- .../org/elasticsearch/client/SnapshotIT.java | 4 +- .../SnapshotClientDocumentationIT.java | 4 +- .../rest-api-spec/api/snapshot.create.json | 15 ++++++ .../create/CreateSnapshotRequest.java | 7 +-- .../action/support/IndicesOptions.java | 20 +------- .../cluster/RestCreateSnapshotAction.java | 2 + .../create/CreateSnapshotRequestTests.java | 5 +- .../action/support/IndicesOptionsTests.java | 48 ------------------- 10 files changed, 29 insertions(+), 81 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java index 26f0b5c647404..81ab4a1c613a7 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RequestConverters.java @@ -920,6 +920,7 @@ static Request createSnapshot(CreateSnapshotRequest createSnapshotRequest) throw Params params = new Params(request); params.withMasterTimeout(createSnapshotRequest.masterNodeTimeout()); params.withWaitForCompletion(createSnapshotRequest.waitForCompletion()); + params.withIndicesOptions(createSnapshotRequest.indicesOptions()); request.setEntity(createEntity(createSnapshotRequest, REQUEST_BODY_CONTENT_TYPE)); return request; } diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java index fa147a338de0a..785469673747c 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/SnapshotClient.java @@ -174,7 +174,7 @@ public void verifyRepositoryAsync(VerifyRepositoryRequest verifyRepositoryReques * See Snapshot and Restore * API on elastic.co */ - 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()); @@ -186,7 +186,7 @@ public CreateSnapshotResponse createSnapshot(CreateSnapshotRequest createSnapsho * See Snapshot and Restore * API on elastic.co */ - public void createSnapshotAsync(CreateSnapshotRequest createSnapshotRequest, RequestOptions options, + public void createAsync(CreateSnapshotRequest createSnapshotRequest, RequestOptions options, ActionListener listener) { restHighLevelClient.performRequestAsyncAndParseEntity(createSnapshotRequest, RequestConverters::createSnapshot, options, CreateSnapshotResponse::fromXContent, listener, emptySet()); 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 index 7ec2ee80f04ac..5dd288e4398d7 100644 --- 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 @@ -57,8 +57,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 { diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java index 2d126fb970cbf..f13875ee67f2a 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/SnapshotClientDocumentationIT.java @@ -417,7 +417,7 @@ 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 @@ -452,7 +452,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)); diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.create.json b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.create.json index 29ae2206c8568..9c90740c5b905 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.create.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/snapshot.create.json @@ -26,6 +26,21 @@ "type": "boolean", "description": "Should this request wait until the operation has completed before returning", "default": false + }, + "ignore_unavailable": { + "type": "boolean", + "description": "Ignore unavailable indexes", + "default": false + }, + "allow_no_indices": { + "type": "boolean", + "description": "Ignore if a wildcard expression resolves to no concrete indices (default: false)" + }, + "expand_wildcards": { + "type": "enum", + "options": [ "open", "closed", "none", "all" ], + "default": "open", + "description": "Whether wildcard expressions should get expanded to open or closed indices (default: open)" } } }, diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java index 2ff01ab01ed1f..01f743bf43083 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java @@ -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; /** @@ -430,11 +430,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); } builder.field("include_global_state", includeGlobalState); - if (indicesOptions != null) { - indicesOptions.toXContent(builder, params); - } - builder.field("wait_for_completion", waitForCompletion); - builder.field("master_node_timeout", masterNodeTimeout.toString()); builder.endObject(); return builder; } diff --git a/server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java b/server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java index 19572a6c212a2..b284ec87dd42c 100644 --- a/server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java +++ b/server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java @@ -22,15 +22,12 @@ import org.elasticsearch.Version; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.xcontent.ToXContentFragment; -import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.rest.RestRequest; import java.io.IOException; import java.util.Collection; import java.util.EnumSet; import java.util.HashSet; -import java.util.Locale; import java.util.Map; import java.util.Set; @@ -41,7 +38,7 @@ * Controls how to deal with unavailable concrete indices (closed or missing), how wildcard expressions are expanded * to actual indices (all, closed or open indices) and how to deal with wildcard expressions that resolve to no indices. */ -public class IndicesOptions implements ToXContentFragment { +public class IndicesOptions { public enum WildcardStates { OPEN, @@ -316,21 +313,6 @@ public static IndicesOptions fromMap(Map 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 diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCreateSnapshotAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCreateSnapshotAction.java index bf2866b57713e..f18ec6c69ec82 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCreateSnapshotAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestCreateSnapshotAction.java @@ -20,6 +20,7 @@ package org.elasticsearch.rest.action.admin.cluster; import org.elasticsearch.action.admin.cluster.snapshots.create.CreateSnapshotRequest; +import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.BaseRestHandler; @@ -54,6 +55,7 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC request.applyContentParser(p -> createSnapshotRequest.source(p.mapOrdered())); createSnapshotRequest.masterNodeTimeout(request.paramAsTime("master_timeout", createSnapshotRequest.masterNodeTimeout())); createSnapshotRequest.waitForCompletion(request.paramAsBoolean("wait_for_completion", false)); + createSnapshotRequest.indicesOptions(IndicesOptions.fromRequest(request, createSnapshotRequest.indicesOptions())); return channel -> client.admin().cluster().createSnapshot(createSnapshotRequest, new RestToXContentListener<>(channel)); } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequestTests.java index 1bde8ab572b72..aee3a29b1393b 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequestTests.java @@ -102,8 +102,9 @@ public void testToXContent() throws IOException { NamedXContentRegistry.EMPTY, null, BytesReference.bytes(builder).streamInput()); Map 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.indicesOptions(original.indicesOptions()); + processed.waitForCompletion(original.waitForCompletion()); + processed.masterNodeTimeout(original.masterNodeTimeout()); processed.source(map); assertEquals(original, processed); diff --git a/server/src/test/java/org/elasticsearch/action/support/IndicesOptionsTests.java b/server/src/test/java/org/elasticsearch/action/support/IndicesOptionsTests.java index 3f754d601b501..24f08bfe019e2 100644 --- a/server/src/test/java/org/elasticsearch/action/support/IndicesOptionsTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/IndicesOptionsTests.java @@ -20,27 +20,14 @@ package org.elasticsearch.action.support; import org.elasticsearch.Version; -import org.elasticsearch.action.support.IndicesOptions.Option; -import org.elasticsearch.action.support.IndicesOptions.WildcardStates; -import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.common.xcontent.ToXContent.MapParams; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.EqualsHashCodeTestUtils; -import java.io.IOException; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; -import java.util.EnumSet; import java.util.HashMap; -import java.util.List; import java.util.Map; import static org.elasticsearch.test.VersionUtils.randomVersionBetween; @@ -289,39 +276,4 @@ public void testFromMap() { assertEquals(fromMap.ignoreUnavailable(), ignoreUnavailable == null ? defaults.ignoreUnavailable() : ignoreUnavailable); assertEquals(fromMap.allowNoIndices(), allowNoIndices == null ? defaults.allowNoIndices() : allowNoIndices); } - - public void testToXContent() throws IOException { - Collection wildcardStates = randomSubsetOf(Arrays.asList(WildcardStates.values())); - Collection