Deprecate passing settings in restore requests (#53268)

Today we accept a `settings` field in snapshot restore requests, but this field
is not used. This commit deprecates it.
This commit is contained in:
David Turner 2020-03-09 10:11:17 +00:00
parent 2fd954a3b7
commit 52ff341814
4 changed files with 16 additions and 149 deletions

View File

@ -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;
@ -50,6 +53,8 @@ import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBo
*/
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;
@ -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;
@ -90,7 +94,9 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
includeGlobalState = in.readBoolean();
partial = in.readBoolean();
includeAliases = in.readBoolean();
settings = readSettingsFromStream(in);
if (in.getVersion().before(Version.V_7_7_0)) {
readSettingsFromStream(in); // formerly the unused settings field
}
indexSettings = readSettingsFromStream(in);
ignoreIndexSettings = in.readStringArray();
}
@ -108,7 +114,9 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
out.writeBoolean(includeGlobalState);
out.writeBoolean(partial);
out.writeBoolean(includeAliases);
writeSettingsToStream(settings, out);
if (out.getVersion().before(Version.V_7_7_0)) {
writeSettingsToStream(Settings.EMPTY, out); // formerly the unused settings field
}
writeSettingsToStream(indexSettings, out);
out.writeStringArray(ignoreIndexSettings);
}
@ -128,9 +136,6 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
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);
}
@ -324,74 +329,6 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
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
*/
@ -526,7 +463,8 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
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")) {
@ -586,13 +524,6 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
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) {
@ -629,7 +560,6 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
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);
}
@ -637,7 +567,7 @@ public class RestoreSnapshotRequest extends MasterNodeRequest<RestoreSnapshotReq
@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;

View File

@ -127,60 +127,6 @@ public class RestoreSnapshotRequestBuilder extends MasterNodeOperationRequestBui
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.
*

View File

@ -61,16 +61,6 @@ public class RestoreSnapshotRequestTests extends AbstractWireSerializingTestCase
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;

View File

@ -91,12 +91,13 @@ public class SnapshotRequestsTests extends ESTestCase {
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 {