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

router.to_async(), simple async/.await examples #281

Merged
merged 14 commits into from
Jun 3, 2020
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ members = [
"examples/handlers/request_data",
"examples/handlers/stateful",
"examples/handlers/simple_async_handlers",
"examples/handlers/simple_async_handlers_await",
"examples/handlers/async_handlers",
"examples/handlers/form_urlencoded",
"examples/handlers/multipart",
Expand Down
1 change: 1 addition & 0 deletions examples/handlers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ We recommend reviewing our handler examples in the order shown below:
1. [Request Data](request_data) - Accessing common request information
1. [Stateful Handlers](stateful) - Keeping state in a handler
1. [Simple Async Handlers](simple_async_handlers) - Async Request Handlers 101
1. [Simple Async Handlers (.await version)](simple_async_handlers_await) - Request Handlers that use async/.await
1. [Async Handlers](async_handlers) - More complicated async request handlers

## Help
Expand Down
3 changes: 3 additions & 0 deletions examples/handlers/simple_async_handlers/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ find yourself doing lots of CPU/memory intensive operations on the web server,
then futures are probably not going to help your performance, and you might be
better off spawning a new thread per request.

If you came here looking for an example that uses async/.await, please read
[Async Request Handlers (.await version)](../simple_async_handlers).
Copy link
Member

Choose a reason for hiding this comment

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

I believe this link is incorrect as it points to itself if I'm not mistaken

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot. Thanks.


## Running

From the `examples/handlers/async_handlers` directory:
Expand Down
17 changes: 17 additions & 0 deletions examples/handlers/simple_async_handlers_await/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[package]
name = "gotham_examples_handlers_simple_async_handlers_await"
description = "An example that does asynchronous work before responding"
version = "0.0.0"
authors = ["David Laban <alsuren@gmail.com>"]
publish = false
edition = "2018"

[dependencies]
gotham = { path = "../../../gotham" }
gotham_derive = { path = "../../../gotham_derive" }

mime = "0.3.16"
futures = "0.3.1"
serde = "1.0.104"
serde_derive = "1.0.104"
tokio = "0.2.9"
49 changes: 49 additions & 0 deletions examples/handlers/simple_async_handlers_await/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Async Request Handlers (.await version)

The idea of async handlers has already been introduced by the post_handler example in
[Request Data](../request_data), which waits for the POST body asyncronously, and resolves
the response future once it has processed the body. The combinator-based version
of this example can be found at [Async Request Handlers](../simple_async_handlers).

This example has exactly the same behavior and API as the combinator-based version,
and it can be used as a reference when converting your code to use async/await.

## Running

From the `examples/handlers/async_handlers` directory:

```
Terminal 1:
Compiling gotham_examples_handlers_simple_async_handlers v0.0.0 (file:///.../gotham/examples/handlers/simple_async_handlers)
Finished dev [unoptimized + debuginfo] target(s) in 8.19 secs
Running `.../gotham/target/debug/gotham_examples_handlers_simple_async_handlers`
Listening for requests at http://127.0.0.1:7878
sleep for 5 seconds once: starting
sleep for 5 seconds once: finished
sleep for one second 5 times: starting
sleep for one second 5 times: finished
Terminal 2:
$ curl 'http://127.0.0.1:7878/sleep?seconds=5'
slept for 5 seconds
$ curl 'http://127.0.0.1:7878/loop?seconds=5'
slept for 1 seconds
slept for 1 seconds
slept for 1 seconds
slept for 1 seconds
slept for 1 seconds
```

## License

Licensed under your option of:

* [MIT License](../../LICENSE-MIT)
* [Apache License, Version 2.0](../../LICENSE-APACHE)

## Community

The following policies guide participation in our project and our community:

* [Code of conduct](../../CODE_OF_CONDUCT.md)
* [Contributing](../../CONTRIBUTING.md)
162 changes: 162 additions & 0 deletions examples/handlers/simple_async_handlers_await/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
//! A basic example showing the request components

use futures::prelude::*;
use std::pin::Pin;
use std::time::{Duration, Instant};

use gotham::hyper::{Body, StatusCode};

use gotham::handler::HandlerResult;
use gotham::helpers::http::response::create_response;
use gotham::router::builder::DefineSingleRoute;
use gotham::router::builder::{build_simple_router, DrawRoutes};
use gotham::router::Router;
use gotham::state::{FromState, State};
use gotham_derive::{StateData, StaticResponseExtender};
use serde_derive::Deserialize;

use tokio::time::delay_until;

type SleepFuture = Pin<Box<dyn Future<Output = Vec<u8>> + Send>>;

#[derive(Deserialize, StateData, StaticResponseExtender)]
struct QueryStringExtractor {
seconds: u64,
}

/// Sneaky hack to make tests take less time. Nothing to see here ;-).
#[cfg(not(test))]
fn get_duration(seconds: u64) -> Duration {
Duration::from_secs(seconds)
}
#[cfg(test)]
fn get_duration(seconds: u64) -> Duration {
Duration::from_millis(seconds)
}
/// All this function does is return a future that resolves after a number of
/// seconds, with a Vec<u8> that tells you how long it slept for.
///
/// Note that it does not block the thread from handling other requests, because
/// it returns a `Future`, which will be managed by the tokio reactor, and
/// called back once the timeout has expired.
///
/// Vec<u8> is chosen because it is one of the things that you need to resolve
/// a HandlerFuture and respond to a request.
///
/// Most things that you call to access remote services (e.g databases and
/// web apis) can be coerced into returning futures that yield useful data,
/// so the patterns that you learn in this example should be applicable to
/// real world problems.
fn sleep(seconds: u64) -> SleepFuture {
let when = Instant::now() + get_duration(seconds);
let delay = delay_until(when.into()).map(move |_| {
format!("slept for {} seconds\n", seconds)
.as_bytes()
.to_vec()
});

delay.boxed()
}

/// This handler sleeps for the requested number of seconds, using the `sleep()`
/// helper method, above.
async fn sleep_handler(mut state: State) -> HandlerResult {
let seconds = QueryStringExtractor::take_from(&mut state).seconds;
println!("sleep for {} seconds once: starting", seconds);
// Here, we call the sleep function and turn its old-style future into
// a new-style future. Note that this step doesn't block: it just sets
// up the timer so that we can use it later.
let sleep_future = sleep(seconds);

// Here is where the serious sleeping happens. We yield execution of
// this block until sleep_future is resolved.
// The Ok("slept for x seconds") value is stored in result.
let data = sleep_future.await;

// Here, we convert the result from `sleep()` into the form that Gotham
// expects: `state` is owned by this block so we need to return it.
// We also convert any errors that we have into the form that Hyper
// expects, using the helper from IntoHandlerError.
let res = create_response(&state, StatusCode::OK, mime::TEXT_PLAIN, data);
println!("sleep for {} seconds once: finished", seconds);
Ok((state, res))
}

/// It calls sleep(1) as many times as needed to make the requested duration.
///
/// Notice how much easier it is to read than the version in
/// `simple_async_handlers`.
async fn loop_handler(mut state: State) -> HandlerResult {
let seconds = QueryStringExtractor::take_from(&mut state).seconds;
println!("sleep for one second {} times: starting", seconds);

// The code within this block reads exactly like syncronous code.
// This is the style that you should aim to write your business
// logic in.
let mut accumulator = Vec::new();
for _ in 0..seconds {
let body = sleep(1).await;
accumulator.extend(body)
}

let res = create_response(
&state,
StatusCode::OK,
mime::TEXT_PLAIN,
Body::from(accumulator),
);
println!("sleep for one second {} times: finished", seconds);
Ok((state, res))
}

/// Create a `Router`.
fn router() -> Router {
build_simple_router(|route| {
route
.get("/sleep")
.with_query_string_extractor::<QueryStringExtractor>()
.to_async(sleep_handler);
route
.get("/loop")
.with_query_string_extractor::<QueryStringExtractor>()
.to_async(loop_handler);
})
}

/// Start a server and use a `Router` to dispatch requests.
pub fn main() {
let addr = "127.0.0.1:7878";
println!("Listening for requests at http://{}", addr);
gotham::start(addr, router())
}

#[cfg(test)]
mod tests {
use gotham::test::TestServer;

use super::*;

fn assert_returns_ok(url_str: &str, expected_response: &str) {
let test_server = TestServer::new(router()).unwrap();
let response = test_server.client().get(url_str).perform().unwrap();

assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
&String::from_utf8(response.read_body().unwrap()).unwrap(),
expected_response
);
}

#[test]
fn sleep_says_how_long_it_slept_for() {
assert_returns_ok("http://localhost/sleep?seconds=2", "slept for 2 seconds\n");
}

#[test]
fn loop_breaks_the_time_into_one_second_sleeps() {
assert_returns_ok(
"http://localhost/loop?seconds=2",
"slept for 1 seconds\nslept for 1 seconds\n",
);
}
}
6 changes: 4 additions & 2 deletions gotham/src/handler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ pub mod assets;

