address review comments

Author: zhangchiqing

storage/operation/badgerimpl/iterator.go

@@ -37,8 +37,9 @@ func newBadgerIterator(db *badger.DB, startPrefix, endPrefix []byte, ops storage
 }
 
 // First seeks to the smallest key greater than or equal to the given key.
-func (i *badgerIterator) First() {
+func (i *badgerIterator) First() bool {
 	i.iter.Seek(i.lowerBound)
+	return i.Valid()
 }
 
 // Valid returns whether the iterator is positioned at a valid key-value pair.

storage/operation/pebbleimpl/iterator.go

@@ -9,7 +9,7 @@ import (
 )
 
 type pebbleIterator struct {
-	iter *pebble.Iterator
+	*pebble.Iterator
 }
 
 var _ storage.Iterator = (*pebbleIterator)(nil)
@@ -33,56 +33,36 @@ func newPebbleIterator(reader pebble.Reader, startPrefix, endPrefix []byte, ops
 	}
 
 	return &pebbleIterator{
-		iter: iter,
+		iter,
 	}, nil
 }
 
-// First seeks to the smallest key greater than or equal to the given key.
-func (i *pebbleIterator) First() {
-	i.iter.First()
-}
-
-// Valid returns whether the iterator is positioned at a valid key-value pair.
-func (i *pebbleIterator) Valid() bool {
-	return i.iter.Valid()
+// IterItem returns the current key-value pair, or nil if done.
+func (i *pebbleIterator) IterItem() storage.IterItem {
+	return pebbleIterItem{i.Iterator}
 }
 
-// Next advances the iterator to the next key-value pair.
+// Next seeks to the smallest key greater than or equal to the given key.
 func (i *pebbleIterator) Next() {
-	i.iter.Next()
-}
-
-// IterItem returns the current key-value pair, or nil if done.
-func (i *pebbleIterator) IterItem() storage.IterItem {
-	return pebbleIterItem{iter: i.iter}
+	i.Iterator.Next()
 }
 
 type pebbleIterItem struct {
-	iter *pebble.Iterator
+	*pebble.Iterator
 }
 
 var _ storage.IterItem = (*pebbleIterItem)(nil)
 
-func (i pebbleIterItem) Key() []byte {
-	return i.iter.Key()
-}
-
 // KeyCopy returns a copy of the key of the item, writing it to dst slice.
 func (i pebbleIterItem) KeyCopy(dst []byte) []byte {
-	return append(dst[:0], i.iter.Key()...)
+	return append(dst[:0], i.Key()...)
 }
 
 func (i pebbleIterItem) Value(fn func([]byte) error) error {
-	val, err := i.iter.ValueAndErr()
+	val, err := i.ValueAndErr()
 	if err != nil {
 		return err
 	}
 
 	return fn(val)
 }
-
-// Close closes the iterator. Iterator must be closed, otherwise it causes memory leak.
-// No errors expected during normal operation
-func (i *pebbleIterator) Close() error {
-	return i.iter.Close()
-}

storage/operation/reads.go

@@ -45,7 +45,7 @@ func IterateKeysByPrefixRange(r storage.Reader, startPrefix []byte, endPrefix []
 			}
 			return false, nil
 		}, nil, nil
-	}, storage.IteratorOption{IterateKeyOnly: true})
+	}, storage.IteratorOption{BadgerIterateKeyOnly: true})
 }
 
 // IterateKey will iterate over all entries in the database, where the key starts with a prefixes in

storage/operation/reads_test.go

@@ -12,6 +12,43 @@ import (
 	"github.com/onflow/flow-go/storage/operation/dbtest"
 )
 
+func TestFirst(t *testing.T) {
+	dbtest.RunWithStorages(t, func(t *testing.T, r storage.Reader, withWriter dbtest.WithWriter) {
+
+		// Create a range of keys around the prefix start/end values
+		keys := [][]byte{
+			{0x10, 0x00},
+			{0x10, 0xff},
+		}
+
+		// Insert the keys into the storage
+		require.NoError(t, withWriter(func(writer storage.Writer) error {
+			for _, key := range keys {
+				value := []byte{0x00} // value are skipped, doesn't matter
+				err := operation.Upsert(key, value)(writer)
+				if err != nil {
+					return err
+				}
+			}
+			return nil
+		}))
+
+		iter, err := r.NewIter([]byte{0x20}, []byte{0x30}, storage.DefaultIteratorOptions())
+		require.NoError(t, err)
+
+		// Check that the iterator is at the first key and return false when matching no key
+		require.False(t, iter.First())
+		require.NoError(t, iter.Close())
+
+		iter, err = r.NewIter([]byte{0x10}, []byte{0x30}, storage.DefaultIteratorOptions())
+		require.NoError(t, err)
+
+		// Check that the iterator is at the first key and return true when matching the first key
+		require.True(t, iter.First())
+		require.NoError(t, iter.Close())
+	})
+}
+
 func TestIterateKeysInPrefixRange(t *testing.T) {
 	dbtest.RunWithStorages(t, func(t *testing.T, r storage.Reader, withWriter dbtest.WithWriter) {
 		// Define the prefix range

storage/operations.go

@@ -17,7 +17,9 @@ type Iterator interface {
 	// This method must be called because it's necessary for the badger implementation
 	// to move the iteration cursor to the first key in the iteration range.
 	// This method must be called before calling Valid, Next, IterItem, or Close.
-	First()
+	// return true if the iterator is pointing to a valid key-value pair after calling First,
+	// return false otherwise.
+	First() bool
 
 	// Valid returns whether the iterator is positioned at a valid key-value pair.
 	// If Valid returns false, the iterator is done and must be closed.