Skip to content

Commit

Permalink
Allow empty configuration for SLM policies (#44465)
Browse files Browse the repository at this point in the history 
* Allow empty configuration for SLM policies

When putting or updating a snapshot lifecycle policy it was not possible
to elide the `config` map. This commit makes the configuration optional,
the same way that it is when taking a snapshot.

Relates to #38461

* Add Objects.requireNonNull for required parts of the policy
  • Loading branch information
@dakrone
dakrone committed Jul 18, 2019
1 parent fe2ef66 commit 3001f79
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 21 deletions.
13 changes: 7 additions & 6 deletions ...vel/src/main/java/org/elasticsearch/client/snapshotlifecycle/SnapshotLifecyclePolicy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

package org.elasticsearch.client.snapshotlifecycle;

import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.xcontent.ConstructingObjectParser;
Expand All @@ -43,8 +43,6 @@ public class SnapshotLifecyclePolicy implements ToXContentObject {
private static final ParseField SCHEDULE = new ParseField("schedule");
private static final ParseField REPOSITORY = new ParseField("repository");
private static final ParseField CONFIG = new ParseField("config");
private static final IndexNameExpressionResolver.DateMathExpressionResolver DATE_MATH_RESOLVER =
new IndexNameExpressionResolver.DateMathExpressionResolver();

@SuppressWarnings("unchecked")
private static final ConstructingObjectParser<SnapshotLifecyclePolicy, String> PARSER =
Expand All @@ -61,11 +59,11 @@ public class SnapshotLifecyclePolicy implements ToXContentObject {
PARSER.declareString(ConstructingObjectParser.constructorArg(), NAME);
PARSER.declareString(ConstructingObjectParser.constructorArg(), SCHEDULE);
PARSER.declareString(ConstructingObjectParser.constructorArg(), REPOSITORY);
PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> p.map(), CONFIG);
PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.map(), CONFIG);
}

public SnapshotLifecyclePolicy(final String id, final String name, final String schedule,
final String repository, Map<String, Object> configuration) {
final String repository, @Nullable Map<String, Object> configuration) {
this.id = Objects.requireNonNull(id);
this.name = name;
this.schedule = schedule;
Expand All @@ -89,6 +87,7 @@ public String getRepository() {
return this.repository;
}

@Nullable
public Map<String, Object> getConfig() {
return this.configuration;
}
Expand All @@ -103,7 +102,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.field(NAME.getPreferredName(), this.name);
builder.field(SCHEDULE.getPreferredName(), this.schedule);
builder.field(REPOSITORY.getPreferredName(), this.repository);
builder.field(CONFIG.getPreferredName(), this.configuration);
if (this.configuration != null) {
builder.field(CONFIG.getPreferredName(), this.configuration);
}
builder.endObject();
return builder;
}
Expand Down
20 changes: 12 additions & 8 deletions ...src/main/java/org/elasticsearch/xpack/core/snapshotlifecycle/SnapshotLifecyclePolicy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.elasticsearch.cluster.Diffable;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.Context;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.UUIDs;
Expand Down Expand Up @@ -76,15 +77,15 @@ public class SnapshotLifecyclePolicy extends AbstractDiffable<SnapshotLifecycleP
PARSER.declareString(ConstructingObjectParser.constructorArg(), NAME);
PARSER.declareString(ConstructingObjectParser.constructorArg(), SCHEDULE);
PARSER.declareString(ConstructingObjectParser.constructorArg(), REPOSITORY);
PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> p.map(), CONFIG);
PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.map(), CONFIG);
}

public SnapshotLifecyclePolicy(final String id, final String name, final String schedule,
final String repository, Map<String, Object> configuration) {
this.id = Objects.requireNonNull(id);
this.name = name;
this.schedule = schedule;
this.repository = repository;
final String repository, @Nullable Map<String, Object> configuration) {
this.id = Objects.requireNonNull(id, "policy id is required");
this.name = Objects.requireNonNull(name, "policy snapshot name is required");
this.schedule = Objects.requireNonNull(schedule, "policy schedule is required");
this.repository = Objects.requireNonNull(repository, "policy snapshot repository is required");
this.configuration = configuration;
}

Expand Down Expand Up @@ -112,6 +113,7 @@ public String getRepository() {
return this.repository;
}

@Nullable
public Map<String, Object> getConfig() {
return this.configuration;
}
Expand Down Expand Up @@ -172,7 +174,7 @@ public ActionRequestValidationException validate() {
}
}

if (configuration.containsKey(METADATA_FIELD_NAME)) {
if (configuration != null && configuration.containsKey(METADATA_FIELD_NAME)) {
if (configuration.get(METADATA_FIELD_NAME) instanceof Map == false) {
err.addValidationError("invalid configuration." + METADATA_FIELD_NAME + " [" + configuration.get(METADATA_FIELD_NAME) +
"]: must be an object if present");
Expand Down Expand Up @@ -265,7 +267,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.field(NAME.getPreferredName(), this.name);
builder.field(SCHEDULE.getPreferredName(), this.schedule);
builder.field(REPOSITORY.getPreferredName(), this.repository);
builder.field(CONFIG.getPreferredName(), this.configuration);
if (this.configuration != null) {
builder.field(CONFIG.getPreferredName(), this.configuration);
}
builder.endObject();
return builder;
}
Expand Down
2 changes: 1 addition & 1 deletion ...main/java/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryItem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public static SnapshotHistoryItem parse(XContentParser parser, String name) {
this.snapshotName = Objects.requireNonNull(snapshotName);
this.operation = Objects.requireNonNull(operation);
this.success = success;
this.snapshotConfiguration = Objects.requireNonNull(snapshotConfiguration);
this.snapshotConfiguration = snapshotConfiguration;
this.errorDetails = errorDetails;
}

Expand Down
9 changes: 6 additions & 3 deletions ...ava/org/elasticsearch/xpack/core/snapshotlifecycle/history/SnapshotHistoryStoreTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,12 @@ public void testIndexNameGeneration() {
}

public static SnapshotLifecyclePolicy randomSnapshotLifecyclePolicy(String id) {
Map<String, Object> config = new HashMap<>();
for (int i = 0; i < randomIntBetween(2, 5); i++) {
config.put(randomAlphaOfLength(4), randomAlphaOfLength(4));
Map<String, Object> config = null;
if (randomBoolean()) {
config = new HashMap<>();
for (int i = 0; i < randomIntBetween(2, 5); i++) {
config.put(randomAlphaOfLength(4), randomAlphaOfLength(4));
}
}
return new SnapshotLifecyclePolicy(id,
randomAlphaOfLength(4),
Expand Down
9 changes: 6 additions & 3 deletions ...src/test/java/org/elasticsearch/xpack/snapshotlifecycle/SnapshotLifecyclePolicyTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,12 @@ protected SnapshotLifecyclePolicy createTestInstance() {
}

public static SnapshotLifecyclePolicy randomSnapshotLifecyclePolicy(String id) {
Map<String, Object> config = new HashMap<>();
for (int i = 0; i < randomIntBetween(2, 5); i++) {
config.put(randomAlphaOfLength(4), randomAlphaOfLength(4));
Map<String, Object> config = null;
if (randomBoolean()) {
config = new HashMap<>();
for (int i = 0; i < randomIntBetween(2, 5); i++) {
config.put(randomAlphaOfLength(4), randomAlphaOfLength(4));
}
}
return new SnapshotLifecyclePolicy(id,
randomAlphaOfLength(4),
Expand Down

0 comments on commit 3001f79

Please sign in to comment.