Skip to content

Commit

Permalink
Fix superclass functional interface resolution in Painless (elastic#8…
Browse files Browse the repository at this point in the history
…1698)

A recent change [1] to how we load our allow list changed the resolution for how Painless looks up 
methods of super classes. However, functional interface loading was not changed which caused a 
bug where a functional interface would not look at its super interfaces for the functional interface 
method [2].

This fixes the issue by going through each super interface until the functional interface method is 
found when the target interface doesn't have the functional interface method.

[1] elastic#76955
[2] elastic#81696

Also a big thanks to @megglos and @TheFireCookie for their help with this issue.
  • Loading branch information
jdconrad committed Dec 14, 2021
1 parent a0c1027 commit 8913b71
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 2 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/81698.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 81698
summary: Fix subclass functional interface resolution in Painless
area: Infra/Scripting
type: regression
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -2116,7 +2116,7 @@ private void setFunctionalInterfaceMethods() {
}
}

private void setFunctionalInterfaceMethod(Class<?> targetClass, PainlessClassBuilder painlessClassBuilder) {
private void setFunctionalInterfaceMethod(Class<?> targetClass, PainlessClassBuilder targetPainlessClassBuilder) {
if (targetClass.isInterface()) {
List<java.lang.reflect.Method> javaMethods = new ArrayList<>();

Expand All @@ -2141,7 +2141,31 @@ private void setFunctionalInterfaceMethod(Class<?> targetClass, PainlessClassBui
} else if (javaMethods.size() == 1) {
java.lang.reflect.Method javaMethod = javaMethods.get(0);
String painlessMethodKey = buildPainlessMethodKey(javaMethod.getName(), javaMethod.getParameterCount());
painlessClassBuilder.functionalInterfaceMethod = painlessClassBuilder.methods.get(painlessMethodKey);

List<Class<?>> superInterfaces = new ArrayList<>();
Set<Class<?>> resolvedInterfaces = new HashSet<>();

superInterfaces.add(targetClass);

while (superInterfaces.isEmpty() == false) {
Class<?> superInterface = superInterfaces.remove(0);

if (resolvedInterfaces.add(superInterface)) {
PainlessClassBuilder functionalInterfacePainlessClassBuilder = classesToPainlessClassBuilders.get(superInterface);

if (functionalInterfacePainlessClassBuilder != null) {
targetPainlessClassBuilder.functionalInterfaceMethod = functionalInterfacePainlessClassBuilder.methods.get(
painlessMethodKey
);

if (targetPainlessClassBuilder.functionalInterfaceMethod != null) {
break;
}
}

superInterfaces.addAll(Arrays.asList(superInterface.getInterfaces()));
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,4 +309,14 @@ public void testReturnVoidDef() {
exec("def b = new StringBuilder(); List l = [1, 2]; l.stream().map(i -> b.setLength(i)).collect(Collectors.toList())")
);
}

public void testUnaryOperator() {
assertEquals("doremi", exec("List lst = ['abc', '123']; lst.replaceAll(f -> f.replace('abc', 'doremi')); lst.get(0);"));
assertEquals("doremi", exec("def lst = ['abc', '123']; lst.replaceAll(f -> f.replace('abc', 'doremi')); lst.get(0);"));
}

public void testBinaryOperator() {
assertEquals(1, exec("def list = new ArrayList(); list.add(1); list.add(2); list.stream().reduce((i1, i2) -> i1).orElse(0)"));
assertEquals(1, exec("List list = new ArrayList(); list.add(1); list.add(2); list.stream().reduce((i1, i2) -> i1).orElse(0)"));
}
}

0 comments on commit 8913b71

Please sign in to comment.