Repository: googleforgames/open-saves
Pull request #261 (2021-06-16 21:57:34)

refactor: Separate blobref.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**:
Separte blobref.Status as a new type to later use in ChunkRef.

**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/pkg/metadb/blobref/blobref.go b/internal/pkg/metadb/blobref/blobref.go
index 1812fe2..96fec83 100644
--- a/internal/pkg/metadb/blobref/blobref.go
+++ b/internal/pkg/metadb/blobref/blobref.go
@@ -15,47 +15,11 @@
 package blobref
 
 import (
-	"errors"
-
 	"cloud.google.com/go/datastore"
 	"github.com/google/uuid"
 	"github.com/googleforgames/open-saves/internal/pkg/metadb/timestamps"
 )
 
-// Status represents a current blob status.
-//
-// Life of a Blob
-//
-// [New Record created] --> [new BlobRef entity with BlobRefStatusInitializing]
-//                         /               \
-//                        / fail            \  success
-//                       v                   v
-//                [BlobRefStatusError]       [BlobRefStatusReady]
-//                       |       x            |
-//      Upload new blob  |        \ fail      | Record deleted or new blob uploaded
-//            or         |         \          v
-//     delete the record |          -----[BlobRefStatusPendingDeletion]
-//                       v                  /
-//  [Delete the blob entity] <-------------/   Garbage collection
-type Status int16
-
-const (
-	// StatusUnknown represents internal error.
-	StatusUnknown Status = iota
-	// StatusInitializing means the blob is currently being prepared, i.e.
-	// being uploaded to the blob store.
-	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.
-	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).
-	StatusError
-)
-
 // BlobRef is a metadata document to keep track of blobs stored in an external blob store.
 type BlobRef struct {
 	// Key is the primary key for the blob entry
@@ -63,7 +27,7 @@ type BlobRef struct {
 	// Size is the byte size of the blob
 	Size int64
 	// Status is the current status of the blob
-	Status 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
@@ -122,36 +86,6 @@ 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 != StatusInitializing {
-		return errors.New("Ready was called when Status is not Initializing")
-	}
-	b.Status = StatusReady
-	b.Timestamps.Update()
-	return nil
-}
-
-// 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 != StatusInitializing && b.Status != StatusReady {
-		return errors.New("MarkForDeletion was called when Status is not either Initializing or Ready")
-	}
-	b.Status = StatusPendingDeletion
-	b.Timestamps.Update()
-	return nil
-}
-
-// Fail marks the BlobRef as BlobRefStatusError and updates Timestamps.
-// Any state can transition to BlobRefStatusError.
-func (b *BlobRef) Fail() error {
-	b.Status = StatusError
-	b.Timestamps.Update()
-	return nil
-}
-
 // ObjectPath returns an object path for the backend blob storage.
 func (b *BlobRef) ObjectPath() string {
 	return b.Key.String()
diff --git a/internal/pkg/metadb/blobref/status.go b/internal/pkg/metadb/blobref/status.go
new file mode 100644
index 0000000..5b973d0
--- /dev/null
+++ b/internal/pkg/metadb/blobref/status.go
@@ -0,0 +1,82 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package blobref
+
+import "errors"
+
+// Status represents a current blob status.
+//
+// Life of a Blob
+//
+// [New Record created] --> [new Status entity with StatusInitializing]
+//                         /               \
+//                        / fail            \  success
+//                       v                   v
+//                [StatusError]            [StatusReady]
+//                       |       x            |
+//      Upload new blob  |        \ fail      | Record deleted or new blob uploaded
+//            or         |         \          v
+//     delete the record |          -------[StatusPendingDeletion]
+//                       v                  /
+//  [Delete the blob entity] <-------------/   Garbage collection
+type Status int16
+
+const (
+	// StatusUnknown represents internal error.
+	StatusUnknown Status = iota
+	// StatusInitializing means the blob is currently being prepared, i.e.
+	// being uploaded to the blob store.
+	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.
+	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).
+	StatusError
+)
+
+var (
+	ErrReadyNotInitializing    = errors.New("Ready was called when Status is not Initializing")
+	ErrMarkForDeletionNotReady = errors.New("MarkForDeletion was called when Status is not either Initializing or Ready")
+)
+
+// Ready changes Status to StatusReady.
+// It returns an error if the current Status is not StatusInitializing.
+func (s *Status) Ready() error {
+	if *s != StatusInitializing {
+		return ErrReadyNotInitializing
+	}
+	*s = StatusReady
+	return nil
+}
+
+// MarkForDeletion marks the Status as StatusPendingDeletion.
+// Returns an error if the current Status is not StatusReady.
+func (s *Status) MarkForDeletion() error {
+	if *s != StatusInitializing && *s != StatusReady {
+		return ErrMarkForDeletionNotReady
+	}
+	*s = StatusPendingDeletion
+	return nil
+}
+
+// Fail marks the Status as StatusError.
+// Any state can transition to StatusError.
+func (s *Status) Fail() {
+	*s = StatusError
+}
diff --git a/internal/pkg/metadb/metadb.go b/internal/pkg/metadb/metadb.go
index 9b42d45..f99cfe4 100644
--- a/internal/pkg/metadb/metadb.go
+++ b/internal/pkg/metadb/metadb.go
@@ -116,7 +116,8 @@ func (m *MetaDB) markBlobRefForDeletion(_ context.Context, tx *ds.Transaction,
 	}
 	record.ExternalBlob = newBlobKey
 	record.Timestamps.Update()
