Interior composability of FromRequest

[mitsuhiko]

I'm encountering the case in our app frequently that I want to internally compose some FromRequest in another FromRequest. In our case for instance we have a standardized request format that internally wraps a payload but also accesses auth information.

So conceptionally one can think of it as some logic around a Auth FromRequest and a Json<T> FromRequest. If DI is used they would be passed to the function independently but the endpoint will never actually access any of the values independently.

So effectively we're looking at a way to combine FormRequest (use With2 and friends internally somehow).

The end goal would be that a user can write a fn handler(req: CustomThing<T>) where CustomThing<T> internally uses other FromRequests.

[fafhrd91]

i made some changes to FromRequest trait. now it looks like:

pub trait FromRequest<S>: Sized
where
    S: 'static,
{
    /// Configuration for conversion process
    type Config: Default;

    /// Future that resolves to a Self
    type Result: Into<Reply<Self>>;

    /// Convert request to a Self
    fn from_request(req: &mut HttpRequest<S>, cfg: &Self::Config) -> Self::Result;
}

first of all you get mutable reference to a request. and Result now is Reply<_> which allows
to use simple values and result instead of future.

for example here how ProjectRequest from sentry_relay could look like:

impl<T: DeserializeOwned + 'static> FromRequest<Arc<TroveState>> for ProjectRequest<T> {
    type Config = ();
    type Result = Result<FutureResponse<Self>>;

    #[inline]
    fn from_request(req: &mut HttpRequest<Arc<TroveState>>, _: &Self::Config) -> Self::Result {
        let auth = get_auth_from_request(req)?;

        let project_id = req.match_info()
            .get("project")
            .unwrap_or_default()
            .parse()
            .map_err(BadProjectRequest::BadProject)?;

        let state = req.state().clone();
        Ok(Box::new(
            JsonBody::new(req.clone())
                .limit(262_144)
                .map_err(|e| BadProjectRequest::BadJson(e).into())
                .map(move |payload| ProjectRequest {
                    auth,
                    project_id,
                    payload: Some(payload),
                    state,
                }),
        ))
    }
}

another change is with Reply, it now implements Future trait. so you can write
something like:

FromRequest::<Json<T>>::from_request(req, &())
    .and_then(|val: T| { ...  });

i am also thinking about adding default impl for tuples of FromRequest's

impl<T1, T2, T3, ...> FromRequest<S> for (T1, T2, T3, ...) 
where
     T1: FromRequest<S>,
     ...
{
    type Config = (T1::Config, ...);
    type Result = (T1, T2, T3, ...);

    fn from_request(...) {}
}

it is not that nice, bbut it would be possible to extract multiple values and use only .with()

[fafhrd91]

@brandur i noticed you built some param extraction system in your podcore project, would you share your ideas?

[mitsuhiko]

For what it's worth the sentry relay now uses a different approach to manually compose: https://github.com/getsentry/sentry-relay/blob/23f92e3df11db18ff1e9a2f90fdfb9fb4b63e04a/server/src/extractors.rs#L102-L135

[fafhrd91]

@mitsuhiko here is how your code looks like with master:

impl<T: FromRequest<Arc<TroveState>> + 'static> FromRequest<Arc<TroveState>> for ProjectRequest<T> {
    type Config = T::Config;
    type Result = Result<FutureResponse<Self>, Error>;

    fn from_request(req: &HttpRequest<Arc<TroveState>>, cfg: &Self::Config) -> Self::Result {
        let mut req = req.clone();

        let auth = get_auth_from_request(&req)?;
        req.extensions_mut().insert(auth);

        let project_id = req.match_info()
            .get("project")
            .unwrap_or_default()
            .parse::<ProjectId>()
            .map_err(BadProjectRequest::BadProject)?;
        req.extensions_mut().insert(project_id);

        Ok(Box::new(T::from_request(&mut req, cfg).into().map(
            move |payload| ProjectRequest {
                http_req: req,
                payload: Some(payload),
            },
        )))
    }
}

[fafhrd91]

added extract method

here is example how manually extract path info

fn index(req: &HttpRequest) -> Result<...> {
    let data = Path::<(usize, String)>::extract(req)?;
    ...
}

or Query

fn index(req: &HttpRequest) -> Result<...> {
    let q = Query::<HashMap<String, String>>::extract(req)?;
    ...
}

[fafhrd91]

added FromRequest impl for tuples, so it is possible to do something like this:

fn index(data: (State<AppState>, Path<(String, ...)>, Form<..>)) -> HttpResponse {
     data.0;   // <- state
     data.1;   // <- path
     ...
}

fn main() {
     App::new().resource("/", |r| r.with(index));
}

@mitsuhiko that is all i could come up.
does anyone have more ideas about extractors?

[mitsuhiko]

@fafhrd91 that looks cool. I'm going to play with this.

[brandur]

I'm afraid to say that I'm doing something that's quite a bit more simplistic.

Every handler invokes a build function on a custom Params type. I have this extracted into a macro to be DRY (which works, but is probably not the right decision):

            req2.body()
                .map_err(|_e| Error::from("Error reading request body"))
                .and_then(move |bytes: Bytes| {
                    time_helpers::log_timed(&log.new(o!("step" => "build_params")), |log| {
                        Params::build(log, &mut req3, Some(bytes.as_ref()))
                    })
                })

Params are just bare structs that may pull some things from app state, some things from request extensions, and other things from the query or request body:

    struct Params {
        account:    Option<model::Account>,
        episode_id: i64,
        podcast_id: i64,
    }

    impl server::Params for Params {
        fn build<S: server::State>(
            _log: &Logger,
            req: &mut HttpRequest<S>,
            _data: Option<&[u8]>,
        ) -> Result<Self> {
            Ok(Self {
                account:    server::account(req),
                episode_id: links::unslug_id(req.match_info().get("id").unwrap())
                    .map_err(|e| error::bad_parameter("episode_id", &e))?,
                podcast_id: links::unslug_id(req.match_info().get("podcast_id").unwrap())
                    .map_err(|e| error::bad_parameter("podcast_id", &e))?,
            })
        }
    }

It's not very sophisticated, but at least it's easy to understand.

[fafhrd91]

@brandur you should try extractor (FromRequest trait) it reduces boilerplate code significantly
check how sentry-relay uses extractors https://github.com/getsentry/sentry-relay/blob/master/server/src/extractors.rs#L103

[fafhrd91]

current implementation is part of 0.6 release. lets continue discussion in different ticket.