-
Notifications
You must be signed in to change notification settings - Fork 1.1k
GH-3344: Treat kotlin.Unit return as null in MMIH #3346
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
Conversation
Fixes spring-projects#3344 When function lambda doesn't return anything (e.g. a `void` method call is the last one), Kotlin produces a `kotlin.Unit` instance as a return value which is not null and produced as a reply message payload. * Fix `MessagingMethodInvokerHelper` to treat a `kotlin.Unit` as `null` for reply making Kotlin lambdas working the same way as Java lambdas when we don't return anything from from there **Cherry-pick to `5.3.x`**
| try { | ||
| return this.invocableHandlerMethod.invoke(message); | ||
| Object result = this.invocableHandlerMethod.invoke(message); | ||
| if (result != null && result.getClass().getName().equals("kotlin.Unit")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we not capture this condition as a final boolean by checking the return type in a CTOR (to avoid running this code on every non-null return)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, we can avoid something indeed, when a return type is void or Unit. However in this case the return type is Object so end-user can return null ()or just don't have return in the function lambda and we don't produce a reply message.
In this case Kotlin for non-return in function lambda produces for us that Unit which is not a null apparently.
So, we produce some weird reply message.
Therefore even if we have some final checks for return type, we still would have some logic to check the returned value to be sure that we don't produce a reply message with Unit payload.
To summarize: when have void return this result is always going to be null, therefore we don't go to check a class name for Unit. If the value not null, it already doesn't matter what void check we have in advance: we still have to check this result for Unit type.
Does it make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we at least do something like...
if (result != null && this.isKotlinLambda && result.getClass().getName().equals("kotlin.Unit")) {?
This result.getClass().getName().equals("kotlin.Unit") just seems like a lot of overhead for this one corner case and not good for the 99.9% of calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! OK. Makes sense.
I'll think what we can do...
Thanks for the pointer!
use it in the `MessagingMethodInvokerHelper` instead of `.getName().equals()`
|
how about this solution? |
|
Yes, that's better but you've tagged it |
spring-integration-core/src/main/java/org/springframework/integration/util/ClassUtils.java
Outdated
Show resolved
Hide resolved
|
Fixed here: db6786c. Thanks for pointing that out! |
* GH-3344: Treat kotlin.Unit return as null in MMIH Fixes #3344 When function lambda doesn't return anything (e.g. a `void` method call is the last one), Kotlin produces a `kotlin.Unit` instance as a return value which is not null and produced as a reply message payload. * Fix `MessagingMethodInvokerHelper` to treat a `kotlin.Unit` as `null` for reply making Kotlin lambdas working the same way as Java lambdas when we don't return anything from from there **Cherry-pick to `5.3.x`** * * Introduce `ClassUtils.isKotlinUnit(Class)` API; use it in the `MessagingMethodInvokerHelper` instead of `.getName().equals()` * * Fix since on new `isKotlinUnit()` API
|
And cherry-picked. |
Fixes #3344
When function lambda doesn't return anything (e.g. a
voidmethod call is the last one),Kotlin produces a
kotlin.Unitinstance as a return value which is not null and producedas a reply message payload.
MessagingMethodInvokerHelperto treat akotlin.Unitasnullfor replymaking Kotlin lambdas working the same way as Java lambdas when we don't return anything
from from there
Cherry-pick to
5.3.x