Show path in panic message when merging overlapping MethodRouters (#1306)

This commit is contained in:
David Pedersen 2022-08-23 16:32:34 +02:00 committed by GitHub
parent fa51cf5266
commit 0e04260a27
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 57 additions and 15 deletions

View file

@ -877,16 +877,30 @@ where
self
}
#[doc = include_str!("../docs/method_routing/merge.md")]
#[track_caller]
pub fn merge(mut self, other: MethodRouter<S, B, E>) -> Self {
pub(crate) fn merge_for_path(
mut self,
path: Option<&str>,
other: MethodRouter<S, B, E>,
) -> Self {
// written using inner functions to generate less IR
#[track_caller]
fn merge_inner<T>(name: &str, first: Option<T>, second: Option<T>) -> Option<T> {
fn merge_inner<T>(
path: Option<&str>,
name: &str,
first: Option<T>,
second: Option<T>,
) -> Option<T> {
match (first, second) {
(Some(_), Some(_)) => panic!(
"Overlapping method route. Cannot merge two method routes that both define `{}`", name
),
(Some(_), Some(_)) => {
if let Some(path) = path {
panic!(
"Overlapping method route. Handler for `{name} {path}` already exists"
)
} else {
panic!("Overlapping method route. Cannot merge two method routes that both define `{name}`")
}
}
(Some(svc), None) => Some(svc),
(None, Some(svc)) => Some(svc),
(None, None) => None,
@ -908,14 +922,14 @@ where
}
}
self.get = merge_inner("get", self.get, other.get);
self.head = merge_inner("head", self.head, other.head);
self.delete = merge_inner("delete", self.delete, other.delete);
self.options = merge_inner("options", self.options, other.options);
self.patch = merge_inner("patch", self.patch, other.patch);
self.post = merge_inner("post", self.post, other.post);
self.put = merge_inner("put", self.put, other.put);
self.trace = merge_inner("trace", self.trace, other.trace);
self.get = merge_inner(path, "GET", self.get, other.get);
self.head = merge_inner(path, "HEAD", self.head, other.head);
self.delete = merge_inner(path, "DELETE", self.delete, other.delete);
self.options = merge_inner(path, "OPTIONS", self.options, other.options);
self.patch = merge_inner(path, "PATCH", self.patch, other.patch);
self.post = merge_inner(path, "POST", self.post, other.post);
self.put = merge_inner(path, "PUT", self.put, other.put);
self.trace = merge_inner(path, "TRACE", self.trace, other.trace);
self.fallback = merge_fallback(self.fallback, other.fallback);
@ -924,6 +938,12 @@ where
self
}
#[doc = include_str!("../docs/method_routing/merge.md")]
#[track_caller]
pub fn merge(self, other: MethodRouter<S, B, E>) -> Self {
self.merge_for_path(None, other)
}
/// Apply a [`HandleErrorLayer`].
///
/// This is a convenience method for doing `self.layer(HandleErrorLayer::new(f))`.
@ -948,6 +968,7 @@ where
T::Future: Send + 'static,
{
// written using an inner function to generate less IR
#[track_caller]
fn set_service<T>(
method_name: &str,
out: &mut Option<T>,

View file

@ -173,7 +173,11 @@ where
{
// if we're adding a new `MethodRouter` to a route that already has one just
// merge them. This makes `.route("/", get(_)).route("/", post(_))` work
let service = Endpoint::MethodRouter(prev_method_router.clone().merge(method_router));
let service = Endpoint::MethodRouter(
prev_method_router
.clone()
.merge_for_path(Some(path), method_router),
);
self.routes.insert(route_id, service);
return self;
} else {

View file

@ -504,6 +504,23 @@ async fn merging_routers_with_fallbacks_panics() {
TestClient::new(one.merge(two));
}
#[test]
#[should_panic(expected = "Overlapping method route. Handler for `GET /foo/bar` already exists")]
fn routes_with_overlapping_method_routes() {
async fn handler() {}
let _: Router = Router::new()
.route("/foo/bar", get(handler))
.route("/foo/bar", get(handler));
}
#[test]
#[should_panic(expected = "Overlapping method route. Handler for `GET /foo/bar` already exists")]
fn merging_with_overlapping_method_routes() {
async fn handler() {}
let app: Router = Router::new().route("/foo/bar", get(handler));
app.clone().merge(app);
}
#[tokio::test]
async fn merging_routers_with_same_paths_but_different_methods() {
let one = Router::new().route("/", get(|| async { "GET" }));