Skip to content

RFC: Support borrowed deserializaton #309

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

RFC: Support borrowed deserializaton #309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
62aa0ab
borrowed deserialization
duarten Mar 26, 2021
52f31e1
update rust version
duarten Mar 26, 2021
f95b1f6
fixes
duarten Mar 27, 2021
de3c7f6
fixes to examples
duarten Mar 27, 2021
50a88f2
fmt
duarten Mar 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
- os: macOS-latest
target: x86_64-unknown-linux-musl
- os: ubuntu-latest
rust: 1.40.0
rust: 1.51.0
target: x86_64-unknown-linux-musl
- os: ubuntu-latest
rust: beta
Expand All @@ -34,7 +34,7 @@ jobs:
rust: nightly
target: x86_64-unknown-linux-musl
- os: macOS-latest
rust: 1.40.0
rust: 1.51.0
- os: macOS-latest
rust: beta
- os: macOS-latest
Expand Down
8 changes: 2 additions & 6 deletions lambda-http/examples/hello-http.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,10 @@
use lambda_http::{
handler,
lambda_runtime::{self, Context},
IntoResponse, Request, RequestExt, Response,
};
use lambda_http::{handler, lambda_runtime::Context, IntoResponse, Request, RequestExt, Response};

type Error = Box<dyn std::error::Error + Send + Sync + 'static>;

#[tokio::main]
async fn main() -> Result<(), Error> {
lambda_runtime::run(handler(func)).await?;
lambda_http::run(handler(func)).await?;
Ok(())
}

