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

Don't use a strict MappingJackson2MessageConverter for SQS message conversion

Open bcalmac opened this issue 3 years ago • 1 comments

Type: Feature

Is your feature request related to a problem? Please describe. We have a set of micro-services that communicate through SQS messages in JSON format. A simple "Hello, world!" prototype fails with MessageConversionException: Cannot convert from [java.lang.String] to [com.clearcapital.uad.event.UadEvent] because I don't have a contentType=application/json message attribute and the MappingJackson2MessageConverter used by PayloadArgumentResolver is configured to be strict. In our case, adding the contentType attribute would not be needed for anything other than awspring because the messages are always JSON. Also, in practice we may publish messages from the command line or the AWS Console and it would be very easy to forget about the content type. All in all, not specifying the content type makes the system more robust and is therefore desirable.

Describe the solution you'd like

  1. Configure the MappingJackson2MessageConverter to not be strict and put it last in the CompositeMessageConverter used by PayloadArgumentResolver. This would make JSON messages work out of the box even without a contentType.

  2. Make it easier to customize the MessageConverter used by PayloadArgumentResolver. Currently it is hardcoded in QueueMessageHandler#initArgumentResolvers to be a CompositeMessageConverter and one can only define a MappingJackson2MessageConverter bean that replaces one of the converters under the composite one. awsspring comes with defaults to cover a wide range of cases, but for a specific project the developers may want to use a specific message converter. In our case I would just use a single non-strict MappingJackson2MessageConverter since all our messages are JSON. Something along the lines of a spring-boot customizer (like for example WebClientCustomizer) would be awesome.

Describe alternatives you've considered I've worked around the issue by defining a non-strict MappingJackson2MessageConverter bean. However, I don't consider this a robust solution. The MappingJackson2MessageConverter class isn't specific to awspring SQS and the bean I defined may be picked up in the future by beans other than SqsAutoConfiguration.SqsConfiguration#queueMessageHandler and have unintended side-effects.

Additional context I would be more than happy to contribute a PR if you consider incorporating any of these suggestions.

bcalmac avatar Dec 21 '21 04:12 bcalmac

I understand the reasoning and agree. Lets go with option 1. - because its simple, and I don't think we should invest much more into current codebase since it is going to be migrated to AWS SDK v2 which means a lot of changes. @bcalmac PR would be awesome.

maciejwalkowiak avatar Feb 05 '22 23:02 maciejwalkowiak

Solved in #374

maciejwalkowiak avatar Aug 31 '22 09:08 maciejwalkowiak

Fixed in #374

maciejwalkowiak avatar Sep 01 '22 07:09 maciejwalkowiak