-
-
Notifications
You must be signed in to change notification settings - Fork 316
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
S3EventNotification deserialization no longer works with AWS SDK v2 S3 Event Notification #1278
Comments
Hey @helpermethod, thanks for bringing this up, your PR is welcome! |
Just one comment @helpermethod - if I understand the feature correctly, I think the best way to handle this might be having an Makes sense? |
Hi @tomazfernandes, I guess I also need to register the argument resolver in AbstractListenerAnnotationBeanPostProcessor? Furthermore: Would it make sense to include |
Hey @helpermethod, sorry for the delay.
In this case let's use the
I would not add a dependency just to solve this one use case, but I'll defer the dependency management decision to @maciejwalkowiak. Thanks and please let me know if there's anything else you need. |
Can we add it as an optional dependency where if this dependency is present, relevant resolvers get registered? |
Type: Bug
Component:
SQS
Describe the bug
S3EventNotification is no longer correctly deserialized when switching from
com.amazonaws:aws-java-sdk-s3
tosoftware.amazon.awssdk:s3-event-notifications
.This is due to the following changes: https://aws.amazon.com/de/blogs/developer/the-aws-sdk-for-java-2-17-removes-its-external-dependency-on-jackson/
Because the constructor and fields are no longer annotated with @JsonCreator and @JsonProperty, Jackson is no longer able to map the JSON correctly.
It's easy to work around it by accepting a String instead of a S3EventNotification in the listener and then performing the deserialization by calling
S3EventNotification.fromJson
but it would be nicer this wouldn't be needed.One solution could be to provide a custom JSON Deserializer for that case. Wouldn't mind creating a PR.
The text was updated successfully, but these errors were encountered: