From 53d32018f9bdb7be4ee80364cb01569d1ff38e60 Mon Sep 17 00:00:00 2001
From: Aurelien FOUCRET <aurelien.foucret@elastic.co>
Date: Tue, 28 Nov 2023 13:58:09 +0100
Subject: [PATCH] Fixing some review comments.

---
 .../script/mustache/CustomMustacheFactory.java         |  3 ++-
 .../script/mustache/CustomReflectionObjectHandler.java |  1 +
 .../script/mustache/MustacheScriptEngine.java          |  6 +++---
 .../script/mustache/MustacheScriptEngineTests.java     | 10 ++++++++++
 4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomMustacheFactory.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomMustacheFactory.java
index b7010a65b53d4..3433a9f94cfdf 100644
--- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomMustacheFactory.java
+++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomMustacheFactory.java
@@ -47,6 +47,7 @@ public final class CustomMustacheFactory extends DefaultMustacheFactory {
     static final String X_WWW_FORM_URLENCODED_MEDIA_TYPE = "application/x-www-form-urlencoded";
 
     private static final String DEFAULT_MEDIA_TYPE = JSON_MEDIA_TYPE;
+    private static final boolean DEFAULT_DETECT_MISSING_PARAMS = false;
 
     private static final Map<String, Supplier<Encoder>> ENCODERS = Map.of(
         V7_JSON_MEDIA_TYPE_WITH_CHARSET,
@@ -366,7 +367,7 @@ public void encode(String s, Writer writer) throws IOException {
      */
     static class Builder {
         private String mediaType = DEFAULT_MEDIA_TYPE;
-        private boolean detectMissingParams = false;
+        private boolean detectMissingParams = DEFAULT_DETECT_MISSING_PARAMS;
 
         private Builder() {}
 
diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomReflectionObjectHandler.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomReflectionObjectHandler.java
index 07fab2ea58420..eec3b07d66745 100644
--- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomReflectionObjectHandler.java
+++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/CustomReflectionObjectHandler.java
@@ -49,6 +49,7 @@ public Object coerce(Object object) {
         }
     }
 
+    @Override
     public Wrapper find(String name, List<Object> scopes) {
         Wrapper wrapper = super.find(name, scopes);
 
diff --git a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java
index 2fd28df3a0312..b2000307a0fd4 100644
--- a/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java
+++ b/modules/lang-mustache/src/main/java/org/elasticsearch/script/mustache/MustacheScriptEngine.java
@@ -117,7 +117,7 @@ public String execute() {
             try {
                 template.execute(writer, params);
             } catch (Exception e) {
-                if (logException(e)) {
+                if (shouldLogException(e)) {
                     logger.error(() -> format("Error running %s", template), e);
                 }
                 throw new GeneralScriptException("Error running " + template, e);
@@ -125,11 +125,11 @@ public String execute() {
             return writer.toString();
         }
 
-        public boolean logException(Throwable e) {
+        public boolean shouldLogException(Throwable e) {
             if (e instanceof InvalidParameterException) {
                 return false;
             }
-            return e.getCause() == null || logException(e.getCause());
+            return e.getCause() == null || shouldLogException(e.getCause());
         }
     }
 
diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java
index 144304d5d36ed..c3ab807fc994c 100644
--- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java
+++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java
@@ -227,6 +227,16 @@ public void testDetectMissingParam() throws IOException {
         }
     }
 
+    public void testMissingParam() throws IOException {
+        Map<String, String> scriptOptions = Collections.emptyMap();
+        String source = "{\"match\": { \"field\": \"{{query_string}}\" }";
+        TemplateScript.Factory compiled = qe.compile(null, source, TemplateScript.CONTEXT, scriptOptions);
+
+        // When the DETECT_MISSING_PARAMS_OPTION is not specified, missing variable is replaced with an empty string.
+        assertThat(compiled.newInstance(Collections.emptyMap()).execute(), equalTo("{\"match\": { \"field\": \"\" }"));
+        assertThat(compiled.newInstance(null).execute(), equalTo("{\"match\": { \"field\": \"\" }"));
+    }
+
     public void testParseTemplateAsSingleStringWithConditionalClause() throws IOException {
         String templateString = """
             {