Externalize TemplateScript.Factory rendering from IngestDocument (#103363)

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

@@ -48,9 +48,10 @@ public final class AppendProcessor extends AbstractProcessor {
     }
 
     @Override
-    public IngestDocument execute(IngestDocument ingestDocument) throws Exception {
-        ingestDocument.appendFieldValue(field, value, allowDuplicates);
-        return ingestDocument;
+    public IngestDocument execute(IngestDocument document) throws Exception {
+        String path = document.renderTemplate(field);
+        document.appendFieldValue(path, value, allowDuplicates);
+        return document;
     }
 
     @Override

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

@@ -64,7 +64,7 @@ public final class RemoveProcessor extends AbstractProcessor {
             }
         } else {
             for (TemplateScript.Factory field : fieldsToRemove) {
-                document.removeField(field);
+                document.removeField(document.renderTemplate(field));
             }
         }
     }

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

@@ -78,12 +78,13 @@ public final class SetProcessor extends AbstractProcessor {
 
     @Override
     public IngestDocument execute(IngestDocument document) {
-        if (overrideEnabled || document.hasField(field) == false || document.getFieldValue(field, Object.class) == null) {
+        String path = document.renderTemplate(field);
+        if (overrideEnabled || document.hasField(path) == false || document.getFieldValue(path, Object.class) == null) {
             if (copyFrom != null) {
                 Object fieldValue = document.getFieldValue(copyFrom, Object.class, ignoreEmptyValue);
-                document.setFieldValue(field, IngestDocument.deepCopy(fieldValue), ignoreEmptyValue);
+                document.setFieldValue(path, IngestDocument.deepCopy(fieldValue), ignoreEmptyValue);
             } else {
-                document.setFieldValue(field, value, ignoreEmptyValue);
+                document.setFieldValue(path, value, ignoreEmptyValue);
             }
         }
         return document;

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

@@ -120,7 +120,7 @@ public class GrokProcessorTests extends ESTestCase {
     public void testNullField() {
         String fieldName = RandomDocumentPicks.randomFieldName(random());
         IngestDocument doc = RandomDocumentPicks.randomIngestDocument(random(), new HashMap<>());
-        doc.setFieldValue(fieldName, null);
+        doc.setFieldValue(fieldName, (Object) null);
         GrokProcessor processor = new GrokProcessor(
             randomAlphaOfLength(10),
             null,
@@ -138,7 +138,7 @@ public class GrokProcessorTests extends ESTestCase {
     public void testNullFieldWithIgnoreMissing() throws Exception {
         String fieldName = RandomDocumentPicks.randomFieldName(random());
         IngestDocument originalIngestDocument = RandomDocumentPicks.randomIngestDocument(random(), new HashMap<>());
-        originalIngestDocument.setFieldValue(fieldName, null);
+        originalIngestDocument.setFieldValue(fieldName, (Object) null);
         IngestDocument ingestDocument = new IngestDocument(originalIngestDocument);
         GrokProcessor processor = new GrokProcessor(
             randomAlphaOfLength(10),

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

@@ -123,7 +123,7 @@ public class RenameProcessorTests extends ESTestCase {
     public void testRenameExistingFieldNullValue() throws Exception {
         IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), new HashMap<>());
         String fieldName = RandomDocumentPicks.randomFieldName(random());
-        ingestDocument.setFieldValue(fieldName, null);
+        ingestDocument.setFieldValue(fieldName, (Object) null);
         String newFieldName = randomValueOtherThanMany(ingestDocument::hasField, () -> RandomDocumentPicks.randomFieldName(random()));
         Processor processor = createRenameProcessor(fieldName, newFieldName, false);
         processor.execute(ingestDocument);

qa/smoke-test-ingest-with-all-dependencies/src/yamlRestTest/java/org/elasticsearch/ingest/IngestDocumentMustacheIT.java

@@ -23,10 +23,13 @@ public class IngestDocumentMustacheIT extends AbstractScriptTestCase {
         Map<String, Object> document = new HashMap<>();
         document.put("foo", "bar");
         IngestDocument ingestDocument = new IngestDocument("index", "id", 1, null, null, document);
-        ingestDocument.setFieldValue(compile("field1"), ValueSource.wrap("1 {{foo}}", scriptService));
+        ingestDocument.setFieldValue(ingestDocument.renderTemplate(compile("field1")), ValueSource.wrap("1 {{foo}}", scriptService));
         assertThat(ingestDocument.getFieldValue("field1", String.class), equalTo("1 bar"));
 
-        ingestDocument.setFieldValue(compile("field1"), ValueSource.wrap("2 {{_source.foo}}", scriptService));
+        ingestDocument.setFieldValue(
+            ingestDocument.renderTemplate(compile("field1")),
+            ValueSource.wrap("2 {{_source.foo}}", scriptService)
+        );
         assertThat(ingestDocument.getFieldValue("field1", String.class), equalTo("2 bar"));
     }
 
@@ -38,11 +41,14 @@ public class IngestDocumentMustacheIT extends AbstractScriptTestCase {
         innerObject.put("qux", Collections.singletonMap("fubar", "hello qux and fubar"));
         document.put("foo", innerObject);
         IngestDocument ingestDocument = new IngestDocument("index", "id", 1, null, null, document);
-        ingestDocument.setFieldValue(compile("field1"), ValueSource.wrap("1 {{foo.bar}} {{foo.baz}} {{foo.qux.fubar}}", scriptService));
+        ingestDocument.setFieldValue(
+            ingestDocument.renderTemplate(compile("field1")),
+            ValueSource.wrap("1 {{foo.bar}} {{foo.baz}} {{foo.qux.fubar}}", scriptService)
+        );
         assertThat(ingestDocument.getFieldValue("field1", String.class), equalTo("1 hello bar hello baz hello qux and fubar"));
 
         ingestDocument.setFieldValue(
-            compile("field1"),
+            ingestDocument.renderTemplate(compile("field1")),
             ValueSource.wrap("2 {{_source.foo.bar}} {{_source.foo.baz}} {{_source.foo.qux.fubar}}", scriptService)
         );
         assertThat(ingestDocument.getFieldValue("field1", String.class), equalTo("2 hello bar hello baz hello qux and fubar"));
@@ -58,7 +64,10 @@ public class IngestDocumentMustacheIT extends AbstractScriptTestCase {
         list.add(null);
         document.put("list2", list);
         IngestDocument ingestDocument = new IngestDocument("index", "id", 1, null, null, document);
-        ingestDocument.setFieldValue(compile("field1"), ValueSource.wrap("1 {{list1.0}} {{list2.0}}", scriptService));
+        ingestDocument.setFieldValue(
+            ingestDocument.renderTemplate(compile("field1")),
+            ValueSource.wrap("1 {{list1.0}} {{list2.0}}", scriptService)
+        );
         assertThat(ingestDocument.getFieldValue("field1", String.class), equalTo("1 foo {field=value}"));
     }
 
@@ -69,7 +78,7 @@ public class IngestDocumentMustacheIT extends AbstractScriptTestCase {
         document.put("_ingest", ingestMap);
         IngestDocument ingestDocument = new IngestDocument("index", "id", 1, null, null, document);
         ingestDocument.setFieldValue(
-            compile("ingest_timestamp"),
+            ingestDocument.renderTemplate(compile("ingest_timestamp")),
             ValueSource.wrap("{{_ingest.timestamp}} and {{_source._ingest.timestamp}}", scriptService)
         );
         assertThat(

qa/smoke-test-ingest-with-all-dependencies/src/yamlRestTest/java/org/elasticsearch/ingest/ValueSourceMustacheIT.java

@@ -57,9 +57,9 @@ public class ValueSourceMustacheIT extends AbstractScriptTestCase {
     public void testAccessSourceViaTemplate() {
         IngestDocument ingestDocument = new IngestDocument("marvel", "id", 1, null, null, new HashMap<>());
         assertThat(ingestDocument.hasField("marvel"), is(false));
-        ingestDocument.setFieldValue(compile("{{_index}}"), ValueSource.wrap("{{_index}}", scriptService));
+        ingestDocument.setFieldValue(ingestDocument.renderTemplate(compile("{{_index}}")), ValueSource.wrap("{{_index}}", scriptService));
         assertThat(ingestDocument.getFieldValue("marvel", String.class), equalTo("marvel"));
-        ingestDocument.removeField(compile("{{marvel}}"));
+        ingestDocument.removeField(ingestDocument.renderTemplate(compile("{{marvel}}")));
         assertThat(ingestDocument.hasField("index"), is(false));
     }
 

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

@@ -19,6 +19,7 @@ import org.elasticsearch.index.mapper.RoutingFieldMapper;
 import org.elasticsearch.index.mapper.SourceFieldMapper;
 import org.elasticsearch.index.mapper.VersionFieldMapper;
 import org.elasticsearch.script.CtxMap;
+import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.script.TemplateScript;
 
 import java.time.ZoneOffset;
@@ -189,18 +190,6 @@ 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 of the field value
-     * @return the value for 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(TemplateScript.Factory 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.
@@ -239,16 +228,6 @@ 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(TemplateScript.Factory 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
@@ -329,15 +308,6 @@ public final class IngestDocument {
         return false;
     }
 
-    /**
-     * Removes the field identified by the provided path.
-     * @param fieldPathTemplate Resolves to the path with dot-notation within the document
-     * @throws IllegalArgumentException if the path is null, empty, invalid or if the field doesn't exist.
-     */
-    public void removeField(TemplateScript.Factory fieldPathTemplate) {
-        removeField(renderTemplate(fieldPathTemplate));
-    }
-
     /**
      * Removes the field identified by the provided path.
      * @param path the path of the field to be removed
@@ -468,17 +438,13 @@ public final class IngestDocument {
      * 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 fieldPathTemplate Resolves to the path with dot-notation within the document
+     * @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
      * @throws IllegalArgumentException if the path is null, empty or invalid.
      */
-    public void appendFieldValue(TemplateScript.Factory fieldPathTemplate, ValueSource valueSource, boolean allowDuplicates) {
-        appendFieldValue(
-            fieldPathTemplate.newInstance(templateModel).execute(),
-            valueSource.copyAndResolve(templateModel),
-            allowDuplicates
-        );
+    public void appendFieldValue(String path, ValueSource valueSource, boolean allowDuplicates) {
+        appendFieldValue(path, valueSource.copyAndResolve(templateModel), allowDuplicates);
     }
 
     /**
@@ -499,26 +465,26 @@ public final class IngestDocument {
      * Sets the provided value to the provided path in the document.
      * Any non existing path element will be created. If the last element is a list,
      * the value will replace the existing list.
-     * @param fieldPathTemplate Resolves to the path with dot-notation within the document
+     * @param path The path within the document in dot-notation
      * @param valueSource The value source that will produce the value to put in for the path key
      * @throws IllegalArgumentException if the path is null, empty, invalid or if the value cannot be set to the
      * item identified by the provided path.
      */
-    public void setFieldValue(TemplateScript.Factory fieldPathTemplate, ValueSource valueSource) {
-        setFieldValue(fieldPathTemplate.newInstance(templateModel).execute(), valueSource.copyAndResolve(templateModel));
+    public void setFieldValue(String path, ValueSource valueSource) {
+        setFieldValue(path, valueSource.copyAndResolve(templateModel));
     }
 
     /**
      * Sets the provided value to the provided path in the document.
      * Any non existing path element will be created. If the last element is a list,
      * the value will replace the existing list.
-     * @param fieldPathTemplate Resolves to the path with dot-notation within the document
+     * @param path The path within the document in dot-notation
      * @param valueSource The value source that will produce the value to put in for the path key
      * @param ignoreEmptyValue The flag to determine whether to exit quietly when the value produced by TemplatedValue is null or empty
      * @throws IllegalArgumentException if the path is null, empty, invalid or if the value cannot be set to the
      * item identified by the provided path.
      */
-    public void setFieldValue(TemplateScript.Factory fieldPathTemplate, ValueSource valueSource, boolean ignoreEmptyValue) {
+    public void setFieldValue(String path, ValueSource valueSource, boolean ignoreEmptyValue) {
         Object value = valueSource.copyAndResolve(templateModel);
         if (ignoreEmptyValue && valueSource instanceof ValueSource.TemplatedValue) {
             if (value == null) {
@@ -530,20 +496,20 @@ public final class IngestDocument {
             }
         }
 
-        setFieldValue(fieldPathTemplate.newInstance(templateModel).execute(), value);
+        setFieldValue(path, value);
     }
 
     /**
      * Sets the provided value to the provided path in the document.
      * Any non existing path element will be created. If the last element is a list,
      * the value will replace the existing list.
-     * @param fieldPathTemplate Resolves to the path with dot-notation within the document
+     * @param path The path within the document in dot-notation
      * @param value The value to put in for the path key
      * @param ignoreEmptyValue The flag to determine whether to exit quietly when the value produced by TemplatedValue is null or empty
      * @throws IllegalArgumentException if the path is null, empty, invalid or if the value cannot be set to the
      * item identified by the provided path.
      */
-    public void setFieldValue(TemplateScript.Factory fieldPathTemplate, Object value, boolean ignoreEmptyValue) {
+    public void setFieldValue(String path, Object value, boolean ignoreEmptyValue) {
         if (ignoreEmptyValue) {
             if (value == null) {
                 return;
@@ -555,7 +521,7 @@ public final class IngestDocument {
             }
         }
 
-        setFieldValue(fieldPathTemplate.newInstance(templateModel).execute(), value);
+        setFieldValue(path, value);
     }
 
     private void setFieldValue(String path, Object value, boolean append, boolean allowDuplicates) {
@@ -724,6 +690,21 @@ public final class IngestDocument {
         );
     }
 
+    /**
+     * Renders a template into a string. This allows field access via both literal fields like {@code "foo.bar.baz"} and dynamic fields
+     * like {@code "{{other_field}}"} (that is, look up the value of the 'other_field' in the document and then use the resulting string as
+     * the field to operate on).
+     * <p>
+     * See {@link ConfigurationUtils#compileTemplate(String, String, String, String, ScriptService)} and associated methods, which
+     * create these {@link TemplateScript.Factory} instances.
+     * <p>
+     * Note: for clarity and efficiency reasons, it is advisable to invoke this method outside IngestDocument itself -- fields should be
+     * rendered by a caller (once), and then passed to an ingest document repeatedly. There are enough methods on IngestDocument that
+     * operate on String paths already, we don't want to mirror all of them with twin methods that accept a template.
+     *
+     * @param template the template or literal string to evaluate
+     * @return a literal string field path
+     */
     public String renderTemplate(TemplateScript.Factory template) {
         return template.newInstance(templateModel).execute();
     }

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

@@ -339,7 +339,7 @@ public class IngestDocumentTests extends ESTestCase {
     }
 
     public void testSetFieldValueNullValue() {
-        ingestDocument.setFieldValue("new_field", null);
+        ingestDocument.setFieldValue("new_field", (Object) null);
         assertThat(ingestDocument.getSourceAndMetadata().containsKey("new_field"), equalTo(true));
         assertThat(ingestDocument.getSourceAndMetadata().get("new_field"), nullValue());
     }

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

@@ -45,7 +45,7 @@ public class ValueSourceTests extends ESTestCase {
 
         IngestDocument ingestDocument = TestIngestDocument.emptyIngestDocument();
         ingestDocument.setFieldValue(
-            new TestTemplateService.MockTemplateScript.Factory("field1"),
+            ingestDocument.renderTemplate(new TestTemplateService.MockTemplateScript.Factory("field1")),
             ValueSource.wrap(myPreciousMap, TestTemplateService.instance())
         );
         ingestDocument.removeField("field1.field2");
@@ -60,7 +60,7 @@ public class ValueSourceTests extends ESTestCase {
 
         IngestDocument ingestDocument = TestIngestDocument.emptyIngestDocument();
         ingestDocument.setFieldValue(
-            new TestTemplateService.MockTemplateScript.Factory("field1"),
+            ingestDocument.renderTemplate(new TestTemplateService.MockTemplateScript.Factory("field1")),
             ValueSource.wrap(myPreciousList, TestTemplateService.instance())
         );
         ingestDocument.removeField("field1.0");