From 399362e1449e4035645ecbad1e7551407b00f01a Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Fri, 25 Jun 2021 17:15:22 +0200 Subject: [PATCH 1/9] Support merging into root and allow_duplicate_keys --- .../reference/ingest/processors/json.asciidoc | 10 +-- .../common/xcontent/FilterXContentParser.java | 5 ++ .../common/xcontent/XContentParser.java | 2 + .../common/xcontent/XContentSubParser.java | 5 ++ .../xcontent/json/JsonXContentParser.java | 5 ++ .../xcontent/support/MapXContentParser.java | 5 ++ .../ingest/common/JsonProcessor.java | 40 +++++++++--- .../ingest/common/Processors.java | 4 +- .../ingest/common/JsonProcessorTests.java | 64 +++++++++++++++---- 9 files changed, 113 insertions(+), 27 deletions(-) diff --git a/docs/reference/ingest/processors/json.asciidoc b/docs/reference/ingest/processors/json.asciidoc index 2ccefbb0ef17c..2dd038e09b2de 100644 --- a/docs/reference/ingest/processors/json.asciidoc +++ b/docs/reference/ingest/processors/json.asciidoc @@ -10,10 +10,12 @@ Converts a JSON string into a structured JSON object. .Json Options [options="header"] |====== -| Name | Required | Default | Description -| `field` | yes | - | The field to be parsed. -| `target_field` | no | `field` | The field that the converted structured object will be written into. Any existing content in this field will be overwritten. -| `add_to_root` | no | false | Flag that forces the serialized json to be injected into the top level of the document. `target_field` must not be set when this option is chosen. +| Name | Required | Default | Description +| `field` | yes | - | The field to be parsed. +| `target_field` | no | `field` | The field that the converted structured object will be written into. Any existing content in this field will be overwritten. +| `add_to_root` | no | false | Flag that forces the serialized json to be merged into the top level of the document. `target_field` must not be set when this option is chosen. +| `allow_duplicate_keys`| no | false | When set to `true`, the JSON parser will not fail if the JSON contains duplicate keys. + Instead, the latest value wins. Allowing duplicate keys also improves execution time. include::common-options.asciidoc[] |====== diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/FilterXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/FilterXContentParser.java index fb9932fe3fbda..f52e7da9e14ce 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/FilterXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/FilterXContentParser.java @@ -33,6 +33,11 @@ public XContentType contentType() { return in.contentType(); } + @Override + public void allowDuplicateKeys(boolean allowDuplicateKeys) { + in.allowDuplicateKeys(allowDuplicateKeys); + } + @Override public Token nextToken() throws IOException { return in.nextToken(); diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java index 6dca3b7c94cb8..7094c77a771a0 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentParser.java @@ -112,6 +112,8 @@ enum NumberType { XContentType contentType(); + void allowDuplicateKeys(boolean allowDuplicateKeys); + Token nextToken() throws IOException; void skipChildren() throws IOException; diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java index 76759a57a6040..4927717ab8dba 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentSubParser.java @@ -43,6 +43,11 @@ public XContentType contentType() { return parser.contentType(); } + @Override + public void allowDuplicateKeys(boolean allowDuplicateKeys) { + parser.allowDuplicateKeys(allowDuplicateKeys); + } + @Override public Token nextToken() throws IOException { if (level > 0) { diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java index 1cc0e9cbd6b13..9b73847b4db0c 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/json/JsonXContentParser.java @@ -44,6 +44,11 @@ public XContentType contentType() { return XContentType.JSON; } + @Override + public void allowDuplicateKeys(boolean allowDuplicateKeys) { + parser.configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, allowDuplicateKeys == false); + } + @Override public Token nextToken() throws IOException { return convertToken(parser.nextToken()); diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java index b87fe9c5c9b86..2fc54003b0b3a 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java @@ -90,6 +90,11 @@ public XContentType contentType() { return xContentType; } + @Override + public void allowDuplicateKeys(boolean allowDuplicateKeys) { + // noop + } + @Override public Token nextToken() throws IOException { if (iterator == null) { diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java index bd84e10fa8fca..617fa1c7bb97f 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java @@ -14,6 +14,7 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; +import org.elasticsearch.common.xcontent.json.JsonXContentParser; import org.elasticsearch.ingest.AbstractProcessor; import org.elasticsearch.ingest.ConfigurationUtils; import org.elasticsearch.ingest.IngestDocument; @@ -36,12 +37,14 @@ public final class JsonProcessor extends AbstractProcessor { private final String field; private final String targetField; private final boolean addToRoot; + private final boolean allowDuplicateKeys; - JsonProcessor(String tag, String description, String field, String targetField, boolean addToRoot) { + JsonProcessor(String tag, String description, String field, String targetField, boolean addToRoot, boolean allowDuplicateKeys) { super(tag, description); this.field = field; this.targetField = targetField; this.addToRoot = addToRoot; + this.allowDuplicateKeys = allowDuplicateKeys; } public String getField() { @@ -56,11 +59,12 @@ boolean isAddToRoot() { return addToRoot; } - public static Object apply(Object fieldValue) { + public static Object apply(Object fieldValue, boolean allowDuplicateKeys) { BytesReference bytesRef = fieldValue == null ? new BytesArray("null") : new BytesArray(fieldValue.toString()); try (InputStream stream = bytesRef.streamInput(); - XContentParser parser = JsonXContent.jsonXContent + JsonXContentParser parser = (JsonXContentParser) JsonXContent.jsonXContent .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, stream)) { + parser.allowDuplicateKeys(allowDuplicateKeys); XContentParser.Token token = parser.nextToken(); Object value = null; if (token == XContentParser.Token.VALUE_NULL) { @@ -84,23 +88,40 @@ public static Object apply(Object fieldValue) { } } - public static void apply(Map ctx, String fieldName) { - Object value = apply(ctx.get(fieldName)); + public static void apply(Map ctx, String fieldName, boolean allowDuplicateKeys) { + Object value = apply(ctx.get(fieldName), allowDuplicateKeys); if (value instanceof Map) { @SuppressWarnings("unchecked") Map map = (Map) value; - ctx.putAll(map); + merge(ctx, map); } else { throw new IllegalArgumentException("cannot add non-map fields to root of document"); } } + @SuppressWarnings("unchecked") + public static void merge(Map target, Map from) { + for (String key : from.keySet()) { + if (target.containsKey(key)) { + Object targetValue = target.get(key); + Object fromValue = from.get(key); + if (targetValue instanceof Map && fromValue instanceof Map) { + merge((Map) targetValue, (Map) fromValue); + } else { + target.put(key, fromValue); + } + } else { + target.put(key, from.get(key)); + } + } + } + @Override public IngestDocument execute(IngestDocument document) throws Exception { if (addToRoot) { - apply(document.getSourceAndMetadata(), field); + apply(document.getSourceAndMetadata(), field, allowDuplicateKeys); } else { - document.setFieldValue(targetField, apply(document.getFieldValue(field, Object.class))); + document.setFieldValue(targetField, apply(document.getFieldValue(field, Object.class), allowDuplicateKeys)); } return document; } @@ -117,6 +138,7 @@ public JsonProcessor create(Map registry, String proc String field = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "field"); String targetField = ConfigurationUtils.readOptionalStringProperty(TYPE, processorTag, config, "target_field"); boolean addToRoot = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "add_to_root", false); + boolean allowDuplicateKeys = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "allow_duplicate_keys", false); if (addToRoot && targetField != null) { throw newConfigurationException(TYPE, processorTag, "target_field", @@ -127,7 +149,7 @@ public JsonProcessor create(Map registry, String proc targetField = field; } - return new JsonProcessor(processorTag, description, field, targetField, addToRoot); + return new JsonProcessor(processorTag, description, field, targetField, addToRoot, allowDuplicateKeys); } } } diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Processors.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Processors.java index c1d56b6c333df..99349737ce8e4 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Processors.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Processors.java @@ -59,7 +59,7 @@ public static String uppercase(String value) { * @return structured JSON object */ public static Object json(Object fieldValue) { - return JsonProcessor.apply(fieldValue); + return JsonProcessor.apply(fieldValue, false); } /** @@ -72,7 +72,7 @@ public static Object json(Object fieldValue) { * contains the JSON string */ public static void json(Map map, String field) { - JsonProcessor.apply(map, field); + JsonProcessor.apply(map, field, false); } /** diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java index a6e8e85679341..0f13c79ae0ca7 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java @@ -32,7 +32,7 @@ public void testExecute() throws Exception { String processorTag = randomAlphaOfLength(3); String randomField = randomAlphaOfLength(3); String randomTargetField = randomAlphaOfLength(2); - JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, randomField, randomTargetField, false); + JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, randomField, randomTargetField, false, false); Map document = new HashMap<>(); Map randomJsonMap = RandomDocumentPicks.randomSource(random()); @@ -47,7 +47,7 @@ public void testExecute() throws Exception { } public void testInvalidValue() { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); document.put("field", "blah blah"); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); @@ -58,7 +58,7 @@ public void testInvalidValue() { } public void testByteArray() { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); document.put("field", new byte[] { 0, 1 }); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); @@ -73,7 +73,7 @@ public void testByteArray() { } public void testNull() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); document.put("field", null); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); @@ -82,7 +82,7 @@ public void testNull() throws Exception { } public void testBoolean() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); boolean value = true; document.put("field", value); @@ -92,7 +92,7 @@ public void testBoolean() throws Exception { } public void testInteger() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); int value = 3; document.put("field", value); @@ -102,7 +102,7 @@ public void testInteger() throws Exception { } public void testDouble() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); double value = 3.0; document.put("field", value); @@ -112,7 +112,7 @@ public void testDouble() throws Exception { } public void testString() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); String value = "hello world"; document.put("field", "\"" + value + "\""); @@ -122,7 +122,7 @@ public void testString() throws Exception { } public void testArray() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); List value = Arrays.asList(true, true, false); document.put("field", value.toString()); @@ -132,7 +132,7 @@ public void testArray() throws Exception { } public void testFieldMissing() { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); @@ -143,7 +143,7 @@ public void testFieldMissing() { public void testAddToRoot() throws Exception { String processorTag = randomAlphaOfLength(3); String randomTargetField = randomAlphaOfLength(2); - JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, "a", randomTargetField, true); + JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, "a", randomTargetField, true, false); Map document = new HashMap<>(); String json = "{\"a\": 1, \"b\": 2}"; @@ -159,8 +159,48 @@ public void testAddToRoot() throws Exception { assertEquals("see", sourceAndMetadata.get("c")); } + public void testMergeRoot() throws Exception { + String processorTag = randomAlphaOfLength(3); + JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, "json", null, true, false); + Map document = new HashMap<>(); + + String json = "{\"foo\": {\"bar\": \"baz\"}}"; + document.put("json", json); + Map inner = new HashMap<>(); + inner.put("bar", "override_me"); + inner.put("qux", "quux"); + document.put("foo", inner); + + IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); + jsonProcessor.execute(ingestDocument); + + assertEquals("baz", ingestDocument.getFieldValue("foo.bar", String.class)); + assertEquals("quux", ingestDocument.getFieldValue("foo.qux", String.class)); + } + + public void testDuplicateKeys() throws Exception { + String processorTag = randomAlphaOfLength(3); + JsonProcessor lenientJsonProcessor = new JsonProcessor(processorTag, null, "a", null, true, true); + + Map document = new HashMap<>(); + String json = "{\"a\": 1, \"a\": 2}"; + document.put("a", json); + document.put("c", "see"); + + IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); + lenientJsonProcessor.execute(ingestDocument); + + Map sourceAndMetadata = ingestDocument.getSourceAndMetadata(); + assertEquals(2, sourceAndMetadata.get("a")); + assertEquals("see", sourceAndMetadata.get("c")); + + JsonProcessor strictJsonProcessor = new JsonProcessor(processorTag, null, "a", null, true, false); + Exception exception = expectThrows(IllegalArgumentException.class, () -> strictJsonProcessor.execute(RandomDocumentPicks.randomIngestDocument(random(), document))); + assertThat(exception.getMessage(), containsString("Duplicate field 'a'")); + } + public void testAddBoolToRoot() { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", true); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", true, false); Map document = new HashMap<>(); document.put("field", true); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); From 78e41375ebca9b9d86004d33a520e53c13ccc6d7 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Fri, 25 Jun 2021 23:09:36 +0200 Subject: [PATCH 2/9] Add rest api spec tests --- .../rest-api-spec/test/ingest/140_json.yml | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/140_json.yml b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/140_json.yml index 95de56b2be3d4..c41ac62d07a31 100644 --- a/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/140_json.yml +++ b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/140_json.yml @@ -4,6 +4,10 @@ teardown: ingest.delete_pipeline: id: "1" ignore: 404 + - do: + ingest.delete_pipeline: + id: "2" + ignore: 404 --- "Test JSON Processor": @@ -71,3 +75,42 @@ teardown: - match: { _source.foo_number: 3 } - is_true: _source.foo_boolean - is_false: _source.foo_null + +--- +"Test JSON Processor duplicate keys": + - do: + ingest.put_pipeline: + id: "2" + body: { + "processors": [ + { + "json" : { + "field" : "json", + "add_to_root": true, + "allow_duplicate_keys": true + } + } + ] + } + - match: { acknowledged: true } + + - do: + index: + index: test + id: 2 + pipeline: "2" + body: { + json: "{\"foo\": {\"bar\": \"baz\"}, \"dupe\": 1, \"dupe\": 2}", + foo: { + bar: "override_me", + qux: "quux" + } + } + + - do: + get: + index: test + id: 2 + - match: { _source.foo.bar: "baz" } + - match: { _source.foo.qux: "quux" } + - match: { _source.dupe: 2 } From 4ea1864352ceeac046d38029156d40c79cfb4bd5 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 28 Jun 2021 09:39:28 +0200 Subject: [PATCH 3/9] Fix checkstyle issue --- .../org/elasticsearch/ingest/common/JsonProcessorTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java index 0f13c79ae0ca7..e0677f509b25e 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java @@ -195,7 +195,8 @@ public void testDuplicateKeys() throws Exception { assertEquals("see", sourceAndMetadata.get("c")); JsonProcessor strictJsonProcessor = new JsonProcessor(processorTag, null, "a", null, true, false); - Exception exception = expectThrows(IllegalArgumentException.class, () -> strictJsonProcessor.execute(RandomDocumentPicks.randomIngestDocument(random(), document))); + Exception exception = expectThrows(IllegalArgumentException.class, () -> + strictJsonProcessor.execute(RandomDocumentPicks.randomIngestDocument(random(), document))); assertThat(exception.getMessage(), containsString("Duplicate field 'a'")); } From e66780621f9fadaaa64367ae96a0b9b54e88bc13 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 28 Jun 2021 15:33:23 +0200 Subject: [PATCH 4/9] Fix indentation --- .../java/org/elasticsearch/ingest/common/JsonProcessor.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java index 617fa1c7bb97f..0a212b89eb69d 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java @@ -14,7 +14,6 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.json.JsonXContent; -import org.elasticsearch.common.xcontent.json.JsonXContentParser; import org.elasticsearch.ingest.AbstractProcessor; import org.elasticsearch.ingest.ConfigurationUtils; import org.elasticsearch.ingest.IngestDocument; @@ -62,7 +61,7 @@ boolean isAddToRoot() { public static Object apply(Object fieldValue, boolean allowDuplicateKeys) { BytesReference bytesRef = fieldValue == null ? new BytesArray("null") : new BytesArray(fieldValue.toString()); try (InputStream stream = bytesRef.streamInput(); - JsonXContentParser parser = (JsonXContentParser) JsonXContent.jsonXContent + XContentParser parser = JsonXContent.jsonXContent .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, stream)) { parser.allowDuplicateKeys(allowDuplicateKeys); XContentParser.Token token = parser.nextToken(); @@ -119,7 +118,7 @@ public static void merge(Map target, Map from) { @Override public IngestDocument execute(IngestDocument document) throws Exception { if (addToRoot) { - apply(document.getSourceAndMetadata(), field, allowDuplicateKeys); + apply(document.getSourceAndMetadata(), field, allowDuplicateKeys); } else { document.setFieldValue(targetField, apply(document.getFieldValue(field, Object.class), allowDuplicateKeys)); } From 9c1755c2259dd5199e8c89c80f37e41e4a9c8f01 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Mon, 28 Jun 2021 16:28:46 +0200 Subject: [PATCH 5/9] Add allowDuplicateKeys XContent test --- .../common/xcontent/cbor/CborXContentParser.java | 5 +++++ .../common/xcontent/smile/SmileXContentParser.java | 5 +++++ .../common/xcontent/BaseXContentTestCase.java | 14 +++++++++++++- .../common/xcontent/cbor/CborXContentTests.java | 7 +++++++ .../common/xcontent/smile/SmileXContentTests.java | 7 +++++++ 5 files changed, 37 insertions(+), 1 deletion(-) diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContentParser.java index fed34b83a4028..d6de473a35aa4 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/cbor/CborXContentParser.java @@ -32,4 +32,9 @@ public CborXContentParser(NamedXContentRegistry xContentRegistry, public XContentType contentType() { return XContentType.CBOR; } + + @Override + public void allowDuplicateKeys(boolean allowDuplicateKeys) { + throw new UnsupportedOperationException("Allowing duplicate keys after the parser has been created is not possible for CBOR"); + } } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContentParser.java index f78064a7d5309..c743849cbca46 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/smile/SmileXContentParser.java @@ -32,4 +32,9 @@ public SmileXContentParser(NamedXContentRegistry xContentRegistry, public XContentType contentType() { return XContentType.SMILE; } + + @Override + public void allowDuplicateKeys(boolean allowDuplicateKeys) { + throw new UnsupportedOperationException("Allowing duplicate keys after the parser has been created is not possible for Smile"); + } } 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 0ff04c02f50d7..9626c19fa1c14 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/BaseXContentTestCase.java @@ -80,7 +80,7 @@ public abstract class BaseXContentTestCase extends ESTestCase { protected abstract XContentType xcontentType(); - private XContentBuilder builder() throws IOException { + protected XContentBuilder builder() throws IOException { return XContentBuilder.builder(xcontentType().xContent()); } @@ -1149,6 +1149,18 @@ public void testChecksForDuplicates() throws Exception { } } + public void testAllowsDuplicates() throws Exception { + XContentBuilder builder = builder() + .startObject() + .field("key", 1) + .field("key", 2) + .endObject(); + try (XContentParser xParser = createParser(builder)) { + xParser.allowDuplicateKeys(true); + assertThat(xParser.map(), equalTo(Map.of("key", 2))); + } + } + public void testNamedObject() throws IOException { Object test1 = new Object(); Object test2 = new Object(); diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/cbor/CborXContentTests.java b/server/src/test/java/org/elasticsearch/common/xcontent/cbor/CborXContentTests.java index c8115215b6455..8aca16eb9877b 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/cbor/CborXContentTests.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/cbor/CborXContentTests.java @@ -12,6 +12,7 @@ import com.fasterxml.jackson.dataformat.cbor.CBORFactory; import org.elasticsearch.common.xcontent.BaseXContentTestCase; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import java.io.ByteArrayOutputStream; @@ -28,4 +29,10 @@ public void testBigInteger() throws Exception { JsonGenerator generator = new CBORFactory().createGenerator(os); doTestBigInteger(generator, os); } + + public void testAllowsDuplicates() throws Exception { + try (XContentParser xParser = createParser(builder().startObject().endObject())) { + expectThrows(UnsupportedOperationException.class, () -> xParser.allowDuplicateKeys(true)); + } + } } diff --git a/server/src/test/java/org/elasticsearch/common/xcontent/smile/SmileXContentTests.java b/server/src/test/java/org/elasticsearch/common/xcontent/smile/SmileXContentTests.java index 25c2de7252da3..45706fa5fa94d 100644 --- a/server/src/test/java/org/elasticsearch/common/xcontent/smile/SmileXContentTests.java +++ b/server/src/test/java/org/elasticsearch/common/xcontent/smile/SmileXContentTests.java @@ -12,6 +12,7 @@ import com.fasterxml.jackson.dataformat.smile.SmileFactory; import org.elasticsearch.common.xcontent.BaseXContentTestCase; +import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import java.io.ByteArrayOutputStream; @@ -28,4 +29,10 @@ public void testBigInteger() throws Exception { JsonGenerator generator = new SmileFactory().createGenerator(os); doTestBigInteger(generator, os); } + + public void testAllowsDuplicates() throws Exception { + try (XContentParser xParser = createParser(builder().startObject().endObject())) { + expectThrows(UnsupportedOperationException.class, () -> xParser.allowDuplicateKeys(true)); + } + } } From 2306bb278510aa8f53e0b7fa2a45138e2e856f74 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 1 Jul 2021 21:09:20 +0200 Subject: [PATCH 6/9] Add add_to_root_recursive_merge flag --- .../reference/ingest/processors/json.asciidoc | 13 ++--- .../ingest/common/JsonProcessor.java | 25 ++++++--- .../ingest/common/Processors.java | 2 +- .../ingest/common/JsonProcessorTests.java | 53 +++++++++++++------ .../rest-api-spec/test/ingest/140_json.yml | 1 + 5 files changed, 63 insertions(+), 31 deletions(-) diff --git a/docs/reference/ingest/processors/json.asciidoc b/docs/reference/ingest/processors/json.asciidoc index 2dd038e09b2de..2110810ec6338 100644 --- a/docs/reference/ingest/processors/json.asciidoc +++ b/docs/reference/ingest/processors/json.asciidoc @@ -10,12 +10,13 @@ Converts a JSON string into a structured JSON object. .Json Options [options="header"] |====== -| Name | Required | Default | Description -| `field` | yes | - | The field to be parsed. -| `target_field` | no | `field` | The field that the converted structured object will be written into. Any existing content in this field will be overwritten. -| `add_to_root` | no | false | Flag that forces the serialized json to be merged into the top level of the document. `target_field` must not be set when this option is chosen. -| `allow_duplicate_keys`| no | false | When set to `true`, the JSON parser will not fail if the JSON contains duplicate keys. - Instead, the latest value wins. Allowing duplicate keys also improves execution time. +| Name | Required | Default | Description +| `field` | yes | - | The field to be parsed. +| `target_field` | no | `field` | The field that the converted structured object will be written into. Any existing content in this field will be overwritten. +| `add_to_root` | no | false | Flag that forces the serialized json to be merged into the top level of the document. `target_field` must not be set when this option is chosen. +| `add_to_root_recursive_merge`| no | false | Flag that controls the merging strategy for `add_to_root`. When set to `false`, only the first level will be merged. When set to `true`, even nested objects will be merged. Only applicable if `add_to_root` is set to `true`. +| `allow_duplicate_keys` | no | false | When set to `true`, the JSON parser will not fail if the JSON contains duplicate keys. + Instead, the latest value wins. Allowing duplicate keys also improves execution time. include::common-options.asciidoc[] |====== diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java index 0a212b89eb69d..f9c631acf52a0 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java @@ -36,14 +36,16 @@ public final class JsonProcessor extends AbstractProcessor { private final String field; private final String targetField; private final boolean addToRoot; + private final boolean addToRootRecursiveMerge; private final boolean allowDuplicateKeys; - JsonProcessor(String tag, String description, String field, String targetField, boolean addToRoot, boolean allowDuplicateKeys) { + JsonProcessor(String tag, String description, String field, String targetField, boolean addToRoot, boolean addToRootRecursiveMerge, boolean allowDuplicateKeys) { super(tag, description); this.field = field; this.targetField = targetField; this.addToRoot = addToRoot; this.allowDuplicateKeys = allowDuplicateKeys; + this.addToRootRecursiveMerge = addToRootRecursiveMerge; } public String getField() { @@ -87,25 +89,29 @@ public static Object apply(Object fieldValue, boolean allowDuplicateKeys) { } } - public static void apply(Map ctx, String fieldName, boolean allowDuplicateKeys) { + public static void apply(Map ctx, String fieldName, boolean allowDuplicateKeys, boolean addToRootRecursiveMerge) { Object value = apply(ctx.get(fieldName), allowDuplicateKeys); if (value instanceof Map) { @SuppressWarnings("unchecked") Map map = (Map) value; - merge(ctx, map); + if (addToRootRecursiveMerge) { + recursiveMerge(ctx, map); + } else { + ctx.putAll(map); + } } else { throw new IllegalArgumentException("cannot add non-map fields to root of document"); } } @SuppressWarnings("unchecked") - public static void merge(Map target, Map from) { + public static void recursiveMerge(Map target, Map from) { for (String key : from.keySet()) { if (target.containsKey(key)) { Object targetValue = target.get(key); Object fromValue = from.get(key); if (targetValue instanceof Map && fromValue instanceof Map) { - merge((Map) targetValue, (Map) fromValue); + recursiveMerge((Map) targetValue, (Map) fromValue); } else { target.put(key, fromValue); } @@ -118,7 +124,7 @@ public static void merge(Map target, Map from) { @Override public IngestDocument execute(IngestDocument document) throws Exception { if (addToRoot) { - apply(document.getSourceAndMetadata(), field, allowDuplicateKeys); + apply(document.getSourceAndMetadata(), field, allowDuplicateKeys, addToRootRecursiveMerge); } else { document.setFieldValue(targetField, apply(document.getFieldValue(field, Object.class), allowDuplicateKeys)); } @@ -137,18 +143,23 @@ public JsonProcessor create(Map registry, String proc String field = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "field"); String targetField = ConfigurationUtils.readOptionalStringProperty(TYPE, processorTag, config, "target_field"); boolean addToRoot = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "add_to_root", false); + boolean addToRootRecursiveMerge = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "add_to_root_recursive_merge", false); boolean allowDuplicateKeys = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "allow_duplicate_keys", false); if (addToRoot && targetField != null) { throw newConfigurationException(TYPE, processorTag, "target_field", "Cannot set a target field while also setting `add_to_root` to true"); } + if (!addToRoot && addToRootRecursiveMerge) { + throw newConfigurationException(TYPE, processorTag, "add_to_root_recursive_merge", + "Cannot set `add_to_root_recursive_merge` to true if `add_to_root` is false"); + } if (targetField == null) { targetField = field; } - return new JsonProcessor(processorTag, description, field, targetField, addToRoot, allowDuplicateKeys); + return new JsonProcessor(processorTag, description, field, targetField, addToRoot, addToRootRecursiveMerge, allowDuplicateKeys); } } } diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Processors.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Processors.java index 99349737ce8e4..154a08ba9a9da 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Processors.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Processors.java @@ -72,7 +72,7 @@ public static Object json(Object fieldValue) { * contains the JSON string */ public static void json(Map map, String field) { - JsonProcessor.apply(map, field, false); + JsonProcessor.apply(map, field, false, false); } /** diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java index e0677f509b25e..883f289e14a49 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java @@ -32,7 +32,7 @@ public void testExecute() throws Exception { String processorTag = randomAlphaOfLength(3); String randomField = randomAlphaOfLength(3); String randomTargetField = randomAlphaOfLength(2); - JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, randomField, randomTargetField, false, false); + JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, randomField, randomTargetField, false, false, false); Map document = new HashMap<>(); Map randomJsonMap = RandomDocumentPicks.randomSource(random()); @@ -47,7 +47,7 @@ public void testExecute() throws Exception { } public void testInvalidValue() { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); Map document = new HashMap<>(); document.put("field", "blah blah"); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); @@ -58,7 +58,7 @@ public void testInvalidValue() { } public void testByteArray() { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); Map document = new HashMap<>(); document.put("field", new byte[] { 0, 1 }); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); @@ -73,7 +73,7 @@ public void testByteArray() { } public void testNull() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); Map document = new HashMap<>(); document.put("field", null); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); @@ -82,7 +82,7 @@ public void testNull() throws Exception { } public void testBoolean() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); Map document = new HashMap<>(); boolean value = true; document.put("field", value); @@ -92,7 +92,7 @@ public void testBoolean() throws Exception { } public void testInteger() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); Map document = new HashMap<>(); int value = 3; document.put("field", value); @@ -102,7 +102,7 @@ public void testInteger() throws Exception { } public void testDouble() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); Map document = new HashMap<>(); double value = 3.0; document.put("field", value); @@ -112,7 +112,7 @@ public void testDouble() throws Exception { } public void testString() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); Map document = new HashMap<>(); String value = "hello world"; document.put("field", "\"" + value + "\""); @@ -122,7 +122,7 @@ public void testString() throws Exception { } public void testArray() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); Map document = new HashMap<>(); List value = Arrays.asList(true, true, false); document.put("field", value.toString()); @@ -132,7 +132,7 @@ public void testArray() throws Exception { } public void testFieldMissing() { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); Map document = new HashMap<>(); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); @@ -143,7 +143,7 @@ public void testFieldMissing() { public void testAddToRoot() throws Exception { String processorTag = randomAlphaOfLength(3); String randomTargetField = randomAlphaOfLength(2); - JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, "a", randomTargetField, true, false); + JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, "a", randomTargetField, true, false, false); Map document = new HashMap<>(); String json = "{\"a\": 1, \"b\": 2}"; @@ -159,11 +159,11 @@ public void testAddToRoot() throws Exception { assertEquals("see", sourceAndMetadata.get("c")); } - public void testMergeRoot() throws Exception { + public void testAddToRootRecursiveMerge() throws Exception { String processorTag = randomAlphaOfLength(3); - JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, "json", null, true, false); - Map document = new HashMap<>(); + JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, "json", null, true, true, false); + Map document = new HashMap<>(); String json = "{\"foo\": {\"bar\": \"baz\"}}"; document.put("json", json); Map inner = new HashMap<>(); @@ -178,9 +178,28 @@ public void testMergeRoot() throws Exception { assertEquals("quux", ingestDocument.getFieldValue("foo.qux", String.class)); } + public void testAddToRootNonRecursiveMerge() throws Exception { + String processorTag = randomAlphaOfLength(3); + JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, "json", null, true, false, false); + + Map document = new HashMap<>(); + String json = "{\"foo\": {\"bar\": \"baz\"}}"; + document.put("json", json); + Map inner = new HashMap<>(); + inner.put("bar", "override_me"); + inner.put("qux", "quux"); + document.put("foo", inner); + + IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); + jsonProcessor.execute(ingestDocument); + + assertEquals("baz", ingestDocument.getFieldValue("foo.bar", String.class)); + assertFalse(ingestDocument.hasField("foo.qux")); + } + public void testDuplicateKeys() throws Exception { String processorTag = randomAlphaOfLength(3); - JsonProcessor lenientJsonProcessor = new JsonProcessor(processorTag, null, "a", null, true, true); + JsonProcessor lenientJsonProcessor = new JsonProcessor(processorTag, null, "a", null, true, false, true); Map document = new HashMap<>(); String json = "{\"a\": 1, \"a\": 2}"; @@ -194,14 +213,14 @@ public void testDuplicateKeys() throws Exception { assertEquals(2, sourceAndMetadata.get("a")); assertEquals("see", sourceAndMetadata.get("c")); - JsonProcessor strictJsonProcessor = new JsonProcessor(processorTag, null, "a", null, true, false); + JsonProcessor strictJsonProcessor = new JsonProcessor(processorTag, null, "a", null, true, false, false); Exception exception = expectThrows(IllegalArgumentException.class, () -> strictJsonProcessor.execute(RandomDocumentPicks.randomIngestDocument(random(), document))); assertThat(exception.getMessage(), containsString("Duplicate field 'a'")); } public void testAddBoolToRoot() { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", true, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", true, false, false); Map document = new HashMap<>(); document.put("field", true); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); diff --git a/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/140_json.yml b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/140_json.yml index c41ac62d07a31..fec7ff20aee6b 100644 --- a/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/140_json.yml +++ b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/140_json.yml @@ -87,6 +87,7 @@ teardown: "json" : { "field" : "json", "add_to_root": true, + "add_to_root_recursive_merge": true, "allow_duplicate_keys": true } } From 62fa155a0eb156110875917dcb4ae283d905f06b Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Thu, 1 Jul 2021 21:16:53 +0200 Subject: [PATCH 7/9] Make checkstyle happy --- .../org/elasticsearch/ingest/common/JsonProcessor.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java index f9c631acf52a0..b6b970797f4d4 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java @@ -39,7 +39,8 @@ public final class JsonProcessor extends AbstractProcessor { private final boolean addToRootRecursiveMerge; private final boolean allowDuplicateKeys; - JsonProcessor(String tag, String description, String field, String targetField, boolean addToRoot, boolean addToRootRecursiveMerge, boolean allowDuplicateKeys) { + JsonProcessor(String tag, String description, String field, String targetField, boolean addToRoot, boolean addToRootRecursiveMerge, + boolean allowDuplicateKeys) { super(tag, description); this.field = field; this.targetField = targetField; @@ -143,14 +144,15 @@ public JsonProcessor create(Map registry, String proc String field = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "field"); String targetField = ConfigurationUtils.readOptionalStringProperty(TYPE, processorTag, config, "target_field"); boolean addToRoot = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "add_to_root", false); - boolean addToRootRecursiveMerge = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "add_to_root_recursive_merge", false); + boolean addToRootRecursiveMerge = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, + "add_to_root_recursive_merge", false); boolean allowDuplicateKeys = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "allow_duplicate_keys", false); if (addToRoot && targetField != null) { throw newConfigurationException(TYPE, processorTag, "target_field", "Cannot set a target field while also setting `add_to_root` to true"); } - if (!addToRoot && addToRootRecursiveMerge) { + if (addToRoot == false && addToRootRecursiveMerge) { throw newConfigurationException(TYPE, processorTag, "add_to_root_recursive_merge", "Cannot set `add_to_root_recursive_merge` to true if `add_to_root` is false"); } From a26190fceadeaa3cb0f7eec2278e932ee160a0d4 Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 6 Jul 2021 12:24:26 +0200 Subject: [PATCH 8/9] Remove add_to_root_recursive_merge flag --- .../reference/ingest/processors/json.asciidoc | 1 - .../ingest/common/JsonProcessor.java | 40 ++--------- .../ingest/common/Processors.java | 2 +- .../ingest/common/JsonProcessorTests.java | 66 ++++--------------- .../rest-api-spec/test/ingest/140_json.yml | 9 +-- 5 files changed, 21 insertions(+), 97 deletions(-) diff --git a/docs/reference/ingest/processors/json.asciidoc b/docs/reference/ingest/processors/json.asciidoc index 2110810ec6338..8619740e792c4 100644 --- a/docs/reference/ingest/processors/json.asciidoc +++ b/docs/reference/ingest/processors/json.asciidoc @@ -14,7 +14,6 @@ Converts a JSON string into a structured JSON object. | `field` | yes | - | The field to be parsed. | `target_field` | no | `field` | The field that the converted structured object will be written into. Any existing content in this field will be overwritten. | `add_to_root` | no | false | Flag that forces the serialized json to be merged into the top level of the document. `target_field` must not be set when this option is chosen. -| `add_to_root_recursive_merge`| no | false | Flag that controls the merging strategy for `add_to_root`. When set to `false`, only the first level will be merged. When set to `true`, even nested objects will be merged. Only applicable if `add_to_root` is set to `true`. | `allow_duplicate_keys` | no | false | When set to `true`, the JSON parser will not fail if the JSON contains duplicate keys. Instead, the latest value wins. Allowing duplicate keys also improves execution time. include::common-options.asciidoc[] diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java index b6b970797f4d4..b3a12fff2369c 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/JsonProcessor.java @@ -36,17 +36,14 @@ public final class JsonProcessor extends AbstractProcessor { private final String field; private final String targetField; private final boolean addToRoot; - private final boolean addToRootRecursiveMerge; private final boolean allowDuplicateKeys; - JsonProcessor(String tag, String description, String field, String targetField, boolean addToRoot, boolean addToRootRecursiveMerge, - boolean allowDuplicateKeys) { + JsonProcessor(String tag, String description, String field, String targetField, boolean addToRoot, boolean allowDuplicateKeys) { super(tag, description); this.field = field; this.targetField = targetField; this.addToRoot = addToRoot; this.allowDuplicateKeys = allowDuplicateKeys; - this.addToRootRecursiveMerge = addToRootRecursiveMerge; } public String getField() { @@ -90,42 +87,21 @@ public static Object apply(Object fieldValue, boolean allowDuplicateKeys) { } } - public static void apply(Map ctx, String fieldName, boolean allowDuplicateKeys, boolean addToRootRecursiveMerge) { + public static void apply(Map ctx, String fieldName, boolean allowDuplicateKeys) { Object value = apply(ctx.get(fieldName), allowDuplicateKeys); if (value instanceof Map) { @SuppressWarnings("unchecked") Map map = (Map) value; - if (addToRootRecursiveMerge) { - recursiveMerge(ctx, map); - } else { - ctx.putAll(map); - } + ctx.putAll(map); } else { throw new IllegalArgumentException("cannot add non-map fields to root of document"); } } - @SuppressWarnings("unchecked") - public static void recursiveMerge(Map target, Map from) { - for (String key : from.keySet()) { - if (target.containsKey(key)) { - Object targetValue = target.get(key); - Object fromValue = from.get(key); - if (targetValue instanceof Map && fromValue instanceof Map) { - recursiveMerge((Map) targetValue, (Map) fromValue); - } else { - target.put(key, fromValue); - } - } else { - target.put(key, from.get(key)); - } - } - } - @Override public IngestDocument execute(IngestDocument document) throws Exception { if (addToRoot) { - apply(document.getSourceAndMetadata(), field, allowDuplicateKeys, addToRootRecursiveMerge); + apply(document.getSourceAndMetadata(), field, allowDuplicateKeys); } else { document.setFieldValue(targetField, apply(document.getFieldValue(field, Object.class), allowDuplicateKeys)); } @@ -144,24 +120,18 @@ public JsonProcessor create(Map registry, String proc String field = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "field"); String targetField = ConfigurationUtils.readOptionalStringProperty(TYPE, processorTag, config, "target_field"); boolean addToRoot = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "add_to_root", false); - boolean addToRootRecursiveMerge = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, - "add_to_root_recursive_merge", false); boolean allowDuplicateKeys = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "allow_duplicate_keys", false); if (addToRoot && targetField != null) { throw newConfigurationException(TYPE, processorTag, "target_field", "Cannot set a target field while also setting `add_to_root` to true"); } - if (addToRoot == false && addToRootRecursiveMerge) { - throw newConfigurationException(TYPE, processorTag, "add_to_root_recursive_merge", - "Cannot set `add_to_root_recursive_merge` to true if `add_to_root` is false"); - } if (targetField == null) { targetField = field; } - return new JsonProcessor(processorTag, description, field, targetField, addToRoot, addToRootRecursiveMerge, allowDuplicateKeys); + return new JsonProcessor(processorTag, description, field, targetField, addToRoot, allowDuplicateKeys); } } } diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Processors.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Processors.java index 154a08ba9a9da..99349737ce8e4 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Processors.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/Processors.java @@ -72,7 +72,7 @@ public static Object json(Object fieldValue) { * contains the JSON string */ public static void json(Map map, String field) { - JsonProcessor.apply(map, field, false, false); + JsonProcessor.apply(map, field, false); } /** diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java index 883f289e14a49..8c988797ab0bf 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/JsonProcessorTests.java @@ -32,7 +32,7 @@ public void testExecute() throws Exception { String processorTag = randomAlphaOfLength(3); String randomField = randomAlphaOfLength(3); String randomTargetField = randomAlphaOfLength(2); - JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, randomField, randomTargetField, false, false, false); + JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, randomField, randomTargetField, false, false); Map document = new HashMap<>(); Map randomJsonMap = RandomDocumentPicks.randomSource(random()); @@ -47,7 +47,7 @@ public void testExecute() throws Exception { } public void testInvalidValue() { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); document.put("field", "blah blah"); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); @@ -58,7 +58,7 @@ public void testInvalidValue() { } public void testByteArray() { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); document.put("field", new byte[] { 0, 1 }); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); @@ -73,7 +73,7 @@ public void testByteArray() { } public void testNull() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); document.put("field", null); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); @@ -82,7 +82,7 @@ public void testNull() throws Exception { } public void testBoolean() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); boolean value = true; document.put("field", value); @@ -92,7 +92,7 @@ public void testBoolean() throws Exception { } public void testInteger() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); int value = 3; document.put("field", value); @@ -102,7 +102,7 @@ public void testInteger() throws Exception { } public void testDouble() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); double value = 3.0; document.put("field", value); @@ -112,7 +112,7 @@ public void testDouble() throws Exception { } public void testString() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); String value = "hello world"; document.put("field", "\"" + value + "\""); @@ -122,7 +122,7 @@ public void testString() throws Exception { } public void testArray() throws Exception { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); List value = Arrays.asList(true, true, false); document.put("field", value.toString()); @@ -132,7 +132,7 @@ public void testArray() throws Exception { } public void testFieldMissing() { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", false, false); Map document = new HashMap<>(); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); @@ -143,7 +143,7 @@ public void testFieldMissing() { public void testAddToRoot() throws Exception { String processorTag = randomAlphaOfLength(3); String randomTargetField = randomAlphaOfLength(2); - JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, "a", randomTargetField, true, false, false); + JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, "a", randomTargetField, true, false); Map document = new HashMap<>(); String json = "{\"a\": 1, \"b\": 2}"; @@ -159,47 +159,9 @@ public void testAddToRoot() throws Exception { assertEquals("see", sourceAndMetadata.get("c")); } - public void testAddToRootRecursiveMerge() throws Exception { - String processorTag = randomAlphaOfLength(3); - JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, "json", null, true, true, false); - - Map document = new HashMap<>(); - String json = "{\"foo\": {\"bar\": \"baz\"}}"; - document.put("json", json); - Map inner = new HashMap<>(); - inner.put("bar", "override_me"); - inner.put("qux", "quux"); - document.put("foo", inner); - - IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); - jsonProcessor.execute(ingestDocument); - - assertEquals("baz", ingestDocument.getFieldValue("foo.bar", String.class)); - assertEquals("quux", ingestDocument.getFieldValue("foo.qux", String.class)); - } - - public void testAddToRootNonRecursiveMerge() throws Exception { - String processorTag = randomAlphaOfLength(3); - JsonProcessor jsonProcessor = new JsonProcessor(processorTag, null, "json", null, true, false, false); - - Map document = new HashMap<>(); - String json = "{\"foo\": {\"bar\": \"baz\"}}"; - document.put("json", json); - Map inner = new HashMap<>(); - inner.put("bar", "override_me"); - inner.put("qux", "quux"); - document.put("foo", inner); - - IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); - jsonProcessor.execute(ingestDocument); - - assertEquals("baz", ingestDocument.getFieldValue("foo.bar", String.class)); - assertFalse(ingestDocument.hasField("foo.qux")); - } - public void testDuplicateKeys() throws Exception { String processorTag = randomAlphaOfLength(3); - JsonProcessor lenientJsonProcessor = new JsonProcessor(processorTag, null, "a", null, true, false, true); + JsonProcessor lenientJsonProcessor = new JsonProcessor(processorTag, null, "a", null, true, true); Map document = new HashMap<>(); String json = "{\"a\": 1, \"a\": 2}"; @@ -213,14 +175,14 @@ public void testDuplicateKeys() throws Exception { assertEquals(2, sourceAndMetadata.get("a")); assertEquals("see", sourceAndMetadata.get("c")); - JsonProcessor strictJsonProcessor = new JsonProcessor(processorTag, null, "a", null, true, false, false); + JsonProcessor strictJsonProcessor = new JsonProcessor(processorTag, null, "a", null, true, false); Exception exception = expectThrows(IllegalArgumentException.class, () -> strictJsonProcessor.execute(RandomDocumentPicks.randomIngestDocument(random(), document))); assertThat(exception.getMessage(), containsString("Duplicate field 'a'")); } public void testAddBoolToRoot() { - JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", true, false, false); + JsonProcessor jsonProcessor = new JsonProcessor("tag", null, "field", "target_field", true, false); Map document = new HashMap<>(); document.put("field", true); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document); diff --git a/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/140_json.yml b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/140_json.yml index fec7ff20aee6b..aec40e9ac070c 100644 --- a/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/140_json.yml +++ b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/140_json.yml @@ -87,7 +87,6 @@ teardown: "json" : { "field" : "json", "add_to_root": true, - "add_to_root_recursive_merge": true, "allow_duplicate_keys": true } } @@ -101,17 +100,11 @@ teardown: id: 2 pipeline: "2" body: { - json: "{\"foo\": {\"bar\": \"baz\"}, \"dupe\": 1, \"dupe\": 2}", - foo: { - bar: "override_me", - qux: "quux" - } + json: "{\"dupe\": 1, \"dupe\": 2}", } - do: get: index: test id: 2 - - match: { _source.foo.bar: "baz" } - - match: { _source.foo.qux: "quux" } - match: { _source.dupe: 2 } From 60dc2d6f53ba40f4e818e28bd1cb07756b059a4e Mon Sep 17 00:00:00 2001 From: Felix Barnsteiner Date: Tue, 6 Jul 2021 12:30:53 +0200 Subject: [PATCH 9/9] polish --- docs/reference/ingest/processors/json.asciidoc | 2 +- .../common/xcontent/support/MapXContentParser.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference/ingest/processors/json.asciidoc b/docs/reference/ingest/processors/json.asciidoc index 8619740e792c4..c466b025924df 100644 --- a/docs/reference/ingest/processors/json.asciidoc +++ b/docs/reference/ingest/processors/json.asciidoc @@ -13,7 +13,7 @@ Converts a JSON string into a structured JSON object. | Name | Required | Default | Description | `field` | yes | - | The field to be parsed. | `target_field` | no | `field` | The field that the converted structured object will be written into. Any existing content in this field will be overwritten. -| `add_to_root` | no | false | Flag that forces the serialized json to be merged into the top level of the document. `target_field` must not be set when this option is chosen. +| `add_to_root` | no | false | Flag that forces the serialized json to be injected into the top level of the document. `target_field` must not be set when this option is chosen. | `allow_duplicate_keys` | no | false | When set to `true`, the JSON parser will not fail if the JSON contains duplicate keys. Instead, the latest value wins. Allowing duplicate keys also improves execution time. include::common-options.asciidoc[] diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java index 2fc54003b0b3a..b23cc553fdf42 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/support/MapXContentParser.java @@ -92,7 +92,7 @@ public XContentType contentType() { @Override public void allowDuplicateKeys(boolean allowDuplicateKeys) { - // noop + throw new UnsupportedOperationException("Allowing duplicate keys is not possible for maps"); } @Override