fix(tree): surface all conflicts on multi-method route registration (#99)

Author: tigerwill90

fox_test.go

@@ -1933,105 +1933,90 @@ func TestRouter_Add_Conflict(t *testing.T) {
 		name      string
 		routes    []string
 		insert    string
-		wantErr   error
 		wantMatch []string
 	}{
 		{
 			name:      "static route already exist",
 			routes:    []string{"/foo/bar", "/foo/baz"},
 			insert:    "/foo/bar",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/foo/bar"},
 		},
 		{
 			name:      "route with same parameters",
 			routes:    []string{"/foo/{foo}"},
 			insert:    "/foo/{foo}",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/foo/{foo}"},
 		},
 		{
 			name:      "route with same wildcard",
 			routes:    []string{"/foo/+{foo}"},
 			insert:    "/foo/+{foo}",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/foo/+{foo}"},
 		},
 		{
 			name:      "route with same parameters but different name",
 			routes:    []string{"/foo/{foo}"},
 			insert:    "/foo/{bar}",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/foo/{foo}"},
 		},
 		{
 			name:      "route with same wildcard but different name",
 			routes:    []string{"/foo/+{foo}"},
 			insert:    "/foo/+{bar}",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/foo/+{foo}"},
 		},
 		{
 			name:      "route with middle same parameters but different name",
 			routes:    []string{"/{foo}/bar"},
 			insert:    "/{other}/bar",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/{foo}/bar"},
 		},
 		{
 			name:      "route with middle same wildcard but different name",
 			routes:    []string{"/+{foo}/bar"},
 			insert:    "/+{other}/bar",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/+{foo}/bar"},
 		},
 		{
 			name:      "route with same regexp parameter",
 			routes:    []string{"/foo/{foo:[A-z]+}"},
 			insert:    "/foo/{foo:[A-z]+}",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/foo/{foo:[A-z]+}"},
 		},
 		{
 			name:      "route with same regexp parameter but different name",
 			routes:    []string{"/foo/{foo:[A-z]+}"},
 			insert:    "/foo/{bar:[A-z]+}",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/foo/{foo:[A-z]+}"},
 		},
 		{
 			name:      "route with same regexp wildcard",
 			routes:    []string{"/foo/+{foo:[A-z]+}"},
 			insert:    "/foo/+{foo:[A-z]+}",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/foo/+{foo:[A-z]+}"},
 		},
 		{
 			name:      "route with same regexp wildcard but different name",
 			routes:    []string{"/foo/+{foo:[A-z]+}"},
 			insert:    "/foo/+{bar:[A-z]+}",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/foo/+{foo:[A-z]+}"},
 		},
 		{
 			name:      "route with middle same regexp parameter but different name",
 			routes:    []string{"/{foo:[A-z]+}/bar"},
 			insert:    "/{other:[A-z]+}/bar",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/{foo:[A-z]+}/bar"},
 		},
 		{
 			name:      "route with middle same regexp wildcard but different name",
 			routes:    []string{"/+{foo:[A-z]+}/bar"},
 			insert:    "/+{other:[A-z]+}/bar",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"/+{foo:[A-z]+}/bar"},
 		},
 		{
 			name:      "simple hostname conflict",
 			routes:    []string{"a.{b}.c/fox", "{a}.b.c/fox"},
 			insert:    "a.{d}.c/fox",
-			wantErr:   ErrRouteNotFound,
 			wantMatch: []string{"a.{b}.c/fox"},
 		},
 	}
@@ -2053,6 +2038,30 @@ func TestRouter_Add_Conflict(t *testing.T) {
 	}
 }
 
+func TestRouter_Add_Conflict_MultiMethod(t *testing.T) {
+	f := MustRouter()
+	f.MustAdd(MethodGet, "/hello/{name}", emptyHandler,
+		WithSchemeMatcher("https"),
+		WithQueryMatcher("foo", "bar"),
+	)
+	f.MustAdd(MethodPost, "/hello/{name}", emptyHandler,
+		WithSchemeMatcher("https"),
+		WithQueryMatcher("foo", "bar"),
+	)
+
+	got := onlyError(f.Add([]string{http.MethodGet, http.MethodPost}, "/hello/{name}", emptyHandler,
+		WithSchemeMatcher("https"),
+		WithQueryMatcher("foo", "bar"),
+	))
+	var conflict *RouteConflictError
+	require.ErrorAs(t, got, &conflict)
+	require.Len(t, conflict.Conflicts, 2)
+	gotMethods := iterutil.Map(slices.Values(conflict.Conflicts), func(r *Route) string {
+		return strings.Join(r.methods, ",")
+	})
+	assert.Equal(t, []string{http.MethodGet, http.MethodPost}, slices.Collect(gotMethods))
+}
+
 func TestRouter_Update_Conflict(t *testing.T) {
 	cases := []struct {
 		name      string

tree.go

@@ -287,10 +287,14 @@ func (t *tXn) insertTokens(p, n *node, tokens []token, route *Route) (*node, err
 		switch t.mode {
 		case modeInsert:
 			if n.isLeaf() {
-				if idx := slices.IndexFunc(n.routes, func(r *Route) bool {
-					return r.matchersEqual(route.matchers) && slicesutil.Overlap(r.methods, route.methods)
-				}); idx >= 0 {
-					return nil, &RouteConflictError{New: route, Conflicts: []*Route{n.routes[idx]}}
+				var conflicts []*Route
+				for _, r := range n.routes {
+					if r.matchersEqual(route.matchers) && slicesutil.Overlap(r.methods, route.methods) {
+						conflicts = append(conflicts, r)
+					}
+				}
+				if len(conflicts) > 0 {
+					return nil, &RouteConflictError{New: route, Conflicts: conflicts}
 				}
 			}