diff --git a/src/internal/kopia/model_store.go b/src/internal/kopia/model_store.go index c6dd906f5..7efe687f2 100644 --- a/src/internal/kopia/model_store.go +++ b/src/internal/kopia/model_store.go @@ -2,6 +2,7 @@ package kopia import ( "context" + "strconv" "github.com/google/uuid" "github.com/kopia/kopia/repo" @@ -11,7 +12,11 @@ import ( "github.com/alcionai/corso/src/internal/model" ) -const stableIDKey = "stableID" +const ( + stableIDKey = "stableID" + modelVersionKey = "storeVersion" + globalModelVersion = 1 +) var ( ErrNotFound = errors.New("not found") @@ -27,12 +32,14 @@ func NewModelStore(c *conn) (*ModelStore, error) { return nil, errors.Wrap(err, "creating ModelStore") } - return &ModelStore{c: c}, nil + return &ModelStore{c: c, modelVersion: globalModelVersion}, nil } // ModelStore must not be accessed after the given KopiaWrapper is closed. type ModelStore struct { c *conn + // Stash a reference here so testing can easily change it. + modelVersion int } func (ms *ModelStore) Close(ctx context.Context) error { @@ -70,6 +77,7 @@ func tagsForModel(s model.Schema, tags map[string]string) (map[string]string, er func tagsForModelWithID( s model.Schema, id model.StableID, + version int, tags map[string]string, ) (map[string]string, error) { if !s.Valid() { @@ -91,6 +99,12 @@ func tagsForModelWithID( res[stableIDKey] = string(id) + if _, ok := res[modelVersionKey]; ok { + return nil, errors.WithStack(errBadTagKey) + } + + res[modelVersionKey] = strconv.Itoa(version) + return res, nil } @@ -112,7 +126,7 @@ func putInner( base.ID = model.StableID(uuid.NewString()) } - tmpTags, err := tagsForModelWithID(s, base.ID, base.Tags) + tmpTags, err := tagsForModelWithID(s, base.ID, base.Version, base.Tags) if err != nil { // Will be wrapped at a higher layer. return err @@ -140,6 +154,8 @@ func (ms *ModelStore) Put( return errors.WithStack(errUnrecognizedSchema) } + m.Base().Version = ms.modelVersion + err := repo.WriteSession( ctx, ms.c, @@ -159,22 +175,45 @@ func (ms *ModelStore) Put( func stripHiddenTags(tags map[string]string) { delete(tags, stableIDKey) + delete(tags, modelVersionKey) delete(tags, manifest.TypeLabelKey) } -func baseModelFromMetadata(m *manifest.EntryMetadata) (*model.BaseModel, error) { +func (ms ModelStore) populateBaseModelFromMetadata( + base *model.BaseModel, + m *manifest.EntryMetadata, +) error { id, ok := m.Labels[stableIDKey] if !ok { - return nil, errors.WithStack(errNoStableID) + return errors.WithStack(errNoStableID) } - res := &model.BaseModel{ - ModelStoreID: m.ID, - ID: model.StableID(id), - Tags: m.Labels, + v, err := strconv.Atoi(m.Labels[modelVersionKey]) + if err != nil { + return errors.Wrap(err, "parsing model version") } - stripHiddenTags(res.Tags) + if v != ms.modelVersion { + return errors.Errorf("bad model version %s", m.Labels[modelVersionKey]) + } + + base.ModelStoreID = m.ID + base.ID = model.StableID(id) + base.Version = v + base.Tags = m.Labels + + stripHiddenTags(base.Tags) + + return nil +} + +func (ms ModelStore) baseModelFromMetadata( + m *manifest.EntryMetadata, +) (*model.BaseModel, error) { + res := &model.BaseModel{} + if err := ms.populateBaseModelFromMetadata(res, m); err != nil { + return nil, err + } return res, nil } @@ -208,7 +247,7 @@ func (ms *ModelStore) GetIDsForType( res := make([]*model.BaseModel, 0, len(metadata)) for _, m := range metadata { - bm, err := baseModelFromMetadata(m) + bm, err := ms.baseModelFromMetadata(m) if err != nil { return nil, errors.Wrap(err, "parsing model metadata") } @@ -304,12 +343,10 @@ func (ms *ModelStore) GetWithModelStoreID( return errors.WithStack(errModelTypeMismatch) } - base := data.Base() - base.Tags = metadata.Labels - stripHiddenTags(base.Tags) - base.ModelStoreID = id - - return nil + return errors.Wrap( + ms.populateBaseModelFromMetadata(data.Base(), metadata), + "getting model by ID", + ) } // checkPrevModelVersion compares the ModelType and ModelStoreID in this model @@ -368,6 +405,8 @@ func (ms *ModelStore) Update( return errors.WithStack(errNoModelStoreID) } + base.Version = ms.modelVersion + // TODO(ashmrtnz): Can remove if bottleneck. if err := ms.checkPrevModelVersion(ctx, s, base); err != nil { return err diff --git a/src/internal/kopia/model_store_test.go b/src/internal/kopia/model_store_test.go index 47c79b445..2a7a04055 100644 --- a/src/internal/kopia/model_store_test.go +++ b/src/internal/kopia/model_store_test.go @@ -29,7 +29,7 @@ func getModelStore(t *testing.T, ctx context.Context) *ModelStore { c, err := openKopiaRepo(t, ctx) require.NoError(t, err) - return &ModelStore{c} + return &ModelStore{c: c, modelVersion: globalModelVersion} } // --------------- @@ -101,6 +101,12 @@ func (suite *ModelStoreIntegrationSuite) TestBadTagsErrors() { manifest.TypeLabelKey: "foo", }, }, + { + name: "storeVersion", + tags: map[string]string{ + manifest.TypeLabelKey: "foo", + }, + }, } for _, test := range table { @@ -204,6 +210,23 @@ func (suite *ModelStoreIntegrationSuite) TestBadTypeErrors() { ) } +func (suite *ModelStoreIntegrationSuite) TestPutGetBadVersion() { + t := suite.T() + schema := model.BackupOpSchema + foo := &fooModel{Bar: uuid.NewString()} + // Avoid some silly test errors from comparing nil to empty map. + foo.Tags = map[string]string{} + + err := suite.m.Put(suite.ctx, schema, foo) + require.NoError(t, err) + + suite.m.modelVersion = 42 + + returned := &fooModel{} + err = suite.m.Get(suite.ctx, schema, foo.ID, returned) + assert.Error(t, err) +} + func (suite *ModelStoreIntegrationSuite) TestPutGet() { table := []struct { s model.Schema @@ -247,6 +270,7 @@ func (suite *ModelStoreIntegrationSuite) TestPutGet() { require.NotEmpty(t, foo.ModelStoreID) require.NotEmpty(t, foo.ID) + require.Equal(t, globalModelVersion, foo.Version) returned := &fooModel{} err = suite.m.Get(suite.ctx, test.s, foo.ID, returned) @@ -340,6 +364,22 @@ func (suite *ModelStoreIntegrationSuite) TestGet_NotFoundErrors() { t, suite.m.GetWithModelStoreID(suite.ctx, model.BackupOpSchema, "baz", nil), ErrNotFound) } +func (suite *ModelStoreIntegrationSuite) TestPutGetOfTypeBadVersion() { + t := suite.T() + schema := model.BackupOpSchema + + foo := &fooModel{Bar: uuid.NewString()} + + err := suite.m.Put(suite.ctx, schema, foo) + require.NoError(t, err) + + suite.m.modelVersion = 42 + + ids, err := suite.m.GetIDsForType(suite.ctx, schema, nil) + assert.Error(t, err) + assert.Empty(t, ids) +} + func (suite *ModelStoreIntegrationSuite) TestPutGetOfType() { table := []struct { s model.Schema @@ -527,12 +567,14 @@ func (suite *ModelStoreIntegrationSuite) TestPutUpdate() { name: "NoTags", mutator: func(m *fooModel) { m.Bar = "baz" + m.Version = 42 }, }, { name: "WithTags", mutator: func(m *fooModel) { m.Bar = "baz" + m.Version = 42 m.Tags = map[string]string{ "a": "42", } @@ -558,11 +600,15 @@ func (suite *ModelStoreIntegrationSuite) TestPutUpdate() { oldModelID := foo.ModelStoreID oldStableID := foo.ID + oldVersion := foo.Version test.mutator(foo) require.NoError(t, m.Update(ctx, theModelType, foo)) assert.Equal(t, oldStableID, foo.ID) + // The version in the model store has not changed so we get the old + // version back. + assert.Equal(t, oldVersion, foo.Version) returned := &fooModel{} require.NoError( @@ -571,7 +617,8 @@ func (suite *ModelStoreIntegrationSuite) TestPutUpdate() { ids, err := m.GetIDsForType(ctx, theModelType, nil) require.NoError(t, err) - assert.Len(t, ids, 1) + require.Len(t, ids, 1) + assert.Equal(t, globalModelVersion, ids[0].Version) if oldModelID == foo.ModelStoreID { // Unlikely, but we don't control ModelStoreID generation and can't diff --git a/src/internal/model/model.go b/src/internal/model/model.go index 8250ce114..5ac1b6042 100644 --- a/src/internal/model/model.go +++ b/src/internal/model/model.go @@ -44,6 +44,9 @@ type BaseModel struct { // 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:"-"` + // Version is a version number that can help track changes across models. + // TODO(ashmrtn): Reference version control documentation. + Version int `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.