Validate Honu Iterator behavior matches LevelDB (#22)

Co-authored-by: Patrick Deziel <[email protected]>

engines/leveldb/iter.go

@@ -59,12 +59,23 @@ func (i *ldbIterator) Prev() bool {
 }
 
 func (i *ldbIterator) Seek(key []byte) bool {
-	// NOTE: no need to do tombstone checking in Seek because Next will be called.
 	// We need to prefix the seek with the correct namespace
 	if i.options.Namespace != "" {
 		key = prepend(i.options.Namespace, key)
 	}
-	return i.ldb.Seek(key)
+
+	if ok := i.ldb.Seek(key); !ok {
+		return false
+	}
+
+	// If we aren't including Tombstones, we need to check if the check if the current
+	// version is a tombstone, and if not, continue to the next non-tombstone object
+	if !i.options.Tombstones {
+		if obj, err := i.Object(); err != nil || obj.Tombstone() {
+			return i.Next()
+		}
+	}
+	return true
 }
 
 func (i *ldbIterator) Key() []byte {

engines/leveldb/iter_test.go

@@ -0,0 +1,109 @@
+package leveldb_test
+
+import (
+	"bytes"
+	"crypto/rand"
+	"fmt"
+	"testing"
+
+	pb "github.com/rotationalio/honu/object"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/protobuf/proto"
+)
+
+// This test verifies LevelDB functionality to ensure that our iterator functionality
+// matches the expected iterator API for leveldb.
+func TestLevelDBFunctionality(t *testing.T) {
+	// Setup a levelDB Engine and create a bunch of keys.
+	engine, _ := setupLevelDBEngine(t)
+	ldb := engine.DB()
+
+	keys := make([][]byte, 0, 100)
+	for i := 0; i < 100; i++ {
+		key := []byte(fmt.Sprintf("%04d", i))
+		keys = append(keys, key)
+		require.NoError(t, ldb.Put(key, randomData(192), nil), "could not put fixture data")
+	}
+
+	// Create an iterator to test Seek/Prev/Next functionality
+	iter := ldb.NewIterator(nil, nil)
+
+	// When next has not been called, what does Prev do?
+	require.False(t, iter.Prev(), "if Prev is called before Next, we expect it to return false")
+
+	// If we seek to the first key, the value of the key should be the first key
+	// If we call Prev, we should get false, because the Seek was to the first key but
+	// what is the value of the cursor, do we have to call Next again to get back to
+	// the first key?
+	require.True(t, iter.Seek(keys[0]), "we should be able to seek to the first key")
+	require.True(t, bytes.Equal(iter.Key(), keys[0]), "the cursor is now at the first key")
+	require.False(t, iter.Prev(), "if we're at the first key, prev should be false")
+	require.Nil(t, iter.Key(), "call Prev moved us behind the first key - so it should now be nil")
+	require.True(t, iter.Next(), "we should now be able to move back to the first key")
+	require.True(t, bytes.Equal(iter.Key(), keys[0]), "the cursor is now at the first key")
+}
+
+// Test Honu seek behavior matches LevelDB  API
+func TestHonuSeek(t *testing.T) {
+	// Setup a levelDB Engine and create a bunch of keys.
+	db, _ := setupLevelDBEngine(t)
+
+	keys := make([][]byte, 0, 100)
+	for i := 0; i < 100; i++ {
+		key := []byte(fmt.Sprintf("%04d", i))
+		keys = append(keys, key)
+		require.NoError(t, db.Put(key, randomObject(key, 192), nil), "could not put fixture data")
+	}
+
+	// Create an iterator to test Seek/Prev/Next functionality
+	iter, err := db.Iter(nil, nil)
+	require.NoError(t, err, "could not create honu leveldb iterator")
+
+	// When next has not been called, what does Prev do?
+	require.False(t, iter.Prev(), "if Prev is called before Next, we expect it to return false")
+
+	// If we seek to the first key, the value of the key should be the first key
+	// If we call Prev, we should get false, because the Seek was to the first key but
+	// what is the value of the cursor, do we have to call Next again to get back to
+	// the first key?
+	require.True(t, iter.Seek(keys[0]), "we should be able to seek to the first key")
+	require.True(t, bytes.Equal(iter.Key(), keys[0]), "the cursor is now at the first key")
+	require.False(t, iter.Prev(), "if we're at the first key, prev should be false")
+	require.Nil(t, iter.Key(), "call Prev moved us behind the first key - so it should now be nil")
+	require.True(t, iter.Next(), "we should now be able to move back to the first key")
+	require.True(t, bytes.Equal(iter.Key(), keys[0]), "the cursor is now at the first key")
+}
+
+// Helper function to generate random data
+func randomData(len int) []byte {
+	data := make([]byte, len)
+	if _, err := rand.Read(data); err != nil {
+		panic(err)
+	}
+	return data
+}
+
+// Helper function to generate a random object data
+func randomObject(key []byte, len int) []byte {
+	obj := &pb.Object{
+		Key:       key,
+		Namespace: "default",
+		Version: &pb.Version{
+			Pid:       1,
+			Version:   1,
+			Region:    "testing",
+			Parent:    nil,
+			Tombstone: false,
+		},
+		Region: "testing",
+		Owner:  "testing",
+		Data:   randomData(len),
+	}
+
+	// Marshal the object
+	data, err := proto.Marshal(obj)
+	if err != nil {
+		panic(err)
+	}
+	return data
+}

honu_test.go

@@ -304,7 +304,7 @@ func TestTombstones(t *testing.T) {
 	// Create a list of keys with integer values
 	keys := make([][]byte, 0, 20)
 	for i := 0; i < 20; i++ {
-		key := []byte(fmt.Sprintf("%00X", i+1))
+		key := []byte(fmt.Sprintf("%04d", i))
 		keys = append(keys, key)
 	}
 
@@ -397,6 +397,37 @@ func TestTombstones(t *testing.T) {
 			require.False(t, obj.Tombstone())
 		}
 	}
+
+	// Test Seek, Next, and Prev with and without Tombstones
+	iter, err := db.Iter(nil, options.WithNamespace("graveyard"))
+	require.NoError(t, err, "could not create honu iterator")
+
+	itert, err := db.Iter(nil, options.WithNamespace("graveyard"), options.WithTombstones())
+	require.NoError(t, err, "could not create honu tombstone iterator")
+
+	// Seek to a non-tombstone key
+	require.True(t, iter.Seek(keys[9]), "could not seek to a non-tombstone key")
+	require.True(t, itert.Seek(keys[9]), "could not seek to a non-tombstone key with tombstone iterator")
+	require.True(t, bytes.Equal(iter.Key(), keys[9]), "unexpected key at iter cursor")
+	require.True(t, bytes.Equal(itert.Key(), keys[9]), "unexpected key at iter cursor with tombstone iterator")
+
+	// Seek to a tombstone key (move to 15 and 14 respectively)
+	require.True(t, iter.Seek(keys[14]), "could not seek to a tombstone key")
+	require.True(t, itert.Seek(keys[14]), "could not seek to a tombstone key with tombstone iterator")
+	require.True(t, bytes.Equal(iter.Key(), keys[15]), "unexpected key at iter cursor")
+	require.True(t, bytes.Equal(itert.Key(), keys[14]), "unexpected key at iter cursor with tombstone iterator")
+
+	// Prev should move us to keys[13] for both two iterators
+	require.True(t, iter.Prev(), "could not prev to a non-tombstone key")
+	require.True(t, itert.Prev(), "could not prev to a non-tombstone key with tombstone iterator")
+	require.True(t, bytes.Equal(iter.Key(), keys[13]), "unexpected key at iter cursor")
+	require.True(t, bytes.Equal(itert.Key(), keys[13]), "unexpected key at iter cursor with tombstone iterator")
+
+	// Next should move us back to 15 and 14 respectively
+	require.True(t, iter.Next(), "could not next to a non-tombstone key")
+	require.True(t, itert.Next(), "could not next to a tombstone key with tombstone iterator")
+	require.True(t, bytes.Equal(iter.Key(), keys[15]), "unexpected key at iter cursor")
+	require.True(t, bytes.Equal(itert.Key(), keys[14]), "unexpected key at iter cursor with tombstone iterator")
 }
 
 func TestTombstonesMultipleNamespaces(t *testing.T) {
@@ -413,7 +444,7 @@ func TestTombstonesMultipleNamespaces(t *testing.T) {
 	// Create a list of keys with integer values
 	keys := make([][]byte, 0, 100)
 	for i := 0; i < 100; i++ {
-		key := []byte(fmt.Sprintf("%00X", i+1))
+		key := []byte(fmt.Sprintf("%04d", i))
 		keys = append(keys, key)
 	}