Cleanup ScriptType

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

@@ -72,7 +72,7 @@ static String operationKey(ScriptContext scriptContext) {
     }
 
     static String sourceKey(ScriptType scriptType) {
-        return SCRIPT_SETTINGS_PREFIX + "." + scriptType.getScriptType();
+        return SCRIPT_SETTINGS_PREFIX + "." + scriptType.getName();
     }
 
     static String getGlobalKey(String lang, ScriptType scriptType) {

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

@@ -50,7 +50,7 @@ public class ScriptSettings {
         for (ScriptType scriptType : ScriptType.values()) {
             scriptTypeSettingMap.put(scriptType, Setting.boolSetting(
                 ScriptModes.sourceKey(scriptType),
-                scriptType.getDefaultScriptEnabled(),
+                scriptType.isDefaultEnabled(),
                 Property.NodeScope));
         }
         SCRIPT_TYPE_SETTING_MAP = Collections.unmodifiableMap(scriptTypeSettingMap);
@@ -102,7 +102,7 @@ private static List<Setting<Boolean>> languageSettings(Map<ScriptType, Setting<B
                 boolean defaultLangAndType = defaultNonFileScriptMode;
                 // Files are treated differently because they are never default-deny
                 if (ScriptType.FILE == scriptType) {
-                    defaultLangAndType = ScriptType.FILE.getDefaultScriptEnabled();
+                    defaultLangAndType = ScriptType.FILE.isDefaultEnabled();
                 }
                 final boolean defaultIfNothingSet = defaultLangAndType;
 

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

@@ -24,66 +24,117 @@
 import org.elasticsearch.common.io.stream.StreamOutput;
 
 import java.io.IOException;
-import java.util.Locale;
+import java.util.Objects;
 
 /**
- * The type of a script, more specifically where it gets loaded from:
- * - provided dynamically at request time
- * - loaded from an index
- * - loaded from file
+ * ScriptType represents the way a script is stored and retrieved from the {@link ScriptService}.
+ * It's also used to by {@link ScriptSettings} and {@link ScriptModes} to determine whether or not
+ * a {@link Script} is allowed to be executed based on both default and user-defined settings.
  */
 public enum ScriptType {
 
-    INLINE(0, "inline", "inline", false),
-    STORED(1, "id", "stored", false),
-    FILE(2, "file", "file", true);
+    /**
+     * INLINE scripts are specified in numerous queries and compiled on-the-fly.
+     * They will be cached based on the lang and code of the script.
+     * They are turned off by default for security purposes.
+     */
+    INLINE ( 0 , "inline" , new ParseField("inline")       , false ),
 
-    private final int val;
-    private final ParseField parseField;
-    private final String scriptType;
-    private final boolean defaultScriptEnabled;
+    /**
+     * STORED scripts are saved as part of the {@link org.elasticsearch.cluster.ClusterState}
+     * based on user requests.  They will be cached when they are first used in a query.
+     * They are turned off by default for security purposes.
+     */
+    STORED ( 1 , "stored" , new ParseField("stored", "id") , false ),
+
+    /**
+     * FILE scripts are loaded from disk either on start-up or on-the-fly depending on
+     * user-defined settings.  They will be compiled and cached as soon as they are loaded
+     * from disk.  They are turned on by default as they should always be safe to execute.
+     */
+    FILE   ( 2 , "file"   , new ParseField("file")         , true  );
 
+    /**
+     * Reads an int from the input stream and converts it to a {@link ScriptType}.
+     * @return The ScriptType read from the stream. Throws an {@link IllegalStateException} if no ScriptType is found based on the id.
+     */
     public static ScriptType readFrom(StreamInput in) throws IOException {
-        int scriptTypeVal = in.readVInt();
-        for (ScriptType type : values()) {
-            if (type.val == scriptTypeVal) {
-                return type;
-            }
-        }
-        throw new IllegalArgumentException("Unexpected value read for ScriptType got [" + scriptTypeVal + "] expected one of ["
-                + INLINE.val + "," + FILE.val + "," + STORED.val + "]");
-    }
+        int id = in.readVInt();
 
-    public static void writeTo(ScriptType scriptType, StreamOutput out) throws IOException{
-        if (scriptType != null) {
-            out.writeVInt(scriptType.val);
+        if (FILE.id == id) {
+            return FILE;
+        } else if (STORED.id == id) {
+            return STORED;
+        } else if (INLINE.id == id) {
+            return INLINE;
         } else {
-            out.writeVInt(INLINE.val); //Default to inline
+            throw new IllegalStateException("Error reading ScriptType id [" + id + "] from stream, expected one of [" +
+                FILE.id + " [" + FILE.name + "], " + STORED.id + " [" + STORED.name + "], " + INLINE.id + " [" + INLINE.name + "]]");
         }
     }
 
-    ScriptType(int val, String name, String scriptType, boolean defaultScriptEnabled) {
-        this.val = val;
-        this.parseField = new ParseField(name);
-        this.scriptType = scriptType;
-        this.defaultScriptEnabled = defaultScriptEnabled;
+    /**
+     * Writes an int to the output stream based on the id of the {@link ScriptType}.
+     * @param type The {@link ScriptType}.  Must not be {@code null}.
+     */
+    public static void writeTo(ScriptType type, StreamOutput out) throws IOException {
+        Objects.requireNonNull(type);
+        out.writeVInt(type.id);
     }
 
-    public ParseField getParseField() {
-        return parseField;
+    private final int id;
+    private final String name;
+    private final ParseField parseField;
+    private final boolean defaultEnabled;
+
+    /**
+     * Standard constructor.
+     * @param id A unique identifier for a type that can be read/written to a stream.
+     * @param name A unique name for a type.
+     * @param parseField Specifies the name used to parse input from queries.
+     * @param defaultEnabled Whether or not an {@link ScriptType} can be run by default.
+     */
+    ScriptType(int id, String name, ParseField parseField, boolean defaultEnabled) {
+        this.id = id;
+        this.name = name;
+        this.parseField = parseField;
+        this.defaultEnabled = defaultEnabled;
     }
 
-    public boolean getDefaultScriptEnabled() {
-        return defaultScriptEnabled;
+    /**
+     * @return The unique id for this {@link ScriptType}.
+     */
+    public int getId() {
+        return id;
     }
 
-    public String getScriptType() {
-        return scriptType;
+    /**
+     * @return The unique name for this {@link ScriptType}.
+     */
+    public String getName() {
+        return name;
     }
 
+    /**
+     * @return Specifies the name used to parse input from queries.
+     */
+    public ParseField getParseField() {
+        return parseField;
+    }
+
+    /**
+     * @return Whether or not a {@link ScriptType} can be run by default.  Note
+     * this can be potentially overriden by any {@link ScriptEngineService}.
+     */
+    public boolean isDefaultEnabled() {
+        return defaultEnabled;
+    }
+
+    /**
+     * @return The same as calling {@link #getName()}.
+     */
     @Override
     public String toString() {
-        return name().toLowerCase(Locale.ROOT);
+        return getName();
     }
-
 }

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

@@ -62,7 +62,7 @@ public void testFineGrainedSettingsDontAffectNativeScripts() throws IOException
         Settings.Builder builder = Settings.builder();
         if (randomBoolean()) {
             ScriptType scriptType = randomFrom(ScriptType.values());
-            builder.put("script" + "." + scriptType.getScriptType(), randomBoolean());
+            builder.put("script" + "." + scriptType.getName(), randomBoolean());
         } else {
             ScriptContext scriptContext = randomFrom(ScriptContext.Standard.values());
             builder.put("script" + "." + scriptContext.getKey(), randomBoolean());

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

@@ -125,7 +125,7 @@ public void testScriptTypeGenericSettings() {
         ScriptType[] randomScriptTypes = randomScriptTypesSet.toArray(new ScriptType[randomScriptTypesSet.size()]);
         Settings.Builder builder = Settings.builder();
         for (int i = 0; i < randomInt; i++) {
-            builder.put("script" + "." + randomScriptTypes[i].getScriptType(), randomScriptModes[i]);
+            builder.put("script" + "." + randomScriptTypes[i].getName(), randomScriptModes[i]);
         }
         this.scriptModes = new ScriptModes(scriptSettings, builder.build());
 

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

@@ -266,9 +266,9 @@ public void testFineGrainedSettings() throws IOException {
         Settings.Builder builder = Settings.builder();
         for (Map.Entry<ScriptType, Boolean> entry : scriptSourceSettings.entrySet()) {
             if (entry.getValue()) {
-                builder.put("script" + "." + entry.getKey().getScriptType(), "true");
+                builder.put("script" + "." + entry.getKey().getName(), "true");
             } else {
-                builder.put("script" + "." + entry.getKey().getScriptType(), "false");
+                builder.put("script" + "." + entry.getKey().getName(), "false");
             }
         }
         for (Map.Entry<ScriptContext, Boolean> entry : scriptContextSettings.entrySet()) {

docs/plugins/lang-javascript.asciidoc

@@ -120,7 +120,7 @@ GET test/_search
     "function_score": {
       "script_score": {
         "script": {
-          "id": "my_script", <2>
+          "stored": "my_script", <2>
           "lang": "javascript",
           "params": {
             "factor": 2

docs/plugins/lang-python.asciidoc

@@ -119,7 +119,7 @@ GET test/_search
     "function_score": {
       "script_score": {
         "script": {
-          "id": "my_script", <2>
+          "stored": "my_script", <2>
           "lang": "python",
           "params": {
             "factor": 2

docs/reference/modules/scripting/using.asciidoc

@@ -8,12 +8,12 @@ the same pattern:
 -------------------------------------
   "script": {
     "lang":   "...",  <1>
-    "inline" | "id" | "file": "...", <2>
+    "inline" | "stored" | "file": "...", <2>
     "params": { ... } <3>
   }
 -------------------------------------
 <1> The language the script is written in, which defaults to `painless`.
-<2> The script itself which may be specified as `inline`, `id`, or `file`.
+<2> The script itself which may be specified as `inline`, `stored`, or `file`.
 <3> Any named parameters that should be passed into the script.
 
 For example, the following script is used in a search request to return a
@@ -211,7 +211,7 @@ GET _scripts/groovy/calculate-score
 // CONSOLE
 // TEST[continued]
 
-Stored scripts can be used by specifying the `lang` and `id` parameters as follows:
+Stored scripts can be used by specifying the `lang` and `stored` parameters as follows:
 
 [source,js]
 --------------------------------------------------
@@ -221,7 +221,7 @@ GET _search
     "script": {
       "script": {
         "lang": "groovy",
-        "id":   "calculate-score",
+        "stored": "calculate-score",
         "params": {
           "my_modifier": 2
         }

docs/reference/query-dsl/template-query.asciidoc

@@ -108,7 +108,7 @@ GET /_search
 {
     "query": {
         "template": {
-            "id": "my_template", <1>
+            "stored": "my_template", <1>
             "params" : {
                 "query_string" : "all about search"
             }

modules/lang-mustache/src/test/resources/rest-api-spec/test/lang_mustache/40_template_query.yaml

@@ -44,15 +44,15 @@
       warnings:
           - '[template] query is deprecated, use search template api instead'
       search:
-        body: { "query": { "template": { "id": "1", "params": { "my_value": "value1" } } } }
+        body: { "query": { "template": { "stored": "1", "params": { "my_value": "value1" } } } }
 
   - match: { hits.total: 1 }
 
   - do:
       warnings:
           - '[template] query is deprecated, use search template api instead'
       search:
-        body: { "query": { "template": { "id": "/mustache/1", "params": { "my_value": "value1" } } } }
+        body: { "query": { "template": { "stored": "/mustache/1", "params": { "my_value": "value1" } } } }
 
   - match: { hits.total: 1 }