GH-3079: Use getMostSpecificMethod for SpEL calls #3082
Conversation
| @@ -700,7 +701,15 @@ public IntegrationFlow wireTapFlow4() { | |||
| public IntegrationFlow wireTapFlow5() { | |||
| return f -> f | |||
| .wireTap(sf -> sf | |||
| .<String, String>transform(String::toUpperCase) | |||
| .transform(// Must not be lambda for SpEL fallback behavior on empty payload | |||
There was a problem hiding this comment.
I am not sure why the payload resolver doesn't allow an empty string but I suppose it's rare so falling back to SpEL is ok.
There was a problem hiding this comment.
I treat this like a note and not as some confusion or advice to change something.
Correct?
There was a problem hiding this comment.
Yes, it's fine; I was just making a comment; since the problem only arises with an empty payload.
There was a problem hiding this comment.
Well, it also is there when we use a @UseSpelInvoker.
I believe I had to come up with the @Transformer against that GenericTransformer impl test-case, but it doesn't matter so far: I don't think it makes sense to use @Transformer and don't rely on powerful POJO method call support.
| .isThrownBy(() -> processor.processMessage(new GenericMessage<>("foo"))) | ||
| .withCauseInstanceOf(SpelEvaluationException.class); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Hmmm - I don't think this is right - there is no relationship between the methods on these two classes - it looks like getMostSpecificMethod() doesn't check that the method is in the class hierarchy - seems like a bug to me. I think this test should still pass.
Consider ignoring instead and perhaps open an issue in SF to see whether they consider the behavior to be correct?
There was a problem hiding this comment.
OK. Makes sense.
It bumped me as well: why getMostSpecificMethod() doesn't care about the class of source.
I'll revert the test change with an appropriate @Ignore.
We still don't pass for Kotlin variant, but that's also out of SI scope...
There was a problem hiding this comment.
Looks like it's intentional...
return targetClass.getMethod(method.getName(), method.getParameterTypes());But it seems odd to me.
There was a problem hiding this comment.
Here it is: spring-projects/spring-framework#23824
Fixes spring-projects#3079 It turns out that we need to use a *most specific* method for the target to be called reflectively from the SpEL NOTE: We drop a *Method not found* error in case of wrong method source since now we find a right source via `ClassUtils.getMostSpecificMethod()` **Cherry-pick to 5.1.x**
…thodNotFound()` instead of removal
* GH-3079: Use getMostSpecificMethod for SpEL calls Fixes #3079 It turns out that we need to use a *most specific* method for the target to be called reflectively from the SpEL NOTE: We drop a *Method not found* error in case of wrong method source since now we find a right source via `ClassUtils.getMostSpecificMethod()` **Cherry-pick to 5.1.x** * * `@Ignore` `MethodInvokingMessageProcessorTests.testProcessMessageMethodNotFound()` instead of removal
|
Cherry-picked as 80ed25e after resolving conflicts. |
Fixes #3079
It turns out that we need to use a most specific method for the target
to be called reflectively from the SpEL
NOTE: We drop a Method not found error in case of wrong method source
since now we find a right source via
ClassUtils.getMostSpecificMethod()Cherry-pick to 5.1.x