Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compiled SpEL expression fail when used with registered function [SPR-12359] #16964

Closed
spring-projects-issues opened this issue Oct 22, 2014 · 3 comments
Assignees
Labels
in: core type: bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Oct 22, 2014

Christoph Strobl opened SPR-12359 and commented

Calling registered custom functions does not work once compiled SpEL expression is used. (tried with 4.1, 4.1.1 and 4.1.2 snapshot)

org.springframework.expression.spel.SpelEvaluationException: EL1072E:(pos 0): An exception occurred whilst evaluating a compiled expression
	at org.springframework.expression.spel.standard.SpelExpression.getValue(SpelExpression.java:180)

The below tests passes once doFormat is registered as context.registerFunction("doFormat", String.class.getDeclaredMethod("format", String.class, Object[].class)); but fails when using some custom class.

Test
public void compiledExpressionShouldWorkWhenUsingCustomFunctionWithVarargs() throws NoSuchMethodException, SecurityException {

  SpelExpressionParser parser = new SpelExpressionParser(new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, this .getClass().getClassLoader()));

  SpelExpression expression = parser.parseRaw("#doFormat([0], #arg)");

  StandardEvaluationContext context = new StandardEvaluationContext(new Object[] { "hey %s" });
  context.registerFunction("doFormat", DelegatingStringFormat.class.getDeclaredMethod("format", String.class,  Object[].class));
  context.setVariable("arg", "there");

  expression.setEvaluationContext(context);

  assertThat(expression.getValue(String.class), is("hey there"));
  assertThat(expression.getValue(String.class), is("hey there"));

  // have to call it 3 time to fail
  assertThat(expression.getValue(String.class), is("hey there"));
}

static class DelegatingStringFormat {

  static String format(String s, Object... args) {
    return String.format(s, args);
  }
}

The above will work once we change the method signature of DelegatingStringFormat.format to static String format(String s, Object[] args).

Another failing sample not using varargs:

@Test
public void compiledExpressionShouldWorkWhenUsingCustomFunction() throws NoSuchMethodException, SecurityException {

  SpelExpressionParser parser = new SpelExpressionParser(new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE,  this.getClass().getClassLoader()));

  SpelExpression expression = parser.parseRaw("#doCompare([0], #arg)");

  StandardEvaluationContext context = new StandardEvaluationContext(new  Object[] { "1" });
  context.registerFunction("doCompare", SomeCompareMethod.class.getDeclaredMethod("compare", Object.class, Object.class));
  context.setVariable("arg", "2");

  expression.setEvaluationContext(context);

  assertThat(expression.getValue(Integer.class), is(-1));
  assertThat(expression.getValue(Integer.class), is(-1));

  // have to call it 3 time to fail
  assertThat(expression.getValue(Integer.class), is(-1));
}

static class SomeCompareMethod {

  static int compare(Object o1, Object o2) {
    return new ComparableComparator().compare(o1, o2);
  }
}

Affects: 4.1 GA, 4.1.1

Issue Links:

  • #17127 Regression: SpEL expression with Arrays.asList and empty array

Referenced from: commits a40e424

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 22, 2014

Juergen Hoeller commented

Andy Clement, another one for you, I'm afraid...

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 22, 2014

Andy Clement commented

No problem Juergen, keep them coming :)

I think here the function compilation needs to be brought in line with method compilation (which is much more sophisticated) like I did with constructor compilation. In the second case I believe the problem is simply that the method and its declaring class are not public and although with reflection we don't care about that (in the interpreted case) it will only be compilable if they are public. So the fix will be checking that they are visible (public) for compilation.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Oct 24, 2014

Andy Clement commented

I've just committed the changes I wanted to make to address this:

  • the isCompilable() check for function reference is now much smarter. Previously it wasn't fully checking visibility of the target function (which needs to be public in a public class).
  • I also managed to throw away the argument handling for function reference and change it to use the sophisticated argument handling code that we have in place for method invocations. This gets function references to varargs methods behaving.
  • loads of new tests around function reference, compilation and varargs.

Thanks for the test cases Christoph, they are invaluable. Feel free to kick the tyres on this some more. I would like to make a more sophisticated refactoring of the varargs handling but I'm a little nervous about disturbing the code too much at the moment, maybe the next time I'm in there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core type: bug
Projects
None yet
Development

No branches or pull requests

2 participants