Add an ignoreMissing parameter to IngestDocument's removeField method (elastic#125232) (elastic#125260)

Author: joegallo

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

@@ -58,15 +58,9 @@ public IngestDocument execute(IngestDocument document) {
     }
 
     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);
         }
     }
 
@@ -75,13 +69,7 @@ private void fieldsToKeepProcessor(IngestDocument document) {
             .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<TemplateScript.Factory> fieldsToKeep, IngestDocument document) {

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

@@ -316,15 +316,29 @@ public boolean hasField(String path, boolean failOutOfRange) {
 
     /**
      * 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);
         }
@@ -335,13 +349,13 @@ public void removeField(String path) {
         } 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) {
@@ -352,11 +366,13 @@ public void removeField(String path) {
                 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));
         }
     }

server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java

@@ -850,6 +850,23 @@ public void testRemoveField() {
         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));