Skip to content

Commit d08dcb5

Browse files
sohaibiftikharsohaibiftikhar
committed
Fixes after review (2)
-- Started using RandomObjects instead of RandomDocumentPicks -- Removed changes to ElasticsearchAssertions
1 parent 1de794e commit d08dcb5

File tree

7 files changed

+184
-114
lines changed

7 files changed

+184
-114
lines changed

‎server/src/main/java/org/elasticsearch/action/ingest/SimulatePipelineResponse.java‎

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -62,45 +62,38 @@ public class SimulatePipelineResponse extends ActionResponse implements ToXConte
6262
static {
6363
PARSER.declareObjectArray(
6464
constructorArg(),
65-
(p, c) -> {
66-
ensureExpectedToken(Token.START_OBJECT, p.currentToken(), p::getTokenLocation);
67-
boolean isVerbose = false;
68-
boolean isFirst = true;
65+
(parser, context) -> {
66+
ensureExpectedToken(Token.START_OBJECT, parser.currentToken(), parser::getTokenLocation);
6967
SimulateDocumentResult result = null;
70-
while (p.nextToken().equals(Token.FIELD_NAME)) {
71-
switch (p.currentName()) {
68+
while (parser.nextToken() != Token.END_OBJECT) {
69+
ensureExpectedToken(parser.currentToken(), Token.FIELD_NAME, parser::getTokenLocation);
70+
String fieldName = parser.currentName();
71+
parser.nextToken();
72+
switch(fieldName) {
7273
case SimulateDocumentVerboseResult.PROCESSOR_RESULT_FIELD:
73-
assert isFirst || isVerbose;
74-
isVerbose = true;
75-
ensureExpectedToken(Token.START_ARRAY, p.nextToken(), p::getTokenLocation);
74+
ensureExpectedToken(Token.START_ARRAY, parser.currentToken(), parser::getTokenLocation);
7675
List<SimulateProcessorResult> results = new ArrayList<>();
77-
while (p.nextToken().equals(Token.START_OBJECT)) {
78-
results.add(SimulateProcessorResult.fromXContent(p));
76+
while (parser.nextToken().equals(Token.START_OBJECT)) {
77+
results.add(SimulateProcessorResult.fromXContent(parser));
7978
}
80-
ensureExpectedToken(Token.END_ARRAY, p.currentToken(), p::getTokenLocation);
79+
ensureExpectedToken(Token.END_ARRAY, parser.currentToken(), parser::getTokenLocation);
8180
result = new SimulateDocumentVerboseResult(results);
8281
break;
8382
case WriteableIngestDocument.DOC_FIELD:
8483
case "error":
85-
assert !isVerbose;
86-
if (p.currentName().equals("error")) {
87-
p.nextToken();
88-
result = new SimulateDocumentBaseResult(ElasticsearchException.fromXContent(p));
84+
if (fieldName.equals("error")) {
85+
result = new SimulateDocumentBaseResult(ElasticsearchException.fromXContent(parser));
8986
} else {
9087
result = new SimulateDocumentBaseResult(
91-
WriteableIngestDocument.INGEST_DOC_PARSER.apply(p, null).getIngestDocument()
88+
WriteableIngestDocument.INGEST_DOC_PARSER.apply(parser, null).getIngestDocument()
9289
);
9390
}
94-
ensureExpectedToken(Token.END_OBJECT, p.currentToken(), p::getTokenLocation);
91+
ensureExpectedToken(Token.END_OBJECT, parser.currentToken(), parser::getTokenLocation);
9592
break;
9693
default:
97-
p.nextToken();
98-
p.skipChildren();
99-
break;
94+
parser.skipChildren();
10095
}
101-
isFirst = false;
10296
}
103-
ensureExpectedToken(Token.END_OBJECT, p.currentToken(), p::getTokenLocation);
10497
assert result != null;
10598
return result;
10699
},

‎server/src/test/java/org/elasticsearch/action/ingest/SimulateDocumentBaseResultTests.java‎

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,19 @@
2222
import org.elasticsearch.common.io.stream.BytesStreamOutput;
2323
import org.elasticsearch.common.io.stream.StreamInput;
2424
import org.elasticsearch.common.xcontent.XContentParser;
25-
import org.elasticsearch.ingest.RandomDocumentPicks;
2625
import org.elasticsearch.ingest.IngestDocument;
2726
import org.elasticsearch.test.AbstractXContentTestCase;
2827

2928
import java.io.IOException;
29+
import java.util.StringJoiner;
30+
import java.util.function.Predicate;
31+
import java.util.function.Supplier;
3032

3133
import static org.elasticsearch.ingest.IngestDocumentMatcher.assertIngestDocument;
3234
import static org.hamcrest.CoreMatchers.containsString;
3335
import static org.hamcrest.CoreMatchers.equalTo;
3436
import static org.hamcrest.CoreMatchers.instanceOf;
35-
import static org.elasticsearch.action.ingest.WriteableIngestDocumentTests.assertEqualIngestDocs;
37+
import static org.elasticsearch.action.ingest.WriteableIngestDocumentTests.createRandomIngestDoc;
3638

3739
public class SimulateDocumentBaseResultTests extends AbstractXContentTestCase<SimulateDocumentBaseResult> {
3840

@@ -55,20 +57,24 @@ public void testSerialization() throws IOException {
5557
}
5658
}
5759

58-
protected SimulateDocumentBaseResult createTestInstance(boolean isFailure) {
60+
static SimulateDocumentBaseResult createTestInstance(boolean isFailure) {
5961
SimulateDocumentBaseResult simulateDocumentBaseResult;
6062
if (isFailure) {
6163
simulateDocumentBaseResult = new SimulateDocumentBaseResult(new IllegalArgumentException("test"));
6264
} else {
63-
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random());
65+
IngestDocument ingestDocument = createRandomIngestDoc();
6466
simulateDocumentBaseResult = new SimulateDocumentBaseResult(ingestDocument);
6567
}
6668
return simulateDocumentBaseResult;
6769
}
6870

71+
private static SimulateDocumentBaseResult createTestInstanceWithFailures() {
72+
return createTestInstance(randomBoolean());
73+
}
74+
6975
@Override
7076
protected SimulateDocumentBaseResult createTestInstance() {
71-
return createTestInstance(randomBoolean());
77+
return createTestInstance(false);
7278
}
7379

7480
@Override
@@ -78,11 +84,27 @@ protected SimulateDocumentBaseResult doParseInstance(XContentParser parser) {
7884

7985
@Override
8086
protected boolean supportsUnknownFields() {
81-
return false;
87+
return true;
88+
}
89+
90+
@Override
91+
protected Predicate<String> getRandomFieldsExcludeFilter() {
92+
// We cannot have random fields in the _source field and _ingest field
93+
return field ->
94+
field.contains(
95+
new StringJoiner(".")
96+
.add(WriteableIngestDocument.DOC_FIELD)
97+
.add(WriteableIngestDocument.SOURCE_FIELD).toString()
98+
) ||
99+
field.contains(
100+
new StringJoiner(".")
101+
.add(WriteableIngestDocument.DOC_FIELD)
102+
.add(WriteableIngestDocument.INGEST_FIELD).toString()
103+
);
82104
}
83105

84106
public static void assertEqualDocs(SimulateDocumentBaseResult response, SimulateDocumentBaseResult parsedResponse) {
85-
assertEqualIngestDocs(response.getIngestDocument(), parsedResponse.getIngestDocument());
107+
assertEquals(response.getIngestDocument(), parsedResponse.getIngestDocument());
86108
if (response.getFailure() != null) {
87109
assertNotNull(parsedResponse.getFailure());
88110
assertThat(
@@ -101,6 +123,21 @@ public void assertEqualInstances(SimulateDocumentBaseResult response, SimulateDo
101123

102124
@Override
103125
protected boolean assertToXContentEquivalence() {
104-
return false;
126+
return true;
127+
}
128+
129+
/**
130+
* Test parsing {@link SimulateDocumentBaseResult} with inner failures as they don't support asserting on xcontent
131+
* equivalence, given that exceptions are not parsed back as the same original class. We run the usual
132+
* {@link AbstractXContentTestCase#testFromXContent()} without failures, and this other test with failures where
133+
* we disable asserting on xcontent equivalence at the end.
134+
*/
135+
public void testFromXContentWithFailures() throws IOException {
136+
Supplier<SimulateDocumentBaseResult> instanceSupplier = SimulateDocumentBaseResultTests::createTestInstanceWithFailures;
137+
//exceptions are not of the same type whenever parsed back
138+
boolean assertToXContentEquivalence = false;
139+
AbstractXContentTestCase.testFromXContent(NUMBER_OF_TEST_RUNS, instanceSupplier, supportsUnknownFields(),
140+
getShuffleFieldsExceptions(), getRandomFieldsExcludeFilter(), this::createParser, this::doParseInstance,
141+
this::assertEqualInstances, assertToXContentEquivalence, getToXContentParams());
105142
}
106143
}

‎server/src/test/java/org/elasticsearch/action/ingest/SimulateDocumentVerboseResultTests.java‎

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,34 +21,49 @@
2121
import org.elasticsearch.common.xcontent.XContentParser;
2222
import org.elasticsearch.test.AbstractXContentTestCase;
2323

24+
import java.io.IOException;
2425
import java.util.ArrayList;
2526
import java.util.List;
27+
import java.util.StringJoiner;
28+
import java.util.function.Predicate;
29+
import java.util.function.Supplier;
2630

2731
public class SimulateDocumentVerboseResultTests extends AbstractXContentTestCase<SimulateDocumentVerboseResult> {
2832

29-
@Override
30-
protected SimulateDocumentVerboseResult createTestInstance() {
33+
static SimulateDocumentVerboseResult createTestInstance(boolean withFailures) {
3134
int numDocs = randomIntBetween(0, 10);
3235
List<SimulateProcessorResult> results = new ArrayList<>();
3336
for (int i = 0; i<numDocs; i++) {
37+
boolean isSuccessful = !(withFailures && randomBoolean());
38+
boolean isIgnoredError = withFailures && randomBoolean();
3439
results.add(
35-
SimulateProcessorResultTests.createTestInstance(randomBoolean(), randomBoolean())
40+
SimulateProcessorResultTests
41+
.createTestInstance(isSuccessful, isIgnoredError)
3642
);
3743
}
3844
return new SimulateDocumentVerboseResult(results);
3945
}
4046

47+
private static SimulateDocumentVerboseResult createTestInstanceWithFailures() {
48+
return createTestInstance(true);
49+
}
50+
51+
@Override
52+
protected SimulateDocumentVerboseResult createTestInstance() {
53+
return createTestInstance(false);
54+
}
55+
4156
@Override
4257
protected SimulateDocumentVerboseResult doParseInstance(XContentParser parser) {
4358
return SimulateDocumentVerboseResult.fromXContent(parser);
4459
}
4560

4661
@Override
4762
protected boolean supportsUnknownFields() {
48-
return false;
63+
return true;
4964
}
5065

51-
protected static void assertEqualDocs(SimulateDocumentVerboseResult response,
66+
static void assertEqualDocs(SimulateDocumentVerboseResult response,
5267
SimulateDocumentVerboseResult parsedResponse) {
5368
assertEquals(response.getProcessorResults().size(), parsedResponse.getProcessorResults().size());
5469
for (int i=0; i < response.getProcessorResults().size(); i++) {
@@ -67,6 +82,37 @@ protected void assertEqualInstances(SimulateDocumentVerboseResult response,
6782

6883
@Override
6984
protected boolean assertToXContentEquivalence() {
70-
return false;
85+
return true;
86+
}
87+
88+
@Override
89+
protected Predicate<String> getRandomFieldsExcludeFilter() {
90+
// We cannot have random fields in the _source field and _ingest field
91+
return field ->
92+
field.contains(
93+
new StringJoiner(".")
94+
.add(WriteableIngestDocument.DOC_FIELD)
95+
.add(WriteableIngestDocument.SOURCE_FIELD).toString()
96+
) ||
97+
field.contains(
98+
new StringJoiner(".")
99+
.add(WriteableIngestDocument.DOC_FIELD)
100+
.add(WriteableIngestDocument.INGEST_FIELD).toString()
101+
);
102+
}
103+
104+
/**
105+
* Test parsing {@link SimulateDocumentVerboseResult} with inner failures as they don't support asserting on xcontent
106+
* equivalence, given that exceptions are not parsed back as the same original class. We run the usual
107+
* {@link AbstractXContentTestCase#testFromXContent()} without failures, and this other test with failures where we
108+
* disable asserting on xcontent equivalence at the end.
109+
*/
110+
public void testFromXContentWithFailures() throws IOException {
111+
Supplier<SimulateDocumentVerboseResult> instanceSupplier = SimulateDocumentVerboseResultTests::createTestInstanceWithFailures;
112+
//exceptions are not of the same type whenever parsed back
113+
boolean assertToXContentEquivalence = false;
114+
AbstractXContentTestCase.testFromXContent(NUMBER_OF_TEST_RUNS, instanceSupplier, supportsUnknownFields(),
115+
getShuffleFieldsExceptions(), getRandomFieldsExcludeFilter(), this::createParser, this::doParseInstance,
116+
this::assertEqualInstances, assertToXContentEquivalence, getToXContentParams());
71117
}
72118
}

‎server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineResponseTests.java‎

Lines changed: 24 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,13 @@
2222
import org.elasticsearch.common.io.stream.BytesStreamOutput;
2323
import org.elasticsearch.common.io.stream.StreamInput;
2424
import org.elasticsearch.common.xcontent.XContentParser;
25-
import org.elasticsearch.ingest.IngestDocument;
26-
import org.elasticsearch.ingest.RandomDocumentPicks;
2725
import org.elasticsearch.test.AbstractXContentTestCase;
2826

2927
import java.io.IOException;
3028
import java.util.ArrayList;
3129
import java.util.Iterator;
3230
import java.util.List;
31+
import java.util.StringJoiner;
3332
import java.util.function.Predicate;
3433
import java.util.function.Supplier;
3534

@@ -94,35 +93,18 @@ public void testSerialization() throws IOException {
9493
}
9594
}
9695

97-
public static SimulatePipelineResponse createInstance(String pipelineId, boolean isVerbose, boolean withFailure) {
96+
static SimulatePipelineResponse createInstance(String pipelineId, boolean isVerbose, boolean withFailure) {
9897
int numResults = randomIntBetween(1, 10);
9998
List<SimulateDocumentResult> results = new ArrayList<>(numResults);
10099
for (int i = 0; i < numResults; i++) {
101-
boolean isFailure = withFailure && randomBoolean();
102-
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random());
103100
if (isVerbose) {
104-
int numProcessors = randomIntBetween(1, 10);
105-
List<SimulateProcessorResult> processorResults = new ArrayList<>(numProcessors);
106-
for (int j = 0; j < numProcessors; j++) {
107-
String processorTag = randomAlphaOfLengthBetween(1, 10);
108-
SimulateProcessorResult processorResult;
109-
if (isFailure) {
110-
processorResult = new SimulateProcessorResult(processorTag, new IllegalArgumentException("test"));
111-
} else {
112-
processorResult = new SimulateProcessorResult(processorTag, ingestDocument);
113-
}
114-
processorResults.add(processorResult);
115-
}
116-
results.add(new SimulateDocumentVerboseResult(processorResults));
101+
results.add(
102+
SimulateDocumentVerboseResultTests.createTestInstance(withFailure)
103+
);
117104
} else {
118-
results.add(new SimulateDocumentBaseResult(ingestDocument));
119-
SimulateDocumentBaseResult simulateDocumentBaseResult;
120-
if (isFailure) {
121-
simulateDocumentBaseResult = new SimulateDocumentBaseResult(new IllegalArgumentException("test"));
122-
} else {
123-
simulateDocumentBaseResult = new SimulateDocumentBaseResult(ingestDocument);
124-
}
125-
results.add(simulateDocumentBaseResult);
105+
results.add(
106+
SimulateDocumentBaseResultTests.createTestInstance(withFailure && randomBoolean())
107+
);
126108
}
127109
}
128110
return new SimulatePipelineResponse(pipelineId, isVerbose, results);
@@ -159,14 +141,14 @@ protected void assertEqualInstances(SimulatePipelineResponse response,
159141
assertEquals(response.getResults().size(), parsedResponse.getResults().size());
160142
for (int i=0; i < response.getResults().size(); i++) {
161143
if (response.isVerbose()) {
162-
assert response.getResults().get(i) instanceof SimulateDocumentVerboseResult;
163-
assert parsedResponse.getResults().get(i) instanceof SimulateDocumentVerboseResult;
144+
assertThat(response.getResults().get(i), instanceOf(SimulateDocumentVerboseResult.class));
145+
assertThat(parsedResponse.getResults().get(i), instanceOf(SimulateDocumentVerboseResult.class));
164146
SimulateDocumentVerboseResult responseResult = (SimulateDocumentVerboseResult)response.getResults().get(i);
165147
SimulateDocumentVerboseResult parsedResult = (SimulateDocumentVerboseResult)parsedResponse.getResults().get(i);
166148
SimulateDocumentVerboseResultTests.assertEqualDocs(responseResult, parsedResult);
167149
} else {
168-
assert response.getResults().get(i) instanceof SimulateDocumentBaseResult;
169-
assert parsedResponse.getResults().get(i) instanceof SimulateDocumentBaseResult;
150+
assertThat(response.getResults().get(i), instanceOf(SimulateDocumentBaseResult.class));
151+
assertThat(parsedResponse.getResults().get(i), instanceOf(SimulateDocumentBaseResult.class));
170152
SimulateDocumentBaseResult responseResult = (SimulateDocumentBaseResult)response.getResults().get(i);
171153
SimulateDocumentBaseResult parsedResult = (SimulateDocumentBaseResult)parsedResponse.getResults().get(i);
172154
SimulateDocumentBaseResultTests.assertEqualDocs(responseResult, parsedResult);
@@ -176,8 +158,18 @@ protected void assertEqualInstances(SimulatePipelineResponse response,
176158

177159
@Override
178160
protected Predicate<String> getRandomFieldsExcludeFilter() {
179-
// We cannot have random fields in the _source field
180-
return field -> field.contains("doc._source") || field.contains("doc._ingest");
161+
// We cannot have random fields in the _source field and _ingest field
162+
return field ->
163+
field.contains(
164+
new StringJoiner(".")
165+
.add(WriteableIngestDocument.DOC_FIELD)
166+
.add(WriteableIngestDocument.SOURCE_FIELD).toString()
167+
) ||
168+
field.contains(
169+
new StringJoiner(".")
170+
.add(WriteableIngestDocument.DOC_FIELD)
171+
.add(WriteableIngestDocument.INGEST_FIELD).toString()
172+
);
181173
}
182174

183175
/**

0 commit comments

Comments
 (0)