Indexed Scripts/Templates : Cleanup

This contains several cleanups to the indexed scripts.
Remove the unused FetchSourceContext from the Get request..
Add lang,_version,_id to the REST GET API.
Removes the routing from GetIndexedScriptRequest since the script index is a single shard that is replicated across all nodes.
Fix backward compatible template file reference
Before 1.3.0 on disk scripts could be referenced by requesting
````
_search/template

{
  "template" : "ondiskscript"
}
````
This was broken in 1.3.0 by requiring
````
{
  "template" :
  {
    "file" : "ondiskscript"
  }
}
````
This commit restores the previous behavior.
Remove support for preference, realtime and refresh
These parameters don't make sense anymore for indexed scripts as we always force the preference to _local and
always refresh after a Put to the indexed scripts index.

Closes elastic#7568
Closes elastic#7559
Closes elastic#7647
Closes elastic#7567

rest-api-spec/test/template/20_search.yaml

@@ -36,3 +36,8 @@
         body: {  "id" : "1", "params" : { "my_value" : "value1_foo", "my_size" : 1 } }
   - match: { hits.total: 1 }
 
+  - do:
+      catch: /ElasticsearchIllegalArgumentException.Unable.to.find.on.disk.script.simple1/
+      search_template:
+        body: { "template" : "simple1" }
+

src/main/java/org/elasticsearch/action/indexedscripts/get/GetIndexedScriptRequest.java

@@ -45,13 +45,6 @@ public class GetIndexedScriptRequest extends ActionRequest<GetIndexedScriptReque
 
     protected String scriptLang;
     protected String id;
-    protected String preference;
-    protected String routing;
-    private FetchSourceContext fetchSourceContext;
-
-    private boolean refresh = false;
-
-    Boolean realtime;
 
     private VersionType versionType = VersionType.INTERNAL;
     private long version = Versions.MATCH_ANY;
@@ -117,24 +110,6 @@ public GetIndexedScriptRequest id(String id) {
         return this;
     }
 
-    /**
-     * Controls the shard routing of the request. Using this value to hash the shard
-     * and not the id.
-     */
-    public GetIndexedScriptRequest routing(String routing) {
-        this.routing = routing;
-        return this;
-    }
-
-    /**
-     * Sets the preference to execute the get. Defaults to randomize across shards. Can be set to
-     * <tt>_local</tt> to prefer local shards, <tt>_primary</tt> to execute only on primary shards, or
-     * a custom value, which guarantees that the same order will be used across different requests.
-     */
-    public GetIndexedScriptRequest preference(String preference) {
-        this.preference = preference;
-        return this;
-    }
 
     public String scriptLang() {
         return scriptLang;
@@ -144,37 +119,6 @@ public String id() {
         return id;
     }
 
-    public String routing() {
-        return routing;
-    }
-
-    public String preference() {
-        return this.preference;
-    }
-
-    /**
-     * Should a refresh be executed before this get operation causing the operation to
-     * return the latest value. Note, heavy get should not set this to <tt>true</tt>. Defaults
-     * to <tt>false</tt>.
-     */
-    public GetIndexedScriptRequest refresh(boolean refresh) {
-        this.refresh = refresh;
-        return this;
-    }
-
-    public boolean refresh() {
-        return this.refresh;
-    }
-
-    public boolean realtime() {
-        return this.realtime == null ? true : this.realtime;
-    }
-
-    public GetIndexedScriptRequest realtime(Boolean realtime) {
-        this.realtime = realtime;
-        return this;
-    }
-
     /**
      * Sets the version, which will cause the get operation to only be performed if a matching
      * version exists and no changes happened on the doc since then.
@@ -209,19 +153,17 @@ public void readFrom(StreamInput in) throws IOException {
         }
         scriptLang = in.readString();
         id = in.readString();
-        preference = in.readOptionalString();
-        refresh = in.readBoolean();
-        byte realtime = in.readByte();
-        if (realtime == 0) {
-            this.realtime = false;
-        } else if (realtime == 1) {
-            this.realtime = true;
+        if (in.getVersion().before(Version.V_1_5_0)) {
+            in.readOptionalString(); //Preference
+            in.readBoolean(); //Refresh
+            in.readByte(); //Realtime
         }
-
         this.versionType = VersionType.fromValue(in.readByte());
         this.version = Versions.readVersionWithVLongForBW(in);
 
-        fetchSourceContext = FetchSourceContext.optionalReadFromStream(in);
+        if (in.getVersion().before(Version.V_1_5_0)) {
+            FetchSourceContext.optionalReadFromStream(in);
+        }
     }
 
     @Override
@@ -233,24 +175,23 @@ public void writeTo(StreamOutput out) throws IOException {
         }
         out.writeString(scriptLang);
         out.writeString(id);
-        out.writeOptionalString(preference);
-        out.writeBoolean(refresh);
-        if (realtime == null) {
-            out.writeByte((byte) -1);
-        } else if (!realtime) {
-            out.writeByte((byte) 0);
-        } else {
-            out.writeByte((byte) 1);
+        if (out.getVersion().before(Version.V_1_5_0)) {
+            out.writeOptionalString("_local"); //Preference
+            out.writeBoolean(true); //Refresh
+            out.writeByte((byte) -1); //Realtime
         }
 
         out.writeByte(versionType.getValue());
         Versions.writeVersionWithVLongForBW(version, out);
 
-        FetchSourceContext.optionalWriteToStream(fetchSourceContext, out);
+        if (out.getVersion().before(Version.V_1_5_0)) {
+            FetchSourceContext.optionalWriteToStream(null, out);
+        }
+
     }
 
     @Override
     public String toString() {
-        return "[" + ScriptService.SCRIPT_INDEX + "][" + scriptLang + "][" + id + "]: routing [" + routing + "]";
+        return "[" + ScriptService.SCRIPT_INDEX + "][" + scriptLang + "][" + id + "]";
     }
 }

src/main/java/org/elasticsearch/action/indexedscripts/get/GetIndexedScriptRequestBuilder.java

@@ -52,31 +52,6 @@ public GetIndexedScriptRequestBuilder setId(String id) {
         return this;
     }
 
-    /**
-     * Sets the preference to execute the search. Defaults to randomize across shards. Can be set to
-     * <tt>_local</tt> to prefer local shards, <tt>_primary</tt> to execute only on primary shards, or
-     * a custom value, which guarantees that the same order will be used across different requests.
-     */
-    public GetIndexedScriptRequestBuilder setPreference(String preference) {
-        request.preference(preference);
-        return this;
-    }
-
-    /**
-     * Should a refresh be executed before this get operation causing the operation to
-     * return the latest value. Note, heavy get should not set this to <tt>true</tt>. Defaults
-     * to <tt>false</tt>.
-     */
-    public GetIndexedScriptRequestBuilder setRefresh(boolean refresh) {
-        request.refresh(refresh);
-        return this;
-    }
-
-    public GetIndexedScriptRequestBuilder setRealtime(Boolean realtime) {
-        request.realtime(realtime);
-        return this;
-    }
-
     /**
      * Sets the version, which will cause the get operation to only be performed if a matching
      * version exists and no changes happened on the doc since then.

src/main/java/org/elasticsearch/index/query/TemplateQueryParser.java

@@ -115,9 +115,6 @@ public static TemplateContext parse(XContentParser parser, String paramsFieldnam
                 currentFieldName = parser.currentName();
             } else if (parameterMap.containsKey(currentFieldName)) {
                 type = parameterMap.get(currentFieldName);
-
-
-
                 if (token == XContentParser.Token.START_OBJECT && !parser.hasTextCharacters()) {
                     XContentBuilder builder = XContentBuilder.builder(parser.contentType().xContent());
                     builder.copyCurrentStructure(parser);

src/main/java/org/elasticsearch/rest/action/script/RestGetIndexedScriptAction.java

@@ -42,6 +42,11 @@
  */
 public class RestGetIndexedScriptAction extends BaseRestHandler {
 
+    private final static String LANG_FIELD = "lang";
+    private final static String ID_FIELD = "_id";
+    private final static String VERSION_FIELD = "_version";
+    private final static String SCRIPT_FIELD = "script";
+
     @Inject
     public RestGetIndexedScriptAction(Settings settings, RestController controller, Client client) {
         this(settings, controller, true, client);
@@ -54,14 +59,15 @@ protected RestGetIndexedScriptAction(Settings settings, RestController controlle
         }
     }
 
-    protected String getScriptLang(RestRequest request) {
-        return request.param("lang");
+    protected String getScriptFieldName() {
+        return SCRIPT_FIELD;
     }
 
-    protected String getScriptFieldName() {
-        return "script";
+    protected String getScriptLang(RestRequest request) {
+        return request.param(LANG_FIELD);
     }
 
+
     @Override
     public void handleRequest(final RestRequest request, final RestChannel channel, Client client) {
         final GetIndexedScriptRequest getRequest = new GetIndexedScriptRequest(getScriptLang(request), request.param("id"));
@@ -78,6 +84,9 @@ public RestResponse buildResponse(GetIndexedScriptResponse response) throws Exce
                         String script = response.getScript();
                         builder.startObject();
                         builder.field(getScriptFieldName(), script);
+                        builder.field(VERSION_FIELD, response.getVersion());
+                        builder.field(LANG_FIELD, response.getScriptLang());
+                        builder.field(ID_FIELD, response.getId());
                         builder.endObject();
                         return new BytesRestResponse(OK, builder);
                     } catch( IOException|ClassCastException e ){

src/main/java/org/elasticsearch/search/SearchService.java

@@ -22,6 +22,7 @@
 import com.carrotsearch.hppc.ObjectOpenHashSet;
 import com.carrotsearch.hppc.ObjectSet;
 import com.carrotsearch.hppc.cursors.ObjectCursor;
+import com.google.common.base.Charsets;
 import com.google.common.collect.ImmutableMap;
 import org.apache.lucene.index.AtomicReaderContext;
 import org.apache.lucene.index.NumericDocValues;
@@ -77,7 +78,6 @@
 import org.elasticsearch.threadpool.ThreadPool;
 
 import java.io.IOException;
-import java.nio.charset.Charset;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
@@ -614,11 +614,21 @@ private void parseTemplate(ShardSearchRequest request) {
 
                 if (templateContext.scriptType().equals(ScriptService.ScriptType.INLINE)) {
                     //Try to double parse for nested template id/file
-                    parser = XContentFactory.xContent(templateContext.template().getBytes(Charset.defaultCharset())).createParser(templateContext.template().getBytes(Charset.defaultCharset()));
-                    TemplateQueryParser.TemplateContext innerContext = TemplateQueryParser.parse(parser, "params");
-                    if (hasLength(innerContext.template()) && !innerContext.scriptType().equals(ScriptService.ScriptType.INLINE)) {
-                        //An inner template referring to a filename or id
-                        templateContext = new TemplateQueryParser.TemplateContext(innerContext.scriptType(), innerContext.template(), templateContext.params());
+                    parser = null;
+                    try {
+                        byte[] templateBytes = templateContext.template().getBytes(Charsets.UTF_8);
+                        parser = XContentFactory.xContent(templateBytes).createParser(templateBytes);
+                    } catch (ElasticsearchParseException epe) {
+                        //This was an non-nested template, the parse failure was due to this, it is safe to assume this refers to a file
+                        //for backwards compatibility and keep going
+                        templateContext = new TemplateQueryParser.TemplateContext(ScriptService.ScriptType.FILE, templateContext.template(), templateContext.params());
+                    }
+                    if (parser != null) {
+                        TemplateQueryParser.TemplateContext innerContext = TemplateQueryParser.parse(parser, "params");
+                        if (hasLength(innerContext.template()) && !innerContext.scriptType().equals(ScriptService.ScriptType.INLINE)) {
+                            //An inner template referring to a filename or id
+                            templateContext = new TemplateQueryParser.TemplateContext(innerContext.scriptType(), innerContext.template(), templateContext.params());
+                        }
                     }
                 }
             } catch (IOException e) {

src/test/java/org/elasticsearch/action/indexedscripts/get/GetIndexedScriptRequestTests.java

@@ -34,19 +34,10 @@ public class GetIndexedScriptRequestTests extends ElasticsearchTestCase {
     @Test
     public void testGetIndexedScriptRequestSerialization() throws IOException {
         GetIndexedScriptRequest request = new GetIndexedScriptRequest("lang", "id");
-        if (randomBoolean()) {
-            request.realtime(false);
-        }
-        if (randomBoolean()) {
-            request.refresh(true);
-        }
         if (randomBoolean()) {
             request.version(randomIntBetween(1, Integer.MAX_VALUE));
             request.versionType(randomFrom(VersionType.values()));
         }
-        if (randomBoolean()) {
-            request.routing(randomAsciiOfLength(randomIntBetween(1, 10)));
-        }
 
         BytesStreamOutput out = new BytesStreamOutput();
         out.setVersion(randomVersion());
@@ -59,8 +50,6 @@ public void testGetIndexedScriptRequestSerialization() throws IOException {
 
         assertThat(request2.id(), equalTo(request.id()));
         assertThat(request2.scriptLang(), equalTo(request.scriptLang()));
-        assertThat(request2.realtime(), equalTo(request.realtime()));
-        assertThat(request2.refresh(), equalTo(request.refresh()));
         assertThat(request2.version(), equalTo(request.version()));
         assertThat(request2.versionType(), equalTo(request.versionType()));
     }