Skip to content
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

Added SES Event type #130

Merged
merged 7 commits into from
Jun 17, 2020
Merged

Added SES Event type #130

merged 7 commits into from
Jun 17, 2020

Conversation

adam-fowler
Copy link
Member

Add SES Lambda Event type

Motivation:

Want to support Lambdas triggered by SES

Modifications:

  • Added AWSLambdaEvents/SES.swift which include SES.Event
  • Added DateTimeCoding propertyWrapper to support dates of the format "Wed, 7 Oct 2015 12:34:56 -0700".
  • Added SESTests

Result:

You can now decode SES events.

@swift-server-bot
Copy link

Can one of the admins verify this patch?

3 similar comments
@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

let dateString = try container.decode(String.self)
guard let date = Self.dateFormatter.date(from: dateString) else {
throw DecodingError.dataCorruptedError(in: container, debugDescription:
"Expected date to be in iso8601 date format with fractional seconds, but `\(dateString)` does not forfill format")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy and paste?! iso8601 🙃

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorted but message may need to change if we get a more specific date format name.

Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very happy with this. Two small nits. Otherwise this is perfect to go!

@@ -67,3 +69,30 @@ public struct ISO8601WithFractionalSecondsCoding: Decodable {
return formatter
}
}

@propertyWrapper
public struct DateTimeCoding: Decodable {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure about the naming since it is very generic? Is there a special name for this format? @tomerd @weissi

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we need to find a more specific name. Is this used only in SES?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets also add a unit test for this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure about the naming since it is very generic? Is there a special name for this format? @tomerd @weissi

RFC is particularly unhelpful https://tools.ietf.org/html/rfc5322#section-3.3

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s appears to be a standard internet format. It is the same one used in http headers.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AWS docs mention 5322: https://docs.aws.amazon.com/ses/latest/DeveloperGuide/sending-concepts-email-format.html so perhaps we name it RFC5322DateTimeCoding?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed

@tomerd
Copy link
Contributor

tomerd commented Jun 16, 2020

@swift-server-bot test this please

@tomerd
Copy link
Contributor

tomerd commented Jun 16, 2020

@swift-server-bot add to whitelist

@tomerd tomerd added this to the 0.1.1 milestone Jun 17, 2020
Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @adam-fowler. This looks great!

Copy link
Contributor

@tomerd tomerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great, but please add a quick unit test for the new date formatter

@adam-fowler
Copy link
Member Author

looks great, but please add a quick unit test for the new date formatter

done

@tomerd tomerd merged commit d2f5c1c into swift-server:master Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants