From 8153d2ca6901741abdccbc71f67dad19544a998b Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Fri, 9 Dec 2016 12:48:28 +0100 Subject: [PATCH] Enable strict duplicate checks for JSON content With this commit we enable the Jackson feature 'STRICT_DUPLICATE_DETECTION' by default. This ensures that JSON keys are always unique. While this has a performance impact, benchmarking has indicated that the typical drop in indexing throughput is around 1 - 2%. As a last resort, we allow users to still disable strict duplicate checks by setting `-Des.json.strict_duplicate_detection=false` which is intentionally undocumented. Relates elastic/elasticsearchelastic/elasticsearch#19614 Original commit: elastic/x-pack-elasticsearch@cced57b884dc4d89724027a42cffeea694cc0d74 --- .../attachment/EmailAttachmentParsersTests.java | 5 +++-- .../watcher/actions/email/EmailActionTests.java | 4 +--- .../condition/ArrayCompareConditionTests.java | 13 +++++++------ 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/notification/email/attachment/EmailAttachmentParsersTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/notification/email/attachment/EmailAttachmentParsersTests.java index 23fd4f40004..8b36e7671fe 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/notification/email/attachment/EmailAttachmentParsersTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/notification/email/attachment/EmailAttachmentParsersTests.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.notification.email.attachment; +import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.xcontent.ToXContent; @@ -143,8 +144,8 @@ public class EmailAttachmentParsersTests extends ESTestCase { 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(), is("Duplicate field 'my-name.json'")); } } diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java index 211e446fca3..ba5c3cfdfb1 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java @@ -263,9 +263,7 @@ public class EmailActionTests extends ESTestCase { } builder.endObject(); } - } - - if (textBody != null || htmlBody != null) { + } else if (textBody != null || htmlBody != null) { builder.startObject("body"); if (textBody != null) { if (randomBoolean()) { diff --git a/elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareConditionTests.java b/elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareConditionTests.java index 8472e3bf7c5..5105ed25d8d 100644 --- a/elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareConditionTests.java +++ b/elasticsearch/src/test/java/org/elasticsearch/xpack/watcher/condition/ArrayCompareConditionTests.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.watcher.condition; +import com.fasterxml.jackson.core.JsonParseException; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -215,8 +216,8 @@ public class ArrayCompareConditionTests extends ESTestCase { XContentParser parser = JsonXContent.jsonXContent.createParser(builder.bytes()); 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); } @@ -263,8 +264,8 @@ public class ArrayCompareConditionTests extends ESTestCase { XContentParser parser = JsonXContent.jsonXContent.createParser(builder.bytes()); 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); } @@ -288,8 +289,8 @@ public class ArrayCompareConditionTests extends ESTestCase { XContentParser parser = JsonXContent.jsonXContent.createParser(builder.bytes()); 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); }