Refactor SQS and SNS

[Mariscal6]

• Update aws sdk (aws-sdk-go-v2)
• Create SQS/SNS configuration
• Handling contexts
• Cancelling operations
• All test pass

I don't quite know how to handle the error if a message cannot be deleted.

[georgedbarr]

Hi @Mariscal6. Appreciate your work on this - I'm really keen to use this package for an upcoming project.

I have a request if that's okay. At the moment it seems we can't specify the MessageDeduplicationId or MessageGroupId fields in the sns.PublishInput struct as it is hidden away in the package. The MessageGroupId field is required for FIFO topics. Would it be possible to allow the client to specify that property, maybe by using the Watermill Message metadata?

I have left a couple of small suggestions - I Hope you don't mind 😄

[Mariscal6]

Hi @Mariscal6. Appreciate your work on this - I'm really keen to use this package for an upcoming project.

I have a request if that's okay. At the moment it seems we can't specify the MessageDeduplicationId or MessageGroupId fields in the sns.PublishInput struct as it is hidden away in the package. The MessageGroupId field is required for FIFO topics. Would it be possible to allow the client to specify that property, maybe by using the Watermill Message metadata?

I have left a couple of small suggestions - I Hope you don't mind 😄

@georgedbarr Thanks for the feedback.

Yes, I think adding it is useful. I don't know if it's worth doing it in this version. I tried to isolate myself as much as possible from the FIFO Queues, because I had the feeling that they were going to give me quite a headache.

I'll add the changes that were suggested to me on discord and come back to this topic. Thank you!

[Mariscal6]

@georgedbarr added. But there is no test for this yet, keep that in mind.

[Mariscal6]

II think there is something wrong with the docker image or with my code but I cannot see the error.

You can easily reproduce the error running cmd/sns-sqs/main.go. And setting a break point in the SQS Marshaller, you will see you get different messages formats. The first iteration works fine (I have seen it work up to three messages in a row).
Then it changes the format, and there are no more attributes in the MessageAttributes field.
So the uuid cannot be extracted and the tests fail 👎

[Mariscal6]

@m110 @roblaszczak Any feedback? Sorry for pushing, I don't want it to be forgotten :)

[Mariscal6]

The error I am encountering is due to the fact that the attributes when there is a subscription of sns, are shared in the body instead of the MessageAttributes

{"Type":"Notification","Token":"","MessageId":"900582b0-3695-4175-9343-153103b42816","TopicArn":"arn:aws:sns:us-east-1:100010001000:local-topic1","Subject":"","Message":"{\"some_json\": \"body4\"}","Timestamp":"2023-08-13T12:09:46Z","SignatureVersion":"1","Signature":"BV3D7uWZEmU2aSmVDZo9/gFtiTTS3o1c0v7ovCfUVlvybaOelxlqzAX8+ZnPYC24UN2gytKg4an+OziYL03TDn6fQI8Qb8gdxmwPDwXE7xlzFbbX0QO6kBsk9zwvKC6O3UpUFOQQ1mFFWG67CPN1nO+7/ZQzd9rqHmvOC7wcb/PIQ3LIJu3OQK0bFXrWHbep1RZ9hzFA2lloUa6gp+F4LMOeZw5a+NHlJDOhrVL8ibGbiWoG0iNqb0ugKi1iawnEstRPCI8Qo09JpaWcGndthZqUua4EJq4WK+3i+31gLcjxgmJO3Zbjy82z+4gM8bk+ufoDnbBnid+8hi/scE5bIA==","SigningCertURL":"http://goaws.com:4100/SimpleNotificationService/900582b0-3695-4175-9343-153103b42816.pem","UnsubscribeURL":"http://goaws.com:4100/?Action=Unsubscribe\u0026SubscriptionArn=arn:aws:sns:us-east-1:100010001000:local-topic1:6f8f948c-f3d2-424b-8637-9f557aa82772","SubscribeURL":"","MessageAttributes":{"UUID":{"Type":"StringValue","Value":"01H7QCTSZJ88S8W8RPZMK8KMJV"}}}

They say that using the RawMessageDelivery option should fix it but it is not the case, at least for the image we are using for testing. Honestly, I have tried using localstack but the performance is worst than pafortin/goaws, so I would not change it.

https://stackoverflow.com/questions/44238656/how-to-add-sqs-message-attributes-in-sns-subscription/49753291#49753291:~:text=From%20aws%20documentation%3A

[m110]

I'll look into it this week, thank you for contributing!

[Nikola-Milovic]

What are the blockers for getting this merged? Is any help needed?

[Nikola-Milovic]

Suggested change

	func SetEndPoint(endpoint string) config.LoadOptionsFunc {
	func EndpointOption(endpoint string) config.LoadOptionsFunc {

Since its no longer setting anything

Also the package connection is a bit iffy on this one, maybe config, where other config related functionality could reside in the future? Nitpick

[Mariscal6]

thanks for the review Nikola!

[Mariscal6]

@m110 can we merge this?

[m110]

Thank you everyone for contributing here!