Skip to content

Commit a79f8c2

Browse files
danielmitterdorferdanielmitterdorfer
committed
Address review comments
1 parent ba0618d commit a79f8c2

File tree

6 files changed

+64
-53
lines changed

6 files changed

+64
-53
lines changed

‎client/rest-high-level/src/test/java/org/elasticsearch/client/RequestConvertersTests.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2205,14 +2205,14 @@ public void testRestoreSnapshot() throws IOException {
22052205

22062206
RestoreSnapshotRequest restoreSnapshotRequest = new RestoreSnapshotRequest(repository, snapshot);
22072207
setRandomMasterTimeout(restoreSnapshotRequest, expectedParams);
2208-
Boolean waitForCompletion = randomBoolean();
2209-
restoreSnapshotRequest.waitForCompletion(waitForCompletion);
2210-
if (waitForCompletion) {
2211-
expectedParams.put("wait_for_completion", waitForCompletion.toString());
2208+
if (randomBoolean()) {
2209+
restoreSnapshotRequest.waitForCompletion(true);
2210+
expectedParams.put("wait_for_completion", "true");
22122211
}
22132212
if (randomBoolean()) {
2214-
restoreSnapshotRequest.masterNodeTimeout("120s");
2215-
expectedParams.put("master_timeout", "120s");
2213+
String timeout = randomTimeValue();
2214+
restoreSnapshotRequest.masterNodeTimeout(timeout);
2215+
expectedParams.put("master_timeout", timeout);
22162216
}
22172217

22182218
Request request = RequestConverters.restoreSnapshot(restoreSnapshotRequest);

‎client/rest-high-level/src/test/java/org/elasticsearch/client/SnapshotIT.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,13 +220,17 @@ public void testRestoreSnapshot() throws IOException {
220220
assertTrue(putRepositoryResponse.isAcknowledged());
221221

222222
createIndex(testIndex, Settings.EMPTY);
223+
assertTrue("index [" + testIndex + "] should have been created", indexExists(testIndex));
223224

224225
CreateSnapshotRequest createSnapshotRequest = new CreateSnapshotRequest(testRepository, testSnapshot);
225226
createSnapshotRequest.indices(testIndex);
226227
createSnapshotRequest.waitForCompletion(true);
227228
CreateSnapshotResponse createSnapshotResponse = createTestSnapshot(createSnapshotRequest);
228229
assertEquals(RestStatus.OK, createSnapshotResponse.status());
229230

231+
deleteIndex(testIndex);
232+
assertFalse("index [" + testIndex + "] should have been deleted", indexExists(testIndex));
233+
230234
RestoreSnapshotRequest request = new RestoreSnapshotRequest(testRepository, testSnapshot);
231235
request.waitForCompletion(true);
232236
request.renamePattern(testIndex);

‎server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java‎

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -534,13 +534,13 @@ public RestoreSnapshotRequest source(Map<String, Object> source) {
534534
} else if (name.equals("rename_pattern")) {
535535
if (entry.getValue() instanceof String) {
536536
renamePattern((String) entry.getValue());
537-
} else if (entry.getValue() != null) {
537+
} else {
538538
throw new IllegalArgumentException("malformed rename_pattern");
539539
}
540540
} else if (name.equals("rename_replacement")) {
541541
if (entry.getValue() instanceof String) {
542542
renameReplacement((String) entry.getValue());
543-
} else if (entry.getValue() != null) {
543+
} else {
544544
throw new IllegalArgumentException("malformed rename_replacement");
545545
}
546546
} else if (name.equals("index_settings")) {
@@ -577,8 +577,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
577577
if (indicesOptions != null) {
578578
indicesOptions.toXContent(builder, params);
579579
}
580-
builder.field("rename_pattern", renamePattern);
581-
builder.field("rename_replacement", renameReplacement);
580+
if (renamePattern != null) {
581+
builder.field("rename_pattern", renamePattern);
582+
}
583+
if (renameReplacement != null) {
584+
builder.field("rename_replacement", renameReplacement);
585+
}
582586
builder.field("include_global_state", includeGlobalState);
583587
builder.field("partial", partial);
584588
builder.field("include_aliases", includeAliases);
@@ -646,21 +650,6 @@ public int hashCode() {
646650

647651
@Override
648652
public String toString() {
649-
return "RestoreSnapshotRequest{" +
650-
"snapshot='" + snapshot + '\'' +
651-
", repository='" + repository + '\'' +
652-
", indices=" + (indices == null ? null : Arrays.asList(indices)) +
653-
", indicesOptions=" + indicesOptions +
654-
", renamePattern='" + renamePattern + '\'' +
655-
", renameReplacement='" + renameReplacement + '\'' +
656-
", waitForCompletion=" + waitForCompletion +
657-
", includeGlobalState=" + includeGlobalState +
658-
", partial=" + partial +
659-
", includeAliases=" + includeAliases +
660-
", settings=" + settings +
661-
", indexSettings=" + indexSettings +
662-
", ignoreIndexSettings=" + (ignoreIndexSettings == null ? null : Arrays.asList(ignoreIndexSettings)) +
663-
", masterNodeTimeout=" + masterNodeTimeout +
664-
'}';
653+
return Strings.toString(this);
665654
}
666655
}

‎server/src/main/java/org/elasticsearch/snapshots/RestoreInfo.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.elasticsearch.snapshots;
2020

2121
import org.elasticsearch.common.ParseField;
22+
import org.elasticsearch.common.Strings;
2223
import org.elasticsearch.common.io.stream.StreamInput;
2324
import org.elasticsearch.common.io.stream.StreamOutput;
2425
import org.elasticsearch.common.io.stream.Streamable;
@@ -285,11 +286,6 @@ public int hashCode() {
285286

286287
@Override
287288
public String toString() {
288-
return "RestoreInfo{" +
289-
"name='" + name + '\'' +
290-
", indices=" + indices +
291-
", totalShards=" + totalShards +
292-
", successfulShards=" + successfulShards +
293-
'}';
289+
return Strings.toString(this);
294290
}
295291
}

‎server/src/test/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequestTests.java‎

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,14 @@
2121

2222
import org.elasticsearch.action.support.IndicesOptions;
2323
import org.elasticsearch.common.bytes.BytesReference;
24+
import org.elasticsearch.common.io.stream.Writeable;
2425
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
2526
import org.elasticsearch.common.xcontent.ToXContent;
2627
import org.elasticsearch.common.xcontent.XContentBuilder;
2728
import org.elasticsearch.common.xcontent.XContentFactory;
2829
import org.elasticsearch.common.xcontent.XContentParser;
2930
import org.elasticsearch.common.xcontent.XContentType;
30-
import org.elasticsearch.test.ESTestCase;
31+
import org.elasticsearch.test.AbstractWireSerializingTestCase;
3132

3233
import java.io.IOException;
3334
import java.util.ArrayList;
@@ -39,14 +40,8 @@
3940
import java.util.List;
4041
import java.util.Map;
4142

42-
public class RestoreSnapshotRequestTests extends ESTestCase {
43-
// tests creating XContent and parsing with source(Map) equivalency
44-
public void testToXContent() throws IOException {
45-
String repo = randomAlphaOfLength(5);
46-
String snapshot = randomAlphaOfLength(10);
47-
48-
RestoreSnapshotRequest original = new RestoreSnapshotRequest(repo, snapshot);
49-
43+
public class RestoreSnapshotRequestTests extends AbstractWireSerializingTestCase<RestoreSnapshotRequest> {
44+
private RestoreSnapshotRequest randomState(RestoreSnapshotRequest instance) {
5045
if (randomBoolean()) {
5146
List<String> indices = new ArrayList<>();
5247
int count = randomInt(3) + 1;
@@ -55,16 +50,16 @@ public void testToXContent() throws IOException {
5550
indices.add(randomAlphaOfLength(randomInt(3) + 2));
5651
}
5752

58-
original.indices(indices);
53+
instance.indices(indices);
5954
}
6055
if (randomBoolean()) {
61-
original.renamePattern(randomUnicodeOfLengthBetween(1, 100));
56+
instance.renamePattern(randomUnicodeOfLengthBetween(1, 100));
6257
}
6358
if (randomBoolean()) {
64-
original.renameReplacement(randomUnicodeOfLengthBetween(1, 100));
59+
instance.renameReplacement(randomUnicodeOfLengthBetween(1, 100));
6560
}
66-
original.partial(randomBoolean());
67-
original.includeAliases(randomBoolean());
61+
instance.partial(randomBoolean());
62+
instance.includeAliases(randomBoolean());
6863

6964
if (randomBoolean()) {
7065
Map<String, Object> settings = new HashMap<>();
@@ -74,7 +69,7 @@ public void testToXContent() throws IOException {
7469
settings.put(randomAlphaOfLengthBetween(2, 5), randomAlphaOfLengthBetween(2, 5));
7570
}
7671

77-
original.settings(settings);
72+
instance.settings(settings);
7873
}
7974
if (randomBoolean()) {
8075
Map<String, Object> indexSettings = new HashMap<>();
@@ -83,28 +78,50 @@ public void testToXContent() throws IOException {
8378
for (int i = 0; i < count; ++i) {
8479
indexSettings.put(randomAlphaOfLengthBetween(2, 5), randomAlphaOfLengthBetween(2, 5));;
8580
}
86-
original.indexSettings(indexSettings);
81+
instance.indexSettings(indexSettings);
8782
}
8883

89-
original.includeGlobalState(randomBoolean());
84+
instance.includeGlobalState(randomBoolean());
9085

9186
if (randomBoolean()) {
9287
Collection<IndicesOptions.WildcardStates> wildcardStates = randomSubsetOf(
9388
Arrays.asList(IndicesOptions.WildcardStates.values()));
9489
Collection<IndicesOptions.Option> options = randomSubsetOf(
9590
Arrays.asList(IndicesOptions.Option.ALLOW_NO_INDICES, IndicesOptions.Option.IGNORE_UNAVAILABLE));
9691

97-
original.indicesOptions(new IndicesOptions(
92+
instance.indicesOptions(new IndicesOptions(
9893
options.isEmpty() ? IndicesOptions.Option.NONE : EnumSet.copyOf(options),
9994
wildcardStates.isEmpty() ? IndicesOptions.WildcardStates.NONE : EnumSet.copyOf(wildcardStates)));
10095
}
10196

102-
original.waitForCompletion(randomBoolean());
97+
instance.waitForCompletion(randomBoolean());
10398

10499
if (randomBoolean()) {
105-
original.masterNodeTimeout("60s");
100+
instance.masterNodeTimeout(randomTimeValue());
106101
}
102+
return instance;
103+
}
104+
105+
@Override
106+
protected RestoreSnapshotRequest createTestInstance() {
107+
return randomState(new RestoreSnapshotRequest(randomAlphaOfLength(5), randomAlphaOfLength(10)));
108+
}
109+
110+
@Override
111+
protected Writeable.Reader<RestoreSnapshotRequest> instanceReader() {
112+
return RestoreSnapshotRequest::new;
113+
}
114+
115+
@Override
116+
protected RestoreSnapshotRequest mutateInstance(RestoreSnapshotRequest instance) throws IOException {
117+
RestoreSnapshotRequest copy = copyInstance(instance);
118+
// ensure that at least one property is different
119+
copy.repository("copied-" + instance.repository());
120+
return randomState(copy);
121+
}
107122

123+
public void testSource() throws IOException {
124+
RestoreSnapshotRequest original = createTestInstance();
108125
XContentBuilder builder = original.toXContent(XContentFactory.jsonBuilder(), new ToXContent.MapParams(Collections.emptyMap()));
109126
XContentParser parser = XContentType.JSON.xContent().createParser(
110127
NamedXContentRegistry.EMPTY, null, BytesReference.bytes(builder).streamInput());
@@ -113,7 +130,7 @@ public void testToXContent() throws IOException {
113130
// we will only restore properties from the map that are contained in the request body. All other
114131
// properties are restored from the original (in the actual REST action this is restored from the
115132
// REST path and request parameters).
116-
RestoreSnapshotRequest processed = new RestoreSnapshotRequest(repo, snapshot);
133+
RestoreSnapshotRequest processed = new RestoreSnapshotRequest(original.repository(), original.snapshot());
117134
processed.masterNodeTimeout(original.masterNodeTimeout());
118135
processed.waitForCompletion(original.waitForCompletion());
119136

‎test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,11 @@ protected static void createIndex(String name, Settings settings, String mapping
536536
client().performRequest(request);
537537
}
538538

539+
protected static void deleteIndex(String name) throws IOException {
540+
Request request = new Request("DELETE", "/" + name);
541+
client().performRequest(request);
542+
}
543+
539544
protected static void updateIndexSettings(String index, Settings.Builder settings) throws IOException {
540545
updateIndexSettings(index, settings.build());
541546
}

0 commit comments

Comments
 (0)