From 6302fb65a37914527478f58e8a6b437c501ec50f Mon Sep 17 00:00:00 2001 From: Tal Levy Date: Wed, 20 Apr 2016 13:36:17 -0700 Subject: [PATCH] add ability to disable ability to override values of existing fields in set processor --- .../ingest/core/ConfigurationUtils.java | 22 ++++++++++ .../ingest/core/IngestDocument.java | 22 ++++++++++ .../ingest/processor/SetProcessor.java | 19 ++++++-- .../ingest/core/ConfigurationUtilsTests.java | 21 ++++++++- .../ingest/core/IngestDocumentTests.java | 4 +- .../processor/SetProcessorFactoryTests.java | 17 +++++++- .../ingest/processor/SetProcessorTests.java | 43 ++++++++++++++++--- docs/reference/ingest/ingest-node.asciidoc | 1 + 8 files changed, 136 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/ingest/core/ConfigurationUtils.java b/core/src/main/java/org/elasticsearch/ingest/core/ConfigurationUtils.java index fc2c99626f8..685f6f41de3 100644 --- a/core/src/main/java/org/elasticsearch/ingest/core/ConfigurationUtils.java +++ b/core/src/main/java/org/elasticsearch/ingest/core/ConfigurationUtils.java @@ -83,6 +83,28 @@ public final class ConfigurationUtils { value.getClass().getName() + "]"); } + public static Boolean readBooleanProperty(String processorType, String processorTag, Map configuration, + String propertyName, boolean defaultValue) { + Object value = configuration.remove(propertyName); + if (value == null) { + return defaultValue; + } else { + return readBoolean(processorType, processorTag, propertyName, value).booleanValue(); + } + } + + private static Boolean readBoolean(String processorType, String processorTag, String propertyName, Object value) { + if (value == null) { + return null; + } + if (value instanceof Boolean) { + return (Boolean) value; + } + throw newConfigurationException(processorType, processorTag, propertyName, "property isn't a boolean, but of type [" + + value.getClass().getName() + "]"); + } + + /** * Returns and removes the specified property from the specified configuration map. * diff --git a/core/src/main/java/org/elasticsearch/ingest/core/IngestDocument.java b/core/src/main/java/org/elasticsearch/ingest/core/IngestDocument.java index d7ee168e995..7b7199b657a 100644 --- a/core/src/main/java/org/elasticsearch/ingest/core/IngestDocument.java +++ b/core/src/main/java/org/elasticsearch/ingest/core/IngestDocument.java @@ -116,6 +116,18 @@ public final class IngestDocument { return cast(path, context, clazz); } + /** + * Returns the value contained in the document with the provided templated path + * @param pathTemplate The path within the document in dot-notation + * @param clazz The expected class fo the field value + * @return the value fro the provided path if existing, null otherwise + * @throws IllegalArgumentException if the pathTemplate is null, empty, invalid, if the field doesn't exist, + * or if the field that is found at the provided path is not of the expected type. + */ + public T getFieldValue(TemplateService.Template pathTemplate, Class clazz) { + return getFieldValue(renderTemplate(pathTemplate), clazz); + } + /** * Returns the value contained in the document for the provided path as a byte array. * If the path value is a string, a base64 decode operation will happen. @@ -141,6 +153,16 @@ public final class IngestDocument { } } + /** + * Checks whether the document contains a value for the provided templated path + * @param fieldPathTemplate the template for the path within the document in dot-notation + * @return true if the document contains a value for the field, false otherwise + * @throws IllegalArgumentException if the path is null, empty or invalid + */ + public boolean hasField(TemplateService.Template fieldPathTemplate) { + return hasField(renderTemplate(fieldPathTemplate)); + } + /** * Checks whether the document contains a value for the provided path * @param path The path within the document in dot-notation diff --git a/core/src/main/java/org/elasticsearch/ingest/processor/SetProcessor.java b/core/src/main/java/org/elasticsearch/ingest/processor/SetProcessor.java index 67e2414f29f..936fe62afd4 100644 --- a/core/src/main/java/org/elasticsearch/ingest/processor/SetProcessor.java +++ b/core/src/main/java/org/elasticsearch/ingest/processor/SetProcessor.java @@ -36,15 +36,25 @@ public final class SetProcessor extends AbstractProcessor { public static final String TYPE = "set"; + private final boolean overrideEnabled; private final TemplateService.Template field; private final ValueSource value; - SetProcessor(String tag, TemplateService.Template field, ValueSource value) { + SetProcessor(String tag, TemplateService.Template field, ValueSource value) { + this(tag, field, value, true); + } + + SetProcessor(String tag, TemplateService.Template field, ValueSource value, boolean overrideEnabled) { super(tag); + this.overrideEnabled = overrideEnabled; this.field = field; this.value = value; } + public boolean isOverrideEnabled() { + return overrideEnabled; + } + public TemplateService.Template getField() { return field; } @@ -55,7 +65,9 @@ public final class SetProcessor extends AbstractProcessor { @Override public void execute(IngestDocument document) { - document.setFieldValue(field, value); + if (overrideEnabled || document.hasField(field) == false || document.getFieldValue(field, Object.class) == null) { + document.setFieldValue(field, value); + } } @Override @@ -75,7 +87,8 @@ public final class SetProcessor extends AbstractProcessor { public SetProcessor doCreate(String processorTag, Map config) throws Exception { String field = ConfigurationUtils.readStringProperty(TYPE, processorTag, config, "field"); Object value = ConfigurationUtils.readObject(TYPE, processorTag, config, "value"); - return new SetProcessor(processorTag, templateService.compile(field), ValueSource.wrap(value, templateService)); + boolean overrideEnabled = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "override", true); + return new SetProcessor(processorTag, templateService.compile(field), ValueSource.wrap(value, templateService), overrideEnabled); } } } diff --git a/core/src/test/java/org/elasticsearch/ingest/core/ConfigurationUtilsTests.java b/core/src/test/java/org/elasticsearch/ingest/core/ConfigurationUtilsTests.java index 35765b41159..32c2dd24881 100644 --- a/core/src/test/java/org/elasticsearch/ingest/core/ConfigurationUtilsTests.java +++ b/core/src/test/java/org/elasticsearch/ingest/core/ConfigurationUtilsTests.java @@ -34,7 +34,6 @@ import java.util.Map; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.sameInstance; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; @@ -45,6 +44,8 @@ public class ConfigurationUtilsTests extends ESTestCase { public void setConfig() { config = new HashMap<>(); config.put("foo", "bar"); + config.put("boolVal", true); + config.put("null", null); config.put("arr", Arrays.asList("1", "2", "3")); List list = new ArrayList<>(); list.add(2); @@ -68,6 +69,24 @@ public class ConfigurationUtilsTests extends ESTestCase { } } + public void testReadBooleanProperty() { + Boolean val = ConfigurationUtils.readBooleanProperty(null, null, config, "boolVal", false); + assertThat(val, equalTo(true)); + } + + public void testReadNullBooleanProperty() { + Boolean val = ConfigurationUtils.readBooleanProperty(null, null, config, "null", false); + assertThat(val, equalTo(false)); + } + + public void testReadBooleanPropertyInvalidType() { + try { + ConfigurationUtils.readBooleanProperty(null, null, config, "arr", true); + } catch (ElasticsearchParseException e) { + assertThat(e.getMessage(), equalTo("[arr] property isn't a boolean, but of type [java.util.Arrays$ArrayList]")); + } + } + // TODO(talevy): Issue with generics. This test should fail, "int" is of type List public void testOptional_InvalidType() { List val = ConfigurationUtils.readList(null, null, config, "int"); diff --git a/core/src/test/java/org/elasticsearch/ingest/core/IngestDocumentTests.java b/core/src/test/java/org/elasticsearch/ingest/core/IngestDocumentTests.java index 72f58ae22e0..b706c85afac 100644 --- a/core/src/test/java/org/elasticsearch/ingest/core/IngestDocumentTests.java +++ b/core/src/test/java/org/elasticsearch/ingest/core/IngestDocumentTests.java @@ -199,7 +199,7 @@ public class IngestDocumentTests extends ESTestCase { public void testGetFieldValueNull() { try { - ingestDocument.getFieldValue(null, String.class); + ingestDocument.getFieldValue((String) null, String.class); fail("get field value should have failed"); } catch (IllegalArgumentException e) { assertThat(e.getMessage(), equalTo("path cannot be null nor empty")); @@ -263,7 +263,7 @@ public class IngestDocumentTests extends ESTestCase { public void testHasFieldNull() { try { - ingestDocument.hasField(null); + ingestDocument.hasField((String) null); fail("has field should have failed"); } catch (IllegalArgumentException e) { assertThat(e.getMessage(), equalTo("path cannot be null nor empty")); diff --git a/core/src/test/java/org/elasticsearch/ingest/processor/SetProcessorFactoryTests.java b/core/src/test/java/org/elasticsearch/ingest/processor/SetProcessorFactoryTests.java index 2db2dcd5e1c..8810517de8f 100644 --- a/core/src/test/java/org/elasticsearch/ingest/processor/SetProcessorFactoryTests.java +++ b/core/src/test/java/org/elasticsearch/ingest/processor/SetProcessorFactoryTests.java @@ -22,7 +22,6 @@ package org.elasticsearch.ingest.processor; import org.elasticsearch.ElasticsearchParseException; import org.elasticsearch.ingest.TestTemplateService; import org.elasticsearch.ingest.core.AbstractProcessorFactory; -import org.elasticsearch.ingest.core.Processor; import org.elasticsearch.test.ESTestCase; import org.junit.Before; @@ -51,6 +50,22 @@ public class SetProcessorFactoryTests extends ESTestCase { assertThat(setProcessor.getTag(), equalTo(processorTag)); assertThat(setProcessor.getField().execute(Collections.emptyMap()), equalTo("field1")); assertThat(setProcessor.getValue().copyAndResolve(Collections.emptyMap()), equalTo("value1")); + assertThat(setProcessor.isOverrideEnabled(), equalTo(true)); + } + + public void testCreateWithOverride() throws Exception { + boolean overrideEnabled = randomBoolean(); + Map config = new HashMap<>(); + config.put("field", "field1"); + config.put("value", "value1"); + config.put("override", overrideEnabled); + String processorTag = randomAsciiOfLength(10); + config.put(AbstractProcessorFactory.TAG_KEY, processorTag); + SetProcessor setProcessor = factory.create(config); + assertThat(setProcessor.getTag(), equalTo(processorTag)); + assertThat(setProcessor.getField().execute(Collections.emptyMap()), equalTo("field1")); + assertThat(setProcessor.getValue().copyAndResolve(Collections.emptyMap()), equalTo("value1")); + assertThat(setProcessor.isOverrideEnabled(), equalTo(overrideEnabled)); } public void testCreateNoFieldPresent() throws Exception { diff --git a/core/src/test/java/org/elasticsearch/ingest/processor/SetProcessorTests.java b/core/src/test/java/org/elasticsearch/ingest/processor/SetProcessorTests.java index 283825cdad8..37c20e186b9 100644 --- a/core/src/test/java/org/elasticsearch/ingest/processor/SetProcessorTests.java +++ b/core/src/test/java/org/elasticsearch/ingest/processor/SetProcessorTests.java @@ -38,7 +38,7 @@ public class SetProcessorTests extends ESTestCase { IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random()); String fieldName = RandomDocumentPicks.randomExistingFieldName(random(), ingestDocument); Object fieldValue = RandomDocumentPicks.randomFieldValue(random()); - Processor processor = createSetProcessor(fieldName, fieldValue); + Processor processor = createSetProcessor(fieldName, fieldValue, true); processor.execute(ingestDocument); assertThat(ingestDocument.hasField(fieldName), equalTo(true)); assertThat(ingestDocument.getFieldValue(fieldName, Object.class), equalTo(fieldValue)); @@ -50,7 +50,7 @@ public class SetProcessorTests extends ESTestCase { IngestDocument testIngestDocument = RandomDocumentPicks.randomIngestDocument(random(), new HashMap<>()); Object fieldValue = RandomDocumentPicks.randomFieldValue(random()); String fieldName = RandomDocumentPicks.addRandomField(random(), testIngestDocument, fieldValue); - Processor processor = createSetProcessor(fieldName, fieldValue); + Processor processor = createSetProcessor(fieldName, fieldValue, true); processor.execute(ingestDocument); assertThat(ingestDocument.hasField(fieldName), equalTo(true)); assertThat(ingestDocument.getFieldValue(fieldName, Object.class), equalTo(fieldValue)); @@ -59,7 +59,7 @@ public class SetProcessorTests extends ESTestCase { public void testSetFieldsTypeMismatch() throws Exception { IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), new HashMap<>()); ingestDocument.setFieldValue("field", "value"); - Processor processor = createSetProcessor("field.inner", "value"); + Processor processor = createSetProcessor("field.inner", "value", true); try { processor.execute(ingestDocument); fail("processor execute should have failed"); @@ -68,16 +68,47 @@ public class SetProcessorTests extends ESTestCase { } } + public void testSetNewFieldWithOverrideDisabled() throws Exception { + IngestDocument ingestDocument = new IngestDocument(new HashMap<>(), new HashMap<>()); + String fieldName = RandomDocumentPicks.randomFieldName(random()); + Object fieldValue = RandomDocumentPicks.randomFieldValue(random()); + Processor processor = createSetProcessor(fieldName, fieldValue, false); + processor.execute(ingestDocument); + assertThat(ingestDocument.hasField(fieldName), equalTo(true)); + assertThat(ingestDocument.getFieldValue(fieldName, Object.class), equalTo(fieldValue)); + } + + public void testSetExistingFieldWithOverrideDisabled() throws Exception { + IngestDocument ingestDocument = new IngestDocument(new HashMap<>(), new HashMap<>()); + Object fieldValue = "foo"; + String fieldName = RandomDocumentPicks.addRandomField(random(), ingestDocument, fieldValue); + Processor processor = createSetProcessor(fieldName, "bar", false); + processor.execute(ingestDocument); + assertThat(ingestDocument.hasField(fieldName), equalTo(true)); + assertThat(ingestDocument.getFieldValue(fieldName, Object.class), equalTo(fieldValue)); + } + + public void testSetExistingNullFieldWithOverrideDisabled() throws Exception { + IngestDocument ingestDocument = new IngestDocument(new HashMap<>(), new HashMap<>()); + Object fieldValue = null; + Object newValue = "bar"; + String fieldName = RandomDocumentPicks.addRandomField(random(), ingestDocument, fieldValue); + Processor processor = createSetProcessor(fieldName, newValue, false); + processor.execute(ingestDocument); + assertThat(ingestDocument.hasField(fieldName), equalTo(true)); + assertThat(ingestDocument.getFieldValue(fieldName, Object.class), equalTo(newValue)); + } + public void testSetMetadata() throws Exception { IngestDocument.MetaData randomMetaData = randomFrom(IngestDocument.MetaData.values()); - Processor processor = createSetProcessor(randomMetaData.getFieldName(), "_value"); + Processor processor = createSetProcessor(randomMetaData.getFieldName(), "_value", true); IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random()); processor.execute(ingestDocument); assertThat(ingestDocument.getFieldValue(randomMetaData.getFieldName(), String.class), Matchers.equalTo("_value")); } - private static Processor createSetProcessor(String fieldName, Object fieldValue) { + private static Processor createSetProcessor(String fieldName, Object fieldValue, boolean overrideEnabled) { TemplateService templateService = TestTemplateService.instance(); - return new SetProcessor(randomAsciiOfLength(10), templateService.compile(fieldName), ValueSource.wrap(fieldValue, templateService)); + return new SetProcessor(randomAsciiOfLength(10), templateService.compile(fieldName), ValueSource.wrap(fieldValue, templateService), overrideEnabled); } } diff --git a/docs/reference/ingest/ingest-node.asciidoc b/docs/reference/ingest/ingest-node.asciidoc index 95bf7cc2d5e..93dabbd9122 100644 --- a/docs/reference/ingest/ingest-node.asciidoc +++ b/docs/reference/ingest/ingest-node.asciidoc @@ -1179,6 +1179,7 @@ its value will be replaced with the provided one. | Name | Required | Default | Description | `field` | yes | - | The field to insert, upsert, or update | `value` | yes | - | The value to be set for the field +| `override`| no | true | If processor will update fields with pre-existing non-null-valued field. When set to `false`, such fields will not be touched. |====== [source,js]