Skip to content

Commit 74e031e

Browse files
authored
Scripting: Rename CompiledType to FactoryType in ScriptContext (elastic#24897)
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.
1 parent 8eab1fe commit 74e031e

File tree

33 files changed

+131
-173
lines changed

33 files changed

+131
-173
lines changed

core/src/main/java/org/elasticsearch/action/update/UpdateHelper.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import org.elasticsearch.index.shard.ShardId;
4747
import org.elasticsearch.script.ExecutableScript;
4848
import org.elasticsearch.script.Script;
49-
import org.elasticsearch.script.ScriptContext;
5049
import org.elasticsearch.script.ScriptService;
5150
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
5251
import org.elasticsearch.search.lookup.SourceLookup;
@@ -300,8 +299,8 @@ Result prepareUpdateScriptRequest(ShardId shardId, UpdateRequest request, GetRes
300299
private Map<String, Object> executeScript(Script script, Map<String, Object> ctx) {
301300
try {
302301
if (scriptService != null) {
303-
ExecutableScript.Compiled compiledScript = scriptService.compile(script, ExecutableScript.UPDATE_CONTEXT);
304-
ExecutableScript executableScript = compiledScript.newInstance(script.getParams());
302+
ExecutableScript.Factory factory = scriptService.compile(script, ExecutableScript.UPDATE_CONTEXT);
303+
ExecutableScript executableScript = factory.newInstance(script.getParams());
305304
executableScript.setNextVar(ContextFields.CTX, ctx);
306305
executableScript.run();
307306
}

core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java

+12-12
Original file line numberDiff line numberDiff line change
@@ -329,43 +329,43 @@ public final Index index() {
329329
* Compiles (or retrieves from cache) and binds the parameters to the
330330
* provided script
331331
*/
332-
public final SearchScript getSearchScript(Script script, ScriptContext<SearchScript.Compiled> context) {
332+
public final SearchScript getSearchScript(Script script, ScriptContext<SearchScript.Factory> context) {
333333
failIfFrozen();
334-
SearchScript.Compiled compiled = scriptService.compile(script, context);
335-
return compiled.newInstance(script.getParams(), lookup());
334+
SearchScript.Factory factory = scriptService.compile(script, context);
335+
return factory.newInstance(script.getParams(), lookup());
336336
}
337337
/**
338338
* Returns a lazily created {@link SearchScript} that is compiled immediately but can be pulled later once all
339339
* parameters are available.
340340
*/
341341
public final Function<Map<String, Object>, SearchScript> getLazySearchScript(
342-
Script script, ScriptContext<SearchScript.Compiled> context) {
342+
Script script, ScriptContext<SearchScript.Factory> context) {
343343
// TODO: this "lazy" binding can be removed once scripted metric aggs have their own contexts, which take _agg/_aggs as a parameter
344344
failIfFrozen();
345-
SearchScript.Compiled compiled = scriptService.compile(script, context);
346-
return (p) -> compiled.newInstance(p, lookup());
345+
SearchScript.Factory factory = scriptService.compile(script, context);
346+
return (p) -> factory.newInstance(p, lookup());
347347
}
348348

349349
/**
350350
* Compiles (or retrieves from cache) and binds the parameters to the
351351
* provided script
352352
*/
353-
public final ExecutableScript getExecutableScript(Script script, ScriptContext<ExecutableScript.Compiled> context) {
353+
public final ExecutableScript getExecutableScript(Script script, ScriptContext<ExecutableScript.Factory> context) {
354354
failIfFrozen();
355-
ExecutableScript.Compiled compiled = scriptService.compile(script, context);
356-
return compiled.newInstance(script.getParams());
355+
ExecutableScript.Factory factory = scriptService.compile(script, context);
356+
return factory.newInstance(script.getParams());
357357
}
358358

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

371371
/**

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

+5-5
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ public interface ExecutableScript {
4141
*/
4242
Object run();
4343

44-
interface Compiled {
44+
interface Factory {
4545
ExecutableScript newInstance(Map<String, Object> params);
4646
}
4747

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

5050
// TODO: remove these once each has its own script interface
51-
ScriptContext<Compiled> AGGS_CONTEXT = new ScriptContext<>("aggs_executable", Compiled.class);
52-
ScriptContext<Compiled> UPDATE_CONTEXT = new ScriptContext<>("update", Compiled.class);
53-
ScriptContext<Compiled> INGEST_CONTEXT = new ScriptContext<>("ingest", Compiled.class);
51+
ScriptContext<Factory> AGGS_CONTEXT = new ScriptContext<>("aggs_executable", Factory.class);
52+
ScriptContext<Factory> UPDATE_CONTEXT = new ScriptContext<>("update", Factory.class);
53+
ScriptContext<Factory> INGEST_CONTEXT = new ScriptContext<>("ingest", Factory.class);
5454
}

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

+12-15
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,6 @@
2020
package org.elasticsearch.script;
2121

2222
import java.lang.reflect.Method;
23-
import java.util.Collections;
24-
import java.util.HashMap;
25-
import java.util.Map;
2623

2724
/**
2825
* The information necessary to compile and run a script.
@@ -32,45 +29,45 @@
3229
* <p>
3330
* There are two related classes which must be supplied to construct a {@link ScriptContext}.
3431
* <p>
35-
* The <i>CompiledType</i> is a factory class for constructing instances of a script. The
36-
* {@link ScriptService} returns an instance of <i>CompiledType</i> when compiling a script. This class
32+
* The <i>FactoryType</i> is a factory class for constructing instances of a script. The
33+
* {@link ScriptService} returns an instance of <i>FactoryType</i> when compiling a script. This class
3734
* must be stateless so it is cacheable by the {@link ScriptService}. It must have an abstract method
3835
* named {@code newInstance} which {@link ScriptEngine} implementations will define.
3936
* <p>
4037
* The <i>InstanceType</i> is a class returned by the {@code newInstance} method of the
41-
* <i>CompiledType</i>. It is an instance of a script and may be stateful. Instances of
38+
* <i>FactoryType</i>. It is an instance of a script and may be stateful. Instances of
4239
* the <i>InstanceType</i> may be executed multiple times by a caller with different arguments. This
4340
* class must have an abstract method named {@code execute} which {@link ScriptEngine} implementations
4441
* will define.
4542
*/
46-
public final class ScriptContext<CompiledType> {
43+
public final class ScriptContext<FactoryType> {
4744

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

5148
/** A factory class for constructing instances of a script. */
52-
public final Class<CompiledType> compiledClazz;
49+
public final Class<FactoryType> factoryClazz;
5350

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

5754
/** Construct a context with the related instance and compiled classes. */
58-
public ScriptContext(String name, Class<CompiledType> compiledClazz) {
55+
public ScriptContext(String name, Class<FactoryType> factoryClazz) {
5956
this.name = name;
60-
this.compiledClazz = compiledClazz;
57+
this.factoryClazz = factoryClazz;
6158
Method newInstanceMethod = null;
62-
for (Method method : compiledClazz.getMethods()) {
59+
for (Method method : factoryClazz.getMethods()) {
6360
if (method.getName().equals("newInstance")) {
6461
if (newInstanceMethod != null) {
65-
throw new IllegalArgumentException("Cannot have multiple newInstance methods on CompiledType class ["
66-
+ compiledClazz.getName() + "] for script context [" + name + "]");
62+
throw new IllegalArgumentException("Cannot have multiple newInstance methods on FactoryType class ["
63+
+ factoryClazz.getName() + "] for script context [" + name + "]");
6764
}
6865
newInstanceMethod = method;
6966
}
7067
}
7168
if (newInstanceMethod == null) {
72-
throw new IllegalArgumentException("Could not find method newInstance on CompiledType class ["
73-
+ compiledClazz.getName() + "] for script context [" + name + "]");
69+
throw new IllegalArgumentException("Could not find method newInstance on FactoryType class ["
70+
+ factoryClazz.getName() + "] for script context [" + name + "]");
7471
}
7572
instanceClazz = newInstanceMethod.getReturnType();
7673
}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ public interface ScriptEngine extends Closeable {
3939
* @param code actual source of the script
4040
* @param context the context this script will be used for
4141
* @param params compile-time parameters (such as flags to the compiler)
42-
* @return A compiled script of the CompiledType from {@link ScriptContext}
42+
* @return A compiled script of the FactoryType from {@link ScriptContext}
4343
*/
44-
<CompiledType> CompiledType compile(String name, String code, ScriptContext<CompiledType> context, Map<String, String> params);
44+
<FactoryType> FactoryType compile(String name, String code, ScriptContext<FactoryType> context, Map<String, String> params);
4545

4646
@Override
4747
default void close() throws IOException {}

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ void setMaxCompilationsPerMinute(Integer newMaxPerMinute) {
221221
*
222222
* @return a compiled script which may be used to construct instances of a script for the given context
223223
*/
224-
public <CompiledType> CompiledType compile(Script script, ScriptContext<CompiledType> context) {
224+
public <FactoryType> FactoryType compile(Script script, ScriptContext<FactoryType> context) {
225225
Objects.requireNonNull(script);
226226
Objects.requireNonNull(context);
227227

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

294294
if (compiledScript != null) {
295-
return context.compiledClazz.cast(compiledScript);
295+
return context.factoryClazz.cast(compiledScript);
296296
}
297297

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

329-
return context.compiledClazz.cast(compiledScript);
329+
return context.factoryClazz.cast(compiledScript);
330330
}
331331
}
332332

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

339339
/**

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ public interface SearchScript {
3838
*/
3939
boolean needsScores();
4040

41-
interface Compiled {
41+
interface Factory {
4242
SearchScript newInstance(Map<String, Object> params, SearchLookup lookup);
4343
}
4444

45-
ScriptContext<Compiled> CONTEXT = new ScriptContext<>("search", Compiled.class);
45+
ScriptContext<Factory> CONTEXT = new ScriptContext<>("search", Factory.class);
4646
// TODO: remove aggs context when it has its own interface
47-
ScriptContext<SearchScript.Compiled> AGGS_CONTEXT = new ScriptContext<>("aggs", Compiled.class);
47+
ScriptContext<Factory> AGGS_CONTEXT = new ScriptContext<>("aggs", Factory.class);
4848
}

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

+2-3
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
import org.elasticsearch.index.shard.SearchOperationListener;
4949
import org.elasticsearch.indices.IndicesService;
5050
import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason;
51-
import org.elasticsearch.script.ScriptContext;
5251
import org.elasticsearch.script.ScriptService;
5352
import org.elasticsearch.script.SearchScript;
5453
import org.elasticsearch.search.aggregations.AggregationInitializationException;
@@ -685,8 +684,8 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc
685684
}
686685
if (source.scriptFields() != null) {
687686
for (org.elasticsearch.search.builder.SearchSourceBuilder.ScriptField field : source.scriptFields()) {
688-
SearchScript.Compiled compiled = scriptService.compile(field.script(), SearchScript.CONTEXT);
689-
SearchScript searchScript = compiled.newInstance(field.script().getParams(), context.lookup());
687+
SearchScript.Factory factory = scriptService.compile(field.script(), SearchScript.CONTEXT);
688+
SearchScript searchScript = factory.newInstance(field.script().getParams(), context.lookup());
690689
context.scriptFields().add(new ScriptField(field.fieldName(), searchScript, field.ignoreFailure()));
691690
}
692691
}

core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import org.elasticsearch.index.query.QueryShardException;
3131
import org.elasticsearch.script.ExecutableScript;
3232
import org.elasticsearch.script.Script;
33-
import org.elasticsearch.script.ScriptContext;
3433
import org.elasticsearch.search.aggregations.InternalAggregation;
3534
import org.elasticsearch.search.internal.SearchContext;
3635

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

9392
@Override
9493
public SignificanceHeuristic rewrite(InternalAggregation.ReduceContext context) {
95-
ExecutableScript.Compiled compiled = context.scriptService().compile(script, ExecutableScript.AGGS_CONTEXT);
96-
return new ExecutableScriptHeuristic(script, compiled.newInstance(script.getParams()));
94+
ExecutableScript.Factory factory = context.scriptService().compile(script, ExecutableScript.AGGS_CONTEXT);
95+
return new ExecutableScriptHeuristic(script, factory.newInstance(script.getParams()));
9796
}
9897

9998
@Override

core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/InternalScriptedMetric.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.common.xcontent.XContentBuilder;
2525
import org.elasticsearch.script.ExecutableScript;
2626
import org.elasticsearch.script.Script;
27-
import org.elasticsearch.script.ScriptContext;
2827
import org.elasticsearch.search.aggregations.InternalAggregation;
2928
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator;
3029

@@ -95,9 +94,9 @@ public InternalAggregation doReduce(List<InternalAggregation> aggregations, Redu
9594
if (firstAggregation.reduceScript.getParams() != null) {
9695
vars.putAll(firstAggregation.reduceScript.getParams());
9796
}
98-
ExecutableScript.Compiled compiled = reduceContext.scriptService().compile(
97+
ExecutableScript.Factory factory = reduceContext.scriptService().compile(
9998
firstAggregation.reduceScript, ExecutableScript.AGGS_CONTEXT);
100-
ExecutableScript script = compiled.newInstance(vars);
99+
ExecutableScript script = factory.newInstance(vars);
101100
aggregation = Collections.singletonList(script.run());
102101
} else if (reduceContext.isFinalReduce()) {
103102
aggregation = Collections.singletonList(aggregationObjects);

core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketscript/BucketScriptPipelineAggregator.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.elasticsearch.common.io.stream.StreamOutput;
2424
import org.elasticsearch.script.ExecutableScript;
2525
import org.elasticsearch.script.Script;
26-
import org.elasticsearch.script.ScriptContext;
2726
import org.elasticsearch.search.DocValueFormat;
2827
import org.elasticsearch.search.aggregations.AggregationExecutionException;
2928
import org.elasticsearch.search.aggregations.InternalAggregation;
@@ -90,7 +89,7 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
9089
(InternalMultiBucketAggregation<InternalMultiBucketAggregation, InternalMultiBucketAggregation.InternalBucket>) aggregation;
9190
List<? extends InternalMultiBucketAggregation.InternalBucket> buckets = originalAgg.getBuckets();
9291

93-
ExecutableScript.Compiled compiledScript = reduceContext.scriptService().compile(script, ExecutableScript.AGGS_CONTEXT);
92+
ExecutableScript.Factory factory = reduceContext.scriptService().compile(script, ExecutableScript.AGGS_CONTEXT);
9493
List<InternalMultiBucketAggregation.InternalBucket> newBuckets = new ArrayList<>();
9594
for (InternalMultiBucketAggregation.InternalBucket bucket : buckets) {
9695
Map<String, Object> vars = new HashMap<>();
@@ -111,7 +110,7 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
111110
if (skipBucket) {
112111
newBuckets.add(bucket);
113112
} else {
114-
ExecutableScript executableScript = compiledScript.newInstance(vars);
113+
ExecutableScript executableScript = factory.newInstance(vars);
115114
Object returned = executableScript.run();
116115
if (returned == null) {
117116
newBuckets.add(bucket);

core/src/main/java/org/elasticsearch/search/aggregations/pipeline/bucketselector/BucketSelectorPipelineAggregator.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.elasticsearch.common.io.stream.StreamOutput;
2525
import org.elasticsearch.script.ExecutableScript;
2626
import org.elasticsearch.script.Script;
27-
import org.elasticsearch.script.ScriptContext;
2827
import org.elasticsearch.search.aggregations.InternalAggregation;
2928
import org.elasticsearch.search.aggregations.InternalAggregation.ReduceContext;
3029
import org.elasticsearch.search.aggregations.InternalMultiBucketAggregation;
@@ -83,7 +82,7 @@ public InternalAggregation reduce(InternalAggregation aggregation, ReduceContext
8382
(InternalMultiBucketAggregation<InternalMultiBucketAggregation, InternalMultiBucketAggregation.InternalBucket>) aggregation;
8483
List<? extends InternalMultiBucketAggregation.InternalBucket> buckets = originalAgg.getBuckets();
8584

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

0 commit comments

Comments
 (0)