pub use self::error::{HandlerError, IntoHandlerError};

/// A type alias for the results returned by async fns that can be passed to to_async.
pub type HandlerResult = std::result::Result<(State, Response<Body>), (State, HandlerError)>;

/// A type alias for the trait objects returned by `HandlerService`.
///
/// When the `Future` resolves to an error, the `(State, HandlerError)` value is used to generate
/// an appropriate HTTP error response.
pub type HandlerFuture =
dyn Future<Output = std::result::Result<(State, Response<Body>), (State, HandlerError)>> + Send;
pub type HandlerFuture = dyn Future<Output = HandlerResult> + Send;

/// A `Handler` is an asynchronous function, taking a `State` value which represents the request
/// and related runtime state, and returns a future which resolves to a response.
Expand Down
56 changes: 55 additions & 1 deletion gotham/src/router/builder/single.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,17 @@ use std::panic::RefUnwindSafe;

use crate::extractor::{PathExtractor, QueryStringExtractor};
use crate::handler::assets::{DirHandler, FileHandler, FileOptions, FilePathExtractor};
use crate::handler::{Handler, NewHandler};
use crate::handler::{Handler, HandlerResult, NewHandler};
use crate::pipeline::chain::PipelineHandleChain;
use crate::router::builder::{
ExtendRouteMatcher, ReplacePathExtractor, ReplaceQueryStringExtractor, SingleRouteBuilder,
};
use crate::router::route::dispatch::DispatcherImpl;
use crate::router::route::matcher::RouteMatcher;
use crate::router::route::{Delegation, Extractors, RouteImpl};
use crate::state::State;
use core::future::Future;
use futures::FutureExt;

