From 7f650cea8673ee3169e51a41c1a8038e38c59ef4 Mon Sep 17 00:00:00 2001 From: Jae Kwon Date: Tue, 12 Dec 2017 14:45:31 -0800 Subject: [PATCH] Remove Prev from Iterator --- db/c_level_db.go | 7 ---- db/cache_db_test.go | 83 --------------------------------------------- db/common_test.go | 51 ++++------------------------ db/mem_db_test.go | 2 +- db/types.go | 13 ++++--- db/util_test.go | 82 +------------------------------------------- 6 files changed, 15 insertions(+), 223 deletions(-) delete mode 100644 db/cache_db_test.go diff --git a/db/c_level_db.go b/db/c_level_db.go index e867b0004..11a6e5ff7 100644 --- a/db/c_level_db.go +++ b/db/c_level_db.go @@ -202,13 +202,6 @@ func (c cLevelDBIterator) Next() { c.itr.Next() } -func (c cLevelDBIterator) Prev() { - if !c.itr.Valid() { - panic("cLevelDBIterator Prev() called when invalid") - } - c.itr.Prev() -} - func (c cLevelDBIterator) Release() { c.itr.Close() } diff --git a/db/cache_db_test.go b/db/cache_db_test.go deleted file mode 100644 index 2a2684fe2..000000000 --- a/db/cache_db_test.go +++ /dev/null @@ -1,83 +0,0 @@ -package db - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func bz(s string) []byte { return []byte(s) } - -func TestCacheDB(t *testing.T) { - mem := NewMemDB() - cdb := mem.CacheDB() - - require.Empty(t, cdb.Get(bz("key1")), "Expected `key1` to be empty") - - mem.Set(bz("key1"), bz("value1")) - cdb.Set(bz("key1"), bz("value1")) - require.Equal(t, bz("value1"), cdb.Get(bz("key1"))) - - cdb.Set(bz("key1"), bz("value2")) - require.Equal(t, bz("value2"), cdb.Get(bz("key1"))) - require.Equal(t, bz("value1"), mem.Get(bz("key1"))) - - cdb.Write() - require.Equal(t, bz("value2"), mem.Get(bz("key1"))) - - require.Panics(t, func() { cdb.Write() }, "Expected second cdb.Write() to fail") - - cdb = mem.CacheDB() - cdb.Delete(bz("key1")) - require.Empty(t, cdb.Get(bz("key1"))) - require.Equal(t, mem.Get(bz("key1")), bz("value2")) - - cdb.Write() - require.Empty(t, cdb.Get(bz("key1")), "Expected `key1` to be empty") - require.Empty(t, mem.Get(bz("key1")), "Expected `key1` to be empty") -} - -func TestCacheDBWriteLock(t *testing.T) { - mem := NewMemDB() - cdb := mem.CacheDB() - require.NotPanics(t, func() { cdb.Write() }) - require.Panics(t, func() { cdb.Write() }) - cdb = mem.CacheDB() - require.NotPanics(t, func() { cdb.Write() }) - require.Panics(t, func() { cdb.Write() }) -} - -func TestCacheDBWriteLockNested(t *testing.T) { - mem := NewMemDB() - cdb := mem.CacheDB() - cdb2 := cdb.CacheDB() - require.NotPanics(t, func() { cdb2.Write() }) - require.Panics(t, func() { cdb2.Write() }) - cdb2 = cdb.CacheDB() - require.NotPanics(t, func() { cdb2.Write() }) - require.Panics(t, func() { cdb2.Write() }) -} - -func TestCacheDBNested(t *testing.T) { - mem := NewMemDB() - cdb := mem.CacheDB() - cdb.Set(bz("key1"), bz("value1")) - - require.Empty(t, mem.Get(bz("key1"))) - require.Equal(t, bz("value1"), cdb.Get(bz("key1"))) - cdb2 := cdb.CacheDB() - require.Equal(t, bz("value1"), cdb2.Get(bz("key1"))) - - cdb2.Set(bz("key1"), bz("VALUE2")) - require.Equal(t, []byte(nil), mem.Get(bz("key1"))) - require.Equal(t, bz("value1"), cdb.Get(bz("key1"))) - require.Equal(t, bz("VALUE2"), cdb2.Get(bz("key1"))) - - cdb2.Write() - require.Equal(t, []byte(nil), mem.Get(bz("key1"))) - require.Equal(t, bz("VALUE2"), cdb.Get(bz("key1"))) - - cdb.Write() - require.Equal(t, bz("VALUE2"), mem.Get(bz("key1"))) - -} diff --git a/db/common_test.go b/db/common_test.go index 505864c20..09fad8424 100644 --- a/db/common_test.go +++ b/db/common_test.go @@ -23,16 +23,6 @@ func checkNextPanics(t *testing.T, itr Iterator) { assert.Panics(t, func() { itr.Next() }, "checkNextPanics expected panic but didn't") } -func checkPrevPanics(t *testing.T, itr Iterator) { - assert.Panics(t, func() { itr.Prev() }, "checkPrevPanics expected panic but didn't") -} - -func checkPrev(t *testing.T, itr Iterator, expected bool) { - itr.Prev() - valid := itr.Valid() - assert.Equal(t, expected, valid) -} - func checkItem(t *testing.T, itr Iterator, key []byte, value []byte) { k, v := itr.Key(), itr.Value() assert.Exactly(t, key, k) @@ -44,7 +34,6 @@ func checkInvalid(t *testing.T, itr Iterator) { checkKeyPanics(t, itr) checkValuePanics(t, itr) checkNextPanics(t, itr) - checkPrevPanics(t, itr) } func checkKeyPanics(t *testing.T, itr Iterator) { @@ -67,7 +56,7 @@ func TestDBIteratorSingleKey(t *testing.T) { t.Run(fmt.Sprintf("Backend %s", backend), func(t *testing.T) { db := newTempDB(t, backend) db.SetSync(bz("1"), bz("value_1")) - itr := db.Iterator() + itr := db.Iterator(BeginningKey(), EndingKey()) checkValid(t, itr, true) checkNext(t, itr, false) @@ -88,15 +77,12 @@ func TestDBIteratorTwoKeys(t *testing.T) { db.SetSync(bz("2"), bz("value_1")) { // Fail by calling Next too much - itr := db.Iterator() + itr := db.Iterator(BeginningKey(), EndingKey()) checkValid(t, itr, true) for i := 0; i < 10; i++ { checkNext(t, itr, true) checkValid(t, itr, true) - - checkPrev(t, itr, true) - checkValid(t, itr, true) } checkNext(t, itr, true) @@ -110,27 +96,6 @@ func TestDBIteratorTwoKeys(t *testing.T) { // Once invalid... checkInvalid(t, itr) } - - { // Fail by calling Prev too much - itr := db.Iterator() - checkValid(t, itr, true) - - for i := 0; i < 10; i++ { - checkNext(t, itr, true) - checkValid(t, itr, true) - - checkPrev(t, itr, true) - checkValid(t, itr, true) - } - - checkPrev(t, itr, false) - checkValid(t, itr, false) - - checkPrevPanics(t, itr) - - // Once invalid... - checkInvalid(t, itr) - } }) } } @@ -139,32 +104,30 @@ func TestDBIteratorEmpty(t *testing.T) { for backend, _ := range backends { t.Run(fmt.Sprintf("Backend %s", backend), func(t *testing.T) { db := newTempDB(t, backend) - itr := db.Iterator() + itr := db.Iterator(BeginningKey(), EndingKey()) checkInvalid(t, itr) }) } } -func TestDBIteratorEmptySeek(t *testing.T) { +func TestDBIteratorEmptyBeginAfter(t *testing.T) { for backend, _ := range backends { t.Run(fmt.Sprintf("Backend %s", backend), func(t *testing.T) { db := newTempDB(t, backend) - itr := db.Iterator() - itr.Seek(bz("1")) + itr := db.Iterator(bz("1"), EndingKey()) checkInvalid(t, itr) }) } } -func TestDBIteratorBadSeek(t *testing.T) { +func TestDBIteratorNonemptyBeginAfter(t *testing.T) { for backend, _ := range backends { t.Run(fmt.Sprintf("Backend %s", backend), func(t *testing.T) { db := newTempDB(t, backend) db.SetSync(bz("1"), bz("value_1")) - itr := db.Iterator() - itr.Seek(bz("2")) + itr := db.Iterator(bz("2"), EndingKey()) checkInvalid(t, itr) }) diff --git a/db/mem_db_test.go b/db/mem_db_test.go index b5c9167c8..42e242857 100644 --- a/db/mem_db_test.go +++ b/db/mem_db_test.go @@ -19,7 +19,7 @@ func TestMemDbIterator(t *testing.T) { db.Set(k, value) } - iter := db.Iterator() + iter := db.Iterator(BeginningKey(), EndingKey()) i := 0 for ; iter.Valid(); iter.Next() { assert.Equal(t, db.Get(iter.Key()), iter.Value(), "values dont match for key") diff --git a/db/types.go b/db/types.go index 7422a5155..8306813c7 100644 --- a/db/types.go +++ b/db/types.go @@ -90,17 +90,11 @@ type Iterator interface { Valid() bool // Next moves the iterator to the next sequential key in the database, as - // defined by the Comparator in the ReadOptions used to create this Iterator. + // defined by order of iteration. // // If Valid returns false, this method will panic. Next() - // Prev moves the iterator to the previous sequential key in the database, as - // defined by the Comparator in the ReadOptions used to create this Iterator. - // - // If Valid returns false, this method will panic. - Prev() - // Key returns the key of the cursor. // // If Valid returns false, this method will panic. @@ -120,3 +114,8 @@ type Iterator interface { // Release deallocates the given Iterator. Release() } + +// For testing convenience. +func bz(s string) []byte { + return []byte(s) +} diff --git a/db/util_test.go b/db/util_test.go index 55a41bf5b..4f8b9c456 100644 --- a/db/util_test.go +++ b/db/util_test.go @@ -66,7 +66,6 @@ func TestPrefixIteratorMatches1N(t *testing.T) { db.SetSync(bz("a/1"), bz("value_1")) db.SetSync(bz("a/3"), bz("value_3")) itr := IteratePrefix(db, []byte("a/")) - itr.Seek(bz("a/1")) checkValid(t, itr, true) checkItem(t, itr, bz("a/1"), bz("value_1")) @@ -82,32 +81,6 @@ func TestPrefixIteratorMatches1N(t *testing.T) { } } -// Search for a/1, fail by too much Prev() -func TestPrefixIteratorMatches1P(t *testing.T) { - for backend, _ := range backends { - t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { - db := newTempDB(t, backend) - db.SetSync(bz("a/1"), bz("value_1")) - db.SetSync(bz("a/3"), bz("value_3")) - itr := IteratePrefix(db, []byte("a/")) - itr.Seek(bz("a/1")) - - checkValid(t, itr, true) - checkItem(t, itr, bz("a/1"), bz("value_1")) - checkNext(t, itr, true) - checkItem(t, itr, bz("a/3"), bz("value_3")) - checkPrev(t, itr, true) - checkItem(t, itr, bz("a/1"), bz("value_1")) - - // Bad! - checkPrev(t, itr, false) - - // Once invalid... - checkInvalid(t, itr) - }) - } -} - // Search for a/2, fail by too much Next() func TestPrefixIteratorMatches2N(t *testing.T) { for backend, _ := range backends { @@ -116,41 +89,15 @@ func TestPrefixIteratorMatches2N(t *testing.T) { db.SetSync(bz("a/1"), bz("value_1")) db.SetSync(bz("a/3"), bz("value_3")) itr := IteratePrefix(db, []byte("a/")) - itr.Seek(bz("a/2")) checkValid(t, itr, true) - checkItem(t, itr, bz("a/3"), bz("value_3")) - checkPrev(t, itr, true) checkItem(t, itr, bz("a/1"), bz("value_1")) checkNext(t, itr, true) - checkItem(t, itr, bz("a/3"), bz("value_3")) - - // Bad! - checkNext(t, itr, false) - - // Once invalid... - checkInvalid(t, itr) - }) - } -} - -// Search for a/2, fail by too much Prev() -func TestPrefixIteratorMatches2P(t *testing.T) { - for backend, _ := range backends { - t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { - db := newTempDB(t, backend) - db.SetSync(bz("a/1"), bz("value_1")) - db.SetSync(bz("a/3"), bz("value_3")) - itr := IteratePrefix(db, []byte("a/")) - itr.Seek(bz("a/2")) - checkValid(t, itr, true) checkItem(t, itr, bz("a/3"), bz("value_3")) - checkPrev(t, itr, true) - checkItem(t, itr, bz("a/1"), bz("value_1")) // Bad! - checkPrev(t, itr, false) + checkNext(t, itr, false) // Once invalid... checkInvalid(t, itr) @@ -166,11 +113,8 @@ func TestPrefixIteratorMatches3N(t *testing.T) { db.SetSync(bz("a/1"), bz("value_1")) db.SetSync(bz("a/3"), bz("value_3")) itr := IteratePrefix(db, []byte("a/")) - itr.Seek(bz("a/3")) checkValid(t, itr, true) - checkItem(t, itr, bz("a/3"), bz("value_3")) - checkPrev(t, itr, true) checkItem(t, itr, bz("a/1"), bz("value_1")) checkNext(t, itr, true) checkItem(t, itr, bz("a/3"), bz("value_3")) @@ -183,27 +127,3 @@ func TestPrefixIteratorMatches3N(t *testing.T) { }) } } - -// Search for a/3, fail by too much Prev() -func TestPrefixIteratorMatches3P(t *testing.T) { - for backend, _ := range backends { - t.Run(fmt.Sprintf("Prefix w/ backend %s", backend), func(t *testing.T) { - db := newTempDB(t, backend) - db.SetSync(bz("a/1"), bz("value_1")) - db.SetSync(bz("a/3"), bz("value_3")) - itr := IteratePrefix(db, []byte("a/")) - itr.Seek(bz("a/3")) - - checkValid(t, itr, true) - checkItem(t, itr, bz("a/3"), bz("value_3")) - checkPrev(t, itr, true) - checkItem(t, itr, bz("a/1"), bz("value_1")) - - // Bad! - checkPrev(t, itr, false) - - // Once invalid... - checkInvalid(t, itr) - }) - } -}