Skip to content

Commit 6a1bdb0

Browse files
authored
Place the errors field first in bulk response (#98303)
The bulk response is a map of fields that is inherently unordered. However, we have some control over the order in which fields are serialized. This commit places the errors field first to allow clients who want to incrementally parse the response until the errors field is encountered potentially encounter the field earlier. There is also a test added to inform us if a future version of jackson were to change the order of serialization.
1 parent 5206587 commit 6a1bdb0

File tree

2 files changed

+63
-12
lines changed

2 files changed

+63
-12
lines changed

server/src/main/java/org/elasticsearch/action/bulk/BulkResponse.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,11 @@ public RestStatus status() {
142142
@Override
143143
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
144144
builder.startObject();
145+
builder.field(ERRORS, hasFailures());
145146
builder.field(TOOK, tookInMillis);
146147
if (ingestTookInMillis != BulkResponse.NO_INGEST_TOOK) {
147148
builder.field(INGEST_TOOK, ingestTookInMillis);
148149
}
149-
builder.field(ERRORS, hasFailures());
150150
builder.startArray(ITEMS);
151151
for (BulkItemResponse item : this) {
152152
item.toXContent(builder, params);

server/src/test/java/org/elasticsearch/action/bulk/BulkResponseTests.java

+62-11
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import static org.elasticsearch.action.bulk.BulkResponse.NO_INGEST_TOOK;
3030
import static org.elasticsearch.common.xcontent.XContentHelper.toXContent;
3131
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent;
32+
import static org.hamcrest.Matchers.equalTo;
3233

3334
public class BulkResponseTests extends ESTestCase {
3435

@@ -47,17 +48,7 @@ public void testToAndFromXContent() throws IOException {
4748
DocWriteRequest.OpType opType = randomFrom(DocWriteRequest.OpType.values());
4849

4950
if (frequently()) {
50-
Tuple<? extends DocWriteResponse, ? extends DocWriteResponse> randomDocWriteResponses = null;
51-
if (opType == DocWriteRequest.OpType.INDEX || opType == DocWriteRequest.OpType.CREATE) {
52-
randomDocWriteResponses = IndexResponseTests.randomIndexResponse();
53-
} else if (opType == DocWriteRequest.OpType.DELETE) {
54-
randomDocWriteResponses = DeleteResponseTests.randomDeleteResponse();
55-
} else if (opType == DocWriteRequest.OpType.UPDATE) {
56-
randomDocWriteResponses = UpdateResponseTests.randomUpdateResponse(xContentType);
57-
} else {
58-
fail("Test does not support opType [" + opType + "]");
59-
}
60-
51+
Tuple<? extends DocWriteResponse, ? extends DocWriteResponse> randomDocWriteResponses = success(opType, xContentType);
6152
bulkItems[i] = BulkItemResponse.success(i, opType, randomDocWriteResponses.v1());
6253
expectedBulkItems[i] = BulkItemResponse.success(i, opType, randomDocWriteResponses.v2());
6354
} else {
@@ -97,4 +88,64 @@ public void testToAndFromXContent() throws IOException {
9788
BytesReference expectedFinalBytes = toXContent(parsedBulkResponse, xContentType, humanReadable);
9889
assertToXContentEquivalent(expectedFinalBytes, finalBytes, xContentType);
9990
}
91+
92+
public void testToXContentPlacesErrorsFirst() throws IOException {
93+
XContentType xContentType = randomFrom(XContentType.values());
94+
boolean humanReadable = randomBoolean();
95+
96+
boolean errors = false;
97+
long took = randomFrom(randomNonNegativeLong(), -1L);
98+
long ingestTook = randomFrom(randomNonNegativeLong(), NO_INGEST_TOOK);
99+
int nbBulkItems = randomIntBetween(1, 10);
100+
101+
BulkItemResponse[] bulkItems = new BulkItemResponse[nbBulkItems];
102+
103+
for (int i = 0; i < nbBulkItems; i++) {
104+
DocWriteRequest.OpType opType = randomFrom(DocWriteRequest.OpType.values());
105+
if (frequently()) {
106+
Tuple<? extends DocWriteResponse, ? extends DocWriteResponse> randomDocWriteResponses = success(opType, xContentType);
107+
108+
BulkItemResponse success = BulkItemResponse.success(i, opType, randomDocWriteResponses.v1());
109+
bulkItems[i] = success;
110+
} else {
111+
errors = true;
112+
String index = randomAlphaOfLength(5);
113+
String id = randomAlphaOfLength(5);
114+
115+
Tuple<Throwable, ElasticsearchException> failures = randomExceptions();
116+
117+
Exception bulkItemCause = (Exception) failures.v1();
118+
bulkItems[i] = BulkItemResponse.failure(i, opType, new BulkItemResponse.Failure(index, id, bulkItemCause));
119+
}
120+
}
121+
122+
BulkResponse bulkResponse = new BulkResponse(bulkItems, took, ingestTook);
123+
BytesReference originalBytes = toXContent(bulkResponse, xContentType, ToXContent.EMPTY_PARAMS, humanReadable);
124+
125+
try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) {
126+
assertThat(parser.nextToken(), equalTo(XContentParser.Token.START_OBJECT));
127+
XContentParser.Token firstField = parser.nextToken();
128+
assertThat(firstField, equalTo(XContentParser.Token.FIELD_NAME));
129+
assertThat(parser.currentName(), equalTo("errors"));
130+
assertThat(parser.nextToken(), equalTo(XContentParser.Token.VALUE_BOOLEAN));
131+
assertThat(parser.booleanValue(), equalTo(errors));
132+
}
133+
}
134+
135+
private static Tuple<? extends DocWriteResponse, ? extends DocWriteResponse> success(
136+
DocWriteRequest.OpType opType,
137+
XContentType xContentType
138+
) {
139+
Tuple<? extends DocWriteResponse, ? extends DocWriteResponse> randomDocWriteResponses = null;
140+
if (opType == DocWriteRequest.OpType.INDEX || opType == DocWriteRequest.OpType.CREATE) {
141+
randomDocWriteResponses = IndexResponseTests.randomIndexResponse();
142+
} else if (opType == DocWriteRequest.OpType.DELETE) {
143+
randomDocWriteResponses = DeleteResponseTests.randomDeleteResponse();
144+
} else if (opType == DocWriteRequest.OpType.UPDATE) {
145+
randomDocWriteResponses = UpdateResponseTests.randomUpdateResponse(xContentType);
146+
} else {
147+
fail("Test does not support opType [" + opType + "]");
148+
}
149+
return randomDocWriteResponses;
150+
}
100151
}

0 commit comments

Comments
 (0)