From af074592d58323bec7c6fa5d7d0a385461e31396 Mon Sep 17 00:00:00 2001 From: Benjamin Bengfort Date: Thu, 9 Dec 2021 20:34:43 -0600 Subject: [PATCH] Better Namespace Handling (#12) Ensures default namespace is used when empty string is provided via the options and also ensures that the iterator provides a seamless Seek experience when iterating across a namespace. --- engines/leveldb/iter.go | 26 ++++++++++++++++++-------- engines/leveldb/leveldb.go | 10 +++++----- iterator/empty.go | 18 +++++++++++++----- iterator/empty_test.go | 7 +++++-- iterator/iterator.go | 3 +++ options/options.go | 9 ++++++--- options/options_test.go | 7 ++++++- 7 files changed, 56 insertions(+), 24 deletions(-) diff --git a/engines/leveldb/iter.go b/engines/leveldb/iter.go index bb0ed7c..5f89865 100644 --- a/engines/leveldb/iter.go +++ b/engines/leveldb/iter.go @@ -11,23 +11,29 @@ import ( // NewLevelDBIterator creates a new iterator that wraps a leveldb Iterator with object // management access and Honu-specific serialization. -func NewLevelDBIterator(iter iterator.Iterator) honuiter.Iterator { - return &ldbIterator{ldb: iter} +func NewLevelDBIterator(iter iterator.Iterator, namespace string) honuiter.Iterator { + return &ldbIterator{ldb: iter, namespace: namespace} } // Wraps the underlying leveldb iterator to provide object management access. type ldbIterator struct { - ldb iterator.Iterator + ldb iterator.Iterator + namespace string } // Type check for the ldbIterator var _ honuiter.Iterator = &ldbIterator{} -func (i *ldbIterator) Next() bool { return i.ldb.Next() } -func (i *ldbIterator) Prev() bool { return i.ldb.Prev() } -func (i *ldbIterator) Seek(key []byte) bool { return i.ldb.Seek(key) } -func (i *ldbIterator) Error() error { return i.ldb.Error() } -func (i *ldbIterator) Release() { i.ldb.Release() } +func (i *ldbIterator) Next() bool { return i.ldb.Next() } +func (i *ldbIterator) Prev() bool { return i.ldb.Prev() } +func (i *ldbIterator) Error() error { return i.ldb.Error() } +func (i *ldbIterator) Release() { i.ldb.Release() } + +func (i *ldbIterator) Seek(key []byte) bool { + // We need to prefix the seek with the correct namespace + key = prepend(i.namespace, key) + return i.ldb.Seek(key) +} func (i *ldbIterator) Key() []byte { // Fetch the key then split the namespace from the key @@ -54,3 +60,7 @@ func (i *ldbIterator) Object() (obj *pb.Object, err error) { } return obj, nil } + +func (i *ldbIterator) Namespace() string { + return i.namespace +} diff --git a/engines/leveldb/leveldb.go b/engines/leveldb/leveldb.go index 59583bb..7a1b9ed 100644 --- a/engines/leveldb/leveldb.go +++ b/engines/leveldb/leveldb.go @@ -83,7 +83,7 @@ func (db *LevelDBEngine) Get(key []byte, options *opts.Options) (value []byte, e // Thread-unsafe get that is called both by the Transaction and the Store. func (db *LevelDBEngine) get(key []byte, options *opts.Options) (value []byte, err error) { - //Create a default to prevent SigFaults when accessing options. + //Create a default to prevent panics when accessing options. if options == nil { if options, err = opts.New(); err != nil { return nil, err @@ -121,7 +121,7 @@ func (db *LevelDBEngine) Put(key, value []byte, options *opts.Options) (err erro // Thread-unsafe put that is called both by the Transaction and the Store. func (db *LevelDBEngine) put(key, value []byte, options *opts.Options) (err error) { - //Create a default to prevent SigFaults when accessing options. + //Create a default to prevent panics when accessing options. if options == nil { if options, err = opts.New(); err != nil { return err @@ -158,7 +158,7 @@ func (db *LevelDBEngine) Delete(key []byte, options *opts.Options) (err error) { // Thread-unsafe delete that is called both by the Transaction and the Store. func (db *LevelDBEngine) delete(key []byte, options *opts.Options) (err error) { - //Create a default to prevent SigFaults when accessing options. + //Create a default to prevent panics when accessing options. if options == nil { if options, err = opts.New(); err != nil { return err @@ -172,7 +172,7 @@ func (db *LevelDBEngine) delete(key []byte, options *opts.Options) (err error) { } func (db *LevelDBEngine) Iter(prefix []byte, options *opts.Options) (i iterator.Iterator, err error) { - //Create a default to prevent SigFaults when accessing options. + //Create a default to prevent panic when accessing options. if options == nil { if options, err = opts.New(); err != nil { return nil, err @@ -186,7 +186,7 @@ func (db *LevelDBEngine) Iter(prefix []byte, options *opts.Options) (i iterator. if len(prefix) > 0 { slice = util.BytesPrefix(prefix) } - return NewLevelDBIterator(db.ldb.NewIterator(slice, nil)), nil + return NewLevelDBIterator(db.ldb.NewIterator(slice, nil), options.Namespace), nil } var nssep = []byte("::") diff --git a/iterator/empty.go b/iterator/empty.go index 70025e3..32b5bc7 100644 --- a/iterator/empty.go +++ b/iterator/empty.go @@ -1,16 +1,23 @@ package iterator -import pb "github.com/rotationalio/honu/object" +import ( + pb "github.com/rotationalio/honu/object" + "github.com/rotationalio/honu/options" +) // NewEmptyIterator creates an empty iterator that returns nothing. The err parameter // can be nil, but if not nil the given err will be returned by the Error method. -func NewEmptyIterator(err error) Iterator { - return &emptyIterator{err: err} +func NewEmptyIterator(err error, namespace string) Iterator { + if namespace == "" { + namespace = options.NamespaceDefault + } + return &emptyIterator{err: err, namespace: namespace} } type emptyIterator struct { - released bool - err error + released bool + err error + namespace string } var _ Iterator = &emptyIterator{} @@ -29,3 +36,4 @@ func (*emptyIterator) Value() []byte { return nil } func (*emptyIterator) Object() (*pb.Object, error) { return nil, nil } func (i *emptyIterator) Error() error { return i.err } func (i *emptyIterator) Release() { i.released = true } +func (i *emptyIterator) Namespace() string { return i.namespace } diff --git a/iterator/empty_test.go b/iterator/empty_test.go index 82b8e1d..a4d9249 100644 --- a/iterator/empty_test.go +++ b/iterator/empty_test.go @@ -5,18 +5,20 @@ import ( "testing" . "github.com/rotationalio/honu/iterator" + "github.com/rotationalio/honu/options" "github.com/stretchr/testify/require" ) func TestEmptyIterator(t *testing.T) { // Check that the empty iterator returns expected values - iter := NewEmptyIterator(nil) + iter := NewEmptyIterator(nil, "") require.False(t, iter.Next()) require.False(t, iter.Prev()) require.False(t, iter.Seek([]byte("foo"))) require.Nil(t, iter.Key()) require.Nil(t, iter.Value()) require.NoError(t, iter.Error()) + require.Equal(t, options.NamespaceDefault, iter.Namespace()) obj, err := iter.Object() require.NoError(t, err) @@ -31,7 +33,7 @@ func TestEmptyIterator(t *testing.T) { require.EqualError(t, iter.Error(), ErrIterReleased.Error()) // Check that the empty iterator can be initialized with an error - iter = NewEmptyIterator(errors.New("something bad happened")) + iter = NewEmptyIterator(errors.New("something bad happened"), "foo") require.EqualError(t, iter.Error(), "something bad happened") // Ensure that calling any of the iterator methods do not change the error @@ -40,6 +42,7 @@ func TestEmptyIterator(t *testing.T) { require.False(t, iter.Seek([]byte("foo"))) require.Nil(t, iter.Key()) require.Nil(t, iter.Value()) + require.Equal(t, "foo", iter.Namespace()) obj, err = iter.Object() require.NoError(t, err) diff --git a/iterator/iterator.go b/iterator/iterator.go index 0c6e346..d9ca2d4 100644 --- a/iterator/iterator.go +++ b/iterator/iterator.go @@ -60,4 +60,7 @@ type Iterator interface { // Seek moves the iterator to the first key/value pair whose key is greater than or // equal to the given key. It returns whether such pair exists. Seek(key []byte) bool + + // Namespace returns the current namespace the iterator is operating on. + Namespace() string } diff --git a/options/options.go b/options/options.go index 0ad2c93..7230cdc 100644 --- a/options/options.go +++ b/options/options.go @@ -6,13 +6,13 @@ import ( ) const ( - defaultNamespace = "default" + NamespaceDefault = "default" ) // New creates a per-call Options object based on the variadic SetOptions closures // supplied by the user. New also sets sensible defaults for various options. func New(options ...SetOptions) (cfg *Options, err error) { - cfg = &Options{Namespace: defaultNamespace} + cfg = &Options{Namespace: NamespaceDefault} for _, option := range options { if err = option(cfg); err != nil { return nil, err @@ -37,7 +37,10 @@ type SetOptions func(cfg *Options) error // WithNamespace returns a closure that sets a namespace other than the default. func WithNamespace(namespace string) SetOptions { return func(cfg *Options) error { - cfg.Namespace = namespace + // If namespace is empty, keep default namespace + if namespace != "" { + cfg.Namespace = namespace + } return nil } } diff --git a/options/options_test.go b/options/options_test.go index 4cf8404..9ca4f40 100644 --- a/options/options_test.go +++ b/options/options_test.go @@ -13,7 +13,7 @@ func TestHonuOptions(t *testing.T) { // Test default options opts, err := options.New() require.NoError(t, err, "could not create options") - require.Equal(t, "default", opts.Namespace) + require.Equal(t, options.NamespaceDefault, opts.Namespace) require.False(t, opts.Destroy) // Test setting multiple options @@ -21,6 +21,11 @@ func TestHonuOptions(t *testing.T) { require.NoError(t, err, "could not create options") require.Equal(t, "foo", opts.Namespace) require.True(t, opts.Destroy) + + // Ensuring setting empty string namespace still ends up as the default namespace + opts, err = options.New(options.WithNamespace("")) + require.NoError(t, err, "could not create options with empty string namespace") + require.Equal(t, options.NamespaceDefault, opts.Namespace) } func TestLevelDBReadOptions(t *testing.T) {