From 3ed9b655d11902143020bc6cadb1c5c64412f92d Mon Sep 17 00:00:00 2001 From: Gerard Snaauw Date: Tue, 3 Jan 2023 14:48:34 +0100 Subject: [PATCH] Add ErrKeyNotFound to reader.Get (#41) --- badger/badger.go | 2 +- bbolt/bbolt.go | 4 ++++ bbolt/bbolt_test.go | 4 +--- kvtests/common_tests.go | 10 ++++------ redis7/redis.go | 4 ++-- store.go | 16 ++++++++++------ store_test.go | 11 +++++++++++ 7 files changed, 33 insertions(+), 18 deletions(-) diff --git a/badger/badger.go b/badger/badger.go index c0f889e..e8e535b 100644 --- a/badger/badger.go +++ b/badger/badger.go @@ -243,7 +243,7 @@ func (t badgerShelf) Get(key stoabs.Key) ([]byte, error) { item, err := t.tx.badgerTx.Get(t.key(key).Bytes()) if err != nil { if errors.Is(err, badger.ErrKeyNotFound) { - return nil, nil + return nil, stoabs.ErrKeyNotFound } return nil, err } diff --git a/bbolt/bbolt.go b/bbolt/bbolt.go index e53e56a..bf43e19 100644 --- a/bbolt/bbolt.go +++ b/bbolt/bbolt.go @@ -252,6 +252,10 @@ type bboltShelf struct { func (t bboltShelf) Get(key stoabs.Key) ([]byte, error) { value := t.bucket.Get(key.Bytes()) + if value == nil { + return nil, stoabs.ErrKeyNotFound + } + // Because things will go terribly wrong when you use a []byte returned by BBolt outside its transaction, // we want to make sure to work with a copy. // diff --git a/bbolt/bbolt_test.go b/bbolt/bbolt_test.go index 46d3167..8c7b68f 100644 --- a/bbolt/bbolt_test.go +++ b/bbolt/bbolt_test.go @@ -89,9 +89,7 @@ func TestBBolt_WriteShelf(t *testing.T) { actual, err = reader.Get(stoabs.BytesKey(key)) return err }) - if !assert.NoError(t, err) { - return - } + assert.ErrorIs(t, err, stoabs.ErrKeyNotFound) assert.Nil(t, actual) }) } diff --git a/kvtests/common_tests.go b/kvtests/common_tests.go index 760c782..ce76222 100644 --- a/kvtests/common_tests.go +++ b/kvtests/common_tests.go @@ -116,7 +116,7 @@ func TestReadingAndWriting(t *testing.T, storeProvider StoreProvider) { t.Fatal() return nil }) - assert.NoError(t, err) + assert.ErrorIs(t, err, stoabs.ErrKeyNotFound) }) t.Run("read non-existing key", func(t *testing.T) { @@ -134,9 +134,7 @@ func TestReadingAndWriting(t *testing.T, storeProvider StoreProvider) { actual, err = reader.Get(bytesKey) return err }) - if !assert.NoError(t, err) { - return - } + assert.ErrorIs(t, err, stoabs.ErrKeyNotFound) assert.Nil(t, actual) }) }) @@ -598,7 +596,7 @@ func TestWriteTransactions(t *testing.T, storeProvider StoreProvider) { actual, err = reader.Get(bytesKey) return err }) - assert.NoError(t, err) + assert.ErrorIs(t, err, stoabs.ErrKeyNotFound) assert.Nil(t, actual) }) }) @@ -685,7 +683,7 @@ func TestDelete(t *testing.T, storeProvider StoreProvider) { actual, err = reader.Get(bytesKey) return err }) - assert.NoError(t, err) + assert.ErrorIs(t, err, stoabs.ErrKeyNotFound) assert.Nil(t, actual) }) t.Run("delete non-existing entry", func(t *testing.T) { diff --git a/redis7/redis.go b/redis7/redis.go index 24a635f..7140279 100644 --- a/redis7/redis.go +++ b/redis7/redis.go @@ -328,8 +328,8 @@ func (s shelf) Delete(key stoabs.Key) error { func (s shelf) Get(key stoabs.Key) ([]byte, error) { result, err := s.reader.Get(s.ctx, s.toRedisKey(key)).Result() - if err == redis.Nil { - return nil, nil + if errors.Is(err, redis.Nil) { + return nil, stoabs.ErrKeyNotFound } else if err != nil { return nil, stoabs.DatabaseError(err) } diff --git a/store.go b/store.go index 3f6f18f..4c7e1db 100644 --- a/store.go +++ b/store.go @@ -44,7 +44,7 @@ type ErrDatabase struct { func (e ErrDatabase) Error() string { // Use Sprintf to avoid dereferencing of wrapped nil error - return fmt.Sprintf("Database Error: %s", e.error) + return fmt.Sprintf("database error: %s", e.error) } func (e ErrDatabase) Is(other error) bool { @@ -58,6 +58,12 @@ func (e ErrDatabase) Unwrap() error { // ErrStoreIsClosed is returned when an operation is executed on a closed store. Is also a ErrDatabase. var ErrStoreIsClosed = DatabaseError(errors.New("database not open")) +// ErrCommitFailed is returned when the commit of transaction fails. Is also a ErrDatabase. +var ErrCommitFailed = DatabaseError(errors.New("unable to commit transaction")) + +// ErrKeyNotFound is returned when the requested key does not exist +var ErrKeyNotFound = errors.New("key not found") + const DefaultTransactionTimeout = 30 * time.Second const defaultLockAcquisitionTimeout = 3 * time.Second @@ -138,7 +144,8 @@ type CallerFn func(key Key, value []byte) error // Reader is used to read from a shelf. type Reader interface { - // Get returns the value for the given key. If it does not exist it returns nil. + // Get returns the value for the given key. + // If the key does not exist it returns ErrKeyNotFound. // Returns a ErrDatabase if unsuccessful. Get(key Key) ([]byte, error) // Iterate walks over all key/value pairs for this shelf. Ordering is not guaranteed. @@ -164,9 +171,6 @@ type Writer interface { Delete(key Key) error } -// ErrCommitFailed is returned when the commit of transaction fails. Is also a ErrDatabase. -var ErrCommitFailed = DatabaseError(errors.New("unable to commit transaction")) - type Store interface { // Close releases all resources associated with the store. It is safe to call multiple (subsequent) times. // The context being passed can be used to specify a timeout for the close operation. @@ -259,7 +263,7 @@ type ReadTx interface { type NilReader struct{} func (n NilReader) Get(_ Key) ([]byte, error) { - return nil, nil + return nil, ErrKeyNotFound } func (n NilReader) Iterate(_ CallerFn, _ Key) error { diff --git a/store_test.go b/store_test.go index 873cb3d..a390992 100644 --- a/store_test.go +++ b/store_test.go @@ -48,6 +48,9 @@ func TestDatabaseError(t *testing.T) { assert.ErrorAs(t, ErrStoreIsClosed, new(ErrDatabase), "ErrStoreIsClosed should be a ErrDatabase") assert.ErrorAs(t, ErrCommitFailed, new(ErrDatabase), "ErrCommitFailed should be a ErrDatabase") }) + t.Run("does not wrap non-db errors", func(t *testing.T) { + assert.False(t, errors.As(ErrKeyNotFound, new(ErrDatabase)), "ErrKeyNotFound is not a ErrDatabase") + }) t.Run("does not double wrap", func(t *testing.T) { firstError := DatabaseError(errors.New("this is wrapped")) secondError := DatabaseError(fmt.Errorf("this is not wrapped: %w", firstError)) @@ -68,3 +71,11 @@ func TestNewErrorWriter(t *testing.T) { assert.ErrorIs(t, ErrDatabase{}, err) }) } + +func TestNilReader_Get(t *testing.T) { + t.Run("returns error", func(t *testing.T) { + data, err := NilReader{}.Get(BytesKey{}) + assert.ErrorIs(t, err, ErrKeyNotFound) + assert.Nil(t, data) + }) +}