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

feat: Extract generic type for KafkaListeners in batch mode #89

Conversation

timonback
Copy link
Member

The KafkaPayloadTypeExtractor contains the logic to extract the payload type, which previously was part of ClassLevelKafkaListenerScanner and MethodLevelKafkaListenerScanner.

If the parameter is of type List<?>, reflection is used to extract the underlying type.
This solution assumes by using the List type, Kafka batch mode is used.

A future improvement can be to only extract the generic when the KafkaListener annotation is used together with the batch property. A library update for spring-kafka is possible: https://docs.spring.io/spring-kafka/api/org/springframework/kafka/annotation/KafkaListener.html#batch()

To demo it, one listener in ExampleConsumer is updated to use batch mode.

Relates to #87

Copy link
Member

@stavshamir stavshamir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, it's good to see so much code re-used. Please see my comments, I really think we can use this extractor for the amqp plugin as well

@timonback timonback marked this pull request as draft October 8, 2022 15:24
@timonback timonback force-pushed the feature/payload-schema-for-kafka-batch-listener branch 2 times, most recently from 5289884 to 60db5a6 Compare October 23, 2022 19:03
@timonback
Copy link
Member Author

I updated the PR with a larger refactoring and moving duplicated code to Abstract classes in core. The amqp and kafka listeners are slimmer now, although there is still some complexity through the abstract methods.
For reviewing, I suggest to check out the code locally.

Since the release is around the corner, I think this is a feature for the following release.

Regarding the batch mode, yes, this by default assumes that all List Payloads are batch methods. In the future this could be made configurable (batch-mode: yes/no), but at this point I would assume that people define their own payload types and not use List in non-batch mode (defining <MyType extends List<?> should work around the list detection anyway).

The KafkaPayloadTypeExtractor contains the logic to extract the payload type,
which previously was part of ClassLevelKafkaListenerScanner and MethodLevelKafkaListenerScanner.

If the parameter is of type List<?>, reflection is used to extract the underlying type.

To demo it, one listener in ExampleConsumer is updated to use batch mode.
Fixes nondeterministic (HashMap) ordering when channels for the same topic are merged
@timonback timonback force-pushed the feature/payload-schema-for-kafka-batch-listener branch from f63bbe0 to cfe5942 Compare October 25, 2022 20:38
@timonback timonback marked this pull request as ready for review October 25, 2022 21:05
@timonback timonback requested a review from stavshamir November 1, 2022 20:35
@stavshamir
Copy link
Member

stavshamir commented Nov 2, 2022

I really like this refactoring, it will make it much much easier to implement more plugins based on listener annotations. Great job!

@stavshamir stavshamir merged commit b30891f into springwolf:master Nov 2, 2022
@timonback timonback deleted the feature/payload-schema-for-kafka-batch-listener branch December 22, 2022 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants