Allow empty/missing SLM retention configuration

Semi-related to elastic#44465, this allows the `"retention"` configuration map to be missing. Relates to elastic#43663
dakrone · Jul 30, 2019 · 5ba2fcc · 5ba2fcc
1 parent 646f5bb
commit 5ba2fcc
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 15 deletions.
diff --git a/...t/rest-high-level/src/main/java/org/elasticsearch/client/slm/SnapshotLifecyclePolicy.java b/...t/rest-high-level/src/main/java/org/elasticsearch/client/slm/SnapshotLifecyclePolicy.java
@@ -63,16 +63,16 @@ public class SnapshotLifecyclePolicy implements ToXContentObject {
         PARSER.declareString(ConstructingObjectParser.constructorArg(), SCHEDULE);
         PARSER.declareString(ConstructingObjectParser.constructorArg(), REPOSITORY);
         PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.map(), CONFIG);
-        PARSER.declareObject(ConstructingObjectParser.constructorArg(), SnapshotRetentionConfiguration::parse, RETENTION);
+        PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), SnapshotRetentionConfiguration::parse, RETENTION);
     }
 
     public SnapshotLifecyclePolicy(final String id, final String name, final String schedule,
-                                   final String repository, @Nullable Map<String, Object> configuration,
-                                   SnapshotRetentionConfiguration retentionPolicy) {
-        this.id = Objects.requireNonNull(id);
-        this.name = name;
-        this.schedule = schedule;
-        this.repository = repository;
+                                   final String repository, @Nullable final Map<String, Object> configuration,
+                                   @Nullable final SnapshotRetentionConfiguration retentionPolicy) {
+        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;
         this.retentionPolicy = retentionPolicy;
     }
@@ -98,6 +98,7 @@ public Map<String, Object> getConfig() {
         return this.configuration;
     }
 
+    @Nullable
     public SnapshotRetentionConfiguration getRetentionPolicy() {
         return this.retentionPolicy;
     }
@@ -115,7 +116,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         if (this.configuration != null) {
             builder.field(CONFIG.getPreferredName(), this.configuration);
         }
-        builder.field(RETENTION.getPreferredName(), this.retentionPolicy);
+        if (this.retentionPolicy != null) {
+            builder.field(RETENTION.getPreferredName(), this.retentionPolicy);
+        }
         builder.endObject();
         return builder;
     }

diff --git a/...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicy.java b/...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicy.java
@@ -82,12 +82,12 @@ public class SnapshotLifecyclePolicy extends AbstractDiffable<SnapshotLifecycleP
         PARSER.declareString(ConstructingObjectParser.constructorArg(), SCHEDULE);
         PARSER.declareString(ConstructingObjectParser.constructorArg(), REPOSITORY);
         PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), (p, c) -> p.map(), CONFIG);
-        PARSER.declareObject(ConstructingObjectParser.constructorArg(), SnapshotRetentionConfiguration::parse, RETENTION);
+        PARSER.declareObject(ConstructingObjectParser.optionalConstructorArg(), SnapshotRetentionConfiguration::parse, RETENTION);
     }
 
     public SnapshotLifecyclePolicy(final String id, final String name, final String schedule,
-                                   final String repository, @Nullable Map<String, Object> configuration,
-                                   final SnapshotRetentionConfiguration retentionPolicy) {
+                                   final String repository, @Nullable final Map<String, Object> configuration,
+                                   @Nullable final SnapshotRetentionConfiguration retentionPolicy) {
         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");
@@ -102,7 +102,7 @@ public SnapshotLifecyclePolicy(StreamInput in) throws IOException {
         this.schedule = in.readString();
         this.repository = in.readString();
         this.configuration = in.readMap();
-        this.retentionPolicy = new SnapshotRetentionConfiguration(in);
+        this.retentionPolicy = in.readOptionalWriteable(SnapshotRetentionConfiguration::new);
     }
 
     public String getId() {
@@ -126,6 +126,7 @@ public Map<String, Object> getConfig() {
         return this.configuration;
     }
 
+    @Nullable
     public SnapshotRetentionConfiguration getRetentionPolicy() {
         return this.retentionPolicy;
     }
@@ -271,7 +272,7 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeString(this.schedule);
         out.writeString(this.repository);
         out.writeMap(this.configuration);
-        this.retentionPolicy.writeTo(out);
+        out.writeOptionalWriteable(this.retentionPolicy);
     }
 
     @Override
@@ -283,7 +284,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         if (this.configuration != null) {
             builder.field(CONFIG.getPreferredName(), this.configuration);
         }
-        builder.field(RETENTION.getPreferredName(), this.retentionPolicy);
+        if (this.retentionPolicy != null) {
+            builder.field(RETENTION.getPreferredName(), this.retentionPolicy);
+        }
         builder.endObject();
         return builder;
     }

diff --git a/.../src/test/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadataTests.java b/.../src/test/java/org/elasticsearch/xpack/core/slm/SnapshotLifecyclePolicyMetadataTests.java
@@ -111,7 +111,7 @@ public static SnapshotLifecyclePolicy randomSnapshotLifecyclePolicy(String polic
     }
 
     public static SnapshotRetentionConfiguration randomRetention() {
-        return new SnapshotRetentionConfiguration(rarely() ? null :
+        return rarely() ? null : new SnapshotRetentionConfiguration(rarely() ? null :
             TimeValue.parseTimeValue(randomTimeValue(), "random retention generation"));
     }