From cce830b69cb8d08ade1265bccc1d4dde8b0efda6 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Fri, 16 Dec 2016 14:01:46 +0100 Subject: [PATCH 1/2] Enable strict duplicate checks for all XContent types With this commit we enable the Jackson feature 'STRICT_DUPLICATE_DETECTION' by default for all XContent types (not only JSON). We have also changed the name of the system property to disable this feature from `es.json.strict_duplicate_detection` to the now more appropriate name `es.xcontent.strict_duplicate_detection`. Relates elastic/elasticsearch#19614 Relates elastic/elasticsearch#22073 --- .../common/xcontent/XContent.java | 22 +++++++++++++++++ .../common/xcontent/cbor/CborXContent.java | 2 ++ .../common/xcontent/json/JsonXContent.java | 24 +------------------ .../common/xcontent/smile/SmileXContent.java | 2 ++ .../common/xcontent/yaml/YamlXContent.java | 2 ++ .../loader/JsonSettingsLoaderTests.java | 6 ++--- .../loader/YamlSettingsLoaderTests.java | 4 ++++ .../common/xcontent/BaseXContentTestCase.java | 18 ++++++++++++++ .../ConstructingObjectParserTests.java | 4 ++-- .../xcontent/json/JsonXContentTests.java | 10 -------- .../index/query/BoolQueryBuilderTests.java | 5 ++-- .../query/ConstantScoreQueryBuilderTests.java | 5 ++-- .../FunctionScoreQueryBuilderTests.java | 5 ++-- .../aggregations/AggregatorParsingTests.java | 9 +++---- .../phrase/DirectCandidateGeneratorTests.java | 3 ++- .../migration/migrate_6_0/rest.asciidoc | 6 ++--- ...tClientYamlTestFragmentParserTestCase.java | 9 ++++--- .../yaml/parser/DoSectionParserTests.java | 7 ++++++ ...entYamlSuiteRestApiParserFailingTests.java | 9 +++---- 19 files changed, 93 insertions(+), 59 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/XContent.java b/core/src/main/java/org/elasticsearch/common/xcontent/XContent.java index c73f5f19d25e3..72210f09d9b72 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/XContent.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/XContent.java @@ -19,6 +19,7 @@ package org.elasticsearch.common.xcontent; +import org.elasticsearch.common.Booleans; import org.elasticsearch.common.bytes.BytesReference; import java.io.IOException; @@ -33,6 +34,27 @@ */ 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.parseBooleanExact(System.getProperty("es.xcontent.strict_duplicate_detection", "true")); + } + /** * The type this content handles and produces. */ diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java b/core/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java index 4224b5328a712..d79173cfc2b6f 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContent.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.core.JsonEncoding; import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.dataformat.cbor.CBORFactory; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.bytes.BytesReference; @@ -54,6 +55,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()); cborXContent = new CborXContent(); } diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java b/core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java index 4657a554099cf..1b0b351e6ef0d 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContent.java @@ -23,7 +23,6 @@ import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonParser; -import org.elasticsearch.common.Booleans; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.FastStringReader; import org.elasticsearch.common.xcontent.XContent; @@ -50,27 +49,6 @@ public static XContentBuilder contentBuilder() throws IOException { public static final JsonXContent jsonXContent; - /* - * 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 JSON parser will always check for duplicate keys in JSON content. This behavior is enabled by default but - * can be disabled by setting the otherwise undocumented system property "es.json.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. - * - */ - public static boolean isStrictDuplicateDetectionEnabled() { - // Don't allow duplicate keys in JSON content by default but let the user opt out - return Booleans.parseBooleanExact(System.getProperty("es.json.strict_duplicate_detection", "true")); - } - static { jsonFactory = new JsonFactory(); jsonFactory.configure(JsonGenerator.Feature.QUOTE_FIELD_NAMES, true); @@ -78,7 +56,7 @@ public static boolean isStrictDuplicateDetectionEnabled() { 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, isStrictDuplicateDetectionEnabled()); + jsonFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, XContent.isStrictDuplicateDetectionEnabled()); jsonXContent = new JsonXContent(); } diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java b/core/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java index 94ac9b9435626..643326cd82fa7 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContent.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.core.JsonEncoding; import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.dataformat.smile.SmileFactory; import com.fasterxml.jackson.dataformat.smile.SmileGenerator; import org.elasticsearch.common.bytes.BytesReference; @@ -55,6 +56,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()); smileXContent = new SmileXContent(); } diff --git a/core/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java b/core/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java index 54da03118d70b..7413f05f5831c 100644 --- a/core/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java +++ b/core/src/main/java/org/elasticsearch/common/xcontent/yaml/YamlXContent.java @@ -20,6 +20,7 @@ package org.elasticsearch.common.xcontent.yaml; import com.fasterxml.jackson.core.JsonEncoding; +import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.bytes.BytesReference; @@ -50,6 +51,7 @@ public static XContentBuilder contentBuilder() throws IOException { static { yamlFactory = new YAMLFactory(); + yamlFactory.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, XContent.isStrictDuplicateDetectionEnabled()); yamlXContent = new YamlXContent(); } diff --git a/core/src/test/java/org/elasticsearch/common/settings/loader/JsonSettingsLoaderTests.java b/core/src/test/java/org/elasticsearch/common/settings/loader/JsonSettingsLoaderTests.java index 70439ef8bf962..d917c0d12c074 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/loader/JsonSettingsLoaderTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/loader/JsonSettingsLoaderTests.java @@ -22,7 +22,7 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsException; -import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.test.ESTestCase; import static org.hamcrest.CoreMatchers.containsString; @@ -49,8 +49,8 @@ public void testSimpleJsonSettings() throws Exception { } public void testDuplicateKeysThrowsException() { - assumeFalse("Test only makes sense if JSON parser doesn't have strict duplicate checks enabled", - JsonXContent.isStrictDuplicateDetectionEnabled()); + 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).build()); assertEquals(e.getCause().getClass(), ElasticsearchParseException.class); diff --git a/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java b/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java index 7c956de8f9a76..eeb2df7e1d68c 100644 --- a/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java +++ b/core/src/test/java/org/elasticsearch/common/settings/loader/YamlSettingsLoaderTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsException; +import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.test.ESTestCase; import java.nio.charset.StandardCharsets; @@ -68,6 +69,9 @@ public void testIndentationWithExplicitDocumentStart() throws Exception { } public void testDuplicateKeysThrowsException() { + assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", + XContent.isStrictDuplicateDetectionEnabled()); + String yaml = "foo: bar\nfoo: baz"; SettingsException e = expectThrows(SettingsException.class, () -> { Settings.builder().loadFromSource(yaml); diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java b/core/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java index 57e9e0d52161c..1c0a92dbd743f 100644 --- a/core/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java +++ b/core/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java @@ -22,6 +22,7 @@ import com.fasterxml.jackson.core.JsonGenerationException; import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParseException; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.Constants; import org.elasticsearch.cluster.metadata.IndexMetaData; @@ -66,6 +67,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.startsWith; public abstract class BaseXContentTestCase extends ESTestCase { @@ -984,6 +986,22 @@ public void testSelfReferencingIterableTwoLevels() throws IOException { assertThat(e.getMessage(), containsString("Object has already been built and is self-referencing itself")); } + public void testChecksForDuplicates() throws Exception { + assumeTrue("Test only makes sense if XContent parser has strict duplicate checks enabled", + XContent.isStrictDuplicateDetectionEnabled()); + + BytesReference bytes = builder() + .startObject() + .field("key", 1) + .field("key", 2) + .endObject() + .bytes(); + + JsonParseException pex = expectThrows(JsonParseException.class, () -> createParser(xcontentType().xContent(), bytes).map()); + assertThat(pex.getMessage(), startsWith("Duplicate field 'key'")); + } + + private static void expectUnclosedException(ThrowingRunnable runnable) { IllegalStateException e = expectThrows(IllegalStateException.class, runnable); assertThat(e.getMessage(), containsString("Failed to close the XContentBuilder")); 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 387efdb5e5050..6cae59391c5b0 100644 --- a/core/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java +++ b/core/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java @@ -169,8 +169,8 @@ public void testMissingFirstConstructorArgButNotRequired() throws IOException { } public void testRepeatedConstructorParam() throws IOException { - assumeFalse("Test only makes sense if JSON parser doesn't have strict duplicate checks enabled", - JsonXContent.isStrictDuplicateDetectionEnabled()); + 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" diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/json/JsonXContentTests.java b/core/src/test/java/org/elasticsearch/common/xcontent/json/JsonXContentTests.java index b454983043267..4a79ddb4ec6a7 100644 --- a/core/src/test/java/org/elasticsearch/common/xcontent/json/JsonXContentTests.java +++ b/core/src/test/java/org/elasticsearch/common/xcontent/json/JsonXContentTests.java @@ -22,7 +22,6 @@ import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.common.xcontent.BaseXContentTestCase; import org.elasticsearch.common.xcontent.XContentType; @@ -40,13 +39,4 @@ public void testBigInteger() throws Exception { JsonGenerator generator = new JsonFactory().createGenerator(os); doTestBigInteger(generator, os); } - - public void testChecksForDuplicates() throws Exception { - assumeTrue("Test only makes sense if JSON parser doesn't have strict duplicate checks enabled", - JsonXContent.isStrictDuplicateDetectionEnabled()); - - JsonParseException pex = expectThrows(JsonParseException.class, - () -> XContentType.JSON.xContent().createParser("{ \"key\": 1, \"key\": 2 }").map()); - assertEquals("Duplicate field 'key'", pex.getMessage()); - } } diff --git a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java index 8e04ec7f553eb..13854ffc1e214 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java @@ -26,6 +26,7 @@ import org.apache.lucene.search.Query; import org.elasticsearch.common.ParseFieldMatcher; 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.XContentType; @@ -340,8 +341,8 @@ public void testUnknownQueryName() throws IOException { * test that two queries in object throws error */ public void testTooManyQueriesInObject() throws IOException { - assumeFalse("Test only makes sense if JSON parser doesn't have strict duplicate checks enabled", - JsonXContent.isStrictDuplicateDetectionEnabled()); + 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" + diff --git a/core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java index 9aa151e985dae..d8925af70ae91 100644 --- a/core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTests.java @@ -22,6 +22,7 @@ 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.common.xcontent.json.JsonXContent; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.test.AbstractQueryTestCase; @@ -66,8 +67,8 @@ public void testFilterElement() throws IOException { * test that multiple "filter" elements causes {@link ParsingException} */ public void testMultipleFilterElements() throws IOException { - assumeFalse("Test only makes sense if JSON parser doesn't have strict duplicate checks enabled", - JsonXContent.isStrictDuplicateDetectionEnabled()); + 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" + diff --git a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java index 0f79e90aa04d4..50c63e66f58af 100644 --- a/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/functionscore/FunctionScoreQueryBuilderTests.java @@ -36,6 +36,7 @@ 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.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.json.JsonXContent; @@ -739,8 +740,8 @@ public void testMalformedQueryMultipleQueryObjects() throws IOException { } public void testMalformedQueryMultipleQueryElements() throws IOException { - assumeFalse("Test only makes sense if JSON parser doesn't have strict duplicate checks enabled", - JsonXContent.isStrictDuplicateDetectionEnabled()); + 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" + diff --git a/core/src/test/java/org/elasticsearch/search/aggregations/AggregatorParsingTests.java b/core/src/test/java/org/elasticsearch/search/aggregations/AggregatorParsingTests.java index 87ca8ffdd93d3..26b3dd6f8f2af 100644 --- a/core/src/test/java/org/elasticsearch/search/aggregations/AggregatorParsingTests.java +++ b/core/src/test/java/org/elasticsearch/search/aggregations/AggregatorParsingTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.settings.Settings; +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; @@ -104,8 +105,8 @@ public void testTwoTypes() throws Exception { } public void testTwoAggs() throws Exception { - assumeFalse("Test only makes sense if JSON parser doesn't have strict duplicate checks enabled", - JsonXContent.isStrictDuplicateDetectionEnabled()); + 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") @@ -180,8 +181,8 @@ public void testInvalidAggregationName() throws Exception { } public void testSameAggregationName() throws Exception { - assumeFalse("Test only makes sense if JSON parser doesn't have strict duplicate checks enabled", - JsonXContent.isStrictDuplicateDetectionEnabled()); + assumeFalse("Test only makes sense if XContent parser doesn't have strict duplicate checks enabled", + XContent.isStrictDuplicateDetectionEnabled()); final String name = randomAsciiOfLengthBetween(1, 10); XContentBuilder source = JsonXContent.contentBuilder() .startObject() diff --git a/core/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java b/core/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java index 41713bbce9499..27ec964aa4ba2 100644 --- a/core/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java +++ b/core/src/test/java/org/elasticsearch/search/suggest/phrase/DirectCandidateGeneratorTests.java @@ -23,6 +23,7 @@ import org.elasticsearch.common.ParsingException; 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.XContentParser; @@ -149,7 +150,7 @@ public void testIllegalXContent() throws IOException { "Required [field]"); // test two fieldnames - if (JsonXContent.isStrictDuplicateDetectionEnabled()) { + 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\" }"; diff --git a/docs/reference/migration/migrate_6_0/rest.asciidoc b/docs/reference/migration/migrate_6_0/rest.asciidoc index 67931315893b7..baa2c808aefb1 100644 --- a/docs/reference/migration/migrate_6_0/rest.asciidoc +++ b/docs/reference/migration/migrate_6_0/rest.asciidoc @@ -8,10 +8,10 @@ This feature was removed in the 5.x series, but a backwards-compatibility layer system property `elasticsearch.json.allow_unquoted_field_names`. This backwards-compatibility layer has been removed in Elasticsearch 6.0.0. -==== Duplicate Keys in JSON +==== Duplicate Keys in Content -In previous versions of Elasticsearch, JSON documents were allowed to contain duplicate keys. Elasticsearch 6.0.0 - enforces that all keys are unique. +In previous versions of Elasticsearch, documents were allowed to contain duplicate keys. Elasticsearch 6.0.0 + enforces that all keys are unique. This applies to all content types: JSON, CBOR, Yaml and Smile. ==== Analyze API changes diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/parser/AbstractClientYamlTestFragmentParserTestCase.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/parser/AbstractClientYamlTestFragmentParserTestCase.java index 4346115c18422..3b0859d2d8b2e 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/parser/AbstractClientYamlTestFragmentParserTestCase.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/parser/AbstractClientYamlTestFragmentParserTestCase.java @@ -36,8 +36,11 @@ public abstract class AbstractClientYamlTestFragmentParserTestCase extends ESTes @After public void tearDown() throws Exception { super.tearDown(); - //this is the way to make sure that we consumed the whole yaml - assertThat(parser.currentToken(), nullValue()); - parser.close(); + // test may be skipped so we did not create a parser instance + if (parser != null) { + //this is the way to make sure that we consumed the whole yaml + assertThat(parser.currentToken(), nullValue()); + parser.close(); + } } } diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/parser/DoSectionParserTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/parser/DoSectionParserTests.java index 8992b3abde451..6057de6c702e6 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/parser/DoSectionParserTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/parser/DoSectionParserTests.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.test.rest.yaml.parser; +import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.yaml.YamlXContent; import org.elasticsearch.test.rest.yaml.section.ApiCallSection; @@ -126,6 +127,9 @@ 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 }", @@ -216,6 +220,9 @@ 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" + 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 d60fc66b7ec7d..a1d713db127b1 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,6 +18,7 @@ */ package org.elasticsearch.test.rest.yaml.restspec; +import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.common.xcontent.yaml.YamlXContent; @@ -72,8 +73,8 @@ public void testDuplicatePaths() throws Exception { } public void testDuplicateParts() throws Exception { - assumeFalse("Test only makes sense if JSON parser doesn't have strict duplicate checks enabled", - JsonXContent.isStrictDuplicateDetectionEnabled()); + 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/\"," + @@ -106,8 +107,8 @@ public void testDuplicateParts() throws Exception { } public void testDuplicateParams() throws Exception { - assumeFalse("Test only makes sense if JSON parser doesn't have strict duplicate checks enabled", - JsonXContent.isStrictDuplicateDetectionEnabled()); + 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/\"," + From 23f76b890c6d05a287d67c42e8bb81fda9d125e2 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Mon, 19 Dec 2016 09:28:52 +0100 Subject: [PATCH 2/2] More precise wording in migration docs --- docs/reference/migration/migrate_6_0/rest.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/migration/migrate_6_0/rest.asciidoc b/docs/reference/migration/migrate_6_0/rest.asciidoc index baa2c808aefb1..dcbc289300e28 100644 --- a/docs/reference/migration/migrate_6_0/rest.asciidoc +++ b/docs/reference/migration/migrate_6_0/rest.asciidoc @@ -8,7 +8,7 @@ This feature was removed in the 5.x series, but a backwards-compatibility layer system property `elasticsearch.json.allow_unquoted_field_names`. This backwards-compatibility layer has been removed in Elasticsearch 6.0.0. -==== Duplicate Keys in Content +==== Duplicate Keys in JSON, CBOR, Yaml and Smile In previous versions of Elasticsearch, documents were allowed to contain duplicate keys. Elasticsearch 6.0.0 enforces that all keys are unique. This applies to all content types: JSON, CBOR, Yaml and Smile.