Skip to content

introduce the ideas of Before and After middlewares#556

Merged
yoshuawuyts merged 3 commits intohttp-rs:masterfrom
jbr:before-and-after-middlewares
May 29, 2020
Merged

introduce the ideas of Before and After middlewares#556
yoshuawuyts merged 3 commits intohttp-rs:masterfrom
jbr:before-and-after-middlewares

Conversation

@jbr
Copy link
Copy Markdown
Member

@jbr jbr commented May 28, 2020

For discussion, a functional sketch

context

Due to the lack of AsyncFn types in rust currently, we cannot describe the bounds for an async function that has a lifetime in the arguments. As a result, there's no way to write

// this example is not possible
app.middleware(|req: Request<()>, next: Next<()>| async move {
  next.run(req).await
});

and instead function middlewares look like:

// current signature
fn minimal_middleware<'a>(
    request: Request<()>,
    next: Next<'a, ()>,
) -> Pin<Box<dyn Future<Output = Result> + Send + 'a>> {
    Box::pin(async { next.run(request).await })
}

//--snip--

app.middleware(minimal_middleware);

proposed solution

Because the issue is that Next is not owned, it is possible to write simple async signatures for middlewares that exist on either side of the Next call. This PR called them Before and After middlewares currently, but those names probably can be improved. This PR is predicated on an assumption that a meaningful percentage of app-written middlewares exist as transformations of the request OR the response. This enables the following code.

// possible with this pr
app.middleware(Before(|request: Request<()>| async move { request })));
app.middleware(After(|result: Result| async move { result })));

@jbr jbr requested review from Fishrock123 and yoshuawuyts May 28, 2020 23:02
@jbr jbr force-pushed the before-and-after-middlewares branch 3 times, most recently from 1cb9af2 to a017d32 Compare May 28, 2020 23:21
@jbr jbr force-pushed the before-and-after-middlewares branch from a017d32 to 836e58f Compare May 28, 2020 23:25
Copy link
Copy Markdown
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

Overall this LGTM. We should add docs though so folks get a sense of what this is about.

Perhaps we should point out that once async closures / async traits land we no longer need this as people should be able to just write middleware inline using closures.

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.

2 participants