honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[ILM] make IndexLifecycleExplainResponse more resilient to null values (#35800) 

added validation for complete information of step details.

also changed the rendering of explain responses so null strings are not rendered

Another thing that I changed is the format of the client-side response. I found it difficult to maintain the two subtly-different objects, so I migrated the usage of long for the fields, to Long (just as it is on the server-side).
This commit is contained in:
Tal Levy 2018-11-27 07:29:59 -08:00 committed by GitHub
parent a615ab454a
commit df341aba4a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 158 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
client/rest-high-level/src/main/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponse.java
View File

@ -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);
}

2
client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/ExplainLifecycleResponseTests.java
View File

@ -36,7 +36,7 @@ public class ExplainLifecycleResponseTests extends AbstractXContentTestCase<Expl
protected ExplainLifecycleResponse createTestInstance() {
Map<String, IndexLifecycleExplainResponse> 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
client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexExplainResponseTests.java → client/rest-high-level/src/test/java/org/elasticsearch/client/indexlifecycle/IndexLifecycleExplainResponseTests.java
View File

@ -35,7 +35,10 @@ import java.util.List;
import java.util.Objects;
import java.util.function.Supplier;
public class IndexExplainResponseTests extends AbstractXContentTestCase<IndexLifecycleExplainResponse> {
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.startsWith;
public class IndexLifecycleExplainResponseTests extends AbstractXContentTestCase<IndexLifecycleExplainResponse> {
static IndexLifecycleExplainResponse randomIndexExplainResponse() {
if (frequently()) {
@ -50,11 +53,38 @@ public class IndexExplainResponseTests extends AbstractXContentTestCase<IndexLif
}
private static IndexLifecycleExplainResponse randomManagedIndexExplainResponse() {
return IndexLifecycleExplainResponse.newManagedIndexResponse(randomAlphaOfLength(10), randomAlphaOfLength(10),
randomNonNegativeLong(), randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10),
randomBoolean() ? null : randomAlphaOfLength(10), randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong(),
boolean stepNull = randomBoolean();
return IndexLifecycleExplainResponse.newManagedIndexResponse(randomAlphaOfLength(10),
randomAlphaOfLength(10),
randomBoolean() ? null : randomNonNegativeLong(),
stepNull ? null : randomAlphaOfLength(10),
stepNull ? null : randomAlphaOfLength(10),
stepNull ? null : randomAlphaOfLength(10),
randomBoolean() ? null : randomAlphaOfLength(10),
stepNull ? null : randomNonNegativeLong(),
stepNull ? null : randomNonNegativeLong(),
stepNull ? null : randomNonNegativeLong(),
randomBoolean() ? null : new BytesArray(new RandomStepInfo(() -> 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

24
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleExplainResponse.java
View File

@ -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);
}

8
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/LifecycleExecutionState.java
View File

@ -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()) &&

4
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/ExplainLifecycleResponseTests.java
View File

@ -26,7 +26,7 @@ public class ExplainLifecycleResponseTests extends AbstractStreamableXContentTes
protected ExplainLifecycleResponse createTestInstance() {
Map<String, IndexLifecycleExplainResponse> 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<String, IndexLifecycleExplainResponse> indexResponses = new HashMap<>(response.getIndexResponses());
IndexLifecycleExplainResponse indexResponse = IndexExplainResponseTests.randomIndexExplainResponse();
IndexLifecycleExplainResponse indexResponse = IndexLifecycleExplainResponseTests.randomIndexExplainResponse();
indexResponses.put(indexResponse.getIndex(), indexResponse);
return new ExplainLifecycleResponse(indexResponses);
}

72
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/IndexExplainResponseTests.java → x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/IndexLifecycleExplainResponseTests.java
View File

@ -26,7 +26,10 @@ import java.util.List;
import java.util.Objects;
import java.util.function.Supplier;
public class IndexExplainResponseTests extends AbstractSerializingTestCase<IndexLifecycleExplainResponse> {
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.startsWith;
public class IndexLifecycleExplainResponseTests extends AbstractSerializingTestCase<IndexLifecycleExplainResponse> {
static IndexLifecycleExplainResponse randomIndexExplainResponse() {
if (frequently()) {
@ -41,13 +44,40 @@ public class IndexExplainResponseTests extends AbstractSerializingTestCase<Index
}
private static IndexLifecycleExplainResponse randomManagedIndexExplainResponse() {
return IndexLifecycleExplainResponse.newManagedIndexResponse(randomAlphaOfLength(10), randomAlphaOfLength(10),
randomNonNegativeLong(), randomAlphaOfLength(10), randomAlphaOfLength(10), randomAlphaOfLength(10),
randomBoolean() ? null : randomAlphaOfLength(10), randomNonNegativeLong(), randomNonNegativeLong(), randomNonNegativeLong(),
boolean stepNull = randomBoolean();
return IndexLifecycleExplainResponse.newManagedIndexResponse(randomAlphaOfLength(10),
randomAlphaOfLength(10),
randomBoolean() ? null : randomNonNegativeLong(),
stepNull ? null : randomAlphaOfLength(10),
stepNull ? null : randomAlphaOfLength(10),
stepNull ? null : randomAlphaOfLength(10),
randomBoolean() ? null : randomAlphaOfLength(10),
stepNull ? null : randomNonNegativeLong(),
stepNull ? null : randomNonNegativeLong(),
stepNull ? null : randomNonNegativeLong(),
randomBoolean() ? null : new BytesArray(new RandomStepInfo(() -> 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<Index
BytesReference stepInfo = instance.getStepInfo();
PhaseExecutionInfo phaseExecutionInfo = instance.getPhaseExecutionInfo();
if (managed) {
switch (between(0, 12)) {
switch (between(0, 7)) {
case 0:
index = index + randomAlphaOfLengthBetween(1, 5);
break;
@ -87,15 +117,14 @@ public class IndexExplainResponseTests extends AbstractSerializingTestCase<Index
policy = policy + randomAlphaOfLengthBetween(1, 5);
break;
case 2:
phase = phase + randomAlphaOfLengthBetween(1, 5);
phase = randomAlphaOfLengthBetween(1, 5);
action = randomAlphaOfLengthBetween(1, 5);
step = randomAlphaOfLengthBetween(1, 5);
phaseTime = randomValueOtherThan(phaseTime, () -> 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<Index
failedStep = null;
}
break;
case 6:
policyTime += randomLongBetween(0, 100000);
case 4:
policyTime = randomValueOtherThan(policyTime, () -> 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<Index
stepInfo = null;
}
break;
case 11:
case 6:
phaseExecutionInfo = randomValueOtherThan(phaseExecutionInfo, () -> PhaseExecutionInfoTests.randomPhaseExecutionInfo(""));
break;
case 12:
case 7:
return IndexLifecycleExplainResponse.newUnmanagedIndexResponse(index);
default:
throw new AssertionError("Illegal randomisation branch");