-
Notifications
You must be signed in to change notification settings - Fork 113
Add LambdaAPI Example of a Serverless REST API #125
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
Add LambdaAPI Example of a Serverless REST API #125
Conversation
Can one of the admins verify this patch? |
3 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
@@ -42,5 +50,12 @@ let package = Package( | |||
.target(name: "CurrencyExchange", dependencies: [ | |||
.product(name: "AWSLambdaRuntime", package: "swift-aws-lambda-runtime"), | |||
]), | |||
.target(name: "LambdaAPI", dependencies: [ |
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.
its great to have such a comprehensive example! I think it needs a more specific name though - one that describes better the kind of things it demonstrates (see CurrencyExchange for example)
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, what do you think about ProductAPI
?
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.
} | ||
return try decoder.decode(T.self, from: dataBody) | ||
} | ||
} |
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.
see discussion in #129
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.
} | ||
} | ||
|
||
let defaultHeaders = ["Content-Type": "application/json", |
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.
static? also make note about XSS?
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.
- static 49aedc7
) | ||
} | ||
|
||
init<Out: Encodable>(with object: Out, statusCode: AWSLambdaEvents.HTTPResponseStatus) { |
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.
see discussion in #129
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.
statusCode: statusCode, | ||
headers: defaultHeaders, | ||
multiValueHeaders: nil, | ||
body: "{\"message\":\"\(error.localizedDescription)\"}", |
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.
localizedDescription not needed?
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.
} | ||
|
||
init<Out: Encodable>(with result: Result<Out, Error>, statusCode: AWSLambdaEvents.HTTPResponseStatus) { | ||
|
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.
remove redundant empty lines (several spots)
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.
import Foundation | ||
import AWSDynamoDB | ||
|
||
public struct ProductField { |
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.
consider nesting under Product. also consider using enum
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.
self.createdAt = dictionary[ProductField.createdAt]?.s | ||
self.updatedAt = dictionary[ProductField.updatedAt]?.s | ||
} | ||
} |
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.
cc @tachyonics
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 wonder if there is an opportunity to do something cool here based on encoder/decoder
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.
@adam-fowler has added something to simplify this soto-project/soto#319.
Here my original example with the implementation from his branch:
https://github.com/swift-sprinter/aws-serverless-swift-api-template/tree/feature/dynamodb-codable
public let description: String | ||
public var createdAt: String? | ||
public var updatedAt: String? | ||
} |
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.
some are var and some are let, intentional?
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.
Yes, the business logic updates them.
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
import Foundation |
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.
used here?
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.
public let name: String | ||
public let description: String | ||
public var createdAt: String? | ||
public var updatedAt: String? |
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.
should these be dates?
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.
It could be, but I left the String to have a more clear output in the JSON:
With String
{
"updatedAt": "2020-06-20T10:31:39.633Z",
"description": "Finally! Swift 🚀 + AWS Lambda⚡️️",
"createdAt": "2020-06-20T10:31:39.633Z",
"name": "A new template 3",
"sku": "3"
}
With Date
{
"createdAt": 614341899.6329999,
"description": "Finally! Swift 🚀 + AWS Lambda⚡️️",
"updatedAt": 614341899.6329999,
"name": "A new template 3",
"sku": "3"
}
Note that in DynamoDB the date can be saved as Number o String. In the example is saved as String iso8061.
https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/HowItWorks.NamingRulesDataTypes.html#HowItWorks.DataTypes
import Foundation | ||
#if canImport(FoundationNetworking) | ||
import FoundationNetworking | ||
#endif |
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.
FoundationNetworking used?
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.
} | ||
|
||
struct EmptyResponse: Codable { | ||
|
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.
empty line
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.
|
||
struct ProductLambda: LambdaHandler { | ||
|
||
//typealias In = APIGateway.SimpleRequest |
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.
dead 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.
|
||
static func currentRegion() -> Region { | ||
|
||
if let awsRegion = ProcessInfo.processInfo.environment["AWS_REGION"] { |
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.
there is a helper for that: Lambda.env
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.
also may be worth caching to improve perf
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.
- 49aedc7
- It's called only once.
} | ||
|
||
static func tableName() throws -> String { | ||
guard let tableName = ProcessInfo.processInfo.environment["PRODUCTS_TABLE_NAME"] else { |
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.
Lambda.env
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.
let timeout = HTTPClient.Configuration.Timeout(connect: lambdaRuntimeTimeout, | ||
read: lambdaRuntimeTimeout) | ||
let configuration = HTTPClient.Configuration(timeout: timeout) | ||
self.httpClient = HTTPClient(eventLoopGroupProvider: .createNew, configuration: configuration) |
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.
share the event loop instead of creating new
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.
init(eventLoop: EventLoop) { | ||
|
||
let handler = Lambda.env("_HANDLER") ?? "" | ||
self.operation = Operation(rawValue: handler) ?? .unknown |
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.
is unknown an error case?
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.
Yes, if the lambda handler is not one of the supported case.
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.
refactor in
ff3326e
|
||
init(eventLoop: EventLoop) { | ||
|
||
let handler = Lambda.env("_HANDLER") ?? "" |
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.
is unknown handler an error case?
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.
Yes, it's set in the next line.
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.
self.operation = Operation(rawValue: handler) ?? .unknown | ||
|
||
self.region = Self.currentRegion() | ||
logger.info("\(Self.currentRegion())") |
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.
maybe log as part of more meaningful message with additional context/configuration data. also should this be logged at debug or info level?
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.
ff3326e
removed
self.httpClient = HTTPClient(eventLoopGroupProvider: .createNew, configuration: configuration) | ||
|
||
self.db = AWSDynamoDB.DynamoDB(region: region, httpClientProvider: .shared(self.httpClient)) | ||
logger.info("DynamoDB") |
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.
left over?
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.
Yes, I'll remove it.
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.
self.db = AWSDynamoDB.DynamoDB(region: region, httpClientProvider: .shared(self.httpClient)) | ||
logger.info("DynamoDB") | ||
|
||
self.tableName = (try? Self.tableName()) ?? "" |
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.
failure case?
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.
db: db, | ||
tableName: tableName | ||
) | ||
logger.info("ProductService") |
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.
leftover?
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.
Yes, I'll remove it.
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.
import Foundation | ||
#if canImport(FoundationNetworking) | ||
import FoundationNetworking | ||
#endif |
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.
FoundationNetworking used?
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.
} | ||
} | ||
return list | ||
case .unknown: |
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.
maybe this can be caught earlier so not included in the enum?
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.
Removed
} | ||
} | ||
|
||
struct CreateLambdaHandler { |
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.
consider making these into functions, e.g. (did not try to compile, so maybe syntax errors):
func createHandler(service: ProductService, context: Lambda.Context, event: APIGateway.V2.Request) -> EventLoopFuture<Product> {
guard let product: Product = try? event.object() else {
return context.eventLoop.makeFailedFuture(APIError.invalidRequest)
}
return service.createItem(product: product)
}
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.
Refactored in 8649522
self.service = service | ||
} | ||
|
||
func handle(context: Lambda.Context, event: APIGateway.V2.Request) -> EventLoopFuture<Result<Product,Error>> { |
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.
it is not typical to need EventLoopFuture<Result<Product,Error>>
since EventLoopFuture
already covers the error case. so this should be EventLoopFuture<Product>
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.
Refactored in 8649522
return future | ||
} | ||
} | ||
} |
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.
similar comments to all these ^^
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.
Refactored in 8649522
case handlerNotFound | ||
} | ||
|
||
extension Date { |
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.
reuse from Events module?
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.
Used after refactor of Operation
ff3326e
var product = product | ||
let date = Date().iso8601 | ||
product.createdAt = date | ||
product.updatedAt = date |
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.
can the date be stored in more native format like numeric or timestamp?
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.
Note that in DynamoDB the date can be saved as Number o String.
https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/HowItWorks.NamingRulesDataTypes.html#HowItWorks.DataTypes
let input = DynamoDB.ScanInput(tableName: tableName) | ||
return db.scan(input) | ||
} | ||
} |
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.
cc @tachyonics for dynmodb best practices
import AWSLambdaRuntime | ||
import Logging | ||
|
||
let logger = Logger(label: "AWS.Lambda.Products") |
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.
used?
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.
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.
Removed
@@ -0,0 +1,202 @@ | |||
{ |
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.
used?
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.
It's the Swagger you can open with PostMan to have the API definition and test it. I need to insert this in some README. Do you have suggestion for it?
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.
nice to see another comprehensive example @Andrea-Scuderi
some comments inline
@@ -18,12 +18,20 @@ let package = Package( | |||
.executable(name: "APIGateway", targets: ["APIGateway"]), | |||
// fully featured example with domain specific business logic | |||
.executable(name: "CurrencyExchange", targets: ["CurrencyExchange"]), |
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.
@tomerd / @fabianfett we need to remove all executables from the products
. If you export it as a product
, then other libraries can depend on them. swift run
works on all targets
, so just leave out everything that's not meant for other packages to depend on.
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.
Thanks @fabianfett for pointing out to me that this isn't the main Package.swift. So we don't need to remove those here. They still aren't necessary but not an issue to have them.
7f9c268
to
21ccb0d
Compare
Fix ProductLambda init
6cbf22b
to
074b7a0
Compare
hi @Andrea-Scuderi how do you want to proceed with this PR? note some thoughts about such additional examples in https://forums.swift.org/t/examples-and-getting-started-guides/ |
hi @Andrea-Scuderi if you want to keep this around, could you please update this PR to be against the |
updated here #175 |
Motivation
Add an Example to show how to build a Serverless REST API using: