Disambiguate routes when more than one match.

For some path, more than one route can match.  We try hard to
disambiguate them, and if we can't, we return an error with the list of
routes matching the request.

In order to do that, the Match function needs to be able to also return
an error in addition to a route.

4 files changed, 84 insertions, 14 deletions

diff --git a/dispatcher.go b/dispatcher.go
index 8948fad..2e5f277 100644
--- a/dispatcher.go
+++ b/dispatcher.go
@@ -2,6 +2,8 @@ package mooh
 
 import (
 	"strings"
+	"errors"
+	"fmt"
 )
 
 type Dispatcher struct {
@@ -37,20 +39,56 @@ func (self *Dispatcher) AddRoute(path string, method string, code func(*Request)
 	}
 }
 
-func (self *Dispatcher) Match(request Request) *Route {
+func (self *Dispatcher) Match(request Request) (*Route, error) {
 
 	// If the path starts with /, remove it
 	if request.Request.URL.Path[0] == '/' {
 		request.Request.URL.Path = strings.TrimLeft(request.Request.URL.Path, "/")
 	}
 
+	matches := []*Match{}
+
 	for _, r := range self.Routes {
 		match := r.Match(request)
 		if match != nil {
-			return match.Route
+			matches = append(matches, match)
 		}
 	}
-	return nil
+
+	if len(matches) == 0 {
+		return nil, nil
+	} else if len(matches) == 1 {
+		return &matches[0].Route, nil
+	} else {
+		return self.disambiguateMatches(request.Request.URL.Path, matches)
+	}
+	return nil, nil
+}
+
+func (self *Dispatcher) disambiguateMatches(path string, matches []*Match) (*Route, error) {
+	min := -1
+	found := []*Match{}
+
+	for _, m := range matches {
+		req := m.Route.RequiredNamedComponents
+		vars := len(req)
+		if min == -1 || vars < min {
+			found = append(found, m)
+			min = vars
+		} else if vars == min {
+			found = append(found, m)
+		}
+	}
+
+	if len(found) > 1 {
+		msg := fmt.Sprintf("Ambiguous match: path %s could match any of:", path)
+		for _, f := range found {
+			msg = fmt.Sprintf("%s %s", msg, f.Route.Path)
+		}
+		err := errors.New(msg)
+		return nil, err
+	}
+	return &found[0].Route, nil
 }
 
 // func (*dispatcher) ListRoutes() {
diff --git a/mooh.go b/mooh.go
index 6e0670f..dcee3c9 100644
--- a/mooh.go
+++ b/mooh.go
@@ -11,8 +11,12 @@ func (self *Dispatcher) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
 		req,
 	}
 
-	route := self.Match(request)
+	route, err := self.Match(request)
 
+	if err != nil {
+		fmt.Printf("oups")
+		return
+	}
 	if route == nil {
 		fmt.Fprint(resp, "Not Found")
 		return
diff --git a/route.go b/route.go
index d84d026..b26ad5a 100644
--- a/route.go
+++ b/route.go
@@ -14,12 +14,12 @@ type Route struct {
 	RequiredNamedComponents map[string]bool
 	OptionalNamedComponents map[string]bool
 	Length                  int
-	LengthWithoutOptional int
+	LengthWithoutOptional   int
 }
 
 type Match struct {
 	Path    string
-	Route   *Route
+	Route   Route
 	Mapping map[string]string
 	Method  string
 }
@@ -29,7 +29,6 @@ var componentsIsOptional = regexp.MustCompile("^?:")
 var namedComponentsRegex = regexp.MustCompile("^:(.*)$")
 
 func (self *Route) Match(request Request) *Match {
-
 	methodMatch := false
 	for m, _ := range self.Executors {
 		if m == request.Method {
@@ -42,6 +41,7 @@ func (self *Route) Match(request Request) *Match {
 
 	components := strings.Split(request.Request.URL.Path, "/")
 
+	//fmt.Println(fmt.Sprintf("components are %s and %s, the lenght is %d, the length without opt is %d and total is %d", components, self.Components, len(components), self.LengthWithoutOptional, self.Length))
 	if len(components) < self.LengthWithoutOptional || len(components) > self.Length {
 		return nil
 	}
@@ -63,14 +63,12 @@ func (self *Route) Match(request Request) *Match {
 		}
 	}
 
-	match := &Match{
+	return &Match{
 		Path:    request.Request.URL.Path,
-		Route:   self,
-		Mapping: mapping,
+		Route:   *self,
 		Method:  request.Method,
+		Mapping: mapping,
 	}
-
-	return match
 }
 
 func (self *Route) Execute(request *Request) (Response, error) {
diff --git a/router_test.go b/router_test.go
index 28895b9..3a2d66d 100644
--- a/router_test.go
+++ b/router_test.go
@@ -39,7 +39,7 @@ func TestMatch(t *testing.T) {
 	for _, p := range pathToTests {
 		r := http.Request{URL: &p, Method: "GET"}
 		req := Request{&r}
-		m := router.Match(req)
+		m, _ := router.Match(req)
 		if m == nil {
 			t.Fatal()
 		}else{
@@ -62,7 +62,7 @@ func TestMatchOptional(t *testing.T) {
 	for _, p := range pathToTests {
 		r := http.Request{URL: &p, Method: "GET"}
 		req := Request{&r}
-		m := router.Match(req)
+		m, _ := router.Match(req)
 		if m == nil {
 			t.Fatal()
 		}else{
@@ -70,3 +70,33 @@ func TestMatchOptional(t *testing.T) {
 		}
 	}
 }
+
+func TestAmbigiousSimple(t *testing.T) {
+	router := BuildDispatcher()
+	router.AddRoute("/foo/bar", "GET", testRoute)
+	router.AddRoute("/foo/:bar", "GET", testRoute)
+
+	r := Request{&http.Request{Method: "GET", URL: &url.URL{Path: "/foo/bar"}}}
+	m, _ := router.Match(r)
+	if m == nil {
+		t.Fatal()
+	}else{
+		fmt.Println(fmt.Sprintf("%s match for %s", r.Request.URL.Path, m.Path))
+	}
+}
+
+func TestAmbigiousFail(t *testing.T) {
+	router := BuildDispatcher()
+	router.AddRoute("/foo/:bar", "GET", testRoute)
+	router.AddRoute("/:foo/bar", "GET", testRoute)
+
+	r := Request{&http.Request{Method: "GET", URL: &url.URL{Path: "/foo/bar"}}}
+	m, err := router.Match(r)
+	if m != nil {
+		t.Fatal()
+	}
+	if err == nil {
+		t.Fatal()
+	}
+	fmt.Println(err)
+}