Base model revision (#263)

* Update model interface and base struct

* Update code and tests for new Model interface

src/internal/kopia/model_store.go

@@ -90,17 +90,15 @@ func putInner(
 	ctx context.Context,
 	w repo.RepositoryWriter,
 	t modelType,
-	tags map[string]string,
 	m model.Model,
 	create bool,
 ) error {
-	// ModelStoreID does not need to be persisted in the model itself.
+	base := m.Base()
-	m.SetModelStoreID("")
 	if create {
-		m.SetStableID(model.ID(uuid.NewString()))
+		base.StableID = model.ID(uuid.NewString())
 	}
 
-	tmpTags, err := tagsForModelWithID(t, m.GetStableID(), tags)
+	tmpTags, err := tagsForModelWithID(t, base.StableID, base.Tags)
 	if err != nil {
 		// Will be wrapped at a higher layer.
 		return err
@@ -112,7 +110,7 @@ func putInner(
 		return err
 	}
 
-	m.SetModelStoreID(id)
+	base.ModelStoreID = id
 	return nil
 }
 
@@ -121,7 +119,6 @@ func putInner(
 func (ms *ModelStore) Put(
 	ctx context.Context,
 	t modelType,
-	tags map[string]string,
 	m model.Model,
 ) error {
 	err := repo.WriteSession(
@@ -129,7 +126,7 @@ func (ms *ModelStore) Put(
 		ms.wrapper.rep,
 		repo.WriteSessionOptions{Purpose: "ModelStorePut"},
 		func(innerCtx context.Context, w repo.RepositoryWriter) error {
-			err := putInner(innerCtx, w, t, tags, m, true)
+			err := putInner(innerCtx, w, t, m, true)
 			if err != nil {
 				return err
 			}
@@ -169,14 +166,19 @@ func (ms *ModelStore) GetWithModelStoreID(
 		return errors.WithStack(errNoModelStoreID)
 	}
 
-	_, err := ms.wrapper.rep.GetManifest(ctx, id, data)
+	metadata, err := ms.wrapper.rep.GetManifest(ctx, id, data)
 	// TODO(ashmrtnz): Should probably return some recognizable, non-kopia error
 	// if not found. That way kopia doesn't need to be imported to higher layers.
 	if err != nil {
 		return errors.Wrap(err, "getting model data")
 	}
 
-	data.SetModelStoreID(id)
+	base := data.Base()
+	base.Tags = metadata.Labels
+	// Hide the fact that StableID and modelType are just a tag from the user.
+	delete(base.Tags, stableIDKey)
+	delete(base.Tags, manifest.TypeLabelKey)
+	base.ModelStoreID = id
 	return nil
 }
 

src/internal/kopia/model_store_test.go

@@ -76,10 +76,12 @@ func (suite *ModelStoreIntegrationSuite) TestBadTagsErrors() {
 		assert.NoError(t, m.wrapper.Close(ctx))
 	}()
 
-	foo := &fooModel{Bar: uuid.NewString()}
 	for _, test := range table {
 		suite.T().Run(test.name, func(t *testing.T) {
-			assert.Error(t, m.Put(ctx, BackupOpModel, test.tags, foo))
+			foo := &fooModel{Bar: uuid.NewString()}
+			foo.Tags = test.tags
+
+			assert.Error(t, m.Put(ctx, BackupOpModel, foo))
 		})
 	}
 }
@@ -94,12 +96,12 @@ func (suite *ModelStoreIntegrationSuite) TestNoIDsErrors() {
 	}()
 
 	noStableID := &fooModel{Bar: uuid.NewString()}
-	noStableID.SetStableID("")
+	noStableID.Base().StableID = ""
-	noStableID.SetModelStoreID(manifest.ID(uuid.NewString()))
+	noStableID.Base().ModelStoreID = manifest.ID(uuid.NewString())
 
 	noModelStoreID := &fooModel{Bar: uuid.NewString()}
-	noModelStoreID.SetStableID(model.ID(uuid.NewString()))
+	noModelStoreID.Base().StableID = model.ID(uuid.NewString())
-	noModelStoreID.SetModelStoreID("")
+	noModelStoreID.Base().ModelStoreID = ""
 
 	assert.Error(t, m.GetWithModelStoreID(ctx, "", nil))
 }
@@ -143,25 +145,52 @@ func (suite *ModelStoreIntegrationSuite) TestPutGet() {
 	for _, test := range table {
 		suite.T().Run(test.t.String(), func(t *testing.T) {
 			foo := &fooModel{Bar: uuid.NewString()}
+			// Avoid some silly test errors from comparing nil to empty map.
+			foo.Tags = map[string]string{}
 
-			err := m.Put(ctx, test.t, nil, foo)
+			err := m.Put(ctx, test.t, foo)
 			test.check(t, err)
 
 			if test.hasErr {
 				return
 			}
 
-			require.NotEmpty(t, foo.GetModelStoreID())
+			require.NotEmpty(t, foo.Base().ModelStoreID)
-			require.NotEmpty(t, foo.GetStableID())
+			require.NotEmpty(t, foo.Base().StableID)
 
 			returned := &fooModel{}
-			err = m.GetWithModelStoreID(ctx, foo.GetModelStoreID(), returned)
+			err = m.GetWithModelStoreID(ctx, foo.Base().ModelStoreID, returned)
 			require.NoError(t, err)
 			assert.Equal(t, foo, returned)
 		})
 	}
 }
 
+func (suite *ModelStoreIntegrationSuite) TestPutGet_WithTags() {
+	ctx := context.Background()
+	t := suite.T()
+
+	m := getModelStore(t, ctx)
+	defer func() {
+		assert.NoError(t, m.wrapper.Close(ctx))
+	}()
+
+	foo := &fooModel{Bar: uuid.NewString()}
+	foo.Tags = map[string]string{
+		"bar": "baz",
+	}
+
+	require.NoError(t, m.Put(ctx, BackupOpModel, foo))
+
+	require.NotEmpty(t, foo.Base().ModelStoreID)
+	require.NotEmpty(t, foo.Base().StableID)
+
+	returned := &fooModel{}
+	err := m.GetWithModelStoreID(ctx, foo.Base().ModelStoreID, returned)
+	require.NoError(t, err)
+	assert.Equal(t, foo, returned)
+}
+
 func (suite *ModelStoreIntegrationSuite) TestGet_NotFoundErrors() {
 	ctx := context.Background()
 	t := suite.T()

src/internal/model/model.go

@@ -7,14 +7,13 @@ import (
 type ID string
 
 type Model interface {
-	GetStableID() ID
+	// Returns a handle to the BaseModel for this model.
-	SetStableID(id ID)
+	Base() *BaseModel
-	GetModelStoreID() manifest.ID
-	SetModelStoreID(id manifest.ID)
 }
 
 // BaseModel defines required fields for models stored in ModelStore. Structs
-// that wish to be stored should embed this struct.
+// 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
 	// object in the ModelStore.
@@ -25,21 +24,13 @@ type BaseModel struct {
 	// 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
 	// field should be treated as read-only by users.
-	ModelStoreID manifest.ID `json:"modelStoreID,omitempty"`
+	ModelStoreID manifest.ID `json:"-"`
+	// Tags associated with this model in the store to facilitate lookup. Tags in
+	// the struct are not serialized directly into the stored model, but are part
+	// of the metadata for the model.
+	Tags map[string]string `json:"-"`
 }
 
-func (bm *BaseModel) GetStableID() ID {
+func (bm *BaseModel) Base() *BaseModel {
-	return bm.StableID
+	return bm
-}
-
-func (bm *BaseModel) SetStableID(id ID) {
-	bm.StableID = id
-}
-
-func (bm *BaseModel) GetModelStoreID() manifest.ID {
-	return bm.ModelStoreID
-}
-
-func (bm *BaseModel) SetModelStoreID(id manifest.ID) {
-	bm.ModelStoreID = id
 }