From ba7e536e1dcb8fced4dbef2a2ce40118528b16f6 Mon Sep 17 00:00:00 2001
From: javanna <cavannaluca@gmail.com>
Date: Tue, 17 Nov 2015 15:45:34 +0100
Subject: [PATCH] better error and tests for empty and null values in Data
 containsProperty addField and getProperty

---
 .../java/org/elasticsearch/ingest/Data.java   | 15 +++++--
 .../org/elasticsearch/ingest/DataTests.java   | 44 +++++++++++++++++++
 2 files changed, 56 insertions(+), 3 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 2f9132f83c0..1c347be4d62 100644
--- a/plugins/ingest/src/main/java/org/elasticsearch/ingest/Data.java
+++ b/plugins/ingest/src/main/java/org/elasticsearch/ingest/Data.java
@@ -55,6 +55,10 @@ public final class Data {
     }
 
     public boolean containsProperty(String path) {
+        if (path == null || path.length() == 0) {
+            return false;
+        }
+
         boolean containsProperty = false;
         String[] pathElements = Strings.splitStringToArray(path, '.');
         if (pathElements.length == 0) {
@@ -86,17 +90,22 @@ public final class Data {
 
     /**
      * add `value` to path in document. If path does not exist,
-     * nested hashmaps will be put in as parent key values until
+     * nested maps will be put in as parent key values until
      * leaf key name in path is reached.
      *
      * @param path The path within the document in dot-notation
      * @param value The value to put in for the path key
      */
     public void addField(String path, Object value) {
+        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;
         String writeKey = pathElements[pathElements.length - 1];
         Map<String, Object> inner = 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 2b3d7b4c6f9..511d465495c 100644
--- a/plugins/ingest/src/test/java/org/elasticsearch/ingest/DataTests.java
+++ b/plugins/ingest/src/test/java/org/elasticsearch/ingest/DataTests.java
@@ -54,6 +54,14 @@ public class DataTests extends ESTestCase {
         assertThat(data.getProperty("not.here"), nullValue());
     }
 
+    public void testGetPropertyNull() {
+        assertNull(data.getProperty(null));
+    }
+
+    public void testGetPropertyEmpty() {
+        assertNull(data.getProperty(""));
+    }
+
     public void testContainsProperty() {
         assertTrue(data.containsProperty("fizz"));
     }
@@ -70,6 +78,14 @@ public class DataTests extends ESTestCase {
         assertFalse(data.containsProperty("fizz.doesnotexist"));
     }
 
+    public void testContainsPropertyNull() {
+        assertFalse(data.containsProperty(null));
+    }
+
+    public void testContainsPropertyEmpty() {
+        assertFalse(data.containsProperty(""));
+    }
+
     public void testSimpleAddField() {
         data.addField("new_field", "foo");
         assertThat(data.getDocument().get("new_field"), equalTo("foo"));
@@ -107,11 +123,39 @@ public class DataTests extends ESTestCase {
     public void testAddFieldOnExistingParentTypeMismatch() {
         try {
             data.addField("fizz.buzz.new", "bar");
+            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."));
         }
     }
 
+    public void testAddFieldNullName() {
+        try {
+            data.addField(null, "bar");
+            fail("add field should have failed");
+        } catch(IllegalArgumentException e) {
+            assertThat(e.getMessage(), equalTo("cannot add null or empty field"));
+        }
+    }
+
+    public void testAddFieldEmptyName() {
+        try {
+            data.addField("", "bar");
+            fail("add field should have failed");
+        } catch(IllegalArgumentException e) {
+            assertThat(e.getMessage(), equalTo("cannot add null or empty field"));
+        }
+    }
+
+    public void testAddFieldNullValue() {
+        try {
+            data.addField("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);