From 370afa371b1c7712131ddea559ee2f47d319979a Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 20 Sep 2016 17:01:00 -0400 Subject: [PATCH] Make reindex-from-remote ignore unknown fields reindex-from-remote should ignore unknown fields so it is mostly future compatible. This makes it ignore unknown fields by adding an option to `ObjectParser` and `ConstructingObjectParser` that, if enabled, causes them to ignore unknown fields. Closes #20504 --- .../common/xcontent/AbstractObjectParser.java | 3 + .../xcontent/ConstructingObjectParser.java | 34 +++++- .../common/xcontent/ObjectParser.java | 44 ++++++- .../ConstructingObjectParserTests.java | 42 +++++++ .../common/xcontent/ObjectParserTests.java | 110 ++++++++++++++++-- .../reindex/remote/RemoteResponseParsers.java | 68 +++-------- .../RemoteScrollableHitSourceTests.java | 41 ++++++- .../responses/main/with_unknown_fields.json | 22 ++++ 8 files changed, 290 insertions(+), 74 deletions(-) create mode 100644 modules/reindex/src/test/resources/responses/main/with_unknown_fields.json diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java index 6f8a606d9a6..a623a86c9b1 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/AbstractObjectParser.java @@ -60,6 +60,9 @@ public abstract class AbstractObjectParser void declareField(BiConsumer consumer, NoContextParser parser, ParseField parseField, ValueType type) { + if (parser == null) { + throw new IllegalArgumentException("[parser] is required"); + } declareField(consumer, (p, c) -> parser.parse(p), parseField, type); } diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java index e1400463a72..b8a42cd1e13 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java @@ -103,7 +103,7 @@ public final class ConstructingObjectParser builder) { - objectParser = new ObjectParser<>(name); + this(name, false, builder); + } + + /** + * Build the parser. + * + * @param name The name given to the delegate ObjectParser for error identification. Use what you'd use if the object worked with + * ObjectParser. + * @param ignoreUnknownFields Should this parser ignore unknown fields? This should generally be set to true only when parsing responses + * from external systems, never when parsing requests from users. + * @param builder A function that builds the object from an array of Objects. Declare this inline with the parser, casting the elements + * of the array to the arguments so they work with your favorite constructor. The objects in the array will be in the same order + * that you declared the {{@link #constructorArg()}s and none will be null. If any of the constructor arguments aren't defined in + * the XContent then parsing will throw an error. We use an array here rather than a {@code Map} to save on + * allocations. + */ + public ConstructingObjectParser(String name, boolean ignoreUnknownFields, Function builder) { + objectParser = new ObjectParser<>(name, ignoreUnknownFields, null); this.builder = builder; } @@ -153,6 +170,19 @@ public final class ConstructingObjectParser void declareField(BiConsumer consumer, ContextParser parser, ParseField parseField, ValueType type) { + if (consumer == null) { + throw new IllegalArgumentException("[consumer] is required"); + } + if (parser == null) { + throw new IllegalArgumentException("[parser] is required"); + } + if (parseField == null) { + throw new IllegalArgumentException("[parseField] is required"); + } + if (type == null) { + throw new IllegalArgumentException("[type] is required"); + } + if (consumer == REQUIRED_CONSTRUCTOR_ARG_MARKER || consumer == OPTIONAL_CONSTRUCTOR_ARG_MARKER) { /* * Constructor arguments are detected by this "marker" consumer. It keeps the API looking clean even if it is a bit sleezy. We diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java b/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java index 44d9e6e1993..2abd6e66df8 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.common.xcontent; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; @@ -83,6 +84,11 @@ public final class ObjectParser fieldParserMap = new HashMap<>(); private final String name; private final Supplier valueSupplier; + /** + * Should this parser ignore unknown fields? This should generally be set to true only when parsing responses from external systems, + * never when parsing requests from users. + */ + private final boolean ignoreUnknownFields; /** * Creates a new ObjectParser instance with a name. This name is used to reference the parser in exceptions and messages. @@ -96,9 +102,21 @@ public final class ObjectParser valueSupplier) { + public ObjectParser(String name, @Nullable Supplier valueSupplier) { + this(name, false, valueSupplier); + } + + /** + * Creates a new ObjectParser instance which a name. + * @param name the parsers name, used to reference the parser in exceptions and messages. + * @param ignoreUnknownFields Should this parser ignore unknown fields? This should generally be set to true only when parsing + * responses from external systems, never when parsing requests from users. + * @param valueSupplier a supplier that creates a new Value instance used when the parser is used as an inner object parser. + */ + public ObjectParser(String name, boolean ignoreUnknownFields, @Nullable Supplier valueSupplier) { this.name = name; this.valueSupplier = valueSupplier; + this.ignoreUnknownFields = ignoreUnknownFields; } /** @@ -144,9 +162,13 @@ public final class ObjectParser p, ParseField parseField, ValueType type) { + if (parseField == null) { + throw new IllegalArgumentException("[parseField] is required"); + } + if (type == null) { + throw new IllegalArgumentException("[type] is required"); + } FieldParser fieldParser = new FieldParser(p, type.supportedTokens(), parseField, type); for (String fieldValue : parseField.getAllNamesIncludedDeprecated()) { fieldParserMap.putIfAbsent(fieldValue, fieldParser); @@ -178,6 +206,12 @@ public final class ObjectParser void declareField(BiConsumer consumer, ContextParser parser, ParseField parseField, ValueType type) { + if (consumer == null) { + throw new IllegalArgumentException("[consumer] is required"); + } + if (parser == null) { + throw new IllegalArgumentException("[parser] is required"); + } declareField((p, v, c) -> consumer.accept(v, parser.parse(p, c)), parseField, type); } @@ -362,7 +396,7 @@ public final class ObjectParser parser = fieldParserMap.get(fieldName); - if (parser == null) { + if (parser == null && false == ignoreUnknownFields) { throw new IllegalArgumentException("[" + name + "] unknown field [" + fieldName + "], parser not found"); } return parser; diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java b/core/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java index bef4a047ef5..cad712951d9 100644 --- a/core/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java +++ b/core/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java @@ -25,6 +25,8 @@ import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.xcontent.AbstractObjectParser.ContextParser; +import org.elasticsearch.common.xcontent.AbstractObjectParser.NoContextParser; import org.elasticsearch.test.ESTestCase; import org.hamcrest.Matcher; @@ -43,6 +45,27 @@ import static org.hamcrest.Matchers.nullValue; public class ConstructingObjectParserTests extends ESTestCase { private static final ParseFieldMatcherSupplier MATCHER = () -> ParseFieldMatcher.STRICT; + public void testNullDeclares() { + ConstructingObjectParser objectParser = new ConstructingObjectParser<>("foo", a -> null); + Exception e = expectThrows(IllegalArgumentException.class, + () -> objectParser.declareField(null, (r, c) -> null, new ParseField("test"), ObjectParser.ValueType.STRING)); + assertEquals("[consumer] is required", e.getMessage()); + e = expectThrows(IllegalArgumentException.class, () -> objectParser.declareField( + (o, v) -> {}, (ContextParser) null, + new ParseField("test"), ObjectParser.ValueType.STRING)); + assertEquals("[parser] is required", e.getMessage()); + e = expectThrows(IllegalArgumentException.class, () -> objectParser.declareField( + (o, v) -> {}, (NoContextParser) null, + new ParseField("test"), ObjectParser.ValueType.STRING)); + assertEquals("[parser] is required", e.getMessage()); + e = expectThrows(IllegalArgumentException.class, () -> objectParser.declareField( + (o, v) -> {}, (r, c) -> null, null, ObjectParser.ValueType.STRING)); + assertEquals("[parseField] is required", e.getMessage()); + e = expectThrows(IllegalArgumentException.class, () -> objectParser.declareField( + (o, v) -> {}, (r, c) -> null, new ParseField("test"), null)); + assertEquals("[type] is required", e.getMessage()); + } + /** * Builds the object in random order and parses it. */ @@ -261,6 +284,25 @@ public class ConstructingObjectParserTests extends ESTestCase { assertTrue(result.fooSet); } + public void testIgnoreUnknownFields() throws IOException { + XContentParser parser = XContentType.JSON.xContent().createParser( + "{\n" + + " \"test\" : \"foo\",\n" + + " \"junk\" : 2\n" + + "}"); + class TestStruct { + public final String test; + public TestStruct(String test) { + this.test = test; + } + } + ConstructingObjectParser objectParser = new ConstructingObjectParser<>("foo", true, a -> + new TestStruct((String) a[0])); + objectParser.declareString(constructorArg(), new ParseField("test")); + TestStruct s = objectParser.apply(parser, MATCHER); + assertEquals(s.test, "foo"); + } + private static class HasCtorArguments implements ToXContent { @Nullable final String animal; diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java b/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java index a8d26e87ecf..2cc4889be9d 100644 --- a/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java +++ b/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java @@ -18,20 +18,22 @@ */ package org.elasticsearch.common.xcontent; -import static org.hamcrest.Matchers.hasSize; +import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.ParseFieldMatcher; +import org.elasticsearch.common.ParseFieldMatcherSupplier; +import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.xcontent.AbstractObjectParser.ContextParser; +import org.elasticsearch.common.xcontent.AbstractObjectParser.NoContextParser; +import org.elasticsearch.common.xcontent.ObjectParser.NamedObjectParser; +import org.elasticsearch.common.xcontent.ObjectParser.ValueType; +import org.elasticsearch.test.ESTestCase; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import org.elasticsearch.common.ParseField; -import org.elasticsearch.common.ParseFieldMatcher; -import org.elasticsearch.common.ParseFieldMatcherSupplier; -import org.elasticsearch.common.ParsingException; -import org.elasticsearch.common.xcontent.ObjectParser.NamedObjectParser; -import org.elasticsearch.common.xcontent.ObjectParser.ValueType; -import org.elasticsearch.test.ESTestCase; +import static org.hamcrest.Matchers.hasSize; public class ObjectParserTests extends ESTestCase { @@ -72,6 +74,27 @@ public class ObjectParserTests extends ESTestCase { + "FieldParser{preferred_name=test_number, supportedTokens=[VALUE_STRING, VALUE_NUMBER], type=INT}]}"); } + public void testNullDeclares() { + ObjectParser objectParser = new ObjectParser<>("foo"); + Exception e = expectThrows(IllegalArgumentException.class, + () -> objectParser.declareField(null, (r, c) -> null, new ParseField("test"), ObjectParser.ValueType.STRING)); + assertEquals("[consumer] is required", e.getMessage()); + e = expectThrows(IllegalArgumentException.class, () -> objectParser.declareField( + (o, v) -> {}, (ContextParser) null, + new ParseField("test"), ObjectParser.ValueType.STRING)); + assertEquals("[parser] is required", e.getMessage()); + e = expectThrows(IllegalArgumentException.class, () -> objectParser.declareField( + (o, v) -> {}, (NoContextParser) null, + new ParseField("test"), ObjectParser.ValueType.STRING)); + assertEquals("[parser] is required", e.getMessage()); + e = expectThrows(IllegalArgumentException.class, () -> objectParser.declareField( + (o, v) -> {}, (r, c) -> null, null, ObjectParser.ValueType.STRING)); + assertEquals("[parseField] is required", e.getMessage()); + e = expectThrows(IllegalArgumentException.class, () -> objectParser.declareField( + (o, v) -> {}, (r, c) -> null, new ParseField("test"), null)); + assertEquals("[type] is required", e.getMessage()); + } + public void testObjectOrDefault() throws IOException { XContentParser parser = XContentType.JSON.xContent().createParser("{\"object\" : { \"test\": 2}}"); ObjectParser objectParser = new ObjectParser<>("foo", StaticTestStruct::new); @@ -440,6 +463,77 @@ public class ObjectParserTests extends ESTestCase { assertEquals("[named] doesn't support arrays. Use a single object with multiple fields.", e.getCause().getMessage()); } + public void testIgnoreUnknownFields() throws IOException { + XContentBuilder b = XContentBuilder.builder(XContentType.JSON.xContent()); + b.startObject(); + { + b.field("test", "foo"); + b.field("junk", 2); + } + b.endObject(); + b = shuffleXContent(b); + XContentParser parser = XContentType.JSON.xContent().createParser(b.bytes()); + + class TestStruct { + public String test; + } + ObjectParser objectParser = new ObjectParser<>("foo", true, null); + objectParser.declareField((i, c, x) -> c.test = i.text(), new ParseField("test"), ObjectParser.ValueType.STRING); + TestStruct s = objectParser.parse(parser, new TestStruct(), STRICT_PARSING); + assertEquals(s.test, "foo"); + } + + public void testIgnoreUnknownObjects() throws IOException { + XContentBuilder b = XContentBuilder.builder(XContentType.JSON.xContent()); + b.startObject(); + { + b.field("test", "foo"); + b.startObject("junk"); + { + b.field("really", "junk"); + } + b.endObject(); + } + b.endObject(); + b = shuffleXContent(b); + XContentParser parser = XContentType.JSON.xContent().createParser(b.bytes()); + + class TestStruct { + public String test; + } + ObjectParser objectParser = new ObjectParser<>("foo", true, null); + objectParser.declareField((i, c, x) -> c.test = i.text(), new ParseField("test"), ObjectParser.ValueType.STRING); + TestStruct s = objectParser.parse(parser, new TestStruct(), STRICT_PARSING); + assertEquals(s.test, "foo"); + } + + public void testIgnoreUnknownArrays() throws IOException { + XContentBuilder b = XContentBuilder.builder(XContentType.JSON.xContent()); + b.startObject(); + { + b.field("test", "foo"); + b.startArray("junk"); + { + b.startObject(); + { + b.field("really", "junk"); + } + b.endObject(); + } + b.endArray(); + } + b.endObject(); + b = shuffleXContent(b); + XContentParser parser = XContentType.JSON.xContent().createParser(b.bytes()); + class TestStruct { + public String test; + } + ObjectParser objectParser = new ObjectParser<>("foo", true, null); + objectParser.declareField((i, c, x) -> c.test = i.text(), new ParseField("test"), ObjectParser.ValueType.STRING); + TestStruct s = objectParser.parse(parser, new TestStruct(), STRICT_PARSING); + assertEquals(s.test, "foo"); + } + static class NamedObjectHolder { public static final ObjectParser PARSER = new ObjectParser<>("named_object_holder", NamedObjectHolder::new); diff --git a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteResponseParsers.java b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteResponseParsers.java index 1ee96f27c8d..7ecec0aa19b 100644 --- a/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteResponseParsers.java +++ b/modules/reindex/src/main/java/org/elasticsearch/index/reindex/remote/RemoteResponseParsers.java @@ -47,10 +47,7 @@ import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constru import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; /** - * Parsers to convert the response from the remote host into objects useful for {@link RemoteScrollableHitSource}. Lots of data is - * intentionally thrown on the floor because we don't need it but ObjectParser and friends are strict about blowing up when they see - * elements they don't understand. So you'll see a lot of BiConsumers that look like "(b, v) -> {}". That means "I don't care about the - * value here, just throw it away and don't blow up. + * Parsers to convert the response from the remote host into objects useful for {@link RemoteScrollableHitSource}. */ final class RemoteResponseParsers { private RemoteResponseParsers() {} @@ -58,8 +55,8 @@ final class RemoteResponseParsers { /** * Parser for an individual {@code hit} element. */ - public static final ConstructingObjectParser HIT_PARSER = new ConstructingObjectParser<>("hit", - a -> { + public static final ConstructingObjectParser HIT_PARSER = + new ConstructingObjectParser<>("hit", true, a -> { int i = 0; String index = (String) a[i++]; String type = (String) a[i++]; @@ -90,26 +87,23 @@ final class RemoteResponseParsers { HIT_PARSER.declareString(BasicHit::setParent, new ParseField("_parent")); HIT_PARSER.declareLong(BasicHit::setTTL, new ParseField("_ttl")); HIT_PARSER.declareLong(BasicHit::setTimestamp, new ParseField("_timestamp")); - HIT_PARSER.declareField((b, v) -> {}, p -> null, new ParseField("_score"), ValueType.FLOAT_OR_NULL); - HIT_PARSER.declareStringArray((b, v) -> {}, new ParseField("sort")); } /** * Parser for the {@code hits} element. Parsed to an array of {@code [total (Long), hits (List)]}. */ - public static final ConstructingObjectParser HITS_PARSER = new ConstructingObjectParser<>("hits", - a -> a); + public static final ConstructingObjectParser HITS_PARSER = + new ConstructingObjectParser<>("hits", true, a -> a); static { HITS_PARSER.declareLong(constructorArg(), new ParseField("total")); HITS_PARSER.declareObjectArray(constructorArg(), HIT_PARSER, new ParseField("hits")); - HITS_PARSER.declareField((b, v) -> {}, p -> null, new ParseField("max_score"), ValueType.FLOAT_OR_NULL); } /** * Parser for {@code failed} shards in the {@code _shards} elements. */ public static final ConstructingObjectParser SEARCH_FAILURE_PARSER = - new ConstructingObjectParser<>("failure", a -> { + new ConstructingObjectParser<>("failure", true, a -> { int i = 0; String index = (String) a[i++]; Integer shardId = (Integer) a[i++]; @@ -135,7 +129,6 @@ final class RemoteResponseParsers { return p.text(); } }, new ParseField("reason"), ValueType.OBJECT_OR_STRING); - SEARCH_FAILURE_PARSER.declareInt((b, v) -> {}, new ParseField("status")); } /** @@ -143,7 +136,7 @@ final class RemoteResponseParsers { * parses to an empty list. */ public static final ConstructingObjectParser, ParseFieldMatcherSupplier> SHARDS_PARSER = - new ConstructingObjectParser<>("_shards", a -> { + new ConstructingObjectParser<>("_shards", true, a -> { @SuppressWarnings("unchecked") List failures = (List) a[0]; failures = failures == null ? emptyList() : failures; @@ -151,13 +144,10 @@ final class RemoteResponseParsers { }); static { SHARDS_PARSER.declareObjectArray(optionalConstructorArg(), SEARCH_FAILURE_PARSER, new ParseField("failures")); - SHARDS_PARSER.declareInt((b, v) -> {}, new ParseField("total")); - SHARDS_PARSER.declareInt((b, v) -> {}, new ParseField("successful")); - SHARDS_PARSER.declareInt((b, v) -> {}, new ParseField("failed")); } public static final ConstructingObjectParser RESPONSE_PARSER = - new ConstructingObjectParser<>("search_response", a -> { + new ConstructingObjectParser<>("search_response", true, a -> { int i = 0; Throwable catastrophicFailure = (Throwable) a[i++]; if (catastrophicFailure != null) { @@ -189,9 +179,6 @@ final class RemoteResponseParsers { RESPONSE_PARSER.declareString(optionalConstructorArg(), new ParseField("_scroll_id")); RESPONSE_PARSER.declareObject(optionalConstructorArg(), HITS_PARSER, new ParseField("hits")); RESPONSE_PARSER.declareObject(optionalConstructorArg(), SHARDS_PARSER, new ParseField("_shards")); - RESPONSE_PARSER.declareInt((b, v) -> {}, new ParseField("took")); - RESPONSE_PARSER.declareBoolean((b, v) -> {}, new ParseField("terminated_early")); - RESPONSE_PARSER.declareInt((b, v) -> {}, new ParseField("status")); } /** @@ -200,7 +187,7 @@ final class RemoteResponseParsers { public static class ThrowableBuilder { public static final BiFunction PARSER; static { - ObjectParser parser = new ObjectParser<>("reason", ThrowableBuilder::new); + ObjectParser parser = new ObjectParser<>("reason", true, ThrowableBuilder::new); PARSER = parser.andThen(ThrowableBuilder::build); parser.declareString(ThrowableBuilder::setType, new ParseField("type")); parser.declareString(ThrowableBuilder::setReason, new ParseField("reason")); @@ -209,14 +196,6 @@ final class RemoteResponseParsers { // So we can give a nice error for parsing exceptions parser.declareInt(ThrowableBuilder::setLine, new ParseField("line")); parser.declareInt(ThrowableBuilder::setColumn, new ParseField("col")); - - // So we don't blow up on search exceptions - parser.declareString((b, v) -> {}, new ParseField("phase")); - parser.declareBoolean((b, v) -> {}, new ParseField("grouped")); - parser.declareField((p, v, c) -> p.skipChildren(), new ParseField("failed_shards"), ValueType.OBJECT_ARRAY); - - // Just throw away the root_cause - parser.declareField((p, v, c) -> p.skipChildren(), new ParseField("root_cause"), ValueType.OBJECT_ARRAY); } private String type; @@ -269,34 +248,15 @@ final class RemoteResponseParsers { } } - /** - * Parses the {@code version} field of the main action. There are a surprising number of fields in this that we don't need! - */ - public static final ConstructingObjectParser VERSION_PARSER = new ConstructingObjectParser<>( - "version", a -> Version.fromString((String) a[0])); - static { - VERSION_PARSER.declareString(constructorArg(), new ParseField("number")); - VERSION_PARSER.declareBoolean((p, v) -> {}, new ParseField("snapshot_build")); - VERSION_PARSER.declareBoolean((p, v) -> {}, new ParseField("build_snapshot")); - VERSION_PARSER.declareString((p, v) -> {}, new ParseField("build_hash")); - VERSION_PARSER.declareString((p, v) -> {}, new ParseField("build_date")); - VERSION_PARSER.declareString((p, v) -> {}, new ParseField("build_timestamp")); - VERSION_PARSER.declareString((p, v) -> {}, new ParseField("lucene_version")); - } - /** * Parses the main action to return just the {@linkplain Version} that it returns. We throw everything else out. */ public static final ConstructingObjectParser MAIN_ACTION_PARSER = new ConstructingObjectParser<>( - "/", a -> (Version) a[0]); + "/", true, a -> (Version) a[0]); static { - MAIN_ACTION_PARSER.declareBoolean((p, v) -> {}, new ParseField("ok")); - MAIN_ACTION_PARSER.declareInt((p, v) -> {}, new ParseField("status")); - MAIN_ACTION_PARSER.declareString((p, v) -> {}, new ParseField("name")); - MAIN_ACTION_PARSER.declareString((p, v) -> {}, new ParseField("cluster_name")); - MAIN_ACTION_PARSER.declareString((p, v) -> {}, new ParseField("cluster_uuid")); - MAIN_ACTION_PARSER.declareString((p, v) -> {}, new ParseField("name")); - MAIN_ACTION_PARSER.declareString((p, v) -> {}, new ParseField("tagline")); - MAIN_ACTION_PARSER.declareObject(constructorArg(), VERSION_PARSER, new ParseField("version")); + ConstructingObjectParser versionParser = new ConstructingObjectParser<>( + "version", true, a -> Version.fromString((String) a[0])); + versionParser.declareString(constructorArg(), new ParseField("number")); + MAIN_ACTION_PARSER.declareObject(constructorArg(), versionParser, new ParseField("version")); } } diff --git a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteScrollableHitSourceTests.java b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteScrollableHitSourceTests.java index 6407bc0195b..351eb49f906 100644 --- a/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteScrollableHitSourceTests.java +++ b/modules/reindex/src/test/java/org/elasticsearch/index/reindex/remote/RemoteScrollableHitSourceTests.java @@ -113,11 +113,42 @@ public class RemoteScrollableHitSourceTests extends ESTestCase { } public void testLookupRemoteVersion() throws Exception { - sourceWithMockedRemoteCall(false, "main/0_20_5.json").lookupRemoteVersion(v -> assertEquals(Version.fromString("0.20.5"), v)); - sourceWithMockedRemoteCall(false, "main/0_90_13.json").lookupRemoteVersion(v -> assertEquals(Version.fromString("0.90.13"), v)); - sourceWithMockedRemoteCall(false, "main/1_7_5.json").lookupRemoteVersion(v -> assertEquals(Version.fromString("1.7.5"), v)); - sourceWithMockedRemoteCall(false, "main/2_3_3.json").lookupRemoteVersion(v -> assertEquals(Version.V_2_3_3, v)); - sourceWithMockedRemoteCall(false, "main/5_0_0_alpha_3.json").lookupRemoteVersion(v -> assertEquals(Version.V_5_0_0_alpha3, v)); + AtomicBoolean called = new AtomicBoolean(); + sourceWithMockedRemoteCall(false, "main/0_20_5.json").lookupRemoteVersion(v -> { + assertEquals(Version.fromString("0.20.5"), v); + called.set(true); + }); + assertTrue(called.get()); + called.set(false); + sourceWithMockedRemoteCall(false, "main/0_90_13.json").lookupRemoteVersion(v -> { + assertEquals(Version.fromString("0.90.13"), v); + called.set(true); + }); + assertTrue(called.get()); + called.set(false); + sourceWithMockedRemoteCall(false, "main/1_7_5.json").lookupRemoteVersion(v -> { + assertEquals(Version.fromString("1.7.5"), v); + called.set(true); + }); + assertTrue(called.get()); + called.set(false); + sourceWithMockedRemoteCall(false, "main/2_3_3.json").lookupRemoteVersion(v -> { + assertEquals(Version.V_2_3_3, v); + called.set(true); + }); + assertTrue(called.get()); + called.set(false); + sourceWithMockedRemoteCall(false, "main/5_0_0_alpha_3.json").lookupRemoteVersion(v -> { + assertEquals(Version.V_5_0_0_alpha3, v); + called.set(true); + }); + assertTrue(called.get()); + called.set(false); + sourceWithMockedRemoteCall(false, "main/with_unknown_fields.json").lookupRemoteVersion(v -> { + assertEquals(Version.V_5_0_0_alpha3, v); + called.set(true); + }); + assertTrue(called.get()); } public void testParseStartOk() throws Exception { diff --git a/modules/reindex/src/test/resources/responses/main/with_unknown_fields.json b/modules/reindex/src/test/resources/responses/main/with_unknown_fields.json new file mode 100644 index 00000000000..6aec2496314 --- /dev/null +++ b/modules/reindex/src/test/resources/responses/main/with_unknown_fields.json @@ -0,0 +1,22 @@ +{ + "name" : "Crazy Node With Weird Stuff In The Response", + "cluster_name" : "distribution_run", + "cats": "knock things over", + "cake": "is tasty", + "version" : { + "number" : "5.0.0-alpha3", + "build_hash" : "42e092f", + "build_date" : "2016-05-26T16:55:45.405Z", + "build_snapshot" : true, + "lucene_version" : "6.0.0", + "blort_version" : "not even a valid version number, what are you going to do about it?" + }, + "tagline" : "You Know, for Search", + "extra_object" : { + "stuff": "stuff" + }, + "extra_array" : [ + "stuff", + "more stuff" + ] +}