Merge pull request #17902 from talevy/fix/17659-set-override

add ability to disable ability to override values of existing fields in set processor

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<String, Object> 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.
      *

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> T getFieldValue(TemplateService.Template pathTemplate, Class<T> 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

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)  {
+        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,8 +65,10 @@ public final class SetProcessor extends AbstractProcessor {
 
     @Override
     public void execute(IngestDocument document) {
+        if (overrideEnabled || document.hasField(field) == false || document.getFieldValue(field, Object.class) == null) {
             document.setFieldValue(field, value);
         }
+    }
 
     @Override
     public String getType() {
@@ -75,7 +87,8 @@ public final class SetProcessor extends AbstractProcessor {
         public SetProcessor doCreate(String processorTag, Map<String, Object> 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);
         }
     }
 }

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<Integer> 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<Integer>
     public void testOptional_InvalidType() {
         List<String> val = ConfigurationUtils.readList(null, null, config, "int");

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"));

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<String, Object> 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 {

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);
     }
 }

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]