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

Simplify not adding type header in SqsTemplate sent messages #659

Closed
tomazfernandes opened this issue Feb 3, 2023 · 8 comments
Closed
Assignees
Labels
component: sqs SQS integration related issue status: ideal-for-contribution We agree it's nice to have but it is not team priority type: enhancement Smaller enhancement in existing integration
Milestone

Comments

@tomazfernandes
Copy link
Contributor

Currently in order to not add type information to messages sent with the SqsTemplate it's necessary to do something like this:

SqsTemplate template = SqsTemplate.builder().sqsAsyncClient(this.asyncClient)
		.configureDefaultConverter(converter -> converter.setPayloadTypeHeaderValueFunction(msg -> null))
		.build();

We could make it more user-friendly and perhaps have a TemplateOptions.doNotAddTypeInformationToSentMessages() method or something similar.

@tomazfernandes tomazfernandes added the component: sqs SQS integration related issue label Feb 3, 2023
@tomazfernandes tomazfernandes added this to the 3.x milestone Feb 3, 2023
@tomazfernandes tomazfernandes self-assigned this Feb 12, 2023
@tomazfernandes tomazfernandes removed their assignment May 17, 2023
@tomazfernandes tomazfernandes added the status: ideal-for-contribution We agree it's nice to have but it is not team priority label Aug 24, 2023
@tomazfernandes tomazfernandes added the type: enhancement Smaller enhancement in existing integration label Jan 5, 2024
@levys17
Copy link
Contributor

levys17 commented Feb 8, 2024

May I try this one? Seems to be good to start!

@tomazfernandes
Copy link
Contributor Author

Sounds good @levys17!

Let me know if there's anything you'd like to discuss.

@levys17
Copy link
Contributor

levys17 commented Feb 22, 2024

Hii, @tomazfernandes!

Considering that AbstractMessagingMessageConverter is responsible to set headers ( typeHeader included), what do you thing about implement this method in this class?

public void sendMessageWithoutTypeHeader() {
		this.payloadTypeHeaderFunction = message -> null;
}
@Test
	void shouldSendAndReceiveRecordMessageWithoutPayloadInfoHeader() {
		SqsTemplate template = SqsTemplate.builder().sqsAsyncClient(this.asyncClient)
				.configureDefaultConverter(converter -> converter.sendMessageWithoutTypeHeader())
				.build();
		SampleRecord testRecord = new SampleRecord("Hello world!",
				"From shouldSendAndReceiveRecordMessageWithoutPayloadInfoHeader!");
		SendResult<SampleRecord> result = template.send(RECORD_WITHOUT_TYPE_HEADER_QUEUE_NAME, testRecord);
		assertThat(result).isNotNull();
		Optional<Message<SampleRecord>> receivedMessage = template
				.receive(from -> from.queue(RECORD_WITHOUT_TYPE_HEADER_QUEUE_NAME), SampleRecord.class);
		assertThat(receivedMessage).isPresent().get().extracting(Message::getPayload).isEqualTo(testRecord);
	}

Should I keep going implementing this way or perhaps there is a better way?

@tomazfernandes
Copy link
Contributor Author

Hey @levys17, sorry for the delay!

what do you thing about implement this method in this class?

This makes sense. I had thought of setting it in the ContainerOptions to try to make it more obvious and easier to find, but maybe it'd just add unnecessary complexity.

Regarding the method name, how about converter.doNotSendTypeHeader()? Seems a bit more straightforward, but I'm open to suggestions.

Let me know your thoughts, thanks!

@levys17
Copy link
Contributor

levys17 commented Feb 27, 2024

No problem, @tomazfernandes! Thanks for answer!
Ok, I'll finish the implementation considering this. About the method name, seems really better than I used.

I'll open a PR soon.

Thanks!

@maciejwalkowiak
Copy link
Contributor

@tomazfernandes looks like a good candidate for 3.1.1?

@tomazfernandes
Copy link
Contributor Author

@tomazfernandes looks like a good candidate for 3.1.1?

Yup, definitely!

@maciejwalkowiak maciejwalkowiak modified the milestones: 3.x, 3.1.1 Mar 15, 2024
maciejwalkowiak added a commit that referenced this issue Mar 15, 2024
…#1066)

---------

Co-authored-by: Maciej Walkowiak <walkowiak.maciej@yahoo.com>
@maciejwalkowiak
Copy link
Contributor

Fixed in #1066

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: sqs SQS integration related issue status: ideal-for-contribution We agree it's nice to have but it is not team priority type: enhancement Smaller enhancement in existing integration
Projects
None yet
Development

No branches or pull requests

3 participants