Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce the amount of boilerplate needed to declare partials in the routes #108

Closed
reeze opened this issue Dec 12, 2018 · 7 comments
Closed
Labels
design Open design question

Comments

@reeze
Copy link

reeze commented Dec 12, 2018

When playing with Tide, I found that NamedPath is not friendly to use compare to Rocket.

It seems tedious when working with parameters if we don't using macro to make API easier to reason about.

Rocket has a convention the named segment's name are the handler's parameter name

#[get("/user/<id>")]
fn handler(id i32) {
}

I know there is an id and is an int. It is simple.

https://github.com/rust-net-web/tide/pull/32/files

Most of time named path is simple type. Consider following case :

A restful url for a comment api

/posts/$(post_id)/$(comment_id)  # both of them should be int, that is it.

I have to duplicate and declare a type for every single named path.

struct Number(i32);
struct CommmentId(i32);

 impl NamedComponent for Number {
    const NAME: &'static str = "num";
}

impl NamedComponent for CommentId {
    const NAME: &'static str = "comment_id";
}

async fn comment_detail(num: Named<Number>, comment_id: Named<Comment>);

app.at("/posts/<num>/<comment_id>").get(comment_detail);

By current design: app.at("/posts/<num>/<num> is not going to work properly(last num wins)

Maybe we need to give a NamedPath a name from routing rule but not a type's name. A probably solution might be using const generics to give a type a name

app.at("/posts/<num:Number>/<comment_id:Number>").get(comment_detail);

async fn comment_detail(num: Named<"num", Number>, comment_id: Named<"comment_id", Number>);

but const generics is not implemented.

Another solution might be using macros, but according to this post: https://rust-lang-nursery.github.io/wg-net/2018/10/16/tide-routing.html

- Avoid macros and code generation at the core; prefer simple, “plain Rust” APIs.

Any more ideas to make bridge route rule to endpoint declaration easier?

@yoshuawuyts yoshuawuyts added the design Open design question label Jan 10, 2019
@yoshuawuyts yoshuawuyts changed the title Improve NamedPath Reduce the amount of boilerplate needed to declare partials in the routes Jan 10, 2019
@yoshuawuyts
Copy link
Member

@reeze thanks for opening this issue! -- I don't have any suggestions on how to improve, but I agree the example you posted seems rather verbose, and it'd be nice if we could reduce the amount of boilerplate needed.

Perhaps @tirr-c or @aturon might have any ideas?

@tirr-c
Copy link
Collaborator

tirr-c commented Jan 10, 2019

I think it's very likely that all named segments (or, wildcards) are used in the endpoint. So it would be nice if we provide a way to extract all of those at once, and access each of them via method call or indexing.

async fn handler(segments: Segments) -> Response {
    let number = segments.get::<usize>("number");
    // ...
}

We need to think how to propagate parse errors though. Maybe returning Result would be sufficient.

@petejodo
Copy link
Collaborator

That would mean a possible situation like this could occur...

async fn handler(segments: Segments) -> Response {
  let number = segments.get::<usize>("num");
  // handler only requires "num" which "/{foo}/{num}" satisfies
}

// ...

app.at("/{foo}/{num}").get(handler);

...and since no errors get thrown, this wouldn't get surfaced immediately. Maybe Segments (which I think a name like PathSegments would be more appropriate) could somehow force you to exhaust all the named segments for the route it is handling otherwise it would error out but I'm not sure what that would look like.

@tirr-c
Copy link
Collaborator

tirr-c commented Jan 18, 2019

@petejodo I don't think we have to error on non-exhaustive use of segments. As of now it's totally okay to partially use named segments. However it would be great if we get a log message when some segments are not used!

@pepsighan
Copy link

Writing get for the named segments would be tedious and routes would not be able to move to the next match if failed.

What about the actix way of getting the args?

use actix_web::{App, Path, Result, http};

/// extract path info from "/users/{userid}/{friend}" url
/// {userid} -  - deserializes to a u32
/// {friend} - deserializes to a String
fn index(info: Path<(u32, String)>) -> Result<String> {
    Ok(format!("Welcome {}! {}", info.1, info.0))
}

@HeroicKatora
Copy link
Contributor

HeroicKatora commented Jan 20, 2019

I propose that this could be solved by submitting the name of the segment as additional information to the router when registering the path. This makes it possible to choose between a typed generic or slightly more parameters. The former is stricter type while the latter is flexible. This could look like the following (preliminary syntax), see my pull request #126 for more details:

async fn display_number(nr: Named<i32>) -> String {
    format!("Segment number: {}", nr.0)
}

async fn two_segments(from: Named<String>, to: Named<String>) {
    format!("Message from {} to {}", &from, &to)M
}

fn main() {
    let mut app = tide::App::new(());
    app.at("/{num}").get_with(display_number, SegmentName("num".into()));
    // Or extracting multiple segments at once by providing a tuple as second argument.
    app.at("/msg/{from}/{to}").get_width(two_segments, 
        (SegmentName("from".into()), SegmentName("to".into())));
    app.serve();
}

Since this relies only on runtime values, it would not be unimaginable that the SegmentName arguments could be generated automatically in a wrapper.

@aturon
Copy link
Collaborator

aturon commented Apr 10, 2019

This is resolved in #156, which takes a radically different approach to extraction.

@aturon aturon closed this as completed Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Open design question
Projects
None yet
Development

No branches or pull requests

7 participants