From 650986dcf7471046194d1a6373fcbb00de74efe5 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Tue, 30 Oct 2018 15:19:39 -0500 Subject: [PATCH 1/4] ingest: dot_expander_processor, ignore_missing and prevent null * support for ignore_missing, such that if the field does not exist in source document the processor will be skipped * fix bug such that if the location to expand to already exists as nested objects, don't append a null value. --- docs/reference/ingest/ingest-node.asciidoc | 7 ++- .../ingest/common/DotExpanderProcessor.java | 14 +++-- .../common/DotExpanderProcessorTests.java | 55 +++++++++++++++---- 3 files changed, 58 insertions(+), 18 deletions(-) diff --git a/docs/reference/ingest/ingest-node.asciidoc b/docs/reference/ingest/ingest-node.asciidoc index eeb914facc2c6..e68bec5260109 100644 --- a/docs/reference/ingest/ingest-node.asciidoc +++ b/docs/reference/ingest/ingest-node.asciidoc @@ -1286,9 +1286,10 @@ Otherwise these <> can't be accessed by any .Dot Expand Options [options="header"] |====== -| Name | Required | Default | Description -| `field` | yes | - | The field to expand into an object field -| `path` | no | - | The field that contains the field to expand. Only required if the field to expand is part another object field, because the `field` option can only understand leaf fields. +| Name | Required | Default | Description +| `field` | yes | - | The field to expand into an object field +| `path` | no | - | The field that contains the field to expand. Only required if the field to expand is part another object field, because the `field` option can only understand leaf fields. +| `ignore_missing` | no | `false` | If `true` and `field` does not exist or is `null`, the processor quietly exits without modifying the document |====== [source,js] diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DotExpanderProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DotExpanderProcessor.java index 0698f6ed0a6c9..bf39b5d7bb84c 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DotExpanderProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DotExpanderProcessor.java @@ -32,11 +32,13 @@ public final class DotExpanderProcessor extends AbstractProcessor { private final String path; private final String field; + private final boolean ignoreMissing; - DotExpanderProcessor(String tag, String path, String field) { + DotExpanderProcessor(String tag, String path, String field, boolean ignoreMissing) { super(tag); this.path = path; this.field = field; + this.ignoreMissing = ignoreMissing; } @Override @@ -54,8 +56,10 @@ public IngestDocument execute(IngestDocument ingestDocument) throws Exception { if (ingestDocument.hasField(path)) { Object value = map.remove(field); - ingestDocument.appendFieldValue(path, value); - } else { + if(value != null) { + ingestDocument.appendFieldValue(path, value); + } + } else if (ignoreMissing == false){ // check whether we actually can expand the field in question into an object field. // part of the path may already exist and if part of it would be a value field (string, integer etc.) // then we can't override it with an object field and we should fail with a good reason. @@ -115,7 +119,9 @@ public Processor create(Map processorFactories, Strin } String path = ConfigurationUtils.readOptionalStringProperty(TYPE, tag, config, "path"); - return new DotExpanderProcessor(tag, path, field); + boolean ignoreMissing = ConfigurationUtils.readBooleanProperty(TYPE, tag, config, "ignore_missing", false); + + return new DotExpanderProcessor(tag, path, field, ignoreMissing); } } } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java index fde7f0c9b8a02..dd606d8c5d9e7 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java @@ -37,7 +37,7 @@ public void testEscapeFields() throws Exception { Map source = new HashMap<>(); source.put("foo.bar", "baz1"); IngestDocument document = new IngestDocument(source, Collections.emptyMap()); - DotExpanderProcessor processor = new DotExpanderProcessor("_tag", null, "foo.bar"); + DotExpanderProcessor processor = new DotExpanderProcessor("_tag", null, "foo.bar", false); processor.execute(document); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", String.class), equalTo("baz1")); @@ -45,7 +45,7 @@ public void testEscapeFields() throws Exception { source = new HashMap<>(); source.put("foo.bar.baz", "value"); document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", null, "foo.bar.baz"); + processor = new DotExpanderProcessor("_tag", null, "foo.bar.baz", false); processor.execute(document); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", Map.class).size(), equalTo(1)); @@ -55,7 +55,7 @@ public void testEscapeFields() throws Exception { source.put("foo.bar", "baz1"); source.put("foo", new HashMap<>(Collections.singletonMap("bar", "baz2"))); document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", null, "foo.bar"); + processor = new DotExpanderProcessor("_tag", null, "foo.bar", false); processor.execute(document); assertThat(document.getSourceAndMetadata().size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", List.class).size(), equalTo(2)); @@ -66,7 +66,7 @@ public void testEscapeFields() throws Exception { source.put("foo.bar", "2"); source.put("foo", new HashMap<>(Collections.singletonMap("bar", 1))); document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", null, "foo.bar"); + processor = new DotExpanderProcessor("_tag", null, "foo.bar", false); processor.execute(document); assertThat(document.getSourceAndMetadata().size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", List.class).size(), equalTo(2)); @@ -79,7 +79,7 @@ public void testEscapeFields_valueField() throws Exception { source.put("foo.bar", "baz1"); source.put("foo", "baz2"); IngestDocument document1 = new IngestDocument(source, Collections.emptyMap()); - Processor processor1 = new DotExpanderProcessor("_tag", null, "foo.bar"); + Processor processor1 = new DotExpanderProcessor("_tag", null, "foo.bar", false); // foo already exists and if a leaf field and therefor can't be replaced by a map field: Exception e = expectThrows(IllegalArgumentException.class, () -> processor1.execute(document1)); assertThat(e.getMessage(), equalTo("cannot expend [foo.bar], because [foo] is not an object field, but a value field")); @@ -90,7 +90,7 @@ public void testEscapeFields_valueField() throws Exception { Processor processor = new RenameProcessor("_tag", new TestTemplateService.MockTemplateScript.Factory("foo"), new TestTemplateService.MockTemplateScript.Factory("foo.bar"), false); processor.execute(document); - processor = new DotExpanderProcessor("_tag", null, "foo.bar"); + processor = new DotExpanderProcessor("_tag", null, "foo.bar", false); processor.execute(document); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar.0", String.class), equalTo("baz2")); @@ -99,7 +99,7 @@ public void testEscapeFields_valueField() throws Exception { source = new HashMap<>(); source.put("foo.bar", "baz1"); document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", null, "foo.bar"); + processor = new DotExpanderProcessor("_tag", null, "foo.bar", false); processor.execute(document); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", String.class), equalTo("baz1")); @@ -108,7 +108,7 @@ public void testEscapeFields_valueField() throws Exception { source.put("foo.bar.baz", "baz1"); source.put("foo", new HashMap<>(Collections.singletonMap("bar", new HashMap<>()))); document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", null, "foo.bar.baz"); + processor = new DotExpanderProcessor("_tag", null, "foo.bar.baz", false); processor.execute(document); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", Map.class).size(), equalTo(1)); @@ -118,7 +118,7 @@ public void testEscapeFields_valueField() throws Exception { source.put("foo.bar.baz", "baz1"); source.put("foo", new HashMap<>(Collections.singletonMap("bar", "baz2"))); IngestDocument document2 = new IngestDocument(source, Collections.emptyMap()); - Processor processor2 = new DotExpanderProcessor("_tag", null, "foo.bar.baz"); + Processor processor2 = new DotExpanderProcessor("_tag", null, "foo.bar.baz", false); e = expectThrows(IllegalArgumentException.class, () -> processor2.execute(document2)); assertThat(e.getMessage(), equalTo("cannot expend [foo.bar.baz], because [foo.bar] is not an object field, but a value field")); } @@ -127,7 +127,7 @@ public void testEscapeFields_path() throws Exception { Map source = new HashMap<>(); source.put("foo", new HashMap<>(Collections.singletonMap("bar.baz", "value"))); IngestDocument document = new IngestDocument(source, Collections.emptyMap()); - DotExpanderProcessor processor = new DotExpanderProcessor("_tag", "foo", "bar.baz"); + DotExpanderProcessor processor = new DotExpanderProcessor("_tag", "foo", "bar.baz", false); processor.execute(document); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", Map.class).size(), equalTo(1)); @@ -136,11 +136,44 @@ public void testEscapeFields_path() throws Exception { source = new HashMap<>(); source.put("field", new HashMap<>(Collections.singletonMap("foo.bar.baz", "value"))); document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", "field", "foo.bar.baz"); + processor = new DotExpanderProcessor("_tag", "field", "foo.bar.baz", false); processor.execute(document); assertThat(document.getFieldValue("field.foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("field.foo.bar", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("field.foo.bar.baz", String.class), equalTo("value")); } + public void testEscapeFields_ignoreMissing() throws Exception { + Map source = new HashMap<>(); + source.put("foo.bar", "baz1"); + IngestDocument document = new IngestDocument(source, Collections.emptyMap()); + DotExpanderProcessor processor = new DotExpanderProcessor("_tag", null, "abc.def", true); + processor.execute(document); + assertFalse(document.hasField("foo.bar")); + assertFalse(document.hasField("abc.def")); + + source = new HashMap<>(); + source.put("foo.bar", "baz1"); + document = new IngestDocument(source, Collections.emptyMap()); + processor = new DotExpanderProcessor("_tag", null, "abc.def", false); + processor.execute(document); + assertFalse(document.hasField("foo.bar")); + //ignore_missing = false, so null field is created + assertTrue(document.hasField("abc.def")); + assertNull(document.getFieldValue("abc.def", Object.class)); + } + + public void testEscapeFields_doNotAddNull() throws Exception { + Map source = new HashMap<>(); + Map inner = new HashMap<>(); + inner.put("bar", "baz1"); + source.put("foo", inner); + IngestDocument document = new IngestDocument(source, Collections.emptyMap()); + DotExpanderProcessor processor = new DotExpanderProcessor("_tag", null, "foo.bar", false); + processor.execute(document); + assertTrue(document.hasField("foo.bar")); + //since the exact structure already exists don't append a null to existing structure + assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); + assertThat(document.getFieldValue("foo.bar", String.class), equalTo("baz1")); + } } From a628f29bd84be14dc55be8ff4689a6a2f109e021 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 1 Nov 2018 08:10:37 -0500 Subject: [PATCH 2/4] Revert "ingest: dot_expander_processor, ignore_missing and prevent null" This reverts commit 650986dcf7471046194d1a6373fcbb00de74efe5. --- docs/reference/ingest/ingest-node.asciidoc | 7 +-- .../ingest/common/DotExpanderProcessor.java | 14 ++--- .../common/DotExpanderProcessorTests.java | 55 ++++--------------- 3 files changed, 18 insertions(+), 58 deletions(-) diff --git a/docs/reference/ingest/ingest-node.asciidoc b/docs/reference/ingest/ingest-node.asciidoc index e68bec5260109..eeb914facc2c6 100644 --- a/docs/reference/ingest/ingest-node.asciidoc +++ b/docs/reference/ingest/ingest-node.asciidoc @@ -1286,10 +1286,9 @@ Otherwise these <> can't be accessed by any .Dot Expand Options [options="header"] |====== -| Name | Required | Default | Description -| `field` | yes | - | The field to expand into an object field -| `path` | no | - | The field that contains the field to expand. Only required if the field to expand is part another object field, because the `field` option can only understand leaf fields. -| `ignore_missing` | no | `false` | If `true` and `field` does not exist or is `null`, the processor quietly exits without modifying the document +| Name | Required | Default | Description +| `field` | yes | - | The field to expand into an object field +| `path` | no | - | The field that contains the field to expand. Only required if the field to expand is part another object field, because the `field` option can only understand leaf fields. |====== [source,js] diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DotExpanderProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DotExpanderProcessor.java index bf39b5d7bb84c..0698f6ed0a6c9 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DotExpanderProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DotExpanderProcessor.java @@ -32,13 +32,11 @@ public final class DotExpanderProcessor extends AbstractProcessor { private final String path; private final String field; - private final boolean ignoreMissing; - DotExpanderProcessor(String tag, String path, String field, boolean ignoreMissing) { + DotExpanderProcessor(String tag, String path, String field) { super(tag); this.path = path; this.field = field; - this.ignoreMissing = ignoreMissing; } @Override @@ -56,10 +54,8 @@ public IngestDocument execute(IngestDocument ingestDocument) throws Exception { if (ingestDocument.hasField(path)) { Object value = map.remove(field); - if(value != null) { - ingestDocument.appendFieldValue(path, value); - } - } else if (ignoreMissing == false){ + ingestDocument.appendFieldValue(path, value); + } else { // check whether we actually can expand the field in question into an object field. // part of the path may already exist and if part of it would be a value field (string, integer etc.) // then we can't override it with an object field and we should fail with a good reason. @@ -119,9 +115,7 @@ public Processor create(Map processorFactories, Strin } String path = ConfigurationUtils.readOptionalStringProperty(TYPE, tag, config, "path"); - boolean ignoreMissing = ConfigurationUtils.readBooleanProperty(TYPE, tag, config, "ignore_missing", false); - - return new DotExpanderProcessor(tag, path, field, ignoreMissing); + return new DotExpanderProcessor(tag, path, field); } } } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java index dd606d8c5d9e7..fde7f0c9b8a02 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java @@ -37,7 +37,7 @@ public void testEscapeFields() throws Exception { Map source = new HashMap<>(); source.put("foo.bar", "baz1"); IngestDocument document = new IngestDocument(source, Collections.emptyMap()); - DotExpanderProcessor processor = new DotExpanderProcessor("_tag", null, "foo.bar", false); + DotExpanderProcessor processor = new DotExpanderProcessor("_tag", null, "foo.bar"); processor.execute(document); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", String.class), equalTo("baz1")); @@ -45,7 +45,7 @@ public void testEscapeFields() throws Exception { source = new HashMap<>(); source.put("foo.bar.baz", "value"); document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", null, "foo.bar.baz", false); + processor = new DotExpanderProcessor("_tag", null, "foo.bar.baz"); processor.execute(document); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", Map.class).size(), equalTo(1)); @@ -55,7 +55,7 @@ public void testEscapeFields() throws Exception { source.put("foo.bar", "baz1"); source.put("foo", new HashMap<>(Collections.singletonMap("bar", "baz2"))); document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", null, "foo.bar", false); + processor = new DotExpanderProcessor("_tag", null, "foo.bar"); processor.execute(document); assertThat(document.getSourceAndMetadata().size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", List.class).size(), equalTo(2)); @@ -66,7 +66,7 @@ public void testEscapeFields() throws Exception { source.put("foo.bar", "2"); source.put("foo", new HashMap<>(Collections.singletonMap("bar", 1))); document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", null, "foo.bar", false); + processor = new DotExpanderProcessor("_tag", null, "foo.bar"); processor.execute(document); assertThat(document.getSourceAndMetadata().size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", List.class).size(), equalTo(2)); @@ -79,7 +79,7 @@ public void testEscapeFields_valueField() throws Exception { source.put("foo.bar", "baz1"); source.put("foo", "baz2"); IngestDocument document1 = new IngestDocument(source, Collections.emptyMap()); - Processor processor1 = new DotExpanderProcessor("_tag", null, "foo.bar", false); + Processor processor1 = new DotExpanderProcessor("_tag", null, "foo.bar"); // foo already exists and if a leaf field and therefor can't be replaced by a map field: Exception e = expectThrows(IllegalArgumentException.class, () -> processor1.execute(document1)); assertThat(e.getMessage(), equalTo("cannot expend [foo.bar], because [foo] is not an object field, but a value field")); @@ -90,7 +90,7 @@ public void testEscapeFields_valueField() throws Exception { Processor processor = new RenameProcessor("_tag", new TestTemplateService.MockTemplateScript.Factory("foo"), new TestTemplateService.MockTemplateScript.Factory("foo.bar"), false); processor.execute(document); - processor = new DotExpanderProcessor("_tag", null, "foo.bar", false); + processor = new DotExpanderProcessor("_tag", null, "foo.bar"); processor.execute(document); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar.0", String.class), equalTo("baz2")); @@ -99,7 +99,7 @@ public void testEscapeFields_valueField() throws Exception { source = new HashMap<>(); source.put("foo.bar", "baz1"); document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", null, "foo.bar", false); + processor = new DotExpanderProcessor("_tag", null, "foo.bar"); processor.execute(document); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", String.class), equalTo("baz1")); @@ -108,7 +108,7 @@ public void testEscapeFields_valueField() throws Exception { source.put("foo.bar.baz", "baz1"); source.put("foo", new HashMap<>(Collections.singletonMap("bar", new HashMap<>()))); document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", null, "foo.bar.baz", false); + processor = new DotExpanderProcessor("_tag", null, "foo.bar.baz"); processor.execute(document); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", Map.class).size(), equalTo(1)); @@ -118,7 +118,7 @@ public void testEscapeFields_valueField() throws Exception { source.put("foo.bar.baz", "baz1"); source.put("foo", new HashMap<>(Collections.singletonMap("bar", "baz2"))); IngestDocument document2 = new IngestDocument(source, Collections.emptyMap()); - Processor processor2 = new DotExpanderProcessor("_tag", null, "foo.bar.baz", false); + Processor processor2 = new DotExpanderProcessor("_tag", null, "foo.bar.baz"); e = expectThrows(IllegalArgumentException.class, () -> processor2.execute(document2)); assertThat(e.getMessage(), equalTo("cannot expend [foo.bar.baz], because [foo.bar] is not an object field, but a value field")); } @@ -127,7 +127,7 @@ public void testEscapeFields_path() throws Exception { Map source = new HashMap<>(); source.put("foo", new HashMap<>(Collections.singletonMap("bar.baz", "value"))); IngestDocument document = new IngestDocument(source, Collections.emptyMap()); - DotExpanderProcessor processor = new DotExpanderProcessor("_tag", "foo", "bar.baz", false); + DotExpanderProcessor processor = new DotExpanderProcessor("_tag", "foo", "bar.baz"); processor.execute(document); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", Map.class).size(), equalTo(1)); @@ -136,44 +136,11 @@ public void testEscapeFields_path() throws Exception { source = new HashMap<>(); source.put("field", new HashMap<>(Collections.singletonMap("foo.bar.baz", "value"))); document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", "field", "foo.bar.baz", false); + processor = new DotExpanderProcessor("_tag", "field", "foo.bar.baz"); processor.execute(document); assertThat(document.getFieldValue("field.foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("field.foo.bar", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("field.foo.bar.baz", String.class), equalTo("value")); } - public void testEscapeFields_ignoreMissing() throws Exception { - Map source = new HashMap<>(); - source.put("foo.bar", "baz1"); - IngestDocument document = new IngestDocument(source, Collections.emptyMap()); - DotExpanderProcessor processor = new DotExpanderProcessor("_tag", null, "abc.def", true); - processor.execute(document); - assertFalse(document.hasField("foo.bar")); - assertFalse(document.hasField("abc.def")); - - source = new HashMap<>(); - source.put("foo.bar", "baz1"); - document = new IngestDocument(source, Collections.emptyMap()); - processor = new DotExpanderProcessor("_tag", null, "abc.def", false); - processor.execute(document); - assertFalse(document.hasField("foo.bar")); - //ignore_missing = false, so null field is created - assertTrue(document.hasField("abc.def")); - assertNull(document.getFieldValue("abc.def", Object.class)); - } - - public void testEscapeFields_doNotAddNull() throws Exception { - Map source = new HashMap<>(); - Map inner = new HashMap<>(); - inner.put("bar", "baz1"); - source.put("foo", inner); - IngestDocument document = new IngestDocument(source, Collections.emptyMap()); - DotExpanderProcessor processor = new DotExpanderProcessor("_tag", null, "foo.bar", false); - processor.execute(document); - assertTrue(document.hasField("foo.bar")); - //since the exact structure already exists don't append a null to existing structure - assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); - assertThat(document.getFieldValue("foo.bar", String.class), equalTo("baz1")); - } } From a3980e6f962177516ebddc4465d8c31a6d19ae2d Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Thu, 1 Nov 2018 09:19:31 -0500 Subject: [PATCH 3/4] remove ignore_missing in favor of fixing null insertion bug --- .../ingest/common/DotExpanderProcessor.java | 38 ++++++++++--------- .../common/DotExpanderProcessorTests.java | 37 ++++++++++++++++++ 2 files changed, 57 insertions(+), 18 deletions(-) diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DotExpanderProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DotExpanderProcessor.java index 0698f6ed0a6c9..f4215af8390dd 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DotExpanderProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DotExpanderProcessor.java @@ -52,28 +52,30 @@ public IngestDocument execute(IngestDocument ingestDocument) throws Exception { map = ingestDocument.getSourceAndMetadata(); } - if (ingestDocument.hasField(path)) { - Object value = map.remove(field); - ingestDocument.appendFieldValue(path, value); - } else { - // check whether we actually can expand the field in question into an object field. - // part of the path may already exist and if part of it would be a value field (string, integer etc.) - // then we can't override it with an object field and we should fail with a good reason. - // IngestDocument#setFieldValue(...) would fail too, but the error isn't very understandable - for (int index = path.indexOf('.'); index != -1; index = path.indexOf('.', index + 1)) { - String partialPath = path.substring(0, index); - if (ingestDocument.hasField(partialPath)) { - Object val = ingestDocument.getFieldValue(partialPath, Object.class); - if ((val instanceof Map) == false) { - throw new IllegalArgumentException("cannot expend [" + path + "], because [" + partialPath + + if (map.containsKey(field)) { + if (ingestDocument.hasField(path)) { + Object value = map.remove(field); + ingestDocument.appendFieldValue(path, value); + } else { + // check whether we actually can expand the field in question into an object field. + // part of the path may already exist and if part of it would be a value field (string, integer etc.) + // then we can't override it with an object field and we should fail with a good reason. + // IngestDocument#setFieldValue(...) would fail too, but the error isn't very understandable + for (int index = path.indexOf('.'); index != -1; index = path.indexOf('.', index + 1)) { + String partialPath = path.substring(0, index); + if (ingestDocument.hasField(partialPath)) { + Object val = ingestDocument.getFieldValue(partialPath, Object.class); + if ((val instanceof Map) == false) { + throw new IllegalArgumentException("cannot expend [" + path + "], because [" + partialPath + "] is not an object field, but a value field"); + } + } else { + break; } - } else { - break; } + Object value = map.remove(field); + ingestDocument.setFieldValue(path, value); } - Object value = map.remove(field); - ingestDocument.setFieldValue(path, value); } return ingestDocument; } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java index fde7f0c9b8a02..91e309ba2b50e 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java @@ -143,4 +143,41 @@ public void testEscapeFields_path() throws Exception { assertThat(document.getFieldValue("field.foo.bar.baz", String.class), equalTo("value")); } + + public void testEscapeFields_doNotingIfFieldNotInSourceDoc() throws Exception { + //asking to expand a (literal) field that is not present in the source document + Map source = new HashMap<>(); + source.put("foo.bar", "baz1"); + IngestDocument document = new IngestDocument(source, Collections.emptyMap()); + //abc.def does not exist in source, so don't mutate document + DotExpanderProcessor processor = new DotExpanderProcessor("_tag", null, "abc.def"); + processor.execute(document); + //hasField returns false since it requires the expanded form, which is not expanded since we did not ask for it to be + assertFalse(document.hasField("foo.bar")); + //nothing has changed + assertEquals(document.getSourceAndMetadata().get("foo.bar"), source.get("foo.bar")); + assertEquals(document.getSourceAndMetadata(), source); + //abc.def is not found anywhere + assertFalse(document.hasField("abc.def")); + assertFalse(document.getSourceAndMetadata().containsKey("abc")); + assertFalse(document.getSourceAndMetadata().containsKey("abc.def")); + + //asking to expand a (literal) field that does not exist, but the nested field does exist + source = new HashMap<>(); + Map inner = new HashMap<>(); + inner.put("bar", "baz1"); + source.put("foo", inner); + document = new IngestDocument(source, Collections.emptyMap()); + //foo.bar, the literal value (as opposed to nested value) does not exist in source, so don't mutate document + processor = new DotExpanderProcessor("_tag", null, "foo.bar"); + processor.execute(document); + //hasField returns true because the nested/expanded form exists in the source document + assertTrue(document.hasField("foo.bar")); + //nothing changed + assertEquals(document.getSourceAndMetadata().get("foo.bar"), source.get("foo.bar")); + assertEquals(document.getSourceAndMetadata(), source); + assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); + assertThat(document.getFieldValue("foo.bar", String.class), equalTo("baz1")); + } + } From 6fdf05da725957dc79deae807ea98d5973853a49 Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Mon, 5 Nov 2018 13:26:27 -0600 Subject: [PATCH 4/4] fix typo, remove misleading assertions --- .../ingest/common/DotExpanderProcessorTests.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java index 91e309ba2b50e..d6a207b859eb0 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DotExpanderProcessorTests.java @@ -144,7 +144,7 @@ public void testEscapeFields_path() throws Exception { } - public void testEscapeFields_doNotingIfFieldNotInSourceDoc() throws Exception { + public void testEscapeFields_doNothingIfFieldNotInSourceDoc() throws Exception { //asking to expand a (literal) field that is not present in the source document Map source = new HashMap<>(); source.put("foo.bar", "baz1"); @@ -155,8 +155,7 @@ public void testEscapeFields_doNotingIfFieldNotInSourceDoc() throws Exception { //hasField returns false since it requires the expanded form, which is not expanded since we did not ask for it to be assertFalse(document.hasField("foo.bar")); //nothing has changed - assertEquals(document.getSourceAndMetadata().get("foo.bar"), source.get("foo.bar")); - assertEquals(document.getSourceAndMetadata(), source); + assertEquals(document.getSourceAndMetadata().get("foo.bar"), "baz1"); //abc.def is not found anywhere assertFalse(document.hasField("abc.def")); assertFalse(document.getSourceAndMetadata().containsKey("abc")); @@ -174,8 +173,6 @@ public void testEscapeFields_doNotingIfFieldNotInSourceDoc() throws Exception { //hasField returns true because the nested/expanded form exists in the source document assertTrue(document.hasField("foo.bar")); //nothing changed - assertEquals(document.getSourceAndMetadata().get("foo.bar"), source.get("foo.bar")); - assertEquals(document.getSourceAndMetadata(), source); assertThat(document.getFieldValue("foo", Map.class).size(), equalTo(1)); assertThat(document.getFieldValue("foo.bar", String.class), equalTo("baz1")); }