Merge pull request #126 from DRK3/FixGetBulkWithNilArgumentErrorHandling

fix: Incorrect error handling when GetBulk argument is nil

pkg/storage/couchdb/couchdbstore.go

@@ -321,6 +321,10 @@ func (c *CouchDBStore) Get(k string) ([]byte, error) {
 // attachments.
 // If values are stored as attachments, then there are still optimizations that could be done - see TODO #124.
 func (c *CouchDBStore) GetBulk(keys ...string) ([][]byte, error) {
+	if keys == nil {
+		return nil, storage.ErrGetBulkKeysStringSliceNil
+	}
+
 	rawDocs, err := c.getRawDocs(keys)
 	if err != nil {
 		return nil, fmt.Errorf(getRawDocsFailureErrMsg, err)

pkg/storage/couchdb/couchdbstore_test.go

@@ -914,6 +914,7 @@ func TestCouchDBStore_GetBulk(t *testing.T) {
 				fmt.Errorf(getBulkKeyNotFound, testDocKey2, storage.ErrValueNotFound)).Error())
 		require.Nil(t, values)
 	})
+
 	t.Run("Value (stored as JSON) not found after being deleted", func(t *testing.T) {
 		provider := initializeTest(t)
 
@@ -1028,6 +1029,29 @@ func TestCouchDBStore_GetBulk(t *testing.T) {
 					fmt.Errorf(failureWhileReadingAttachmentContent, errFailingReadAll))).Error())
 		require.Nil(t, values)
 	})
+	t.Run("Failure: nil argument", func(t *testing.T) {
+		provider := initializeTest(t)
+
+		store := createAndOpenTestStore(t, provider)
+
+		err := store.Put(testDocKey1, []byte(testJSONValue1))
+		require.NoError(t, err)
+
+		values, err := store.GetBulk(nil...)
+		require.EqualError(t, err, storage.ErrGetBulkKeysStringSliceNil.Error())
+		require.Nil(t, values)
+	})
+	t.Run("Value not found, bulk get called with only one key", func(t *testing.T) {
+		provider := initializeTest(t)
+
+		store := createAndOpenTestStore(t, provider)
+
+		values, err := store.GetBulk(testDocKey1)
+		require.EqualError(t, err,
+			fmt.Errorf(failureWhileGettingStoredValuesFromRawDocs,
+				fmt.Errorf(getBulkKeyNotFound, testDocKey1, storage.ErrValueNotFound)).Error())
+		require.Nil(t, values)
+	})
 }
 
 func TestCouchDBStore_GetAll(t *testing.T) {

pkg/storage/errors.go

@@ -43,3 +43,7 @@ var ErrNilValues = errors.New("values slice cannot be nil")
 // ErrKeysAndValuesDifferentLengths is returned when an attempt is made to call the PutBulk method with
 // differently sized keys and values arrays.
 var ErrKeysAndValuesDifferentLengths = errors.New("keys and values must be the same length")
+
+// ErrGetBulkKeysStringSliceNil is returned when an attempt is made to call the GetBulk method with a nil slice of
+// strings.
+var ErrGetBulkKeysStringSliceNil = errors.New("keys string slice cannot be nil")

pkg/storage/memstore/memstore.go

@@ -136,6 +136,10 @@ func (m *MemStore) Get(k string) ([]byte, error) {
 // GetBulk fetches the values associated with the given keys. This method works in an all-or-nothing manner.
 // It returns an error if any of the keys don't exist. If even one key is missing, then no values are returned.
 func (m *MemStore) GetBulk(keys ...string) ([][]byte, error) {
+	if keys == nil {
+		return nil, storage.ErrGetBulkKeysStringSliceNil
+	}
+
 	storedValues := make([][]byte, len(keys))
 
 	m.mux.RLock()

pkg/storage/memstore/memstore_test.go

@@ -177,6 +177,20 @@ func TestMemStore_GetBulk(t *testing.T) {
 		require.EqualError(t, err, fmt.Errorf(getBulkKeyNotFound, "testKey", storage.ErrValueNotFound).Error())
 		require.Nil(t, values)
 	})
+	t.Run("Failure: key not found, called with a single key", func(t *testing.T) {
+		store := MemStore{db: make(map[string][]byte)}
+
+		values, err := store.GetBulk("testKey")
+		require.EqualError(t, err, fmt.Errorf(getBulkKeyNotFound, "testKey", storage.ErrValueNotFound).Error())
+		require.Nil(t, values)
+	})
+	t.Run("Failure: nil argument", func(t *testing.T) {
+		store := MemStore{db: make(map[string][]byte)}
+
+		values, err := store.GetBulk(nil...)
+		require.EqualError(t, err, storage.ErrGetBulkKeysStringSliceNil.Error())
+		require.Nil(t, values)
+	})
 }
 
 func TestMemStore_GetAll(t *testing.T) {

pkg/storage/mockstore/mockstore.go

@@ -116,6 +116,10 @@ func (s *MockStore) Get(k string) ([]byte, error) {
 // GetBulk fetches the values associated with the given keys. This method works in an all-or-nothing manner.
 // It returns an error if any of the keys don't exist. If even one key is missing, then no values are returned.
 func (s *MockStore) GetBulk(keys ...string) ([][]byte, error) {
+	if keys == nil {
+		return nil, storage.ErrGetBulkKeysStringSliceNil
+	}
+
 	storedValues := make([][]byte, len(keys))
 
 	s.lock.RLock()