spring-cloud-aws icon indicating copy to clipboard operation
spring-cloud-aws copied to clipboard

Add `SqsTemplate`

Open maciejwalkowiak opened this issue 2 years ago • 1 comments

We need something to send messages to SQS, likely SqsTemplate, that integrates with the rest of SQS components.

maciejwalkowiak avatar Sep 04 '22 11:09 maciejwalkowiak

cc @tomazfernandes

maciejwalkowiak avatar Sep 04 '22 11:09 maciejwalkowiak

Hi, I'll add some initial considerations. Please let me know if this makes sense to you.

Sync vs. Async

I think one key feature of AWS SDK 2.0 is its non-blocking APIs, so IMO we should offer as much to users.

One way I thought we could go about this would be having separate SqsOperations and SqsAsyncOperations interfaces, and then a SqsTemplate implementing both. Users would be able to inject one of the interfaces for having an API streamlined for their use-case (blocking / async), or inject the template and have access to both.

Internally, all implementation would be async, with the blocking API invoking the async implementation with a join call in the end. Of course this means we'd need to keep the SqsAsyncClient auto configured. IMO I wouldn't bother auto configuring the blocking client unless users request it.

Sending Messages

One thing the current version apparently doesn't do very well is that it doesn't seem to handle Headers properly. IMO Message Headers should be translated to the proper MessageAttributes, and we should never serialize a Spring Message as is, but rather separate the JSON serialized payload and headers.

I wouldn't be too worried to implement the Spring Messaging interfaces here - only if the methods make sense.

Receiving Messages

I think the key part here is allowing users to receive both the deserialized POJO payload directly, or a Spring Message with the proper Message Attributes as Headers. We'd probably need separate methods for each use case.

tomazfernandes avatar Oct 19 '22 14:10 tomazfernandes

Thanks @tomazfernandes. I agree, all makes sense. We can also adopt idea of async operations for other integrations.

maciejwalkowiak avatar Oct 20 '22 09:10 maciejwalkowiak

We can also adopt idea of async operations for other integrations.

Sure! Though this raises another question, which is exposing the Sync vs Async clients as beans.

I know it's traditional for Spring Cloud AWS to expose Clients as beans directly. However usually in Spring Frameworks we have abstractions around the client classes, such as xyzTemplate, xyzAdmin, xyzConsumer, xyzProducer and xyzFactory.

This allows us to e.g. control the client instance lifecycle through Spring lifecycle hooks, add interceptors, advices, enable creating multiple instances with the same configurations, etc.

So I'm thinking we could refrain from exposing clients as beans. We could instead offer an auto configured Template with both blocking and async operations, and for the remaining functions either offer an additional Admin abstraction, or a simple doWithClient(Consumer<SqsAsyncClient> consumer) method in the template for methods we don't expose in our own API.

I also think in this case at least for the SQS integration it'd make sense to have a SqsClientFactory abstraction to be passed to the MessageListenerContainer and MessageListenerContainerFactory instead of the raw client.

But I'm aware this might be too big of a conceptual change for us to do now, and perhaps more work than we'll have time for before 3.0 - just wanted to see if it makes sense to you.

WDYT?

tomazfernandes avatar Oct 20 '22 15:10 tomazfernandes

I think exposing clients as beans is a right way to go as we do not intend to wrap all client functionality with "our" classes. That's what all Spring Data modules do. And while I understand the reasoning, even if we decide to go this path this would definitely mean that we need to postpone the release - which I am sure I don't want to.

maciejwalkowiak avatar Oct 21 '22 02:10 maciejwalkowiak

Sure, makes sense. Just wanted to know your thoughts on it - thanks!

I think we can leave any async / blocking matters for other integrations as a separate issue, and use this one as a first take on this structure with async.

tomazfernandes avatar Oct 21 '22 17:10 tomazfernandes