/// Describes the API for defining a single route, after determining which request paths will be
/// dispatched here. The API here uses chained function calls to build and add the route into the
Expand Down Expand Up @@ -106,6 +109,57 @@ pub trait DefineSingleRoute {
where
H: Handler + RefUnwindSafe + Copy + Send + Sync + 'static;

/// Similar to `to`, but accepts an `async fn`
///
/// # Examples
///
/// ```rust
/// # extern crate gotham;
/// # extern crate hyper;
/// #
/// # use hyper::{Body, Response, StatusCode};
/// # use gotham::handler::HandlerResult;
/// # use gotham::state::State;
/// # use gotham::router::Router;
/// # use gotham::router::builder::*;
/// # use gotham::pipeline::new_pipeline;
/// # use gotham::pipeline::single::*;
/// # use gotham::middleware::session::NewSessionMiddleware;
/// # use gotham::test::TestServer;
/// #
/// async fn my_handler(state: State) -> HandlerResult {
/// // Handler implementation elided.
/// # Ok((state, Response::builder().status(StatusCode::ACCEPTED).body(Body::empty()).unwrap()))
/// }
/// #
/// # fn router() -> Router {
/// # let (chain, pipelines) = single_pipeline(
/// # new_pipeline().add(NewSessionMiddleware::default()).build()
/// # );
///
/// build_router(chain, pipelines, |route| {
/// route.get("/request/path").to_async(my_handler);
/// })
/// #
/// # }
/// #
/// # fn main() {
/// # let test_server = TestServer::new(router()).unwrap();
/// # let response = test_server.client()
/// # .get("https://example.com/request/path")
/// # .perform()
/// # .unwrap();
/// # assert_eq!(response.status(), StatusCode::ACCEPTED);
/// # }
/// ```
fn to_async<H, Fut>(self, handler: H)
where
Self: Sized,
H: (FnOnce(State) -> Fut) + RefUnwindSafe + Copy + Send + Sync + 'static,
Fut: Future<Output = HandlerResult> + Send + 'static,
{
self.to_new_handler(move || Ok(move |s: State| handler(s).boxed()))
}
Copy link
Contributor Author

@alsuren alsuren Jan 21, 2020

Choose a reason for hiding this comment

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

I have implemented this directly on the trait because it feels like the most backwards-compatible thing to do.

In practice, the only implementation of DefineSingleRoute in the gotham codebase is SingleRouteBuilder. Would you rather I implement it on SingleRouteBuilder and force all external implementors of DefineSingleRoute to implement it?

Also, can someone look extra-hard at the marker traits that I've used on H and Fut, and make sure they're all appropriate? (I copied some of them from .to(), and I'm not sure they're all needed)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure. In a world where you don't care about backwards compatibility, which would you go for? The versioning model we are on allows us to break things, and we can provide documentation for migration if it's complicated.

The traits seem ok to me (when looking at a diff). I feel like Copy and 'static might be unnecessary...?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, the .to should map to (State, Response<Body>) (normal fn), Pin<Box<HandlerFuture>> (future fn) and HandlerResult (async fn).

We can take it one step forward and disallow the mapping to (State, Response<Body>) and replace it with HandlerResult for consistency.

Copy link
Contributor

@pksunkara pksunkara Feb 20, 2020

Choose a reason for hiding this comment

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

If both future fn and async fn could not be supported at the same time and we are willing to break, then we should give priority to async fn and create a new to_future or something for the future fn.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@whitfin that sounds like the right question to ask. Good thinking. In an ideal world, I would probably think about making this trait private or something (I think that a recent compiler release that allows traits to be marked nonexhaustive, so they can't be implemented by external people?). In the short term, "program to the interface" seems to be the pattern that's being used here, so refactoring the implementation down into SingleRouteBuilder sounds like the thing to do. I will add this to the checklist, and do it once I have all of the other questions worked out.

@pksunkara: (I think that this is a separate discussion) as it stands in my PR (using your terminology) I can pass a "future fn" to .to_async(), because Pin<Box> implements Future<Output = HandlerResult>, but I can't pass "normal fn".

When I wrote this, I didn't think it's possible to create a single function that can accept both an impl Trait and a struct. I seem to remember, rustc complaining that my blanket impl HandlerFuture for impl Future might conflict with my impl HandlerFuture for (State, Response<Body>) if someone else implements Future for (State, Response<Body>)... or something. You can get away with it for Pin<Box<HandlerFuture>> because in that case your impl is over two struct types that the compiler can guarantee won't overlap. This was last year though, so I might be misremembering, or the compiler might have become more lenient since then.

I've been mulling it over in my head over the last couple of days, and I think that it might be possible to impl Future for (State, Response<Body>), and get on with our lives. Do you think that would work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=19c7034bdda2cb834f574c041788111f

error[E0117]: only traits defined in the current crate can be implemented for arbitrary types
 --> src/lib.rs:7:1
  |
7 | impl Future for (State, String) {
  | ^^^^^^^^^^^^^^^^---------------
  | |               |
  | |               this is not defined in the current crate because tuples are always foreign
  | impl doesn't use only types from inside the current crate
  |
  = note: define and implement a trait or new type instead

:(

Copy link
Contributor

@pksunkara pksunkara Feb 22, 2020

Choose a reason for hiding this comment

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

How about making the return object HandlerResult for normal fn?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same deal: HandlerResult is just a Result that's generic over tuples, so it' also foreign.

9 | impl Future for HandlerResult {
  | ^^^^^^^^^^^^^^^^-------------
  | |               |
  | |               `std::result::Result` is not defined in the current crate
  | impl doesn't use only types from inside the current crate

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=1e88ddbdf314a06d0871b2b7fe10ee82

I think that we need a separate .to() for tuples vs .to_async() for futures. It might be that we should call them .to_sync() and .to() though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The traits seem ok to me (when looking at a diff). I feel like Copy and 'static might be unnecessary...?

I had a quick play about and I couldn't see a way to get rid of them.

The handler callback is FnOnce + Copy. This is a bit like Fn, but a bit easier to implement with closures or something? If I remove the + Copy then I end up with a handler function that can only be run once.

+ 'static is required by .to_new_handler(). It feels a bit excessive at first glance, but I think it's fine: the routes we define normally live for the entire duration of the program, and are normally implemented as top-level fns. I haven't really dug into whether there is a more appropriate lifetime than this, so I'm just going to keep what's there.

/// Directs the route to the given `NewHandler`. This gives more control over how `Handler`
/// values are constructed.
///
Expand Down