From ab5b6491840eddbe8c19e43d3440682280ecf55e Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 18 Nov 2015 10:40:16 +0100 Subject: [PATCH] accept null values and adapt hasPropertyValue return value --- .../java/org/elasticsearch/ingest/Data.java | 60 +++++++++++-------- .../org/elasticsearch/ingest/DataTests.java | 24 +++++--- 2 files changed, 51 insertions(+), 33 deletions(-) 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 ab9ab077ec9..c64b8865607 100644 --- a/plugins/ingest/src/main/java/org/elasticsearch/ingest/Data.java +++ b/plugins/ingest/src/main/java/org/elasticsearch/ingest/Data.java @@ -56,26 +56,6 @@ public final class Data { * @throws IllegalArgumentException if the field is present but is not of the type provided as argument. */ public T getPropertyValue(String path, Class clazz) { - Object property = get(path); - if (property == null) { - return null; - } - if (clazz.isInstance(property)) { - return clazz.cast(property); - } - throw new IllegalArgumentException("field [" + path + "] of type [" + property.getClass().getName() + "] cannot be cast to [" + clazz.getName() + "]"); - } - - /** - * Checks whether the document contains a value for the provided path - * @param path The path within the document in dot-notation - * @return true if the document contains a non null value for the property, false otherwise - */ - public boolean hasPropertyValue(String path) { - return get(path) != null; - } - - private Object get(String path) { if (path == null || path.length() == 0) { return null; } @@ -95,7 +75,42 @@ public final class Data { } String leafKey = pathElements[pathElements.length - 1]; - return innerMap.get(leafKey); + Object property = innerMap.get(leafKey); + if (property == null) { + return null; + } + if (clazz.isInstance(property)) { + return clazz.cast(property); + } + throw new IllegalArgumentException("field [" + path + "] of type [" + property.getClass().getName() + "] cannot be cast to [" + clazz.getName() + "]"); + } + + /** + * Checks whether the document contains a value for the provided path + * @param path The path within the document in dot-notation + * @return true if the document contains a value for the property, false otherwise + */ + public boolean hasPropertyValue(String path) { + if (path == null || path.length() == 0) { + return false; + } + String[] pathElements = Strings.splitStringToArray(path, '.'); + assert pathElements.length > 0; + + Map innerMap = document; + for (int i = 0; i < pathElements.length - 1; i++) { + Object obj = innerMap.get(pathElements[i]); + if (obj instanceof Map) { + @SuppressWarnings("unchecked") + Map stringObjectMap = (Map) obj; + innerMap = stringObjectMap; + } else { + return false; + } + } + + String leafKey = pathElements[pathElements.length - 1]; + return innerMap.containsKey(leafKey); } /** @@ -108,9 +123,6 @@ public final class Data { if (path == null || path.length() == 0) { throw new IllegalArgumentException("cannot add null or empty field"); } - if (value == null) { - throw new IllegalArgumentException("cannot add null value to field [" + path + "]"); - } modified = true; String[] pathElements = Strings.splitStringToArray(path, '.'); assert pathElements.length > 0; 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 4c6f53d880e..cf230d1c299 100644 --- a/plugins/ingest/src/test/java/org/elasticsearch/ingest/DataTests.java +++ b/plugins/ingest/src/test/java/org/elasticsearch/ingest/DataTests.java @@ -36,6 +36,7 @@ public class DataTests extends ESTestCase { public void setData() { Map document = new HashMap<>(); document.put("foo", "bar"); + document.put("foo_null", null); document.put("int", 123); Map innerObject = new HashMap<>(); innerObject.put("buzz", "hello world"); @@ -48,6 +49,10 @@ public class DataTests extends ESTestCase { assertThat(data.getPropertyValue("int", Integer.class), equalTo(123)); } + public void testGetPropertyValueNullValue() { + assertThat(data.getPropertyValue("foo_null", Object.class), nullValue()); + } + public void testSimpleGetPropertyValueTypeMismatch() { try { data.getPropertyValue("int", String.class); @@ -100,6 +105,10 @@ public class DataTests extends ESTestCase { assertFalse(data.hasPropertyValue(null)); } + public void testHasPropertyValueNullValue() { + assertTrue(data.hasPropertyValue("foo_null")); + } + public void testHasPropertyValueEmpty() { assertFalse(data.hasPropertyValue("")); } @@ -109,6 +118,12 @@ public class DataTests extends ESTestCase { assertThat(data.getDocument().get("new_field"), equalTo("foo")); } + public void testSetPropertyValueNullValue() { + data.setPropertyValue("new_field", null); + assertThat(data.getDocument().containsKey("new_field"), equalTo(true)); + assertThat(data.getDocument().get("new_field"), nullValue()); + } + @SuppressWarnings("unchecked") public void testNestedSetPropertyValue() { data.setPropertyValue("a.b.c.d", "foo"); @@ -165,15 +180,6 @@ public class DataTests extends ESTestCase { } } - public void testSetPropertyValueNullValue() { - try { - data.setPropertyValue("new_field", null); - fail("add field should have failed"); - } catch(IllegalArgumentException e) { - assertThat(e.getMessage(), equalTo("cannot add null value to field [new_field]")); - } - } - public void testEqualsAndHashcode() throws Exception { String index = randomAsciiOfLengthBetween(1, 10); String type = randomAsciiOfLengthBetween(1, 10);