carllerche / tower-web

A fast, boilerplate free, web framework for Rust
MIT License
980 stars 51 forks source link

Potentially allow for patterns in path segments #154

Open whitfin opened 5 years ago

whitfin commented 5 years ago

Some other frameworks allow for Regex patterns in paths. Could we potentially see this in Tower Web?

I've played around to get something trivial working and it seems as simple as determining a notation (I used :capture:pattern locally) and adding a Segment::Pattern(regex::Regex) value. Is there any reason this shouldn't be added? If not, I can take a shot at a PR once we establish syntax!

carllerche commented 5 years ago

I would be OK with this as long as they match segments (and thus cannot contain /). So, paths would first be split into segments, then you would add a filter to specify a pattern.

Instead of hardcoding it to segments, maybe there is a more generic way to add bind filters... maybe you could even run the filter through arbitrary functions. There could be sugar for using a regex.

Maybe:

#[get("/foo/:bar")]
#[filter(bar = "check_bar")]
fn my_action(&self, bar: &str) -> ... {
    // hello
}

and w/ a regex...

#[get("/foo/:bar")]
#[filter(bar(regex = "ab+c*"))]
fn my_action(&self, bar: &str) -> ... {
    // hello
}

I'm not immediately in love with this syntax... but I thought I would get the convo started.

whitfin commented 5 years ago

I would be OK with this as long as they match segments (and thus cannot contain /). So, paths would first be split into segments, then you would add a filter to specify a pattern.

That's pretty much what I was thinking, yes; arbitrary segment matching just seems like it would end up causing problems :p

The syntax there makes a lot of sense in being flexible, but it does have an ugliness to it with the nested (). I agree though, that this would be better than just adding a new segment type since it opens the door for other things in future.

carllerche commented 5 years ago

Do you have thoughts for alternate syntax?

whitfin commented 5 years ago

I quite like the concept of:

#[get("/foo/:bar")]
#[filter(bar = "check_bar")]
fn my_action(&self, bar: &str) -> ... {
    // hello
}

so I'm wondering if that's sufficient alone. Regex can then just be implemented in check_bar (which I assume returns a boolean?).

I'm not sure if validate might be a better term than filter though, since it's closely aligned with validation of parameters. Were you thinking that this would extend to other arguments rather than just segments (such as headers, body, etc)?

carllerche commented 5 years ago

In my mind, validate would mean if the function returns false, the server would return 400. Is that your expectation?

whitfin commented 5 years ago

@carllerche yes, I think so - I think that would be quite familiar for most as well. But I think I see your point; if something in the path fails validation it should be a 404. Only request body/query string should 400 - perhaps that difference is enough to stick with filter and only operate on the path segments for the time being.

carllerche commented 5 years ago

You are correct, but validate could detect if the param being validated is from the path or not. So, it could still work.

whitfin commented 5 years ago

In that case I do think validate would be good; it opens the door for validation on the body/query string/headers using the same syntax 👍

carllerche commented 5 years ago

Do you have any interest in attempting a PR? It would not be super trivial, but doable.

whitfin commented 5 years ago

@carllerche I can definitely take a look - I'm a little new to the repo so it might take a little while to make any progress though!

carllerche commented 5 years ago

I had another thought about this. I wonder if using an annotation on the path is the wrong way to do it. Instead, perhaps if an action cannot accept all strings, a type should be used.

For example, I believe this works today:

#[get("/foo/:bar")]
fn my_action(&self, bar: u32) -> ... {
    // hello
}

So, perhaps a way to do it would be with validations. Something like:

#[derive(Extract)]
struct Bar {
    #[validate(match = "abc*")]
    val: String,
}

#[get("/foo/:bar")]
#[filter(bar = "check_bar")]
fn my_action(&self, bar: Bar) -> ... {
    // hello
}

or something like that... thoughts?

(Edit: fixed snippet)