Indexed scripts: make sure headers are handed over to internal reques…

…ts and streamline versioning support The get, put and delete indexed script apis map to get, index and delete api and internally create those corresponding requests. We need to make sure that the original headers are handed over to the new request by passing the original request in the constructor when creating the new one. Also streamlined the support for version and version_type in the REST layer since the parameters were not consistently parsed and set to the internal java API requests. Modified the REST delete template and delete script actions to make use of a client instead of using the `ScriptService` directly. Closes #7569
elastic · Sep 4, 2014 · d41b56c · d41b56c
1 parent a89de2f
commit d41b56c
Show file tree

Hide file tree

Showing 19 changed files with 355 additions and 284 deletions.
diff --git a/rest-api-spec/api/delete_script.json b/rest-api-spec/api/delete_script.json
@@ -16,6 +16,17 @@
           "description" : "Script language",
           "required" : true
         }
+      },
+      "params" : {
+        "version": {
+          "type": "number",
+          "description": "Explicit version number for concurrency control"
+        },
+        "version_type": {
+          "type": "enum",
+          "options": ["internal", "external", "external_gte", "force"],
+          "description": "Specific version type"
+        }
       }
     },
     "body": null

diff --git a/rest-api-spec/api/delete_template.json b/rest-api-spec/api/delete_template.json
@@ -12,6 +12,17 @@
         }
       }
     },
+    "params" : {
+      "version": {
+        "type": "number",
+        "description": "Explicit version number for concurrency control"
+      },
+      "version_type": {
+        "type": "enum",
+        "options": ["internal", "external", "external_gte", "force"],
+        "description": "Specific version type"
+      }
+    },
     "body": null
   }
 }
diff --git a/rest-api-spec/api/get_script.json b/rest-api-spec/api/get_script.json
@@ -16,6 +16,17 @@
           "description" : "Script language",
           "required" : true
         }
+      },
+      "params" : {
+        "version": {
+          "type": "number",
+            "description": "Explicit version number for concurrency control"
+          },
+        "version_type": {
+          "type": "enum",
+          "options": ["internal", "external", "external_gte", "force"],
+          "description": "Specific version type"
+        }
       }
     },
     "body": null

diff --git a/rest-api-spec/api/get_template.json b/rest-api-spec/api/get_template.json
@@ -11,11 +11,19 @@
           "description" : "Template ID",
           "required" : true
         }
+      },
+      "params" : {
+        "version": {
+          "type": "number",
+          "description": "Explicit version number for concurrency control"
+        },
+        "version_type": {
+          "type": "enum",
+          "options": ["internal", "external", "external_gte", "force"],
+          "description": "Specific version type"
+        }
       }
     },
-    "body": {
-      "description" : "The document",
-      "required" : false
-    }
+    "body": null
   }
 }
diff --git a/rest-api-spec/api/put_script.json b/rest-api-spec/api/put_script.json
@@ -16,6 +16,23 @@
           "description" : "Script language",
           "required" : true
         }
+      },
+      "params" : {
+        "op_type": {
+          "type" : "enum",
+          "options" : ["index", "create"],
+          "default" : "index",
+          "description" : "Explicit operation type"
+        },
+        "version": {
+          "type": "number",
+          "description": "Explicit version number for concurrency control"
+        },
+        "version_type": {
+          "type": "enum",
+          "options": ["internal", "external", "external_gte", "force"],
+          "description": "Specific version type"
+        }
       }
     },
     "body": {

diff --git a/rest-api-spec/api/put_template.json b/rest-api-spec/api/put_template.json
@@ -13,6 +13,23 @@
         }
       }
     },
