Follow up to Panice to Err PR (#33)

* Follow up to Panice to Err PR - there were some post merge comments, this is meant to address them Signed-off-by: Marko Baricevic <marbar3778@yahoo.com> * fix returns with errors * address pr comments, still need to add func error * address more comments * fixes to rocks and other dbs * fixl linting issues * Apply suggestions from code review
tendermint · Jan 2, 2020 · 5d98246 · 5d98246
1 parent 6c0b388
commit 5d98246
Show file tree

Hide file tree

Showing 15 changed files with 232 additions and 326 deletions.
diff --git a/backend_test.go b/backend_test.go
@@ -335,11 +335,9 @@ func testDBIterator(t *testing.T, backend BackendType) {
 func verifyIterator(t *testing.T, itr Iterator, expected []int64, msg string) {
 	var list []int64
 	for itr.Valid() {
-		key, err := itr.Key()
-		assert.NoError(t, err)
+		key := itr.Key()
 		list = append(list, bytes2Int64(key))
-		err = itr.Next()
-		assert.NoError(t, err)
+		itr.Next()
 	}
 	assert.Equal(t, expected, list, msg)
 }
diff --git a/boltdb.go b/boltdb.go
@@ -135,6 +135,7 @@ func (bdb *BoltDB) Print() error {
 	if err != nil {
 		return err
 	}
+	return nil
 }
 
 func (bdb *BoltDB) Stats() map[string]string {
@@ -310,34 +311,31 @@ func (itr *boltDBIterator) Valid() bool {
 	return true
 }
 
-func (itr *boltDBIterator) Next() error {
-	if err := itr.assertIsValid(); err != nil {
-		return err
-	}
+func (itr *boltDBIterator) Next() {
+	itr.assertIsValid()
 	if itr.isReverse {
 		itr.currentKey, itr.currentValue = itr.itr.Prev()
 	} else {
 		itr.currentKey, itr.currentValue = itr.itr.Next()
 	}
-	return nil
 }
 
-func (itr *boltDBIterator) Key() ([]byte, error) {
-	if err := itr.assertIsValid(); err != nil {
-		return nil, err
-	}
-	return append([]byte{}, itr.currentKey...), nil
+func (itr *boltDBIterator) Key() []byte {
+	itr.assertIsValid()
+	return append([]byte{}, itr.currentKey...)
 }
 
-func (itr *boltDBIterator) Value() ([]byte, error) {
-	if err := itr.assertIsValid(); err != nil {
-		return nil, err
-	}
+func (itr *boltDBIterator) Value() []byte {
+	itr.assertIsValid()
 	var value []byte
 	if itr.currentValue != nil {
 		value = append([]byte{}, itr.currentValue...)
 	}
-	return value, nil
+	return value
+}
+
+func (itr *boltDBIterator) Error() error {
+	return nil
 }
 
 func (itr *boltDBIterator) Close() {
@@ -347,11 +345,10 @@ func (itr *boltDBIterator) Close() {
 	}
 }
 
-func (itr *boltDBIterator) assertIsValid() error {
+func (itr *boltDBIterator) assertIsValid() {
 	if !itr.Valid() {
-		return errors.New("boltdb-iterator is invalid")
+		panic("boltdb-iterator is invalid")
 	}
-	return nil
 }
 
 // nonEmptyKey returns a []byte("nil") if key is empty.

diff --git a/boltdb_test.go b/boltdb_test.go
@@ -20,8 +20,6 @@ func TestBoltDBNewBoltDB(t *testing.T) {
 	db.Close()
 }
 
-//TODO: ADD TESTS!!!
-
 func BenchmarkBoltDBRandomReadsWrites(b *testing.B) {
 	name := fmt.Sprintf("test_%x", randStr(12))
 	db, err := NewBoltDB(name, "")

diff --git a/c_level_db.go b/c_level_db.go
@@ -122,21 +122,14 @@ func (db *CLevelDB) Close() error {
 
 // Implements DB.
 func (db *CLevelDB) Print() error {
-	itr := db.Iterator(nil, nil)
+	itr, err := db.Iterator(nil, nil)
+	if err != nil {
+		return err
+	}
 	defer itr.Close()
-	var err error
-	for ; itr.Valid(); err = itr.Next() {
-		if err != nil {
-			return errors.Wrap(err, "next")
-		}
-		key, err := itr.Key()
-		if err != nil {
-			return errors.Wrap(err, "key")
-		}
-		value, err := itr.Value()
-		if err != nil {
-			return errors.Wrap(err, "value")
-		}
+	for ; itr.Valid(); itr.Next() {
+		key := itr.Key()
+		value := itr.Value()
 		fmt.Printf("[%X]:\t[%X]\n", key, value)
 	}
 	return nil
@@ -303,55 +296,45 @@ func (itr cLevelDBIterator) Valid() bool {
 	return true
 }
 
-func (itr cLevelDBIterator) Key() ([]byte, error) {
-	if err := itr.assertNoError(); err != nil {
-		return nil, err
-	}
-	if err = itr.assertIsValid(); err != nil {
-		return nil, err
-	}
-	return itr.source.Key(), nil
+func (itr cLevelDBIterator) Key() []byte {
+	itr.assertNoError()
+	itr.assertIsValid()
+	return itr.source.Key()
 }
 
-func (itr cLevelDBIterator) Value() ([]byte, error) {
-	if err := itr.assertNoError(); err != nil {
-		return nil, err
-	}
-	if err = itr.assertIsValid(); err != nil {
-		return nil, err
-	}
-	return itr.source.Value(), nil
+func (itr cLevelDBIterator) Value() []byte {
+	itr.assertNoError()
+	itr.assertIsValid()
+	return itr.source.Value()
 }
 
-func (itr cLevelDBIterator) Next() error {
-	if err := itr.assertNoError(); err != nil {
-		return err
-	}
-	if err = itr.assertIsValid(); err != nil {
-		return err
-	}
+func (itr cLevelDBIterator) Next() {
+	itr.assertNoError()
+	itr.assertIsValid()
 	if itr.isReverse {
 		itr.source.Prev()
 	} else {
 		itr.source.Next()
 	}
-	return nil
+}
+
+func (itr cLevelDBIterator) Error() error {
+	return itr.source.GetError()
 }
 
 func (itr cLevelDBIterator) Close() {
 	itr.source.Close()
 }
 
-func (itr cLevelDBIterator) assertNoError() error {
-	if err := itr.source.GetError(); err != nil {
-		return err
+func (itr cLevelDBIterator) assertNoError() {
+	err := itr.source.GetError()
+	if err != nil {
+		panic(err)
 	}
-	return nil
 }
 
-func (itr cLevelDBIterator) assertIsValid() error {
+func (itr cLevelDBIterator) assertIsValid() {
 	if !itr.Valid() {
-		return errors.New("cLevelDBIterator is invalid")
+		panic("cLevelDBIterator is invalid")
 	}
-	return nil
 }
diff --git a/common_test.go b/common_test.go
@@ -34,8 +34,8 @@ func checkNext(t *testing.T, itr Iterator, expected bool) {
 	require.Equal(t, expected, valid)
 }
 
-func checkNextErrors(t *testing.T, itr Iterator) {
-	assert.Error(t, itr.Next(), "checkNextErrors expected an error but didn't")
+func checkNextPanics(t *testing.T, itr Iterator) {
+	assert.Panics(t, func() { itr.Next() }, "checkNextPanics expected an error but didn't")
 }
 
 func checkDomain(t *testing.T, itr Iterator, start, end []byte) {
@@ -45,33 +45,27 @@ func checkDomain(t *testing.T, itr Iterator, start, end []byte) {
 }
 
 func checkItem(t *testing.T, itr Iterator, key []byte, value []byte) {
-	v, err := itr.Value()
-	assert.NoError(t, err)
+	v := itr.Value()
 
-	k, err := itr.Key()
-	assert.NoError(t, err)
+	k := itr.Key()
 
 	assert.Exactly(t, key, k)
 	assert.Exactly(t, value, v)
 }
 
 func checkInvalid(t *testing.T, itr Iterator) {
 	checkValid(t, itr, false)
-	checkKeyErrors(t, itr)
-	checkValueErrors(t, itr)
-	checkNextErrors(t, itr)
+	checkKeyPanics(t, itr)
+	checkValuePanics(t, itr)
+	checkNextPanics(t, itr)
 }
 
-func checkKeyErrors(t *testing.T, itr Iterator) {
-	key, err := itr.Key()
-	assert.Empty(t, key)
-	assert.Error(t, err, "checkKeyErrors expected an error but didn't")
+func checkKeyPanics(t *testing.T, itr Iterator) {
+	assert.Panics(t, func() { itr.Key() }, "checkKeyPanics expected panic but didn't")
 }
 
-func checkValueErrors(t *testing.T, itr Iterator) {
-	value, err := itr.Value()
-	assert.Empty(t, value)
-	assert.Error(t, err, "checkValueErrors expected an error but didn't")
+func checkValuePanics(t *testing.T, itr Iterator) {
+	assert.Panics(t, func() { itr.Value() })
 }
 
 func newTempDB(t *testing.T, backend BackendType) (db DB, dbDir string) {
@@ -190,16 +184,18 @@ func (mockIterator) Valid() bool {
 	return false
 }
 
-func (mockIterator) Next() error {
+func (mockIterator) Next() {}
+
+func (mockIterator) Key() []byte {
 	return nil
 }
 
-func (mockIterator) Key() ([]byte, error) {
-	return nil, nil
+func (mockIterator) Value() []byte {
+	return nil
 }
 
-func (mockIterator) Value() ([]byte, error) {
-	return nil, nil
+func (mockIterator) Error() error {
+	return nil
 }
 
 func (mockIterator) Close() {

diff --git a/db_test.go b/db_test.go
@@ -22,7 +22,7 @@ func TestDBIteratorSingleKey(t *testing.T) {
 			checkValid(t, itr, true)
 			checkNext(t, itr, false)
 			checkValid(t, itr, false)
-			checkNextErrors(t, itr)
+			checkNextPanics(t, itr)
 
 			// Once invalid...
 			checkInvalid(t, itr)
@@ -53,7 +53,7 @@ func TestDBIteratorTwoKeys(t *testing.T) {
 				checkNext(t, itr, false)
 				checkValid(t, itr, false)
 
-				checkNextErrors(t, itr)
+				checkNextPanics(t, itr)
 
 				// Once invalid...
 				checkInvalid(t, itr)
@@ -83,12 +83,9 @@ func TestDBIteratorMany(t *testing.T) {
 			assert.NoError(t, err)
 
 			defer itr.Close()
-			for ; itr.Valid(); err = itr.Next() {
-				assert.NoError(t, err)
-				key, err := itr.Key()
-				assert.NoError(t, err)
-				value, err = itr.Value()
-				assert.NoError(t, err)
+			for ; itr.Valid(); itr.Next() {
+				key := itr.Key()
+				value = itr.Value()
 				value1, err := db.Get(key)
 				assert.NoError(t, err)
 				assert.Equal(t, value1, value)