Repository: googleforgames/open-saves
Pull request #241 (2021-05-03 20:22:00)

refactor: rename BlobRefStatus to Status
<!--  Thanks for sending a pull request!  Here are some tips for you:

1. If this is your first time, please read our contributor guidelines: https://github.com/googleforgames/open-saves/blob/main/docs/contributing.md and developer guide https://github.com/googleforgames/open-saves/blob/main/docs/development.md
2. Please label this pull request according to what type of issue you are addressing.
3. Ensure you have added or ran the appropriate tests for your PR.
-->

**What type of PR is this?**
/kind refactor

**What this PR does / Why we need it**:
Rename BlobRefStatus to Status to make it less redundant as it is now in a separate package. BlobRefStatus* constants are also changed to Status*.

**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Closes #<issue number>`, or `Closes (paste link of issue)`.
-->
Closes #

**Special notes for your reviewer**:



Patch:
diff --git a/internal/app/collector/collector.go b/internal/app/collector/collector.go
index 8d47369..3de1710 100644
--- a/internal/app/collector/collector.go
+++ b/internal/app/collector/collector.go
@@ -84,17 +84,17 @@ func Run(ctx context.Context, cfg *Config) {
 }
 
 func (c *Collector) run(ctx context.Context) {
-	var statuses = []blobref.BlobRefStatus{
-		blobref.BlobRefStatusPendingDeletion,
-		blobref.BlobRefStatusError,
-		blobref.BlobRefStatusInitializing,
+	var statuses = []blobref.Status{
+		blobref.StatusPendingDeletion,
+		blobref.StatusError,
+		blobref.StatusInitializing,
 	}
 	for _, s := range statuses {
 		c.deleteMatchingBlobRefs(ctx, s, c.cfg.Before)
 	}
 }
 
-func (c *Collector) deleteMatchingBlobRefs(ctx context.Context, status blobref.BlobRefStatus, olderThan time.Time) error {
+func (c *Collector) deleteMatchingBlobRefs(ctx context.Context, status blobref.Status, olderThan time.Time) error {
 	log.Infof("Garbage collecting BlobRef objects with status = %v, and older than %v", status, olderThan)
 	cursor, err := c.metaDB.ListBlobRefsByStatus(ctx, status, olderThan)
 	if err != nil {
@@ -112,7 +112,7 @@ func (c *Collector) deleteMatchingBlobRefs(ctx context.Context, status blobref.B
 		}
 		if err := c.blob.Delete(ctx, blob.ObjectPath()); err != nil {
 			log.Errorf("Blob.Delete failed for key(%v): %v", blob.Key, err)
-			if blob.Status != blobref.BlobRefStatusError {
+			if blob.Status != blobref.StatusError {
 				blob.Fail()
 				_, err := c.metaDB.UpdateBlobRef(ctx, blob)
 				if err != nil {
diff --git a/internal/pkg/metadb/blobref/blobref.go b/internal/pkg/metadb/blobref/blobref.go
index 1182f72..b3dd508 100644
--- a/internal/pkg/metadb/blobref/blobref.go
+++ b/internal/pkg/metadb/blobref/blobref.go
@@ -23,7 +23,7 @@ import (
 	"github.com/googleforgames/open-saves/internal/pkg/metadb/timestamps"
 )
 
-// BlobRefStatus represents a current blob status.
+// Status represents a current blob status.
 //
 // Life of a Blob
 //
@@ -38,23 +38,23 @@ import (
 //     delete the record |          -----[BlobRefStatusPendingDeletion]
 //                       v                  /
 //  [Delete the blob entity] <-------------/   Garbage collection
-type BlobRefStatus int16
+type Status int16
 
 const (
-	// BlobRefStatusUnknown represents internal error.
-	BlobRefStatusUnknown BlobRefStatus = iota
-	// BlobRefStatusInitializing means the blob is currently being prepared, i.e.
+	// StatusUnknown represents internal error.
+	StatusUnknown Status = iota
+	// StatusInitializing means the blob is currently being prepared, i.e.
 	// being uploaded to the blob store.
-	BlobRefStatusInitializing
-	// BlobRefStatusReady means the blob is committed and ready for use.
-	BlobRefStatusReady
-	// BlobRefStatusPendingDeletion means the blob is no longer referenced by
+	StatusInitializing
+	// StatusReady means the blob is committed and ready for use.
+	StatusReady
+	// StatusPendingDeletion means the blob is no longer referenced by
 	// any Record entities and needs to be deleted.
-	BlobRefStatusPendingDeletion
-	// BlobRefStatusError means the blob was not uploaded due to client or server
+	StatusPendingDeletion
+	// StatusError means the blob was not uploaded due to client or server
 	// errors and the corresponding record needs to be updated (either by
 	// retrying blob upload or deleting the entry).
-	BlobRefStatusError
+	StatusError
 )
 
 // BlobRef is a metadata document to keep track of blobs stored in an external blob store.
@@ -64,7 +64,7 @@ type BlobRef struct {
 	// Size is the byte size of the blob
 	Size int64
 	// Status is the current status of the blob
-	Status BlobRefStatus
+	Status Status
 	// StoreKey is the key of the store that the blob belongs to
 	StoreKey string
 	// RecordKey is the key of the record that the blob belongs to
@@ -116,7 +116,7 @@ func NewBlobRef(size int64, storeKey, recordKey string) *BlobRef {
 	b := new(BlobRef)
 	b.Key = uuid.New()
 	b.Size = size
-	b.Status = BlobRefStatusInitializing
+	b.Status = StatusInitializing
 	b.StoreKey = storeKey
 	b.RecordKey = recordKey
 	// Nanosecond is fine as it will not be returned to clients.
@@ -127,10 +127,10 @@ func NewBlobRef(size int64, storeKey, recordKey string) *BlobRef {
 // Ready changes Status to BlobRefStatusReady and updates Timestamps.
 // It returns an error if the current Status is not BlobRefStatusInitializing.
 func (b *BlobRef) Ready() error {
-	if b.Status != BlobRefStatusInitializing {
+	if b.Status != StatusInitializing {
 		return errors.New("Ready was called when Status is not Initializing")
 	}
-	b.Status = BlobRefStatusReady
+	b.Status = StatusReady
 	b.Timestamps.UpdateTimestamps(time.Nanosecond)
 	return nil
 }
@@ -138,10 +138,10 @@ func (b *BlobRef) Ready() error {
 // MarkForDeletion marks the BlobRef as BlobRefStatusPendingDeletion and updates Timestamps.
 // Returns an error if the current Status is not BlobRefStatusReady.
 func (b *BlobRef) MarkForDeletion() error {
-	if b.Status != BlobRefStatusInitializing && b.Status != BlobRefStatusReady {
+	if b.Status != StatusInitializing && b.Status != StatusReady {
 		return errors.New("MarkForDeletion was called when Status is not either Initializing or Ready")
 	}
-	b.Status = BlobRefStatusPendingDeletion
+	b.Status = StatusPendingDeletion
 	b.Timestamps.UpdateTimestamps(time.Nanosecond)
 	return nil
 }
@@ -149,7 +149,7 @@ func (b *BlobRef) MarkForDeletion() error {
 // Fail marks the BlobRef as BlobRefStatusError and updates Timestamps.
 // Any state can transition to BlobRefStatusError.
 func (b *BlobRef) Fail() error {
-	b.Status = BlobRefStatusError
+	b.Status = StatusError
 	b.Timestamps.UpdateTimestamps(time.Nanosecond)
 	return nil
 }
diff --git a/internal/pkg/metadb/metadb.go b/internal/pkg/metadb/metadb.go
index eb5f3f8..c20d308 100644
--- a/internal/pkg/metadb/metadb.go
+++ b/internal/pkg/metadb/metadb.go
@@ -438,7 +438,7 @@ func (m *MetaDB) PromoteBlobRefToCurrent(ctx context.Context, blob *blobref.Blob
 			return err
 		}
 
-		if blob.Status != blobref.BlobRefStatusReady {
+		if blob.Status != blobref.StatusReady {
 			if blob.Ready() != nil {
 				return status.Error(codes.Internal, "blob is not ready to become current")
 			}
@@ -506,7 +506,7 @@ func (m *MetaDB) DeleteBlobRef(ctx context.Context, key uuid.UUID) error {
 		if err != nil {
 			return err
 		}
-		if blob.Status == blobref.BlobRefStatusReady {
+		if blob.Status == blobref.StatusReady {
 			return status.Error(codes.FailedPrecondition, "blob is currently marked as ready. mark it for deletion first")
 		}
 		return tx.Delete(m.createBlobKey(key))
@@ -516,7 +516,7 @@ func (m *MetaDB) DeleteBlobRef(ctx context.Context, key uuid.UUID) error {
 
 // ListBlobRefsByStatus returns a cursor that iterates over BlobRefs
 // where Status = status and UpdatedAt < olderThan.
-func (m *MetaDB) ListBlobRefsByStatus(ctx context.Context, status blobref.BlobRefStatus, olderThan time.Time) (*blobref.BlobRefCursor, error) {
+func (m *MetaDB) ListBlobRefsByStatus(ctx context.Context, status blobref.Status, olderThan time.Time) (*blobref.BlobRefCursor, error) {
 	query := m.newQuery(blobKind).Filter("Status = ", int(status)).
 		Filter("Timestamps.UpdatedAt <", olderThan)
 	iter := blobref.NewCursor(m.client.Run(ctx, query))


Test patch:
diff --git a/internal/pkg/metadb/blobref/blobref_test.go b/internal/pkg/metadb/blobref/blobref_test.go
index 0b5f77c..248fc59 100644
--- a/internal/pkg/metadb/blobref/blobref_test.go
+++ b/internal/pkg/metadb/blobref/blobref_test.go
@@ -39,7 +39,7 @@ func TestBlobRef_Save(t *testing.T) {
 
 	blob := BlobRef{
 		Size:      size,
-		Status:    BlobRefStatusInitializing,
+		Status:    StatusInitializing,
 		StoreKey:  store,
 		RecordKey: record,
 	}
@@ -50,7 +50,7 @@ func TestBlobRef_Save(t *testing.T) {
 		},
 		{
 			Name:  "Status",
-			Value: int64(BlobRefStatusInitializing),
+			Value: int64(StatusInitializing),
 		},
 		{
 			Name:  "StoreKey",
@@ -84,7 +84,7 @@ func TestBlobRef_Load(t *testing.T) {
 		},
 		{
 			Name:  "Status",
-			Value: int64(BlobRefStatusReady),
+			Value: int64(StatusReady),
 		},
 		{
 			Name:  "StoreKey",
@@ -97,7 +97,7 @@ func TestBlobRef_Load(t *testing.T) {
 	}
 	expected := &BlobRef{
 		Size:      123,
-		Status:    BlobRefStatusReady,
+		Status:    StatusReady,
 		StoreKey:  store,
 		RecordKey: record,
 	}
@@ -123,7 +123,7 @@ func newInitBlob(t *testing.T) *BlobRef {
 	}
 	assert.NotEqual(t, uuid.Nil, blob.Key)
 	assert.Equal(t, size, blob.Size)
-	assert.Equal(t, BlobRefStatusInitializing, blob.Status)
+	assert.Equal(t, StatusInitializing, blob.Status)
 	assert.Equal(t, store, blob.StoreKey)
 	assert.Equal(t, record, blob.RecordKey)
 	assert.NotEmpty(t, blob.Timestamps.CreatedAt)
@@ -137,21 +137,21 @@ func TestBlobRef_LifeCycle(t *testing.T) {
 
 	// Mark for deletion
 	assert.NoError(t, blob.MarkForDeletion())
-	assert.Equal(t, BlobRefStatusPendingDeletion, blob.Status)
+	assert.Equal(t, StatusPendingDeletion, blob.Status)
 
 	// Start over
 	blob = newInitBlob(t)
 
 	// Ready
 	assert.NoError(t, blob.Ready())
-	assert.Equal(t, BlobRefStatusReady, blob.Status)
+	assert.Equal(t, StatusReady, blob.Status)
 
 	// Invalid transitions
 	assert.Error(t, blob.Ready())
 
 	// Mark for deletion
 	assert.NoError(t, blob.MarkForDeletion())
-	assert.Equal(t, BlobRefStatusPendingDeletion, blob.Status)
+	assert.Equal(t, StatusPendingDeletion, blob.Status)
 
 	// Invalid transitions
 	assert.Error(t, blob.MarkForDeletion())
@@ -167,16 +167,16 @@ func TestBlobRef_Fail(t *testing.T) {
 	blob = newInitBlob(t)
 	assert.NoError(t, blob.Fail())
 
-	blob.Status = BlobRefStatusInitializing
+	blob.Status = StatusInitializing
 	assert.NoError(t, blob.Fail())
 
-	blob.Status = BlobRefStatusPendingDeletion
+	blob.Status = StatusPendingDeletion
 	assert.NoError(t, blob.Fail())
 
-	blob.Status = BlobRefStatusReady
+	blob.Status = StatusReady
 	assert.NoError(t, blob.Fail())
 
-	blob.Status = BlobRefStatusError
+	blob.Status = StatusError
 	assert.NoError(t, blob.Fail())
 }
 
diff --git a/internal/pkg/metadb/metadb_test.go b/internal/pkg/metadb/metadb_test.go
index cfb5175..1b3ff15 100644
--- a/internal/pkg/metadb/metadb_test.go
+++ b/internal/pkg/metadb/metadb_test.go
@@ -397,7 +397,7 @@ func TestMetaDB_SimpleCreateGetDeleteBlobRef(t *testing.T) {
 	blob := &blobref.BlobRef{
 		Key:       blobKey,
 		Size:      12345,
-		Status:    blobref.BlobRefStatusInitializing,
+		Status:    blobref.StatusInitializing,
 		StoreKey:  storeKey,
 		RecordKey: recordKey,
 		Timestamps: timestamps.Timestamps{
@@ -433,7 +433,7 @@ func TestMetaDB_SimpleCreateGetDeleteBlobRef(t *testing.T) {
 		}
 		if assert.NotNil(t, promoBlob) {
 			assert.Equal(t, blobKey, promoBlob.Key)
-			assert.Equal(t, blobref.BlobRefStatusReady, promoBlob.Status)
+			assert.Equal(t, blobref.StatusReady, promoBlob.Status)
 			assert.True(t, beforePromo.Before(promoBlob.Timestamps.UpdatedAt))
 			assert.NotEqual(t, origSig, promoBlob.Timestamps.Signature)
 		}
@@ -462,7 +462,7 @@ func TestMetaDB_SimpleCreateGetDeleteBlobRef(t *testing.T) {
 			assert.Zero(t, delPendRecord.BlobSize)
 		}
 		if assert.NotNil(t, delPendBlob) {
-			assert.Equal(t, blobref.BlobRefStatusPendingDeletion, delPendBlob.Status)
+			assert.Equal(t, blobref.StatusPendingDeletion, delPendBlob.Status)
 		}
 	}
 
@@ -489,7 +489,7 @@ func TestMetaDB_SwapBlobRefs(t *testing.T) {
 
 	blob := &blobref.BlobRef{
 		Key:       uuid.New(),
-		Status:    blobref.BlobRefStatusInitializing,
+		Status:    blobref.StatusInitializing,
 		StoreKey:  store.Key,
 		RecordKey: record.Key,
 	}
@@ -502,7 +502,7 @@ func TestMetaDB_SwapBlobRefs(t *testing.T) {
 
 	newBlob := &blobref.BlobRef{
 		Key:       uuid.New(),
-		Status:    blobref.BlobRefStatusInitializing,
+		Status:    blobref.StatusInitializing,
 		StoreKey:  store.Key,
 		RecordKey: record.Key,
 	}
@@ -515,7 +515,7 @@ func TestMetaDB_SwapBlobRefs(t *testing.T) {
 		}
 		if assert.NotNil(t, newCurrBlob) {
 			assert.Equal(t, newBlob.Key, newCurrBlob.Key)
-			assert.Equal(t, blobref.BlobRefStatusReady, newCurrBlob.Status)
+			assert.Equal(t, blobref.StatusReady, newCurrBlob.Status)
 		}
 	}
 
@@ -523,7 +523,7 @@ func TestMetaDB_SwapBlobRefs(t *testing.T) {
 	if assert.NoError(t, err) {
 		if assert.NotNil(t, oldBlob) {
 			assert.Equal(t, blob.Key, oldBlob.Key)
-			assert.Equal(t, blobref.BlobRefStatusPendingDeletion, oldBlob.Status)
+			assert.Equal(t, blobref.StatusPendingDeletion, oldBlob.Status)
 		}
 	}
 
@@ -547,7 +547,7 @@ func TestMetaDB_UpdateBlobRef(t *testing.T) {
 	setupTestStoreRecord(ctx, t, metaDB, store, record)
 	blob := &blobref.BlobRef{
 		Key:       uuid.New(),
-		Status:    blobref.BlobRefStatusInitializing,
+		Status:    blobref.StatusInitializing,
 		StoreKey:  store.Key,
 		RecordKey: record.Key,
 		Timestamps: timestamps.Timestamps{
@@ -618,7 +618,7 @@ func TestMetaDB_UpdateRecordWithExternalBlobs(t *testing.T) {
 	blobKey := uuid.New()
 	blob := &blobref.BlobRef{
 		Key:       blobKey,
-		Status:    blobref.BlobRefStatusInitializing,
+		Status:    blobref.StatusInitializing,
 		StoreKey:  storeKey,
 		RecordKey: recordKey,
 		Timestamps: timestamps.Timestamps{
@@ -687,7 +687,7 @@ func TestMetaDB_UpdateRecordWithExternalBlobs(t *testing.T) {
 	blob, err = metaDB.GetBlobRef(ctx, blob.Key)
 	if assert.NoError(t, err) {
 		if assert.NotNil(t, blob) {
-			assert.Equal(t, blobref.BlobRefStatusPendingDeletion, blob.Status)
+			assert.Equal(t, blobref.StatusPendingDeletion, blob.Status)
 		}
 	}
 }
@@ -709,11 +709,11 @@ func TestMetaDB_ListBlobsByStatus(t *testing.T) {
 	}
 	setupTestStoreRecord(ctx, t, metaDB, store, record)
 
-	statuses := []blobref.BlobRefStatus{
-		blobref.BlobRefStatusError,
-		blobref.BlobRefStatusInitializing,
-		blobref.BlobRefStatusPendingDeletion,
-		blobref.BlobRefStatusPendingDeletion}
+	statuses := []blobref.Status{
+		blobref.StatusError,
+		blobref.StatusInitializing,
+		blobref.StatusPendingDeletion,
+		blobref.StatusPendingDeletion}
 	blobs := []*blobref.BlobRef{}
 	for i, s := range statuses {
 		blob := &blobref.BlobRef{
@@ -735,7 +735,7 @@ func TestMetaDB_ListBlobsByStatus(t *testing.T) {
 	}
 
 	// Should return iterator.Done and nil when not found
-	iter, err := metaDB.ListBlobRefsByStatus(ctx, blobref.BlobRefStatusError, time.Date(1999, 1, 1, 0, 0, 0, 0, time.UTC))
+	iter, err := metaDB.ListBlobRefsByStatus(ctx, blobref.StatusError, time.Date(1999, 1, 1, 0, 0, 0, 0, time.UTC))
 	assert.NoError(t, err)
 	if assert.NotNil(t, iter) {
 		b, err := iter.Next()
@@ -743,7 +743,7 @@ func TestMetaDB_ListBlobsByStatus(t *testing.T) {
 		assert.Nil(t, b)
 	}
 
-	iter, err = metaDB.ListBlobRefsByStatus(ctx, blobref.BlobRefStatusPendingDeletion, time.Date(2001, 1, 1, 0, 0, 0, 0, time.UTC))
+	iter, err = metaDB.ListBlobRefsByStatus(ctx, blobref.StatusPendingDeletion, time.Date(2001, 1, 1, 0, 0, 0, 0, time.UTC))
 	assert.NoError(t, err)
 	if assert.NotNil(t, iter) {
 		// Should return both of the PendingDeletion entries
@@ -796,7 +796,7 @@ func TestMetaDB_DeleteRecordWithExternalBlob(t *testing.T) {
 	actual, err := metaDB.GetBlobRef(ctx, blob.Key)
 	assert.NoError(t, err, "GetBlobRef should not return error")
 	if assert.NotNil(t, actual) {
-		assert.Equal(t, blobref.BlobRefStatusPendingDeletion, actual.Status)
+		assert.Equal(t, blobref.StatusPendingDeletion, actual.Status)
 	}
 }
 
diff --git a/internal/pkg/metadb/metadbtest/metadbtest_test.go b/internal/pkg/metadb/metadbtest/metadbtest_test.go
index d7d48ef..00f152b 100644
--- a/internal/pkg/metadb/metadbtest/metadbtest_test.go
+++ b/internal/pkg/metadb/metadbtest/metadbtest_test.go
@@ -86,7 +86,7 @@ func TestMetaDBTest_TestAssertEqualBlobRef(t *testing.T) {
 	original := &blobref.BlobRef{
 		Key:       uuid.MustParse("35B7DABC-9523-45E1-995A-D76F3EF29F79"),
 		Size:      12345,
-		Status:    blobref.BlobRefStatusInitializing,
+		Status:    blobref.StatusInitializing,
 		StoreKey:  "storeKey",
 		RecordKey: "recordKey",
 		Timestamps: timestamps.Timestamps{