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

axum 0.6 version Router::new().nest() #1677

Closed
chenlunfu opened this issue Jan 4, 2023 · 10 comments
Closed

axum 0.6 version Router::new().nest() #1677

chenlunfu opened this issue Jan 4, 2023 · 10 comments

Comments

@chenlunfu
Copy link

chenlunfu commented Jan 4, 2023

  • I have looked for existing issues (including closed) about this

Bug Report

thread 'main' panicked at 'Invalid route "/api": insertion failed due to conflict with previously registered route: /api/*__private__axum_nest_tail_param'
thread 'main' panicked at 'Invalid route "/sys": insertion failed due to conflict with previously registered route: /sys/*__private__axum_nest_tail_param'

Version

axum 0.6

Platform

PC

Crates

Description

let app=Router::new().nest("/api",routers()).nest("/api",white_routers());
fn routers()->Router{
    Router::new().nest("/sys", sys()) 
}
fn white_routers()->Router{
    Router::new().nest("/sys",white_sys())
}
pub fn sys()->Router{
    Router::new().route("/setting",...).route("/user",...) 
}
pub fn white_sys()->Router{
    Router::new().route("/white",...) 
}

axum 0.5 work normally,but axum 0.6 work panick

@davidpdrsn
Copy link
Member

Thats an intentional change in 0.6. You can use Router::merge to create one router and nest that. You can't call nest("/api", _) multiple times.

@davidpdrsn davidpdrsn closed this as not planned Won't fix, can't repro, duplicate, stale Jan 5, 2023
@chenlunfu
Copy link
Author

chenlunfu commented Jan 5, 2023

I try Router::merge nest("/api", _),but Router::new().nest("/sys", sys()) cannot merge,this is my try:

let app = Router::new().nest("/api", routers().merge(white_routers()));
fn routers()->Router{
	Router::new().nest("/sys", sys())
}
fn white_routers()->Router{
	Router::new().nest("/sys", white_sys())
}
pub fn sys()->Router{
	Router::new().route("/setting", ...).route("/user", ...)
}
pub fn white_sys()->Router{
	Router::new().route("/white", ...)
}

@jplatte
Copy link
Member

jplatte commented Jan 5, 2023

@chenlunfu You still have the same issue there, you are trying to nest /sys twice in what ends up being a single Router. You need sth. like this:

let app = Router::new().nest("/api", api_routers());
fn api_routers()->Router{
    Router::new().nest("/sys", sys().merge(white_sys())
}
pub fn sys()->Router{
	Router::new().route("/setting", ...).route("/user", ...)
}
pub fn white_sys()->Router{
	Router::new().route("/white", ...)
}

Also I've reformatted your comments a little bit. Please use markdown formatting to make your posts / comments more readable.

@chenlunfu
Copy link
Author

Thank you for your help,But I have two layers because I want to add permissions in one layer,white_routers() no permission required

let app = Router::new().nest("/api", routers().layer(auth).merge(white_routers()));
fn routers()->Router{
	Router::new().nest("/sys", sys())
}
fn white_routers()->Router{
	Router::new().nest("/sys", white_sys())
}
pub fn sys()->Router{
	Router::new().route("/setting", ...).route("/user", ...)
}
pub fn white_sys()->Router{
	Router::new().route("/white", ...)
}

@jplatte
Copy link
Member

jplatte commented Jan 5, 2023

That is no problem, you can do sys().layer(auth).merge(white_sys()) to only apply the auth layer to the sys() router.

@chenlunfu
Copy link
Author

No, There's a problem.such as,
Permission required:
http://127.0.0.1:3000/api/sys/setting
http://127.0.0.1:3000/api/sys/user
http://127.0.0.1:3000/api/store/setting
http://127.0.0.1:3000/api/store/user
...
No permission required:
http://127.0.0.1:3000/api/sys/white
http://127.0.0.1:3000/api/store/white
...
I don't know if I can express myself clearly。I want to add permissions to the api layer,Of course, it can be written in each layer, but it seems a bit silly

@jplatte
Copy link
Member

jplatte commented Jan 5, 2023

Well, you will have to apply the layer in multiple places, or use less nesting (currently, there is a performance regression with nesting anyways). I would likely do something like

let app = Router::new().nest("/api", api_routes());

fn api_routes() -> Router {
    authenticated_routes().merge(public_routes())
}

fn authenticated_routes() -> Router {
    Router::new()
        .route("/sys/setting", ...)
        .route("/sys/user", ...)
        .route("/store/setting", ...)
        .route("/store/user", ...)
        .layer(auth_layer)
}

fn public_routes() -> Router {
    Router::new()
        .route("/sys/white", ...)
        .route("/store/white", ...)
}

@chenlunfu
Copy link
Author

Thank you for your help,but axum 0.5 work normally,Will later versions support it?

@davidpdrsn
Copy link
Member

davidpdrsn commented Jan 5, 2023

but axum 0.5 work normally

You should expect breaking changes when updating from 0.5 to 0.6. Just because something worked on 0.5 doesn't guarantee that it'll work on 0.6 but of course we don't break stuff without a good reason.

Will later versions support it

Perhaps but its not clear yet. There are some tradeoffs to consider. Its also related to #1624 which I haven't had time to look into yet.

@chenlunfu
Copy link
Author

Thank you for your help,But i still think this is a bug. I will try to fix it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants