diff --git a/docs/changelog/105718.yaml b/docs/changelog/105718.yaml new file mode 100644 index 0000000000000..d7b00532ee4bc --- /dev/null +++ b/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 diff --git a/docs/reference/enrich-processor/append-processor.md b/docs/reference/enrich-processor/append-processor.md index e709abeda2189..14d9ac8e5dd1c 100644 --- a/docs/reference/enrich-processor/append-processor.md +++ b/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 + } +} +``` diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/AppendProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/AppendProcessor.java index d27d6a73824b4..20c27516f6474 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/AppendProcessor.java +++ b/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 String getCopyFrom() { 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 AppendProcessor create( } } 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 + ); } } } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/AppendProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/AppendProcessorTests.java index cee2e919e6b7c..e5afa5e1b18f4 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/AppendProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/AppendProcessorTests.java @@ -29,6 +29,7 @@ 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 void testAppendValuesToExistingList() throws Exception { 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 void testAppendValuesToNonExistingList() throws Exception { 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 void testConvertScalarToList() throws Exception { 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 void testAppendingDuplicateValueToScalarDoesNotModifyDocument() throws Ex List 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 void testAppendingUniqueValueToScalar() throws Exception { List 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 void testAppendingToListWithDuplicatesDisallowed() throws Exception { 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 list = new ArrayList<>(); + int size = randomIntBetween(0, 10); + for (int i = 0; i < size; i++) { + list.add(scalar.randomValue()); + } + List checkList = new ArrayList<>(list); + String field = RandomDocumentPicks.addRandomField(random(), ingestDocument, list); + List 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 list = new ArrayList<>(); + int size = randomIntBetween(0, 10); + for (int i = 0; i < size; i++) { + list.add(scalar.randomValue()); + } + List checkList = new ArrayList<>(list); + String field = RandomDocumentPicks.addRandomField(random(), ingestDocument, list); + List 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 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 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 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 result = ingestDocument.getCtxMap().getSource(); assertThat(result.get("foo"), equalTo(1)); @@ -209,27 +329,34 @@ public void testCopyFromOtherField() throws Exception { 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 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 document = new HashMap<>(); + document = new HashMap<>(); Map 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 void testCopyFromCopiesNonPrimitiveMutableTypes() throws Exception { 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 document = new HashMap<>(); // a root map with values of map, set, list, bytes, date @@ -308,8 +435,8 @@ public void testCopyFromDeepCopiesNonPrimitiveMutableTypes() throws Exception { 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 void testCopyFromDeepCopiesNonPrimitiveMutableTypes() throws Exception { 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 ); } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ForEachProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ForEachProcessorTests.java index f8013a4eebe73..e8df7fa9bb9c5 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ForEachProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ForEachProcessorTests.java @@ -207,7 +207,7 @@ public void testModifyFieldsOutsideArray() { 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 ); diff --git a/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/370_append_ignore_empty_values.yml b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/370_append_ignore_empty_values.yml new file mode 100644 index 0000000000000..a03c8cdd670f1 --- /dev/null +++ b/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] } diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java index 55c767e80cd0a..e79accd758dc7 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -641,7 +641,25 @@ public void appendFieldValue(String path, Object value) { * @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 void appendFieldValue(String path, Object value, boolean allowDuplicates) * @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 void appendFieldValue(String path, ValueSource valueSource, boolean allow * 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 void setFieldValue(String path, ValueSource valueSource) { */ 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 void setFieldValue(String path, ValueSource valueSource, boolean ignoreEm * 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 @@ private void setFieldValue(String path, Object value, boolean append, boolean al Object object = map.getOrDefault(leafKey, NOT_FOUND); // getOrDefault is faster than containsKey + get if (object == NOT_FOUND) { List 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 @@ private void setFieldValue(String path, Object value, boolean append, boolean al Object object = map.getOrDefault(leafKey, NOT_FOUND); // getOrDefault is faster than containsKey + get if (object == NOT_FOUND) { List 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 @@ private void setFieldValue(String path, Object value, boolean append, boolean al } 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 @@ private void setFieldValue(String path, Object value, boolean append, boolean al } @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 list; if (maybeList instanceof List) { // maybeList is already a list, we append the provided values to it @@ -936,35 +950,43 @@ private static Object appendValues(Object maybeList, Object value, boolean allow 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 list, Object value) { + private static void innerAppendValues(List 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 list, Object value) { + private static boolean innerAppendValuesWithoutDuplicates(List 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 @@ private static boolean innerAppendValuesWithoutDuplicates(List list, Obj 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 cast(String path, Object object, Class clazz) { if (object == null) { return null; diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestFeatures.java b/server/src/main/java/org/elasticsearch/ingest/IngestFeatures.java index 9063df9253b28..75d1121e3b9e6 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestFeatures.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestFeatures.java @@ -17,6 +17,7 @@ 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 getFeatures() { @@ -25,6 +26,6 @@ public Set getFeatures() { @Override public Set 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); } }