From e35acb79ca8603617f926c26bd18e6560a31c3a9 Mon Sep 17 00:00:00 2001
From: Keepers <104464746+ryanfkeepers@users.noreply.github.com>
Date: Wed, 27 Jul 2022 14:54:58 -0600
Subject: [PATCH] rename stableID to ID (#425)

embedding a baseModel means that objects like backup
have a backup.StableID, but no backup.ID nor backup.
backupID.  This could be confusing for users.  This change
swaps the value and type names, so that baseModels
include an ID value of type StableID.
---
 src/internal/kopia/model_store.go      | 16 ++++++++--------
 src/internal/kopia/model_store_test.go | 24 ++++++++++++------------
 src/internal/model/model.go            | 10 ++++++----
 src/internal/operations/backup.go      |  4 ++--
 src/internal/operations/restore.go     |  4 ++--
 src/pkg/backup/backup.go               |  2 +-
 src/pkg/backup/backup_test.go          |  2 +-
 src/pkg/backup/details/details_test.go | 12 ++++++------
 src/pkg/repository/repository.go       |  4 ++--
 src/pkg/store/backup.go                |  4 ++--
 src/pkg/store/backup_test.go           |  8 ++++----
 src/pkg/store/mock/store_mock.go       |  4 ++--
 src/pkg/store/store.go                 |  4 ++--
 13 files changed, 50 insertions(+), 48 deletions(-)

diff --git a/src/internal/kopia/model_store.go b/src/internal/kopia/model_store.go
index cf70f0070..1ab0c47a7 100644
--- a/src/internal/kopia/model_store.go
+++ b/src/internal/kopia/model_store.go
@@ -66,7 +66,7 @@ func tagsForModel(s model.Schema, tags map[string]string) (map[string]string, er
 // type or if a bad model type is given.
 func tagsForModelWithID(
 	s model.Schema,
-	id model.ID,
+	id model.StableID,
 	tags map[string]string,
 ) (map[string]string, error) {
 	if !s.Valid() {
@@ -106,10 +106,10 @@ func putInner(
 
 	base := m.Base()
 	if create {
-		base.StableID = model.ID(uuid.NewString())
+		base.ID = model.StableID(uuid.NewString())
 	}
 
-	tmpTags, err := tagsForModelWithID(s, base.StableID, base.Tags)
+	tmpTags, err := tagsForModelWithID(s, base.ID, base.Tags)
 	if err != nil {
 		// Will be wrapped at a higher layer.
 		return err
@@ -165,7 +165,7 @@ func baseModelFromMetadata(m *manifest.EntryMetadata) (*model.BaseModel, error)
 
 	res := &model.BaseModel{
 		ModelStoreID: m.ID,
-		StableID:     model.ID(id),
+		ID:           model.StableID(id),
 		Tags:         m.Labels,
 	}
 
@@ -219,7 +219,7 @@ func (ms *ModelStore) GetIDsForType(
 func (ms *ModelStore) getModelStoreID(
 	ctx context.Context,
 	s model.Schema,
-	id model.ID,
+	id model.StableID,
 ) (manifest.ID, error) {
 	if !s.Valid() {
 		return "", errors.New("unrecognized model schema")
@@ -255,7 +255,7 @@ func (ms *ModelStore) getModelStoreID(
 func (ms *ModelStore) Get(
 	ctx context.Context,
 	s model.Schema,
-	id model.ID,
+	id model.StableID,
 	data model.Model,
 ) error {
 	if !s.Valid() {
@@ -320,7 +320,7 @@ func (ms *ModelStore) checkPrevModelVersion(
 		return errors.WithStack(errUnrecognizedSchema)
 	}
 
-	id, err := ms.getModelStoreID(ctx, s, b.StableID)
+	id, err := ms.getModelStoreID(ctx, s, b.ID)
 	if err != nil {
 		return err
 	}
@@ -405,7 +405,7 @@ func (ms *ModelStore) Update(
 // Delete deletes the model with the given StableID. Turns into a noop if id is
 // not empty but the model does not exist. Returns an error if multiple models
 // have the same StableID.
-func (ms *ModelStore) Delete(ctx context.Context, s model.Schema, id model.ID) error {
+func (ms *ModelStore) Delete(ctx context.Context, s model.Schema, id model.StableID) error {
 	if !s.Valid() {
 		return errors.WithStack(errUnrecognizedSchema)
 	}
diff --git a/src/internal/kopia/model_store_test.go b/src/internal/kopia/model_store_test.go
index c0f4d3338..8cff86cba 100644
--- a/src/internal/kopia/model_store_test.go
+++ b/src/internal/kopia/model_store_test.go
@@ -116,11 +116,11 @@ func (suite *ModelStoreIntegrationSuite) TestNoIDsErrors() {
 	theModelType := model.BackupOpSchema
 
 	noStableID := &fooModel{Bar: uuid.NewString()}
-	noStableID.StableID = ""
+	noStableID.ID = ""
 	noStableID.ModelStoreID = manifest.ID(uuid.NewString())
 
 	noModelStoreID := &fooModel{Bar: uuid.NewString()}
-	noModelStoreID.StableID = model.ID(uuid.NewString())
+	noModelStoreID.ID = model.StableID(uuid.NewString())
 	noModelStoreID.ModelStoreID = ""
 
 	assert.Error(t, suite.m.Update(suite.ctx, theModelType, noStableID))
@@ -155,11 +155,11 @@ func (suite *ModelStoreIntegrationSuite) TestBadTypeErrors() {
 	require.NoError(t, suite.m.Put(suite.ctx, model.BackupOpSchema, foo))
 
 	returned := &fooModel{}
-	assert.Error(t, suite.m.Get(suite.ctx, model.RestoreOpSchema, foo.StableID, returned))
+	assert.Error(t, suite.m.Get(suite.ctx, model.RestoreOpSchema, foo.ID, returned))
 	assert.Error(
 		t, suite.m.GetWithModelStoreID(suite.ctx, model.RestoreOpSchema, foo.ModelStoreID, returned))
 
-	assert.Error(t, suite.m.Delete(suite.ctx, model.RestoreOpSchema, foo.StableID))
+	assert.Error(t, suite.m.Delete(suite.ctx, model.RestoreOpSchema, foo.ID))
 }
 
 func (suite *ModelStoreIntegrationSuite) TestPutGet() {
@@ -204,10 +204,10 @@ func (suite *ModelStoreIntegrationSuite) TestPutGet() {
 			}
 
 			require.NotEmpty(t, foo.ModelStoreID)
-			require.NotEmpty(t, foo.StableID)
+			require.NotEmpty(t, foo.ID)
 
 			returned := &fooModel{}
-			err = suite.m.Get(suite.ctx, test.s, foo.StableID, returned)
+			err = suite.m.Get(suite.ctx, test.s, foo.ID, returned)
 			require.NoError(t, err)
 			assert.Equal(t, foo, returned)
 
@@ -230,10 +230,10 @@ func (suite *ModelStoreIntegrationSuite) TestPutGet_WithTags() {
 	require.NoError(t, suite.m.Put(suite.ctx, theModelType, foo))
 
 	require.NotEmpty(t, foo.ModelStoreID)
-	require.NotEmpty(t, foo.StableID)
+	require.NotEmpty(t, foo.ID)
 
 	returned := &fooModel{}
-	err := suite.m.Get(suite.ctx, theModelType, foo.StableID, returned)
+	err := suite.m.Get(suite.ctx, theModelType, foo.ID, returned)
 	require.NoError(t, err)
 	assert.Equal(t, foo, returned)
 
@@ -336,12 +336,12 @@ func (suite *ModelStoreIntegrationSuite) TestPutUpdate() {
 			require.NoError(t, m.Put(ctx, theModelType, foo))
 
 			oldModelID := foo.ModelStoreID
-			oldStableID := foo.StableID
+			oldStableID := foo.ID
 
 			test.mutator(foo)
 
 			require.NoError(t, m.Update(ctx, theModelType, foo))
-			assert.Equal(t, oldStableID, foo.StableID)
+			assert.Equal(t, oldStableID, foo.ID)
 
 			returned := &fooModel{}
 			require.NoError(
@@ -415,7 +415,7 @@ func (suite *ModelStoreIntegrationSuite) TestPutDelete() {
 
 	require.NoError(t, suite.m.Put(suite.ctx, theModelType, foo))
 
-	require.NoError(t, suite.m.Delete(suite.ctx, theModelType, foo.StableID))
+	require.NoError(t, suite.m.Delete(suite.ctx, theModelType, foo.ID))
 
 	returned := &fooModel{}
 	err := suite.m.GetWithModelStoreID(suite.ctx, theModelType, foo.ModelStoreID, returned)
@@ -467,7 +467,7 @@ func (suite *ModelStoreRegressionSuite) TestFailDuringWriteSessionHasNoVisibleEf
 	}()
 
 	foo := &fooModel{Bar: uuid.NewString()}
-	foo.StableID = model.ID(uuid.NewString())
+	foo.ID = model.StableID(uuid.NewString())
 	foo.ModelStoreID = manifest.ID(uuid.NewString())
 	// Avoid some silly test errors from comparing nil to empty map.
 	foo.Tags = map[string]string{}
diff --git a/src/internal/model/model.go b/src/internal/model/model.go
index a71b7fc7d..8250ce114 100644
--- a/src/internal/model/model.go
+++ b/src/internal/model/model.go
@@ -5,8 +5,10 @@ import (
 )
 
 type (
-	ID     string
-	Schema int
+	// StableID is used by BaseModel.ID to uniquely identify objects
+	// stored in the modelStore.
+	StableID string
+	Schema   int
 )
 
 //go:generate go run golang.org/x/tools/cmd/stringer -type=Schema
@@ -32,11 +34,11 @@ type Model interface {
 // that wish to be stored should embed this struct. This struct also represents
 // the common metadata the ModelStore will fill out/use.
 type BaseModel struct {
-	// StableID is an identifier that other objects can use to refer to this
+	// ID is an identifier that other objects can use to refer to this
 	// object in the ModelStore.
 	// Once generated (during Put), it is guaranteed not to change. This field
 	// should be treated as read-only by users.
-	StableID ID `json:"stableID,omitempty"`
+	ID StableID `json:"ID,omitempty"`
 	// ModelStoreID is an internal ID for the model in the store. If present it
 	// can be used for efficient lookups, but should not be used by other models
 	// to refer to this one. This field may change if the model is updated. This
diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go
index 371eec1dc..84766d897 100644
--- a/src/internal/operations/backup.go
+++ b/src/internal/operations/backup.go
@@ -34,7 +34,7 @@ type BackupOperation struct {
 type BackupResults struct {
 	stats.ReadWrites
 	stats.StartAndEndTime
-	BackupID model.ID `json:"backupID"`
+	BackupID model.StableID `json:"backupID"`
 }
 
 // NewBackupOperation constructs and validates a backup operation.
@@ -159,7 +159,7 @@ func (op *BackupOperation) createBackupModels(ctx context.Context, snapID string
 	if err != nil {
 		return errors.Wrap(err, "creating backup model")
 	}
-	op.Results.BackupID = b.StableID
+	op.Results.BackupID = b.ID
 
 	return nil
 }
diff --git a/src/internal/operations/restore.go b/src/internal/operations/restore.go
index cd32ecf54..9ab5e8c56 100644
--- a/src/internal/operations/restore.go
+++ b/src/internal/operations/restore.go
@@ -22,7 +22,7 @@ import (
 type RestoreOperation struct {
 	operation
 
-	BackupID  model.ID           `json:"backupID"`
+	BackupID  model.StableID     `json:"backupID"`
 	Results   RestoreResults     `json:"results"`
 	Selectors selectors.Selector `json:"selectors"` // todo: replace with Selectors
 	Version   string             `json:"version"`
@@ -43,7 +43,7 @@ func NewRestoreOperation(
 	kw *kopia.Wrapper,
 	sw *store.Wrapper,
 	acct account.Account,
-	backupID model.ID,
+	backupID model.StableID,
 	sel selectors.Selector,
 ) (RestoreOperation, error) {
 	op := RestoreOperation{
diff --git a/src/pkg/backup/backup.go b/src/pkg/backup/backup.go
index 63e643b49..54532fbda 100644
--- a/src/pkg/backup/backup.go
+++ b/src/pkg/backup/backup.go
@@ -58,7 +58,7 @@ func (b Backup) Headers() []string {
 func (b Backup) Values() []string {
 	return []string{
 		common.FormatTime(b.CreationTime),
-		string(b.StableID),
+		string(b.ID),
 		b.SnapshotID,
 		b.DetailsID,
 		b.Status,
diff --git a/src/pkg/backup/backup_test.go b/src/pkg/backup/backup_test.go
index e4bc47bd4..0b09fb394 100644
--- a/src/pkg/backup/backup_test.go
+++ b/src/pkg/backup/backup_test.go
@@ -30,7 +30,7 @@ func (suite *BackupSuite) TestBackup_HeadersValues() {
 
 	b := backup.Backup{
 		BaseModel: model.BaseModel{
-			StableID: model.ID("stable"),
+			ID: model.StableID("stable"),
 		},
 		CreationTime: now,
 		SnapshotID:   "snapshot",
diff --git a/src/pkg/backup/details/details_test.go b/src/pkg/backup/details/details_test.go
index a09e52a79..50757487d 100644
--- a/src/pkg/backup/details/details_test.go
+++ b/src/pkg/backup/details/details_test.go
@@ -25,7 +25,7 @@ var (
 	detailsID = uuid.NewString()
 	bu        = backup.Backup{
 		BaseModel: model.BaseModel{
-			StableID:     model.ID(uuid.NewString()),
+			ID:           model.StableID(uuid.NewString()),
 			ModelStoreID: manifest.ID(uuid.NewString()),
 		},
 		CreationTime: time.Now(),
@@ -35,7 +35,7 @@ var (
 	deets = details.Details{
 		DetailsModel: details.DetailsModel{
 			BaseModel: model.BaseModel{
-				StableID:     model.ID(detailsID),
+				ID:           model.StableID(detailsID),
 				ModelStoreID: manifest.ID(uuid.NewString()),
 			},
 		},
@@ -77,7 +77,7 @@ func (suite *StoreDetailsUnitSuite) TestGetDetails() {
 			if err != nil {
 				return
 			}
-			assert.Equal(t, deets.StableID, result.StableID)
+			assert.Equal(t, deets.ID, result.ID)
 		})
 	}
 }
@@ -104,13 +104,13 @@ func (suite *StoreDetailsUnitSuite) TestGetDetailsFromBackupID() {
 	for _, test := range table {
 		suite.T().Run(test.name, func(t *testing.T) {
 			store := &store.Wrapper{Storer: test.mock}
-			dResult, bResult, err := store.GetDetailsFromBackupID(ctx, model.ID(uuid.NewString()))
+			dResult, bResult, err := store.GetDetailsFromBackupID(ctx, model.StableID(uuid.NewString()))
 			test.expect(t, err)
 			if err != nil {
 				return
 			}
-			assert.Equal(t, deets.StableID, dResult.StableID)
-			assert.Equal(t, bu.StableID, bResult.StableID)
+			assert.Equal(t, deets.ID, dResult.ID)
+			assert.Equal(t, bu.ID, bResult.ID)
 		})
 	}
 }
diff --git a/src/pkg/repository/repository.go b/src/pkg/repository/repository.go
index 0dfdcee33..19bf9f297 100644
--- a/src/pkg/repository/repository.go
+++ b/src/pkg/repository/repository.go
@@ -152,7 +152,7 @@ func (r Repository) NewRestore(
 		r.dataLayer,
 		store.NewKopiaStore(r.modelStore),
 		r.Account,
-		model.ID(backupID),
+		model.StableID(backupID),
 		sel)
 }
 
@@ -165,5 +165,5 @@ func (r Repository) Backups(ctx context.Context) ([]backup.Backup, error) {
 // BackupDetails returns the specified backup details object
 func (r Repository) BackupDetails(ctx context.Context, backupID string) (*details.Details, *backup.Backup, error) {
 	sw := store.NewKopiaStore(r.modelStore)
-	return sw.GetDetailsFromBackupID(ctx, model.ID(backupID))
+	return sw.GetDetailsFromBackupID(ctx, model.StableID(backupID))
 }
diff --git a/src/pkg/store/backup.go b/src/pkg/store/backup.go
index 8d427c4c0..994bcc31e 100644
--- a/src/pkg/store/backup.go
+++ b/src/pkg/store/backup.go
@@ -12,7 +12,7 @@ import (
 )
 
 // GetBackup gets a single backup by id.
-func (w Wrapper) GetBackup(ctx context.Context, backupID model.ID) (*backup.Backup, error) {
+func (w Wrapper) GetBackup(ctx context.Context, backupID model.StableID) (*backup.Backup, error) {
 	b := backup.Backup{}
 	err := w.Get(ctx, model.BackupSchema, backupID, &b)
 	if err != nil {
@@ -50,7 +50,7 @@ func (w Wrapper) GetDetails(ctx context.Context, detailsID manifest.ID) (*detail
 }
 
 // GetDetailsFromBackupID retrieves the backup.Details within the specified backup.
-func (w Wrapper) GetDetailsFromBackupID(ctx context.Context, backupID model.ID) (*details.Details, *backup.Backup, error) {
+func (w Wrapper) GetDetailsFromBackupID(ctx context.Context, backupID model.StableID) (*details.Details, *backup.Backup, error) {
 	b, err := w.GetBackup(ctx, backupID)
 	if err != nil {
 		return nil, nil, err
diff --git a/src/pkg/store/backup_test.go b/src/pkg/store/backup_test.go
index 08c11864a..5bf0a913c 100644
--- a/src/pkg/store/backup_test.go
+++ b/src/pkg/store/backup_test.go
@@ -24,7 +24,7 @@ var (
 	detailsID = uuid.NewString()
 	bu        = backup.Backup{
 		BaseModel: model.BaseModel{
-			StableID:     model.ID(uuid.NewString()),
+			ID:           model.StableID(uuid.NewString()),
 			ModelStoreID: manifest.ID(uuid.NewString()),
 		},
 		CreationTime: time.Now(),
@@ -63,12 +63,12 @@ func (suite *StoreBackupUnitSuite) TestGetBackup() {
 	for _, test := range table {
 		suite.T().Run(test.name, func(t *testing.T) {
 			sm := &store.Wrapper{Storer: test.mock}
-			result, err := sm.GetBackup(ctx, model.ID(uuid.NewString()))
+			result, err := sm.GetBackup(ctx, model.StableID(uuid.NewString()))
 			test.expect(t, err)
 			if err != nil {
 				return
 			}
-			assert.Equal(t, bu.StableID, result.StableID)
+			assert.Equal(t, bu.ID, result.ID)
 		})
 	}
 }
@@ -101,7 +101,7 @@ func (suite *StoreBackupUnitSuite) TestGetBackups() {
 				return
 			}
 			assert.Equal(t, 1, len(result))
-			assert.Equal(t, bu.StableID, result[0].StableID)
+			assert.Equal(t, bu.ID, result[0].ID)
 		})
 	}
 }
diff --git a/src/pkg/store/mock/store_mock.go b/src/pkg/store/mock/store_mock.go
index 304cc9d11..37867bdaf 100644
--- a/src/pkg/store/mock/store_mock.go
+++ b/src/pkg/store/mock/store_mock.go
@@ -44,7 +44,7 @@ func unmarshal(b []byte, a any) {
 // deleter iface
 // ------------------------------------------------------------
 
-func (mms *MockModelStore) Delete(ctx context.Context, s model.Schema, id model.ID) error {
+func (mms *MockModelStore) Delete(ctx context.Context, s model.Schema, id model.StableID) error {
 	return mms.err
 }
 
@@ -59,7 +59,7 @@ func (mms *MockModelStore) DeleteWithModelStoreID(ctx context.Context, id manife
 func (mms *MockModelStore) Get(
 	ctx context.Context,
 	s model.Schema,
-	id model.ID,
+	id model.StableID,
 	data model.Model,
 ) error {
 	if mms.err != nil {
diff --git a/src/pkg/store/store.go b/src/pkg/store/store.go
index 8dee958e3..47e0ee09c 100644
--- a/src/pkg/store/store.go
+++ b/src/pkg/store/store.go
@@ -13,9 +13,9 @@ var _ Storer = &kopia.ModelStore{}
 
 type (
 	Storer interface {
-		Delete(ctx context.Context, s model.Schema, id model.ID) error
+		Delete(ctx context.Context, s model.Schema, id model.StableID) error
 		DeleteWithModelStoreID(ctx context.Context, id manifest.ID) error
-		Get(ctx context.Context, s model.Schema, id model.ID, data model.Model) error
+		Get(ctx context.Context, s model.Schema, id model.StableID, data model.Model) error
 		GetIDsForType(ctx context.Context, s model.Schema, tags map[string]string) ([]*model.BaseModel, error)
 		GetWithModelStoreID(ctx context.Context, s model.Schema, id manifest.ID, data model.Model) error
 		Put(ctx context.Context, s model.Schema, m model.Model) error