Skip to content

Deprecate passing settings in restore requests #53268

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

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 @@ -19,13 +19,16 @@

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

import org.apache.logging.log4j.LogManager;
import org.elasticsearch.ElasticsearchGenerationException;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.master.MasterNodeRequest;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
Expand All @@ -50,6 +53,8 @@
*/
public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotRequest> implements ToXContentObject {

private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(LogManager.getLogger(RestoreSnapshotRequest.class));

private String snapshot;
private String repository;
private String[] indices = Strings.EMPTY_ARRAY;
Expand All @@ -60,7 +65,6 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
private boolean includeGlobalState = false;
private boolean partial = false;
private boolean includeAliases = true;
private Settings settings = EMPTY_SETTINGS;
private Settings indexSettings = EMPTY_SETTINGS;
private String[] ignoreIndexSettings = Strings.EMPTY_ARRAY;

Expand Down Expand Up @@ -90,7 +94,9 @@ public RestoreSnapshotRequest(StreamInput in) throws IOException {
includeGlobalState = in.readBoolean();
partial = in.readBoolean();
includeAliases = in.readBoolean();
settings = readSettingsFromStream(in);
if (in.getVersion().before(Version.V_8_0_0)) {
readSettingsFromStream(in); // formerly the unused settings field
}
indexSettings = readSettingsFromStream(in);
ignoreIndexSettings = in.readStringArray();
}
Expand All @@ -108,7 +114,9 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeBoolean(includeGlobalState);
out.writeBoolean(partial);
out.writeBoolean(includeAliases);
writeSettingsToStream(settings, out);
if (out.getVersion().before(Version.V_8_0_0)) {
writeSettingsToStream(Settings.EMPTY, out); // formerly the unused settings field
}
writeSettingsToStream(indexSettings, out);
out.writeStringArray(ignoreIndexSettings);
}
Expand All @@ -128,9 +136,6 @@ public ActionRequestValidationException validate() {
if (indicesOptions == null) {
validationException = addValidationError("indicesOptions is missing", validationException);
}
if (settings == null) {
validationException = addValidationError("settings are missing", validationException);
}
if (indexSettings == null) {
validationException = addValidationError("indexSettings are missing", validationException);
}
Expand Down Expand Up @@ -324,74 +329,6 @@ public RestoreSnapshotRequest partial(boolean partial) {
return this;
}

/**
* Sets repository-specific restore settings.
* <p>
* See repository documentation for more information.
*
* @param settings repository-specific snapshot settings
* @return this request
*/
public RestoreSnapshotRequest settings(Settings settings) {
this.settings = settings;
return this;
}

/**
* Sets repository-specific restore settings.
* <p>
* See repository documentation for more information.
*
* @param settings repository-specific snapshot settings
* @return this request
*/
public RestoreSnapshotRequest settings(Settings.Builder settings) {
this.settings = settings.build();
return this;
}

/**
* Sets repository-specific restore settings in JSON or YAML format
* <p>
* See repository documentation for more information.
*
* @param source repository-specific snapshot settings
* @param xContentType the content type of the source
* @return this request
*/
public RestoreSnapshotRequest settings(String source, XContentType xContentType) {
this.settings = Settings.builder().loadFromSource(source, xContentType).build();
return this;
}

/**
* Sets repository-specific restore settings
* <p>
* See repository documentation for more information.
*
* @param source repository-specific snapshot settings
* @return this request
*/
public RestoreSnapshotRequest settings(Map<String, Object> source) {
try {
XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON);
builder.map(source);
settings(Strings.toString(builder), builder.contentType());
} catch (IOException e) {
throw new ElasticsearchGenerationException("Failed to generate [" + source + "]", e);
}
return this;
}

/**
* Returns repository-specific restore settings
*
* @return restore settings
*/
public Settings settings() {
return this.settings;
}

