Merge "TBR: ref: Replace V23_ROOT with JIRI_ROOT everywhere."
diff --git a/services/syncbase/server/app.go b/services/syncbase/server/app.go
index 9bcec0d..a2b1c4f 100644
--- a/services/syncbase/server/app.go
+++ b/services/syncbase/server/app.go
@@ -9,6 +9,7 @@
"sync"
"v.io/v23/context"
+ "v.io/v23/glob"
"v.io/v23/rpc"
"v.io/v23/security/access"
wire "v.io/v23/services/syncbase"
@@ -41,8 +42,6 @@
////////////////////////////////////////
// RPC methods
-// TODO(sadovsky): Implement Glob__ or GlobChildren__.
-
// TODO(sadovsky): Require the app name to match the client's blessing name.
// I.e. reserve names at the app level of the hierarchy.
func (a *app) Create(ctx *context.T, call rpc.ServerCall, perms access.Permissions) error {
@@ -84,17 +83,17 @@
return data.Perms, util.FormatVersion(data.Version), nil
}
-func (a *app) ListDatabases(ctx *context.T, call rpc.ServerCall) ([]string, error) {
+func (a *app) GlobChildren__(ctx *context.T, call rpc.GlobChildrenServerCall, matcher *glob.Element) error {
if !a.exists {
- return nil, verror.New(verror.ErrNoExist, ctx, a.name)
+ return verror.New(verror.ErrNoExist, ctx, a.name)
}
// Check perms.
sn := a.s.st.NewSnapshot()
if err := util.GetWithAuth(ctx, call, sn, a.stKey(), &appData{}); err != nil {
sn.Abort()
- return nil, err
+ return err
}
- return util.ListChildren(ctx, call, sn, util.JoinKeyParts(util.DbInfoPrefix, a.name))
+ return util.GlobChildren(ctx, call, matcher, sn, sn.Abort, util.JoinKeyParts(util.DbInfoPrefix, a.name))
}
////////////////////////////////////////
diff --git a/services/syncbase/server/dispatcher.go b/services/syncbase/server/dispatcher.go
index 06a6757..a5b9bba 100644
--- a/services/syncbase/server/dispatcher.go
+++ b/services/syncbase/server/dispatcher.go
@@ -33,34 +33,32 @@
var auth security.Authorizer = security.AllowEveryone()
func (disp *dispatcher) Lookup(ctx *context.T, suffix string) (interface{}, security.Authorizer, error) {
+ // TODO(sadovsky): Can we drop this TrimPrefix?
suffix = strings.TrimPrefix(suffix, "/")
-
if len(suffix) == 0 {
return wire.ServiceServer(disp.s), auth, nil
}
+ parts := strings.SplitN(suffix, "/", 2)
// If the first slash-separated component of suffix is SyncbaseSuffix,
// dispatch to the sync module.
- parts := strings.SplitN(suffix, "/", 2)
if parts[0] == util.SyncbaseSuffix {
return interfaces.SyncServer(disp.s.sync), auth, nil
}
- // Otherwise, split on NameSepWithSlashes to get hierarchy component names.
- parts = strings.SplitN(suffix, pubutil.NameSepWithSlashes, 2)
-
- // Validate all key atoms up front, so that we can avoid doing so in all our
- // method implementations.
- appName := parts[0]
- if !pubutil.ValidName(appName) {
+ // Validate all name components up front, so that we can avoid doing so in all
+ // our method implementations.
+ escAppName := parts[0]
+ appName, ok := pubutil.Unescape(escAppName)
+ if !ok || !pubutil.ValidAppName(appName) {
return nil, nil, wire.NewErrInvalidName(ctx, suffix)
}
- aExists := false
+ appExists := false
var a *app
if aInt, err := disp.s.App(nil, nil, appName); err == nil {
a = aInt.(*app) // panics on failure, as desired
- aExists = true
+ appExists = true
} else {
if verror.ErrorID(err) != verror.ErrNoExist.ID {
return nil, nil, err
@@ -78,7 +76,7 @@
// All database, table, and row methods require the app to exist. If it
// doesn't, abort early.
- if !aExists {
+ if !appExists {
return nil, nil, verror.New(verror.ErrNoExist, ctx, a.name)
}
diff --git a/services/syncbase/server/nosql/database.go b/services/syncbase/server/nosql/database.go
index f6e14e1..40da19e 100644
--- a/services/syncbase/server/nosql/database.go
+++ b/services/syncbase/server/nosql/database.go
@@ -7,11 +7,11 @@
import (
"math/rand"
"path"
- "strings"
"sync"
"time"
"v.io/v23/context"
+ "v.io/v23/glob"
"v.io/v23/rpc"
"v.io/v23/security/access"
wire "v.io/v23/services/syncbase/nosql"
@@ -127,8 +127,6 @@
////////////////////////////////////////
// RPC methods
-// TODO(sadovsky): Implement Glob__ or GlobChildren__.
-
func (d *databaseReq) Create(ctx *context.T, call rpc.ServerCall, metadata *wire.SchemaMetadata, perms access.Permissions) error {
if d.exists {
return verror.New(verror.ErrExist, ctx, d.name)
@@ -195,7 +193,7 @@
}
}
}
- return strings.Join([]string{d.name, util.JoinBatchInfo(batchType, id)}, util.BatchSep), nil
+ return util.BatchSep + util.JoinBatchInfo(batchType, id), nil
}
func (d *databaseReq) Commit(ctx *context.T, call rpc.ServerCall, schemaVersion int32) error {
@@ -318,24 +316,26 @@
return data.Perms, util.FormatVersion(data.Version), nil
}
-func (d *databaseReq) ListTables(ctx *context.T, call rpc.ServerCall) ([]string, error) {
+func (d *databaseReq) GlobChildren__(ctx *context.T, call rpc.GlobChildrenServerCall, matcher *glob.Element) error {
if !d.exists {
- return nil, verror.New(verror.ErrNoExist, ctx, d.name)
+ return verror.New(verror.ErrNoExist, ctx, d.name)
}
- impl := func(sntx store.SnapshotOrTransaction) ([]string, error) {
+ impl := func(sntx store.SnapshotOrTransaction, closeSntx func() error) error {
// Check perms.
- if err := util.GetWithAuth(ctx, call, sntx, d.stKey(), &databaseData{}); err != nil {
- sntx.Abort()
- return nil, err
+ sn := d.st.NewSnapshot()
+ if err := util.GetWithAuth(ctx, call, sn, d.stKey(), &databaseData{}); err != nil {
+ sn.Abort()
+ return err
}
- return util.ListChildren(ctx, call, sntx, util.TablePrefix)
+ return util.GlobChildren(ctx, call, matcher, sn, closeSntx, util.TablePrefix)
}
if d.batchId != nil {
- return impl(d.batchReader())
+ return impl(d.batchReader(), func() error {
+ return nil
+ })
} else {
- sntx := d.st.NewSnapshot()
- defer sntx.Abort()
- return impl(sntx)
+ sn := d.st.NewSnapshot()
+ return impl(sn, sn.Abort)
}
}
diff --git a/services/syncbase/server/nosql/database_watch.go b/services/syncbase/server/nosql/database_watch.go
index 374cab4..1b8f8f9 100644
--- a/services/syncbase/server/nosql/database_watch.go
+++ b/services/syncbase/server/nosql/database_watch.go
@@ -171,8 +171,8 @@
continue
}
parts := util.SplitKeyParts(opKey)
- // TODO(rogulenko): Currently we process only rows, i.e. keys of the form
- // <RowPrefix>:xxx:yyy. Consider processing other keys.
+ // TODO(rogulenko): Currently we only process rows, i.e. keys of the form
+ // <rowPrefix>:xxx:yyy. Consider processing other keys.
if len(parts) != 3 || parts[0] != util.RowPrefix {
continue
}
@@ -188,7 +188,7 @@
continue
}
change := watch.Change{
- Name: naming.Join(table, pubutil.NameSep, row),
+ Name: naming.Join(table, row),
Continued: true,
}
switch op := logEntry.Op.(type) {
diff --git a/services/syncbase/server/nosql/dispatcher.go b/services/syncbase/server/nosql/dispatcher.go
index 4ec16c0..87ff495 100644
--- a/services/syncbase/server/nosql/dispatcher.go
+++ b/services/syncbase/server/nosql/dispatcher.go
@@ -14,7 +14,6 @@
nosqlWire "v.io/v23/services/syncbase/nosql"
pubutil "v.io/v23/syncbase/util"
"v.io/v23/verror"
- "v.io/x/lib/vlog"
"v.io/x/ref/services/syncbase/server/interfaces"
"v.io/x/ref/services/syncbase/server/util"
)
@@ -33,33 +32,60 @@
// RPC method implementations to perform proper authorization.
var auth security.Authorizer = security.AllowEveryone()
+// Note that our client libraries escape component names (app/db/table names,
+// row keys) embedded in object names, and our server dispatchers immediately
+// unescape them. The only parts of the Syncbase implementation that must be
+// aware of this escaping and unescaping are those parts that deal in object
+// names.
+// This approach confers the following benefits:
+// - the scan and exec implementations need not be aware of key escaping;
+// - row locality corresponds to the lexicographic order of unescaped keys (as
+// clients would expect); and
+// - the object names returned by glob, which are escaped by our GlobChildren
+// implementation, are always valid component names.
+//
+// TODO(sadovsky): Escape app, db, and table names (using an aggressive
+// escaping) when persisting them in underlying storage engines, to prevent
+// shadowing. This is not yet a problem because database and table names are
+// restricted to be valid identifiers, such that neither "<appName>:<dbName>"
+// nor "<tableName>:<rowKey>" can be ambiguous.
func (disp *dispatcher) Lookup(ctx *context.T, suffix string) (interface{}, security.Authorizer, error) {
+ // TODO(sadovsky): Can we drop this TrimPrefix?
suffix = strings.TrimPrefix(suffix, "/")
- parts := strings.Split(suffix, pubutil.NameSepWithSlashes)
+ parts := strings.SplitN(suffix, "/", 3) // db, table, row
- if len(parts) == 0 {
- vlog.Fatal("invalid nosql.dispatcher Lookup")
- }
+ // Note, the slice returned by strings.SplitN is guaranteed to contain at
+ // least one element.
+ dbParts := strings.SplitN(parts[0], util.BatchSep, 2)
+ escDbName := dbParts[0]
- dParts := strings.SplitN(parts[0], util.BatchSep, 2)
- dName := dParts[0]
-
- // Validate all key atoms up front, so that we can avoid doing so in all our
- // method implementations.
- if !pubutil.ValidName(dName) {
+ // Validate all name components up front, so that we can avoid doing so in all
+ // our method implementations.
+ dbName, ok := pubutil.Unescape(escDbName)
+ if !ok || !pubutil.ValidDatabaseName(dbName) {
return nil, nil, wire.NewErrInvalidName(ctx, suffix)
}
- for _, s := range parts[1:] {
- if !pubutil.ValidName(s) {
+
+ var tableName, rowKey string
+ if len(parts) > 1 {
+ tableName, ok = pubutil.Unescape(parts[1])
+ if !ok || !pubutil.ValidTableName(tableName) {
return nil, nil, wire.NewErrInvalidName(ctx, suffix)
}
}
- dExists := false
+ if len(parts) > 2 {
+ rowKey, ok = pubutil.Unescape(parts[2])
+ if !ok || !pubutil.ValidRowKey(rowKey) {
+ return nil, nil, wire.NewErrInvalidName(ctx, suffix)
+ }
+ }
+
+ dbExists := false
var d *database
- if dInt, err := disp.a.NoSQLDatabase(nil, nil, dName); err == nil {
+ if dInt, err := disp.a.NoSQLDatabase(nil, nil, dbName); err == nil {
d = dInt.(*database) // panics on failure, as desired
- dExists = true
+ dbExists = true
} else {
if verror.ErrorID(err) != verror.ErrNoExist.ID {
return nil, nil, err
@@ -67,15 +93,15 @@
// Database does not exist. Create a short-lived database object to
// service this request.
d = &database{
- name: dName,
+ name: dbName,
a: disp.a,
}
}
}
dReq := &databaseReq{database: d}
- if len(dParts) == 2 {
- if !setBatchFields(dReq, dParts[1]) {
+ if len(dbParts) == 2 {
+ if !setBatchFields(dReq, dbParts[1]) {
return nil, nil, wire.NewErrInvalidName(ctx, suffix)
}
}
@@ -85,7 +111,7 @@
// All table and row methods require the database to exist. If it doesn't,
// abort early.
- if !dExists {
+ if !dbExists {
return nil, nil, verror.New(verror.ErrNoExist, ctx, d.name)
}
@@ -94,7 +120,7 @@
// execute, the client may not get an error, but in any case ultimately the
// store will end up in a consistent state.
tReq := &tableReq{
- name: parts[1],
+ name: tableName,
d: dReq,
}
if len(parts) == 2 {
@@ -102,7 +128,7 @@
}
rReq := &rowReq{
- key: parts[2],
+ key: rowKey,
t: tReq,
}
if len(parts) == 3 {
@@ -116,6 +142,8 @@
// value of batchInfo (suffix of the database name component). It returns false
// if batchInfo is malformed.
func setBatchFields(d *databaseReq, batchInfo string) bool {
+ // TODO(sadovsky): Maybe share a common keyspace between sns and txs so that
+ // we can avoid including the batch type in the batchInfo string.
batchType, batchId, err := util.SplitBatchInfo(batchInfo)
if err != nil {
return false
diff --git a/services/syncbase/server/nosql/row.go b/services/syncbase/server/nosql/row.go
index 1efe739..758c58a 100644
--- a/services/syncbase/server/nosql/row.go
+++ b/services/syncbase/server/nosql/row.go
@@ -26,8 +26,6 @@
////////////////////////////////////////
// RPC methods
-// TODO(sadovsky): Implement Glob__ or GlobChildren__.
-
func (r *rowReq) Exists(ctx *context.T, call rpc.ServerCall, schemaVersion int32) (bool, error) {
_, err := r.Get(ctx, call, schemaVersion)
return util.ErrorToExists(err)
diff --git a/services/syncbase/server/nosql/table.go b/services/syncbase/server/nosql/table.go
index 9c62d54..7da77fa 100644
--- a/services/syncbase/server/nosql/table.go
+++ b/services/syncbase/server/nosql/table.go
@@ -8,6 +8,7 @@
"strings"
"v.io/v23/context"
+ "v.io/v23/glob"
"v.io/v23/rpc"
"v.io/v23/security/access"
wire "v.io/v23/services/syncbase/nosql"
@@ -30,8 +31,6 @@
////////////////////////////////////////
// RPC methods
-// TODO(sadovsky): Implement Glob__ or GlobChildren__.
-
func (t *tableReq) Create(ctx *context.T, call rpc.ServerCall, schemaVersion int32, perms access.Permissions) error {
if t.d.batchId != nil {
return wire.NewErrBoundToBatch(ctx)
@@ -365,6 +364,25 @@
}
}
+func (t *tableReq) GlobChildren__(ctx *context.T, call rpc.GlobChildrenServerCall, matcher *glob.Element) error {
+ impl := func(sntx store.SnapshotOrTransaction, closeSntx func() error) error {
+ // Check perms.
+ if err := t.checkAccess(ctx, call, sntx, ""); err != nil {
+ closeSntx()
+ return err
+ }
+ return util.GlobChildren(ctx, call, matcher, sntx, closeSntx, util.JoinKeyParts(util.RowPrefix, t.name))
+ }
+ if t.d.batchId != nil {
+ return impl(t.d.batchReader(), func() error {
+ return nil
+ })
+ } else {
+ sn := t.d.st.NewSnapshot()
+ return impl(sn, sn.Abort)
+ }
+}
+
////////////////////////////////////////
// Internal helpers
diff --git a/services/syncbase/server/service.go b/services/syncbase/server/service.go
index f2ec671..2ddf195 100644
--- a/services/syncbase/server/service.go
+++ b/services/syncbase/server/service.go
@@ -13,6 +13,7 @@
"sync"
"v.io/v23/context"
+ "v.io/v23/glob"
"v.io/v23/rpc"
"v.io/v23/security/access"
wire "v.io/v23/services/syncbase"
@@ -135,8 +136,6 @@
////////////////////////////////////////
// RPC methods
-// TODO(sadovsky): Implement Glob__ or GlobChildren__.
-
func (s *service) SetPermissions(ctx *context.T, call rpc.ServerCall, perms access.Permissions, version string) error {
return store.RunInTransaction(s.st, func(tx store.Transaction) error {
data := &serviceData{}
@@ -159,14 +158,14 @@
return data.Perms, util.FormatVersion(data.Version), nil
}
-func (s *service) ListApps(ctx *context.T, call rpc.ServerCall) ([]string, error) {
+func (s *service) GlobChildren__(ctx *context.T, call rpc.GlobChildrenServerCall, matcher *glob.Element) error {
// Check perms.
sn := s.st.NewSnapshot()
if err := util.GetWithAuth(ctx, call, sn, s.stKey(), &serviceData{}); err != nil {
sn.Abort()
- return nil, err
+ return err
}
- return util.ListChildren(ctx, call, sn, util.AppPrefix)
+ return util.GlobChildren(ctx, call, matcher, sn, sn.Abort, util.AppPrefix)
}
////////////////////////////////////////
diff --git a/services/syncbase/server/util/constants.go b/services/syncbase/server/util/constants.go
index a14d006..5841aa9 100644
--- a/services/syncbase/server/util/constants.go
+++ b/services/syncbase/server/util/constants.go
@@ -9,8 +9,6 @@
)
// Constants related to storage engine keys.
-// Note, these are persisted and therefore must not be modified.
-// Below, they are ordered lexicographically by value.
const (
AppPrefix = "$app"
ClockPrefix = "$clock"
@@ -24,6 +22,8 @@
TablePrefix = "$table"
VersionPrefix = "$version"
+ // Note, these are persisted and therefore must not be modified.
+ // Below, they are ordered lexicographically by value.
// TODO(sadovsky): Changing these prefixes breaks various tests. Tests
// generally shouldn't depend on the values of these constants.
/*
@@ -41,7 +41,8 @@
*/
// Separator for parts of storage engine keys.
- // TODO(sadovsky): Allow ":" in names and use a different separator here.
+ // TODO(sadovsky): Allow ":" in names and use a different separator here,
+ // perhaps "%%".
KeyPartSep = ":"
// PrefixRangeLimitSuffix is a key suffix that indicates the end of a prefix
@@ -53,12 +54,13 @@
const (
// Object name component for Syncbase-to-Syncbase (sync) RPCs.
// Sync object names have the form:
- // <syncbase>/@@sync/...
- SyncbaseSuffix = "@@sync"
+ // <syncbase>/%%sync/...
+ // FIXME: update all apps, again...
+ SyncbaseSuffix = "%%sync"
// Separator for batch info in database names.
// Batch object names have the form:
- // <syncbase>/<app>/$/<database>@@<batchInfo>/...
- BatchSep = "@@"
+ // <syncbase>/<app>/<database>%%<batchInfo>/...
+ BatchSep = "%%"
)
// Constants related to syncbase clock.
diff --git a/services/syncbase/server/util/glob_children.go b/services/syncbase/server/util/glob_children.go
new file mode 100644
index 0000000..e874163
--- /dev/null
+++ b/services/syncbase/server/util/glob_children.go
@@ -0,0 +1,42 @@
+// Copyright 2015 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package util
+
+import (
+ "v.io/v23/context"
+ "v.io/v23/glob"
+ "v.io/v23/naming"
+ "v.io/v23/rpc"
+ pubutil "v.io/v23/syncbase/util"
+ "v.io/x/ref/services/syncbase/store"
+)
+
+// Note, Syncbase handles Glob requests by implementing GlobChildren__ at each
+// level (service, app, database, table).
+
+// GlobChildren performs a glob. It calls closeSntx to close sntx.
+func GlobChildren(ctx *context.T, call rpc.GlobChildrenServerCall, matcher *glob.Element, sntx store.SnapshotOrTransaction, closeSntx func() error, stKeyPrefix string) error {
+ prefix, _ := matcher.FixedPrefix()
+ it := sntx.Scan(ScanPrefixArgs(stKeyPrefix, prefix))
+ defer closeSntx()
+ key := []byte{}
+ for it.Advance() {
+ key = it.Key(key)
+ parts := SplitKeyParts(string(key))
+ name := parts[len(parts)-1]
+ if matcher.Match(name) {
+ // TODO(rogulenko): Check for resolve access. (For table glob, this means
+ // checking prefix perms.)
+ // For explanation of Escape(), see comment in server/nosql/dispatcher.go.
+ if err := call.SendStream().Send(naming.GlobChildrenReplyName{Value: pubutil.Escape(name)}); err != nil {
+ return err
+ }
+ }
+ }
+ if err := it.Err(); err != nil {
+ call.SendStream().Send(naming.GlobChildrenReplyError{Value: naming.GlobError{Error: err}})
+ }
+ return nil
+}
diff --git a/services/syncbase/server/util/key_util.go b/services/syncbase/server/util/key_util.go
index c0d79ad..f00dfef 100644
--- a/services/syncbase/server/util/key_util.go
+++ b/services/syncbase/server/util/key_util.go
@@ -13,13 +13,11 @@
)
// JoinKeyParts builds keys for accessing data in the storage engine.
-// TODO(sadovsky): Allow ":" in names and use a different separator here.
func JoinKeyParts(parts ...string) string {
return strings.Join(parts, KeyPartSep)
}
// SplitKeyParts is the inverse of JoinKeyParts.
-// TODO(sadovsky): Allow ":" in names and use a different separator here.
func SplitKeyParts(key string) []string {
return strings.Split(key, KeyPartSep)
}
diff --git a/services/syncbase/server/util/list_children.go b/services/syncbase/server/util/list_children.go
deleted file mode 100644
index eaa68bd..0000000
--- a/services/syncbase/server/util/list_children.go
+++ /dev/null
@@ -1,29 +0,0 @@
-// Copyright 2015 The Vanadium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package util
-
-import (
- "v.io/v23/context"
- "v.io/v23/rpc"
- "v.io/x/ref/services/syncbase/store"
-)
-
-// ListChildren returns the names of all apps, databases, or tables with the
-// given key prefix. Designed for use by Service.ListApps, App.ListDatabases,
-// and Database.ListTables.
-func ListChildren(ctx *context.T, call rpc.ServerCall, sntx store.SnapshotOrTransaction, stKeyPrefix string) ([]string, error) {
- it := sntx.Scan(ScanPrefixArgs(stKeyPrefix, ""))
- key := []byte{}
- res := []string{}
- for it.Advance() {
- key = it.Key(key)
- parts := SplitKeyParts(string(key))
- res = append(res, parts[len(parts)-1])
- }
- if err := it.Err(); err != nil {
- return nil, err
- }
- return res, nil
-}
diff --git a/services/syncbase/testutil/constants.go b/services/syncbase/testutil/constants.go
new file mode 100644
index 0000000..3f65999
--- /dev/null
+++ b/services/syncbase/testutil/constants.go
@@ -0,0 +1,40 @@
+// Copyright 2015 The Vanadium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package testutil
+
+var invalidIdentifiers []string = []string{
+ "/",
+ "a/b",
+ "*",
+ "@@",
+ "dev.v.io/a/admin@myapp.com",
+ "안녕하세요",
+}
+
+var OkDbTableNames []string = []string{
+ "a",
+ "B",
+ "a_",
+ "a__",
+ "a0_",
+ "a_b",
+ "a_0",
+ "foobar",
+ "BARBAZ",
+}
+
+var NotOkAppRowNames []string = []string{
+ "",
+ ":",
+ "\x00",
+ "\xff",
+ "a:b",
+ "a\x00b",
+ "a\xffb",
+}
+
+var OkAppRowNames []string = append(OkDbTableNames, invalidIdentifiers...)
+
+var NotOkDbTableNames []string = append(NotOkAppRowNames, invalidIdentifiers...)
diff --git a/services/syncbase/testutil/layer.go b/services/syncbase/testutil/layer.go
index b490885..755aae9 100644
--- a/services/syncbase/testutil/layer.go
+++ b/services/syncbase/testutil/layer.go
@@ -35,7 +35,7 @@
assertExists(t, ctx, self, "self", false)
// TODO(ivanpi): Exists on child when parent does not exist currently fails
// with an error instead of returning false.
- //assertExists(t, ctx, child, "child", false)
+ // assertExists(t, ctx, child, "child", false)
// Create self.
if err := self.Create(ctx, nil); err != nil {
@@ -92,31 +92,29 @@
assertExists(t, ctx, self3, "self3", false)
}
-// Tests that non-ASCII UTF-8 chars are supported at all layers as long as they
-// satisfy util.ValidName.
-// This test only requires layer.Create to be implemented and thus works for
-// rows.
-func TestCreateNameValidation(t *testing.T, ctx *context.T, i interface{}) {
+// Tests that Create() checks name validity.
+func TestCreateNameValidation(t *testing.T, ctx *context.T, i interface{}, okNames, notOkNames []string) {
parent := makeLayer(i)
-
- // Invalid names.
- // TODO(sadovsky): Add names with slashes to this list once we implement
- // client-side name validation. As it stands, some names with slashes result
- // in RPCs against objects at the next layer of hierarchy, and naming.Join
- // drops some leading and trailing slashes before they reach the server-side
- // name-checking code.
- for _, name := range []string{"a\x00", "\x00a", "@@", "a@@", "@@a", "@@a", "$", "a/$", "$/a"} {
- if err := parent.Child(name).Create(ctx, nil); verror.ErrorID(err) != wire.ErrInvalidName.ID {
- t.Fatalf("Create(%q) should have failed: %v", name, err)
- }
- }
-
- // Valid names.
- for _, name := range []string{"a", "aa", "*", "a*", "*a", "a*b", "a/b", "a/$$", "$$/a", "a/$$/b", "dev.v.io/a/admin@myapp.com", "alice/bob", "안녕하세요"} {
+ for _, name := range okNames {
if err := parent.Child(name).Create(ctx, nil); err != nil {
t.Fatalf("Create(%q) failed: %v", name, err)
}
}
+ for _, name := range notOkNames {
+ if err := parent.Child(name).Create(ctx, nil); err == nil {
+ t.Fatalf("Create(%q) should have failed: %v", name, err)
+ // TODO(sadovsky): Currently for name "" we cannot check for
+ // ErrInvalidName, since parent.Child("") dispatches on the parent
+ // component. Create will still fail, but for a different reason. If/when
+ // we add client-side name validation, we'll be able to check for
+ // ErrInvalidName specifically.
+ // TODO(sadovsky): Will dropping the "TrimPrefix" calls in our server-side
+ // dispatchers fix this?
+ if name != "" && verror.ErrorID(err) != wire.ErrInvalidName.ID {
+ t.Fatalf("Create(%q) should have failed with ErrInvalidName: %v", name, err)
+ }
+ }
+ }
}
// TestDestroy tests that object destruction works as expected.
@@ -155,7 +153,7 @@
assertExists(t, ctx, self, "self", false)
// TODO(ivanpi): Exists on child when parent does not exist currently fails
// with an error instead of returning false.
- //assertExists(t, ctx, child, "child", false)
+ // assertExists(t, ctx, child, "child", false)
// self.Create should succeed, since self was destroyed.
if err := self.Create(ctx, nil); err != nil {
@@ -208,11 +206,11 @@
var err error
got, err = self.ListChildren(ctx)
- want = []string(nil)
+ want = []string{}
if err != nil {
t.Fatalf("self.ListChildren() failed: %v", err)
}
- if !reflect.DeepEqual(got, want) {
+ if !reflect.DeepEqual([]string(got), want) {
t.Fatalf("Lists do not match: got %v, want %v", got, want)
}
@@ -239,19 +237,6 @@
if !reflect.DeepEqual(got, want) {
t.Fatalf("Lists do not match: got %v, want %v", got, want)
}
-
- hello := "안녕하세요"
- if err := self.Child(hello).Create(ctx, nil); err != nil {
- t.Fatalf("hello.Create() failed: %v", err)
- }
- got, err = self.ListChildren(ctx)
- want = []string{"x", "y", hello}
- if err != nil {
- t.Fatalf("self.ListChildren() failed: %v", err)
- }
- if !reflect.DeepEqual(got, want) {
- t.Fatalf("Lists do not match: got %v, want %v", got, want)
- }
}
// TestPerms tests that {Set,Get}Permissions work as expected.
