Deprecate Empty Templates (#30194)

Deprecate the use of empty templates.  Bug fix allows empty
templates/scripts to be loaded on start up for upgrades/restarts, 
but empty templates can no longer be created.

server/src/main/java/org/elasticsearch/script/ScriptMetaData.java

@@ -29,6 +29,8 @@ import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.logging.DeprecationLogger;
+import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -46,6 +48,11 @@ import java.util.Map;
  */
 public final class ScriptMetaData implements MetaData.Custom, Writeable, ToXContentFragment {
 
+    /**
+     * Standard deprecation logger for used to deprecate allowance of empty templates.
+     */
+    private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(ScriptMetaData.class));
+
     /**
      * A builder used to modify the currently stored scripts data held within
      * the {@link ClusterState}.  Scripts can be added or deleted, then built
@@ -161,8 +168,8 @@ public final class ScriptMetaData implements MetaData.Custom, Writeable, ToXCont
      *
      * {@code
      * {
-     *     "<id>" : "<{@link StoredScriptSource#fromXContent(XContentParser)}>",
+     *     "<id>" : "<{@link StoredScriptSource#fromXContent(XContentParser, boolean)}>",
-     *     "<id>" : "<{@link StoredScriptSource#fromXContent(XContentParser)}>",
+     *     "<id>" : "<{@link StoredScriptSource#fromXContent(XContentParser, boolean)}>",
      *     ...
      * }
      * }
@@ -209,6 +216,14 @@ public final class ScriptMetaData implements MetaData.Custom, Writeable, ToXCont
                         lang = id.substring(0, split);
                         id = id.substring(split + 1);
                         source = new StoredScriptSource(lang, parser.text(), Collections.emptyMap());
+
+                        if (source.getSource().isEmpty()) {
+                            if (source.getLang().equals(Script.DEFAULT_TEMPLATE_LANG)) {
+                                DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
+                            } else {
+                                DEPRECATION_LOGGER.deprecated("empty scripts should no longer be used");
+                            }
+                        }
                     }
 
                     exists = scripts.get(id);
@@ -231,7 +246,7 @@ public final class ScriptMetaData implements MetaData.Custom, Writeable, ToXCont
                     }
 
                     exists = scripts.get(id);
-                    source = StoredScriptSource.fromXContent(parser);
+                    source = StoredScriptSource.fromXContent(parser, true);
 
                     if (exists == null) {
                         scripts.put(id, source);

server/src/main/java/org/elasticsearch/script/StoredScriptSource.java

@@ -32,6 +32,8 @@ import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.logging.DeprecationLogger;
+import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.ObjectParser;
@@ -57,6 +59,11 @@ import java.util.Objects;
  */
 public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> implements Writeable, ToXContentObject {
 
+    /**
+     * Standard deprecation logger for used to deprecate allowance of empty templates.
+     */
+    private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(StoredScriptSource.class));
+
     /**
      * Standard {@link ParseField} for outer level of stored script source.
      */
@@ -109,7 +116,7 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
         private void setSource(XContentParser parser) {
             try {
                 if (parser.currentToken() == Token.START_OBJECT) {
-                    //this is really for search templates, that need to be converted to json format
+                    // this is really for search templates, that need to be converted to json format
                     XContentBuilder builder = XContentFactory.jsonBuilder();
                     source = Strings.toString(builder.copyCurrentStructure(parser));
                     options.put(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType());
@@ -131,8 +138,12 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
 
         /**
          * Validates the parameters and creates an {@link StoredScriptSource}.
+         *
+         * @param ignoreEmpty Specify as {@code true} to ignoreEmpty the empty source check.
+         *                    This allow empty templates to be loaded for backwards compatibility.
+         *                    This allow empty templates to be loaded for backwards compatibility.
          */
-        private StoredScriptSource build() {
+        private StoredScriptSource build(boolean ignoreEmpty) {
             if (lang == null) {
                 throw new IllegalArgumentException("must specify lang for stored script");
             } else if (lang.isEmpty()) {
@@ -140,9 +151,25 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
             }
 
             if (source == null) {
-                throw new IllegalArgumentException("must specify source for stored script");
+                if (ignoreEmpty || Script.DEFAULT_TEMPLATE_LANG.equals(lang)) {
+                    if (Script.DEFAULT_TEMPLATE_LANG.equals(lang)) {
+                        DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
+                    } else {
+                        DEPRECATION_LOGGER.deprecated("empty scripts should no longer be used");
+                    }
+                } else {
+                    throw new IllegalArgumentException("must specify source for stored script");
+                }
             } else if (source.isEmpty()) {
-                throw new IllegalArgumentException("source cannot be empty");
+                if (ignoreEmpty || Script.DEFAULT_TEMPLATE_LANG.equals(lang)) {
+                    if (Script.DEFAULT_TEMPLATE_LANG.equals(lang)) {
+                        DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
+                    } else {
+                        DEPRECATION_LOGGER.deprecated("empty scripts should no longer be used");
+                    }
+                } else {
+                    throw new IllegalArgumentException("source cannot be empty");
+                }
             }
 
             if (options.size() > 1 || options.size() == 1 && options.get(Script.CONTENT_TYPE_OPTION) == null) {
@@ -257,6 +284,8 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
             token = parser.nextToken();
 
             if (token == Token.END_OBJECT) {
+                DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
+
                 return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());
             }
 
@@ -271,7 +300,7 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
                 token = parser.nextToken();
 
                 if (token == Token.START_OBJECT) {
-                    return PARSER.apply(parser, null).build();
+                    return PARSER.apply(parser, null).build(false);
                 } else {
                     throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [{, <source>]");
                 }
@@ -280,7 +309,13 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
                     token = parser.nextToken();
 
                     if (token == Token.VALUE_STRING) {
-                        return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, parser.text(), Collections.emptyMap());
+                        String source = parser.text();
+
+                        if (source == null || source.isEmpty()) {
+                            DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
+                        }
+
+                        return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap());
                     }
                 }
 
@@ -293,7 +328,13 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
                         builder.copyCurrentStructure(parser);
                     }
 
-                    return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, Strings.toString(builder), Collections.emptyMap());
+                    String source = Strings.toString(builder);
+
+                    if (source == null || source.isEmpty()) {
+                        DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
+                    }
+
+                    return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap());
                 }
             }
         } catch (IOException ioe) {
@@ -320,9 +361,12 @@ public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> imp
      *
      * Note that the "source" parameter can also handle template parsing including from
      * a complex JSON object.
+     *
+     * @param ignoreEmpty Specify as {@code true} to ignoreEmpty the empty source check.
+     *                    This allows empty templates to be loaded for backwards compatibility.
      */
-    public static StoredScriptSource fromXContent(XContentParser parser) {
+    public static StoredScriptSource fromXContent(XContentParser parser, boolean ignoreEmpty) {
-        return PARSER.apply(parser, null).build();
+        return PARSER.apply(parser, null).build(ignoreEmpty);
     }
 
     /**

server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java

@@ -22,6 +22,8 @@ import org.elasticsearch.cluster.DiffableUtils;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.Writeable;
+import org.elasticsearch.common.logging.DeprecationLogger;
+import org.elasticsearch.common.logging.Loggers;
 import org.elasticsearch.common.xcontent.DeprecationHandler;
 import org.elasticsearch.common.xcontent.NamedXContentRegistry;
 import org.elasticsearch.common.xcontent.XContentBuilder;
@@ -130,6 +132,45 @@ public class ScriptMetaDataTests extends AbstractSerializingTestCase<ScriptMetaD
         assertEquals("1 + 1", result.getStoredScript("_id").getSource());
     }
 
+    public void testLoadEmptyScripts() throws IOException {
+        XContentBuilder builder = XContentFactory.jsonBuilder();
+        builder.startObject().field("mustache#empty", "").endObject();
+        XContentParser parser = XContentType.JSON.xContent()
+            .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
+                BytesReference.bytes(builder).streamInput());
+        ScriptMetaData.fromXContent(parser);
+        assertWarnings("empty templates should no longer be used");
+
+        builder = XContentFactory.jsonBuilder();
+        builder.startObject().field("lang#empty", "").endObject();
+        parser = XContentType.JSON.xContent()
+            .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
+                BytesReference.bytes(builder).streamInput());
+        ScriptMetaData.fromXContent(parser);
+        assertWarnings("empty scripts should no longer be used");
+
+        builder = XContentFactory.jsonBuilder();
+        builder.startObject().startObject("script").field("lang", "lang").field("source", "").endObject().endObject();
+        parser = XContentType.JSON.xContent()
+            .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
+                BytesReference.bytes(builder).streamInput());
+        ScriptMetaData.fromXContent(parser);
+        assertWarnings("empty scripts should no longer be used");
+
+        builder = XContentFactory.jsonBuilder();
+        builder.startObject().startObject("script").field("lang", "mustache").field("source", "").endObject().endObject();
+        parser = XContentType.JSON.xContent()
+            .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
+                BytesReference.bytes(builder).streamInput());
+        ScriptMetaData.fromXContent(parser);
+        assertWarnings("empty templates should no longer be used");
+    }
+
+    @Override
+    protected boolean enableWarningsCheck() {
+        return true;
+    }
+
     private ScriptMetaData randomScriptMetaData(XContentType sourceContentType, int minNumberScripts) throws IOException {
         ScriptMetaData.Builder builder = new ScriptMetaData.Builder(null);
         int numScripts = scaledRandomIntBetween(minNumberScripts, 32);

server/src/test/java/org/elasticsearch/script/StoredScriptSourceTests.java

@@ -58,7 +58,7 @@ public class StoredScriptSourceTests extends AbstractSerializingTestCase<StoredS
 
     @Override
     protected StoredScriptSource doParseInstance(XContentParser parser) {
-        return StoredScriptSource.fromXContent(parser);
+        return StoredScriptSource.fromXContent(parser, false);
     }
 
     @Override

server/src/test/java/org/elasticsearch/script/StoredScriptTests.java

@@ -29,6 +29,7 @@ import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.test.AbstractSerializingTestCase;
 
+import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -204,6 +205,39 @@ public class StoredScriptTests extends AbstractSerializingTestCase<StoredScriptS
         }
     }
 
+    public void testEmptyTemplateDeprecations() throws IOException {
+        try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {
+            builder.startObject().endObject();
+
+            StoredScriptSource parsed = StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON);
+            StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());
+
+            assertThat(parsed, equalTo(source));
+            assertWarnings("empty templates should no longer be used");
+        }
+
+        try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {
+            builder.startObject().field("template", "").endObject();
+
+            StoredScriptSource parsed = StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON);
+            StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());
+
+            assertThat(parsed, equalTo(source));
+            assertWarnings("empty templates should no longer be used");
+        }
+
+        try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {
+            builder.startObject().field("script").startObject().field("lang", "mustache")
+                    .field("source", "").endObject().endObject();
+
+            StoredScriptSource parsed = StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON);
+            StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());
+
+            assertThat(parsed, equalTo(source));
+            assertWarnings("empty templates should no longer be used");
+        }
+    }
+
     @Override
     protected StoredScriptSource createTestInstance() {
         return new StoredScriptSource(
@@ -219,7 +253,7 @@ public class StoredScriptTests extends AbstractSerializingTestCase<StoredScriptS
 
     @Override
     protected StoredScriptSource doParseInstance(XContentParser parser) {
-        return StoredScriptSource.fromXContent(parser);
+        return StoredScriptSource.fromXContent(parser, false);
     }
 
     @Override