feat: Ensure services in axum `#[middleware()]` macro Implement Clone

thlsrms commented 3 months ago

This change requires that services utilized within the #[middleware()] macro for server functions to implement Clone. This ensures that the service being wrapped by the middleware layer can also be cloned, useful for moving a clone of the service into a boxed future to be polled next:

fn call(&mut self, req: Request) -> Self::Future {
    let not_ready_inner = self.inner.clone();
    let ready_inner = std::mem::replace(&mut self.inner, not_ready_inner);

    let future = Box::pin(async move {
        /*
            Async operations
        */
        let inner = ServiceBuilder::new()
            .boxed_clone()
            .map_response(IntoResponse::into_response)
            .service(ready_inner);
        let next = Next { inner };
    };

    ResponseFuture {
        inner: future
    }
}

fn call(&mut self, req: Request<ReqBody>) -> Self::Future {
    let inner = self.inner.clone();
    let authorization = Box::pin(async move {
        /* Query the DB and authorize this user */
    });

    ResponseFuture {
        authorization,
        inner,
    }
}

gbj commented 3 months ago

I just want to flag the difference between "it's useful to allow services to be cloned" and "therefore we should (make a breaking change to) require services to be Clone".

I'm not particularly familiar with the Tower ecosystem beyond what I needed to know of the APIs to implement this a few months ago, so I am not sure how common/uncommon cloning services is.

I wonder if it's possible to achieve the goal of allowing services to be cloned without the breaking change to require them to be cloneable.

thlsrms commented 3 months ago

I just want to flag the difference between "it's useful to allow services to be cloned" and "therefore we should (make a breaking change to) require services to be Clone".

That's fair, and badly worded on my part.

I'm not particularly familiar with the Tower ecosystem beyond what I needed to know of the APIs to implement this a few months ago, so I am not sure how common/uncommon cloning services is.

Cloning is necessary when transferring the inner service's ownership to the Future. This is needed when moving the request into an async block, which is often done to access or modify it asynchronously before passing it to the next service call, or when moving the inner service itself into the async block. Because of &mut self we end up with no other options.

That is until tower, hopefully some day, get blessed by the async trait.

I wonder if it's possible to achieve the goal of allowing services to be cloned without the breaking change to require them to be cloneable.

I'm not sure if this is possible. For a moment I thought that tower::ServiceBuider would be able to do that with boxed_clone but it requires the service to already be Clone

gbj commented 3 months ago

I guess the alternative is to have BoxedService actually be an Arc<Mutex<_>> rather than a Box<_>.

thlsrms commented 2 months ago

I came back to this today, saw your comment and noticed that I was too caught up in the way axum handles their middleware layers.

So I revisited the way I was doing it and came up with this macro that constructs the middleware stack from a sequence of Fn(Request<Body>) -> impl Future<Output = Result<Request<Body>, Response<Body>>> functions:

https://github.com/thlsrms/leptos-axum-login-middleware/blob/4286c48/src/middlewares/macros.rs#L40-L76

#[macro_export]
macro_rules! compose_from_fn {
    ($($func:expr),+) => {{
        use std::sync::Arc;
        use futures_util::future::{BoxFuture, FutureExt};
        use http::{Request, Response };
        use axum::body::Body;

        // Returns a single MiddlewareFn from a vector of MiddlewareFn
        // composing the middleware stack
        let composite_func = {
            Arc::new(move |req: Request<Body>| {
                let funcs = vec![$(move |req| ($func)(req).boxed()),*];
                let mut future = Box::pin(async move { Ok(req) })
                    as BoxFuture<'static, Result<Request<Body>, Response<Body>>>;

                for func in funcs.into_iter() {
                    let prev_future = future;
                    future = Box::pin(async move {
                        let req = prev_future.await?;
                        func(req).await
                    });
                }

                future
            })
        };

        $crate::middlewares::MiddlewareLayer::new(composite_func)
    }};
}

usage:

#[server]
#[middleware(compose_from_fn!(require_login, |req| auth_role(req, auth::Role::User)))]
async fn user_protected_data() -> Result<(), ServerFnError> {
    Ok(())
}

I'm not sure where in Leptos this utility would best fit if I were to make it into a PR. I'm leaving the linked repo as example anyway and closing this PR.

leptos-rs / leptos

feat: Ensure services in axum `#[middleware()]` macro Implement Clone #2664