Skip to content

Commit

Permalink
Scripting: Rename CompiledType to FactoryType in ScriptContext (#24897)
Browse files Browse the repository at this point in the history
This commit renames the concept of the "compiled type" to a "factory
type", along with all implementations of this class to be named Factory.
This brings it inline with the classes purpose.
  • Loading branch information
rjernst authored May 26, 2017
1 parent 8eab1fe commit 74e031e
Show file tree
Hide file tree
Showing 33 changed files with 131 additions and 173 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
import org.elasticsearch.search.lookup.SourceLookup;
Expand Down Expand Up @@ -300,8 +299,8 @@ Result prepareUpdateScriptRequest(ShardId shardId, UpdateRequest request, GetRes
private Map<String, Object> executeScript(Script script, Map<String, Object> ctx) {
try {
if (scriptService != null) {
ExecutableScript.Compiled compiledScript = scriptService.compile(script, ExecutableScript.UPDATE_CONTEXT);
ExecutableScript executableScript = compiledScript.newInstance(script.getParams());
ExecutableScript.Factory factory = scriptService.compile(script, ExecutableScript.UPDATE_CONTEXT);
ExecutableScript executableScript = factory.newInstance(script.getParams());
executableScript.setNextVar(ContextFields.CTX, ctx);
executableScript.run();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,43 +329,43 @@ public final Index index() {
* Compiles (or retrieves from cache) and binds the parameters to the
* provided script
*/
public final SearchScript getSearchScript(Script script, ScriptContext<SearchScript.Compiled> context) {
public final SearchScript getSearchScript(Script script, ScriptContext<SearchScript.Factory> context) {
failIfFrozen();
SearchScript.Compiled compiled = scriptService.compile(script, context);
return compiled.newInstance(script.getParams(), lookup());
SearchScript.Factory factory = scriptService.compile(script, context);
return factory.newInstance(script.getParams(), lookup());
}
/**
* Returns a lazily created {@link SearchScript} that is compiled immediately but can be pulled later once all
* parameters are available.
*/
public final Function<Map<String, Object>, SearchScript> getLazySearchScript(
Script script, ScriptContext<SearchScript.Compiled> context) {
Script script, ScriptContext<SearchScript.Factory> context) {
// TODO: this "lazy" binding can be removed once scripted metric aggs have their own contexts, which take _agg/_aggs as a parameter
failIfFrozen();
SearchScript.Compiled compiled = scriptService.compile(script, context);
return (p) -> compiled.newInstance(p, lookup());
SearchScript.Factory factory = scriptService.compile(script, context);
return (p) -> factory.newInstance(p, lookup());
}

/**
* Compiles (or retrieves from cache) and binds the parameters to the
* provided script
*/
public final ExecutableScript getExecutableScript(Script script, ScriptContext<ExecutableScript.Compiled> context) {
public final ExecutableScript getExecutableScript(Script script, ScriptContext<ExecutableScript.Factory> context) {
failIfFrozen();
ExecutableScript.Compiled compiled = scriptService.compile(script, context);
return compiled.newInstance(script.getParams());
ExecutableScript.Factory factory = scriptService.compile(script, context);
return factory.newInstance(script.getParams());
}

/**
* Returns a lazily created {@link ExecutableScript} that is compiled immediately but can be pulled later once all
* parameters are available.
*/
public final Function<Map<String, Object>, ExecutableScript> getLazyExecutableScript(
Script script, ScriptContext<ExecutableScript.Compiled> context) {
Script script, ScriptContext<ExecutableScript.Factory> context) {
// TODO: this "lazy" binding can be removed once scripted metric aggs have their own contexts, which take _agg/_aggs as a parameter
failIfFrozen();
ExecutableScript.Compiled compiled = scriptService.compile(script, context);
return compiled::newInstance;
ExecutableScript.Factory factory = scriptService.compile(script, context);
return factory::newInstance;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ public interface ExecutableScript {
*/
Object run();

interface Compiled {
interface Factory {
ExecutableScript newInstance(Map<String, Object> params);
}

ScriptContext<Compiled> CONTEXT = new ScriptContext<>("executable", Compiled.class);
ScriptContext<Factory> CONTEXT = new ScriptContext<>("executable", Factory.class);

// TODO: remove these once each has its own script interface
ScriptContext<Compiled> AGGS_CONTEXT = new ScriptContext<>("aggs_executable", Compiled.class);
ScriptContext<Compiled> UPDATE_CONTEXT = new ScriptContext<>("update", Compiled.class);
ScriptContext<Compiled> INGEST_CONTEXT = new ScriptContext<>("ingest", Compiled.class);
ScriptContext<Factory> AGGS_CONTEXT = new ScriptContext<>("aggs_executable", Factory.class);
ScriptContext<Factory> UPDATE_CONTEXT = new ScriptContext<>("update", Factory.class);
ScriptContext<Factory> INGEST_CONTEXT = new ScriptContext<>("ingest", Factory.class);
}
27 changes: 12 additions & 15 deletions core/src/main/java/org/elasticsearch/script/ScriptContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@
package org.elasticsearch.script;

import java.lang.reflect.Method;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

/**
* The information necessary to compile and run a script.
Expand All @@ -32,45 +29,45 @@
* <p>
* There are two related classes which must be supplied to construct a {@link ScriptContext}.
* <p>
* The <i>CompiledType</i> is a factory class for constructing instances of a script. The
* {@link ScriptService} returns an instance of <i>CompiledType</i> when compiling a script. This class
* The <i>FactoryType</i> is a factory class for constructing instances of a script. The
* {@link ScriptService} returns an instance of <i>FactoryType</i> when compiling a script. This class
* must be stateless so it is cacheable by the {@link ScriptService}. It must have an abstract method
* named {@code newInstance} which {@link ScriptEngine} implementations will define.
* <p>
* The <i>InstanceType</i> is a class returned by the {@code newInstance} method of the
* <i>CompiledType</i>. It is an instance of a script and may be stateful. Instances of
* <i>FactoryType</i>. It is an instance of a script and may be stateful. Instances of
* the <i>InstanceType</i> may be executed multiple times by a caller with different arguments. This
* class must have an abstract method named {@code execute} which {@link ScriptEngine} implementations
* will define.
*/
public final class ScriptContext<CompiledType> {
public final class ScriptContext<FactoryType> {

/** A unique identifier for this context. */
public final String name;

/** A factory class for constructing instances of a script. */
public final Class<CompiledType> compiledClazz;
public final Class<FactoryType> factoryClazz;

/** A class that is an instance of a script. */
public final Class<?> instanceClazz;

/** Construct a context with the related instance and compiled classes. */
public ScriptContext(String name, Class<CompiledType> compiledClazz) {
public ScriptContext(String name, Class<FactoryType> factoryClazz) {
this.name = name;
this.compiledClazz = compiledClazz;
this.factoryClazz = factoryClazz;
Method newInstanceMethod = null;
for (Method method : compiledClazz.getMethods()) {
for (Method method : factoryClazz.getMethods()) {
if (method.getName().equals("newInstance")) {
if (newInstanceMethod != null) {
throw new IllegalArgumentException("Cannot have multiple newInstance methods on CompiledType class ["
+ compiledClazz.getName() + "] for script context [" + name + "]");
throw new IllegalArgumentException("Cannot have multiple newInstance methods on FactoryType class ["
+ factoryClazz.getName() + "] for script context [" + name + "]");
}
newInstanceMethod = method;
}
}
if (newInstanceMethod == null) {
throw new IllegalArgumentException("Could not find method newInstance on CompiledType class ["
+ compiledClazz.getName() + "] for script context [" + name + "]");
throw new IllegalArgumentException("Could not find method newInstance on FactoryType class ["
+ factoryClazz.getName() + "] for script context [" + name + "]");
}
instanceClazz = newInstanceMethod.getReturnType();
}
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/org/elasticsearch/script/ScriptEngine.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ public interface ScriptEngine extends Closeable {
* @param code actual source of the script
* @param context the context this script will be used for
* @param params compile-time parameters (such as flags to the compiler)
* @return A compiled script of the CompiledType from {@link ScriptContext}
* @return A compiled script of the FactoryType from {@link ScriptContext}
*/
<CompiledType> CompiledType compile(String name, String code, ScriptContext<CompiledType> context, Map<String, String> params);
<FactoryType> FactoryType compile(String name, String code, ScriptContext<FactoryType> context, Map<String, String> params);

@Override
default void close() throws IOException {}
Expand Down
12 changes: 6 additions & 6 deletions core/src/main/java/org/elasticsearch/script/ScriptService.java
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ void setMaxCompilationsPerMinute(Integer newMaxPerMinute) {
*
* @return a compiled script which may be used to construct instances of a script for the given context
*/
public <CompiledType> CompiledType compile(Script script, ScriptContext<CompiledType> context) {
public <FactoryType> FactoryType compile(Script script, ScriptContext<FactoryType> context) {
Objects.requireNonNull(script);
Objects.requireNonNull(context);

Expand Down Expand Up @@ -292,7 +292,7 @@ public <CompiledType> CompiledType compile(Script script, ScriptContext<Compiled
Object compiledScript = cache.get(cacheKey);

if (compiledScript != null) {
return context.compiledClazz.cast(compiledScript);
return context.factoryClazz.cast(compiledScript);
}

// Synchronize so we don't compile scripts many times during multiple shards all compiling a script
Expand Down Expand Up @@ -326,14 +326,14 @@ public <CompiledType> CompiledType compile(Script script, ScriptContext<Compiled
cache.put(cacheKey, compiledScript);
}

return context.compiledClazz.cast(compiledScript);
return context.factoryClazz.cast(compiledScript);
}
}

/** Compiles a template. Note this will be moved to a separate TemplateService in the future. */
public CompiledTemplate compileTemplate(Script script, ScriptContext<ExecutableScript.Compiled> scriptContext) {
ExecutableScript.Compiled compiledScript = compile(script, scriptContext);
return params -> (String)compiledScript.newInstance(params).run();
public CompiledTemplate compileTemplate(Script script, ScriptContext<ExecutableScript.Factory> scriptContext) {
ExecutableScript.Factory factory = compile(script, scriptContext);
return params -> (String) factory.newInstance(params).run();
}

/**
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/org/elasticsearch/script/SearchScript.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ public interface SearchScript {
*/
boolean needsScores();

interface Compiled {
interface Factory {
SearchScript newInstance(Map<String, Object> params, SearchLookup lookup);
}

ScriptContext<Compiled> CONTEXT = new ScriptContext<>("search", Compiled.class);
ScriptContext<Factory> CONTEXT = new ScriptContext<>("search", Factory.class);
// TODO: remove aggs context when it has its own interface
ScriptContext<SearchScript.Compiled> AGGS_CONTEXT = new ScriptContext<>("aggs", Compiled.class);
ScriptContext<Factory> AGGS_CONTEXT = new ScriptContext<>("aggs", Factory.class);
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import org.elasticsearch.index.shard.SearchOperationListener;
import org.elasticsearch.indices.IndicesService;
import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.script.ScriptService;
import org.elasticsearch.script.SearchScript;
import org.elasticsearch.search.aggregations.AggregationInitializationException;
Expand Down Expand Up @@ -685,8 +684,8 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc
}
if (source.scriptFields() != null) {
for (org.elasticsearch.search.builder.SearchSourceBuilder.ScriptField field : source.scriptFields()) {
SearchScript.Compiled compiled = scriptService.compile(field.script(), SearchScript.CONTEXT);
SearchScript searchScript = compiled.newInstance(field.script().getParams(), context.lookup());
SearchScript.Factory factory = scriptService.compile(field.script(), SearchScript.CONTEXT);
SearchScript searchScript = factory.newInstance(field.script().getParams(), context.lookup());
context.scriptFields().add(new ScriptField(field.fieldName(), searchScript, field.ignoreFailure()));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.elasticsearch.index.query.QueryShardException;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.search.aggregations.InternalAggregation;
import org.elasticsearch.search.internal.SearchContext;

Expand Down Expand Up @@ -92,8 +91,8 @@ public void writeTo(StreamOutput out) throws IOException {

@Override
public SignificanceHeuristic rewrite(InternalAggregation.ReduceContext context) {
ExecutableScript.Compiled compiled = context.scriptService().compile(script, ExecutableScript.AGGS_CONTEXT);
return new ExecutableScriptHeuristic(script, compiled.newInstance(script.getParams()));
ExecutableScript.Factory factory = context.scriptService().compile(script, ExecutableScript.AGGS_CONTEXT);
return new ExecutableScriptHeuristic(script, factory.newInstance(script.getParams()));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.search.aggregations.InternalAggregation;
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;

Expand Down Expand Up @@ -95,9 +94,9 @@ public InternalAggregation doReduce(List<InternalAggregation> aggregations, Redu
if (firstAggregation.reduceScript.getParams() != null) {
vars.putAll(firstAggregation.reduceScript.getParams());
}
ExecutableScript.Compiled compiled = reduceContext.scriptService().compile(
ExecutableScript.Factory factory = reduceContext.scriptService().compile(
firstAggregation.reduceScript, ExecutableScript.AGGS_CONTEXT);
ExecutableScript script = compiled.newInstance(vars);
ExecutableScript script = factory.newInstance(vars);
aggregation = Collections.singletonList(script.run());
} else if (reduceContext.isFinalReduce()) {
aggregation = Collections.singletonList(aggregationObjects);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.AggregationExecutionException;
import org.elasticsearch.search.aggregations.InternalAggregation;
Expand Down Expand Up @@ -90,7 +89,7 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
(InternalMultiBucketAggregation<InternalMultiBucketAggregation, InternalMultiBucketAggregation.InternalBucket>) aggregation;
List<? extends InternalMultiBucketAggregation.InternalBucket> buckets = originalAgg.getBuckets();

ExecutableScript.Compiled compiledScript = reduceContext.scriptService().compile(script, ExecutableScript.AGGS_CONTEXT);
ExecutableScript.Factory factory = reduceContext.scriptService().compile(script, ExecutableScript.AGGS_CONTEXT);
List<InternalMultiBucketAggregation.InternalBucket> newBuckets = new ArrayList<>();
for (InternalMultiBucketAggregation.InternalBucket bucket : buckets) {
Map<String, Object> vars = new HashMap<>();
Expand All @@ -111,7 +110,7 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
if (skipBucket) {
newBuckets.add(bucket);
} else {
ExecutableScript executableScript = compiledScript.newInstance(vars);
ExecutableScript executableScript = factory.newInstance(vars);
Object returned = executableScript.run();
if (returned == null) {
newBuckets.add(bucket);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.script.ExecutableScript;
import org.elasticsearch.script.Script;
import org.elasticsearch.script.ScriptContext;
import org.elasticsearch.search.aggregations.InternalAggregation;
import org.elasticsearch.search.aggregations.InternalAggregation.ReduceContext;
import org.elasticsearch.search.aggregations.InternalMultiBucketAggregation;
Expand Down Expand Up @@ -83,7 +82,7 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
(InternalMultiBucketAggregation<InternalMultiBucketAggregation, InternalMultiBucketAggregation.InternalBucket>) aggregation;
List<? extends InternalMultiBucketAggregation.InternalBucket> buckets = originalAgg.getBuckets();

ExecutableScript.Compiled compiledScript = reduceContext.scriptService().compile(script, ExecutableScript.AGGS_CONTEXT);
ExecutableScript.Factory factory = reduceContext.scriptService().compile(script, ExecutableScript.AGGS_CONTEXT);
List<InternalMultiBucketAggregation.InternalBucket> newBuckets = new ArrayList<>();
for (InternalMultiBucketAggregation.InternalBucket bucket : buckets) {
Map<String, Object> vars = new HashMap<>();
Expand All @@ -97,7 +96,7 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
vars.put(varName, value);
}
// TODO: can we use one instance of the script for all buckets? it should be stateless?
ExecutableScript executableScript = compiledScript.newInstance(vars);
ExecutableScript executableScript = factory.newInstance(vars);
Object scriptReturnValue = executableScript.run();
final boolean keepBucket;
// TODO: WTF!!!!!
Expand Down
Loading

0 comments on commit 74e031e

Please sign in to comment.