diff --git a/plugins/ingest/src/main/java/org/elasticsearch/ingest/Data.java b/plugins/ingest/src/main/java/org/elasticsearch/ingest/Data.java index cafa8133877..3c3a25f84af 100644 --- a/plugins/ingest/src/main/java/org/elasticsearch/ingest/Data.java +++ b/plugins/ingest/src/main/java/org/elasticsearch/ingest/Data.java @@ -108,7 +108,10 @@ public final class Data { Map parent = getParent(pathElements); if (parent != null) { String leafKey = pathElements[pathElements.length - 1]; - parent.remove(leafKey); + if (parent.containsKey(leafKey)) { + modified = true; + parent.remove(leafKey); + } } } @@ -137,7 +140,6 @@ public final class Data { if (path == null || path.length() == 0) { throw new IllegalArgumentException("cannot add null or empty field"); } - modified = true; String[] pathElements = Strings.splitStringToArray(path, '.'); assert pathElements.length > 0; @@ -164,6 +166,7 @@ public final class Data { String leafKey = pathElements[pathElements.length - 1]; inner.put(leafKey, value); + modified = true; } public String getIndex() { @@ -178,6 +181,11 @@ public final class Data { return id; } + /** + * Returns the document. Should be used only for reading. Any change made to the returned map will + * not be reflected to the modified flag. Modify the document instead using {@link #setPropertyValue(String, Object)} + * and {@link #removeProperty(String)} + */ public Map getDocument() { return document; } diff --git a/plugins/ingest/src/test/java/org/elasticsearch/ingest/DataTests.java b/plugins/ingest/src/test/java/org/elasticsearch/ingest/DataTests.java index 5179a468b58..64cb6489e4f 100644 --- a/plugins/ingest/src/test/java/org/elasticsearch/ingest/DataTests.java +++ b/plugins/ingest/src/test/java/org/elasticsearch/ingest/DataTests.java @@ -116,12 +116,14 @@ public class DataTests extends ESTestCase { public void testSimpleSetPropertyValue() { data.setPropertyValue("new_field", "foo"); assertThat(data.getDocument().get("new_field"), equalTo("foo")); + assertThat(data.isModified(), equalTo(true)); } public void testSetPropertyValueNullValue() { data.setPropertyValue("new_field", null); assertThat(data.getDocument().containsKey("new_field"), equalTo(true)); assertThat(data.getDocument().get("new_field"), nullValue()); + assertThat(data.isModified(), equalTo(true)); } @SuppressWarnings("unchecked") @@ -136,6 +138,7 @@ public class DataTests extends ESTestCase { assertThat(c.get("d"), instanceOf(String.class)); String d = (String) c.get("d"); assertThat(d, equalTo("foo")); + assertThat(data.isModified(), equalTo(true)); } public void testSetPropertyValueOnExistingField() { @@ -151,6 +154,7 @@ public class DataTests extends ESTestCase { assertThat(innerMap.get("new"), instanceOf(String.class)); String value = (String) innerMap.get("new"); assertThat(value, equalTo("bar")); + assertThat(data.isModified(), equalTo(true)); } public void testSetPropertyValueOnExistingParentTypeMismatch() { @@ -159,6 +163,7 @@ public class DataTests extends ESTestCase { fail("add field should have failed"); } catch(IllegalArgumentException e) { assertThat(e.getMessage(), equalTo("cannot add field to parent [buzz] of type [java.lang.String], [java.util.Map] expected instead.")); + assertThat(data.isModified(), equalTo(false)); } } @@ -168,6 +173,7 @@ public class DataTests extends ESTestCase { fail("add field should have failed"); } catch(IllegalArgumentException e) { assertThat(e.getMessage(), equalTo("cannot add field to null parent, [java.util.Map] expected instead.")); + assertThat(data.isModified(), equalTo(false)); } } @@ -177,6 +183,7 @@ public class DataTests extends ESTestCase { fail("add field should have failed"); } catch(IllegalArgumentException e) { assertThat(e.getMessage(), equalTo("cannot add null or empty field")); + assertThat(data.isModified(), equalTo(false)); } } @@ -186,11 +193,13 @@ public class DataTests extends ESTestCase { fail("add field should have failed"); } catch(IllegalArgumentException e) { assertThat(e.getMessage(), equalTo("cannot add null or empty field")); + assertThat(data.isModified(), equalTo(false)); } } public void testRemoveProperty() { data.removeProperty("foo"); + assertThat(data.isModified(), equalTo(true)); assertThat(data.getDocument().size(), equalTo(2)); assertThat(data.getDocument().containsKey("foo"), equalTo(false)); } @@ -208,25 +217,30 @@ public class DataTests extends ESTestCase { assertThat(map.size(), equalTo(0)); assertThat(data.getDocument().size(), equalTo(3)); assertThat(data.getDocument().containsKey("fizz"), equalTo(true)); + assertThat(data.isModified(), equalTo(true)); } public void testRemoveNonExistingProperty() { data.removeProperty("does_not_exist"); + assertThat(data.isModified(), equalTo(false)); assertThat(data.getDocument().size(), equalTo(3)); } public void testRemoveExistingParentTypeMismatch() { data.removeProperty("foo.test"); + assertThat(data.isModified(), equalTo(false)); assertThat(data.getDocument().size(), equalTo(3)); } public void testRemoveNullProperty() { data.removeProperty(null); + assertThat(data.isModified(), equalTo(false)); assertThat(data.getDocument().size(), equalTo(3)); } public void testRemoveEmptyProperty() { data.removeProperty(""); + assertThat(data.isModified(), equalTo(false)); assertThat(data.getDocument().size(), equalTo(3)); }