From 8892675321968905438611d16ca5cca8db70be03 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Thu, 18 Oct 2018 14:11:04 +0200 Subject: [PATCH 1/4] Remove hand-coded XContent duplicate checks With this commit we cleanup hand-coded duplicate checks in XContent parsing. They were necessary previously but since we reconfigured the underlying parser in #22073 and #22225, these checks are obsolete and were also ineffective unless an undocumented system property has been set. As we also remove this escape hatch, we can remove the additional checks as well. Closes #22253 --- .../xcontent/ConstructingObjectParser.java | 11 ++- .../common/xcontent/XContent.java | 24 ------- .../common/xcontent/cbor/CborXContent.java | 2 +- .../common/xcontent/json/JsonXContent.java | 2 +- .../common/xcontent/smile/SmileXContent.java | 2 +- .../common/xcontent/yaml/YamlXContent.java | 2 +- .../ConstructingObjectParserTests.java | 9 ++- .../common/settings/Settings.java | 22 ++---- .../query/ConstantScoreQueryBuilder.java | 4 -- .../common/settings/SettingsTests.java | 21 ++---- .../common/xcontent/BaseXContentTestCase.java | 3 - .../index/query/BoolQueryBuilderTests.java | 8 +-- .../query/ConstantScoreQueryBuilderTests.java | 8 +-- .../FunctionScoreQueryBuilderTests.java | 6 +- .../AggregatorFactoriesTests.java | 14 ++-- .../phrase/DirectCandidateGeneratorTests.java | 11 +-- .../ClientYamlSuiteRestApiParser.java | 8 --- .../test/rest/yaml/section/DoSection.java | 5 +- ...entYamlSuiteRestApiParserFailingTests.java | 67 ------------------- .../rest/yaml/section/DoSectionTests.java | 19 +----- .../condition/ArrayCompareCondition.java | 17 ----- .../attachment/EmailAttachmentsParser.java | 15 ++--- .../condition/ArrayCompareConditionTests.java | 20 ++---- .../EmailAttachmentParsersTests.java | 8 +-- 24 files changed, 56 insertions(+), 252 deletions(-) diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java index e880781fad781..70d867fe5079f 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java @@ -216,7 +216,7 @@ public void declareField(BiConsumer consumer, ContextParser target.constructorArg(position, parseField, v), parser, parseField, type); + objectParser.declareField((target, v) -> target.constructorArg(position, v), parser, parseField, type); } else { numberOfFields += 1; objectParser.declareField(queueingConsumer(consumer, parseField), parser, parseField, type); @@ -249,7 +249,7 @@ public void declareNamedObjects(BiConsumer> consumer, NamedOb int position = constructorArgInfos.size(); boolean required = consumer == REQUIRED_CONSTRUCTOR_ARG_MARKER; constructorArgInfos.add(new ConstructorArgInfo(parseField, required)); - objectParser.declareNamedObjects((target, v) -> target.constructorArg(position, parseField, v), namedObjectParser, parseField); + objectParser.declareNamedObjects((target, v) -> target.constructorArg(position, v), namedObjectParser, parseField); } else { numberOfFields += 1; objectParser.declareNamedObjects(queueingConsumer(consumer, parseField), namedObjectParser, parseField); @@ -285,7 +285,7 @@ public void declareNamedObjects(BiConsumer> consumer, NamedOb int position = constructorArgInfos.size(); boolean required = consumer == REQUIRED_CONSTRUCTOR_ARG_MARKER; constructorArgInfos.add(new ConstructorArgInfo(parseField, required)); - objectParser.declareNamedObjects((target, v) -> target.constructorArg(position, parseField, v), namedObjectParser, + objectParser.declareNamedObjects((target, v) -> target.constructorArg(position, v), namedObjectParser, wrapOrderedModeCallBack(orderedModeCallback), parseField); } else { numberOfFields += 1; @@ -395,10 +395,7 @@ private class Target { /** * Set a constructor argument and build the target object if all constructor arguments have arrived. */ - private void constructorArg(int position, ParseField parseField, Object value) { - if (constructorArgs[position] != null) { - throw new IllegalArgumentException("Can't repeat param [" + parseField + "]"); - } + private void constructorArg(int position, Object value) { constructorArgs[position] = value; constructorArgsCollected++; if (constructorArgsCollected == constructorArgInfos.size()) { diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContent.java index 1eaaac104f29d..f33182d3af248 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContent.java @@ -19,8 +19,6 @@ package org.elasticsearch.common.xcontent; -import org.elasticsearch.common.Booleans; - import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -32,28 +30,6 @@ * A generic abstraction on top of handling content, inspired by JSON and pull parsing. */ public interface XContent { - - /* - * NOTE: This comment is only meant for maintainers of the Elasticsearch code base and is intentionally not a Javadoc comment as it - * describes an undocumented system property. - * - * - * Determines whether the XContent parser will always check for duplicate keys. This behavior is enabled by default but - * can be disabled by setting the otherwise undocumented system property "es.xcontent.strict_duplicate_detection to "false". - * - * Before we've enabled this mode, we had custom duplicate checks in various parts of the code base. As the user can still disable this - * mode and fall back to the legacy duplicate checks, we still need to keep the custom duplicate checks around and we also need to keep - * the tests around. - * - * If this fallback via system property is removed one day in the future you can remove all tests that call this method and also remove - * the corresponding custom duplicate check code. - * - */ - static boolean isStrictDuplicateDetectionEnabled() { - // Don't allow duplicate keys in JSON content by default but let the user opt out - return Booleans.parseBoolean(System.getProperty("es.xcontent.strict_duplicate_detection", "true"), true); - } - /** * The type this content handles and produces. */ diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java index 34653e5634ab8..774427d401ede 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java @@ -56,7 +56,7 @@ public static XContentBuilder contentBuilder() throws IOException { cborFactory.configure(CBORFactory.Feature.FAIL_ON_SYMBOL_HASH_OVERFLOW, false); // this trips on many mappings now... // Do not automatically close unclosed objects/arrays in com.fasterxml.jackson.dataformat.cbor.CBORGenerator#close() method cborFactory.configure(JsonGenerator.Feature.AUTO_CLOSE_JSON_CONTENT, false); - cborFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, XContent.isStrictDuplicateDetectionEnabled()); + cborFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, true); cborXContent = new CborXContent(); } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java index b2aac37abe57d..99df76c437d61 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java @@ -57,7 +57,7 @@ public static XContentBuilder contentBuilder() throws IOException { jsonFactory.configure(JsonFactory.Feature.FAIL_ON_SYMBOL_HASH_OVERFLOW, false); // this trips on many mappings now... // Do not automatically close unclosed objects/arrays in com.fasterxml.jackson.core.json.UTF8JsonGenerator#close() method jsonFactory.configure(JsonGenerator.Feature.AUTO_CLOSE_JSON_CONTENT, false); - jsonFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, XContent.isStrictDuplicateDetectionEnabled()); + jsonFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, true); jsonXContent = new JsonXContent(); } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java index 5040f81cc130a..c2917ad7a0fb4 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java @@ -58,7 +58,7 @@ public static XContentBuilder contentBuilder() throws IOException { smileFactory.configure(SmileFactory.Feature.FAIL_ON_SYMBOL_HASH_OVERFLOW, false); // this trips on many mappings now... // Do not automatically close unclosed objects/arrays in com.fasterxml.jackson.dataformat.smile.SmileGenerator#close() method smileFactory.configure(JsonGenerator.Feature.AUTO_CLOSE_JSON_CONTENT, false); - smileFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, XContent.isStrictDuplicateDetectionEnabled()); + smileFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, true); smileXContent = new SmileXContent(); } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java index 5c335276bc024..6461544765997 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java @@ -51,7 +51,7 @@ public static XContentBuilder contentBuilder() throws IOException { static { yamlFactory = new YAMLFactory(); - yamlFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, XContent.isStrictDuplicateDetectionEnabled()); + yamlFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, true); yamlXContent = new YamlXContent(); } diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java index 7488cfd7e9c55..4c2f743c4045e 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.common.xcontent; +import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; @@ -165,18 +166,16 @@ public void testMissingFirstConstructorArgButNotRequired() throws IOException { } public void testRepeatedConstructorParam() throws IOException { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); XContentParser parser = createParser(JsonXContent.jsonXContent, "{\n" + " \"vegetable\": 1,\n" + " \"vegetable\": 2\n" + "}"); Throwable e = expectThrows(XContentParseException.class, () -> randomFrom(HasCtorArguments.ALL_PARSERS).apply(parser, null)); - assertEquals("[has_required_arguments] failed to parse field [vegetable]", e.getMessage()); + assertEquals("[2:16] [has_required_arguments] failed to parse object", e.getMessage()); e = e.getCause(); - assertThat(e, instanceOf(IllegalArgumentException.class)); - assertEquals("Can't repeat param [vegetable]", e.getMessage()); + assertThat(e, instanceOf(JsonParseException.class)); + assertThat(e.getMessage(), containsString("Duplicate field 'vegetable'")); } public void testBadParam() throws IOException { diff --git a/server/src/main/java/org/elasticsearch/common/settings/Settings.java b/server/src/main/java/org/elasticsearch/common/settings/Settings.java index 1aeed2aee5115..d350c26ce5acc 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/Settings.java +++ b/server/src/main/java/org/elasticsearch/common/settings/Settings.java @@ -712,21 +712,21 @@ private static void fromXContent(XContentParser parser, StringBuilder keyBuilder } } String key = keyBuilder.toString(); - validateValue(key, list, builder, parser, allowNullValues); + validateValue(key, list, parser, allowNullValues); builder.putList(key, list); } else if (parser.currentToken() == XContentParser.Token.VALUE_NULL) { String key = keyBuilder.toString(); - validateValue(key, null, builder, parser, allowNullValues); + validateValue(key, null, parser, allowNullValues); builder.putNull(key); } else if (parser.currentToken() == XContentParser.Token.VALUE_STRING || parser.currentToken() == XContentParser.Token.VALUE_NUMBER) { String key = keyBuilder.toString(); String value = parser.text(); - validateValue(key, value, builder, parser, allowNullValues); + validateValue(key, value, parser, allowNullValues); builder.put(key, value); } else if (parser.currentToken() == XContentParser.Token.VALUE_BOOLEAN) { String key = keyBuilder.toString(); - validateValue(key, parser.text(), builder, parser, allowNullValues); + validateValue(key, parser.text(), parser, allowNullValues); builder.put(key, parser.booleanValue()); } else { XContentParserUtils.throwUnknownToken(parser.currentToken(), parser.getTokenLocation()); @@ -734,19 +734,7 @@ private static void fromXContent(XContentParser parser, StringBuilder keyBuilder } } - private static void validateValue(String key, Object currentValue, Settings.Builder builder, XContentParser parser, - boolean allowNullValues) { - if (builder.map.containsKey(key)) { - throw new ElasticsearchParseException( - "duplicate settings key [{}] found at line number [{}], column number [{}], previous value [{}], current value [{}]", - key, - parser.getTokenLocation().lineNumber, - parser.getTokenLocation().columnNumber, - builder.map.get(key), - currentValue - ); - } - + private static void validateValue(String key, Object currentValue, XContentParser parser, boolean allowNullValues) { if (currentValue == null && allowNullValues == false) { throw new ElasticsearchParseException( "null-valued setting found for key [{}] found at line number [{}], column number [{}]", diff --git a/server/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java index 18330f9cb4d9b..e8d98a6b00b0e 100644 --- a/server/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java @@ -98,10 +98,6 @@ public static ConstantScoreQueryBuilder fromXContent(XContentParser parser) thro currentFieldName = parser.currentName(); } else if (token == XContentParser.Token.START_OBJECT) { if (INNER_QUERY_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { - if (queryFound) { - throw new ParsingException(parser.getTokenLocation(), "[" + ConstantScoreQueryBuilder.NAME + "]" - + " accepts only one 'filter' element."); - } query = parseInnerQueryBuilder(parser); queryFound = true; } else { diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java index 70e958b974bf1..739920038ecca 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.common.settings; +import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; import org.elasticsearch.common.Strings; @@ -26,13 +27,11 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.VersionUtils; -import org.hamcrest.CoreMatchers; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -554,29 +553,17 @@ public void testSimpleJsonSettings() throws Exception { } public void testDuplicateKeysThrowsException() { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); final String json = "{\"foo\":\"bar\",\"foo\":\"baz\"}"; final SettingsException e = expectThrows(SettingsException.class, () -> Settings.builder().loadFromSource(json, XContentType.JSON).build()); - assertThat( - e.toString(), - CoreMatchers.containsString("duplicate settings key [foo] " + - "found at line number [1], " + - "column number [20], " + - "previous value [bar], " + - "current value [baz]")); + assertThat(e.toString(), containsString("Duplicate field 'foo'")); String yaml = "foo: bar\nfoo: baz"; SettingsException e1 = expectThrows(SettingsException.class, () -> { Settings.builder().loadFromSource(yaml, XContentType.YAML); }); - assertEquals(e1.getCause().getClass(), ElasticsearchParseException.class); - String msg = e1.getCause().getMessage(); - assertTrue( - msg, - msg.contains("duplicate settings key [foo] found at line number [2], column number [6], " + - "previous value [bar], current value [baz]")); + assertEquals(e1.getCause().getClass(), JsonParseException.class); + assertThat(e1.getCause().getMessage(), containsString("Duplicate field 'foo'")); } public void testToXContent() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java b/server/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java index 38e75b921fa72..cd1a209878346 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java @@ -1138,9 +1138,6 @@ public void testSelfReferencingIterableTwoLevels() throws IOException { } public void testChecksForDuplicates() throws Exception { - assumeTrue("Test only makes sense if XContent parser has strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); - XContentBuilder builder = builder() .startObject() .field("key", 1) diff --git a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java index 362adf4a4c996..0414340ad25bb 100644 --- a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java @@ -19,13 +19,13 @@ package org.elasticsearch.index.query; +import com.fasterxml.jackson.core.JsonParseException; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.ParsingException; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; @@ -336,8 +336,6 @@ public void testUnknownQueryName() throws IOException { * test that two queries in object throws error */ public void testTooManyQueriesInObject() throws IOException { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); String clauseType = randomFrom("must", "should", "must_not", "filter"); // should also throw error if invalid query is preceded by a valid one String query = "{\n" + @@ -352,8 +350,8 @@ public void testTooManyQueriesInObject() throws IOException { " }\n" + " }\n" + "}"; - ParsingException ex = expectThrows(ParsingException.class, () -> parseQuery(query)); - assertEquals("[match] malformed query, expected [END_OBJECT] but found [FIELD_NAME]", ex.getMessage()); + JsonParseException ex = expectThrows(JsonParseException.class, () -> parseQuery(query)); + assertTrue(ex.getMessage().contains("Duplicate field 'match'")); } public void testRewrite() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java index 08d234cc54bb3..a308aa8fcddfe 100644 --- a/server/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java @@ -19,10 +19,10 @@ package org.elasticsearch.index.query; +import com.fasterxml.jackson.core.JsonParseException; import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.ParsingException; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.AbstractQueryTestCase; @@ -66,14 +66,12 @@ public void testFilterElement() throws IOException { * test that multiple "filter" elements causes {@link ParsingException} */ public void testMultipleFilterElements() throws IOException { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); String queryString = "{ \"" + ConstantScoreQueryBuilder.NAME + "\" : {\n" + "\"filter\" : { \"term\": { \"foo\": \"a\" } },\n" + "\"filter\" : { \"term\": { \"foo\": \"x\" } },\n" + "} }"; - ParsingException e = expectThrows(ParsingException.class, () -> parseQuery(queryString)); - assertThat(e.getMessage(), containsString("accepts only one 'filter' element")); + JsonParseException e = expectThrows(JsonParseException.class, () -> parseQuery(queryString)); + assertThat(e.getMessage(), containsString("Duplicate field 'filter'")); } /** diff --git a/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java index 624205a1a3cbd..dc8f2b6a0dd6a 100644 --- a/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java @@ -35,7 +35,6 @@ import org.elasticsearch.common.lucene.search.function.FunctionScoreQuery; import org.elasticsearch.common.lucene.search.function.WeightFactorFunction; import org.elasticsearch.common.unit.DistanceUnit; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; @@ -727,8 +726,6 @@ public void testMalformedQueryMultipleQueryObjects() throws IOException { } public void testMalformedQueryMultipleQueryElements() throws IOException { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); String json = "{\n" + " \"function_score\":{\n" + " \"query\":{\n" + @@ -744,7 +741,8 @@ public void testMalformedQueryMultipleQueryElements() throws IOException { " }\n" + " }\n" + "}"; - expectParsingException(json, "[query] is already defined."); + JsonParseException e = expectThrows(JsonParseException.class, () -> parseQuery(json)); + assertThat(e.getMessage(), containsString("Duplicate field 'query'")); } private void expectParsingException(String json, Matcher messageMatcher) { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorFactoriesTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorFactoriesTests.java index 731156457a4f7..0313cd0512126 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorFactoriesTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorFactoriesTests.java @@ -18,11 +18,11 @@ */ package org.elasticsearch.search.aggregations; +import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.NamedXContentRegistry; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; @@ -111,8 +111,6 @@ public void testTwoTypes() throws Exception { } public void testTwoAggs() throws Exception { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); XContentBuilder source = JsonXContent.contentBuilder() .startObject() .startObject("by_date") @@ -138,8 +136,8 @@ public void testTwoAggs() throws Exception { .endObject(); XContentParser parser = createParser(source); assertSame(XContentParser.Token.START_OBJECT, parser.nextToken()); - Exception e = expectThrows(ParsingException.class, () -> AggregatorFactories.parseAggregators(parser)); - assertThat(e.toString(), containsString("Found two sub aggregation definitions under [by_date]")); + Exception e = expectThrows(JsonParseException.class, () -> AggregatorFactories.parseAggregators(parser)); + assertThat(e.toString(), containsString("Duplicate field 'aggs'")); } public void testInvalidAggregationName() throws Exception { @@ -177,8 +175,6 @@ public void testInvalidAggregationName() throws Exception { } public void testSameAggregationName() throws Exception { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); final String name = randomAlphaOfLengthBetween(1, 10); XContentBuilder source = JsonXContent.contentBuilder() .startObject() @@ -195,8 +191,8 @@ public void testSameAggregationName() throws Exception { .endObject(); XContentParser parser = createParser(source); assertSame(XContentParser.Token.START_OBJECT, parser.nextToken()); - Exception e = expectThrows(ParsingException.class, () -> AggregatorFactories.parseAggregators(parser)); - assertThat(e.toString(), containsString("Two sibling aggregations cannot have the same name: [" + name + "]")); + Exception e = expectThrows(JsonParseException.class, () -> AggregatorFactories.parseAggregators(parser)); + assertThat(e.toString(), containsString("Duplicate field '" + name + "'")); } public void testMissingName() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java b/server/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java index ca95310cd501f..d7f0bc0979957 100644 --- a/server/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java +++ b/server/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java @@ -26,7 +26,6 @@ import org.apache.lucene.search.spell.NGramDistance; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParseException; @@ -162,13 +161,9 @@ public void testIllegalXContent() throws IOException { "Required [field]"); // test two fieldnames - if (XContent.isStrictDuplicateDetectionEnabled()) { - logger.info("Skipping test as it uses a custom duplicate check that is obsolete when strict duplicate checks are enabled."); - } else { - directGenerator = "{ \"field\" : \"f1\", \"field\" : \"f2\" }"; - assertIllegalXContent(directGenerator, IllegalArgumentException.class, - "[direct_generator] failed to parse field [field]"); - } + directGenerator = "{ \"field\" : \"f1\", \"field\" : \"f2\" }"; + assertIllegalXContent(directGenerator, XContentParseException.class, + "[direct_generator] failed to parse object"); // test unknown field directGenerator = "{ \"unknown_param\" : \"f1\" }"; diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParser.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParser.java index 9719532b94286..fefd6e6a276c6 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParser.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParser.java @@ -81,9 +81,6 @@ public ClientYamlSuiteRestApi parse(String location, XContentParser parser) thro if (parser.currentToken() == XContentParser.Token.START_OBJECT && "parts".equals(currentFieldName)) { while (parser.nextToken() == XContentParser.Token.FIELD_NAME) { String part = parser.currentName(); - if (restApi.getPathParts().containsKey(part)) { - throw new IllegalArgumentException("Found duplicate part [" + part + "]"); - } parser.nextToken(); if (parser.currentToken() != XContentParser.Token.START_OBJECT) { throw new IllegalArgumentException("Expected parts field in rest api definition to contain an object"); @@ -94,12 +91,7 @@ public ClientYamlSuiteRestApi parse(String location, XContentParser parser) thro if (parser.currentToken() == XContentParser.Token.START_OBJECT && "params".equals(currentFieldName)) { while (parser.nextToken() == XContentParser.Token.FIELD_NAME) { - String param = parser.currentName(); - if (restApi.getParams().containsKey(param)) { - throw new IllegalArgumentException("Found duplicate param [" + param + "]"); - } - parser.nextToken(); if (parser.currentToken() != XContentParser.Token.START_OBJECT) { throw new IllegalArgumentException("Expected params field in rest api definition to contain an object"); diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index e1346d3f6967d..14cfddde63e13 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -150,10 +150,7 @@ public static DoSection parse(XContentParser parser) throws IOException { String body = parser.text(); XContentParser bodyParser = JsonXContent.jsonXContent .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, body); - //multiple bodies are supported e.g. in case of bulk provided as a whole string - while(bodyParser.nextToken() != null) { - apiCallSection.addBody(bodyParser.mapOrdered()); - } + apiCallSection.addBody(bodyParser.mapOrdered()); } else { apiCallSection.addParam(paramName, parser.text()); } diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserFailingTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserFailingTests.java index 208232dcfd7cd..8d1502577502e 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserFailingTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/restspec/ClientYamlSuiteRestApiParserFailingTests.java @@ -18,7 +18,6 @@ */ package org.elasticsearch.test.rest.yaml.restspec; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.yaml.YamlXContent; import org.elasticsearch.test.ESTestCase; @@ -71,72 +70,6 @@ public void testDuplicatePaths() throws Exception { "}", "ping.json", "Found duplicate path [/pingtwo]"); } - public void testDuplicateParts() throws Exception { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); - parseAndExpectFailure("{\n" + - " \"ping\": {" + - " \"documentation\": \"http://www.elasticsearch.org/guide/\"," + - " \"methods\": [\"PUT\"]," + - " \"url\": {" + - " \"path\": \"/\"," + - " \"paths\": [\"/\"]," + - " \"parts\": {" + - " \"index\": {" + - " \"type\" : \"string\",\n" + - " \"description\" : \"index part\"\n" + - " }," + - " \"type\": {" + - " \"type\" : \"string\",\n" + - " \"description\" : \"type part\"\n" + - " }," + - " \"index\": {" + - " \"type\" : \"string\",\n" + - " \"description\" : \"index parameter part\"\n" + - " }" + - " }," + - " \"params\": {" + - " \"type\" : \"boolean\",\n" + - " \"description\" : \"Whether specified concrete indices should be ignored when unavailable (missing or closed)\"" + - " }" + - " }," + - " \"body\": null" + - " }" + - "}", "ping.json", "Found duplicate part [index]"); - } - - public void testDuplicateParams() throws Exception { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); - parseAndExpectFailure("{\n" + - " \"ping\": {" + - " \"documentation\": \"http://www.elasticsearch.org/guide/\"," + - " \"methods\": [\"PUT\"]," + - " \"url\": {" + - " \"path\": \"/\"," + - " \"paths\": [\"/\"]," + - " \"parts\": {" + - " }," + - " \"params\": {" + - " \"timeout\": {" + - " \"type\" : \"string\",\n" + - " \"description\" : \"timeout parameter\"\n" + - " }," + - " \"refresh\": {" + - " \"type\" : \"string\",\n" + - " \"description\" : \"refresh parameter\"\n" + - " }," + - " \"timeout\": {" + - " \"type\" : \"string\",\n" + - " \"description\" : \"timeout parameter again\"\n" + - " }" + - " }" + - " }," + - " \"body\": null" + - " }" + - "}", "ping.json", "Found duplicate param [timeout]"); - } - public void testBrokenSpecShouldThrowUsefulExceptionWhenParsingFailsOnParams() throws Exception { parseAndExpectFailure(BROKEN_SPEC_PARAMS, "ping.json", "Expected params field in rest api definition to contain an object"); } diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java index a55470d67a69a..e3ec40ab2daa5 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java @@ -24,7 +24,6 @@ import org.elasticsearch.client.Node; import org.elasticsearch.client.NodeSelector; import org.elasticsearch.common.logging.DeprecationLogger; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentLocation; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.yaml.YamlXContent; @@ -215,9 +214,6 @@ public void testParseDoSectionWithJsonMultipleBodiesAsLongString() throws Except } public void testParseDoSectionWithJsonMultipleBodiesRepeatedProperty() throws Exception { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); - String[] bodies = new String[] { "{ \"index\": { \"_index\":\"test_index\", \"_type\":\"test_type\", \"_id\":\"test_id\" } }", "{ \"f1\":\"v1\", \"f2\":42 }", @@ -226,9 +222,7 @@ public void testParseDoSectionWithJsonMultipleBodiesRepeatedProperty() throws Ex "bulk:\n" + " refresh: true\n" + " body: \n" + - " " + bodies[0] + "\n" + - " body: \n" + - " " + bodies[1] + " " + bodies[0] ); DoSection doSection = DoSection.parse(parser); @@ -305,9 +299,6 @@ public void testParseDoSectionWithYamlMultipleBodies() throws Exception { } public void testParseDoSectionWithYamlMultipleBodiesRepeatedProperty() throws Exception { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); - parser = createParser(YamlXContent.yamlXContent, "bulk:\n" + " refresh: true\n" + @@ -315,14 +306,10 @@ public void testParseDoSectionWithYamlMultipleBodiesRepeatedProperty() throws Ex " index:\n" + " _index: test_index\n" + " _type: test_type\n" + - " _id: test_id\n" + - " body:\n" + - " f1: v1\n" + - " f2: 42\n" + " _id: test_id\n" ); - String[] bodies = new String[2]; + String[] bodies = new String[1]; bodies[0] = "{\"index\": {\"_index\": \"test_index\", \"_type\": \"test_type\", \"_id\": \"test_id\"}}"; - bodies[1] = "{ \"f1\":\"v1\", \"f2\": 42 }"; DoSection doSection = DoSection.parse(parser); ApiCallSection apiCallSection = doSection.getApiCallSection(); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareCondition.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareCondition.java index 6aeccf734c2f4..c1c458b1d661a 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareCondition.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareCondition.java @@ -69,7 +69,6 @@ public static ArrayCompareCondition parse(Clock clock, String watchId, XContentP String path = null; Op op = null; Object value = null; - boolean haveValue = false; Quantifier quantifier = null; XContentParser.Token token; @@ -86,11 +85,6 @@ public static ArrayCompareCondition parse(Clock clock, String watchId, XContentP parser.nextToken(); path = parser.text(); } else { - if (op != null) { - throw new ElasticsearchParseException("could not parse [{}] condition for watch [{}]. encountered " + - "duplicate comparison operator, but already saw [{}].", TYPE, watchId, parser.currentName(), op - .id()); - } try { op = Op.resolve(parser.currentName()); } catch (IllegalArgumentException iae) { @@ -102,11 +96,6 @@ public static ArrayCompareCondition parse(Clock clock, String watchId, XContentP while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { if (parser.currentName().equals("value")) { - if (haveValue) { - throw new ElasticsearchParseException("could not parse [{}] condition for watch [{}]. " + - "encountered duplicate field \"value\", but already saw value [{}].", TYPE, - watchId, value); - } token = parser.nextToken(); if (!op.supportsStructures() && !token.isValue() && token != XContentParser.Token.VALUE_NULL) { throw new ElasticsearchParseException("could not parse [{}] condition for watch [{}]. " + @@ -115,13 +104,7 @@ public static ArrayCompareCondition parse(Clock clock, String watchId, XContentP op.name().toLowerCase(Locale.ROOT), token); } value = XContentUtils.readValue(parser, token); - haveValue = true; } else if (parser.currentName().equals("quantifier")) { - if (quantifier != null) { - throw new ElasticsearchParseException("could not parse [{}] condition for watch [{}]. " + - "encountered duplicate field \"quantifier\", but already saw quantifier [{}].", - TYPE, watchId, quantifier.id()); - } parser.nextToken(); try { quantifier = Quantifier.resolve(parser.text()); diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/attachment/EmailAttachmentsParser.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/attachment/EmailAttachmentsParser.java index 2b4957f815444..66654facf14da 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/attachment/EmailAttachmentsParser.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/notification/email/attachment/EmailAttachmentsParser.java @@ -11,19 +11,18 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collections; -import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; public class EmailAttachmentsParser { - - private Map parsers; + private final Map parsers; public EmailAttachmentsParser(Map parsers) { this.parsers = Collections.unmodifiableMap(parsers); } public EmailAttachments parse(XContentParser parser) throws IOException { - Map attachments = new LinkedHashMap<>(); + List attachments = new ArrayList<>(); String currentFieldName = null; XContentParser.Token token; while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { @@ -42,18 +41,14 @@ public EmailAttachments parse(XContentParser parser) throws IOException { throw new ElasticsearchParseException("Cannot parse attachment of type [{}]", currentAttachmentType); } EmailAttachmentParser.EmailAttachment emailAttachment = emailAttachmentParser.parse(currentFieldName, parser); - if (attachments.containsKey(emailAttachment.id())) { - throw new ElasticsearchParseException("Attachment with id [{}] has already been created, must be renamed", - emailAttachment.id()); - } - attachments.put(emailAttachment.id(), emailAttachment); + attachments.add(emailAttachment); // one further to skip the end_object from the attachment parser.nextToken(); } } } - return new EmailAttachments(new ArrayList<>(attachments.values())); + return new EmailAttachments(attachments); } public Map getParsers() { diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareConditionTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareConditionTests.java index d97842fa359e0..fcd2767b52c7c 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareConditionTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareConditionTests.java @@ -5,9 +5,9 @@ */ package org.elasticsearch.xpack.watcher.condition; +import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; @@ -196,8 +196,6 @@ public void testParse() throws IOException { } public void testParseContainsDuplicateOperator() throws IOException { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); ArrayCompareCondition.Op op = randomFrom(ArrayCompareCondition.Op.values()); ArrayCompareCondition.Quantifier quantifier = randomFrom(ArrayCompareCondition.Quantifier.values()); Object value = randomFrom("value", 1, null); @@ -219,8 +217,8 @@ public void testParseContainsDuplicateOperator() throws IOException { XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder)); parser.nextToken(); - expectedException.expect(ElasticsearchParseException.class); - expectedException.expectMessage("duplicate comparison operator"); + expectedException.expect(JsonParseException.class); + expectedException.expectMessage("Duplicate field '" + op.id() + "'"); ArrayCompareCondition.parse(ClockMock.frozen(), "_id", parser); } @@ -249,8 +247,6 @@ public void testParseContainsUnknownOperator() throws IOException { } public void testParseContainsDuplicateValue() throws IOException { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); ArrayCompareCondition.Op op = randomFrom(ArrayCompareCondition.Op.values()); ArrayCompareCondition.Quantifier quantifier = randomFrom(ArrayCompareCondition.Quantifier.values()); Object value = randomFrom("value", 1, null); @@ -269,15 +265,13 @@ public void testParseContainsDuplicateValue() throws IOException { XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder)); parser.nextToken(); - expectedException.expect(ElasticsearchParseException.class); - expectedException.expectMessage("duplicate field \"value\""); + expectedException.expect(JsonParseException.class); + expectedException.expectMessage("Duplicate field 'value'"); ArrayCompareCondition.parse(ClockMock.frozen(), "_id", parser); } public void testParseContainsDuplicateQuantifier() throws IOException { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); ArrayCompareCondition.Op op = randomFrom(ArrayCompareCondition.Op.values()); ArrayCompareCondition.Quantifier quantifier = randomFrom(ArrayCompareCondition.Quantifier.values()); Object value = randomFrom("value", 1, null); @@ -296,8 +290,8 @@ public void testParseContainsDuplicateQuantifier() throws IOException { XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder)); parser.nextToken(); - expectedException.expect(ElasticsearchParseException.class); - expectedException.expectMessage("duplicate field \"quantifier\""); + expectedException.expect(JsonParseException.class); + expectedException.expectMessage("Duplicate field 'quantifier'"); ArrayCompareCondition.parse(ClockMock.frozen(), "_id", parser); } diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/EmailAttachmentParsersTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/EmailAttachmentParsersTests.java index 27f8330ed4782..15d7bb6e07d20 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/EmailAttachmentParsersTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/EmailAttachmentParsersTests.java @@ -5,11 +5,11 @@ */ package org.elasticsearch.xpack.watcher.notification.email.attachment; +import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.test.ESTestCase; @@ -116,8 +116,6 @@ public void testThatToXContentSerializationWorks() throws Exception { } public void testThatTwoAttachmentsWithTheSameIdThrowError() throws Exception { - assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", - XContent.isStrictDuplicateDetectionEnabled()); Map parsers = new HashMap<>(); parsers.put("test", new TestEmailAttachmentParser()); EmailAttachmentsParser parser = new EmailAttachmentsParser(parsers); @@ -146,8 +144,8 @@ public void testThatTwoAttachmentsWithTheSameIdThrowError() throws Exception { parser.parse(xContentParser); fail("Expected parser to fail but did not happen"); - } catch (ElasticsearchParseException e) { - assertThat(e.getMessage(), is("Attachment with id [my-name.json] has already been created, must be renamed")); + } catch (JsonParseException e) { + assertThat(e.getMessage(), containsString("Duplicate field 'my-name.json'")); } } From 5be7f08893355f47bec3d191f99a75a4b8790519 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Thu, 18 Oct 2018 15:17:09 +0200 Subject: [PATCH 2/4] Remove obsolete tests --- .../ConstructingObjectParserTests.java | 15 ---- .../common/settings/SettingsTests.java | 15 ---- .../index/query/BoolQueryBuilderTests.java | 23 ------ .../query/ConstantScoreQueryBuilderTests.java | 13 --- .../FunctionScoreQueryBuilderTests.java | 20 ----- .../AggregatorFactoriesTests.java | 52 ------------ .../phrase/DirectCandidateGeneratorTests.java | 5 -- .../condition/ArrayCompareConditionTests.java | 79 ------------------- .../EmailAttachmentParsersTests.java | 36 --------- 9 files changed, 258 deletions(-) diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java index 4c2f743c4045e..5fc3e612c18f7 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.common.xcontent; -import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.common.CheckedFunction; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; @@ -40,7 +39,6 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.nullValue; public class ConstructingObjectParserTests extends ESTestCase { @@ -165,19 +163,6 @@ public void testMissingFirstConstructorArgButNotRequired() throws IOException { assertEquals((Integer) 2, parsed.vegetable); } - public void testRepeatedConstructorParam() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, - "{\n" - + " \"vegetable\": 1,\n" - + " \"vegetable\": 2\n" - + "}"); - Throwable e = expectThrows(XContentParseException.class, () -> randomFrom(HasCtorArguments.ALL_PARSERS).apply(parser, null)); - assertEquals("[2:16] [has_required_arguments] failed to parse object", e.getMessage()); - e = e.getCause(); - assertThat(e, instanceOf(JsonParseException.class)); - assertThat(e.getMessage(), containsString("Duplicate field 'vegetable'")); - } - public void testBadParam() throws IOException { XContentParser parser = createParser(JsonXContent.jsonXContent, "{\n" diff --git a/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java b/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java index 739920038ecca..74c6a80fa3277 100644 --- a/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java +++ b/server/src/test/java/org/elasticsearch/common/settings/SettingsTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.common.settings; -import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.Version; import org.elasticsearch.common.Strings; @@ -552,20 +551,6 @@ public void testSimpleJsonSettings() throws Exception { assertThat(settings.getAsList("test1.test3").get(1), equalTo("test3-2")); } - public void testDuplicateKeysThrowsException() { - final String json = "{\"foo\":\"bar\",\"foo\":\"baz\"}"; - final SettingsException e = expectThrows(SettingsException.class, - () -> Settings.builder().loadFromSource(json, XContentType.JSON).build()); - assertThat(e.toString(), containsString("Duplicate field 'foo'")); - - String yaml = "foo: bar\nfoo: baz"; - SettingsException e1 = expectThrows(SettingsException.class, () -> { - Settings.builder().loadFromSource(yaml, XContentType.YAML); - }); - assertEquals(e1.getCause().getClass(), JsonParseException.class); - assertThat(e1.getCause().getMessage(), containsString("Duplicate field 'foo'")); - } - public void testToXContent() throws IOException { // this is just terrible but it's the existing behavior! Settings test = Settings.builder().putList("foo.bar", "1", "2", "3").put("foo.bar.baz", "test").build(); diff --git a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java index 0414340ad25bb..4a5e303a0542e 100644 --- a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.query; -import com.fasterxml.jackson.core.JsonParseException; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.ConstantScoreQuery; @@ -332,28 +331,6 @@ public void testUnknownQueryName() throws IOException { assertEquals("no [query] registered for [unknown_query]", ex.getMessage()); } - /** - * test that two queries in object throws error - */ - public void testTooManyQueriesInObject() throws IOException { - String clauseType = randomFrom("must", "should", "must_not", "filter"); - // should also throw error if invalid query is preceded by a valid one - String query = "{\n" + - " \"bool\": {\n" + - " \"" + clauseType + "\": {\n" + - " \"match\": {\n" + - " \"foo\": \"bar\"\n" + - " },\n" + - " \"match\": {\n" + - " \"baz\": \"buzz\"\n" + - " }\n" + - " }\n" + - " }\n" + - "}"; - JsonParseException ex = expectThrows(JsonParseException.class, () -> parseQuery(query)); - assertTrue(ex.getMessage().contains("Duplicate field 'match'")); - } - public void testRewrite() throws IOException { BoolQueryBuilder boolQueryBuilder = new BoolQueryBuilder(); boolean mustRewrite = false; diff --git a/server/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java index a308aa8fcddfe..6b928f376f3a3 100644 --- a/server/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.query; -import com.fasterxml.jackson.core.JsonParseException; import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.Query; import org.elasticsearch.common.ParsingException; @@ -62,18 +61,6 @@ public void testFilterElement() throws IOException { assertThat(e.getMessage(), containsString("requires a 'filter' element")); } - /** - * test that multiple "filter" elements causes {@link ParsingException} - */ - public void testMultipleFilterElements() throws IOException { - String queryString = "{ \"" + ConstantScoreQueryBuilder.NAME + "\" : {\n" + - "\"filter\" : { \"term\": { \"foo\": \"a\" } },\n" + - "\"filter\" : { \"term\": { \"foo\": \"x\" } },\n" + - "} }"; - JsonParseException e = expectThrows(JsonParseException.class, () -> parseQuery(queryString)); - assertThat(e.getMessage(), containsString("Duplicate field 'filter'")); - } - /** * test that "filter" does not accept an array of queries, throws {@link ParsingException} */ diff --git a/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java index dc8f2b6a0dd6a..163386c12b6c9 100644 --- a/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java @@ -725,26 +725,6 @@ public void testMalformedQueryMultipleQueryObjects() throws IOException { expectParsingException(json, equalTo("[bool] malformed query, expected [END_OBJECT] but found [FIELD_NAME]")); } - public void testMalformedQueryMultipleQueryElements() throws IOException { - String json = "{\n" + - " \"function_score\":{\n" + - " \"query\":{\n" + - " \"bool\":{\n" + - " \"must\":{\"match\":{\"field\":\"value\"}}" + - " }\n" + - " },\n" + - " \"query\":{\n" + - " \"bool\":{\n" + - " \"must\":{\"match\":{\"field\":\"value\"}}" + - " }\n" + - " }\n" + - " }\n" + - " }\n" + - "}"; - JsonParseException e = expectThrows(JsonParseException.class, () -> parseQuery(json)); - assertThat(e.getMessage(), containsString("Duplicate field 'query'")); - } - private void expectParsingException(String json, Matcher messageMatcher) { ParsingException e = expectThrows(ParsingException.class, () -> parseQuery(json)); assertThat(e.getMessage(), messageMatcher); diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorFactoriesTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorFactoriesTests.java index 0313cd0512126..8b636f2d6a6a0 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorFactoriesTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/AggregatorFactoriesTests.java @@ -18,7 +18,6 @@ */ package org.elasticsearch.search.aggregations; -import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.settings.Settings; @@ -110,36 +109,6 @@ public void testTwoTypes() throws Exception { assertThat(e.toString(), containsString("Found two aggregation type definitions in [in_stock]: [filter] and [terms]")); } - public void testTwoAggs() throws Exception { - XContentBuilder source = JsonXContent.contentBuilder() - .startObject() - .startObject("by_date") - .startObject("date_histogram") - .field("field", "timestamp") - .field("interval", "month") - .endObject() - .startObject("aggs") - .startObject("tag_count") - .startObject("cardinality") - .field("field", "tag") - .endObject() - .endObject() - .endObject() - .startObject("aggs") // 2nd "aggs": illegal - .startObject("tag_count2") - .startObject("cardinality") - .field("field", "tag") - .endObject() - .endObject() - .endObject() - .endObject() - .endObject(); - XContentParser parser = createParser(source); - assertSame(XContentParser.Token.START_OBJECT, parser.nextToken()); - Exception e = expectThrows(JsonParseException.class, () -> AggregatorFactories.parseAggregators(parser)); - assertThat(e.toString(), containsString("Duplicate field 'aggs'")); - } - public void testInvalidAggregationName() throws Exception { Matcher matcher = Pattern.compile("[^\\[\\]>]+").matcher(""); String name; @@ -174,27 +143,6 @@ public void testInvalidAggregationName() throws Exception { assertThat(e.toString(), containsString("Invalid aggregation name [" + name + "]")); } - public void testSameAggregationName() throws Exception { - final String name = randomAlphaOfLengthBetween(1, 10); - XContentBuilder source = JsonXContent.contentBuilder() - .startObject() - .startObject(name) - .startObject("terms") - .field("field", "a") - .endObject() - .endObject() - .startObject(name) - .startObject("terms") - .field("field", "b") - .endObject() - .endObject() - .endObject(); - XContentParser parser = createParser(source); - assertSame(XContentParser.Token.START_OBJECT, parser.nextToken()); - Exception e = expectThrows(JsonParseException.class, () -> AggregatorFactories.parseAggregators(parser)); - assertThat(e.toString(), containsString("Duplicate field '" + name + "'")); - } - public void testMissingName() throws Exception { XContentBuilder source = JsonXContent.contentBuilder() .startObject() diff --git a/server/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java b/server/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java index d7f0bc0979957..4ae19a1b6b04b 100644 --- a/server/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java +++ b/server/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java @@ -160,11 +160,6 @@ public void testIllegalXContent() throws IOException { assertIllegalXContent(directGenerator, IllegalArgumentException.class, "Required [field]"); - // test two fieldnames - directGenerator = "{ \"field\" : \"f1\", \"field\" : \"f2\" }"; - assertIllegalXContent(directGenerator, XContentParseException.class, - "[direct_generator] failed to parse object"); - // test unknown field directGenerator = "{ \"unknown_param\" : \"f1\" }"; assertIllegalXContent(directGenerator, IllegalArgumentException.class, diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareConditionTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareConditionTests.java index fcd2767b52c7c..8b072dddc7e31 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareConditionTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareConditionTests.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.xpack.watcher.condition; -import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -195,34 +194,6 @@ public void testParse() throws IOException { assertThat(condition.getQuantifier(), is(quantifier)); } - public void testParseContainsDuplicateOperator() throws IOException { - ArrayCompareCondition.Op op = randomFrom(ArrayCompareCondition.Op.values()); - ArrayCompareCondition.Quantifier quantifier = randomFrom(ArrayCompareCondition.Quantifier.values()); - Object value = randomFrom("value", 1, null); - XContentBuilder builder = - jsonBuilder().startObject() - .startObject("key1.key2") - .field("path", "key3.key4") - .startObject(op.id()) - .field("value", value) - .field("quantifier", quantifier.id()) - .endObject() - .startObject(op.id()) - .field("value", value) - .field("quantifier", quantifier.id()) - .endObject() - .endObject() - .endObject(); - - XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder)); - parser.nextToken(); - - expectedException.expect(JsonParseException.class); - expectedException.expectMessage("Duplicate field '" + op.id() + "'"); - - ArrayCompareCondition.parse(ClockMock.frozen(), "_id", parser); - } - public void testParseContainsUnknownOperator() throws IOException { ArrayCompareCondition.Quantifier quantifier = randomFrom(ArrayCompareCondition.Quantifier.values()); Object value = randomFrom("value", 1, null); @@ -246,56 +217,6 @@ public void testParseContainsUnknownOperator() throws IOException { ArrayCompareCondition.parse(ClockMock.frozen(), "_id", parser); } - public void testParseContainsDuplicateValue() throws IOException { - ArrayCompareCondition.Op op = randomFrom(ArrayCompareCondition.Op.values()); - ArrayCompareCondition.Quantifier quantifier = randomFrom(ArrayCompareCondition.Quantifier.values()); - Object value = randomFrom("value", 1, null); - XContentBuilder builder = - jsonBuilder().startObject() - .startObject("key1.key2") - .field("path", "key3.key4") - .startObject(op.id()) - .field("value", value) - .field("value", value) - .field("quantifier", quantifier.id()) - .endObject() - .endObject() - .endObject(); - - XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder)); - parser.nextToken(); - - expectedException.expect(JsonParseException.class); - expectedException.expectMessage("Duplicate field 'value'"); - - ArrayCompareCondition.parse(ClockMock.frozen(), "_id", parser); - } - - public void testParseContainsDuplicateQuantifier() throws IOException { - ArrayCompareCondition.Op op = randomFrom(ArrayCompareCondition.Op.values()); - ArrayCompareCondition.Quantifier quantifier = randomFrom(ArrayCompareCondition.Quantifier.values()); - Object value = randomFrom("value", 1, null); - XContentBuilder builder = - jsonBuilder().startObject() - .startObject("key1.key2") - .field("path", "key3.key4") - .startObject(op.id()) - .field("value", value) - .field("quantifier", quantifier.id()) - .field("quantifier", quantifier.id()) - .endObject() - .endObject() - .endObject(); - - XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder)); - parser.nextToken(); - - expectedException.expect(JsonParseException.class); - expectedException.expectMessage("Duplicate field 'quantifier'"); - - ArrayCompareCondition.parse(ClockMock.frozen(), "_id", parser); - } - public void testParseContainsUnknownQuantifier() throws IOException { ArrayCompareCondition.Op op = randomFrom(ArrayCompareCondition.Op.values()); Object value = randomFrom("value", 1, null); diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/EmailAttachmentParsersTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/EmailAttachmentParsersTests.java index 15d7bb6e07d20..74ce937d5efea 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/EmailAttachmentParsersTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/notification/email/attachment/EmailAttachmentParsersTests.java @@ -5,7 +5,6 @@ */ package org.elasticsearch.xpack.watcher.notification.email.attachment; -import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; @@ -29,7 +28,6 @@ import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.core.Is.is; @@ -115,40 +113,6 @@ public void testThatToXContentSerializationWorks() throws Exception { } } - public void testThatTwoAttachmentsWithTheSameIdThrowError() throws Exception { - Map parsers = new HashMap<>(); - parsers.put("test", new TestEmailAttachmentParser()); - EmailAttachmentsParser parser = new EmailAttachmentsParser(parsers); - - List attachments = new ArrayList<>(); - attachments.add(new TestEmailAttachment("my-name.json", "value")); - attachments.add(new TestEmailAttachment("my-name.json", "value")); - - EmailAttachments emailAttachments = new EmailAttachments(attachments); - XContentBuilder builder = jsonBuilder(); - builder.startObject(); - emailAttachments.toXContent(builder, ToXContent.EMPTY_PARAMS); - builder.endObject(); - logger.info("JSON is: " + Strings.toString(builder)); - - XContentParser xContentParser = createParser(builder); - try { - XContentParser.Token token = xContentParser.currentToken(); - assertNull(token); - - token = xContentParser.nextToken(); - assertThat(token, equalTo(XContentParser.Token.START_OBJECT)); - - token = xContentParser.nextToken(); - assertThat(token, equalTo(XContentParser.Token.FIELD_NAME)); - - parser.parse(xContentParser); - fail("Expected parser to fail but did not happen"); - } catch (JsonParseException e) { - assertThat(e.getMessage(), containsString("Duplicate field 'my-name.json'")); - } - } - public class TestEmailAttachmentParser implements EmailAttachmentParser { @Override From b4b909d723b228ed73ffa4163c8b04ed893d43f7 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Thu, 18 Oct 2018 16:24:26 +0200 Subject: [PATCH 3/4] Remove more obsolete tests --- .../rest/yaml/section/DoSectionTests.java | 54 ------------------- 1 file changed, 54 deletions(-) diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java index e3ec40ab2daa5..777ff74611885 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java @@ -213,32 +213,6 @@ public void testParseDoSectionWithJsonMultipleBodiesAsLongString() throws Except assertThat(apiCallSection.getBodies().size(), equalTo(4)); } - public void testParseDoSectionWithJsonMultipleBodiesRepeatedProperty() throws Exception { - String[] bodies = new String[] { - "{ \"index\": { \"_index\":\"test_index\", \"_type\":\"test_type\", \"_id\":\"test_id\" } }", - "{ \"f1\":\"v1\", \"f2\":42 }", - }; - parser = createParser(YamlXContent.yamlXContent, - "bulk:\n" + - " refresh: true\n" + - " body: \n" + - " " + bodies[0] - ); - - DoSection doSection = DoSection.parse(parser); - ApiCallSection apiCallSection = doSection.getApiCallSection(); - - assertThat(apiCallSection, notNullValue()); - assertThat(apiCallSection.getApi(), equalTo("bulk")); - assertThat(apiCallSection.getParams().size(), equalTo(1)); - assertThat(apiCallSection.getParams().get("refresh"), equalTo("true")); - assertThat(apiCallSection.hasBody(), equalTo(true)); - assertThat(apiCallSection.getBodies().size(), equalTo(bodies.length)); - for (int i = 0; i < bodies.length; i++) { - assertJsonEquals(apiCallSection.getBodies().get(i), bodies[i]); - } - } - public void testParseDoSectionWithYamlBody() throws Exception { parser = createParser(YamlXContent.yamlXContent, "search:\n" + @@ -298,34 +272,6 @@ public void testParseDoSectionWithYamlMultipleBodies() throws Exception { } } - public void testParseDoSectionWithYamlMultipleBodiesRepeatedProperty() throws Exception { - parser = createParser(YamlXContent.yamlXContent, - "bulk:\n" + - " refresh: true\n" + - " body:\n" + - " index:\n" + - " _index: test_index\n" + - " _type: test_type\n" + - " _id: test_id\n" - ); - String[] bodies = new String[1]; - bodies[0] = "{\"index\": {\"_index\": \"test_index\", \"_type\": \"test_type\", \"_id\": \"test_id\"}}"; - - DoSection doSection = DoSection.parse(parser); - ApiCallSection apiCallSection = doSection.getApiCallSection(); - - assertThat(apiCallSection, notNullValue()); - assertThat(apiCallSection.getApi(), equalTo("bulk")); - assertThat(apiCallSection.getParams().size(), equalTo(1)); - assertThat(apiCallSection.getParams().get("refresh"), equalTo("true")); - assertThat(apiCallSection.hasBody(), equalTo(true)); - assertThat(apiCallSection.getBodies().size(), equalTo(bodies.length)); - - for (int i = 0; i < bodies.length; i++) { - assertJsonEquals(apiCallSection.getBodies().get(i), bodies[i]); - } - } - public void testParseDoSectionWithYamlBodyMultiGet() throws Exception { parser = createParser(YamlXContent.yamlXContent, "mget:\n" + From bb936ebadd5261e0c7dc3727116ca84907d6d02c Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Thu, 18 Oct 2018 16:24:46 +0200 Subject: [PATCH 4/4] Allow multiple bodies as one string again --- .../org/elasticsearch/test/rest/yaml/section/DoSection.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index 14cfddde63e13..e1346d3f6967d 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -150,7 +150,10 @@ public static DoSection parse(XContentParser parser) throws IOException { String body = parser.text(); XContentParser bodyParser = JsonXContent.jsonXContent .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, body); - apiCallSection.addBody(bodyParser.mapOrdered()); + //multiple bodies are supported e.g. in case of bulk provided as a whole string + while(bodyParser.nextToken() != null) { + apiCallSection.addBody(bodyParser.mapOrdered()); + } } else { apiCallSection.addParam(paramName, parser.text()); }