make use of incoming Message's replyTopic header if needed

[NathanQingyangXu]

as per reference doc: https://docs.spring.io/spring-kafka/docs/current/reference/html/#reply-message:

Starting with version 2.5, the framework will detect if these headers are missing and populate them with the topic - either the topic determined from the @sendto value or the incoming KafkaHeaders.REPLY_TOPIC header (if present)

Seems we overlooked the incoming KafkaHeaders.REPLY_TOPIC logic.

[artembilan]

@NathanQingyangXu ,

thank you very much for so thoughtful contributions!

Would you mind double checking if PR build failure is related somehow to your changes: https://github.com/spring-projects/spring-kafka/actions/runs/6903843623/job/18783345784?pr=2904 ?

[artembilan]

Can you, please, stop squashing your commits?
It is really much easier to review if there is a history of commits in the PR.
And sometime we just review the latest commit which might not include changes for other files.
When we merge PR, we squash to preserve only one commit in the main for the issue we are resolving.

Thanks for understanding!

[NathanQingyangXu]

Can you, please, stop squashing your commits? It is really much easier to review if there is a history of commits in the PR. And sometime we just review the latest commit which might not include changes for other files. When we merge PR, we squash to preserve only one commit in the main for the issue we are resolving.

Thanks for understanding!

a little bit late to read this comment. yeah, for sure. I totally understand. Will keep that in mind from now on. thanks.
btw, I am not very confident about my changes in this PR (seems a big overlooking if valid). I am a developer whose job requires good Spring Kafka experience and I think contributing directly is the best way to learn. Thanks for your patience and good product.

[NathanQingyangXu]

@NathanQingyangXu ,

thank you very much for so thoughtful contributions!

Would you mind double checking if PR build failure is related somehow to your changes: https://github.com/spring-projects/spring-kafka/actions/runs/6903843623/job/18783345784?pr=2904 ?

as usual, unit testing cases protected. Fixed the bug and now they would not complain.

[artembilan]

Looks like this fix introduces some new functionality.
Any chances to have ti covered with unit test, please?

[NathanQingyangXu]

New unit testing cases are desperately needed. My current concern is to confirm my PR validity. Will add testing cases soon.

…

On Fri, Nov 17, 2023, 1:56 p.m. Artem Bilan ***@***.***> wrote: ***@***.**** requested changes on this pull request. Looks like this fix introduces some new functionality. Any chances to have ti covered with unit test, please? ------------------------------ In spring-kafka/src/main/java/org/springframework/kafka/listener/adapter/MessagingMessageListenerAdapter.java <#2904 (comment)> : > @@ -482,14 +489,14 @@ else if (result instanceof Message) { iterableOfMessages = iterator.next() instanceof Message; Let's leave it wit this assumption for now! We can look into that later. Let's concentrate on missed headers logic in this your PR! — Reply to this email directly, view it on GitHub <#2904 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB6UYASBEMEO4VPXER7A4F3YE6XOPAVCNFSM6AAAAAA7PHPRSWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMZXGY4DQMRVGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[NathanQingyangXu]

During unit testing case development, I found this PR is invalid. For the following code snippet:

/**
* Return the default expression when no SendTo value is present.
* @return the expression.
* @since 2.2.15
*/
public static String getDefaultReplyTopicExpression() {
return PARSER_CONTEXT.getExpressionPrefix() + "source.headers['"
		+ KafkaHeaders.REPLY_TOPIC + "']" + PARSER_CONTEXT.getExpressionSuffix();
}

so if no reply topic is specified, the above default expression will ensure we got the value from KafkaHeaders.REPLY_TOPIC header.
There are other minior issues valid in this PR, but given the gist of it is invalid, I close this PR for now.