From e4f4886d40386a55d092123de688eddf2376cde2 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Mon, 19 Jun 2017 13:19:09 +0200 Subject: [PATCH] [Test] Extend parsing checks for DocWriteResponses (#25257) This commit changes the parsing logic of DocWriteResponse, ReplicationResponse and GetResult so that it skips any unknown additional fields (for forward compatibility reasons). This affects the IndexResponse, UpdateResponse,DeleteResponse and GetResponse objects. --- .../client/RestHighLevelClient.java | 4 ++ .../action/DocWriteResponse.java | 10 +--- .../elasticsearch/action/get/GetResponse.java | 26 +++++++++ .../replication/ReplicationResponse.java | 15 ++--- .../elasticsearch/index/get/GetResult.java | 14 +++-- .../action/bulk/BulkItemResponseTests.java | 10 ++-- .../action/delete/DeleteResponseTests.java | 30 +++++++++- .../action/get/GetResponseTests.java | 44 ++++++++++++++- .../action/index/IndexResponseTests.java | 30 +++++++++- .../action/update/UpdateResponseTests.java | 56 +++++++++++++++---- 10 files changed, 198 insertions(+), 41 deletions(-) diff --git a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java index e40c3c223eb..bdf2bf918d0 100644 --- a/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java +++ b/client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java @@ -395,6 +395,10 @@ public class RestHighLevelClient { try { return responseConverter.apply(e.getResponse()); } catch (Exception innerException) { + //the exception is ignored as we now try to parse the response as an error. + //this covers cases like get where 404 can either be a valid document not found response, + //or an error for which parsing is completely different. We try to consider the 404 response as a valid one + //first. If parsing of the response breaks, we fall back to parsing it as an error. throw parseResponseException(e); } } diff --git a/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java b/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java index 64f63025279..6b1cf09bd73 100644 --- a/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java +++ b/core/src/main/java/org/elasticsearch/action/DocWriteResponse.java @@ -43,8 +43,6 @@ import java.net.URLEncoder; import java.util.Locale; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; -import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownField; -import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownToken; /** * A base class for the response of a write operation that involves a single doc @@ -351,17 +349,15 @@ public abstract class DocWriteResponse extends ReplicationResponse implements Wr context.setSeqNo(parser.longValue()); } else if (_PRIMARY_TERM.equals(currentFieldName)) { context.setPrimaryTerm(parser.longValue()); - } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); } } else if (token == XContentParser.Token.START_OBJECT) { if (_SHARDS.equals(currentFieldName)) { context.setShardInfo(ShardInfo.fromXContent(parser)); } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); // skip potential inner objects for forward compatibility } - } else { - throwUnknownToken(token, parser.getTokenLocation()); + } else if (token == XContentParser.Token.START_ARRAY) { + parser.skipChildren(); // skip potential inner arrays for forward compatibility } } diff --git a/core/src/main/java/org/elasticsearch/action/get/GetResponse.java b/core/src/main/java/org/elasticsearch/action/get/GetResponse.java index 296fbe6610e..156005fab24 100644 --- a/core/src/main/java/org/elasticsearch/action/get/GetResponse.java +++ b/core/src/main/java/org/elasticsearch/action/get/GetResponse.java @@ -21,6 +21,7 @@ package org.elasticsearch.action.get; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.StreamInput; @@ -33,6 +34,7 @@ import org.elasticsearch.index.get.GetResult; import java.io.IOException; import java.util.Iterator; +import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -158,8 +160,32 @@ public class GetResponse extends ActionResponse implements Iterable, T return getResult.toXContent(builder, params); } + /** + * This method can be used to parse a {@link GetResponse} object when it has been printed out + * as a xcontent using the {@link #toXContent(XContentBuilder, Params)} method. + *

+ * For forward compatibility reason this method might not fail if it tries to parse a field it + * doesn't know. But before returning the result it will check that enough information were + * parsed to return a valid {@link GetResponse} instance and throws a {@link ParsingException} + * otherwise. This is the case when we get a 404 back, which can be parsed as a normal + * {@link GetResponse} with found set to false, or as an elasticsearch exception. The caller + * of this method needs a way to figure out whether we got back a valid get response, which + * can be done by catching ParsingException. + * + * @param parser {@link XContentParser} to parse the response from + * @return a {@link GetResponse} + * @throws IOException is an I/O exception occurs during the parsing + */ public static GetResponse fromXContent(XContentParser parser) throws IOException { GetResult getResult = GetResult.fromXContent(parser); + + // At this stage we ensure that we parsed enough information to return + // a valid GetResponse instance. If it's not the case, we throw an + // exception so that callers know it and can handle it correctly. + if (getResult.getIndex() == null && getResult.getType() == null && getResult.getId() == null) { + throw new ParsingException(parser.getTokenLocation(), + String.format(Locale.ROOT, "Missing required fields [%s,%s,%s]", GetResult._INDEX, GetResult._TYPE, GetResult._ID)); + } return new GetResponse(getResult); } diff --git a/core/src/main/java/org/elasticsearch/action/support/replication/ReplicationResponse.java b/core/src/main/java/org/elasticsearch/action/support/replication/ReplicationResponse.java index 4b1873e8d06..b8a5f3782bd 100644 --- a/core/src/main/java/org/elasticsearch/action/support/replication/ReplicationResponse.java +++ b/core/src/main/java/org/elasticsearch/action/support/replication/ReplicationResponse.java @@ -40,7 +40,6 @@ import java.util.Arrays; import java.util.List; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; -import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownField; /** * Base class for write action responses. @@ -187,8 +186,8 @@ public class ReplicationResponse extends ActionResponse { total = parser.intValue(); } else if (SUCCESSFUL.equals(currentFieldName)) { successful = parser.intValue(); - } else if (FAILED.equals(currentFieldName) == false) { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + } else { + parser.skipChildren(); } } else if (token == XContentParser.Token.START_ARRAY) { if (FAILURES.equals(currentFieldName)) { @@ -197,8 +196,10 @@ public class ReplicationResponse extends ActionResponse { failuresList.add(Failure.fromXContent(parser)); } } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); // skip potential inner arrays for forward compatibility } + } else if (token == XContentParser.Token.START_OBJECT) { + parser.skipChildren(); // skip potential inner arrays for forward compatibility } } Failure[] failures = EMPTY; @@ -365,15 +366,15 @@ public class ReplicationResponse extends ActionResponse { status = RestStatus.valueOf(parser.text()); } else if (PRIMARY.equals(currentFieldName)) { primary = parser.booleanValue(); - } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); } } else if (token == XContentParser.Token.START_OBJECT) { if (REASON.equals(currentFieldName)) { reason = ElasticsearchException.fromXContent(parser); } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); // skip potential inner objects for forward compatibility } + } else if (token == XContentParser.Token.START_ARRAY) { + parser.skipChildren(); // skip potential inner arrays for forward compatibility } } return new Failure(new ShardId(shardIndex, IndexMetaData.INDEX_UUID_NA_VALUE, shardId), nodeId, reason, status, primary); diff --git a/core/src/main/java/org/elasticsearch/index/get/GetResult.java b/core/src/main/java/org/elasticsearch/index/get/GetResult.java index 3837a39c5c5..6569902ceff 100644 --- a/core/src/main/java/org/elasticsearch/index/get/GetResult.java +++ b/core/src/main/java/org/elasticsearch/index/get/GetResult.java @@ -20,6 +20,7 @@ package org.elasticsearch.index.get; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressorFactory; import org.elasticsearch.common.io.stream.StreamInput; @@ -43,14 +44,13 @@ import java.util.Objects; import static java.util.Collections.emptyMap; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; -import static org.elasticsearch.common.xcontent.XContentParserUtils.throwUnknownField; import static org.elasticsearch.index.get.GetField.readGetField; public class GetResult implements Streamable, Iterable, ToXContentObject { - private static final String _INDEX = "_index"; - private static final String _TYPE = "_type"; - private static final String _ID = "_id"; + public static final String _INDEX = "_index"; + public static final String _TYPE = "_type"; + public static final String _ID = "_id"; private static final String _VERSION = "_version"; private static final String FOUND = "found"; private static final String FIELDS = "fields"; @@ -273,7 +273,7 @@ public class GetResult implements Streamable, Iterable, ToXContentObje String currentFieldName = parser.currentName(); String index = null, type = null, id = null; long version = -1; - boolean found = false; + Boolean found = null; BytesReference source = null; Map fields = new HashMap<>(); while((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { @@ -307,8 +307,10 @@ public class GetResult implements Streamable, Iterable, ToXContentObje fields.put(getField.getName(), getField); } } else { - throwUnknownField(currentFieldName, parser.getTokenLocation()); + parser.skipChildren(); // skip potential inner objects for forward compatibility } + } else if (token == XContentParser.Token.START_ARRAY) { + parser.skipChildren(); // skip potential inner arrays for forward compatibility } } return new GetResult(index, type, id, version, found, source, fields); diff --git a/core/src/test/java/org/elasticsearch/action/bulk/BulkItemResponseTests.java b/core/src/test/java/org/elasticsearch/action/bulk/BulkItemResponseTests.java index 24f3afdd795..4a55f0c8b95 100644 --- a/core/src/test/java/org/elasticsearch/action/bulk/BulkItemResponseTests.java +++ b/core/src/test/java/org/elasticsearch/action/bulk/BulkItemResponseTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.action.DocWriteResponse; import org.elasticsearch.action.bulk.BulkItemResponse.Failure; import org.elasticsearch.action.delete.DeleteResponseTests; import org.elasticsearch.action.index.IndexResponseTests; +import org.elasticsearch.action.update.UpdateResponse; import org.elasticsearch.action.update.UpdateResponseTests; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.collect.Tuple; @@ -137,11 +138,12 @@ public class BulkItemResponseTests extends ESTestCase { assertDeepEquals((ElasticsearchException) expectedFailure.getCause(), (ElasticsearchException) actualFailure.getCause()); } else { + DocWriteResponse expectedDocResponse = expected.getResponse(); + DocWriteResponse actualDocResponse = expected.getResponse(); + + IndexResponseTests.assertDocWriteResponse(expectedDocResponse, actualDocResponse); if (expected.getOpType() == DocWriteRequest.OpType.UPDATE) { - UpdateResponseTests.assertUpdateResponse(expected.getResponse(), actual.getResponse()); - } else { - // assertDocWriteResponse check the result for INDEX/CREATE and DELETE operations - IndexResponseTests.assertDocWriteResponse(expected.getResponse(), actual.getResponse()); + assertEquals(((UpdateResponse) expectedDocResponse).getGetResult(), ((UpdateResponse)actualDocResponse).getGetResult()); } } } diff --git a/core/src/test/java/org/elasticsearch/action/delete/DeleteResponseTests.java b/core/src/test/java/org/elasticsearch/action/delete/DeleteResponseTests.java index 95fbbe8ed14..b90ac66b420 100644 --- a/core/src/test/java/org/elasticsearch/action/delete/DeleteResponseTests.java +++ b/core/src/test/java/org/elasticsearch/action/delete/DeleteResponseTests.java @@ -32,9 +32,11 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.RandomObjects; import java.io.IOException; +import java.util.function.Predicate; import static org.elasticsearch.action.index.IndexResponseTests.assertDocWriteResponse; import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_UUID_NA_VALUE; +import static org.elasticsearch.test.XContentTestUtils.insertRandomFields; public class DeleteResponseTests extends ESTestCase { @@ -56,16 +58,40 @@ public class DeleteResponseTests extends ESTestCase { } public void testToAndFromXContent() throws IOException { + doFromXContentTestWithRandomFields(false); + } + + /** + * This test adds random fields and objects to the xContent rendered out to + * ensure we can parse it back to be forward compatible with additions to + * the xContent + */ + public void testFromXContentWithRandomFields() throws IOException { + doFromXContentTestWithRandomFields(true); + } + + private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws IOException { final Tuple tuple = randomDeleteResponse(); DeleteResponse deleteResponse = tuple.v1(); DeleteResponse expectedDeleteResponse = tuple.v2(); boolean humanReadable = randomBoolean(); final XContentType xContentType = randomFrom(XContentType.values()); - BytesReference deleteResponseBytes = toShuffledXContent(deleteResponse, xContentType, ToXContent.EMPTY_PARAMS, humanReadable); + BytesReference originalBytes = toShuffledXContent(deleteResponse, xContentType, ToXContent.EMPTY_PARAMS, humanReadable); + BytesReference mutated; + if (addRandomFields) { + // The ShardInfo.Failure's exception is rendered out in a "reason" object. We shouldn't add anything random there + // because exception rendering and parsing are very permissive: any extra object or field would be rendered as + // a exception custom metadata and be parsed back as a custom header, making it impossible to compare the results + // in this test. + Predicate excludeFilter = path -> path.contains("reason"); + mutated = insertRandomFields(xContentType, originalBytes, excludeFilter, random()); + } else { + mutated = originalBytes; + } DeleteResponse parsedDeleteResponse; - try (XContentParser parser = createParser(xContentType.xContent(), deleteResponseBytes)) { + try (XContentParser parser = createParser(xContentType.xContent(), mutated)) { parsedDeleteResponse = DeleteResponse.fromXContent(parser); assertNull(parser.nextToken()); } diff --git a/core/src/test/java/org/elasticsearch/action/get/GetResponseTests.java b/core/src/test/java/org/elasticsearch/action/get/GetResponseTests.java index f755d05fc7f..aa753ef817f 100644 --- a/core/src/test/java/org/elasticsearch/action/get/GetResponseTests.java +++ b/core/src/test/java/org/elasticsearch/action/get/GetResponseTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.get; +import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; @@ -30,27 +31,53 @@ import org.elasticsearch.index.get.GetField; import org.elasticsearch.index.get.GetResult; import org.elasticsearch.test.ESTestCase; +import java.io.IOException; import java.util.Collections; +import java.util.function.Predicate; import static org.elasticsearch.common.xcontent.XContentHelper.toXContent; import static org.elasticsearch.index.get.GetResultTests.copyGetResult; import static org.elasticsearch.index.get.GetResultTests.mutateGetResult; import static org.elasticsearch.index.get.GetResultTests.randomGetResult; import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; +import static org.elasticsearch.test.XContentTestUtils.insertRandomFields; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent; public class GetResponseTests extends ESTestCase { public void testToAndFromXContent() throws Exception { + doFromXContentTestWithRandomFields(false); + } + + /** + * This test adds random fields and objects to the xContent rendered out to + * ensure we can parse it back to be forward compatible with additions to + * the xContent + */ + public void testFromXContentWithRandomFields() throws IOException { + doFromXContentTestWithRandomFields(true); + } + + private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws IOException { XContentType xContentType = randomFrom(XContentType.values()); Tuple tuple = randomGetResult(xContentType); GetResponse getResponse = new GetResponse(tuple.v1()); GetResponse expectedGetResponse = new GetResponse(tuple.v2()); boolean humanReadable = randomBoolean(); BytesReference originalBytes = toShuffledXContent(getResponse, xContentType, ToXContent.EMPTY_PARAMS, humanReadable, "_source"); - //test that we can parse what we print out + + BytesReference mutated; + if (addRandomFields) { + // "_source" and "fields" just consists of key/value pairs, we shouldn't add anything random there. It is already + // randomized in the randomGetResult() method anyway. Also, we cannot add anything in the root object since this is + // where GetResult's metadata fields are rendered out while // other fields are rendered out in a "fields" object. + Predicate excludeFilter = (s) -> s.isEmpty() || s.contains("fields") || s.contains("_source"); + mutated = insertRandomFields(xContentType, originalBytes, excludeFilter, random()); + } else { + mutated = originalBytes; + } GetResponse parsedGetResponse; - try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) { + try (XContentParser parser = createParser(xContentType.xContent(), mutated)) { parsedGetResponse = GetResponse.fromXContent(parser); assertNull(parser.nextToken()); } @@ -90,6 +117,19 @@ public class GetResponseTests extends ESTestCase { checkEqualsAndHashCode(new GetResponse(randomGetResult(XContentType.JSON).v1()), GetResponseTests::copyGetResponse, GetResponseTests::mutateGetResponse); } + + public void testFromXContentThrowsParsingException() throws IOException { + GetResponse getResponse = new GetResponse(new GetResult(null, null, null, randomIntBetween(1, 5), randomBoolean(), null, null)); + + XContentType xContentType = randomFrom(XContentType.values()); + BytesReference originalBytes = toShuffledXContent(getResponse, xContentType, ToXContent.EMPTY_PARAMS, randomBoolean()); + + try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) { + ParsingException exception = expectThrows(ParsingException.class, () -> GetResponse.fromXContent(parser)); + assertEquals("Missing required fields [_index,_type,_id]", exception.getMessage()); + } + } + private static GetResponse copyGetResponse(GetResponse getResponse) { return new GetResponse(copyGetResult(getResponse.getResult)); } diff --git a/core/src/test/java/org/elasticsearch/action/index/IndexResponseTests.java b/core/src/test/java/org/elasticsearch/action/index/IndexResponseTests.java index 58947a7173e..feeded03f88 100644 --- a/core/src/test/java/org/elasticsearch/action/index/IndexResponseTests.java +++ b/core/src/test/java/org/elasticsearch/action/index/IndexResponseTests.java @@ -33,9 +33,11 @@ import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.RandomObjects; import java.io.IOException; +import java.util.function.Predicate; import static org.elasticsearch.action.support.replication.ReplicationResponseTests.assertShardInfo; import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_UUID_NA_VALUE; +import static org.elasticsearch.test.XContentTestUtils.insertRandomFields; public class IndexResponseTests extends ESTestCase { @@ -57,16 +59,40 @@ public class IndexResponseTests extends ESTestCase { } public void testToAndFromXContent() throws IOException { + doFromXContentTestWithRandomFields(false); + } + + /** + * This test adds random fields and objects to the xContent rendered out to + * ensure we can parse it back to be forward compatible with additions to + * the xContent + */ + public void testFromXContentWithRandomFields() throws IOException { + doFromXContentTestWithRandomFields(true); + } + + private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws IOException { final Tuple tuple = randomIndexResponse(); IndexResponse indexResponse = tuple.v1(); IndexResponse expectedIndexResponse = tuple.v2(); boolean humanReadable = randomBoolean(); XContentType xContentType = randomFrom(XContentType.values()); - BytesReference indexResponseBytes = toShuffledXContent(indexResponse, xContentType, ToXContent.EMPTY_PARAMS, humanReadable); + BytesReference originalBytes = toShuffledXContent(indexResponse, xContentType, ToXContent.EMPTY_PARAMS, humanReadable); + BytesReference mutated; + if (addRandomFields) { + // The ShardInfo.Failure's exception is rendered out in a "reason" object. We shouldn't add anything random there + // because exception rendering and parsing are very permissive: any extra object or field would be rendered as + // a exception custom metadata and be parsed back as a custom header, making it impossible to compare the results + // in this test. + Predicate excludeFilter = path -> path.contains("reason"); + mutated = insertRandomFields(xContentType, originalBytes, excludeFilter, random()); + } else { + mutated = originalBytes; + } IndexResponse parsedIndexResponse; - try (XContentParser parser = createParser(xContentType.xContent(), indexResponseBytes)) { + try (XContentParser parser = createParser(xContentType.xContent(), mutated)) { parsedIndexResponse = IndexResponse.fromXContent(parser); assertNull(parser.nextToken()); } diff --git a/core/src/test/java/org/elasticsearch/action/update/UpdateResponseTests.java b/core/src/test/java/org/elasticsearch/action/update/UpdateResponseTests.java index 1c80ddca1c5..7423cc5adf1 100644 --- a/core/src/test/java/org/elasticsearch/action/update/UpdateResponseTests.java +++ b/core/src/test/java/org/elasticsearch/action/update/UpdateResponseTests.java @@ -40,11 +40,15 @@ import java.io.IOException; import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.function.Predicate; import static org.elasticsearch.action.DocWriteResponse.Result.DELETED; import static org.elasticsearch.action.DocWriteResponse.Result.NOT_FOUND; import static org.elasticsearch.action.DocWriteResponse.Result.UPDATED; import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_UUID_NA_VALUE; +import static org.elasticsearch.common.xcontent.XContentHelper.toXContent; +import static org.elasticsearch.test.XContentTestUtils.insertRandomFields; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertToXContentEquivalent; public class UpdateResponseTests extends ESTestCase { @@ -81,29 +85,59 @@ public class UpdateResponseTests extends ESTestCase { } public void testToAndFromXContent() throws IOException { - final XContentType xContentType = randomFrom(XContentType.values()); + doFromXContentTestWithRandomFields(false); + } + + /** + * This test adds random fields and objects to the xContent rendered out to + * ensure we can parse it back to be forward compatible with additions to + * the xContent + */ + public void testFromXContentWithRandomFields() throws IOException { + doFromXContentTestWithRandomFields(true); + } + + private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws IOException { + final XContentType xContentType = randomFrom(XContentType.JSON); final Tuple tuple = randomUpdateResponse(xContentType); UpdateResponse updateResponse = tuple.v1(); UpdateResponse expectedUpdateResponse = tuple.v2(); boolean humanReadable = randomBoolean(); - BytesReference updateResponseBytes = toShuffledXContent(updateResponse, xContentType, ToXContent.EMPTY_PARAMS, humanReadable); + BytesReference originalBytes = toShuffledXContent(updateResponse, xContentType, ToXContent.EMPTY_PARAMS, humanReadable); + BytesReference mutated; + if (addRandomFields) { + // - The ShardInfo.Failure's exception is rendered out in a "reason" object. We shouldn't add anything random there + // because exception rendering and parsing are very permissive: any extra object or field would be rendered as + // a exception custom metadata and be parsed back as a custom header, making it impossible to compare the results + // in this test. + // - The GetResult's "_source" and "fields" just consists of key/value pairs, we shouldn't add anything random there. + // It is already randomized in the randomGetResult() method anyway. Also, we cannot add anything within the "get" + // object since this is where GetResult's metadata fields are rendered out and they would be parsed back as + // extra metadata fields. + Predicate excludeFilter = path -> path.contains("reason") || path.contains("get"); + mutated = insertRandomFields(xContentType, originalBytes, excludeFilter, random()); + } else { + mutated = originalBytes; + } UpdateResponse parsedUpdateResponse; - try (XContentParser parser = createParser(xContentType.xContent(), updateResponseBytes)) { + try (XContentParser parser = createParser(xContentType.xContent(), mutated)) { parsedUpdateResponse = UpdateResponse.fromXContent(parser); assertNull(parser.nextToken()); } - // We can't use equals() to compare the original and the parsed delete response - // because the random delete response can contain shard failures with exceptions, - // and those exceptions are not parsed back with the same types. - assertUpdateResponse(expectedUpdateResponse, parsedUpdateResponse); - } + IndexResponseTests.assertDocWriteResponse(expectedUpdateResponse, parsedUpdateResponse); + if (addRandomFields == false) { + assertEquals(expectedUpdateResponse.getGetResult(), parsedUpdateResponse.getGetResult()); + } - public static void assertUpdateResponse(UpdateResponse expected, UpdateResponse actual) { - IndexResponseTests.assertDocWriteResponse(expected, actual); - assertEquals(expected.getGetResult(), actual.getGetResult()); + // Prints out the parsed UpdateResponse object to verify that it is the same as the expected output. + // If random fields have been inserted, it checks that they have been filtered out and that they do + // not alter the final output of the parsed object. + BytesReference parsedBytes = toXContent(parsedUpdateResponse, xContentType, humanReadable); + BytesReference expectedBytes = toXContent(expectedUpdateResponse, xContentType, humanReadable); + assertToXContentEquivalent(expectedBytes, parsedBytes, xContentType); } /**