Expand Down
4 changes: 2 additions & 2 deletions lambda-http/src/ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl Error for PayloadError {
/// as well as `{"x":1, "y":2}` respectively.
///
/// ```rust,no_run
/// use lambda_http::{handler, lambda_runtime::{self, Context}, Body, IntoResponse, Request, Response, RequestExt};
/// use lambda_http::{handler, lambda_runtime::Context, Body, IntoResponse, Request, Response, RequestExt};
/// use serde::Deserialize;
///
/// type Error = Box<dyn std::error::Error + Send + Sync + 'static>;
Expand All @@ -81,7 +81,7 @@ impl Error for PayloadError {
///
/// #[tokio::main]
/// async fn main() -> Result<(), Error> {
/// lambda_runtime::run(handler(add)).await?;
/// lambda_http::run(handler(add)).await?;
/// Ok(())
/// }
///
Expand Down
64 changes: 52 additions & 12 deletions lambda-http/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@
//! your function's execution path.
//!
//! ```rust,no_run
//! use lambda_http::{handler, lambda_runtime::{self, Error}};
//! use lambda_http::{handler, lambda_runtime::Error};
//!
//! #[tokio::main]
//! async fn main() -> Result<(), Error> {
//! // initialize dependencies once here for the lifetime of your
//! // lambda task
//! lambda_runtime::run(handler(|request, context| async { Ok("👋 world!") })).await?;
//! lambda_http::run(handler(|request, context| async { Ok("👋 world!") })).await?;
//! Ok(())
//! }
//! ```
Expand All @@ -34,11 +34,11 @@
//! with the [`RequestExt`](trait.RequestExt.html) trait.
//!
//! ```rust,no_run
//! use lambda_http::{handler, lambda_runtime::{self, Context, Error}, IntoResponse, Request, RequestExt};
//! use lambda_http::{handler, lambda_runtime::{Context, Error}, IntoResponse, Request, RequestExt};
//!
//! #[tokio::main]
//! async fn main() -> Result<(), Error> {
//! lambda_runtime::run(handler(hello)).await?;
//! lambda_http::run(handler(hello)).await?;
//! Ok(())
//! }
//!
Expand All @@ -62,7 +62,7 @@
extern crate maplit;

pub use http::{self, Response};
pub use lambda_runtime::{self, Context};
pub use lambda_runtime::{self, Context, Deserializable};
use lambda_runtime::{Error, Handler as LambdaHandler};

mod body;
Expand All @@ -76,6 +76,7 @@ use crate::{
response::LambdaResponse,
};
use std::{
fmt,
future::Future,
pin::Pin,
task::{Context as TaskContext, Poll},
Expand All @@ -89,9 +90,9 @@ pub type Request = http::Request<Body>;
/// This can be viewed as a `lambda_runtime::Handler` constrained to `http` crate `Request` and `Response` types
pub trait Handler: Sized {
/// The type of Error that this Handler will return
type Error;
type Error: fmt::Display + Send + Sync;
/// The type of Response this Handler will return
type Response: IntoResponse;
type Response: IntoResponse + Send + Sync;
Comment on lines -92 to +95
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these bounds need to be added? If they're necessary, we'd need to prepare a new breaking change, which I'd rather avoid if not necessary.

Copy link
Author

Choose a reason for hiding this comment

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

These bounds are lifted from run(), so in that sense they're not really breaking (any Handler would already need to satisfy those bounds for it to be useful).

/// The type of Future this Handler will return
type Fut: Future<Output = Result<Self::Response, Self::Error>> + Send + 'static;
/// Function used to execute handler behavior
Expand All @@ -107,7 +108,7 @@ pub fn handler<H: Handler>(handler: H) -> Adapter<H> {
impl<F, R, Fut> Handler for F
where
F: Fn(Request, Context) -> Fut,
R: IntoResponse,
R: IntoResponse + Send + Sync,
Copy link
Contributor

Choose a reason for hiding this comment

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

(Repeating this comment) Do these bounds need to be added? If they're necessary, we'd need to prepare a new breaking change, which I'd rather avoid if not necessary.

Fut: Future<Output = Result<R, Error>> + Send + 'static,
{
type Response = R;
Expand All @@ -126,7 +127,8 @@ pub struct TransformResponse<R, E> {

impl<R, E> Future for TransformResponse<R, E>
where
R: IntoResponse,
R: IntoResponse + Send + Sync,
E: Send + Sync,
Comment on lines +130 to +131
Copy link
Contributor

Choose a reason for hiding this comment

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

(Repeating this comment) Do these bounds need to be added? If they're necessary, we'd need to prepare a new breaking change, which I'd rather avoid if not necessary.

{
type Output = Result<LambdaResponse, E>;
fn poll(mut self: Pin<&mut Self>, cx: &mut TaskContext) -> Poll<Self::Output> {
Expand Down Expand Up @@ -159,13 +161,51 @@ impl<H: Handler> Handler for Adapter<H> {
}
}

impl<H: Handler> LambdaHandler<LambdaRequest<'_>, LambdaResponse> for Adapter<H> {
impl<'de, H, A> LambdaHandler<'de, A, LambdaResponse> for Adapter<H>
where
A: Deserializable<'de, Deserialize = LambdaRequest<'de>>,
H: Handler + Send + Sync + 'static,
<H as Handler>::Error: fmt::Display + Send + Sync,
H::Response: Send + Sync,
{
type Error = H::Error;
type Fut = TransformResponse<H::Response, Self::Error>;
type Fut = TransformResponse<H::Response, H::Error>;

fn call(&self, event: LambdaRequest<'_>, context: Context) -> Self::Fut {
fn call(&self, event: LambdaRequest<'de>, context: Context) -> Self::Fut {
let request_origin = event.request_origin();
let fut = Box::pin(self.handler.call(event.into(), context));
TransformResponse { request_origin, fut }
}
}

/// Starts the Lambda Rust runtime and begins polling for events on the [Lambda
/// Runtime APIs](https://docs.aws.amazon.com/lambda/latest/dg/runtimes-api.html).
///
/// # Example
/// ```no_run
/// use lambda_http::{handler, Context, IntoResponse, Request, RequestExt};
/// use serde_json::Value;
///
/// type Error = Box<dyn std::error::Error + Send + Sync + 'static>;
///
/// #[tokio::main]
/// async fn main() -> Result<(), Error> {
/// lambda_http::run(handler(func)).await?;
/// Ok(())
/// }
///
/// async fn func(event: Request, _: Context) -> Result<impl IntoResponse, Error> {
/// Ok(format!("Hello, {}!", event.query_string_parameters().get("first_name").unwrap()).into_response())
/// }
/// ```
pub async fn run<H>(adapter: Adapter<H>) -> Result<(), Error>
where
H: Handler + Send + Sync + 'static,
<H as Handler>::Error: std::fmt::Display + Send,
{
struct LambdaRequestBorrowed {}
impl<'de> Deserializable<'de> for LambdaRequestBorrowed {
type Deserialize = LambdaRequest<'de>;
}
lambda_runtime::run::<LambdaRequestBorrowed, _, _>(adapter).await
}
53 changes: 53 additions & 0 deletions lambda-runtime/examples/basic-borrowed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// This example requires the following input to succeed:
// { "command": "do something" }

use lambda_runtime::{handler_fn, Context, Deserializable, Error};
use log::LevelFilter;
use serde::{Deserialize, Serialize};
use simple_logger::SimpleLogger;

/// This is also a made-up example. Requests come into the runtime as unicode
/// strings in json format, which can map to any structure that implements `serde::Deserialize`
/// The runtime pays no attention to the contents of the request payload.
#[derive(Deserialize)]
struct Request<'a> {
command: &'a str,
}

/// This is a made-up example of what a response structure may look like.
/// There is no restriction on what it can be. The runtime requires responses
/// to be serialized into json. The runtime pays no attention
/// to the contents of the response payload.
#[derive(Serialize)]
struct Response {
req_id: String,
msg: String,
}

#[tokio::main]
async fn main() -> Result<(), Error> {
// required to enable CloudWatch error logging by the runtime
// can be replaced with any other method of initializing `log`
SimpleLogger::new().with_level(LevelFilter::Info).init().unwrap();

let func = handler_fn(my_handler);
struct BorrowedRequest {}
impl<'de> Deserializable<'de> for BorrowedRequest {
type Deserialize = Request<'de>;
}
lambda_runtime::run::<BorrowedRequest, _, _>(func).await?;
Ok(())
}

pub(crate) async fn my_handler(event: Request<'_>, ctx: Context) -> Result<Response, Error> {
// extract some useful info from the request
let command = event.command;
// prepare the response
let resp = Response {
req_id: ctx.request_id,
msg: format!("Command {} executed.", command),
};

// return `Response` (it will be serialized to JSON automatically by the runtime)
Ok(resp)
}
2 changes: 1 addition & 1 deletion lambda-runtime/examples/basic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ async fn main() -> Result<(), Error> {
SimpleLogger::new().with_level(LevelFilter::Info).init().unwrap();

let func = handler_fn(my_handler);
lambda_runtime::run(func).await?;
lambda_runtime::run::<Request, _, _>(func).await?;
Copy link
Contributor

@rimutaka rimutaka Mar 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

A comment about _,_> here would be good.

To be honest, I don't understand how this change works and why we need lambda_runtime::run::<Value, _, _>(func).await?;, but that's just me not know how GATs and SERDE well enough. What I think would be good is to explain what the _,_> part means, why need it and if it should be copied as-is or there can be something in place of _. Quite often _ is used in examples for brevity, which raises the question, what do I put in there in my case? Do I go with _ or do I have to provide some value?

A macro will definitely help resolve the confusion for dummies like me.

Copy link
Author

Choose a reason for hiding this comment

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

I think a final version of this PR will definitely include the macro - there's just a lot of incidental complexity without it. The _'s are currently used for brevity, because we are required to provide the first generic parameter. Because that first one is different from the actual handler argument for borrowed deserialize, there's no way for the compiler to infer it. That's not true for the handler itself and the return type, so we can just say _.

Copy link
Contributor

Choose a reason for hiding this comment

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

@duarten, thank you for explaining it to me. I think I get it now :)
I paraphrased your explanation as a comment:

// The 1st param (Request) has to be specified every time because the compiler cannot infer it.
// The 2nd and 3rd parameters can use their default types and be omitted (_).

Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion lambda-runtime/examples/error-handling.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ async fn main() -> Result<(), Error> {

// call the actual handler of the request
let func = handler_fn(func);
lambda_runtime::run(func).await?;
lambda_runtime::run::<Value, _, _>(func).await?;
Ok(())
}

Expand Down
17 changes: 11 additions & 6 deletions lambda-runtime/src/client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ mod endpoint_tests {
},
simulated,
types::Diagnostic,
Error, Runtime,
Error, Handler, Runtime,
};
use http::{uri::PathAndQuery, HeaderValue, Method, Request, Response, StatusCode, Uri};
use hyper::{server::conn::Http, service::service_fn, Body};
Expand Down Expand Up @@ -252,6 +252,15 @@ mod endpoint_tests {
}
}

async fn do_run<F>(runtime: Runtime<simulated::Connector>, handler: F) -> Result<(), Error>
where
F: for<'de> Handler<'de, serde_json::Value, serde_json::Value> + Send + Sync + 'static,
{
let client = &runtime.client;
let incoming = incoming(client).take(1);
Ok(runtime.run(incoming, handler).await?)
}

#[tokio::test]
async fn successful_end_to_end_run() -> Result<(), Error> {
let (client, server) = io::duplex(64);
Expand All @@ -272,11 +281,7 @@ mod endpoint_tests {
async fn func(event: serde_json::Value, _: crate::Context) -> Result<serde_json::Value, Error> {
Ok(event)
}
let f = crate::handler_fn(func);

let client = &runtime.client;
let incoming = incoming(client).take(1);
runtime.run(incoming, f).await?;
do_run(runtime, crate::handler_fn(func)).await?;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why this code was taken out into a separate function? The previous version was easier to follow. Just my 2c.

Copy link
Author

Choose a reason for hiding this comment

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

That's because of rust-lang/rust#44721, we currently can't mix impl Trait and explicit generic arguments.


// shutdown server
tx.send(()).expect("Receiver has been dropped");
Expand Down
Loading