/**
* Sets the list of index settings and index settings groups that shouldn't be restored from snapshot
*/
Expand Down Expand Up @@ -526,7 +463,8 @@ public RestoreSnapshotRequest source(Map<String, Object> source) {
if (!(entry.getValue() instanceof Map)) {
throw new IllegalArgumentException("malformed settings section");
}
settings((Map<String, Object>) entry.getValue());
DEPRECATION_LOGGER.deprecatedAndMaybeLog("RestoreSnapshotRequest#settings",
"specifying [settings] when restoring a snapshot has no effect and will not be supported in a future version");
} else if (name.equals("include_global_state")) {
includeGlobalState = nodeBooleanValue(entry.getValue(), "include_global_state");
} else if (name.equals("include_aliases")) {
Expand Down Expand Up @@ -586,13 +524,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.field("include_global_state", includeGlobalState);
builder.field("partial", partial);
builder.field("include_aliases", includeAliases);
if (settings != null) {
builder.startObject("settings");
if (settings.isEmpty() == false) {
settings.toXContent(builder, params);
}
builder.endObject();
}
if (indexSettings != null) {
builder.startObject("index_settings");
if (indexSettings.isEmpty() == false) {
Expand Down Expand Up @@ -629,15 +560,14 @@ public boolean equals(Object o) {
Objects.equals(indicesOptions, that.indicesOptions) &&
Objects.equals(renamePattern, that.renamePattern) &&
Objects.equals(renameReplacement, that.renameReplacement) &&
Objects.equals(settings, that.settings) &&
Objects.equals(indexSettings, that.indexSettings) &&
Arrays.equals(ignoreIndexSettings, that.ignoreIndexSettings);
}

@Override
public int hashCode() {
int result = Objects.hash(snapshot, repository, indicesOptions, renamePattern, renameReplacement, waitForCompletion,
includeGlobalState, partial, includeAliases, settings, indexSettings);
includeGlobalState, partial, includeAliases, indexSettings);
result = 31 * result + Arrays.hashCode(indices);
result = 31 * result + Arrays.hashCode(ignoreIndexSettings);
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,60 +127,6 @@ public RestoreSnapshotRequestBuilder setRenameReplacement(String renameReplaceme
return this;
}


/**
* Sets repository-specific restore settings.
* <p>
* See repository documentation for more information.
*
* @param settings repository-specific snapshot settings
* @return this builder
*/
public RestoreSnapshotRequestBuilder setSettings(Settings settings) {
request.settings(settings);
return this;
}

/**
* Sets repository-specific restore settings.
* <p>
* See repository documentation for more information.
*
* @param settings repository-specific snapshot settings
* @return this builder
*/
public RestoreSnapshotRequestBuilder setSettings(Settings.Builder settings) {
request.settings(settings);
return this;
}

/**
* Sets repository-specific restore settings in JSON or YAML format
* <p>
* See repository documentation for more information.
*
* @param source repository-specific snapshot settings
* @param xContentType the content type of the source
* @return this builder
*/
public RestoreSnapshotRequestBuilder setSettings(String source, XContentType xContentType) {
request.settings(source, xContentType);
return this;
}

/**
* Sets repository-specific restore settings
* <p>
* See repository documentation for more information.
*
* @param source repository-specific snapshot settings
* @return this builder
*/
public RestoreSnapshotRequestBuilder setSettings(Map<String, Object> source) {
request.settings(source);
return this;
}

/**
* If this parameter is set to true the operation will wait for completion of restore process before returning.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,6 @@ private RestoreSnapshotRequest randomState(RestoreSnapshotRequest instance) {
instance.partial(randomBoolean());
instance.includeAliases(randomBoolean());

if (randomBoolean()) {
Map<String, Object> settings = new HashMap<>();
int count = randomInt(3) + 1;

for (int i = 0; i < count; ++i) {
settings.put(randomAlphaOfLengthBetween(2, 5), randomAlphaOfLengthBetween(2, 5));
}

instance.settings(settings);
}
if (randomBoolean()) {
Map<String, Object> indexSettings = new HashMap<>();
int count = randomInt(3) + 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,13 @@ public void testRestoreSnapshotRequestParsing() throws IOException {
assertEquals("rename-from", request.renamePattern());
assertEquals("rename-to", request.renameReplacement());
assertEquals(partial, request.partial());
assertEquals("val1", request.settings().get("set1"));
assertArrayEquals(request.ignoreIndexSettings(), new String[]{"set2", "set3"});
boolean expectedIgnoreAvailable = includeIgnoreUnavailable
? indicesOptions.ignoreUnavailable()
: IndicesOptions.strictExpandOpen().ignoreUnavailable();
assertEquals(expectedIgnoreAvailable, request.indicesOptions().ignoreUnavailable());

assertWarnings("specifying [settings] when restoring a snapshot has no effect and will not be supported in a future version");
}

public void testCreateSnapshotRequestParsing() throws IOException {
Expand Down