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

Spring Messaging batch payload incompatible with Kotlin #29963

Closed
samyempcc opened this issue Feb 13, 2023 · 6 comments
Closed

Spring Messaging batch payload incompatible with Kotlin #29963

samyempcc opened this issue Feb 13, 2023 · 6 comments
Assignees
Labels
for: external-project Needs a fix in external project in: messaging Issues in messaging modules (jms, messaging) status: declined A suggestion or change that we don't feel we should currently apply theme: kotlin An issue related to Kotlin support

Comments

@samyempcc
Copy link

samyempcc commented Feb 13, 2023

When using batched messaging with Kafka, if the message listener is using Kotlin List<Message<*>> in the listener, the ClassUtils.isAssignable is checking if the method is "compatible" with java.util.List and does not work with kotlin's default List:
org.springframework.messaging.handler.annotation.support.PayloadMethodArgumentResolver.resolveArgument

Kotlin by default uses List from kotlin.collections and the reflection test are not assignable.

Expected to have it work with kotlin's collection identical to the Java version.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 13, 2023
@sdeleuze sdeleuze added the theme: kotlin An issue related to Kotlin support label Feb 14, 2023
@sdeleuze sdeleuze self-assigned this Feb 14, 2023
@sdeleuze
Copy link
Contributor

sdeleuze commented Feb 14, 2023

I think Kotlin lists are expected to be java.util.List as well, see this code sample.

Could you please provide a reproducer?

@sdeleuze sdeleuze added the status: waiting-for-feedback We need additional information before we can continue label Feb 14, 2023
@samyempcc
Copy link
Author

I will try to put a full reproducer once I get some time. Meanwhile this workaround fixed it for me:
@KafkaListener() fun batchListener(records: java.util.List<Message<*>?>) {..}
This returns the list of GenericMessage.

When using Kotlin's default List like:
@KafkaListener() fun batchListener(records: List<Message<*>?>) {..}
the records would not be a list of Message, but a list of the payload inside the message.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 14, 2023
@rstoyanchev rstoyanchev added the in: messaging Issues in messaging modules (jms, messaging) label Feb 14, 2023
@sdeleuze sdeleuze removed the status: feedback-provided Feedback has been provided label Feb 16, 2023
@sdeleuze
Copy link
Contributor

I would be really interested by a repro to understand what happens here. When using Kotlin List, what types do you get here for targetClass and payloadClass?

@sdeleuze sdeleuze added the status: waiting-for-feedback We need additional information before we can continue label Feb 16, 2023
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Feb 23, 2023
@samyempcc
Copy link
Author

I would be really interested by a repro to understand what happens here. When using Kotlin List, what types do you get here for targetClass and payloadClass?

Digging further, I saw the root cause is actually in: https://github.com/spring-projects/spring-kafka/blob/main/spring-kafka/src/main/java/org/springframework/kafka/listener/adapter/MessagingMessageListenerAdapter.java#L682

with Kotlin the paramType resolves to:
? extends org.springframework.messaging.Message<?>

while for java, it is:
org.springframework.messaging.Message<?>

The rest of the code flow in that method ends up assigning the payload content based on the test for paramType and ends up behaving differently for kotlin.

I have a working code to reproduce the issue given a local kafka running on port 9092:

https://github.com/samyem/spring-messaging-issue29963/blob/master/src/main/kotlin/com/example/issue29963/service/BatchTest.kt#L35

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Feb 26, 2023
@sdeleuze
Copy link
Contributor

Thanks for digging into that. Indeed as discusses in #22313, Kotlin lists involve declaration site-variance so extra care should be taken for this use case. The related code seems to be on Spring Kafka side so please open a related issue on https://github.com/spring-projects/spring-kafka/issues. cc @garyrussell

@sdeleuze sdeleuze closed this as not planned Won't fix, can't repro, duplicate, stale Feb 27, 2023
@sdeleuze sdeleuze added status: declined A suggestion or change that we don't feel we should currently apply for: external-project Needs a fix in external project and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project Needs a fix in external project in: messaging Issues in messaging modules (jms, messaging) status: declined A suggestion or change that we don't feel we should currently apply theme: kotlin An issue related to Kotlin support
Projects
None yet
Development

No branches or pull requests

4 participants