-
Notifications
You must be signed in to change notification settings - Fork 34
Add Codable helpers for FunctionURL, APIGatewayV2, and SQS #90
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
Conversation
|
Thank you @natanrolnik for this welcome change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for this addition. I left one question and one suggestion. Also, be sure to include the license header on all source files and apply the swift formatting (there is a script to help you in ./scriptsformat.sh`)
Sources/AWSLambdaEvents/Codable Helpers/APIGatewayV2+Encode.swift
Outdated
Show resolved
Hide resolved
Sources/AWSLambdaEvents/Codable Helpers/APIGatewayV2+Decode.swift
Outdated
Show resolved
Hide resolved
Sources/AWSLambdaEvents/Codable Helpers/APIGatewayV2+Decode.swift
Outdated
Show resolved
Hide resolved
|
@sebsto done! I've unified them into two protocols: |
5fafa20 to
03ac6c9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you or the changes. I still have two quetsions.
| } | ||
| } | ||
|
|
||
| extension APIGatewayV2Response: EncodableResponse { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like the only difference between APIGAteway's init() and the protocol is the order of the parameters. Unless I missed something else, is there a reason we can't use the existing order for the parameters and delete this piece of code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're correct. I thought about changing the order, but it would be a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that we can't change the order on the APIGatewayResponse, but can you change the order on the EncodableResponse protocol ? That way APIGatewayResponse will automatically implement the EncodableResponse protocol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this init() function to allow this change
03ac6c9 to
cbc5418
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Add helpers for FunctionURL, APIGatewayV2, and SQS events to ease usage of
DecodableandEncodabletypesMotivation:
When using Function URL (or API Gateway) events and outputs, and SQS events, it is very common to decode a
Decodablebody, or encode aCodabletype into a function URL response. This PR adds convenient methods that are easy to use, but also maintain a level of customization by allowing custom JSON encoders/decoders.Modifications:
Encodabletypes into aFunctionURLReponseorAPIGatewayV2ResponseDecodabletypes from aFunctionURLRequestorAPIGatewayV2RequestDecodables fromSQSEventrecords, or from a singleSQSEvent.MessageResult:
The changes allow the following usage:
Instead of:
The same is similar when decoding SQS event messages.
And when encoding a response:
Instead of: