Rocket: Route queries without ? for optional parameters

[pepsighan]

Version: 0.3.0

The route with queries can only match with request with queries. But in cases where the parameters are options, requesting with a brother-less ? is kind of awkward.

#[get("/login?<redirect_url>")]
fn login(redirect_url: Option<RedirectUrl>) -> Redirect {
    ...
}

If there is no RedirectUrl, then this resource could be requested with a simple /login rather than a /login?. But in order to have such behaviour, I must duplicate the code with a new #[get("/login")] function.

It would be nice to have an additional rule of having ? omitted if the all parameters are optional and no query-less route exists.

Also, another request, is route regex in the timeline of rocket?

[SergioBenitez]

To summarize, there appear to be two requests here:

1. Let paths with Option query parameter types match against request URIs without a ?.
2. Allow regular expressions in route parameters. Presumably something like <param:[0-9]+>.

The primary issue with 1 is that it removes the symmetry in matching incoming requests against a route attribute. You would no longer be able to textually match a route against a URI: a route with a ? could match against a request _without_ a ?. Said another way, the ? in a route attribute currently means: "I expect a query parameter." With this change, the meaning depends on the type of the parameter, even though the syntax doesn't imply this.

One potential way to skirt this issue is to change the syntax so that the ? in _inside_ the brackets. That is, you might write the route attribute instead as:

#[get("/login<?redirect_url>")]

This looks a little funky to me. But it's an idea.

To the core issue: is it a big issue to have two routes?

#[get("/login?<redirect_url>")]
fn login(redirect_url: RedirectUrl) -> Redirect {
    ...
}

#[get("/login")]
fn login_two() -> Redirect {
    ...
}

// compared to the proposed...

#[get("/login?<redirect_url>")]
fn login(redirect_url: Option<RedirectUrl>) -> Redirect {
    match redirect_url {
        Some(url) => ...,
        None => ...,
    }
}

Looks like it's just one more line of code and spacing, at worst. This has the nice advantage that you don't need to check the Option variant, the code is more self-descriptive, and the types become more meaningful. The disadvantage, of course, is what seems to be duplication, though I'd argue that these two are, in fact, different.

One the second request: no, regex in Rocket's core routing syntax is not currently in the roadmap. This is because it's already very simple to procedurally accomplish the same thing via guards: request guards, parameter guards, data guards, and segments guards. In this case, the amount of code you need to write does increase quite a bit (from 1 line to at least 5), but things like const generics should reduce this back down to 1. I'm beginning to lean toward accepting #339, however, which will expand Rocket's core routing syntax abilities greatly.

[pepsighan]

Though it is just an addition of 3 lines to duplicate the optional ? but still it does not sit well. Writing an intention in a single block looks elegant. Internally, there is a difference of fn call and matches(cost of fn call is higher afaik, though for our application needs it is not worth much).

#[get("/login<?redirect_url>")]

The syntax that you thought is as rusty as ?Sized. And as for regex, it could be implemented as guards but wanted to know if a concise syntax would be supported. Good to know, in a new way, it might see the light of day.

[max-frai]

And what about ? in the url. Duplicate routes is not the case. It's the part of query string standard. There could be a few arguments in any order and any of them could be dropped.

/login?foo=1&bar=2
/login?foo=1
/login?bar=2
/login

All of these should refer to the same routing

[SergioBenitez]

@max-frai Sorry, I'm not sure I follow what you're saying. Could you rephrase?

[lukesutton]

I'd like to add something to this issue, since I've been finding Rocket's handling of query strings — particularly with GET requests — to be a bit awkward.

One common task with query strings is to use them to set the ordering of results and/or filtering. For example on a web-shop I may wish to allow users to order products by name, price or when the product was added. I may also want to give them the option of limiting results to a specific brand. So I may have a query string like /products?order=name&brand=hasbro. This case is pretty simple to handle by defining a struct and implementing the requisite FromForm and FromFormValue traits. However there are many cases where I don't find this sufficient.

I still want to be able to route to the same handler regardless if there all, some or none of the query-string options specified, falling back to defaults where a specific option isn't provided. Handling all or some of the query-string options is easy enough by defining a struct like so:

#[derive(FromForm)]
struct Options {
  order: Option<Order>,
  brand: Option<String>
}

But I'm a bit stuck on how to provide defaults. I don't actually want order and brand to be an Option<T>. And in the case where the user requests /products I want a default Options with the default order and brand.

I'm sure I could figure it using a combination of Option<T> and extra handlers, but that feels very awkward to me. The current implementation seems particularly odd when I can specify my handler like this:

#[get("/products?<options>")]
fn products(options: Option<Options>) -> String {
  // ...
}

Which will respond successfully to the request /products?garbage=option, but not to /products. This produces an outcome which is lenient for the former request, but oddly strict for the latter.

[TatriX]

So currently I have to do something like this:

/// return multiple articles, ordered by most recent first
#[get("/articles")]
fn get_articles(conn: db::Conn) -> Json<Value> {
    Json(json!({
        "articles": db::articles::find(&conn, FindArticles::default())
    }))
}

/// return multiple articles, ordered by most recent first
#[get("/articles?<params>")]
fn get_articles_with_params(params: FindArticles, conn: db::Conn) -> Json<Value> {
    Json(json!({ "articles": db::articles::find(&conn, params) }))
}

and then mount both routes.

    rocket::ignite()
        .mount(
            "/api",
            routes![
                routes::articles::get_articles,
                routes::articles::get_articles_with_params,
                ...
            ],
        )

where FindArticles looks like this:

#[derive(FromForm, Default)]
pub struct FindArticles {
    tag: Option<String>,
    author: Option<String>,
    limit: Option<i64>,
    offset: Option<i64>,
}

It looks like a lot of boilerplate for the most basic functionality which is used almost everywhere on the web.

I think /articles?<params> should match with /articles if params is Option<T> or implements Default.

[ascclemens]

As someone who is new to Rocket, this feature not existing was very surprising to me. I actually thought I was doing something wrong with my routing until I found this issue.

As a programmer, I would expect to be able to handle optional query strings on a route without making two routes (and then pulling their logic out into yet another function). That doesn't make any sense. It seems like path?<query> where query is T, would mean that query is mandatory, while query being Option<T> would mean that query is optional.

Additionally, as a user, I would not expect adding ? to the end of a URL to make a previously-200 request turn into a 404. That isn't how routing works in the real world.

[SergioBenitez]

Accepted for 0.4. Tracked in #608 as part of the new query-string handling.