From de33f5a911ba154c32a780004f7ad7efa0f50330 Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Fri, 13 Nov 2015 15:14:10 -0800 Subject: [PATCH] adds tests and guards against null values in some mutate methods --- .../processor/mutate/MutateProcessor.java | 16 +++++-- .../org/elasticsearch/ingest/DataTests.java | 6 +++ .../mutate/MutateProcessorTests.java | 45 +++++++++++++++++++ .../test/ingest/80_simulate.yaml | 2 +- 4 files changed, 64 insertions(+), 5 deletions(-) diff --git a/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/mutate/MutateProcessor.java b/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/mutate/MutateProcessor.java index 9b9d98e3fef..4b4e7a193ce 100644 --- a/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/mutate/MutateProcessor.java +++ b/plugins/ingest/src/main/java/org/elasticsearch/ingest/processor/mutate/MutateProcessor.java @@ -201,7 +201,9 @@ public final class MutateProcessor implements Processor { private void doSplit(Data data) { for(Map.Entry entry : split.entrySet()) { Object oldVal = data.getProperty(entry.getKey()); - if (oldVal instanceof String) { + if (oldVal == null) { + throw new IllegalArgumentException("Cannot split field. [" + entry.getKey() + "] is null."); + } else if (oldVal instanceof String) { data.addField(entry.getKey(), Arrays.asList(((String) oldVal).split(entry.getValue()))); } else { throw new IllegalArgumentException("Cannot split a field that is not a String type"); @@ -247,7 +249,9 @@ public final class MutateProcessor implements Processor { private void doTrim(Data data) { for(String field : trim) { Object val = data.getProperty(field); - if (val instanceof String) { + if (val == null) { + throw new IllegalArgumentException("Cannot trim field. [" + field + "] is null."); + } else if (val instanceof String) { data.addField(field, ((String) val).trim()); } else { throw new IllegalArgumentException("Cannot trim field:" + field + " with type: " + val.getClass()); @@ -258,7 +262,9 @@ public final class MutateProcessor implements Processor { private void doUppercase(Data data) { for(String field : uppercase) { Object val = data.getProperty(field); - if (val instanceof String) { + if (val == null) { + throw new IllegalArgumentException("Cannot uppercase field. [" + field + "] is null."); + } else if (val instanceof String) { data.addField(field, ((String) val).toUpperCase(Locale.ROOT)); } else { throw new IllegalArgumentException("Cannot uppercase field:" + field + " with type: " + val.getClass()); @@ -269,7 +275,9 @@ public final class MutateProcessor implements Processor { private void doLowercase(Data data) { for(String field : lowercase) { Object val = data.getProperty(field); - if (val instanceof String) { + if (val == null) { + throw new IllegalArgumentException("Cannot lowercase field. [" + field + "] is null."); + } else if (val instanceof String) { data.addField(field, ((String) val).toLowerCase(Locale.ROOT)); } else { throw new IllegalArgumentException("Cannot lowercase field:" + field + " with type: " + val.getClass()); 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 d17a354b139..754c70d6525 100644 --- a/plugins/ingest/src/test/java/org/elasticsearch/ingest/DataTests.java +++ b/plugins/ingest/src/test/java/org/elasticsearch/ingest/DataTests.java @@ -28,6 +28,7 @@ import java.util.Map; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; public class DataTests extends ESTestCase { @@ -51,6 +52,11 @@ public class DataTests extends ESTestCase { assertThat(data.getProperty("fizz.buzz"), equalTo("hello world")); } + public void testGetPropertyNotFound() { + data.getProperty("not.here"); + assertThat(data.getProperty("not.here"), nullValue()); + } + public void testContainsProperty() { assertTrue(data.containsProperty("fizz")); } diff --git a/plugins/ingest/src/test/java/org/elasticsearch/ingest/processor/mutate/MutateProcessorTests.java b/plugins/ingest/src/test/java/org/elasticsearch/ingest/processor/mutate/MutateProcessorTests.java index f5420eb30ee..371b4621485 100644 --- a/plugins/ingest/src/test/java/org/elasticsearch/ingest/processor/mutate/MutateProcessorTests.java +++ b/plugins/ingest/src/test/java/org/elasticsearch/ingest/processor/mutate/MutateProcessorTests.java @@ -110,6 +110,18 @@ public class MutateProcessorTests extends ESTestCase { assertThat(data.getProperty("ip"), equalTo(Arrays.asList("127", "0", "0", "1"))); } + public void testSplitNullValue() throws IOException { + Map split = new HashMap<>(); + split.put("not.found", "\\."); + Processor processor = new MutateProcessor(null, null, null, split, null, null, null, null, null, null); + try { + processor.execute(data); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), equalTo("Cannot split field. [not.found] is null.")); + } + } + public void testGsub() throws IOException { List gsubExpressions = Collections.singletonList(new GsubExpression("ip", Pattern.compile("\\."), "-")); Processor processor = new MutateProcessor(null, null, null, null, gsubExpressions, null, null, null, null, null); @@ -156,6 +168,17 @@ public class MutateProcessorTests extends ESTestCase { assertThat(data.getProperty("to_strip"), equalTo("clean")); } + public void testTrimNullValue() throws IOException { + List trim = Collections.singletonList("not.found"); + Processor processor = new MutateProcessor(null, null, null, null, null, null, null, trim, null, null); + try { + processor.execute(data); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), equalTo("Cannot trim field. [not.found] is null.")); + } + } + public void testUppercase() throws IOException { List uppercase = Collections.singletonList("foo"); Processor processor = new MutateProcessor(null, null, null, null, null, null, null, null, uppercase, null); @@ -164,6 +187,17 @@ public class MutateProcessorTests extends ESTestCase { assertThat(data.getProperty("foo"), equalTo("BAR")); } + public void testUppercaseNullValue() throws IOException { + List uppercase = Collections.singletonList("not.found"); + Processor processor = new MutateProcessor(null, null, null, null, null, null, null, null, uppercase, null); + try { + processor.execute(data); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), equalTo("Cannot uppercase field. [not.found] is null.")); + } + } + public void testLowercase() throws IOException { List lowercase = Collections.singletonList("alpha"); Processor processor = new MutateProcessor(null, null, null, null, null, null, null, null, null, lowercase); @@ -171,4 +205,15 @@ public class MutateProcessorTests extends ESTestCase { assertThat(data.getDocument().size(), equalTo(7)); assertThat(data.getProperty("alpha"), equalTo("abcd")); } + + public void testLowercaseNullValue() throws IOException { + List lowercase = Collections.singletonList("not.found"); + Processor processor = new MutateProcessor(null, null, null, null, null, null, null, null, null, lowercase); + try { + processor.execute(data); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage(), equalTo("Cannot lowercase field. [not.found] is null.")); + } + } } diff --git a/plugins/ingest/src/test/resources/rest-api-spec/test/ingest/80_simulate.yaml b/plugins/ingest/src/test/resources/rest-api-spec/test/ingest/80_simulate.yaml index 371aeedd3ef..c27d1438030 100644 --- a/plugins/ingest/src/test/resources/rest-api-spec/test/ingest/80_simulate.yaml +++ b/plugins/ingest/src/test/resources/rest-api-spec/test/ingest/80_simulate.yaml @@ -207,7 +207,7 @@ ] } - length: { docs: 2 } - - match: { docs.0.error.type: "null_pointer_exception" } + - match: { docs.0.error.type: "illegal_argument_exception" } - is_true: docs.1.doc.modified - match: { docs.1.doc._source.foo: "BAR" }