Type-safe path/query parameter handling, using serde

[radix]

Users should be able to define structures that hold parameters that need to be passed to a request handler. We could use the serde Deserialize trait. The goal is to make it easier for users to access parameters without having to deal with error-handling themselves, and hopefully to avoid the current situation where making a typo in your handler vs your route means that you get a runtime error instead of a compile-time error.

The goal is to somehow allow the user to type this:

#[derive(Deserialize)]
struct MyParams {
    name: String,
}

and then, define a handler like this:

fn my_handler(req: HttpRequest<_>, my_params: MyParams) -> _ {
}

or, alternatively:

fn my_hadler(req: HttpRequest<_>) -> _ {
    let my_params = req.get_params::<MyParams>();
}

This will require some way to declare the route for my_handler with MyParams.

[radix]

open question: will this obsolete FromParam, or will FromParam continue to be used somehow? TBD when implementing

[mockersf]

For query params, it is already possible:

#[derive(Deserialize)]
struct MyQueryParams {
    name: String,
}

pub fn my_handler(req: HttpRequest<AppState>) -> _ {
    match serde_urlencoded::from_str::< MyQueryParams >(req.query_string()) {
        Ok(params) => ...
        _ => ...
    }
}

There are two serve deserialiser that seems to do that:
https://github.com/samscott89/serde_qs
https://github.com/nox/serde_urlencoded

For path parameters, I think that would be quite a bit more complicated to do. Maybe add a resource and a handler methods that take a struct that implement FromStr instead of the expected path, and pass the resulting struct to the handler, and match the first that doesn't return an Err

[fafhrd91]

it is question of ergonomics. how to make it easy to use. also you can extract data from urlencoded or multipart body, in that case you need to deal with bytes stream.

i am not sure about path, but maybe it makes sense. actually i think switching to FromStr is backward compatible

[fafhrd91]

does anyone want to experiment with this issue?

[fafhrd91]

I've been thinking about something like this, should be backward compatible.
thoughts?

#[derive(Deserialize)]
struct MyQueryParams {
    name: String,
}

pub fn my_handler(req: HttpRequest<AppState>, params: MyQueryParams) -> _ {
    ...
}

let app = Application::new()
        .resource("/index.html", |r| r.h(
               WithParams::<MyQueryParams>::(my_handler)))

let app = Application::new()
        .resource("/index.html", |r| r.h(
               WithJson::<MyQueryParams>::(my_handler)))

let app = Application::new()
        .resource("/index.html", |r| r.h(
               WithFormData::<MyQueryParams>::(my_handler)))

[DoumanAsh]

I'm thinking that the old way to extract query params from request on demand seems less verbose.
But I guess such API does make sense when user expect multiple variants of query params, and want have separate handler for each of them (though nothing prevents user to do it himself).

[fafhrd91]

i added HttpRequest::extract() api

@radix @DoumanAsh what do you think?

[brandur]

@fafhrd91 Nice! I assume that the result comes back as an Err if the parameter couldn't located, which I think would bubble back to the user as a 500?

A raw Err is probably appropriate for Path, which presumably wouldn't have matched the handler if it wasn't present, but in the example it might be useful to show how to map that result to a 400, which will likely be a very common (and probably recommended) pattern.

[fafhrd91]

by default it returns 400. otherwise you just do usual stuff with Result<T, actix_web::Error> like map_err, etc

[fafhrd91]

I am not sure about Path/Query types. And should extractor use both path match info and query params at the same time

[brandur]

by default it returns 400. otherwise you just do usual stuff with Result<T, actix_web::Error> like map_err, etc

Ah, I see. I still think that this is the kind of thing that couldn't hurt to be documented somewhere — a user of the library (like myself) will almost certainly want to return specific types of errors to clients on a bad validation, and it's not obvious that it sends back a 400 without looking at source code.

I am not sure about Path/Query types. And should extractor use both path match info and query params at the same time

I'm personally still ramping up to the point where I have any reasonable intuition as to what a good Rust API looks like, but I have to admit that understanding exactly what Path and Query were confused me a bit. Given that they're almost more like symbolic types to provide certain functionality for extract, I'd almost prefer to see more explicit names like PathExtractor, QueryExtractor.

That said, maybe retooling the API to use methods like extract_query and extract_path would be just as good. The extensibility of your current API is nice, but in practice I'm not sure how far people will really need to go beyond and query and path.

And should extractor use both path match info and query params at the same time

Well the good part about your current implementation is that you could have a PathAndQuery struct as well ... ;) (I'm not actually sure if this is a good idea.)

[fafhrd91]

@brandur you sound reasonable :) I'll add extract_path and extract_query (initially I use extract_path)

[fafhrd91]

I changed api

[DoumanAsh]

@fafhrd91 That looks neat 👌

[fafhrd91]

Added new with api

Thoughts? Maybe different name?
@brandur?

[fafhrd91]

@brandur there is user guide section on error handling in actix-web https://actix.github.io/actix-web/guide/qs_4_5.html

[brandur]

@fafhrd91

Added new with api

Thoughts? Maybe different name?
@brandur?

Wow, that's awesome! A couple questions:

