Repository: TBD54566975/ssi-service
Pull request #274 (2023-02-02 20:43:54)

adding exists to db interface and other error handling
# Overview
* Handle case where `current-list-index >= bitStringLength`
* Only generate the new statusListIndex, and array after doing a read and making sure it does not exist

codecov-commenter: # [Codecov](https://codecov.io/gh/TBD54566975/ssi-service/pull/274?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) Report
> Merging [#274](https://codecov.io/gh/TBD54566975/ssi-service/pull/274?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) (d2b5510) into [main](https://codecov.io/gh/TBD54566975/ssi-service/commit/13ea9a07869343abeb74cb0f5d95baaa9a2ad28a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) (13ea9a0) will **increase** coverage by `0.47%`.
> The diff coverage is `87.50%`.

```diff
@@            Coverage Diff             @@
##             main     #274      +/-   ##
==========================================
+ Coverage   24.22%   24.69%   +0.47%     
==========================================
  Files          31       31              
  Lines        3142     3166      +24     
==========================================
+ Hits          761      782      +21     
- Misses       2263     2265       +2     
- Partials      118      119       +1     
```


| [Impacted Files](https://codecov.io/gh/TBD54566975/ssi-service/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975) | Coverage Δ | |
|---|---|---|
| [pkg/storage/storage.go](https://codecov.io/gh/TBD54566975/ssi-service/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975#diff-cGtnL3N0b3JhZ2Uvc3RvcmFnZS5nbw==) | `40.62% <ø> (ø)` | |
| [pkg/storage/redis.go](https://codecov.io/gh/TBD54566975/ssi-service/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975#diff-cGtnL3N0b3JhZ2UvcmVkaXMuZ28=) | `57.93% <62.50%> (+0.16%)` | :arrow_up: |
| [pkg/storage/bolt.go](https://codecov.io/gh/TBD54566975/ssi-service/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975#diff-cGtnL3N0b3JhZ2UvYm9sdC5nbw==) | `70.00% <100.00%> (+2.47%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TBD54566975)


Patch:
diff --git a/pkg/service/credential/storage.go b/pkg/service/credential/storage.go
index f332539..15e9175 100644
--- a/pkg/service/credential/storage.go
+++ b/pkg/service/credential/storage.go
@@ -80,26 +80,42 @@ type StatusListIndex struct {
 
 func NewCredentialStorage(db storage.ServiceStorage) (*Storage, error) {
 	if db == nil {
-		return nil, errors.New("bolt db reference is nil")
+		return nil, util.LoggingNewError("bolt db reference is nil")
 	}
 
-	randUniqueList := randomUniqueNum(bitStringLength)
-	uniqueNumBytes, err := json.Marshal(randUniqueList)
+	listIndexKeyExists, err := db.Exists(context.Background(), statusListIndexNamespace, currentListIndexKey)
 	if err != nil {
-		return nil, util.LoggingErrorMsg(err, "could not marshal random unique numbers")
+		return nil, util.LoggingErrorMsg(err, "exists for currentListIndexKey")
 	}
 
-	if err := db.Write(context.Background(), statusListIndexNamespace, statusListIndexesKey, uniqueNumBytes); err != nil {
-		return nil, util.LoggingErrorMsg(err, "problem writing status list indexes to db")
+	statusListIndexesKeyExists, err := db.Exists(context.Background(), statusListIndexNamespace, statusListIndexesKey)
+	if err != nil {
+		return nil, util.LoggingErrorMsg(err, "exists for statusListIndexesKey")
 	}
 
-	statusListIndexBytes, err := json.Marshal(StatusListIndex{Index: 0})
-	if err != nil {
-		return nil, util.LoggingErrorMsg(err, "could not marshal status list index bytes")
+	if listIndexKeyExists != statusListIndexesKeyExists {
+		return nil, util.LoggingNewError("list index and status list index not in the same state")
 	}
 
-	if err := db.Write(context.Background(), statusListIndexNamespace, currentListIndexKey, statusListIndexBytes); err != nil {
-		return nil, util.LoggingErrorMsg(err, "problem writing current list index to db")
+	if !listIndexKeyExists && !statusListIndexesKeyExists {
+		randUniqueList := randomUniqueNum(bitStringLength)
+		uniqueNumBytes, err := json.Marshal(randUniqueList)
+		if err != nil {
+			return nil, util.LoggingErrorMsg(err, "could not marshal random unique numbers")
+		}
+
+		if err := db.Write(context.Background(), statusListIndexNamespace, statusListIndexesKey, uniqueNumBytes); err != nil {
+			return nil, util.LoggingErrorMsg(err, "problem writing status list indexes to db")
+		}
+
+		statusListIndexBytes, err := json.Marshal(StatusListIndex{Index: 0})
+		if err != nil {
+			return nil, util.LoggingErrorMsg(err, "could not marshal status list index bytes")
+		}
+
+		if err := db.Write(context.Background(), statusListIndexNamespace, currentListIndexKey, statusListIndexBytes); err != nil {
+			return nil, util.LoggingErrorMsg(err, "problem writing current list index to db")
+		}
 	}
 
 	return &Storage{db: db}, nil
@@ -177,12 +193,16 @@ func (cs *Storage) IncrementStatusListIndex(ctx context.Context) error {
 func (cs *Storage) GetIncrementStatusListIndexWriteContext(ctx context.Context) (*WriteContext, error) {
 	gotCurrentListIndexBytes, err := cs.db.Read(ctx, statusListIndexNamespace, currentListIndexKey)
 	if err != nil {
-		return nil, util.LoggingErrorMsgf(err, "could not get list index")
+		return nil, util.LoggingErrorMsg(err, "could not get list index")
 	}
 
 	var statusListIndex StatusListIndex
 	if err = json.Unmarshal(gotCurrentListIndexBytes, &statusListIndex); err != nil {
-		return nil, util.LoggingErrorMsgf(err, "could not unmarshal unique numbers")
+		return nil, util.LoggingErrorMsg(err, "could not unmarshal unique numbers")
+	}
+
+	if statusListIndex.Index >= bitStringLength-1 {
+		return nil, util.LoggingErrorMsg(err, "no more indexes available for status list index")
 	}
 
 	statusListIndexBytes, err := json.Marshal(StatusListIndex{Index: statusListIndex.Index + 1})
diff --git a/pkg/storage/bolt.go b/pkg/storage/bolt.go
index e855d34..04fa344 100644
--- a/pkg/storage/bolt.go
+++ b/pkg/storage/bolt.go
@@ -77,6 +77,27 @@ type boltTx struct {
 	b *BoltDB
 }
 
+func (b *BoltDB) Exists(ctx context.Context, namespace, key string) (bool, error) {
+	exists := true
+	var result []byte
+
+	err := b.db.View(func(tx *bolt.Tx) error {
+		bucket := tx.Bucket([]byte(namespace))
+		if bucket == nil {
+			exists = false
+			return nil
+		}
+		result = bucket.Get([]byte(key))
+		return nil
+	})
+
+	if result == nil {
+		exists = false
+	}
+
+	return exists, err
+}
+
 // TODO: Implement to be transactional
 func (btx *boltTx) Write(ctx context.Context, namespace, key string, value []byte) error {
 	return btx.b.Write(ctx, namespace, key, value)
diff --git a/pkg/storage/redis.go b/pkg/storage/redis.go
index 50ed2a1..da5e24c 100644
--- a/pkg/storage/redis.go
+++ b/pkg/storage/redis.go
@@ -118,6 +118,17 @@ func (b *RedisDB) Execute(ctx context.Context, businessLogicFunc BusinessLogicFu
 	return finalOutput, nil
 }
 
+func (b *RedisDB) Exists(ctx context.Context, namespace, key string) (bool, error) {
+	nameSpaceKey := getRedisKey(namespace, key)
+	existsInt, err := b.db.Exists(ctx, nameSpaceKey).Result()
+	if err != nil {
+		return false, errors.Wrap(err, "checking if exists")
+	}
+
+	exists := existsInt != 0
+	return exists, nil
+}
+
 func (b *RedisDB) Write(ctx context.Context, namespace, key string, value []byte) error {
 	nameSpaceKey := getRedisKey(namespace, key)
 	return b.db.Set(ctx, nameSpaceKey, value, 0).Err()
diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go
index 38b5c12..a70b755 100644
--- a/pkg/storage/storage.go
+++ b/pkg/storage/storage.go
@@ -38,6 +38,7 @@ type ServiceStorage interface {
 	Write(ctx context.Context, namespace, key string, value []byte) error
 	WriteMany(ctx context.Context, namespace, key []string, value [][]byte) error
 	Read(ctx context.Context, namespace, key string) ([]byte, error)
+	Exists(ctx context.Context, namespace, key string) (bool, error)
 	ReadAll(ctx context.Context, namespace string) (map[string][]byte, error)
 	ReadPrefix(ctx context.Context, namespace, prefix string) (map[string][]byte, error)
 	ReadAllKeys(ctx context.Context, namespace string) ([]string, error)


Test patch:
diff --git a/pkg/server/router/credential_test.go b/pkg/server/router/credential_test.go
index e207e46..18d8270 100644
--- a/pkg/server/router/credential_test.go
+++ b/pkg/server/router/credential_test.go
@@ -529,4 +529,76 @@ func TestCredentialRouter(t *testing.T) {
 
 		assert.NotEqualValues(tt, encodedListAfterRevoke, encodedList)
 	})
+
+	t.Run("Existing Status List Indexes Used After Restart", func(tt *testing.T) {
+		statusListIndexNamespace := "status-list-index"
+
+		statusListIndexesKey := "status-list-indexes"
+		currentListIndexKey := "current-list-index"
+
+		bolt := setupTestDB(tt)
+		assert.NotNil(tt, bolt)
+
+		// Make sure there is nothing in DB before we create storage
+		value, err := bolt.Read(context.Background(), statusListIndexNamespace, currentListIndexKey)
+		assert.NoError(tt, err)
+		assert.Empty(tt, value)
+
+		value, err = bolt.Read(context.Background(), statusListIndexNamespace, statusListIndexesKey)
+		assert.NoError(tt, err)
+		assert.Empty(tt, value)
+
+		exists, err := bolt.Exists(context.Background(), statusListIndexNamespace, currentListIndexKey)
+		assert.NoError(tt, err)
+		assert.False(tt, exists)
+
+		exists, err = bolt.Exists(context.Background(), statusListIndexNamespace, statusListIndexesKey)
+		assert.NoError(tt, err)
+		assert.False(tt, exists)
+
+		// Create the storage
+		credStorage, err := credential.NewCredentialStorage(bolt)
+		assert.NoError(tt, err)
+		assert.NotEmpty(tt, credStorage)
+
+		// Make sure that values are there after we create a new credential storage
+		exists, err = bolt.Exists(context.Background(), statusListIndexNamespace, currentListIndexKey)
+		assert.NoError(tt, err)
+		assert.True(tt, exists)
+
+		exists, err = bolt.Exists(context.Background(), statusListIndexNamespace, statusListIndexesKey)
+		assert.NoError(tt, err)
+		assert.True(tt, exists)
+
+		value, err = bolt.Read(context.Background(), statusListIndexNamespace, currentListIndexKey)
+		assert.NoError(tt, err)
+		assert.NotEmpty(tt, value)
+
+		statusListIndexes, err := bolt.Read(context.Background(), statusListIndexNamespace, statusListIndexesKey)
+		assert.NoError(tt, err)
+		assert.NotEmpty(tt, value)
+
+		// "Restart" the service
+		credStorage, err = credential.NewCredentialStorage(bolt)
+		assert.NoError(tt, err)
+		assert.NotEmpty(tt, credStorage)
+
+		exists, err = bolt.Exists(context.Background(), statusListIndexNamespace, currentListIndexKey)
+		assert.NoError(tt, err)
+		assert.True(tt, exists)
+
+		exists, err = bolt.Exists(context.Background(), statusListIndexNamespace, statusListIndexesKey)
+		assert.NoError(tt, err)
+		assert.True(tt, exists)
+
+		value, err = bolt.Read(context.Background(), statusListIndexNamespace, currentListIndexKey)
+		assert.NoError(tt, err)
+		assert.NotEmpty(tt, value)
+
+		statusListIndexesAfterRestart, err := bolt.Read(context.Background(), statusListIndexNamespace, statusListIndexesKey)
+		assert.NoError(tt, err)
+		assert.NotEmpty(tt, statusListIndexesAfterRestart)
+
+		assert.Equal(tt, statusListIndexes, statusListIndexesAfterRestart)
+	})
 }
diff --git a/pkg/storage/db_test.go b/pkg/storage/db_test.go
index ae7375d..2145988 100644
--- a/pkg/storage/db_test.go
+++ b/pkg/storage/db_test.go
@@ -199,6 +199,71 @@ func TestDBEmptyNamespace(t *testing.T) {
 	}
 }
 
+func TestDBExists_FalseWhenNoNamespaceKey(t *testing.T) {
+	for _, dbImpl := range getDBImplementations(t) {
+		db := dbImpl
+
+		exists, err := db.Exists(context.Background(), "dnenamespace", "dnekey")
+		assert.NoError(t, err)
+		assert.False(t, exists)
+	}
+}
+
+func TestDBExists_FalseWhenKeyIsAbsent(t *testing.T) {
+	for _, dbImpl := range getDBImplementations(t) {
+		db := dbImpl
+
+		namespace := "exists"
+
+		err := db.Write(context.Background(), namespace, "dnedne", nil)
+		assert.NoError(t, err)
+
+		exists, err := db.Exists(context.Background(), namespace, "otherdnekey")
+		assert.NoError(t, err)
+		assert.False(t, exists)
+	}
+}
+
+func TestDBExists_TrueWhenNilKey(t *testing.T) {
+	for _, dbImpl := range getDBImplementations(t) {
+		db := dbImpl
+
+		namespace := "exists"
+		key := "key1"
+
+		err := db.Write(context.Background(), namespace, key, nil)
+		assert.NoError(t, err)
+
+		exists, err := db.Exists(context.Background(), namespace, key)
+		assert.NoError(t, err)
+		assert.True(t, exists)
+	}
+}
+
+func TestDBExists_FalseAfterDeletingKey(t *testing.T) {
+	for _, dbImpl := range getDBImplementations(t) {
+		db := dbImpl
+
+		namespace := "exists"
+		key := "key1"
+
+		dummyData := []byte("dummy")
+		err := db.Write(context.Background(), namespace, key, dummyData)
+		assert.NoError(t, err)
+
+		exists, err := db.Exists(context.Background(), namespace, key)
+		assert.NoError(t, err)
+		assert.True(t, exists)
+
+		err = db.Delete(context.Background(), namespace, key)
+		assert.NoError(t, err)
+
+		exists, err = db.Exists(context.Background(), namespace, key)
+		assert.NoError(t, err)
+		assert.False(t, exists)
+	}
+}
+
 type testStruct struct {
 	Status int    `json:"status"`
 	Reason string `json:"reason"`