issues/450: mux; detect route conflicts in more cases

CHANGELOG.md

@@ -4,7 +4,8 @@ Most recent version is listed first.
 
 
 # v0.0.99
-- ong/errors: errors; retain stack traces when errors are wrapped with fmt: https://github.com/komuw/ong/pull/452
+- ong/errors: retain stack traces when errors are wrapped with fmt: https://github.com/komuw/ong/pull/452
+- ong/mux: detect route conflicts in more cases: https://github.com/komuw/ong/pull/453
 
 # v0.0.98
 - ong/errors: add equivalent functions from standard library: https://github.com/komuw/ong/pull/449

internal/mx/route.go

@@ -201,7 +201,7 @@ func (r *router) detectConflict(method, pattern string, originalHandler http.Han
 		sameLen := len(incomingSegments) == len(existingSegments)
 		if !sameLen {
 			// no conflict
-			break
+			continue
 		}
 
 		errMsg := fmt.Errorf(`

internal/mx/route_test.go

@@ -383,6 +383,20 @@ func TestConflicts(t *testing.T) {
 		errH := r.handle(http.MethodGet, "/index.php", secondRoute(msg2), secondRoute(msg2))
 		attest.Ok(t, errH)
 	})
+
+	t.Run("http MethodAll conflicts with all other methods", func(t *testing.T) {
+		t.Parallel()
+		r := newRouter(nil)
+
+		msg1 := "firstRoute"
+		msg2 := "secondRoute"
+		err := r.handle(http.MethodGet, "/post", firstRoute(msg1), firstRoute(msg1))
+		attest.Ok(t, err)
+
+		// This one returns with a conflict message.
+		errB := r.handle(MethodAll, "post/", secondRoute(msg2), secondRoute(msg2))
+		attest.Error(t, errB)
+	})
 }
 
 func TestNotFound(t *testing.T) {

mux/mux_test.go

@@ -0,0 +1,48 @@
+package mux
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/komuw/ong/config"
+	"go.akshayshah.org/attest"
+)
+
+func TestNew(t *testing.T) {
+	routes := func() []Route {
+		return []Route{
+			NewRoute("/home", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})),
+			NewRoute("/home/", MethodAll, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})),
+		}
+	}
+
+	// There are other tests in internal/mx
+	t.Run("conflict detected", func(t *testing.T) {
+		rtz := []Route{}
+		rtz = append(rtz, tarpitRoutes()...)
+		rtz = append(rtz, routes()...)
+
+		attest.Panics(t, func() {
+			_ = New(config.DevOpts(nil, "secretKey12@34String"), nil, rtz...)
+		})
+	})
+
+	t.Run("okay", func(t *testing.T) {
+		rtz := []Route{
+			NewRoute("/home", MethodGet, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})),
+			NewRoute("/health/", MethodAll, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})),
+		}
+		// does not panic.
+		_ = New(config.DevOpts(nil, "secretKey12@34String"), nil, rtz...)
+	})
+}
+
+func tarpitRoutes() []Route {
+	return []Route{
+		NewRoute(
+			"/libraries/joomla/",
+			MethodAll,
+			http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}),
+		),
+	}
+}