Repository: googleforgames/open-saves (go)

Issue:
Make timestamps.NewTimestamps a standalone function
NewTimestamps should create a new Timestamps instance and be an independent function without taking a receiver.

Pull request:
refactor: timestamps.New is now standalone
<!--  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**:
- Make timestamps.New a standalone function to avoid create and modify
- Rename NewTimestamps and UpdateTimestamps to New and Update to remove redundancy

**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 #246 

**Special notes for your reviewer**:


Patch:
diff --git a/internal/pkg/metadb/blobref/blobref.go b/internal/pkg/metadb/blobref/blobref.go
index b3dd508..5c73569 100644
--- a/internal/pkg/metadb/blobref/blobref.go
+++ b/internal/pkg/metadb/blobref/blobref.go
@@ -113,15 +113,15 @@ func (b *BlobRef) LoadKey(k *datastore.Key) error {
 //	- Set Status to BlobRefStatusInitializing
 //	- Set current time to Timestamps (both created and updated at)
 func NewBlobRef(size int64, storeKey, recordKey string) *BlobRef {
-	b := new(BlobRef)
-	b.Key = uuid.New()
-	b.Size = size
-	b.Status = StatusInitializing
-	b.StoreKey = storeKey
-	b.RecordKey = recordKey
-	// Nanosecond is fine as it will not be returned to clients.
-	b.Timestamps.NewTimestamps(time.Nanosecond)
-	return b
+	return &BlobRef{
+		Key:       uuid.New(),
+		Size:      size,
+		Status:    StatusInitializing,
+		StoreKey:  storeKey,
+		RecordKey: recordKey,
+		// Nanosecond is fine as it will not be returned to clients.
+		Timestamps: timestamps.New(time.Nanosecond),
+	}
 }
 
 // Ready changes Status to BlobRefStatusReady and updates Timestamps.
@@ -131,7 +131,7 @@ func (b *BlobRef) Ready() error {
 		return errors.New("Ready was called when Status is not Initializing")
 	}
 	b.Status = StatusReady
-	b.Timestamps.UpdateTimestamps(time.Nanosecond)
+	b.Timestamps.Update(time.Nanosecond)
 	return nil
 }
 
@@ -142,7 +142,7 @@ func (b *BlobRef) MarkForDeletion() error {
 		return errors.New("MarkForDeletion was called when Status is not either Initializing or Ready")
 	}
 	b.Status = StatusPendingDeletion
-	b.Timestamps.UpdateTimestamps(time.Nanosecond)
+	b.Timestamps.Update(time.Nanosecond)
 	return nil
 }
 
