diff --git a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/RemoveProcessor.java b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/RemoveProcessor.java index 1b1dc67dcb8d..072162c479a4 100644 --- a/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/RemoveProcessor.java +++ b/modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/RemoveProcessor.java @@ -59,15 +59,9 @@ public final class RemoveProcessor extends AbstractProcessor { } private void fieldsToRemoveProcessor(IngestDocument document) { - // micro-optimization note: actual for-each loops here rather than a .forEach because it happens to be ~5% faster in benchmarks - if (ignoreMissing) { - for (TemplateScript.Factory field : fieldsToRemove) { - removeWhenPresent(document, document.renderTemplate(field)); - } - } else { - for (TemplateScript.Factory field : fieldsToRemove) { - document.removeField(document.renderTemplate(field)); - } + // micro-optimization note: actual for-each loop here rather than a .forEach because it happens to be ~5% faster in benchmarks + for (TemplateScript.Factory field : fieldsToRemove) { + document.removeField(document.renderTemplate(field), ignoreMissing); } } @@ -76,13 +70,7 @@ public final class RemoveProcessor extends AbstractProcessor { .stream() .filter(documentField -> IngestDocument.Metadata.isMetadata(documentField) == false) .filter(documentField -> shouldKeep(documentField, fieldsToKeep, document) == false) - .forEach(documentField -> removeWhenPresent(document, documentField)); - } - - private static void removeWhenPresent(IngestDocument document, String documentField) { - if (document.hasField(documentField)) { - document.removeField(documentField); - } + .forEach(documentField -> document.removeField(documentField, true)); } static boolean shouldKeep(String documentField, List fieldsToKeep, IngestDocument document) { diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java index caaac91ed200..11c725bacc51 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -317,15 +317,29 @@ public final class IngestDocument { /** * Removes the field identified by the provided path. + * * @param path the path of the field to be removed * @throws IllegalArgumentException if the path is null, empty, invalid or if the field doesn't exist. */ public void removeField(String path) { + removeField(path, false); + } + + /** + * Removes the field identified by the provided path. + * + * @param path the path of the field to be removed + * @param ignoreMissing The flag to determine whether to throw an exception when `path` is not found in the document. + * @throws IllegalArgumentException if the path is null, empty, or invalid; or if the field doesn't exist (and ignoreMissing is false). + */ + public void removeField(String path, boolean ignoreMissing) { final FieldPath fieldPath = FieldPath.of(path); Object context = fieldPath.initialContext(this); ResolveResult result = resolve(fieldPath.pathElements, fieldPath.pathElements.length - 1, path, context); if (result.wasSuccessful) { context = result.resolvedObject; + } else if (ignoreMissing) { + return; // nothing was found, so there's nothing to remove :shrug: } else { throw new IllegalArgumentException(result.errorMessage); } @@ -336,13 +350,13 @@ public final class IngestDocument { } else if (context instanceof IngestCtxMap map) { // optimization: handle IngestCtxMap separately from Map if (map.containsKey(leafKey)) { map.remove(leafKey); - } else { + } else if (ignoreMissing == false) { throw new IllegalArgumentException(Errors.notPresent(path, leafKey)); } } else if (context instanceof Map map) { if (map.containsKey(leafKey)) { map.remove(leafKey); - } else { + } else if (ignoreMissing == false) { throw new IllegalArgumentException(Errors.notPresent(path, leafKey)); } } else if (context instanceof List list) { @@ -353,11 +367,13 @@ public final class IngestDocument { throw new IllegalArgumentException(Errors.notInteger(path, leafKey), e); } if (index < 0 || index >= list.size()) { - throw new IllegalArgumentException(Errors.outOfBounds(path, index, list.size())); + if (ignoreMissing == false) { + throw new IllegalArgumentException(Errors.outOfBounds(path, index, list.size())); + } } else { list.remove(index); } - } else { + } else if (ignoreMissing == false) { throw new IllegalArgumentException(Errors.cannotRemove(path, leafKey, context)); } } diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java index 86aa276dff58..b7120eec3d25 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java @@ -850,6 +850,23 @@ public class IngestDocumentTests extends ESTestCase { assertThat(document.getIngestMetadata().size(), equalTo(0)); } + public void testRemoveFieldIgnoreMissing() { + document.removeField("foo", randomBoolean()); + assertThat(document.getSourceAndMetadata().size(), equalTo(10)); + assertThat(document.getSourceAndMetadata().containsKey("foo"), equalTo(false)); + document.removeField("_index", randomBoolean()); + assertThat(document.getSourceAndMetadata().size(), equalTo(9)); + assertThat(document.getSourceAndMetadata().containsKey("_index"), equalTo(false)); + + // if ignoreMissing is false, we throw an exception for values that aren't found + IllegalArgumentException e; + e = expectThrows(IllegalArgumentException.class, () -> document.removeField("fizz.some.nonsense", false)); + assertThat(e.getMessage(), is("field [some] not present as part of path [fizz.some.nonsense]")); + + // but no exception is thrown if ignoreMissing is true + document.removeField("fizz.some.nonsense", true); + } + public void testRemoveInnerField() { document.removeField("fizz.buzz"); assertThat(document.getSourceAndMetadata().size(), equalTo(11));