Skip to content

Conversation

garyrussell
Copy link
Contributor

Resolves #2304

@SendTo was only detected on the concrete listener method.

Use AnnotatedElementUtils.findMergedAnnotation() instead.

cherry-pick to 2.9.x, 2.8.x

Resolves spring-projects#2304

`@SendTo` was only detected on the concrete listener method.

Use `AnnotatedElementUtils.findMergedAnnotation()` instead.

**cherry-pick to 2.9.x, 2.8.x**
@artembilan
Copy link
Member

I know the solution is simple and really re-aligned with what we have so far with @KafkaListener/@KafkaHandler.
But I wonder why would one need this?
Who would make so sophisticated design that there is a need to pull those annotation to super class/ interface?
Any real sample, please?

Even if we can do this, it doesn't mean that it makes sense in the target projects.

Thanks

@VovkaSOL
Copy link

@artembilan hi, i have many kafka controllers with topic name depends on request type name. I try to write one abstract super controller with change annotation text in runtime by generic type. https://pastebin.com/mdqRTVS4
It works with recieve, but not work with @sendto reply.

@artembilan
Copy link
Member

I see, @VovkaSOL .

So, you hacking annotation attributes values via reflection based on the actual generic arguments from the specific implementation.
Too smart and too complicated. 😄

I would say those actual generic arguments - KafkaSuperController<GetTransfersByDate.Request,GetTransfersByDate.Response>, - are going to be compiled in anyway.
So, when a new topic raises, you have to write some code to support and compile. Therefore your solution is not so dynamic and perhaps could be done with less suffer from overcomplication with that reflection.

Don't get me wrong - don't want to offend you: I perhaps would implement something similar just because it looks cool and smart and really very abstract too easy to extend.
But this makes it too tied to topic names and does not let to have flexibility to change any logic for particular listener not affecting others.

Another solution which does not require reflection hacks and extra generics support would be possible with Spring Integration and its dynamic flows support: https://docs.spring.io/spring-integration/docs/current/reference/html/dsl.html#java-dsl-runtime-flows.
This sample demonstrates exactly runtime listener registrations: https://github.com/spring-projects/spring-integration-samples/tree/main/dsl/kafka-dsl

Thank you for sharing your experience!

@VovkaSOL
Copy link

@artembilan thanks, i understand my code not most users case)
We have 15 microservices in production, each type of request has its own topic by type with auto-creation. I can't rewrite so much code for spring integration :-) I create issue because i see that @KafkaListener support inheritance, but @sendto not. I didn’t understand a little why this is so, it seems that uniformity will be clearer, well, it doesn’t seem to break anything. Big thanks @garyrussell for PR 👍

@artembilan artembilan merged commit a5a28cc into spring-projects:main Jun 14, 2022
@artembilan
Copy link
Member

... and cherry-picked to 2.9.x & 2.8.x

@garyrussell garyrussell deleted the GH-2304 branch June 14, 2022 18:56
@garyrussell
Copy link
Contributor Author

@VovkaSOL FYI, 2.8.7 is scheduled to be released next Tuesday.

@artembilan
Copy link
Member

@VovkaSOL ,

re. your "15 microservices": you know that @KafkaListener.topics() and @SendTo(value) can be as properties placeholders. So, you may have just a single project, but every instance may come with different configurations.

@VovkaSOL
Copy link

@VovkaSOL FYI, 2.8.7 is scheduled to be released next Tuesday.

wonderful, thanks 👍

@VovkaSOL
Copy link

@VovkaSOL ,

re. your "15 microservices": you know that @KafkaListener.topics() and @SendTo(value) can be as properties placeholders. So, you may have just a single project, but every instance may come with different configurations.

offcouse, now we have a standard topic configuration through properties, but there are too many properties on the client and server side. I'm trying to simplify it :-)
image

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.

Replace getAnnotation to findAnnotation in MethodKafkaListenerEndpoint::getReplyTopic()
3 participants