diff options
| author | Owen Jacobson <owen@grimoire.ca> | 2025-06-17 00:46:24 -0400 |
|---|---|---|
| committer | Owen Jacobson <owen@grimoire.ca> | 2025-06-17 01:08:19 -0400 |
| commit | 424fb08ecd315c67dd3862c29e87eea7bf32f65c (patch) | |
| tree | bf85cba9c93766b8651d16de9984bbeb48d58fdf /src/ui | |
| parent | 4a7fb2c5cf7265c5ef6a78051c1bb73e7d3ef086 (diff) | |
Unify `setup_required` middlewares.
The two middlewares were identical but for the specific `IntoResponse` impl used to generate the response when setup has not been completed. However, unifying them while still using `from_fn_with_state` lead to this horrorshow:
.route_layer(middleware::from_fn_with_state(
app.clone(),
|state, req, next| {
setup::middeware::setup_required(UNAVAILABLE, state, req, next)
}
))
It's a lot to read, and it surfaces the entire signature of a state-driven middleware `fn` into the call site solely to close over one argument (`UNAVAILABLE`).
Rather than doing that, I've converted this middleware into a full blown Tower middleware, following <https://docs.rs/axum/latest/axum/middleware/index.html#towerservice-and-pinboxdyn-future>. I considered taking this further and implementing a custom future to remove the allocation for `Box::pin`, but honestly, that allocation isn't hurting anyone and this code already got long enough in the translation.
The new API looks like:
.route_layer(setup::Required::or_unavailable(app.clone()))
Or like:
.route_layer(setup::Required::with_fallback(app.clone(), RESPONSE))
One thing I would have liked to have avoided is the additional `app.clone()` argument, but there isn't a way to extract the _state_ from a request inside of an Axum middleware. It has to be passed in externally - that's what `from_fn_with_state` is doing under the hood, as well. Using `State` as an extractor doesn't work; the `State` extractor is special in a _bunch_ of ways, and this is one of them. Other extractors would work. Realistically, I'd probably want to explore interfaces like
.route_layer(setup::Required(app).or_unavailable())
or
.route_layer(app.setup().required().or_unavailable())
Diffstat (limited to 'src/ui')
| -rw-r--r-- | src/ui/middleware.rs | 15 | ||||
| -rw-r--r-- | src/ui/mod.rs | 1 | ||||
| -rw-r--r-- | src/ui/routes/mod.rs | 9 |
3 files changed, 6 insertions, 19 deletions
diff --git a/src/ui/middleware.rs b/src/ui/middleware.rs deleted file mode 100644 index f60ee1c..0000000 --- a/src/ui/middleware.rs +++ /dev/null @@ -1,15 +0,0 @@ -use axum::{ - extract::{Request, State}, - middleware::Next, - response::{IntoResponse, Redirect, Response}, -}; - -use crate::{app::App, error::Internal}; - -pub async fn setup_required(State(app): State<App>, request: Request, next: Next) -> Response { - match app.setup().completed().await { - Ok(true) => next.run(request).await, - Ok(false) => Redirect::to("/setup").into_response(), - Err(error) => Internal::from(error).into_response(), - } -} diff --git a/src/ui/mod.rs b/src/ui/mod.rs index f8caa48..e834bba 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -1,6 +1,5 @@ mod assets; mod error; -mod middleware; mod mime; mod routes; diff --git a/src/ui/routes/mod.rs b/src/ui/routes/mod.rs index 80dc1e5..328eb73 100644 --- a/src/ui/routes/mod.rs +++ b/src/ui/routes/mod.rs @@ -1,6 +1,6 @@ -use axum::{Router, middleware, routing::get}; +use axum::{Router, response::Redirect, routing::get}; -use crate::{app::App, ui::middleware::setup_required}; +use crate::app::App; mod ch; mod get; @@ -21,7 +21,10 @@ pub fn router(app: &App) -> Router<App> { .route("/login", get(login::get::handler)) .route("/ch/{channel}", get(ch::channel::get::handler)) .route("/invite/{invite}", get(invite::invite::get::handler)) - .route_layer(middleware::from_fn_with_state(app.clone(), setup_required)), + .route_layer(crate::setup::Required::with_fallback( + app.clone(), + Redirect::to("/setup"), + )), ] .into_iter() .fold(Router::default(), Router::merge) |
