Extend events.SimpleEmailReceiptAction fields #211
Conversation
The action object contains different data depending on the type. Previously only the fields relevant to the Lambda type were included, preventing use of the SimpleEmailReceiptAction for other types.
|
I'm thinking there should be more extensive tests included as part of this PR, as the structure of the type is significantly more complex. Is a change like this likely to be accepted? |
* ReceiptAction FunctionArn -> FunctionARN * ReceiptAction json:"smtpReplayCode" -> json:"smtpReplyCode"
A summary like as a godoc comment would help, as would adding a few more tests cases to ses_test.go testdata/ses-event.json contains a Let me know if you'd like to do these updates, otherwise I'll try to do them soonish. |
|
That's great, I can spend some more time now working through your feedback. |
Using omitempty like this will prevent the inclusion of fields with empty strings. There are possibly scenarios where adding omitempty to the existing FunctionARN and InvocationType fields would change the behaviour of systems that expect the fields to be present and assigned an empty string. I've decided that the consistency of the fields when Unmarshaling is a bigger priority here though, as the chances of real world applications depending on this behaviour seems pretty remote to me.
This test only covered an SES lambda event, and did not cover other types of SES events. Adding SES-S3 and SES-SNS events here is improving the test coverage.
|
I think these three new commits should address your suggestions. |
Codecov Report
@@ Coverage Diff @@
## master #211 +/- ##
======================================
Coverage 77.2% 77.2%
======================================
Files 18 18
Lines 636 636
======================================
Hits 491 491
Misses 104 104
Partials 41 41Continue to review full report at Codecov.
|
|
Thanks! |
In my team, we regularly use SES events, but we Unmarshal into a custom type because the version in this repository omits fields that we require. The fields we use are documented here: https://docs.aws.amazon.com/ses/latest/DeveloperGuide/receiving-email-notifications-contents.html#receiving-email-notifications-contents-action-object
Specifically the
bucketNameandobjectKeyfields on theactionobject, which as documented, are only present when the type of the action isS3. In this PR, I've included all documented fields in the SimpleEmailReceiptAction type. This will allow my team to delete our custom type and use a type from an official source when Unmarshaling SES events.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.