Simplify not adding type header in SqsTemplate sent messages (#659) #1066
Conversation
levys17
requested review from
maciejwalkowiak,
MatejNedic and
tomazfernandes
as code owners
There was a problem hiding this comment.
Hey @levys17, implementation looks good!
Can you please add this option to the documentation and include a unit test?
Thanks!
Oh, sorry, nevermind about the test, now I see we already had one. So if you can just add it to the docs please - let me know if you need any help with that. |
Hey, @tomazfernandes ! Do you think it's ok? |
...src/main/java/io/awspring/cloud/sqs/support/converter/AbstractMessagingMessageConverter.java
Outdated
Show resolved
Hide resolved
Hey @levys17! That's about it, but please commit the text to the PR so it's easier to review if you can. |
github-actions
bot
added
the
type: documentation
There was a problem hiding this comment.
Hey @levys17, thanks for the changes, we're almost there.
Left a couple of comments.
...src/main/java/io/awspring/cloud/sqs/support/converter/AbstractMessagingMessageConverter.java
Outdated
Show resolved
Hide resolved
|
Hey, @tomazfernandes ! Thanks for the review! Can you check again? |
|
Done! @tomazfernandes |
| @@ -305,7 +305,7 @@ void shouldSendAndReceiveBatchFifo() { | |||
| @Test | |||
| void shouldSendAndReceiveRecordMessageWithoutPayloadInfoHeader() { | |||
| SqsTemplate template = SqsTemplate.builder().sqsAsyncClient(this.asyncClient) | |||
| .configureDefaultConverter(converter -> converter.setPayloadTypeHeaderValueFunction(msg -> null)) | |||
| .configureDefaultConverter(converter -> converter.doNotSendTypeHeader()) | |||
There was a problem hiding this comment.
Something seems off here - shouldn't this be converter.doNotSendPayloadTypeHeader()?
What am I missing?
There was a problem hiding this comment.
Yup, CI broke. @levys17, if you can please fix the test. Thanks!
There was a problem hiding this comment.
Srry! I forgot to add the test class while commiting 😅
📢 Type of change
📜 Description
doNotSendTypeHeader()inAbstractMessagingMessageConverterto simplify the way to not send type information in messages sent withSqsTemplate.💡 Motivation and Context
Issue related: #659
💚 How did you test it?
Updated unit test
shouldSendAndReceiveRecordMessageWithoutPayloadInfoHeaderinSqsTemplateIntegrationTestswith this new implementation instead of old one.📝 Checklist
🔮 Next steps