honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Scripting: Remove unnecessary generic type from ScriptContext (#24877) 

When developing the new ScriptContext, the compiled type was original
generic, so that the instance type was also necessary. However, since
CompiledType is all that is used by the compile method signature, we
actually don't need the instance type to be generic. This commit removes
the InstanceType, and finds the Class for it through reflection on the
CompiledType method.
This commit is contained in:
Ryan Ernst 2017-05-24 19:20:49 -07:00 committed by GitHub
parent 1daacd97b0
commit 5581a0b2f0
6 changed files with 115 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java
View File

@ -329,7 +329,7 @@ public class QueryShardContext extends QueryRewriteContext {
* Compiles (or retrieves from cache) and binds the parameters to the
* provided script
*/
public final SearchScript getSearchScript(Script script, ScriptContext<SearchScript, SearchScript.Compiled> context) {
public final SearchScript getSearchScript(Script script, ScriptContext<SearchScript.Compiled> context) {
failIfFrozen();
SearchScript.Compiled compiled = scriptService.compile(script, context);
return compiled.newInstance(script.getParams(), lookup());
@ -339,7 +339,7 @@ public class QueryShardContext extends QueryRewriteContext {
* parameters are available.
*/
public final Function<Map<String, Object>, SearchScript> getLazySearchScript(
Script script, ScriptContext<SearchScript, SearchScript.Compiled> context) {
Script script, ScriptContext<SearchScript.Compiled> 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);
@ -350,7 +350,7 @@ public class QueryShardContext extends QueryRewriteContext {
* Compiles (or retrieves from cache) and binds the parameters to the
* provided script
*/
public final ExecutableScript getExecutableScript(Script script, ScriptContext<ExecutableScript, ExecutableScript.Compiled> context) {
public final ExecutableScript getExecutableScript(Script script, ScriptContext<ExecutableScript.Compiled> context) {
failIfFrozen();
ExecutableScript.Compiled compiled = scriptService.compile(script, context);
return compiled.newInstance(script.getParams());
@ -361,7 +361,7 @@ public class QueryShardContext extends QueryRewriteContext {
* parameters are available.
*/
public final Function<Map<String, Object>, ExecutableScript> getLazyExecutableScript(
Script script, ScriptContext<ExecutableScript, ExecutableScript.Compiled> context) {
Script script, ScriptContext<ExecutableScript.Compiled> 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);

67
core/src/main/java/org/elasticsearch/script/ScriptContext.java
View File

@ -19,6 +19,7 @@
package org.elasticsearch.script;
import java.lang.reflect.Method;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
@ -31,36 +32,36 @@ import java.util.Map;
* <p>
* There are two related classes which must be supplied to construct a {@link ScriptContext}.
* <p>
* The <i>InstanceType</i> is a class, possibly stateful, that is an instance of a script. 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 <i>execute</i> which {@link ScriptEngine} implementations
* will define.
* <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
* must be stateless so it is cacheable by the {@link ScriptService}. It must have an abstract method
* named <i>newInstance</i> which returns <i>InstanceType</i> which {@link ScriptEngine} implementations
* 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
* 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<InstanceType, CompiledType> {
public final class ScriptContext<CompiledType> {
public static final ScriptContext<SearchScript, SearchScript.Compiled> AGGS =
new ScriptContext<>("aggs", SearchScript.class, SearchScript.Compiled.class);
public static final ScriptContext<SearchScript, SearchScript.Compiled> SEARCH =
new ScriptContext<>("search", SearchScript.class, SearchScript.Compiled.class);
public static final ScriptContext<SearchScript.Compiled> AGGS =
new ScriptContext<>("aggs", SearchScript.Compiled.class);
public static final ScriptContext<SearchScript.Compiled> SEARCH =
new ScriptContext<>("search", SearchScript.Compiled.class);
// TODO: remove this once each agg calling scripts has its own context
public static final ScriptContext<ExecutableScript, ExecutableScript.Compiled> AGGS_EXECUTABLE =
new ScriptContext<>("aggs_executable", ExecutableScript.class, ExecutableScript.Compiled.class);
public static final ScriptContext<ExecutableScript, ExecutableScript.Compiled> UPDATE =
new ScriptContext<>("update", ExecutableScript.class, ExecutableScript.Compiled.class);
public static final ScriptContext<ExecutableScript, ExecutableScript.Compiled> INGEST =
new ScriptContext<>("ingest", ExecutableScript.class, ExecutableScript.Compiled.class);
public static final ScriptContext<ExecutableScript, ExecutableScript.Compiled> EXECUTABLE =
new ScriptContext<>("executable", ExecutableScript.class, ExecutableScript.Compiled.class);
public static final ScriptContext<ExecutableScript.Compiled> AGGS_EXECUTABLE =
new ScriptContext<>("aggs_executable", ExecutableScript.Compiled.class);
public static final ScriptContext<ExecutableScript.Compiled> UPDATE =
new ScriptContext<>("update", ExecutableScript.Compiled.class);
public static final ScriptContext<ExecutableScript.Compiled> INGEST =
new ScriptContext<>("ingest", ExecutableScript.Compiled.class);
public static final ScriptContext<ExecutableScript.Compiled> EXECUTABLE =
new ScriptContext<>("executable", ExecutableScript.Compiled.class);
public static final Map<String, ScriptContext<?, ?>> BUILTINS;
public static final Map<String, ScriptContext<?>> BUILTINS;
static {
Map<String, ScriptContext<?, ?>> builtins = new HashMap<>();
Map<String, ScriptContext<?>> builtins = new HashMap<>();
builtins.put(AGGS.name, AGGS);
builtins.put(SEARCH.name, SEARCH);
builtins.put(AGGS_EXECUTABLE.name, AGGS_EXECUTABLE);
@ -73,16 +74,30 @@ public final class ScriptContext<InstanceType, CompiledType> {
/** A unique identifier for this context. */
public final String name;
/** A class that is an instance of a script. */
public final Class<InstanceType> instanceClazz;
/** A factory class for constructing instances of a script. */
public final Class<CompiledType> compiledClazz;
/** 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<InstanceType> instanceClazz, Class<CompiledType> compiledClazz) {
public ScriptContext(String name, Class<CompiledType> compiledClazz) {
this.name = name;
this.instanceClazz = instanceClazz;
this.compiledClazz = compiledClazz;
Method newInstanceMethod = null;
for (Method method : compiledClazz.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 + "]");
}
newInstanceMethod = method;
}
}
if (newInstanceMethod == null) {
throw new IllegalArgumentException("Could not find method newInstance on CompiledType class ["
+ compiledClazz.getName() + "] for script context [" + name + "]");
}
instanceClazz = newInstanceMethod.getReturnType();
}
}

2
core/src/main/java/org/elasticsearch/script/ScriptModule.java
View File

@ -36,7 +36,7 @@ public class ScriptModule {
public ScriptModule(Settings settings, List<ScriptPlugin> scriptPlugins) {
Map<String, ScriptEngine> engines = new HashMap<>();
Map<String, ScriptContext<?, ?>> contexts = new HashMap<>(ScriptContext.BUILTINS);
Map<String, ScriptContext<?>> contexts = new HashMap<>(ScriptContext.BUILTINS);
for (ScriptPlugin plugin : scriptPlugins) {
for (ScriptContext context : plugin.getContexts()) {
ScriptContext oldContext = contexts.put(context.name, context);

8
core/src/main/java/org/elasticsearch/script/ScriptService.java
View File

@ -81,7 +81,7 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
private final Set<String> contextsAllowed;
private final Map<String, ScriptEngine> engines;
private final Map<String, ScriptContext<?,?>> contexts;
private final Map<String, ScriptContext<?>> contexts;
private final Cache<CacheKey, Object> cache;
@ -94,7 +94,7 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
private double scriptsPerMinCounter;
private double compilesAllowedPerNano;
public ScriptService(Settings settings, Map<String, ScriptEngine> engines, Map<String, ScriptContext<?,?>> contexts) {
public ScriptService(Settings settings, Map<String, ScriptEngine> engines, Map<String, ScriptContext<?>> contexts) {
super(settings);
Objects.requireNonNull(settings);
@ -221,7 +221,7 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
*
* @return a compiled script which may be used to construct instances of a script for the given context
*/
public <InstanceType, CompiledType> CompiledType compile(Script script, ScriptContext<InstanceType, CompiledType> context) {
public <CompiledType> CompiledType compile(Script script, ScriptContext<CompiledType> context) {
Objects.requireNonNull(script);
Objects.requireNonNull(context);
@ -338,7 +338,7 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
}
/** Compiles a template. Note this will be moved to a separate TemplateService in the future. */
public CompiledTemplate compileTemplate(Script script, ScriptContext<ExecutableScript, ExecutableScript.Compiled> scriptContext) {
public CompiledTemplate compileTemplate(Script script, ScriptContext<ExecutableScript.Compiled> scriptContext) {
ExecutableScript.Compiled compiledScript = compile(script, scriptContext);
return params -> (String)compiledScript.newInstance(params).run();
}

63
core/src/test/java/org/elasticsearch/script/ScriptContextTests.java Normal file
View File

@ -0,0 +1,63 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.script;
import org.elasticsearch.test.ESTestCase;
public class ScriptContextTests extends ESTestCase {
public interface TwoNewInstance {
String newInstance(int foo, int bar);
String newInstance(int foo);
}
public interface MissingNewInstance {
String typoNewInstanceMethod(int foo);
}
public interface DummyScript {
int execute(int foo);
interface Compiled {
DummyScript newInstance();
}
}
public void testTwoNewInstanceMethods() {
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
new ScriptContext<>("test", TwoNewInstance.class));
assertEquals("Cannot have multiple newInstance methods on CompiledType class ["
+ TwoNewInstance.class.getName() + "] for script context [test]", e.getMessage());
}
public void testMissingNewInstanceMethod() {
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
new ScriptContext<>("test", MissingNewInstance.class));
assertEquals("Could not find method newInstance on CompiledType class ["
+ MissingNewInstance.class.getName() + "] for script context [test]", e.getMessage());
}
public void testInstanceTypeReflection() {
ScriptContext<?> context = new ScriptContext<>("test", DummyScript.Compiled.class);
assertEquals("test", context.name);
assertEquals(DummyScript.class, context.instanceClazz);
assertEquals(DummyScript.Compiled.class, context.compiledClazz);
}
}

4
core/src/test/java/org/elasticsearch/script/ScriptServiceTests.java
View File

@ -51,7 +51,7 @@ public class ScriptServiceTests extends ESTestCase {
private ScriptEngine scriptEngine;
private Map<String, ScriptEngine> engines;
private Map<String, ScriptContext<?,?>> contexts;
private Map<String, ScriptContext<?>> contexts;
private ScriptService scriptService;
private Settings baseSettings;
@ -213,7 +213,7 @@ public class ScriptServiceTests extends ESTestCase {
builder.put(ScriptService.SCRIPT_CACHE_SIZE_SETTING.getKey(), 1);
buildScriptService(builder.build());
Script script = new Script(ScriptType.INLINE, "test", "1+1", Collections.emptyMap());
ScriptContext<?, ?> context = randomFrom(contexts.values());
ScriptContext<?> context = randomFrom(contexts.values());
scriptService.compile(script, context);
scriptService.compile(script, context);
assertEquals(1L, scriptService.stats().getCompilations());