veyron/services/mounttable: Fix Glob bug.
The change fixes a bug where Glob is called on a name that is below a
mount point, e.g. "server" is mounted at "a/b" and we call
"a/b/c".Glob(*). The correct behavior is to return "" -> "server/c".
Previously, this would have returned nothing.
Change-Id: I5bbeb60488b34c9ecc2439e8270c9abe90185f5f
diff --git a/runtimes/google/naming/namespace/all_test.go b/runtimes/google/naming/namespace/all_test.go
index c29e4f9..6350997 100644
--- a/runtimes/google/naming/namespace/all_test.go
+++ b/runtimes/google/naming/namespace/all_test.go
@@ -9,6 +9,7 @@
"testing"
"time"
+ "veyron/lib/glob"
_ "veyron/lib/testutil"
"veyron/runtimes/google/naming/namespace"
service "veyron/services/mounttable/lib"
@@ -17,7 +18,9 @@
"veyron2/ipc"
"veyron2/naming"
"veyron2/rt"
+ "veyron2/security"
"veyron2/services/mounttable"
+ "veyron2/services/mounttable/types"
"veyron2/vlog"
)
@@ -52,12 +55,56 @@
return replies
}
-type testServer struct{}
+type testServer struct {
+ suffix string
+}
-func (*testServer) KnockKnock(call ipc.ServerCall) string {
+func (testServer) KnockKnock(call ipc.ServerCall) string {
return "Who's there?"
}
+// Glob applies pattern to the following tree:
+// "" -> {level1} -> {level2}
+// "".Glob("*") returns "level1"
+// "".Glob("...") returns "level1" and "level1/level2"
+// "level1".Glob("*") returns "level2"
+func (t *testServer) Glob(call ipc.ServerCall, pattern string) error {
+ g, err := glob.Parse(pattern)
+ if err != nil {
+ return err
+ }
+ tree := []string{"", "level1", "level2"}
+ for i, leaf := range tree {
+ if leaf == t.suffix {
+ return t.globLoop(call, "", g, tree[i+1:])
+ }
+ }
+ return nil
+}
+
+func (t *testServer) globLoop(call ipc.ServerCall, prefix string, g *glob.Glob, tree []string) error {
+ if g.Len() == 0 {
+ if err := call.Send(types.MountEntry{Name: prefix}); err != nil {
+ return err
+ }
+ }
+ if g.Finished() || len(tree) == 0 {
+ return nil
+ }
+ if ok, left := g.MatchInitialSegment(tree[0]); ok {
+ if err := t.globLoop(call, naming.Join(prefix, tree[0]), left, tree[1:]); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+type dispatcher struct{}
+
+func (d *dispatcher) Lookup(suffix, method string) (ipc.Invoker, security.Authorizer, error) {
+ return ipc.ReflectInvoker(&testServer{suffix}), nil, nil
+}
+
func knockKnock(t *testing.T, runtime veyron2.Runtime, name string) {
client := runtime.Client()
ctx := runtime.NewContext()
@@ -167,7 +214,7 @@
return root, mps
}
-// createNamespace creates a hiearachy of mounttables and servers
+// createNamespace creates a hierarchy of mounttables and servers
// as follows:
// /mt1, /mt2, /mt3, /mt4, /mt5, /joke1, /joke2, /joke3.
// That is, mt1 is a mount table mounted in the root mount table,
@@ -177,7 +224,7 @@
jokes := make(map[string]*serverEntry)
// Let's run some non-mount table services.
for _, j := range []string{j1MP, j2MP, j3MP} {
- disp := ipc.LeafDispatcher(&testServer{}, nil)
+ disp := &dispatcher{}
jokes[j] = runServer(t, r, disp, j)
}
return root, mts, jokes, func() {
@@ -338,12 +385,12 @@
ns := r.Namespace()
ns.SetRoots(root.name)
- // Let's run some non-mount table servic
+ // Let's run some non-mount table services
for _, j := range []string{j1MP, j2MP, j3MP} {
testResolve(t, r, ns, j, jokes[j].name)
knockKnock(t, r, j)
globalName := naming.JoinAddressName(mts["mt4"].name, j)
- disp := ipc.LeafDispatcher(&testServer{}, nil)
+ disp := &dispatcher{}
gj := "g_" + j
jokes[gj] = runServer(t, r, disp, globalName)
testResolve(t, r, ns, "mt4/"+j, jokes[gj].name)
@@ -365,7 +412,9 @@
tln := []string{"baz", "mt1", "mt2", "mt3", "mt4", "mt5", "joke1", "joke2", "joke3"}
barbaz := []string{"mt4/foo/bar", "mt4/foo/baz"}
+ level12 := []string{"joke1/level1", "joke1/level1/level2", "joke2/level1", "joke2/level1/level2", "joke3/level1", "joke3/level1/level2"}
foo := append([]string{"mt4/foo"}, barbaz...)
+ foo = append(foo, level12...)
// Try various globs.
globTests := []struct {
pattern string
@@ -378,22 +427,27 @@
{"m*", []string{"mt1", "mt2", "mt3", "mt4", "mt5"}},
{"mt[2,3]", []string{"mt2", "mt3"}},
{"*z", []string{"baz"}},
+ {"joke1/*", []string{"joke1/level1"}},
+ {"j?ke1/level1/*", []string{"joke1/level1/level2"}},
+ {"joke1/level1/*", []string{"joke1/level1/level2"}},
+ {"joke1/level1/level2/...", []string{"joke1/level1/level2"}},
{"...", append(append(tln, foo...), "")},
{"*/...", append(tln, foo...)},
{"*/foo/*", barbaz},
{"*/*/*z", []string{"mt4/foo/baz"}},
{"*/f??/*z", []string{"mt4/foo/baz"}},
+ {"mt4/foo/baz", []string{"mt4/foo/baz"}},
}
for _, test := range globTests {
out := doGlob(t, r, ns, test.pattern, 0)
- compare(t, "Glob", test.pattern, test.expected, out)
+ compare(t, "Glob", test.pattern, out, test.expected)
// Do the same with a full rooted name.
out = doGlob(t, r, ns, naming.JoinAddressName(root.name, test.pattern), 0)
var expectedWithRoot []string
for _, s := range test.expected {
expectedWithRoot = append(expectedWithRoot, naming.JoinAddressName(root.name, s))
}
- compare(t, "Glob", test.pattern, expectedWithRoot, out)
+ compare(t, "Glob", test.pattern, out, expectedWithRoot)
}
}
diff --git a/services/mounttable/lib/mounttable.go b/services/mounttable/lib/mounttable.go
index 2a57fdc..f87605f 100644
--- a/services/mounttable/lib/mounttable.go
+++ b/services/mounttable/lib/mounttable.go
@@ -382,12 +382,27 @@
mt.Lock()
defer mt.Unlock()
- // If we can't find the node corresponding to the current name then there are no results.
+ // If the current name is not fully resolvable on this nameserver we
+ // don't need to evaluate the glob expression. Send a partially resolved
+ // name back to the client.
n := mt.findNode(ms.cleanedElems, false)
if n == nil {
+ ms.linkToLeaf(reply)
return nil
}
mt.globStep(n, "", g, context, reply)
return nil
}
+
+func (ms *mountContext) linkToLeaf(reply mounttable.GlobbableServiceGlobStream) {
+ n, elems := ms.mt.walk(ms.mt.root, ms.cleanedElems)
+ if n == nil {
+ return
+ }
+ servers := n.mount.servers.copyToSlice()
+ for i, s := range servers {
+ servers[i].Server = naming.Join(s.Server, slashSlashJoin(elems))
+ }
+ reply.SendStream().Send(types.MountEntry{Name: "", Servers: servers})
+}
diff --git a/services/mounttable/lib/mounttable_test.go b/services/mounttable/lib/mounttable_test.go
index 85cd332..76ed43d 100644
--- a/services/mounttable/lib/mounttable_test.go
+++ b/services/mounttable/lib/mounttable_test.go
@@ -18,6 +18,7 @@
"veyron2/rt"
"veyron2/security"
"veyron2/services/mounttable"
+ "veyron2/services/mounttable/types"
"veyron2/vlog"
)
@@ -333,7 +334,7 @@
defer server.Stop()
// set up a mount space
- fakeServer := naming.JoinAddressName(estr, "quux")
+ fakeServer := naming.JoinAddressName(estr, "//quux")
doMount(t, naming.JoinAddressName(estr, "//one/bright/day"), fakeServer, true, rootID)
doMount(t, naming.JoinAddressName(estr, "//in/the/middle"), fakeServer, true, rootID)
doMount(t, naming.JoinAddressName(estr, "//of/the/night"), fakeServer, true, rootID)
@@ -347,6 +348,7 @@
{"...", []string{"", "one", "in", "of", "one/bright", "in/the", "of/the", "one/bright/day", "in/the/middle", "of/the/night"}},
{"*/...", []string{"one", "in", "of", "one/bright", "in/the", "of/the", "one/bright/day", "in/the/middle", "of/the/night"}},
{"one/...", []string{"one", "one/bright", "one/bright/day"}},
+ {"of/the/night/two/dead/boys", []string{"of/the/night"}},
{"*/the", []string{"in/the", "of/the"}},
{"*/the/...", []string{"in/the", "of/the", "in/the/middle", "of/the/night"}},
{"o*", []string{"one", "of"}},
@@ -356,6 +358,39 @@
out := doGlob(t, naming.JoinAddressName(estr, "//"), test.in, rootID)
checkMatch(t, test.expected, out)
}
+
+ // Test Glob on a name that is under a mounted server. The result should the
+ // the address the mounted server with the extra suffix.
+ {
+ name := naming.JoinAddressName(estr, "//of/the/night/two/dead/boys/got/up/to/fight")
+ pattern := "*"
+ m, err := mounttable.BindGlobbable(name, quuxClient(rootID))
+ if err != nil {
+ boom(t, "Failed to BindMountTable: %s", err)
+ }
+ stream, err := m.Glob(rt.R().NewContext(), pattern)
+ if err != nil {
+ boom(t, "Failed call to %s.Glob(%s): %s", name, pattern, err)
+ }
+ var results []types.MountEntry
+ iterator := stream.RecvStream()
+ for iterator.Advance() {
+ results = append(results, iterator.Value())
+ }
+ if err := iterator.Err(); err != nil {
+ boom(t, "Glob %s: %s", name, err)
+ }
+ if len(results) != 1 {
+ boom(t, "Unexpected number of results. Got %v, want 1", len(results))
+ }
+ if results[0].Name != "" {
+ boom(t, "Unexpected name. Got %v, want ''", results[0].Name)
+ }
+ _, suffix := naming.SplitAddressName(results[0].Servers[0].Server)
+ if expected := "//quux/two/dead/boys/got/up/to/fight"; suffix != expected {
+ boom(t, "Unexpected suffix. Got %v, want %v", suffix, expected)
+ }
+ }
}
func TestGlobACLs(t *testing.T) {