+    "params" : {
+      "op_type": {
+        "type" : "enum",
+        "options" : ["index", "create"],
+        "default" : "index",
+        "description" : "Explicit operation type"
+      },
+      "version": {
+        "type": "number",
+        "description": "Explicit version number for concurrency control"
+      },
+      "version_type": {
+        "type": "enum",
+        "options": ["internal", "external", "external_gte", "force"],
+        "description": "Specific version type"
+      }
+    },
     "body": {
       "description" : "The document",
       "required" : true

diff --git a/rest-api-spec/test/script/20_versions.yaml b/rest-api-spec/test/script/20_versions.yaml
@@ -0,0 +1,112 @@
+---
+"External version":
+
+ - do:
+      put_script:
+          lang:           groovy
+          id:             1
+          body: { "script":  "_score * doc[\"myParent.weight\"].value" }
+          version_type:   external
+          version:        0
+
+ - match:   { _version: 0 }
+
+ - do:
+      put_script:
+          lang:           groovy
+          id:             1
+          body: { "script":  "_score * doc[\"myParent.weight\"].value" }
+          version_type:   external
+          version:        5
+
+ - match:   { _version: 5 }
+
+ - do:
+      catch:      conflict
+      get_script:
+          lang:   groovy
+          id:     1
+          version: 3
+
+ - do:
+      catch:      conflict
+      delete_script:
+          lang:   groovy
+          id:     1
+          version: 3
+
+ - do:
+      get_script:
+          lang:   groovy
+          id:     1
+          version: 5
+          version_type: external
+ - is_true: script
+
+ - do:
+      get_script:
+          lang:   groovy
+          id:     1
+          version: 5
+          version_type: external_gte
+ - is_true: script
+
+ - do:
+      catch:      conflict
+      get_script:
+          lang:   groovy
+          id:     1
+          version: 10
+          version_type: external_gte
+
+ - do:
+      catch:      conflict
+      delete_script:
+          lang:   groovy
+          id:     1
+          version: 3
+          version_type: external
+
+ - do:
+      get_script:
+          lang:   groovy
+          id:     1
+          version: 5
+          version_type: force
+ - is_true: script
+
+ - do:
+      get_script:
+          lang:   groovy
+          id:     1
+          version: 10
+          version_type: force
+ - is_true: script
+
+ - do:
+      catch:             conflict
+      put_script:
+          lang:           groovy
+          id:             1
+          body: { "script":  "_score * doc[\"myParent.weight\"].value" }
+          version_type:   external
+          version:        5
+
+ - do:
+      catch:             conflict
+      put_script:
+          lang:           groovy
+          id:             1
+          body: { "script":  "_score * doc[\"myParent.weight\"].value" }
+          version_type:   external
+          version:        0
+
+ - do:
+      put_script:
+          lang:           groovy
+          id:             1
+          body: { "script":  "_score * doc[\"myParent.weight\"].value" }
+          version_type:   external
+          version:        6
+
+ - match:   { _version: 6}
diff --git a/src/main/java/org/elasticsearch/action/delete/DeleteRequest.java b/src/main/java/org/elasticsearch/action/delete/DeleteRequest.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.action.delete;
 
+import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.support.replication.ShardReplicationOperationRequest;
 import org.elasticsearch.common.Nullable;
@@ -86,6 +87,14 @@ public DeleteRequest(DeleteRequest request) {
     public DeleteRequest() {
     }
 
+    /**
+     * Creates a delete request caused by some other request, which is provided as an
+     * argument so that its headers and context can be copied to the new request
+     */
+    public DeleteRequest(ActionRequest request) {
+        super(request);
+    }
+
     @Override
     public ActionRequestValidationException validate() {
         ActionRequestValidationException validationException = super.validate();

diff --git a/src/main/java/org/elasticsearch/action/index/IndexRequest.java b/src/main/java/org/elasticsearch/action/index/IndexRequest.java
@@ -21,6 +21,7 @@
 
 import com.google.common.base.Charsets;
 import org.elasticsearch.*;
+import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.RoutingMissingException;
 import org.elasticsearch.action.TimestampParsingException;
@@ -151,6 +152,14 @@ public static OpType fromString(String sOpType) throws ElasticsearchIllegalArgum
     public IndexRequest() {
     }
 
+    /**
+     * Creates an index request caused by some other request, which is provided as an
+     * argument so that its headers and context can be copied to the new request
+     */
+    public IndexRequest(ActionRequest request) {
+        super(request);
+    }
+
     /**
      * Constructs a new index request against the specific index. The {@link #type(String)}
      * {@link #source(byte[])} must be set.

diff --git a/...va/org/elasticsearch/action/indexedscripts/delete/TransportDeleteIndexedScriptAction.java b/...va/org/elasticsearch/action/indexedscripts/delete/TransportDeleteIndexedScriptAction.java
@@ -20,11 +20,10 @@
 package org.elasticsearch.action.indexedscripts.delete;
 
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.action.delete.DeleteResponse;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.DelegatingActionListener;
-import org.elasticsearch.action.delete.DeleteResponse;
 import org.elasticsearch.action.support.HandledTransportAction;
-import org.elasticsearch.client.Client;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.script.ScriptService;
@@ -37,14 +36,12 @@
 public class TransportDeleteIndexedScriptAction extends HandledTransportAction<DeleteIndexedScriptRequest,  DeleteIndexedScriptResponse> {
 
     private final ScriptService scriptService;
-    private final Client client;
 
     @Inject
     public TransportDeleteIndexedScriptAction(Settings settings, ThreadPool threadPool, ScriptService scriptService,
-                                              Client client, TransportService transportService, ActionFilters actionFilters) {
+                                              TransportService transportService, ActionFilters actionFilters) {
         super(settings, DeleteIndexedScriptAction.NAME, threadPool, transportService, actionFilters);
         this.scriptService = scriptService;
-        this.client = client;
     }
 
     @Override
@@ -54,10 +51,10 @@ public DeleteIndexedScriptRequest newRequestInstance(){
 
     @Override
     protected void doExecute(final DeleteIndexedScriptRequest request, final ActionListener<DeleteIndexedScriptResponse> listener) {
-        scriptService.deleteScriptFromIndex(client, request.scriptLang(), request.id(), request.version(), new DelegatingActionListener<DeleteResponse, DeleteIndexedScriptResponse>(listener) {
+        scriptService.deleteScriptFromIndex(request, new DelegatingActionListener<DeleteResponse, DeleteIndexedScriptResponse>(listener) {
             @Override
             public DeleteIndexedScriptResponse getDelegatedFromInstigator(DeleteResponse deleteResponse){
-                return new DeleteIndexedScriptResponse(deleteResponse.getIndex(),deleteResponse.getType(),deleteResponse.getType(),deleteResponse.getVersion(), deleteResponse.isFound());
+                return new DeleteIndexedScriptResponse(deleteResponse.getIndex(), deleteResponse.getType(), deleteResponse.getId(), deleteResponse.getVersion(), deleteResponse.isFound());
             }
         });
     }

diff --git a/...ain/java/org/elasticsearch/action/indexedscripts/get/TransportGetIndexedScriptAction.java b/...ain/java/org/elasticsearch/action/indexedscripts/get/TransportGetIndexedScriptAction.java
@@ -23,7 +23,6 @@
 import org.elasticsearch.action.get.GetResponse;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.HandledTransportAction;
-import org.elasticsearch.client.Client;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.script.ScriptService;
@@ -36,14 +35,12 @@
 public class TransportGetIndexedScriptAction extends HandledTransportAction<GetIndexedScriptRequest, GetIndexedScriptResponse> {
 
     private final ScriptService scriptService;
-    private final Client client;
 
     @Inject
     public TransportGetIndexedScriptAction(Settings settings, ThreadPool threadPool, ScriptService scriptService,
-                                           TransportService transportService, Client client, ActionFilters actionFilters) {
+                                           TransportService transportService, ActionFilters actionFilters) {
         super(settings, GetIndexedScriptAction.NAME, threadPool,transportService,  actionFilters);
         this.scriptService = scriptService;
-        this.client = client;
     }
 
     @Override
@@ -53,7 +50,7 @@ public GetIndexedScriptRequest newRequestInstance(){
 
     @Override
     public void doExecute(GetIndexedScriptRequest request, ActionListener<GetIndexedScriptResponse> listener){
-        GetResponse scriptResponse = scriptService.queryScriptIndex(client, request.scriptLang(), request.id(), request.version(), request.versionType());
+        GetResponse scriptResponse = scriptService.queryScriptIndex(request);
         listener.onResponse(new GetIndexedScriptResponse(scriptResponse));
     }
 }
diff --git a/...ain/java/org/elasticsearch/action/indexedscripts/put/TransportPutIndexedScriptAction.java b/...ain/java/org/elasticsearch/action/indexedscripts/put/TransportPutIndexedScriptAction.java
@@ -24,7 +24,6 @@
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.DelegatingActionListener;
 import org.elasticsearch.action.support.HandledTransportAction;
-import org.elasticsearch.client.Client;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.script.ScriptService;
@@ -37,14 +36,11 @@
 public class TransportPutIndexedScriptAction extends HandledTransportAction<PutIndexedScriptRequest, PutIndexedScriptResponse> {
 
     private final ScriptService scriptService;
-    private final Client client;
 
     @Inject
     public TransportPutIndexedScriptAction(Settings settings, ThreadPool threadPool,
-                                           ScriptService scriptService, Client client,
-                                           TransportService transportService, ActionFilters actionFilters) {
+                                           ScriptService scriptService, TransportService transportService, ActionFilters actionFilters) {
         super(settings, PutIndexedScriptAction.NAME, threadPool, transportService, actionFilters);
-        this.client = client;
         this.scriptService = scriptService;
     }
 
@@ -55,12 +51,11 @@ public PutIndexedScriptRequest newRequestInstance(){
 
     @Override
     protected void doExecute(final PutIndexedScriptRequest request, final ActionListener<PutIndexedScriptResponse> listener) {
-        scriptService.putScriptToIndex(client, request.safeSource(), request.scriptLang(), request.id(), null, request.opType().toString(), request.version(), request.versionType(), new DelegatingActionListener<IndexResponse,PutIndexedScriptResponse>(listener) {
+        scriptService.putScriptToIndex(request, new DelegatingActionListener<IndexResponse,PutIndexedScriptResponse>(listener) {
             @Override
             public PutIndexedScriptResponse getDelegatedFromInstigator(IndexResponse indexResponse){
                 return new PutIndexedScriptResponse(indexResponse.getType(),indexResponse.getId(),indexResponse.getVersion(),indexResponse.isCreated());
             }
         });
     }
-
 }