Parse script on storage instead of on retrieval

Parsing a script on retrieval causes it to be re-parsed on every single script call, which can be very expensive for large frequently called scripts. This change switches to parsing scripts only once during store operation.

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

@@ -24,6 +24,7 @@ import org.elasticsearch.cluster.Diff;
 import org.elasticsearch.cluster.DiffableUtils;
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.common.ParsingException;
+import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -65,7 +66,7 @@ public final class ScriptMetaData implements MetaData.Custom {
         if (scriptAsBytes == null) {
             return null;
         }
-        return parseStoredScript(scriptAsBytes);
+        return scriptAsBytes.utf8ToString();
     }
 
     public static String parseStoredScript(BytesReference scriptAsBytes) {
@@ -78,6 +79,9 @@ public final class ScriptMetaData implements MetaData.Custom {
              XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {
             parser.nextToken();
             parser.nextToken();
+            if (parser.currentToken() == Token.END_OBJECT) {
+                throw new IllegalArgumentException("Empty script");
+            }
             switch (parser.currentName()) {
                 case "script":
                 case "template":
@@ -115,10 +119,8 @@ public final class ScriptMetaData implements MetaData.Custom {
                 case FIELD_NAME:
                     key = parser.currentName();
                     break;
-                case START_OBJECT:
-                    XContentBuilder contentBuilder = XContentBuilder.builder(parser.contentType().xContent());
-                    contentBuilder.copyCurrentStructure(parser);
-                    scripts.put(key, new ScriptAsBytes(contentBuilder.bytes()));
+                case VALUE_STRING:
+                    scripts.put(key, new ScriptAsBytes(new BytesArray(parser.text())));
                     break;
                 default:
                     throw new ParsingException(parser.getTokenLocation(), "Unexpected token [" + token + "]");
@@ -147,7 +149,7 @@ public final class ScriptMetaData implements MetaData.Custom {
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         for (Map.Entry<String, ScriptAsBytes> entry : scripts.entrySet()) {
-            builder.rawField(entry.getKey(), entry.getValue().script);
+            builder.field(entry.getKey(), entry.getValue().script.utf8ToString());
         }
         return builder;
     }
@@ -188,8 +190,8 @@ public final class ScriptMetaData implements MetaData.Custom {
     @Override
     public String toString() {
         return "ScriptMetaData{" +
-                "scripts=" + scripts +
-                '}';
+            "scripts=" + scripts +
+            '}';
     }
 
     static String toKey(String language, String id) {
@@ -216,7 +218,8 @@ public final class ScriptMetaData implements MetaData.Custom {
         }
 
         public Builder storeScript(String lang, String id, BytesReference script) {
-            scripts.put(toKey(lang, id), new ScriptAsBytes(script));
+            BytesReference scriptBytest = new BytesArray(parseStoredScript(script));
+            scripts.put(toKey(lang, id), new ScriptAsBytes(scriptBytest));
             return this;
         }
 

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

@@ -98,15 +98,15 @@ public class ScriptMetaDataTests extends ESTestCase {
 
     public void testDiff() throws Exception {
         ScriptMetaData.Builder builder = new ScriptMetaData.Builder(null);
-        builder.storeScript("lang", "1", new BytesArray("abc"));
-        builder.storeScript("lang", "2", new BytesArray("def"));
-        builder.storeScript("lang", "3", new BytesArray("ghi"));
+        builder.storeScript("lang", "1", new BytesArray("{\"foo\":\"abc\"}"));
+        builder.storeScript("lang", "2", new BytesArray("{\"foo\":\"def\"}"));
+        builder.storeScript("lang", "3", new BytesArray("{\"foo\":\"ghi\"}"));
         ScriptMetaData scriptMetaData1 = builder.build();
 
         builder = new ScriptMetaData.Builder(scriptMetaData1);
-        builder.storeScript("lang", "2", new BytesArray("changed"));
+        builder.storeScript("lang", "2", new BytesArray("{\"foo\":\"changed\"}"));
         builder.deleteScript("lang", "3");
-        builder.storeScript("lang", "4", new BytesArray("jkl"));
+        builder.storeScript("lang", "4", new BytesArray("{\"foo\":\"jkl\"}"));
         ScriptMetaData scriptMetaData2 = builder.build();
 
         ScriptMetaData.ScriptMetadataDiff diff = (ScriptMetaData.ScriptMetadataDiff) scriptMetaData2.diff(scriptMetaData1);
@@ -118,19 +118,19 @@ public class ScriptMetaDataTests extends ESTestCase {
         assertNotNull(((DiffableUtils.MapDiff) diff.pipelines).getUpserts().get("lang#4"));
 
         ScriptMetaData result = (ScriptMetaData) diff.apply(scriptMetaData1);
-        assertEquals(new BytesArray("abc"), result.getScriptAsBytes("lang", "1"));
-        assertEquals(new BytesArray("changed"), result.getScriptAsBytes("lang", "2"));
-        assertEquals(new BytesArray("jkl"), result.getScriptAsBytes("lang", "4"));
+        assertEquals(new BytesArray("{\"foo\":\"abc\"}"), result.getScriptAsBytes("lang", "1"));
+        assertEquals(new BytesArray("{\"foo\":\"changed\"}"), result.getScriptAsBytes("lang", "2"));
+        assertEquals(new BytesArray("{\"foo\":\"jkl\"}"), result.getScriptAsBytes("lang", "4"));
     }
 
     public void testBuilder() {
         ScriptMetaData.Builder builder = new ScriptMetaData.Builder(null);
         builder.storeScript("_lang", "_id", new BytesArray("{\"script\":\"1 + 1\"}"));
 
-        IllegalArgumentException e =
-                expectThrows(IllegalArgumentException.class, () -> builder.storeScript("_lang#", "_id", new BytesArray("{}")));
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+            () -> builder.storeScript("_lang#", "_id", new BytesArray("{\"foo\": \"bar\"}")));
         assertEquals("stored script language can't contain: '#'", e.getMessage());
-        e = expectThrows(IllegalArgumentException.class, () -> builder.storeScript("_lang", "_id#", new BytesArray("{}")));
+        e = expectThrows(IllegalArgumentException.class, () -> builder.storeScript("_lang", "_id#", new BytesArray("{\"foo\": \"bar\"}")));
         assertEquals("stored script id can't contain: '#'", e.getMessage());
         e = expectThrows(IllegalArgumentException.class, () -> builder.deleteScript("_lang#", "_id"));
         assertEquals("stored script language can't contain: '#'", e.getMessage());

core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java

@@ -429,14 +429,14 @@ public class ScriptServiceTests extends ESTestCase {
         ScriptMetaData scriptMetaData = result.getMetaData().custom(ScriptMetaData.TYPE);
         assertNotNull(scriptMetaData);
         assertEquals("abc", scriptMetaData.getScript("_lang", "_id"));
-        assertEquals(script, scriptMetaData.getScriptAsBytes("_lang", "_id"));
     }
 
     public void testDeleteScript() throws Exception {
         ClusterState cs = ClusterState.builder(new ClusterName("_name"))
                 .metaData(MetaData.builder()
                         .putCustom(ScriptMetaData.TYPE,
-                                new ScriptMetaData.Builder(null).storeScript("_lang", "_id", new BytesArray("abc")).build()))
+                                new ScriptMetaData.Builder(null).storeScript("_lang", "_id",
+                                    new BytesArray("{\"script\":\"abc\"}")).build()))
                 .build();
 
         DeleteStoredScriptRequest request = new DeleteStoredScriptRequest("_lang", "_id");