diff --git a/internal/pathtree/tree.go b/internal/pathtree/tree.go index 23f7102..20fc06d 100644 --- a/internal/pathtree/tree.go +++ b/internal/pathtree/tree.go @@ -33,8 +33,11 @@ type Params map[string]string // nolint: gocyclo func (n *Node) AddRoute(route string, value interface{}) error { if route == "/" { - n.value = value - return nil + if n.value == nil { + n.value = value + return nil + } + return fmt.Errorf("route / conflicts with existing route") } segments := toSegments(route) diff --git a/internal/pathtree/tree_test.go b/internal/pathtree/tree_test.go index c628b7b..44c4e66 100644 --- a/internal/pathtree/tree_test.go +++ b/internal/pathtree/tree_test.go @@ -7,233 +7,239 @@ import ( "github.com/stretchr/testify/assert" ) -func TestResolve_Root(t *testing.T) { - tree := NewNode() - tree.AddRoute("/", function.ID("testid")) +func TestAddRoute(t *testing.T) { + t.Run("root conflict", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/", function.ID("testid1")) - functionID, _ := tree.Resolve("/") + err := tree.AddRoute("/", function.ID("testid2")) - assert.Equal(t, function.ID("testid"), functionID) -} + assert.EqualError(t, err, "route / conflicts with existing route") + }) -func TestResolve_NoRoot(t *testing.T) { - tree := NewNode() + t.Run("static route conflict", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/a", function.ID("testid1")) - functionID, _ := tree.Resolve("/") + err := tree.AddRoute("/a", function.ID("testid2")) - assert.Nil(t, functionID) -} + assert.EqualError(t, err, "route /a conflicts with existing route") + }) -func TestResolve_Static(t *testing.T) { - tree := NewNode() - tree.AddRoute("/a", function.ID("testid1")) - tree.AddRoute("/b", function.ID("testid2")) - tree.AddRoute("/a/b", function.ID("testid3")) - tree.AddRoute("/d/e/f", function.ID("testid4")) + t.Run("parametrized route conflict", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/:foo", function.ID("testid1")) - functionID, _ := tree.Resolve("/a") - assert.Equal(t, function.ID("testid1"), functionID) + err := tree.AddRoute("/:bar", function.ID("testid2")) - functionID, _ = tree.Resolve("/b") - assert.Equal(t, function.ID("testid2"), functionID) + assert.EqualError(t, err, `parameter with different name ("foo") already defined: for route: /:bar`) + }) - functionID, _ = tree.Resolve("/a/b") - assert.Equal(t, function.ID("testid3"), functionID) + t.Run("parameterized and static route conflict", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/:foo", function.ID("testid1")) - functionID, _ = tree.Resolve("/d/e/f") - assert.Equal(t, function.ID("testid4"), functionID) -} + err := tree.AddRoute("/bar", function.ID("testid2")) -func TestResolve_StaticConflict(t *testing.T) { - tree := NewNode() - tree.AddRoute("/a", function.ID("testid1")) + assert.EqualError(t, err, `parameter with different name ("foo") already defined: for route: /bar`) + }) - err := tree.AddRoute("/a", function.ID("testid2")) + t.Run("wildcard conflict", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/*foo", function.ID("testid1")) - assert.EqualError(t, err, "route /a conflicts with existing route") -} + err := tree.AddRoute("/*bar", function.ID("testid2")) -func TestResolve_NoPath(t *testing.T) { - tree := NewNode() - tree.AddRoute("/a/b/c/d", function.ID("testid1")) + assert.EqualError(t, err, `wildcard with different name ("foo") already defined: for route: /*bar`) + }) - functionID, _ := tree.Resolve("/b") - assert.Nil(t, functionID) - functionID, _ = tree.Resolve("/a/b") - assert.Nil(t, functionID) -} + t.Run("wildcard and paramerized route conflict", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/*foo", function.ID("testid1")) -func TestResolve_TrailingSlash(t *testing.T) { - tree := NewNode() - tree.AddRoute("/a/", function.ID("testid1")) + err := tree.AddRoute("/bar", function.ID("testid2")) + assert.EqualError(t, err, `wildcard with different name ("foo") already defined: for route: /bar`) - functionID, _ := tree.Resolve("/a") - assert.Nil(t, functionID) - functionID, _ = tree.Resolve("/a/") - assert.Equal(t, function.ID("testid1"), functionID) -} + err = tree.AddRoute("/:bar", function.ID("testid2")) + assert.EqualError(t, err, `wildcard with different name ("foo") already defined: for route: /:bar`) + }) -func TestResolve_Param(t *testing.T) { - tree := NewNode() - tree.AddRoute("/:name", function.ID("testid1")) - tree.AddRoute("/:name/:id", function.ID("testid2")) + t.Run("paramerized route and wildcard conflict", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/:foo", function.ID("testid1")) - functionID, params := tree.Resolve("/foo") - assert.Equal(t, function.ID("testid1"), functionID) - assert.EqualValues(t, Params{"name": "foo"}, params) + err := tree.AddRoute("/*bar", function.ID("testid2")) + assert.EqualError(t, err, `wildcard with different name ("foo") already defined: for route: /*bar`) + }) - functionID, params = tree.Resolve("/foo/1") - assert.Equal(t, function.ID("testid2"), functionID) - assert.EqualValues(t, Params{"name": "foo", "id": "1"}, params) -} + t.Run("wildcard not last", func(t *testing.T) { + tree := NewNode() -func TestResolve_ParamNoMatch(t *testing.T) { - tree := NewNode() - tree.AddRoute("/:name", function.ID("testid1")) + err := tree.AddRoute("/*foo/bar", function.ID("testid1")) - functionID, _ := tree.Resolve("/foo/bar/baz") - assert.Nil(t, functionID) + assert.EqualError(t, err, "wildcard must be the last parameter") + }) } -func TestResolve_ParamAndStatic(t *testing.T) { - tree := NewNode() - tree.AddRoute("/:name/bar/:id", function.ID("testid1")) +func TestResolve(t *testing.T) { + t.Run("root", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/", function.ID("testid")) - functionID, params := tree.Resolve("/foo/bar/baz") - assert.Equal(t, function.ID("testid1"), functionID) - assert.EqualValues(t, Params{"name": "foo", "id": "baz"}, params) -} + functionID, _ := tree.Resolve("/") -func TestResolve_ParamConflict(t *testing.T) { - tree := NewNode() - tree.AddRoute("/:foo", function.ID("testid1")) + assert.Equal(t, function.ID("testid"), functionID) + }) - err := tree.AddRoute("/:bar", function.ID("testid2")) + t.Run("no root", func(t *testing.T) { + tree := NewNode() - assert.EqualError(t, err, `parameter with different name ("foo") already defined: for route: /:bar`) -} + functionID, _ := tree.Resolve("/") -func TestResolve_ParamStaticConflict(t *testing.T) { - tree := NewNode() - tree.AddRoute("/:foo", function.ID("testid1")) + assert.Nil(t, functionID) + }) - err := tree.AddRoute("/bar", function.ID("testid2")) + t.Run("static", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/a", function.ID("testid1")) + tree.AddRoute("/b", function.ID("testid2")) + tree.AddRoute("/a/b", function.ID("testid3")) + tree.AddRoute("/d/e/f", function.ID("testid4")) - assert.EqualError(t, err, `parameter with different name ("foo") already defined: for route: /bar`) -} + functionID, _ := tree.Resolve("/a") + assert.Equal(t, function.ID("testid1"), functionID) -func TestResolve_StaticParamConflict(t *testing.T) { - tree := NewNode() - tree.AddRoute("/foo/:bar", function.ID("testid1")) + functionID, _ = tree.Resolve("/b") + assert.Equal(t, function.ID("testid2"), functionID) - err := tree.AddRoute("/:bar", function.ID("testid2")) + functionID, _ = tree.Resolve("/a/b") + assert.Equal(t, function.ID("testid3"), functionID) - assert.EqualError(t, err, "static route already defined for route: /:bar") -} + functionID, _ = tree.Resolve("/d/e/f") + assert.Equal(t, function.ID("testid4"), functionID) + }) -func TestResolve_Wildcard(t *testing.T) { - tree := NewNode() - tree.AddRoute("/*foo", function.ID("testid1")) + t.Run("no path", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/a/b/c/d", function.ID("testid1")) - functionID, params := tree.Resolve("/foo/bar/baz") - assert.Equal(t, function.ID("testid1"), functionID) - assert.EqualValues(t, Params{"foo": "foo/bar/baz"}, params) -} + functionID, _ := tree.Resolve("/b") + assert.Nil(t, functionID) + functionID, _ = tree.Resolve("/a/b") + assert.Nil(t, functionID) + }) -func TestResolve_WildcardNotLast(t *testing.T) { - tree := NewNode() + t.Run("trailing slash", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/a/", function.ID("testid1")) - err := tree.AddRoute("/*foo/bar", function.ID("testid1")) + functionID, _ := tree.Resolve("/a") + assert.Nil(t, functionID) + functionID, _ = tree.Resolve("/a/") + assert.Equal(t, function.ID("testid1"), functionID) + }) - assert.EqualError(t, err, "wildcard must be the last parameter") -} + t.Run("param", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/:name", function.ID("testid1")) + tree.AddRoute("/:name/:id", function.ID("testid2")) -func TestResolve_WildcardConflict(t *testing.T) { - tree := NewNode() - tree.AddRoute("/*foo", function.ID("testid1")) + functionID, params := tree.Resolve("/foo") + assert.Equal(t, function.ID("testid1"), functionID) + assert.EqualValues(t, Params{"name": "foo"}, params) - err := tree.AddRoute("/*bar", function.ID("testid2")) + functionID, params = tree.Resolve("/foo/1") + assert.Equal(t, function.ID("testid2"), functionID) + assert.EqualValues(t, Params{"name": "foo", "id": "1"}, params) + }) - assert.EqualError(t, err, `wildcard with different name ("foo") already defined: for route: /*bar`) -} + t.Run("param doesn't match", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/:name", function.ID("testid1")) -func TestResolve_WildcardParamConflict(t *testing.T) { - tree := NewNode() - tree.AddRoute("/*foo", function.ID("testid1")) + functionID, _ := tree.Resolve("/foo/bar/baz") + assert.Nil(t, functionID) + }) - err := tree.AddRoute("/bar", function.ID("testid2")) - assert.EqualError(t, err, `wildcard with different name ("foo") already defined: for route: /bar`) + t.Run("param and static", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/:name/bar/:id", function.ID("testid1")) - err = tree.AddRoute("/:bar", function.ID("testid2")) - assert.EqualError(t, err, `wildcard with different name ("foo") already defined: for route: /:bar`) -} + functionID, params := tree.Resolve("/foo/bar/baz") + assert.Equal(t, function.ID("testid1"), functionID) + assert.EqualValues(t, Params{"name": "foo", "id": "baz"}, params) + }) -func TestResolve_ParamWildcardConflict(t *testing.T) { - tree := NewNode() - tree.AddRoute("/:foo", function.ID("testid1")) + t.Run("wildcard", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/*foo", function.ID("testid1")) - err := tree.AddRoute("/*bar", function.ID("testid2")) - assert.EqualError(t, err, `wildcard with different name ("foo") already defined: for route: /*bar`) + functionID, params := tree.Resolve("/foo/bar/baz") + assert.Equal(t, function.ID("testid1"), functionID) + assert.EqualValues(t, Params{"foo": "foo/bar/baz"}, params) + }) } func TestDeleteRoute_Root(t *testing.T) { - tree := NewNode() - tree.AddRoute("/", function.ID("testid")) - tree.DeleteRoute("/") - - functionID, _ := tree.Resolve("/") - - assert.Nil(t, functionID) -} - -func TestDeleteRoute_Static(t *testing.T) { - tree := NewNode() - tree.AddRoute("/a/b/c", function.ID("testid1")) - tree.DeleteRoute("/a/b/c") - - functionID, _ := tree.Resolve("/a/b/c") - - assert.Nil(t, functionID) -} - -func TestDeleteRoute_StaticWithChild(t *testing.T) { - tree := NewNode() - tree.AddRoute("/a", function.ID("testid1")) - tree.AddRoute("/a/b", function.ID("testid2")) - tree.DeleteRoute("/a") - - functionID, _ := tree.Resolve("/a") - assert.Nil(t, functionID) - functionID, _ = tree.Resolve("/a/b") - assert.Equal(t, function.ID("testid2"), functionID) -} - -func TestDeleteRoute_ParamWithChild(t *testing.T) { - tree := NewNode() - tree.AddRoute("/:foo", function.ID("testid1")) - tree.AddRoute("/:foo/bar", function.ID("testid2")) - - tree.DeleteRoute("/:foo") - - functionID, _ := tree.Resolve("/a") - assert.Nil(t, functionID) - functionID, _ = tree.Resolve("/a/bar") - assert.Equal(t, function.ID("testid2"), functionID) -} - -func TestDeleteRoute_NonExisting(t *testing.T) { - tree := NewNode() - - err := tree.DeleteRoute("/a") - assert.EqualError(t, err, "unable to delete node non existing node") -} - -func TestDeleteRoute_DeleteParamAddStatic(t *testing.T) { - tree := NewNode() - tree.AddRoute("/:foo", function.ID("testid1")) - tree.DeleteRoute("/:foo") - tree.AddRoute("/a", function.ID("testid2")) - - functionID, _ := tree.Resolve("/a") - assert.Equal(t, function.ID("testid2"), functionID) + t.Run("root", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/", function.ID("testid")) + tree.DeleteRoute("/") + + functionID, _ := tree.Resolve("/") + + assert.Nil(t, functionID) + }) + + t.Run("static", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/a/b/c", function.ID("testid1")) + tree.DeleteRoute("/a/b/c") + + functionID, _ := tree.Resolve("/a/b/c") + + assert.Nil(t, functionID) + }) + + t.Run("static without child", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/a", function.ID("testid1")) + tree.AddRoute("/a/b", function.ID("testid2")) + tree.DeleteRoute("/a") + + functionID, _ := tree.Resolve("/a") + assert.Nil(t, functionID) + functionID, _ = tree.Resolve("/a/b") + assert.Equal(t, function.ID("testid2"), functionID) + }) + + t.Run("param with child", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/:foo", function.ID("testid1")) + tree.AddRoute("/:foo/bar", function.ID("testid2")) + + tree.DeleteRoute("/:foo") + + functionID, _ := tree.Resolve("/a") + assert.Nil(t, functionID) + functionID, _ = tree.Resolve("/a/bar") + assert.Equal(t, function.ID("testid2"), functionID) + }) + + t.Run("non existing route", func(t *testing.T) { + tree := NewNode() + + err := tree.DeleteRoute("/a") + assert.EqualError(t, err, "unable to delete node non existing node") + }) + + t.Run("param and static", func(t *testing.T) { + tree := NewNode() + tree.AddRoute("/:foo", function.ID("testid1")) + tree.DeleteRoute("/:foo") + tree.AddRoute("/a", function.ID("testid2")) + + functionID, _ := tree.Resolve("/a") + assert.Equal(t, function.ID("testid2"), functionID) + }) } diff --git a/libkv/subscription.go b/libkv/subscription.go index 8e9e1d1..40117b1 100644 --- a/libkv/subscription.go +++ b/libkv/subscription.go @@ -163,10 +163,9 @@ func (service Service) GetSubscription(space string, id subscription.ID) (*subsc } func (service Service) checkForPathConflict(space, method, path string) error { - kvs, err := service.SubscriptionStore.List(spacePath(space), &store.ReadOptions{Consistent: true}) - tree := pathtree.NewNode() + kvs, err := service.SubscriptionStore.List(spacePath(space), &store.ReadOptions{Consistent: true}) for _, kv := range kvs { sub := &subscription.Subscription{} err = json.NewDecoder(bytes.NewReader(kv.Value)).Decode(sub)