Invoke methods via public interface/superclass in compiled SpEL expressions

[drekbour]

Affects: 6.0.x

---

Many categories of basic SpelExpression are not compiled into a performant form due to a pattern of not acknowledging a Method can be called from superclass or interface. Best described with two simple failing tests for two scenarios. I have raised the issue as this feels like something resolvable.

ITEM1 MethodReference.isCompilable

   SpelExpressionParser parser = new SpelExpressionParser();
   StandardEvaluationContext context = new StandardEvaluationContext();

    @Test
    public void basic_expressions_are_compiled() {
        SpelExpression exp = (SpelExpression)parser.parseExpression("{2021, 2022}.contains(year)");
        assert exp.getValue(context, LocalDate.parse("2022-01-19")) == true;
        assert exp.compileExpression(); // FAIL - not compiled
    }

In this case it is the contains ...
org.springframework.expression.spel.ast.MethodReference#isCompilable
... checks concrete class and fails because it is not public...
public boolean java.util.Collections$UnmodifiableCollection.contains(java.lang.Object)
... even though MethodExecutor is already aware that the 'methodToInvoke' will be against an interface
public abstract boolean java.util.Collection.contains(java.lang.Object)

If the execution is on the interface, the check must be too.

Note: Similar expressions on properties (not methods) compile just fine (e.g. {2021, 2022}.size)

ITEM2 OptimalPropertyAccessor.isCompilable

However OptimalPropertyAccessor doesn't seem to understand overridden methods of parent classes (not interfaces which works fine as above).

    public static class Y { public int getI() {return 1;} }
    private static class Z extends Y { public int getI() {return 2;} }

    @Test
    public void basic_expressions_are_compiled() {       
        SpelExpression exp = (SpelExpression)parser.parseExpression("i");
        Y y = new Z();
        assert exp.getValue(context, y) == 2;
        assert exp.compileExpression(); // FAIL - not compiled
    }

This is because OptimalPropertyAccessor.isCompilable() relies on Method.getDeclaringClass() which has this behaviour

public boolean isCompilable() {
	return (Modifier.isPublic(this.member.getModifiers()) &&
			Modifier.isPublic(this.member.getDeclaringClass().getModifiers()));
}

[sbrannen]

Hi @drekbour,

Congratulations on submitting your first issue for the Spring Framework! 👍

And thanks for the detailed write-up.

It turns out that I recently had similar questions about our mixed use of ClassUtils.getInterfaceMethodIfPossible() and ReflectiveMethodExecutor.discoverPublicDeclaringClass() in SpEL, and I was wondering if we shouldn't rather use some sort of combination of the two.

As you pointed out, we may need to revisit MethodReference.isCompilable() and OptimalPropertyAccessor.isCompilable() as well.

In any case, we'll investigate our options.

Cheers,

Sam

[sbrannen]

To simplify matters for whoever investigates this further, I've combined the two provided test cases into a single test class.

package example;

import java.lang.reflect.Modifier;
import java.util.Collections;
import java.util.List;

import org.junit.jupiter.api.Test;

import org.springframework.expression.Expression;
import org.springframework.expression.spel.ast.InlineList;
import org.springframework.expression.spel.standard.SpelCompiler;
import org.springframework.expression.spel.standard.SpelExpression;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.StandardEvaluationContext;

import static org.assertj.core.api.Assertions.assertThat;

class SpelPrivateTypesTests {

	private final SpelExpressionParser parser = new SpelExpressionParser();

	private final StandardEvaluationContext context = new StandardEvaluationContext();

	private Expression expression;


	/**
	 * Note that {@link InlineList} creates a list and wraps it via
	 * {@link Collections#unmodifiableList(List)}, whose concrete type is private.
	 */
	@Test
	void privateSubclassOverridesMethodInPublicInterface() {
		expression = parser.parseExpression("{2021, 2022}");
		List<?> inlineList = expression.getValue(List.class);
		assertThat(Modifier.isPublic(inlineList.getClass().getModifiers())).isFalse();

		expression = parser.parseExpression("{2021, 2022}.contains(2022)");
		Boolean result = expression.getValue(context, Boolean.class);
		assertThat(result).isTrue();

		assertCanCompile(expression);
		result = expression.getValue(context, Boolean.class);
		assertThat(result).isTrue();
	}

	@Test
	void privateSubclassOverridesMethodInPublicSuperclass() {
		expression = parser.parseExpression("number");
		PublicSuperclass privateSubclass = new PrivateSubclass();
		Integer result = expression.getValue(context, privateSubclass, Integer.class);
		assertThat(result).isEqualTo(2);

		assertCanCompile(expression);
		result = expression.getValue(context, privateSubclass, Integer.class);
		assertThat(result).isEqualTo(2);
	}


	private static void assertCanCompile(Expression expression) {
		assertThat(SpelCompiler.compile(expression))
				.as(() -> "Expression <%s> should be compilable"
					.formatted(((SpelExpression) expression).toStringAST()))
				.isTrue();
	}


	public static class PublicSuperclass {

		public int getNumber() {
			return 1;
		}
	}

	private static class PrivateSubclass extends PublicSuperclass {

		@Override
		public int getNumber() {
			return 2;
		}
	}

}

Changing the expression in the first test to new java.util.ArrayList({2021, 2022}).contains(2022) allows it to pass (since ArrayList is a public type).

Making PrivateSubclass public allows the second test to pass.

[sbrannen]

Current work on this issue can be viewed in the following feature branch.

main...sbrannen:spring-framework:issues/gh-29857-SpEL-compilation-public-methods-in-private-subtypes

[sbrannen]

Closed via c79436f

[sbrannen]

While working on #29847, I noticed that similar issues exist when accessing a property by indexing into an object in Indexer.

In light of that, I am reopening this issue to address those issues as well.

[sbrannen]

Work on this issue has been completed in c79436f, 65d7762, and f4c1ad7.

@drekbour and anyone else interested, feel free to try out the changes in upcoming Spring Framework 6.2.0 snapshots.

If you encounter any issues, please report those back here.

Thanks,

Sam

---

Information on working with snapshot builds can be found here: https://github.com/spring-projects/spring-framework/wiki/Spring-Framework-Artifacts#snapshots