Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@ public GenericElasticsearchScript() {}
public abstract Object execute(
Map<String, Object> params, double _score, Map<String, ScriptDocValues<?>> doc, Object _value, Map<?, ?> ctx);

public abstract boolean uses$_score();
public abstract boolean uses$ctx();
public abstract boolean needs_score();
public abstract boolean needsCtx();
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public SearchScript newInstance(final LeafReaderContext context) {
}
@Override
public boolean needsScores() {
return painlessScript.uses$_score();
return painlessScript.needs_score();
}
};
return context.factoryClazz.cast(factory);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import java.util.function.Function;

import static java.util.Collections.unmodifiableList;
import static org.elasticsearch.painless.WriterConstants.USES_PARAMETER_METHOD_TYPE;
import static org.elasticsearch.painless.WriterConstants.NEEDS_PARAMETER_METHOD_TYPE;

/**
* Information about the interface being implemented by the painless script.
Expand All @@ -40,7 +40,7 @@ public class ScriptClassInfo {
private final org.objectweb.asm.commons.Method executeMethod;
private final Definition.Type executeMethodReturnType;
private final List<MethodArgument> executeArguments;
private final List<org.objectweb.asm.commons.Method> usesMethods;
private final List<org.objectweb.asm.commons.Method> needsMethods;
private final List<org.objectweb.asm.commons.Method> getMethods;
private final List<Definition.Type> getReturns;

Expand All @@ -49,7 +49,7 @@ public ScriptClassInfo(Definition definition, Class<?> baseClass) {

// Find the main method and the uses$argName methods
java.lang.reflect.Method executeMethod = null;
List<org.objectweb.asm.commons.Method> usesMethods = new ArrayList<>();
List<org.objectweb.asm.commons.Method> needsMethods = new ArrayList<>();
List<org.objectweb.asm.commons.Method> getMethods = new ArrayList<>();
List<Definition.Type> getReturns = new ArrayList<>();
for (java.lang.reflect.Method m : baseClass.getMethods()) {
Expand All @@ -65,16 +65,8 @@ public ScriptClassInfo(Definition definition, Class<?> baseClass) {
+ "] has more than one.");
}
}
if (m.getName().startsWith("uses$")) {
if (false == m.getReturnType().equals(boolean.class)) {
throw new IllegalArgumentException("Painless can only implement uses$ methods that return boolean but ["
+ baseClass.getName() + "#" + m.getName() + "] returns [" + m.getReturnType().getName() + "].");
}
if (m.getParameterTypes().length > 0) {
throw new IllegalArgumentException("Painless can only implement uses$ methods that do not take parameters but ["
+ baseClass.getName() + "#" + m.getName() + "] does.");
}
usesMethods.add(new org.objectweb.asm.commons.Method(m.getName(), USES_PARAMETER_METHOD_TYPE.toMethodDescriptorString()));
if (m.getName().startsWith("needs") && m.getReturnType().equals(boolean.class) && m.getParameterTypes().length == 0) {
needsMethods.add(new org.objectweb.asm.commons.Method(m.getName(), NEEDS_PARAMETER_METHOD_TYPE.toMethodDescriptorString()));
}
if (m.getName().startsWith("get") && m.getName().equals("getClass") == false && Modifier.isStatic(m.getModifiers()) == false) {
getReturns.add(
Expand Down Expand Up @@ -106,15 +98,7 @@ public ScriptClassInfo(Definition definition, Class<?> baseClass) {
argumentNames.add(argumentNamesConstant[arg]);
}
this.executeArguments = unmodifiableList(arguments);

// Validate that the uses$argName methods reference argument names
for (org.objectweb.asm.commons.Method usesMethod : usesMethods) {
if (false == argumentNames.contains(usesMethod.getName().substring("uses$".length()))) {
throw new IllegalArgumentException("Painless can only implement uses$ methods that match a parameter name but ["
+ baseClass.getName() + "#" + usesMethod.getName() + "] doesn't match any of " + argumentNames + ".");
}
}
this.usesMethods = unmodifiableList(usesMethods);
this.needsMethods = unmodifiableList(needsMethods);
this.getMethods = unmodifiableList(getMethods);
this.getReturns = unmodifiableList(getReturns);
}
Expand Down Expand Up @@ -152,8 +136,8 @@ public List<MethodArgument> getExecuteArguments() {
/**
* The {@code uses$varName} methods that must be implemented by Painless to complete implementing the interface.
*/
public List<org.objectweb.asm.commons.Method> getUsesMethods() {
return usesMethods;
public List<org.objectweb.asm.commons.Method> getNeedsMethods() {
return needsMethods;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ final class ScriptImpl extends SearchScript {
variables.putAll(leafLookup.asMap());
}

scoreLookup = script.uses$_score() ? this::getScore : () -> 0.0;
ctxLookup = script.uses$ctx() ? variables -> (Map<?, ?>) variables.get("ctx") : variables -> null;
scoreLookup = script.needs_score() ? this::getScore : () -> 0.0;
ctxLookup = script.needsCtx() ? variables -> (Map<?, ?>) variables.get("ctx") : variables -> null;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public final class WriterConstants {
public static final Type COLLECTIONS_TYPE = Type.getType(Collections.class);
public static final Method EMPTY_MAP_METHOD = getAsmMethod(Map.class, "emptyMap");

public static final MethodType USES_PARAMETER_METHOD_TYPE = MethodType.methodType(boolean.class);
public static final MethodType NEEDS_PARAMETER_METHOD_TYPE = MethodType.methodType(boolean.class);

public static final Type MAP_TYPE = Type.getType(Map.class);
public static final Method MAP_GET = getAsmMethod(Object.class, "get", Object.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,11 +347,14 @@ public void write() {
clinit.endMethod();
}

// Write any uses$varName methods for used variables
for (org.objectweb.asm.commons.Method usesMethod : scriptClassInfo.getUsesMethods()) {
MethodWriter ifaceMethod = new MethodWriter(Opcodes.ACC_PUBLIC, usesMethod, visitor, globals.getStatements(), settings);
// Write any needsVarName methods for used variables
for (org.objectweb.asm.commons.Method needsMethod : scriptClassInfo.getNeedsMethods()) {
String name = needsMethod.getName();
name = name.substring(5);
name = Character.toLowerCase(name.charAt(0)) + name.substring(1);
MethodWriter ifaceMethod = new MethodWriter(Opcodes.ACC_PUBLIC, needsMethod, visitor, globals.getStatements(), settings);
ifaceMethod.visitCode();
ifaceMethod.push(reserved.getUsedVariables().contains(usesMethod.getName().substring("uses$".length())));
ifaceMethod.push(reserved.getUsedVariables().contains(name));
ifaceMethod.returnValue();
ifaceMethod.endMethod();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@ public void testDefArrayArg() {
public abstract static class ManyArgs {
public static final String[] PARAMETERS = new String[] {"a", "b", "c", "d"};
public abstract Object execute(int a, int b, int c, int d);
public abstract boolean uses$a();
public abstract boolean uses$b();
public abstract boolean uses$c();
public abstract boolean uses$d();
public abstract boolean needsA();
public abstract boolean needsB();
public abstract boolean needsC();
public abstract boolean needsD();
}
public void testManyArgs() {
Compiler compiler = new Compiler(ManyArgs.class, Definition.BUILTINS);
Expand All @@ -174,20 +174,20 @@ public void testManyArgs() {

// While we're here we can verify that painless correctly finds used variables
ManyArgs script = (ManyArgs)scriptEngine.compile(compiler, null, "a", emptyMap());
assertTrue(script.uses$a());
assertFalse(script.uses$b());
assertFalse(script.uses$c());
assertFalse(script.uses$d());
assertTrue(script.needsA());
assertFalse(script.needsB());
assertFalse(script.needsC());
assertFalse(script.needsD());
script = (ManyArgs)scriptEngine.compile(compiler, null, "a + b + c", emptyMap());
assertTrue(script.uses$a());
assertTrue(script.uses$b());
assertTrue(script.uses$c());
assertFalse(script.uses$d());
assertTrue(script.needsA());
assertTrue(script.needsB());
assertTrue(script.needsC());
assertFalse(script.needsD());
script = (ManyArgs)scriptEngine.compile(compiler, null, "a + b + c + d", emptyMap());
assertTrue(script.uses$a());
assertTrue(script.uses$b());
assertTrue(script.uses$c());
assertTrue(script.uses$d());
assertTrue(script.needsA());
assertTrue(script.needsB());
assertTrue(script.needsC());
assertTrue(script.needsD());
}

public abstract static class VarargTest {
Expand Down Expand Up @@ -473,43 +473,4 @@ public void testTwoExecuteMethods() {
assertEquals("Painless can only implement interfaces that have a single method named [execute] but ["
+ TwoExecuteMethods.class.getName() + "] has more than one.", e.getMessage());
}

public abstract static class BadUsesReturn {
public static final String[] PARAMETERS = new String[] {"foo"};
public abstract Object execute(String foo);
public abstract Object uses$foo();
}
public void testBadUsesReturn() {
Compiler compiler = new Compiler(BadUsesReturn.class, Definition.BUILTINS);
Exception e = expectScriptThrows(IllegalArgumentException.class, false, () ->
scriptEngine.compile(compiler, null, "null", emptyMap()));
assertEquals("Painless can only implement uses$ methods that return boolean but [" + BadUsesReturn.class.getName()
+ "#uses$foo] returns [java.lang.Object].", e.getMessage());
}

public abstract static class BadUsesParameter {
public static final String[] PARAMETERS = new String[] {"foo", "bar"};
public abstract Object execute(String foo, String bar);
public abstract boolean uses$bar(boolean foo);
}
public void testBadUsesParameter() {
Compiler compiler = new Compiler(BadUsesParameter.class, Definition.BUILTINS);
Exception e = expectScriptThrows(IllegalArgumentException.class, false, () ->
scriptEngine.compile(compiler, null, "null", emptyMap()));
assertEquals("Painless can only implement uses$ methods that do not take parameters but [" + BadUsesParameter.class.getName()
+ "#uses$bar] does.", e.getMessage());
}

public abstract static class BadUsesName {
public static final String[] PARAMETERS = new String[] {"foo", "bar"};
public abstract Object execute(String foo, String bar);
public abstract boolean uses$baz();
}
public void testBadUsesName() {
Compiler compiler = new Compiler(BadUsesName.class, Definition.BUILTINS);
Exception e = expectScriptThrows(IllegalArgumentException.class, false, () ->
scriptEngine.compile(compiler, null, "null", emptyMap()));
assertEquals("Painless can only implement uses$ methods that match a parameter name but [" + BadUsesName.class.getName()
+ "#uses$baz] doesn't match any of [foo, bar].", e.getMessage());
}
}