@@ -150,7 +150,7 @@ func (b *BlobRef) MarkForDeletion() error {
 // Any state can transition to BlobRefStatusError.
 func (b *BlobRef) Fail() error {
 	b.Status = StatusError
-	b.Timestamps.UpdateTimestamps(time.Nanosecond)
+	b.Timestamps.Update(time.Nanosecond)
 	return nil
 }
 
diff --git a/internal/pkg/metadb/metadb.go b/internal/pkg/metadb/metadb.go
index c20d308..b8f38fc 100644
--- a/internal/pkg/metadb/metadb.go
+++ b/internal/pkg/metadb/metadb.go
@@ -23,6 +23,7 @@ import (
 	"github.com/googleforgames/open-saves/internal/pkg/metadb/blobref"
 	"github.com/googleforgames/open-saves/internal/pkg/metadb/record"
 	"github.com/googleforgames/open-saves/internal/pkg/metadb/store"
+	"github.com/googleforgames/open-saves/internal/pkg/metadb/timestamps"
 	"google.golang.org/api/iterator"
 	"google.golang.org/api/option"
 	"google.golang.org/grpc/codes"
@@ -115,7 +116,7 @@ func (m *MetaDB) markBlobRefForDeletion(_ context.Context, tx *ds.Transaction,
 		return nil, status.Error(codes.FailedPrecondition, "the record doesn't have an external blob associated")
 	}
 	record.ExternalBlob = newBlobKey
-	record.Timestamps.UpdateTimestamps(timestampPrecision)
+	record.Timestamps.Update(timestampPrecision)
 	if blob.MarkForDeletion() != nil && blob.Fail() != nil {
 		return nil, status.Errorf(codes.Internal, "failed to transition the blob state for deletion: current = %v", blob.Status)
 	}
@@ -169,7 +170,7 @@ func (m *MetaDB) Disconnect(ctx context.Context) error {
 
 // CreateStore creates a new store.
 func (m *MetaDB) CreateStore(ctx context.Context, store *store.Store) (*store.Store, error) {
-	store.Timestamps.NewTimestamps(timestampPrecision)
+	store.Timestamps = timestamps.New(timestampPrecision)
 	key := m.createStoreKey(store.Key)
 	mut := ds.NewInsert(key, store)
 	if err := m.mutateSingle(ctx, mut); err != nil {
@@ -228,7 +229,7 @@ func (m *MetaDB) DeleteStore(ctx context.Context, key string) error {
 // InsertRecord creates a new Record in the store specified with storeKey.
 // Returns error if there is already a record with the same key.
 func (m *MetaDB) InsertRecord(ctx context.Context, storeKey string, record *record.Record) (*record.Record, error) {
-	record.Timestamps.NewTimestamps(timestampPrecision)
+	record.Timestamps = timestamps.New(timestampPrecision)
 	rkey := m.createRecordKey(storeKey, record.Key)
 	_, err := m.client.RunInTransaction(ctx, func(tx *ds.Transaction) error {
 		dskey := m.createStoreKey(storeKey)
@@ -292,7 +293,7 @@ func (m *MetaDB) UpdateRecord(ctx context.Context, storeKey string, key string,
 			}
 		}
 
-		toUpdate.Timestamps.UpdateTimestamps(timestampPrecision)
+		toUpdate.Timestamps.Update(timestampPrecision)
 		return m.mutateSingleInTransaction(tx, ds.NewUpdate(rkey, toUpdate))
 	})
 	if err != nil {
@@ -343,7 +344,7 @@ func (m *MetaDB) DeleteRecord(ctx context.Context, storeKey, key string) error {
 
 // InsertBlobRef inserts a new BlobRef object to the datastore.
 func (m *MetaDB) InsertBlobRef(ctx context.Context, blob *blobref.BlobRef) (*blobref.BlobRef, error) {
-	blob.Timestamps.NewTimestamps(timestampPrecision)
+	blob.Timestamps = timestamps.New(timestampPrecision)
 	rkey := m.createRecordKey(blob.StoreKey, blob.RecordKey)
 	_, err := m.client.RunInTransaction(ctx, func(tx *ds.Transaction) error {
 		if exists, err := m.recordExists(ctx, tx, rkey); err != nil {
@@ -420,7 +421,7 @@ func (m *MetaDB) PromoteBlobRefToCurrent(ctx context.Context, blob *blobref.Blob
 		if record.ExternalBlob == uuid.Nil {
 			// Simply add the new blob if previously didn't have a blob
 			record.Blob = nil
-			record.Timestamps.UpdateTimestamps(timestampPrecision)
+			record.Timestamps.Update(timestampPrecision)
 		} else {
 			// Mark previous blob for deletion
 			oldBlob, err := m.getBlobRef(ctx, tx, record.ExternalBlob)
@@ -475,7 +476,7 @@ func (m *MetaDB) RemoveBlobFromRecord(ctx context.Context, storeKey string, reco
 		if record.ExternalBlob == uuid.Nil && len(record.Blob) > 0 {
 			// Delete the inline blob
 			record.Blob = nil
-			record.Timestamps.UpdateTimestamps(timestampPrecision)
+			record.Timestamps.Update(timestampPrecision)
 			return m.mutateSingleInTransaction(tx, ds.NewUpdate(rkey, record))
 		}
 
diff --git a/internal/pkg/metadb/timestamps/timestamps.go b/internal/pkg/metadb/timestamps/timestamps.go
index 962f00b..ab9fbd7 100644
--- a/internal/pkg/metadb/timestamps/timestamps.go
+++ b/internal/pkg/metadb/timestamps/timestamps.go
@@ -43,19 +43,22 @@ type Timestamps struct {
 // Assert Timestamps implements the PropertyLoadSaver interface.
 var _ datastore.PropertyLoadSaver = new(Timestamps)
 
-// NewTimestamps sets CreatedAt and UpdatedAt to time.Now() and Signature to uuid.New().
-func (t *Timestamps) NewTimestamps(d time.Duration) {
+// New returns a new Timestamps instance with
+// CreatedAt and UpdatedAt set to time.Now() and Signature set to uuid.New().
+func New(d time.Duration) Timestamps {
 	// This needs to be the lowest precision of all backend that MetaDB currently
 	// supports. Currently it's 1 microsecond.
 	// Datastore: 1 microsecond: https://cloud.google.com/datastore/docs/concepts/entities#date_and_time
 	now := time.Now().UTC().Truncate(d)
-	t.CreatedAt = now
-	t.UpdatedAt = now
-	t.Signature = uuid.New()
+	return Timestamps{
+		CreatedAt: now,
+		UpdatedAt: now,
+		Signature: uuid.New(),
+	}
 }
 
-// UpdateTimestamps updates the UpdatedAt and Signature fields with time.Now() and uuid.New().
-func (t *Timestamps) UpdateTimestamps(d time.Duration) {
+// Update updates the UpdatedAt and Signature fields with time.Now() and uuid.New().
+func (t *Timestamps) Update(d time.Duration) {
 	t.UpdatedAt = time.Now().UTC().Truncate(d)
 	t.Signature = uuid.New()
 }


Test patch:
diff --git a/internal/pkg/metadb/metadb_test.go b/internal/pkg/metadb/metadb_test.go
index 1b3ff15..043be6e 100644
--- a/internal/pkg/metadb/metadb_test.go
+++ b/internal/pkg/metadb/metadb_test.go
@@ -230,7 +230,7 @@ func TestMetaDB_SimpleCreateGetDeleteRecord(t *testing.T) {
 	expected := cloneRecord(record)
 
 	createdRecord, err := metaDB.InsertRecord(ctx, storeKey, record)
-	expected.Timestamps.NewTimestamps(timestampPrecision)
+	expected.Timestamps = timestamps.New(timestampPrecision)
 	// Copy the new signature as we cannot generate the same UUID.
 	expected.Timestamps.Signature = createdRecord.Timestamps.Signature
 	if err != nil {
@@ -316,7 +316,7 @@ func TestMetaDB_UpdateRecord(t *testing.T) {
 		OwnerID:    "record owner",
 		Tags:       []string{"abc", "def"},
 	}
-	work.Timestamps.NewTimestamps(timestampPrecision)
+	work.Timestamps = timestamps.New(timestampPrecision)
 	expected := cloneRecord(work)
 
 	updated, err := metaDB.UpdateRecord(ctx, storeKey, recordKey,
@@ -330,7 +330,7 @@ func TestMetaDB_UpdateRecord(t *testing.T) {
 	expected.Tags = append(expected.Tags, "ghi")
 	work.OwnerID = "NewOwner"
 	expected.OwnerID = work.OwnerID
-	expected.Timestamps.NewTimestamps(timestampPrecision)
+	expected.Timestamps = timestamps.New(timestampPrecision)
 
 	// Make sure UpdateRecord doesn't update CreatedAt
 	work.Timestamps.CreatedAt = time.Unix(0, 0)
diff --git a/internal/pkg/metadb/timestamps/timestamps_test.go b/internal/pkg/metadb/timestamps/timestamps_test.go
index 3805961..6cb4be6 100644
--- a/internal/pkg/metadb/timestamps/timestamps_test.go
+++ b/internal/pkg/metadb/timestamps/timestamps_test.go
@@ -31,8 +31,7 @@ func now() time.Time {
 
 func TestTimestamps_NewTimestamps(t *testing.T) {
 	beforeNew := now()
-	var ts Timestamps
-	ts.NewTimestamps(precision)
+	ts := New(precision)
 	afterNew := now()
 	assert.NotNil(t, ts)
 
@@ -52,14 +51,13 @@ func TestTimestamps_NewTimestamps(t *testing.T) {
 }
 
 func TestTimestamps_Update(t *testing.T) {
-	var ts Timestamps
-	ts.NewTimestamps(precision)
+	ts := New(precision)
 	ocreated := ts.CreatedAt
 	oupdated := ts.UpdatedAt
 	osignature := ts.Signature
 	assert.NotNil(t, ts)
 	beforeUpdate := now()
-	ts.UpdateTimestamps(precision)
+	ts.Update(precision)
 	afterUpdate := now()
 
 	assert.Same(t, time.UTC, ts.UpdatedAt.Location())
@@ -74,8 +72,7 @@ func TestTimestamps_Update(t *testing.T) {
 }
 
 func TestTimestamps_Save(t *testing.T) {
-	var ts Timestamps
-	ts.NewTimestamps(precision)
+	ts := New(precision)
 	actual, err := ts.Save()
 	assert.NoError(t, err)
 	expected := []datastore.Property{