-	if blob.MarkForDeletion() != nil && blob.Fail() != nil {
+	if blob.MarkForDeletion() != nil {
+		blob.Fail()
 		return nil, status.Errorf(codes.Internal, "failed to transition the blob state for deletion: current = %v", blob.Status)
 	}
 	_, err := tx.Mutate(ds.NewUpdate(m.createBlobKey(blob.Key), blob))
@@ -442,6 +443,7 @@ func (m *MetaDB) PromoteBlobRefToCurrent(ctx context.Context, blob *blobref.Blob
 			if blob.Ready() != nil {
 				return status.Error(codes.Internal, "blob is not ready to become current")
 			}
+			blob.Timestamps.Update()
 			if _, err := tx.Mutate(ds.NewUpdate(m.createBlobKey(blob.Key), blob)); err != nil {
 				return err
 			}


Test patch:
diff --git a/internal/pkg/metadb/blobref/blobref_test.go b/internal/pkg/metadb/blobref/blobref_test.go
index 248fc59..279b2e1 100644
--- a/internal/pkg/metadb/blobref/blobref_test.go
+++ b/internal/pkg/metadb/blobref/blobref_test.go
@@ -108,78 +108,6 @@ func TestBlobRef_Load(t *testing.T) {
 	}
 }
 
-func newInitBlob(t *testing.T) *BlobRef {
-	const (
-		size   = int64(4)
-		name   = "abc"
-		store  = "store"
-		record = "record"
-	)
-
-	// Initialize
-	blob := NewBlobRef(size, store, record)
-	if blob == nil {
-		t.Fatal("NewBlobRef returned nil.")
-	}
-	assert.NotEqual(t, uuid.Nil, blob.Key)
-	assert.Equal(t, size, blob.Size)
-	assert.Equal(t, StatusInitializing, blob.Status)
-	assert.Equal(t, store, blob.StoreKey)
-	assert.Equal(t, record, blob.RecordKey)
-	assert.NotEmpty(t, blob.Timestamps.CreatedAt)
-	assert.NotEmpty(t, blob.Timestamps.UpdatedAt)
-	assert.NotEmpty(t, blob.Timestamps.Signature)
-	return blob
-}
-
-func TestBlobRef_LifeCycle(t *testing.T) {
-	blob := newInitBlob(t)
-
-	// Mark for deletion
-	assert.NoError(t, blob.MarkForDeletion())
-	assert.Equal(t, StatusPendingDeletion, blob.Status)
-
-	// Start over
-	blob = newInitBlob(t)
-
-	// Ready
-	assert.NoError(t, blob.Ready())
-	assert.Equal(t, StatusReady, blob.Status)
-
-	// Invalid transitions
-	assert.Error(t, blob.Ready())
-
-	// Mark for deletion
-	assert.NoError(t, blob.MarkForDeletion())
-	assert.Equal(t, StatusPendingDeletion, blob.Status)
-
-	// Invalid transitions
-	assert.Error(t, blob.MarkForDeletion())
-	assert.Error(t, blob.Ready())
-}
-
-func TestBlobRef_Fail(t *testing.T) {
-	blob := new(BlobRef)
-
-	// Fail should work for BlobStatusUnknown too.
-	assert.NoError(t, blob.Fail())
-
-	blob = newInitBlob(t)
-	assert.NoError(t, blob.Fail())
-
-	blob.Status = StatusInitializing
-	assert.NoError(t, blob.Fail())
-
-	blob.Status = StatusPendingDeletion
-	assert.NoError(t, blob.Fail())
-
-	blob.Status = StatusReady
-	assert.NoError(t, blob.Fail())
-
-	blob.Status = StatusError
-	assert.NoError(t, blob.Fail())
-}
-
 func TestBlobRef_GetObjectPath(t *testing.T) {
 	blob := NewBlobRef(0, "", "")
 
diff --git a/internal/pkg/metadb/blobref/status_test.go b/internal/pkg/metadb/blobref/status_test.go
new file mode 100644
index 0000000..eed8f6f
--- /dev/null
+++ b/internal/pkg/metadb/blobref/status_test.go
@@ -0,0 +1,75 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package blobref
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestStatus_LifeCycle(t *testing.T) {
+	status := StatusInitializing
+
+	// Mark for deletion
+	assert.NoError(t, status.MarkForDeletion())
+	assert.Equal(t, StatusPendingDeletion, status)
+
+	// Start over
+	status = StatusInitializing
+
+	// Ready
+	assert.NoError(t, status.Ready())
+	assert.Equal(t, StatusReady, status)
+
+	// Invalid transitions
+	assert.Error(t, status.Ready())
+
+	// Mark for deletion
+	assert.NoError(t, status.MarkForDeletion())
+	assert.Equal(t, StatusPendingDeletion, status)
+
+	// Invalid transitions
+	assert.Error(t, status.MarkForDeletion())
+	assert.Error(t, status.Ready())
+}
+
+func TestStatus_Fail(t *testing.T) {
+	status := StatusUnknown
+
+	// Fail should work for BlobStatusUnknown too.
+	status.Fail()
+	assert.Equal(t, StatusError, status)
+
+	status = StatusInitializing
+	status.Fail()
+	assert.Equal(t, StatusError, status)
+
+	status = StatusInitializing
+	status.Fail()
+	assert.Equal(t, StatusError, status)
+
+	status = StatusPendingDeletion
+	status.Fail()
+	assert.Equal(t, StatusError, status)
+
+	status = StatusReady
+	status.Fail()
+	assert.Equal(t, StatusError, status)
+
+	status = StatusError
+	status.Fail()
+	assert.Equal(t, StatusError, status)
+}
diff --git a/internal/pkg/metadb/metadb_test.go b/internal/pkg/metadb/metadb_test.go
index 4c7c0f3..107a4f4 100644
--- a/internal/pkg/metadb/metadb_test.go
+++ b/internal/pkg/metadb/metadb_test.go
@@ -557,7 +557,7 @@ func TestMetaDB_UpdateBlobRef(t *testing.T) {
 
 	setupTestBlobRef(ctx, t, metaDB, blob)
 
-	assert.NoError(t, blob.Fail())
+	blob.Fail()
 	blob.Timestamps.UpdatedAt = time.Unix(234, 0)
 
 	updatedBlob, err := metaDB.UpdateBlobRef(ctx, blob)