Skip to content

Cleanup ScriptService & friends in preparation for #6418 #9992

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
javanna merged 1 commit into from
Mar 7, 2015
Merged

Cleanup ScriptService & friends in preparation for #6418 #9992

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/main/java/org/elasticsearch/index/query/ScriptFilterParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
HashedBytesRef cacheKey = null;
// also, when caching, since its isCacheable is false, will result in loading all bit set...
String script = null;
String scriptLang = null;
String scriptLang;
Map<String, Object> params = null;

String filterName = null;
Expand Down Expand Up @@ -130,12 +130,9 @@ public static class ScriptFilter extends Filter {

private final SearchScript searchScript;

private final ScriptService.ScriptType scriptType;

public ScriptFilter(String scriptLang, String script, ScriptService.ScriptType scriptType, Map<String, Object> params, ScriptService scriptService, SearchLookup searchLookup) {
this.script = script;
this.params = params;
this.scriptType = scriptType;
this.searchScript = scriptService.search(searchLookup, scriptLang, script, scriptType, newHashMap(params));
}

Expand Down
1 change: 0 additions & 1 deletion src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.elasticsearch.ElasticsearchIllegalArgumentException;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.search.aggregations.support.ValuesSource;

import java.io.IOException;
import java.util.Map;
Expand Down
15 changes: 6 additions & 9 deletions src/main/java/org/elasticsearch/index/query/TemplateQueryParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,15 @@
package org.elasticsearch.index.query;

import org.apache.lucene.search.Query;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.logging.ESLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.script.mustache.MustacheScriptEngineService;

import java.io.IOException;
import java.util.HashMap;
Expand All @@ -52,9 +50,9 @@ public class TemplateQueryParser implements QueryParser {

private final static Map<String,ScriptService.ScriptType> parametersToTypes = new HashMap<>();
static {
parametersToTypes.put("query",ScriptService.ScriptType.INLINE);
parametersToTypes.put("file",ScriptService.ScriptType.FILE);
parametersToTypes.put("id",ScriptService.ScriptType.INDEXED);
parametersToTypes.put("query", ScriptService.ScriptType.INLINE);
parametersToTypes.put("file", ScriptService.ScriptType.FILE);
parametersToTypes.put("id", ScriptService.ScriptType.INDEXED);
Copy link
Contributor

@colings86 colings86 Mar 5, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not something for this PR but we now have a convention of using script for inline, script_file for file and script_id for indexed scripts (using the ScriptParameterParser to maintain the consistency). Should we follow that convention here? I can see we may want to keep the inline as query but maybe we should also support script for inline? and file and id can probably be replaced with script_file and script_id?

Copy link
Member Author

@javanna javanna Mar 5, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good to me, patches welcome :)

Copy link
Contributor

@colings86 colings86 Mar 5, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened #9995 for this :)

}

@Inject
Expand All @@ -78,15 +76,14 @@ public String[] names() {
public Query parse(QueryParseContext parseContext) throws IOException {
XContentParser parser = parseContext.parser();
TemplateContext templateContext = parse(parser, PARAMS, parametersToTypes);
ExecutableScript executable = this.scriptService.executable("mustache", templateContext.template(), templateContext.scriptType(), templateContext.params());
ExecutableScript executable = this.scriptService.executable(MustacheScriptEngineService.NAME, templateContext.template(), templateContext.scriptType(), templateContext.params());

BytesReference querySource = (BytesReference) executable.run();

try (XContentParser qSourceParser = XContentFactory.xContent(querySource).createParser(querySource)) {
final QueryParseContext context = new QueryParseContext(parseContext.index(), parseContext.indexQueryParserService());
context.reset(qSourceParser);
Query result = context.parseInnerQuery();
return result;
return context.parseInnerQuery();
}
}

Expand Down
5 changes: 1 addition & 4 deletions ...in/java/org/elasticsearch/index/query/functionscore/script/ScriptScoreFunctionParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ public String[] getNames() {
public ScoreFunction parse(QueryParseContext parseContext, XContentParser parser) throws IOException, QueryParsingException {
ScriptParameterParser scriptParameterParser = new ScriptParameterParser();
String script = null;
String scriptLang = null;
Map<String, Object> vars = null;
ScriptService.ScriptType scriptType = null;
String currentFieldName = null;
Expand All @@ -82,15 +81,13 @@ public ScoreFunction parse(QueryParseContext parseContext, XContentParser parser
script = scriptValue.script();
scriptType = scriptValue.scriptType();
}
scriptLang = scriptParameterParser.lang();

if (script == null) {
throw new QueryParsingException(parseContext.index(), NAMES[0] + " requires 'script' field");
}

SearchScript searchScript;
try {
searchScript = parseContext.scriptService().search(parseContext.lookup(), scriptLang, script, scriptType, vars);
searchScript = parseContext.scriptService().search(parseContext.lookup(), scriptParameterParser.lang(), script, scriptType, vars);
return new ScriptScoreFunction(script, vars, searchScript);
} catch (Exception e) {
throw new QueryParsingException(parseContext.index(), NAMES[0] + " the script could not be loaded", e);
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/elasticsearch/rest/action/template/RestDeleteSearchTemplateAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.script.RestDeleteIndexedScriptAction;
import org.elasticsearch.script.mustache.MustacheScriptEngineService;

import static org.elasticsearch.rest.RestRequest.Method.DELETE;

Expand All @@ -37,6 +38,6 @@ public RestDeleteSearchTemplateAction(Settings settings, RestController controll

@Override
protected String getScriptLang(RestRequest request) {
return "mustache";
return MustacheScriptEngineService.NAME;
}
}
3 changes: 2 additions & 1 deletion src/main/java/org/elasticsearch/rest/action/template/RestGetSearchTemplateAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.rest.RestController;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.script.RestGetIndexedScriptAction;
import org.elasticsearch.script.mustache.MustacheScriptEngineService;

import static org.elasticsearch.rest.RestRequest.Method.GET;

Expand All @@ -40,7 +41,7 @@ public RestGetSearchTemplateAction(Settings settings, RestController controller,

@Override
protected String getScriptLang(RestRequest request) {
return "mustache";
return MustacheScriptEngineService.NAME;
}

@Override
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/elasticsearch/rest/action/template/RestPutSearchTemplateAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.rest.*;
import org.elasticsearch.rest.action.script.RestPutIndexedScriptAction;
import org.elasticsearch.script.mustache.MustacheScriptEngineService;

import static org.elasticsearch.rest.RestRequest.Method.POST;
import static org.elasticsearch.rest.RestRequest.Method.PUT;
Expand Down Expand Up @@ -58,6 +59,6 @@ public void handleRequest(RestRequest request, RestChannel channel, final Client

@Override
protected String getScriptLang(RestRequest request) {
return "mustache";
return MustacheScriptEngineService.NAME;
}
}
4 changes: 3 additions & 1 deletion src/main/java/org/elasticsearch/script/NativeScriptEngineService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
*/
public class NativeScriptEngineService extends AbstractComponent implements ScriptEngineService {

public static final String NAME = "native";

private final ImmutableMap<String, NativeScriptFactory> scripts;

@Inject
Expand All @@ -44,7 +46,7 @@ public NativeScriptEngineService(Settings settings, Map<String, NativeScriptFact

@Override
public String[] types() {
return new String[]{"native"};
return new String[]{NAME};
}

@Override
Expand Down
Loading