diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponse.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponse.java index 58ba7e63c03..f79639e2443 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponse.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponse.java @@ -31,6 +31,7 @@ import org.elasticsearch.common.xcontent.json.JsonXContent; import java.io.IOException; import java.util.Objects; +import java.util.stream.Stream; public class IndexLifecycleExplainResponse implements ToXContentObject { @@ -58,14 +59,14 @@ public class IndexLifecycleExplainResponse implements ToXContentObject { (String) a[0], (boolean) a[1], (String) a[2], - (long) (a[3] == null ? -1L: a[3]), + (Long) a[3], (String) a[4], (String) a[5], (String) a[6], (String) a[7], - (long) (a[8] == null ? -1L: a[8]), - (long) (a[9] == null ? -1L: a[9]), - (long) (a[10] == null ? -1L: a[10]), + (Long) a[8], + (Long) a[9], + (Long) a[10], (BytesReference) a[11], (PhaseExecutionInfo) a[12])); static { @@ -95,36 +96,47 @@ public class IndexLifecycleExplainResponse implements ToXContentObject { private final String action; private final String step; private final String failedStep; - private final long lifecycleDate; - private final long phaseTime; - private final long actionTime; - private final long stepTime; + private final Long lifecycleDate; + private final Long phaseTime; + private final Long actionTime; + private final Long stepTime; private final boolean managedByILM; private final BytesReference stepInfo; private final PhaseExecutionInfo phaseExecutionInfo; - public static IndexLifecycleExplainResponse newManagedIndexResponse(String index, String policyName, long lifecycleDate, + public static IndexLifecycleExplainResponse newManagedIndexResponse(String index, String policyName, Long lifecycleDate, String phase, String action, String step, String failedStep, - long phaseTime, long actionTime, long stepTime, + Long phaseTime, Long actionTime, Long stepTime, BytesReference stepInfo, PhaseExecutionInfo phaseExecutionInfo) { return new IndexLifecycleExplainResponse(index, true, policyName, lifecycleDate, phase, action, step, failedStep, phaseTime, actionTime, stepTime, stepInfo, phaseExecutionInfo); } public static IndexLifecycleExplainResponse newUnmanagedIndexResponse(String index) { - return new IndexLifecycleExplainResponse(index, false, null, -1L, null, null, null, null, -1L, -1L, -1L, null, null); + return new IndexLifecycleExplainResponse(index, false, null, null, null, null, null, null, null, null, null, null, null); } - private IndexLifecycleExplainResponse(String index, boolean managedByILM, String policyName, long lifecycleDate, - String phase, String action, String step, String failedStep, long phaseTime, long actionTime, - long stepTime, BytesReference stepInfo, PhaseExecutionInfo phaseExecutionInfo) { + private IndexLifecycleExplainResponse(String index, boolean managedByILM, String policyName, Long lifecycleDate, + String phase, String action, String step, String failedStep, Long phaseTime, Long actionTime, + Long stepTime, BytesReference stepInfo, PhaseExecutionInfo phaseExecutionInfo) { if (managedByILM) { if (policyName == null) { throw new IllegalArgumentException("[" + POLICY_NAME_FIELD.getPreferredName() + "] cannot be null for managed index"); } + // check to make sure that step details are either all null or all set. + long numNull = Stream.of(phase, action, step, phaseTime, actionTime, stepTime).filter(Objects::isNull).count(); + if (numNull > 0 && numNull < 6) { + throw new IllegalArgumentException("managed index response must have complete step details [" + + PHASE_FIELD.getPreferredName() + "=" + phase + ", " + + PHASE_TIME_FIELD.getPreferredName() + "=" + phaseTime + ", " + + ACTION_FIELD.getPreferredName() + "=" + action + ", " + + ACTION_TIME_FIELD.getPreferredName() + "=" + actionTime + ", " + + STEP_FIELD.getPreferredName() + "=" + step + ", " + + STEP_TIME_FIELD.getPreferredName() + "=" + stepTime + "]"); + } } else { - if (policyName != null || lifecycleDate >= 0 || phase != null || action != null || step != null || failedStep != null - || phaseTime >= 0 || actionTime >= 0 || stepTime >= 0 || stepInfo != null || phaseExecutionInfo != null) { + if (policyName != null || lifecycleDate != null || phase != null || action != null || step != null || failedStep != null + || phaseTime != null || actionTime != null || stepTime != null || stepInfo != null || phaseExecutionInfo != null) { throw new IllegalArgumentException( "Unmanaged index response must only contain fields: [" + MANAGED_BY_ILM_FIELD + ", " + INDEX_FIELD + "]"); } @@ -203,13 +215,27 @@ public class IndexLifecycleExplainResponse implements ToXContentObject { builder.field(MANAGED_BY_ILM_FIELD.getPreferredName(), managedByILM); if (managedByILM) { builder.field(POLICY_NAME_FIELD.getPreferredName(), policyName); - builder.timeField(LIFECYCLE_DATE_MILLIS_FIELD.getPreferredName(), LIFECYCLE_DATE_FIELD.getPreferredName(), lifecycleDate); - builder.field(PHASE_FIELD.getPreferredName(), phase); - builder.timeField(PHASE_TIME_MILLIS_FIELD.getPreferredName(), PHASE_TIME_FIELD.getPreferredName(), phaseTime); - builder.field(ACTION_FIELD.getPreferredName(), action); - builder.timeField(ACTION_TIME_MILLIS_FIELD.getPreferredName(), ACTION_TIME_FIELD.getPreferredName(), actionTime); - builder.field(STEP_FIELD.getPreferredName(), step); - builder.timeField(STEP_TIME_MILLIS_FIELD.getPreferredName(), STEP_TIME_FIELD.getPreferredName(), stepTime); + if (lifecycleDate != null) { + builder.timeField(LIFECYCLE_DATE_MILLIS_FIELD.getPreferredName(), LIFECYCLE_DATE_FIELD.getPreferredName(), lifecycleDate); + } + if (phase != null) { + builder.field(PHASE_FIELD.getPreferredName(), phase); + } + if (phaseTime != null) { + builder.timeField(PHASE_TIME_MILLIS_FIELD.getPreferredName(), PHASE_TIME_FIELD.getPreferredName(), phaseTime); + } + if (action != null) { + builder.field(ACTION_FIELD.getPreferredName(), action); + } + if (actionTime != null) { + builder.timeField(ACTION_TIME_MILLIS_FIELD.getPreferredName(), ACTION_TIME_FIELD.getPreferredName(), actionTime); + } + if (step != null) { + builder.field(STEP_FIELD.getPreferredName(), step); + } + if (stepTime != null) { + builder.timeField(STEP_TIME_MILLIS_FIELD.getPreferredName(), STEP_TIME_FIELD.getPreferredName(), stepTime); + } if (Strings.hasLength(failedStep)) { builder.field(FAILED_STEP_FIELD.getPreferredName(), failedStep); } diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/ExplainLifecycleResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/ExplainLifecycleResponseTests.java index 26eacb04b02..ab4f548c926 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/ExplainLifecycleResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/ExplainLifecycleResponseTests.java @@ -36,7 +36,7 @@ public class ExplainLifecycleResponseTests extends AbstractXContentTestCase indexResponses = new HashMap<>(); for (int i = 0; i < randomIntBetween(0, 2); i++) { - IndexLifecycleExplainResponse indexResponse = IndexExplainResponseTests.randomIndexExplainResponse(); + IndexLifecycleExplainResponse indexResponse = IndexLifecycleExplainResponseTests.randomIndexExplainResponse(); indexResponses.put(indexResponse.getIndex(), indexResponse); } return new ExplainLifecycleResponse(indexResponses); diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexExplainResponseTests.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponseTests.java similarity index 68% rename from client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexExplainResponseTests.java rename to client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponseTests.java index fb7e73ee621..40d9f7e194a 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexExplainResponseTests.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponseTests.java @@ -35,7 +35,10 @@ import java.util.List; import java.util.Objects; import java.util.function.Supplier; -public class IndexExplainResponseTests extends AbstractXContentTestCase { +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.startsWith; + +public class IndexLifecycleExplainResponseTests extends AbstractXContentTestCase { static IndexLifecycleExplainResponse randomIndexExplainResponse() { if (frequently()) { @@ -50,11 +53,38 @@ public class IndexExplainResponseTests extends AbstractXContentTestCase randomAlphaOfLength(10)).toString()), + randomBoolean() ? null : PhaseExecutionInfoTests.randomPhaseExecutionInfo("")); + } + + public void testInvalidStepDetails() { + final int numNull = randomIntBetween(1, 6); + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> + IndexLifecycleExplainResponse.newManagedIndexResponse(randomAlphaOfLength(10), + randomAlphaOfLength(10), + randomBoolean() ? null : randomNonNegativeLong(), + (numNull == 1) ? null : randomAlphaOfLength(10), + (numNull == 2) ? null : randomAlphaOfLength(10), + (numNull == 3) ? null : randomAlphaOfLength(10), + randomBoolean() ? null : randomAlphaOfLength(10), + (numNull == 4) ? null : randomNonNegativeLong(), + (numNull == 5) ? null : randomNonNegativeLong(), + (numNull == 6) ? null : randomNonNegativeLong(), randomBoolean() ? null : new BytesArray(new RandomStepInfo(() -> randomAlphaOfLength(10)).toString()), - randomBoolean() ? null : PhaseExecutionInfoTests.randomPhaseExecutionInfo("")); + randomBoolean() ? null : PhaseExecutionInfoTests.randomPhaseExecutionInfo(""))); + assertThat(exception.getMessage(), startsWith("managed index response must have complete step details")); + assertThat(exception.getMessage(), containsString("=null")); } @Override diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleExplainResponse.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleExplainResponse.java index 7e97170998a..7880703d8ba 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleExplainResponse.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleExplainResponse.java @@ -21,6 +21,7 @@ import org.elasticsearch.common.xcontent.json.JsonXContent; import java.io.IOException; import java.util.Objects; +import java.util.stream.Stream; public class IndexLifecycleExplainResponse implements ToXContentObject, Writeable { @@ -111,6 +112,17 @@ public class IndexLifecycleExplainResponse implements ToXContentObject, Writeabl if (policyName == null) { throw new IllegalArgumentException("[" + POLICY_NAME_FIELD.getPreferredName() + "] cannot be null for managed index"); } + // check to make sure that step details are either all null or all set. + long numNull = Stream.of(phase, action, step, phaseTime, actionTime, stepTime).filter(Objects::isNull).count(); + if (numNull > 0 && numNull < 6) { + throw new IllegalArgumentException("managed index response must have complete step details [" + + PHASE_FIELD.getPreferredName() + "=" + phase + ", " + + PHASE_TIME_FIELD.getPreferredName() + "=" + phaseTime + ", " + + ACTION_FIELD.getPreferredName() + "=" + action + ", " + + ACTION_TIME_FIELD.getPreferredName() + "=" + actionTime + ", " + + STEP_FIELD.getPreferredName() + "=" + step + ", " + + STEP_TIME_FIELD.getPreferredName() + "=" + stepTime + "]"); + } } else { if (policyName != null || lifecycleDate != null || phase != null || action != null || step != null || failedStep != null || phaseTime != null || actionTime != null || stepTime != null || stepInfo != null || phaseExecutionInfo != null) { @@ -244,15 +256,21 @@ public class IndexLifecycleExplainResponse implements ToXContentObject, Writeabl if (lifecycleDate != null) { builder.timeField(LIFECYCLE_DATE_MILLIS_FIELD.getPreferredName(), LIFECYCLE_DATE_FIELD.getPreferredName(), lifecycleDate); } - builder.field(PHASE_FIELD.getPreferredName(), phase); + if (phase != null) { + builder.field(PHASE_FIELD.getPreferredName(), phase); + } if (phaseTime != null) { builder.timeField(PHASE_TIME_MILLIS_FIELD.getPreferredName(), PHASE_TIME_FIELD.getPreferredName(), phaseTime); } - builder.field(ACTION_FIELD.getPreferredName(), action); + if (action != null) { + builder.field(ACTION_FIELD.getPreferredName(), action); + } if (actionTime != null) { builder.timeField(ACTION_TIME_MILLIS_FIELD.getPreferredName(), ACTION_TIME_FIELD.getPreferredName(), actionTime); } - builder.field(STEP_FIELD.getPreferredName(), step); + if (step != null) { + builder.field(STEP_FIELD.getPreferredName(), step); + } if (stepTime != null) { builder.timeField(STEP_TIME_MILLIS_FIELD.getPreferredName(), STEP_TIME_FIELD.getPreferredName(), stepTime); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecycleExecutionState.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecycleExecutionState.java index b2d42bca733..75818eb10cd 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecycleExecutionState.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecycleExecutionState.java @@ -230,10 +230,10 @@ public class LifecycleExecutionState { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; LifecycleExecutionState that = (LifecycleExecutionState) o; - return getLifecycleDate() == that.getLifecycleDate() && - getPhaseTime() == that.getPhaseTime() && - getActionTime() == that.getActionTime() && - getStepTime() == that.getStepTime() && + return Objects.equals(getLifecycleDate(),that.getLifecycleDate()) && + Objects.equals(getPhaseTime(), that.getPhaseTime()) && + Objects.equals(getActionTime(), that.getActionTime()) && + Objects.equals(getStepTime(), that.getStepTime()) && Objects.equals(getPhase(), that.getPhase()) && Objects.equals(getAction(), that.getAction()) && Objects.equals(getStep(), that.getStep()) && diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ExplainLifecycleResponseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ExplainLifecycleResponseTests.java index 8b64e1128c0..cb4a1924f3b 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ExplainLifecycleResponseTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ExplainLifecycleResponseTests.java @@ -26,7 +26,7 @@ public class ExplainLifecycleResponseTests extends AbstractStreamableXContentTes protected ExplainLifecycleResponse createTestInstance() { Map indexResponses = new HashMap<>(); for (int i = 0; i < randomIntBetween(0, 2); i++) { - IndexLifecycleExplainResponse indexResponse = IndexExplainResponseTests.randomIndexExplainResponse(); + IndexLifecycleExplainResponse indexResponse = IndexLifecycleExplainResponseTests.randomIndexExplainResponse(); indexResponses.put(indexResponse.getIndex(), indexResponse); } return new ExplainLifecycleResponse(indexResponses); @@ -40,7 +40,7 @@ public class ExplainLifecycleResponseTests extends AbstractStreamableXContentTes @Override protected ExplainLifecycleResponse mutateInstance(ExplainLifecycleResponse response) { Map indexResponses = new HashMap<>(response.getIndexResponses()); - IndexLifecycleExplainResponse indexResponse = IndexExplainResponseTests.randomIndexExplainResponse(); + IndexLifecycleExplainResponse indexResponse = IndexLifecycleExplainResponseTests.randomIndexExplainResponse(); indexResponses.put(indexResponse.getIndex(), indexResponse); return new ExplainLifecycleResponse(indexResponses); } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/IndexExplainResponseTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleExplainResponseTests.java similarity index 73% rename from x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/IndexExplainResponseTests.java rename to x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleExplainResponseTests.java index 90915d66e5d..6381dd1ae41 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/IndexExplainResponseTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleExplainResponseTests.java @@ -26,7 +26,10 @@ import java.util.List; import java.util.Objects; import java.util.function.Supplier; -public class IndexExplainResponseTests extends AbstractSerializingTestCase { +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.startsWith; + +public class IndexLifecycleExplainResponseTests extends AbstractSerializingTestCase { static IndexLifecycleExplainResponse randomIndexExplainResponse() { if (frequently()) { @@ -41,13 +44,40 @@ public class IndexExplainResponseTests extends AbstractSerializingTestCase randomAlphaOfLength(10)).toString()), randomBoolean() ? null : PhaseExecutionInfoTests.randomPhaseExecutionInfo("")); } + public void testInvalidStepDetails() { + final int numNull = randomIntBetween(1, 6); + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> + IndexLifecycleExplainResponse.newManagedIndexResponse(randomAlphaOfLength(10), + randomAlphaOfLength(10), + randomBoolean() ? null : randomNonNegativeLong(), + (numNull == 1) ? null : randomAlphaOfLength(10), + (numNull == 2) ? null : randomAlphaOfLength(10), + (numNull == 3) ? null : randomAlphaOfLength(10), + randomBoolean() ? null : randomAlphaOfLength(10), + (numNull == 4) ? null : randomNonNegativeLong(), + (numNull == 5) ? null : randomNonNegativeLong(), + (numNull == 6) ? null : randomNonNegativeLong(), + randomBoolean() ? null : new BytesArray(new RandomStepInfo(() -> randomAlphaOfLength(10)).toString()), + randomBoolean() ? null : PhaseExecutionInfoTests.randomPhaseExecutionInfo(""))); + assertThat(exception.getMessage(), startsWith("managed index response must have complete step details")); + assertThat(exception.getMessage(), containsString("=null")); + } + @Override protected IndexLifecycleExplainResponse createTestInstance() { return randomIndexExplainResponse(); @@ -79,7 +109,7 @@ public class IndexExplainResponseTests extends AbstractSerializingTestCase randomLongBetween(0, 100000)); + actionTime = randomValueOtherThan(actionTime, () -> randomLongBetween(0, 100000)); + stepTime = randomValueOtherThan(stepTime, () -> randomLongBetween(0, 100000)); break; case 3: - action = action + randomAlphaOfLengthBetween(1, 5); - break; - case 4: - step = step + randomAlphaOfLengthBetween(1, 5); - break; - case 5: if (Strings.hasLength(failedStep) == false) { failedStep = randomAlphaOfLength(10); } else if (randomBoolean()) { @@ -104,19 +133,10 @@ public class IndexExplainResponseTests extends AbstractSerializingTestCase randomLongBetween(0, 100000)); break; - case 7: - phaseTime += randomLongBetween(0, 100000); - break; - case 8: - actionTime += randomLongBetween(0, 100000); - break; - case 9: - stepTime += randomLongBetween(0, 100000); - break; - case 10: + case 5: if (Strings.hasLength(stepInfo) == false) { stepInfo = new BytesArray(randomByteArrayOfLength(100)); } else if (randomBoolean()) { @@ -126,10 +146,10 @@ public class IndexExplainResponseTests extends AbstractSerializingTestCase PhaseExecutionInfoTests.randomPhaseExecutionInfo("")); break; - case 12: + case 7: return IndexLifecycleExplainResponse.newUnmanagedIndexResponse(index); default: throw new AssertionError("Illegal randomisation branch");