Add option for Append Processor to skip/allow empty values (#105718)

* Add option for Append Processor to skip/allow empty values

If one is using templates to generate values, this option can be helpful
to avoid adding values that resolve into empty strings

Fixes #104813

* Change allow_empty_values to ignore_empty_values

* Update docs/changelog/105718.yaml

* Handle nulls as well

The original version of this PR predated `copy_from`, so the value
wouldn't have been able to be null here. But now that `copy_from`
exists we should handle null as well as empty strings.

* Rewrite setFieldValue in terms of valueNotEmpty

Since it handles nulls now, this logic is identical.

* Rewrite another setFieldValue arity, too

This one is a bit trickier, so I've lampshaded the surprising part
with a comment, and also there's a newly added test on main via

* Verify an invariant

This arity of setFieldValue doesn't handle the single-value
ignore_empty_value case (like in a set processor), it only handles the
multi-value ignore_empty_values case (like in an append
processor). The same thing applies to allow_duplicates, that is also
an append-only concern.

* This test can't handle nulls anymore

* Fix a typo

* Declare these once

and don't use the return value from processor.execute

* Add some empty values into this test

* Ignore empty values with getFieldValue and copy_from

* Re-add the ignore_empty_values docs

* It seems conventional to just label these '9.2'

* Add an example

* Better quoting

* Add a test-only feature

* Add some rest tests

* Fix typos

---------

Co-authored-by: Joe Gallo <joegallo@gmail.com>

docs/changelog/105718.yaml

@@ -0,0 +1,6 @@
+pr: 105718
+summary: Add option for Append Processor to skip/allow empty values
+area: Ingest Node
+type: enhancement
+issues:
+ - 104813

docs/reference/enrich-processor/append-processor.md

@@ -15,8 +15,9 @@ $$$append-options$$$
 | --- | --- | --- | --- |
 | `field` | yes | - | The field to be appended to. Supports [template snippets](docs-content://manage-data/ingest/transform-enrich/ingest-pipelines.md#template-snippets). |
 | `value` | yes* | - | The value to be appended. Supports [template snippets](docs-content://manage-data/ingest/transform-enrich/ingest-pipelines.md#template-snippets). May specify only one of `value` or `copy_from`. |
-| `copy_from` {applies_to}`stack: ga 9.2.0` | no | - | The origin field which will be appended to `field`, cannot set `value` simultaneously. |
+| `copy_from` {applies_to}`stack: ga 9.2` | no | - | The origin field which will be appended to `field`, cannot set `value` simultaneously. |
 | `allow_duplicates` | no | true | If `false`, the processor does not appendvalues already present in the field. |
+| `ignore_empty_values` {applies_to}`stack: ga 9.2` | no | false | If `true`, the processor does not append values that resolve to `null` or an empty string.
 | `media_type` | no | `application/json` | The media type for encoding `value`. Applies only when `value` is a [template snippet](docs-content://manage-data/ingest/transform-enrich/ingest-pipelines.md#template-snippets). Must be one of `application/json`, `text/plain`, or`application/x-www-form-urlencoded`. |
 | `description` | no | - | Description of the processor. Useful for describing the purpose of the processor or its configuration. |
 | `if` | no | - | Conditionally execute the processor. See [Conditionally run a processor](docs-content://manage-data/ingest/transform-enrich/ingest-pipelines.md#conditionally-run-processor). |
@@ -24,6 +25,12 @@ $$$append-options$$$
 | `on_failure` | no | - | Handle failures for the processor. See [Handling pipeline failures](docs-content://manage-data/ingest/transform-enrich/ingest-pipelines.md#handling-pipeline-failures). |
 | `tag` | no | - | Identifier for the processor. Useful for debugging and metrics. |
 
+## Examples [append-processor-examples]
+
+### Simple example [append-processor-simple-example]
+
+Here is an `append` processor definition that adds the string `"production"` as well as the values of the `app` and `owner` fields to the `tags` field:
+
 ```js
 {
   "append": {
@@ -33,3 +40,21 @@ $$$append-options$$$
 }
 ```
 
+### Example using `allow_duplicates` and `ignore_empty_values` [append-processor-example-using-allow-duplicates-and-ignore-empty-values]
+
+```{applies_to}
+stack: ga 9.2
+```
+
+By using `allow_duplicates` and `ignore_empty_values`, it is possible to only append the `host.name` to the `related.hosts` if the `host.name` is not empty and if the value is not already present in `related.hosts`:
+
+```js
+{
+  "append": {
+    "field": "related.hosts",
+    "copy_from": "host.name",
+    "allow_duplicates": false,
+    "ignore_empty_values": true
+  }
+}
+```

modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/AppendProcessor.java

@@ -36,6 +36,7 @@ public final class AppendProcessor extends AbstractProcessor {
     private final ValueSource value;
     private final String copyFrom;
     private final boolean allowDuplicates;
+    private final boolean ignoreEmptyValues;
 
     AppendProcessor(
         String tag,
@@ -43,13 +44,15 @@ public final class AppendProcessor extends AbstractProcessor {
         TemplateScript.Factory field,
         ValueSource value,
         String copyFrom,
-        boolean allowDuplicates
+        boolean allowDuplicates,
+        boolean ignoreEmptyValues
     ) {
         super(tag, description);
         this.field = field;
         this.value = value;
         this.copyFrom = copyFrom;
         this.allowDuplicates = allowDuplicates;
+        this.ignoreEmptyValues = ignoreEmptyValues;
     }
 
     public TemplateScript.Factory getField() {
@@ -68,10 +71,10 @@ public final class AppendProcessor extends AbstractProcessor {
     public IngestDocument execute(IngestDocument document) throws Exception {
         String path = document.renderTemplate(field);
         if (copyFrom != null) {
-            Object fieldValue = document.getFieldValue(copyFrom, Object.class);
-            document.appendFieldValue(path, IngestDocument.deepCopy(fieldValue), allowDuplicates);
+            Object fieldValue = document.getFieldValue(copyFrom, Object.class, ignoreEmptyValues);
+            document.appendFieldValue(path, IngestDocument.deepCopy(fieldValue), allowDuplicates, ignoreEmptyValues);
         } else {
-            document.appendFieldValue(path, value, allowDuplicates);
+            document.appendFieldValue(path, value, allowDuplicates, ignoreEmptyValues);
         }
         return document;
     }
@@ -116,9 +119,17 @@ public final class AppendProcessor extends AbstractProcessor {
                 }
             }
             boolean allowDuplicates = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "allow_duplicates", true);
+            boolean ignoreEmptyValues = ConfigurationUtils.readBooleanProperty(TYPE, processorTag, config, "ignore_empty_values", false);
             TemplateScript.Factory compiledTemplate = ConfigurationUtils.compileTemplate(TYPE, processorTag, "field", field, scriptService);
-
-            return new AppendProcessor(processorTag, description, compiledTemplate, valueSource, copyFrom, allowDuplicates);
+            return new AppendProcessor(
+                processorTag,
+                description,
+                compiledTemplate,
+                valueSource,
+                copyFrom,
+                allowDuplicates,
+                ignoreEmptyValues
+            );
         }
     }
 }

modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/AppendProcessorTests.java

@@ -29,6 +29,7 @@ import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
@@ -52,13 +53,13 @@ public class AppendProcessorTests extends ESTestCase {
         if (randomBoolean()) {
             Object value = scalar.randomValue();
             values.add(value);
-            appendProcessor = createAppendProcessor(field, value, null, true);
+            appendProcessor = createAppendProcessor(field, value, null, true, false);
         } else {
             int valuesSize = randomIntBetween(0, 10);
             for (int i = 0; i < valuesSize; i++) {
                 values.add(scalar.randomValue());
             }
-            appendProcessor = createAppendProcessor(field, values, null, true);
+            appendProcessor = createAppendProcessor(field, values, null, true, false);
         }
         appendProcessor.execute(ingestDocument);
         Object fieldValue = ingestDocument.getFieldValue(field, Object.class);
@@ -81,13 +82,13 @@ public class AppendProcessorTests extends ESTestCase {
         if (randomBoolean()) {
             Object value = scalar.randomValue();
             values.add(value);
-            appendProcessor = createAppendProcessor(field, value, null, true);
+            appendProcessor = createAppendProcessor(field, value, null, true, false);
         } else {
             int valuesSize = randomIntBetween(0, 10);
             for (int i = 0; i < valuesSize; i++) {
                 values.add(scalar.randomValue());
             }
-            appendProcessor = createAppendProcessor(field, values, null, true);
+            appendProcessor = createAppendProcessor(field, values, null, true, false);
         }
         appendProcessor.execute(ingestDocument);
         List<?> list = ingestDocument.getFieldValue(field, List.class);
@@ -105,13 +106,13 @@ public class AppendProcessorTests extends ESTestCase {
         if (randomBoolean()) {
             Object value = scalar.randomValue();
             values.add(value);
-            appendProcessor = createAppendProcessor(field, value, null, true);
+            appendProcessor = createAppendProcessor(field, value, null, true, false);
         } else {
             int valuesSize = randomIntBetween(0, 10);
             for (int i = 0; i < valuesSize; i++) {
                 values.add(scalar.randomValue());
             }
-            appendProcessor = createAppendProcessor(field, values, null, true);
+            appendProcessor = createAppendProcessor(field, values, null, true, false);
         }
         appendProcessor.execute(ingestDocument);
         List<?> fieldValue = ingestDocument.getFieldValue(field, List.class);
@@ -129,7 +130,7 @@ public class AppendProcessorTests extends ESTestCase {
 
         List<Object> valuesToAppend = new ArrayList<>();
         valuesToAppend.add(originalValue);
-        Processor appendProcessor = createAppendProcessor(field, valuesToAppend, null, false);
+        Processor appendProcessor = createAppendProcessor(field, valuesToAppend, null, false, false);
         appendProcessor.execute(ingestDocument);
         Object fieldValue = ingestDocument.getFieldValue(field, Object.class);
         assertThat(fieldValue, not(instanceOf(List.class)));
@@ -144,7 +145,7 @@ public class AppendProcessorTests extends ESTestCase {
         List<Object> valuesToAppend = new ArrayList<>();
         String newValue = randomValueOtherThan(originalValue, () -> randomAlphaOfLengthBetween(1, 10));
         valuesToAppend.add(newValue);
-        Processor appendProcessor = createAppendProcessor(field, valuesToAppend, null, false);
+        Processor appendProcessor = createAppendProcessor(field, valuesToAppend, null, false, false);
         appendProcessor.execute(ingestDocument);
         List<?> list = ingestDocument.getFieldValue(field, List.class);
         assertThat(list.size(), equalTo(2));
@@ -173,22 +174,141 @@ public class AppendProcessorTests extends ESTestCase {
         Collections.sort(valuesToAppend);
 
         // attempt to append both new and existing values
-        Processor appendProcessor = createAppendProcessor(originalField, valuesToAppend, null, false);
+        Processor appendProcessor = createAppendProcessor(originalField, valuesToAppend, null, false, false);
         appendProcessor.execute(ingestDocument);
         List<?> fieldValue = ingestDocument.getFieldValue(originalField, List.class);
         assertThat(fieldValue, sameInstance(list));
         assertThat(fieldValue, containsInAnyOrder(expectedValues.toArray()));
     }
 
+    public void testAppendingToListWithNoEmptyValuesAndEmptyValuesDisallowed() throws Exception {
+        IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random());
+        Scalar scalar = randomValueOtherThan(Scalar.NULL, () -> randomFrom(Scalar.values()));
+        List<Object> list = new ArrayList<>();
+        int size = randomIntBetween(0, 10);
+        for (int i = 0; i < size; i++) {
+            list.add(scalar.randomValue());
+        }
+        List<Object> checkList = new ArrayList<>(list);
+        String field = RandomDocumentPicks.addRandomField(random(), ingestDocument, list);
+        List<Object> values = new ArrayList<>();
+        Processor appendProcessor;
+        if (randomBoolean()) {
+            Object value = scalar.randomValue();
+            values.add(value);
+            appendProcessor = createAppendProcessor(field, value, null, true, true);
+        } else {
+            int valuesSize = randomIntBetween(0, 10);
+            for (int i = 0; i < valuesSize; i++) {
+                values.add(scalar.randomValue());
+            }
+            appendProcessor = createAppendProcessor(field, values, null, true, true);
+        }
+        appendProcessor.execute(ingestDocument);
+        Object fieldValue = ingestDocument.getFieldValue(field, Object.class);
+        assertThat(fieldValue, sameInstance(list));
+        assertThat(list.size(), equalTo(size + values.size()));
+        for (int i = 0; i < size; i++) {
+            assertThat(list.get(i), equalTo(checkList.get(i)));
+        }
+        for (int i = size; i < size + values.size(); i++) {
+            assertThat(list.get(i), equalTo(values.get(i - size)));
+        }
+    }
+
+    public void testAppendingToListEmptyStringAndEmptyValuesDisallowed() throws Exception {
+        IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random());
+        Scalar scalar = Scalar.STRING;
+        List<Object> list = new ArrayList<>();
+        int size = randomIntBetween(0, 10);
+        for (int i = 0; i < size; i++) {
+            list.add(scalar.randomValue());
+        }
+        List<Object> checkList = new ArrayList<>(list);
+        String field = RandomDocumentPicks.addRandomField(random(), ingestDocument, list);
+        List<Object> values = new ArrayList<>();
+        Processor appendProcessor;
+        if (randomBoolean()) {
+            Object value;
+            if (randomBoolean()) {
+                value = "";
+            } else {
+                value = scalar.randomValue();
+                values.add(value);
+            }
+            appendProcessor = createAppendProcessor(field, value, null, true, true);
+        } else {
+            int valuesSize = randomIntBetween(0, 10);
+            List<Object> allValues = new ArrayList<>();
+            for (int i = 0; i < valuesSize; i++) {
+                Object value;
+                if (randomBoolean()) {
+                    value = "";
+                } else {
+                    value = scalar.randomValue();
+                    values.add(value);
+                }
+                allValues.add(value);
+            }
+            appendProcessor = createAppendProcessor(field, allValues, null, true, true);
+        }
+        appendProcessor.execute(ingestDocument);
+        Object fieldValue = ingestDocument.getFieldValue(field, Object.class);
+        assertThat(fieldValue, sameInstance(list));
+        assertThat(list.size(), equalTo(size + values.size()));
+        for (int i = 0; i < size; i++) {
+            assertThat(list.get(i), equalTo(checkList.get(i)));
+        }
+        for (int i = size; i < size + values.size(); i++) {
+            assertThat(list.get(i), equalTo(values.get(i - size)));
+        }
+    }
+
+    public void testAppendingToNonExistingListEmptyStringAndEmptyValuesDisallowed() throws Exception {
+        IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), new HashMap<>());
+        String field = RandomDocumentPicks.randomFieldName(random());
+        Scalar scalar = Scalar.STRING;
+        List<Object> values = new ArrayList<>();
+        Processor appendProcessor;
+        if (randomBoolean()) {
+            Object value;
+            if (randomBoolean()) {
+                value = "";
+            } else {
+                value = scalar.randomValue();
+                values.add(value);
+            }
+            appendProcessor = createAppendProcessor(field, value, null, true, true);
+        } else {
+            List<Object> allValues = new ArrayList<>();
+            int valuesSize = randomIntBetween(0, 10);
+            for (int i = 0; i < valuesSize; i++) {
+                Object value;
+                if (randomBoolean()) {
+                    value = "";
+                } else {
+                    value = scalar.randomValue();
+                    values.add(value);
+                }
+                allValues.add(value);
+            }
+            appendProcessor = createAppendProcessor(field, allValues, null, true, true);
+        }
+        appendProcessor.execute(ingestDocument);
+        List<?> list = ingestDocument.getFieldValue(field, List.class);
+        assertThat(list, not(sameInstance(values)));
+        assertThat(list, equalTo(values));
+    }
+
     public void testCopyFromOtherFieldSimple() throws Exception {
         IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random());
         ingestDocument.setFieldValue("foo", 1);
         ingestDocument.setFieldValue("bar", 2);
         ingestDocument.setFieldValue("baz", new ArrayList<>(List.of(3)));
 
-        createAppendProcessor("bar", null, "foo", false).execute(ingestDocument);
-        createAppendProcessor("baz", null, "bar", false).execute(ingestDocument);
-        createAppendProcessor("quux", null, "baz", false).execute(ingestDocument);
+        createAppendProcessor("bar", null, "foo", false, false).execute(ingestDocument);
+        createAppendProcessor("baz", null, "bar", false, false).execute(ingestDocument);
+        createAppendProcessor("quux", null, "baz", false, false).execute(ingestDocument);
 
         Map<String, Object> result = ingestDocument.getCtxMap().getSource();
         assertThat(result.get("foo"), equalTo(1));
@@ -209,27 +329,34 @@ public class AppendProcessorTests extends ESTestCase {
         String targetField = RandomDocumentPicks.addRandomField(random(), ingestDocument, targetFieldValue);
         String sourceField = RandomDocumentPicks.addRandomField(random(), ingestDocument, additionalValues);
 
-        Processor appendProcessor = createAppendProcessor(targetField, null, sourceField, false);
+        // add two empty values onto the source field, these will be ignored
+        ingestDocument.appendFieldValue(sourceField, null);
+        ingestDocument.appendFieldValue(sourceField, "");
+
+        Processor appendProcessor = createAppendProcessor(targetField, null, sourceField, false, true);
         appendProcessor.execute(ingestDocument);
         List<?> fieldValue = ingestDocument.getFieldValue(targetField, List.class);
         assertThat(fieldValue, sameInstance(targetFieldValue));
         assertThat(fieldValue, containsInAnyOrder(allValues.toArray()));
+        assertThat(fieldValue, not(contains(null, "")));
     }
 
     public void testCopyFromCopiesNonPrimitiveMutableTypes() throws Exception {
+        Map<String, Object> document;
+        IngestDocument ingestDocument;
         final String sourceField = "sourceField";
         final String targetField = "targetField";
-        Processor processor = createAppendProcessor(targetField, null, sourceField, false);
+        Processor processor = createAppendProcessor(targetField, null, sourceField, false, false);
 
         // map types
-        Map<String, Object> document = new HashMap<>();
+        document = new HashMap<>();
         Map<String, Object> sourceMap = new HashMap<>();
         sourceMap.put("foo", "bar");
         document.put(sourceField, sourceMap);
-        IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
-        IngestDocument output = processor.execute(ingestDocument);
+        ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
+        processor.execute(ingestDocument);
         sourceMap.put("foo", "not-bar");
-        Map<?, ?> outputMap = (Map<?, ?>) output.getFieldValue(targetField, List.class).getFirst();
+        Map<?, ?> outputMap = (Map<?, ?>) ingestDocument.getFieldValue(targetField, List.class).getFirst();
         assertThat(outputMap.get("foo"), equalTo("bar"));
 
         // set types
@@ -282,7 +409,7 @@ public class AppendProcessorTests extends ESTestCase {
     public void testCopyFromDeepCopiesNonPrimitiveMutableTypes() throws Exception {
         final String sourceField = "sourceField";
         final String targetField = "targetField";
-        Processor processor = createAppendProcessor(targetField, null, sourceField, false);
+        Processor processor = createAppendProcessor(targetField, null, sourceField, false, false);
         Map<String, Object> document = new HashMap<>();
 
         // a root map with values of map, set, list, bytes, date
@@ -308,8 +435,8 @@ public class AppendProcessorTests extends ESTestCase {
 
         document.put(sourceField, root);
         IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), document);
-        IngestDocument output = processor.execute(ingestDocument);
-        Map<?, ?> outputRoot = (Map<?, ?>) output.getFieldValue(targetField, List.class).getFirst();
+        processor.execute(ingestDocument);
+        Map<?, ?> outputRoot = (Map<?, ?>) ingestDocument.getFieldValue(targetField, List.class).getFirst();
 
         root.put("foo", "not-bar");
         sourceMap.put("foo", "not-bar");
@@ -326,14 +453,21 @@ public class AppendProcessorTests extends ESTestCase {
         assertThat(((Date) outputRoot.get("date")), equalTo(preservedDate));
     }
 
-    private static Processor createAppendProcessor(String fieldName, Object fieldValue, String copyFrom, boolean allowDuplicates) {
+    private static Processor createAppendProcessor(
+        String fieldName,
+        Object fieldValue,
+        String copyFrom,
+        boolean allowDuplicates,
+        boolean ignoreEmptyValues
+    ) {
         return new AppendProcessor(
             randomAlphaOfLength(10),
             null,
             new TestTemplateService.MockTemplateScript.Factory(fieldName),
             ValueSource.wrap(fieldValue, TestTemplateService.instance()),
             copyFrom,
-            allowDuplicates
+            allowDuplicates,
+            ignoreEmptyValues
         );
     }
 

modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ForEachProcessorTests.java

@@ -207,7 +207,7 @@ public class ForEachProcessorTests extends ESTestCase {
             new CompoundProcessor(
                 false,
                 List.of(new UppercaseProcessor("_tag_upper", null, "_ingest._value", false, "_ingest._value")),
-                List.of(new AppendProcessor("_tag", null, template, (model) -> (List.of("added")), null, true))
+                List.of(new AppendProcessor("_tag", null, template, (model) -> (List.of("added")), null, true, false))
             ),
             false
         );

modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/370_append_ignore_empty_values.yml

@@ -0,0 +1,153 @@
+---
+setup:
+  - requires:
+      cluster_features: [ "ingest.append.ignore_empty_values" ]
+      reason: "The ignore_empty_values option of the append processor is new"
+  - do:
+      ingest.put_pipeline:
+        id: "test-pipeline-1"
+        body: >
+          {
+            "processors": [
+              {
+                "append": {
+                  "field": "dest",
+                  "copy_from": "src",
+                  "ignore_empty_values": true
+                }
+              },
+              {
+                "remove": {
+                  "field": "src",
+                  "ignore_missing": true
+                }
+              }
+            ]
+          }
+  - do:
+      indices.create:
+        index: "test-some-index"
+
+---
+teardown:
+  - do:
+      indices.delete:
+        index: "test-some-index"
+        ignore_unavailable: true
+  - do:
+      ingest.delete_pipeline:
+        id: "test-pipeline-1"
+        ignore: 404
+
+---
+"A missing copy_from value is ignored":
+  - do:
+      index:
+        index: test-some-index
+        id: 1
+        pipeline: test-pipeline-1
+        body: >
+          {
+            "dest": [1]
+          }
+
+  - do:
+      get:
+        index: test-some-index
+        id: "1"
+  - match: { _source.dest: [1] }
+
+---
+"A null copy_from value is ignored":
+  - do:
+      index:
+        index: test-some-index
+        id: 1
+        pipeline: test-pipeline-1
+        body: >
+          {
+            "src": null,
+            "dest": [1]
+          }
+
+  - do:
+      get:
+        index: test-some-index
+        id: "1"
+  - match: { _source.dest: [1] }
+
+---
+"An empty string copy_from value is ignored":
+  - do:
+      index:
+        index: test-some-index
+        id: 1
+        pipeline: test-pipeline-1
+        body: >
+          {
+            "src": "",
+            "dest": [1]
+          }
+
+  - do:
+      get:
+        index: test-some-index
+        id: "1"
+  - match: { _source.dest: [1] }
+
+---
+"An empty array copy_from value is ignored":
+  - do:
+      index:
+        index: test-some-index
+        id: 1
+        pipeline: test-pipeline-1
+        body: >
+          {
+            "src": [],
+            "dest": [1]
+          }
+
+  - do:
+      get:
+        index: test-some-index
+        id: "1"
+  - match: { _source.dest: [1] }
+
+---
+"A non-null value is copied":
+  - do:
+      index:
+        index: test-some-index
+        id: 1
+        pipeline: test-pipeline-1
+        body: >
+          {
+            "src": 2,
+            "dest": [1]
+          }
+
+  - do:
+      get:
+        index: test-some-index
+        id: "1"
+  - match: { _source.dest: [1, 2] }
+
+---
+"Empty strings and nulls are ignored in an array":
+  - do:
+      index:
+        index: test-some-index
+        id: 1
+        pipeline: test-pipeline-1
+        body: >
+          {
+            "src": ["", 2, null, 3],
+            "dest": [1]
+          }
+
+  - do:
+      get:
+        index: test-some-index
+        id: "1"
+  - match: { _source.dest: [1, 2, 3] }

server/src/main/java/org/elasticsearch/ingest/IngestDocument.java

@@ -641,7 +641,25 @@ public final class IngestDocument {
      * @throws IllegalArgumentException if the path is null, empty or invalid.
      */
     public void appendFieldValue(String path, Object value, boolean allowDuplicates) {
-        setFieldValue(path, value, true, allowDuplicates);
+        setFieldValue(path, value, true, allowDuplicates, false);
+    }
+
+    /**
+     * Appends the provided value to the provided path in the document.
+     * Any non existing path element will be created.
+     * If the path identifies a list, the value will be appended to the existing list.
+     * If the path identifies a scalar, the scalar will be converted to a list and
+     * the provided value will be added to the newly created list.
+     * Supports multiple values too provided in forms of list, in that case all the values will be appended to the
+     * existing (or newly created) list.
+     * @param path The path within the document in dot-notation
+     * @param value The value or values to append to the existing ones
+     * @param allowDuplicates When false, any values that already exist in the field will not be added
+     * @param ignoreEmptyValues When true, values that resolve to empty strings will not be added
+     * @throws IllegalArgumentException if the path is null, empty or invalid.
+     */
+    public void appendFieldValue(String path, Object value, boolean allowDuplicates, boolean ignoreEmptyValues) {
+        setFieldValue(path, value, true, allowDuplicates, ignoreEmptyValues);
     }
 
     /**
@@ -655,10 +673,11 @@ public final class IngestDocument {
      * @param path The path within the document in dot-notation
      * @param valueSource The value source that will produce the value or values to append to the existing ones
      * @param allowDuplicates When false, any values that already exist in the field will not be added
+     * @param ignoreEmptyValues When true, values that resolve to empty strings will not be added
      * @throws IllegalArgumentException if the path is null, empty or invalid.
      */
-    public void appendFieldValue(String path, ValueSource valueSource, boolean allowDuplicates) {
-        appendFieldValue(path, valueSource.copyAndResolve(templateModel), allowDuplicates);
+    public void appendFieldValue(String path, ValueSource valueSource, boolean allowDuplicates, boolean ignoreEmptyValues) {
+        appendFieldValue(path, valueSource.copyAndResolve(templateModel), allowDuplicates, ignoreEmptyValues);
     }
 
     /**
@@ -672,7 +691,7 @@ public final class IngestDocument {
      * item identified by the provided path.
      */
     public void setFieldValue(String path, Object value) {
-        setFieldValue(path, value, false, true);
+        setFieldValue(path, value, false, false, false);
     }
 
     /**
@@ -700,16 +719,17 @@ public final class IngestDocument {
      */
     public void setFieldValue(String path, ValueSource valueSource, boolean ignoreEmptyValue) {
         Object value = valueSource.copyAndResolve(templateModel);
-        if (ignoreEmptyValue && valueSource instanceof ValueSource.TemplatedValue) {
-            if (value == null) {
-                return;
-            }
-            String valueStr = (String) value;
-            if (valueStr.isEmpty()) {
-                return;
+        if (valueSource instanceof ValueSource.TemplatedValue) {
+            if (ignoreEmptyValue == false || valueNotEmpty(value)) {
+                setFieldValue(path, value);
             }
+        } else {
+            // it may seem a little surprising to not bother checking ignoreEmptyValue value here.
+            // but this corresponds to the case of, e.g., a set processor with a literal value.
+            // so if you have `"value": ""` and `"ignore_empty_value": true` right next to each other
+            // in your processor definition, then, well, that's on you for being a bit silly. ;)
+            setFieldValue(path, value);
         }
-        setFieldValue(path, value);
     }
 
     /**
@@ -723,20 +743,14 @@ public final class IngestDocument {
      * item identified by the provided path.
      */
     public void setFieldValue(String path, Object value, boolean ignoreEmptyValue) {
-        if (ignoreEmptyValue) {
-            if (value == null) {
-                return;
-            }
-            if (value instanceof String string) {
-                if (string.isEmpty()) {
-                    return;
-                }
-            }
+        if (ignoreEmptyValue == false || valueNotEmpty(value)) {
+            setFieldValue(path, value);
         }
-        setFieldValue(path, value);
     }
 
-    private void setFieldValue(String path, Object value, boolean append, boolean allowDuplicates) {
+    private void setFieldValue(String path, Object value, boolean append, boolean allowDuplicates, boolean ignoreEmptyValues) {
+        assert append || (allowDuplicates == false && ignoreEmptyValues == false)
+            : "allowDuplicates and ignoreEmptyValues only apply if append is true";
         final FieldPath fieldPath = FieldPath.of(path, getCurrentAccessPatternSafe());
         Object context = fieldPath.initialContext(this);
         int leafKeyIndex = fieldPath.pathElements.length - 1;
@@ -864,10 +878,10 @@ public final class IngestDocument {
                 Object object = map.getOrDefault(leafKey, NOT_FOUND); // getOrDefault is faster than containsKey + get
                 if (object == NOT_FOUND) {
                     List<Object> list = new ArrayList<>();
-                    appendValues(list, value, allowDuplicates);
+                    appendValues(list, value, allowDuplicates, ignoreEmptyValues);
                     map.put(leafKey, list);
                 } else {
-                    Object list = appendValues(object, value, allowDuplicates);
+                    Object list = appendValues(object, value, allowDuplicates, ignoreEmptyValues);
                     if (list != object) {
                         map.put(leafKey, list);
                     }
@@ -882,10 +896,10 @@ public final class IngestDocument {
                 Object object = map.getOrDefault(leafKey, NOT_FOUND); // getOrDefault is faster than containsKey + get
                 if (object == NOT_FOUND) {
                     List<Object> list = new ArrayList<>();
-                    appendValues(list, value, allowDuplicates);
+                    appendValues(list, value, allowDuplicates, ignoreEmptyValues);
                     map.put(leafKey, list);
                 } else {
-                    Object list = appendValues(object, value, allowDuplicates);
+                    Object list = appendValues(object, value, allowDuplicates, ignoreEmptyValues);
                     if (list != object) {
                         map.put(leafKey, list);
                     }
@@ -911,7 +925,7 @@ public final class IngestDocument {
             } else {
                 if (append) {
                     Object object = list.get(index);
-                    Object newList = appendValues(object, value, allowDuplicates);
+                    Object newList = appendValues(object, value, allowDuplicates, ignoreEmptyValues);
                     if (newList != object) {
                         list.set(index, newList);
                     }
@@ -925,7 +939,7 @@ public final class IngestDocument {
     }
 
     @SuppressWarnings("unchecked")
-    private static Object appendValues(Object maybeList, Object value, boolean allowDuplicates) {
+    private static Object appendValues(Object maybeList, Object value, boolean allowDuplicates, boolean ignoreEmptyValues) {
         List<Object> list;
         if (maybeList instanceof List) {
             // maybeList is already a list, we append the provided values to it
@@ -936,35 +950,43 @@ public final class IngestDocument {
             list.add(maybeList);
         }
         if (allowDuplicates) {
-            innerAppendValues(list, value);
+            innerAppendValues(list, value, ignoreEmptyValues);
             return list;
         } else {
             // if no values were appended due to duplication, return the original object so the ingest document remains unmodified
-            return innerAppendValuesWithoutDuplicates(list, value) ? list : maybeList;
+            return innerAppendValuesWithoutDuplicates(list, value, ignoreEmptyValues) ? list : maybeList;
         }
     }
 
     // helper method for use in appendValues above, please do not call this directly except from that method
-    private static void innerAppendValues(List<Object> list, Object value) {
+    private static void innerAppendValues(List<Object> list, Object value, boolean ignoreEmptyValues) {
         if (value instanceof List<?> l) {
-            list.addAll(l);
-        } else {
+            if (ignoreEmptyValues) {
+                l.forEach((v) -> {
+                    if (valueNotEmpty(v)) {
+                        list.add(v);
+                    }
+                });
+            } else {
+                list.addAll(l);
+            }
+        } else if (ignoreEmptyValues == false || valueNotEmpty(value)) {
             list.add(value);
         }
     }
 
     // helper method for use in appendValues above, please do not call this directly except from that method
-    private static boolean innerAppendValuesWithoutDuplicates(List<Object> list, Object value) {
+    private static boolean innerAppendValuesWithoutDuplicates(List<Object> list, Object value, boolean ignoreEmptyValues) {
         boolean valuesWereAppended = false;
         if (value instanceof List<?> valueList) {
             for (Object val : valueList) {
-                if (list.contains(val) == false) {
+                if (list.contains(val) == false && (ignoreEmptyValues == false || valueNotEmpty(val))) {
                     list.add(val);
                     valuesWereAppended = true;
                 }
             }
         } else {
-            if (list.contains(value) == false) {
+            if (list.contains(value) == false && (ignoreEmptyValues == false || valueNotEmpty(value))) {
                 list.add(value);
                 valuesWereAppended = true;
             }
@@ -972,6 +994,16 @@ public final class IngestDocument {
         return valuesWereAppended;
     }
 
+    private static boolean valueNotEmpty(Object value) {
+        if (value == null) {
+            return false;
+        }
+        if (value instanceof String string) {
+            return string.isEmpty() == false;
+        }
+        return true;
+    }
+
     private static <T> T cast(String path, Object object, Class<T> clazz) {
         if (object == null) {
             return null;

server/src/main/java/org/elasticsearch/ingest/IngestFeatures.java

@@ -17,6 +17,7 @@ import java.util.Set;
 public class IngestFeatures implements FeatureSpecification {
     private static final NodeFeature SIMULATE_INGEST_400_ON_FAILURE = new NodeFeature("simulate.ingest.400_on_failure", true);
     private static final NodeFeature INGEST_APPEND_COPY_FROM = new NodeFeature("ingest.append.copy_from", true);
+    private static final NodeFeature INGEST_APPEND_IGNORE_EMPTY_VALUES = new NodeFeature("ingest.append.ignore_empty_values", true);
 
     @Override
     public Set<NodeFeature> getFeatures() {
@@ -25,6 +26,6 @@ public class IngestFeatures implements FeatureSpecification {
 
     @Override
     public Set<NodeFeature> getTestFeatures() {
-        return Set.of(SIMULATE_INGEST_400_ON_FAILURE, INGEST_APPEND_COPY_FROM);
+        return Set.of(SIMULATE_INGEST_400_ON_FAILURE, INGEST_APPEND_COPY_FROM, INGEST_APPEND_IGNORE_EMPTY_VALUES);
     }
 }