1. Can you specify a with that will take a parameter type can handle a query and path?
2. (Maybe stupid question, but) That will take either sync or async handler right? (i.e., You don't need something like the a vs. f distinction.)

@brandur there is user guide section on error handling in actix-web https://actix.github.io/actix-web/guide/qs_4_5.html

Ah neat. Thanks!

By the way, I wrote a post that talks about actix-web that's getting some attention on HN right now, which you might be interested to read. If you do, let me know if you find any technical inconsistencies.

[fafhrd91]

Wow, that's awesome! A couple questions: Can you specify a with that will take a parameter type can handle a query and path?

It is easy to add new extractor, you just need to implement `HttpRequestExtractor` trait. I need to add `PathAndQuery` type but I am still thinking about design, maybe it is possible to use curring. In any case I will add path and query extractor in one way or another.

(Maybe stupid question, but) That will take either sync or async handler right? (i.e., You don't need something like the a vs. f distinction.)

`with()` accepts both. I’d like to get rid of `.f()` and `.a()` and replace it with one function, but that may need `impl Trait`.

Ah neat. Thanks! By the way, I wrote a post that talks about actix-web <https://brandur.org/rust-web> that's getting some attention on HN right now, which you might be interested to read. If you do, let me know if you find any technical inconsistencies.

I already read it :) nice article! I’d probably use error handling differently but that is up to you.

[brandur]

with() accepts both. I’d like to get rid of .f() and .a() and replace it with one function, but that may need impl Trait.

Ah I see! Okay, in that case: great.

I already read it :) nice article! I’d probably use error handling differently but that is up to you.

Thanks! If the error handling is the only thing that I got wrong, then I consider myself quite lucky ;)

[mockersf]

Extractors look great, thanks @fafhrd91 !
Just so you know, I ran into an issue with serde_urlencoded: Unit Enum are not properly decoded. There is a PR waiting to fix that: nox/serde_urlencoded#30

[fafhrd91]

I simplified extractor a little, Path example.

WithHandler

Thoughts?

[ZelphirKaltstahl]

I don't know what the previous way of doing this was, but I like how the code looks in the "Path example".

[radix]

@fafhrd91 what if you want both a request extractor and the request passed to your handler? e.g. to get the request body while also accessing arguments? shouldn't the handlers take both the info and the request?

[fafhrd91]

@radix state and request are part of extractor implementation

https://actix.github.io/actix-web/actix_web/struct.Path.html#method.request

[fafhrd91]

@brandur added multiple extractor params, it is not elegant at the moment
https://actix.github.io/actix-web/actix_web/struct.Route.html#method.with2

[brandur]

@fafhrd91 Nice!!!

IMO, it'd be slightly more elegant if the with functions continue to take an HttpRequest parameter (in addition to the Path/Query parameters). It seems inevitable that people will want to look at a request header (or something like that), and it'd be cleaner to have direct access to it.

BTW, I didn't notice the impl<T, S> HttpRequestExtractor<S> for Json<T> before. That's great!

[fafhrd91]

@brandur

you can access request and state with Query::request() and Query::state() methods, same for Path. Json is different because of backward compatibility, but maybe it worse to change it.

[fafhrd91]

added impl HttpRequestExtractor for HttpRequest, so now it is possible to write something like:

fn index(req: HttpRequest<S>, path: Path<Params, S>) -> ... {
    unimplemented!()
}

fn main() {
    let app = Application::with_state(State{}).resource(
       "/{username}/index.html", 

       |r| r.method(Method::GET).with2(index));  // <- use `with2` extractor
}

[fafhrd91]

do we really need .extract_path() and .extract_query() at all? it just easier to use .with()

[brandur]

added impl HttpRequestExtractor for HttpRequest, so now it is possible to write something like:

Wow! The type system here continues to amaze in what's possible ...

do we really need .extract_path() and .extract_query() at all? it just easier to use .with()

Yeah +1. .with() seems quite a bit better.

[fafhrd91]

i think this feature is more or less completed

it is even possible to use tuples, so you dont need to define type

fn index(info: Path<(String, u32)>) -> Result<String> {
    Ok(format!("Welcome {}! {}", info.0, info.1))
}

fn main() {
     let app = Application::new().resource(
        "/{username}/{count}/?index.html",       // <- define path parameters
        |r| r.method(Method::GET).with(index));  // <- use `with` extractor
}

[radix]

@fafhrd91 that is AWESOME!

[fafhrd91]

one question is about state. at the moment Path<T, S> and Query<T, S> are generic over application state. i think S is not really needed here. first of all HttpRequest is available as extractor, also i can add State<S> extractor. so it may look like:

fn index(info: Path<(String, u32)>, state: State<S>) -> Result<String> {
    Ok(format!("Welcome {}! {}", info.0, info.1))
}

fn main() {
     let app = Application::new().resource(
        "/{username}/{count}/index.html",       // <- define path parameters
        |r| r.method(Method::GET).with2(index));  // <- use `with` extractor
}

[fafhrd91]

any ideas from what module extractors should be exported?

right now there are, Path, Query, State, Json extractors and they are available from top level module

[ZelphirKaltstahl]

Path feels right, from my point of limited experience and knowledge. My reasoning would be, that you are defining a path to contain parameters, so the tools to work with those parameters should be grouped with Path. State and Json are too abstract. Query I don't know.

[fafhrd91]

closing, I think most of extractors are implemented. lets create separate issues for extra extractors.

here is list of available extractors

https://actix.github.io/actix-web/actix_web/trait.FromRequest.html

[adwhit]

I just want to say that I gave this a try today and it is absolutely fantastic! Amazing work.