Skip to content

Commit

Permalink
Fix ILM Lifecycle Policy to allow unknown fields (#38125)
Browse files Browse the repository at this point in the history
A few of the ILM Lifecycle Policy and classes did not allow for unknown
fields. This commit sets those parsers and fixes the tests for the
parsers.

Relates #36938
Relates #38041
  • Loading branch information
hub-cap authored Feb 1, 2019
1 parent 0feb2a6 commit b49811a
Show file tree
Hide file tree
Showing 25 changed files with 87 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class AllocateAction implements LifecycleAction, ToXContentObject {
static final ParseField REQUIRE_FIELD = new ParseField("require");

@SuppressWarnings("unchecked")
private static final ConstructingObjectParser<AllocateAction, Void> PARSER = new ConstructingObjectParser<>(NAME,
private static final ConstructingObjectParser<AllocateAction, Void> PARSER = new ConstructingObjectParser<>(NAME, true,
a -> new AllocateAction((Integer) a[0], (Map<String, String>) a[1], (Map<String, String>) a[2], (Map<String, String>) a[3]));

static {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
public class DeleteAction implements LifecycleAction, ToXContentObject {
public static final String NAME = "delete";

private static final ObjectParser<DeleteAction, Void> PARSER = new ObjectParser<>(NAME, DeleteAction::new);
private static final ObjectParser<DeleteAction, Void> PARSER = new ObjectParser<>(NAME, true, DeleteAction::new);

public static DeleteAction parse(XContentParser parser) {
return PARSER.apply(parser, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class ForceMergeAction implements LifecycleAction, ToXContentObject {
private static final ParseField MAX_NUM_SEGMENTS_FIELD = new ParseField("max_num_segments");

private static final ConstructingObjectParser<ForceMergeAction, Void> PARSER = new ConstructingObjectParser<>(NAME,
false, a -> {
true, a -> {
int maxNumSegments = (int) a[0];
return new ForceMergeAction(maxNumSegments);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
public class FreezeAction implements LifecycleAction, ToXContentObject {
public static final String NAME = "freeze";

private static final ObjectParser<FreezeAction, Void> PARSER = new ObjectParser<>(NAME, FreezeAction::new);
private static final ObjectParser<FreezeAction, Void> PARSER = new ObjectParser<>(NAME, true, FreezeAction::new);

public static FreezeAction parse(XContentParser parser) {
return PARSER.apply(parser, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class LifecyclePolicy implements ToXContentObject {
static final ParseField PHASES_FIELD = new ParseField("phases");

@SuppressWarnings("unchecked")
public static ConstructingObjectParser<LifecyclePolicy, String> PARSER = new ConstructingObjectParser<>("lifecycle_policy", false,
public static ConstructingObjectParser<LifecyclePolicy, String> PARSER = new ConstructingObjectParser<>("lifecycle_policy", true,
(a, name) -> {
List<Phase> phases = (List<Phase>) a[0];
Map<String, Phase> phaseMap = phases.stream().collect(Collectors.toMap(Phase::getName, Function.identity()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ public class LifecyclePolicyMetadata implements ToXContentObject {
static final ParseField MODIFIED_DATE = new ParseField("modified_date");

@SuppressWarnings("unchecked")
public static final ConstructingObjectParser<LifecyclePolicyMetadata, String> PARSER = new ConstructingObjectParser<>("policy_metadata",
public static final ConstructingObjectParser<LifecyclePolicyMetadata, String> PARSER = new ConstructingObjectParser<>(
"policy_metadata", true,
a -> {
LifecyclePolicy policy = (LifecyclePolicy) a[0];
return new LifecyclePolicyMetadata(policy, (long) a[1], ZonedDateTime.parse((String) a[2]).toInstant().toEpochMilli());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class Phase implements ToXContentObject {
static final ParseField ACTIONS_FIELD = new ParseField("actions");

@SuppressWarnings("unchecked")
private static final ConstructingObjectParser<Phase, String> PARSER = new ConstructingObjectParser<>("phase", false,
private static final ConstructingObjectParser<Phase, String> PARSER = new ConstructingObjectParser<>("phase", true,
(a, name) -> new Phase(name, (TimeValue) a[0], ((List<LifecycleAction>) a[1]).stream()
.collect(Collectors.toMap(LifecycleAction::getName, Function.identity()))));
static {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class PhaseExecutionInfo implements ToXContentObject {
private static final ParseField MODIFIED_DATE_IN_MILLIS_FIELD = new ParseField("modified_date_in_millis");

private static final ConstructingObjectParser<PhaseExecutionInfo, String> PARSER = new ConstructingObjectParser<>(
"phase_execution_info", false,
"phase_execution_info", true,
(a, name) -> new PhaseExecutionInfo((String) a[0], (Phase) a[1], (long) a[2], (long) a[3]));
static {
PARSER.declareString(ConstructingObjectParser.constructorArg(), POLICY_NAME_FIELD);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
public class ReadOnlyAction implements LifecycleAction, ToXContentObject {
public static final String NAME = "readonly";

private static final ObjectParser<ReadOnlyAction, Void> PARSER = new ObjectParser<>(NAME, false, ReadOnlyAction::new);
private static final ObjectParser<ReadOnlyAction, Void> PARSER = new ObjectParser<>(NAME, true, ReadOnlyAction::new);

public static ReadOnlyAction parse(XContentParser parser) {
return PARSER.apply(parser, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class RolloverAction implements LifecycleAction, ToXContentObject {
private static final ParseField MAX_DOCS_FIELD = new ParseField("max_docs");
private static final ParseField MAX_AGE_FIELD = new ParseField("max_age");

private static final ConstructingObjectParser<RolloverAction, Void> PARSER = new ConstructingObjectParser<>(NAME,
private static final ConstructingObjectParser<RolloverAction, Void> PARSER = new ConstructingObjectParser<>(NAME, true,
a -> new RolloverAction((ByteSizeValue) a[0], (TimeValue) a[1], (Long) a[2]));
static {
PARSER.declareField(ConstructingObjectParser.optionalConstructorArg(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class ShrinkAction implements LifecycleAction, ToXContentObject {
private static final ParseField NUMBER_OF_SHARDS_FIELD = new ParseField("number_of_shards");

private static final ConstructingObjectParser<ShrinkAction, Void> PARSER =
new ConstructingObjectParser<>(NAME, a -> new ShrinkAction((Integer) a[0]));
new ConstructingObjectParser<>(NAME, true, a -> new ShrinkAction((Integer) a[0]));

static {
PARSER.declareInt(ConstructingObjectParser.constructorArg(), NUMBER_OF_SHARDS_FIELD);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
public class UnfollowAction implements LifecycleAction, ToXContentObject {
public static final String NAME = "unfollow";

private static final ObjectParser<UnfollowAction, Void> PARSER = new ObjectParser<>(NAME, UnfollowAction::new);
private static final ObjectParser<UnfollowAction, Void> PARSER = new ObjectParser<>(NAME, true, UnfollowAction::new);

public UnfollowAction() {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.function.Predicate;

public class AllocateActionTests extends AbstractXContentTestCase<AllocateAction> {

Expand Down Expand Up @@ -65,7 +66,14 @@ protected AllocateAction doParseInstance(XContentParser parser) {

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

@Override
protected Predicate<String> getRandomFieldsExcludeFilter() {
// this whole structure expects to be maps of strings, so more complex objects would just mess that up.
// setting it this way allows for new fields at the root
return (field) -> field.isEmpty() == false;
}

public void testAllMapsNullOrEmpty() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ protected DeleteAction doParseInstance(XContentParser parser) {

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ protected ForceMergeAction doParseInstance(XContentParser parser) {

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ protected FreezeAction doParseInstance(XContentParser parser) {

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.function.Predicate;

import static org.elasticsearch.client.indexlifecycle.LifecyclePolicyTests.createRandomPolicy;

Expand All @@ -54,7 +55,23 @@ protected GetLifecyclePolicyResponse doParseInstance(XContentParser parser) thro

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

@Override
protected Predicate<String> getRandomFieldsExcludeFilter() {
return (field) ->
// phases is a list of Phase parsable entries only
field.endsWith(".phases")
// these are all meant to be maps of strings, so complex objects will confuse the parser
|| field.endsWith(".include")
|| field.endsWith(".exclude")
|| field.endsWith(".require")
// actions are meant to be a list of LifecycleAction parsable entries only
|| field.endsWith(".actions")
// field.isEmpty() means do not insert an object at the root of the json. This parser expects
// every root level named object to be parsable as a specific type
|| field.isEmpty();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.function.Predicate;

import static org.elasticsearch.client.indexlifecycle.LifecyclePolicyTests.createRandomPolicy;

Expand All @@ -50,7 +51,21 @@ protected LifecyclePolicyMetadata doParseInstance(XContentParser parser) throws

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

@Override
protected Predicate<String> getRandomFieldsExcludeFilter() {
return (field) ->
// phases is a list of Phase parsable entries only
field.endsWith(".phases")
// these are all meant to be maps of strings, so complex objects will confuse the parser
|| field.endsWith(".include")
|| field.endsWith(".exclude")
|| field.endsWith(".require")
// actions are meant to be a list of LifecycleAction parsable entries only
|| field.endsWith(".actions");

}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import static org.hamcrest.Matchers.equalTo;
Expand All @@ -55,7 +56,13 @@ protected LifecyclePolicy doParseInstance(XContentParser parser) {

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

@Override
protected Predicate<String> getRandomFieldsExcludeFilter() {
// these items all have some specific parsing that does not allow them to have additional objects within them.
return (field) -> field.contains("allocate.") || field.equals("phases") || field.endsWith("actions");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.function.Predicate;

public class PhaseExecutionInfoTests extends AbstractXContentTestCase<PhaseExecutionInfo> {

Expand All @@ -53,9 +54,15 @@ protected PhaseExecutionInfo doParseInstance(XContentParser parser) throws IOExc
return PhaseExecutionInfo.parse(parser, phaseName);
}

@Override
protected Predicate<String> getRandomFieldsExcludeFilter() {
// actions are plucked from the named registry, and it fails if the action is not in the named registry
return (field) -> field.equals("phase_definition.actions");
}

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.function.Predicate;

public class PhaseTests extends AbstractXContentTestCase<Phase> {
private String phaseName;
Expand Down Expand Up @@ -61,6 +62,12 @@ protected Phase doParseInstance(XContentParser parser) {
return Phase.parse(parser, phaseName);
}

@Override
protected Predicate<String> getRandomFieldsExcludeFilter() {
// actions are plucked from the named registry, and it fails if the action is not in the named registry
return (field) -> field.equals("actions");
}

@Override
protected NamedXContentRegistry xContentRegistry() {
List<NamedXContentRegistry.Entry> entries = new ArrayList<>(ClusterModule.getNamedXWriteables());
Expand All @@ -70,7 +77,7 @@ protected NamedXContentRegistry xContentRegistry() {

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

public void testDefaultAfter() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ protected ReadOnlyAction doParseInstance(XContentParser parser) {

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ protected RolloverAction doParseInstance(XContentParser parser) {

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ static ShrinkAction randomInstance() {

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}

public void testNonPositiveShardNumber() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ protected UnfollowAction doParseInstance(XContentParser parser) throws IOExcepti

@Override
protected boolean supportsUnknownFields() {
return false;
return true;
}
}

0 comments on commit b49811a

Please sign in to comment.