From 9ec638f763b739e4c2981cb3966ed5765b3b3343 Mon Sep 17 00:00:00 2001 From: ashmrtn <3891298+ashmrtn@users.noreply.github.com> Date: Tue, 8 Aug 2023 17:53:31 -0700 Subject: [PATCH] Backup list filtering (#3979) Now that we store backup models for backups that had errors filter them out during backup list. This ensures behavior doesn't change when we merge PRs for making and labeling backup models with the assist backup tag --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [x] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Issue(s) * #3973 #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/pkg/repository/repository.go | 34 ++- .../repository/repository_unexported_test.go | 220 ++++++++++++++++++ 2 files changed, 251 insertions(+), 3 deletions(-) diff --git a/src/pkg/repository/repository.go b/src/pkg/repository/repository.go index a604a5ac5..3f8813406 100644 --- a/src/pkg/repository/repository.go +++ b/src/pkg/repository/repository.go @@ -449,7 +449,7 @@ func getBackup( return b, nil } -// BackupsByID lists backups by ID. Returns as many backups as possible with +// Backups lists backups by ID. Returns as many backups as possible with // errors for the backups it was unable to retrieve. func (r repository) Backups(ctx context.Context, ids []string) ([]*backup.Backup, *fault.Bus) { var ( @@ -472,10 +472,38 @@ func (r repository) Backups(ctx context.Context, ids []string) ([]*backup.Backup return bups, errs } -// backups lists backups in a repository +// BackupsByTag lists all backups in a repository that contain all the tags +// specified. func (r repository) BackupsByTag(ctx context.Context, fs ...store.FilterOption) ([]*backup.Backup, error) { sw := store.NewKopiaStore(r.modelStore) - return sw.GetBackups(ctx, fs...) + return backupsByTag(ctx, sw, fs) +} + +// backupsByTag returns all backups matching all provided tags. +// +// TODO(ashmrtn): This exists mostly for testing, but we could restructure the +// code in this file so there's a more elegant mocking solution. +func backupsByTag( + ctx context.Context, + sw store.BackupWrapper, + fs []store.FilterOption, +) ([]*backup.Backup, error) { + bs, err := sw.GetBackups(ctx, fs...) + if err != nil { + return nil, clues.Stack(err) + } + + // Filter out assist backup bases as they're considered incomplete and we + // haven't been displaying them before now. + res := make([]*backup.Backup, 0, len(bs)) + + for _, b := range bs { + if t := b.Tags[model.BackupTypeTag]; t != model.AssistBackup { + res = append(res, b) + } + } + + return res, nil } // BackupDetails returns the specified backup.Details diff --git a/src/pkg/repository/repository_unexported_test.go b/src/pkg/repository/repository_unexported_test.go index 0e600157d..344567f74 100644 --- a/src/pkg/repository/repository_unexported_test.go +++ b/src/pkg/repository/repository_unexported_test.go @@ -30,6 +30,41 @@ import ( "github.com/alcionai/corso/src/pkg/store/mock" ) +// --------------------------------------------------------------------------- +// Mocks +// --------------------------------------------------------------------------- + +type mockBackupList struct { + backups []*backup.Backup + err error + check func(fs []store.FilterOption) +} + +func (mbl mockBackupList) GetBackup( + ctx context.Context, + backupID model.StableID, +) (*backup.Backup, error) { + return nil, clues.New("not implemented") +} + +func (mbl mockBackupList) DeleteBackup( + ctx context.Context, + backupID model.StableID, +) error { + return clues.New("not implemented") +} + +func (mbl mockBackupList) GetBackups( + ctx context.Context, + filters ...store.FilterOption, +) ([]*backup.Backup, error) { + if mbl.check != nil { + mbl.check(filters) + } + + return mbl.backups, mbl.err +} + // --------------------------------------------------------------------------- // Unit // --------------------------------------------------------------------------- @@ -100,6 +135,191 @@ func (suite *RepositoryBackupsUnitSuite) TestGetBackup() { } } +func (suite *RepositoryBackupsUnitSuite) TestBackupsByTag() { + unlabeled1 := &backup.Backup{ + BaseModel: model.BaseModel{ + ID: model.StableID(uuid.NewString()), + }, + } + unlabeled2 := &backup.Backup{ + BaseModel: model.BaseModel{ + ID: model.StableID(uuid.NewString()), + }, + } + + merge1 := &backup.Backup{ + BaseModel: model.BaseModel{ + ID: model.StableID(uuid.NewString()), + Tags: map[string]string{ + model.BackupTypeTag: model.MergeBackup, + }, + }, + } + merge2 := &backup.Backup{ + BaseModel: model.BaseModel{ + ID: model.StableID(uuid.NewString()), + Tags: map[string]string{ + model.BackupTypeTag: model.MergeBackup, + }, + }, + } + + assist1 := &backup.Backup{ + BaseModel: model.BaseModel{ + ID: model.StableID(uuid.NewString()), + Tags: map[string]string{ + model.BackupTypeTag: model.AssistBackup, + }, + }, + } + assist2 := &backup.Backup{ + BaseModel: model.BaseModel{ + ID: model.StableID(uuid.NewString()), + Tags: map[string]string{ + model.BackupTypeTag: model.AssistBackup, + }, + }, + } + + table := []struct { + name string + getBackups []*backup.Backup + filters []store.FilterOption + listErr error + expectErr assert.ErrorAssertionFunc + expect []*backup.Backup + }{ + { + name: "UnlabeledOnly", + getBackups: []*backup.Backup{ + unlabeled1, + unlabeled2, + }, + expectErr: assert.NoError, + expect: []*backup.Backup{ + unlabeled1, + unlabeled2, + }, + }, + { + name: "MergeOnly", + getBackups: []*backup.Backup{ + merge1, + merge2, + }, + expectErr: assert.NoError, + expect: []*backup.Backup{ + merge1, + merge2, + }, + }, + { + name: "AssistOnly", + getBackups: []*backup.Backup{ + assist1, + assist2, + }, + expectErr: assert.NoError, + }, + { + name: "UnlabledAndMerge", + getBackups: []*backup.Backup{ + merge1, + unlabeled1, + merge2, + unlabeled2, + }, + expectErr: assert.NoError, + expect: []*backup.Backup{ + merge1, + merge2, + unlabeled1, + unlabeled2, + }, + }, + { + name: "UnlabeledAndAssist", + getBackups: []*backup.Backup{ + unlabeled1, + assist1, + unlabeled2, + assist2, + }, + expectErr: assert.NoError, + expect: []*backup.Backup{ + unlabeled1, + unlabeled2, + }, + }, + { + name: "MergeAndAssist", + getBackups: []*backup.Backup{ + merge1, + assist1, + merge2, + assist2, + }, + expectErr: assert.NoError, + expect: []*backup.Backup{ + merge1, + merge2, + }, + }, + { + name: "UnlabeledAndMergeAndAssist", + getBackups: []*backup.Backup{ + unlabeled1, + merge1, + assist1, + merge2, + unlabeled2, + assist2, + }, + expectErr: assert.NoError, + expect: []*backup.Backup{ + merge1, + merge2, + unlabeled1, + unlabeled2, + }, + }, + { + name: "LookupError", + getBackups: []*backup.Backup{ + unlabeled1, + merge1, + assist1, + merge2, + unlabeled2, + assist2, + }, + listErr: assert.AnError, + expectErr: assert.Error, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + mbl := mockBackupList{ + backups: test.getBackups, + err: test.listErr, + check: func(fs []store.FilterOption) { + assert.ElementsMatch(t, test.filters, fs) + }, + } + + bs, err := backupsByTag(ctx, mbl, test.filters) + test.expectErr(t, err, clues.ToCore(err)) + + assert.ElementsMatch(t, test.expect, bs) + }) + } +} + type mockSSDeleter struct { err error }