From 7cb01b1e2779370a26586102cc502d3d7a321f07 Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Wed, 31 May 2023 11:27:15 -0700 Subject: [PATCH] Wrap new base finder and wire it up (#3524) Take the new base finder code, wrap it in a thin wrapper and use it in place of the previous way of finding base snapshots This solution is not focused on efficiency as it does result in looking up backup models multiple times. It's more to get the new way of finding bases out and then we can go back and smooth things over when we have the chance --- #### Does this PR need a docs update or release note? - [x] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [ ] :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) * #3202 #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [x] :green_heart: E2E --- CHANGELOG.md | 1 + src/internal/kopia/base_finder.go | 37 +++++--- src/internal/kopia/base_finder_test.go | 24 ++--- src/internal/kopia/inject/inject.go | 2 +- src/internal/kopia/wrapper.go | 5 ++ src/internal/operations/backup.go | 23 ++++- src/internal/operations/backup_test.go | 36 ++++---- src/internal/operations/manifests.go | 39 ++++----- src/internal/operations/manifests_test.go | 102 +++++++++------------- 9 files changed, 141 insertions(+), 128 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50c391c64..5b0b9c711 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Added ProtectedResourceName to the backup list json output. ProtectedResourceName holds either a UPN or a WebURL, depending on the resource type. +- Rework base selection logic for incremental backups so it's more likely to find a valid base. ### Fixed - Fix Exchange folder cache population error when parent folder isn't found. diff --git a/src/internal/kopia/base_finder.go b/src/internal/kopia/base_finder.go index e1adef823..837691d89 100644 --- a/src/internal/kopia/base_finder.go +++ b/src/internal/kopia/base_finder.go @@ -15,10 +15,10 @@ import ( "github.com/alcionai/corso/src/pkg/logger" ) -type BackupBases struct { - Backups []BackupEntry - MergeBases []ManifestEntry - AssistBases []ManifestEntry +type backupBases struct { + backups []BackupEntry + mergeBases []ManifestEntry + assistBases []ManifestEntry } type BackupEntry struct { @@ -31,7 +31,7 @@ type baseFinder struct { bg inject.GetBackuper } -func NewBaseFinder( +func newBaseFinder( sm snapshotManager, bg inject.GetBackuper, ) (*baseFinder, error) { @@ -183,11 +183,11 @@ func (b *baseFinder) getBase( return b.findBasesInSet(ctx, reason, metas) } -func (b *baseFinder) FindBases( +func (b *baseFinder) findBases( ctx context.Context, reasons []Reason, tags map[string]string, -) (BackupBases, error) { +) (backupBases, error) { var ( // All maps go from ID -> entry. We need to track by ID so we can coalesce // the reason for selecting something. Kopia assisted snapshots also use @@ -251,9 +251,24 @@ func (b *baseFinder) FindBases( } } - return BackupBases{ - Backups: maps.Values(baseBups), - MergeBases: maps.Values(baseSnaps), - AssistBases: maps.Values(kopiaAssistSnaps), + return backupBases{ + backups: maps.Values(baseBups), + mergeBases: maps.Values(baseSnaps), + assistBases: maps.Values(kopiaAssistSnaps), }, nil } + +func (b *baseFinder) FindBases( + ctx context.Context, + reasons []Reason, + tags map[string]string, +) ([]ManifestEntry, error) { + bb, err := b.findBases(ctx, reasons, tags) + if err != nil { + return nil, clues.Stack(err) + } + + // assistBases contains all snapshots so we can return it while maintaining + // almost all compatibility. + return bb.assistBases, nil +} diff --git a/src/internal/kopia/base_finder_test.go b/src/internal/kopia/base_finder_test.go index 4a9f47100..0c3761618 100644 --- a/src/internal/kopia/base_finder_test.go +++ b/src/internal/kopia/base_finder_test.go @@ -342,10 +342,10 @@ func (suite *BaseFinderUnitSuite) TestNoResult_NoBackupsOrSnapshots() { }, } - bb, err := bf.FindBases(ctx, reasons, nil) + bb, err := bf.findBases(ctx, reasons, nil) assert.NoError(t, err, "getting bases: %v", clues.ToCore(err)) - assert.Empty(t, bb.MergeBases) - assert.Empty(t, bb.AssistBases) + assert.Empty(t, bb.mergeBases) + assert.Empty(t, bb.assistBases) } func (suite *BaseFinderUnitSuite) TestNoResult_ErrorListingSnapshots() { @@ -366,10 +366,10 @@ func (suite *BaseFinderUnitSuite) TestNoResult_ErrorListingSnapshots() { }, } - bb, err := bf.FindBases(ctx, reasons, nil) + bb, err := bf.findBases(ctx, reasons, nil) assert.NoError(t, err, "getting bases: %v", clues.ToCore(err)) - assert.Empty(t, bb.MergeBases) - assert.Empty(t, bb.AssistBases) + assert.Empty(t, bb.mergeBases) + assert.Empty(t, bb.assistBases) } func (suite *BaseFinderUnitSuite) TestGetBases() { @@ -825,7 +825,7 @@ func (suite *BaseFinderUnitSuite) TestGetBases() { bg: &mockModelGetter{data: test.backupData}, } - bb, err := bf.FindBases( + bb, err := bf.findBases( ctx, test.input, nil) @@ -833,17 +833,17 @@ func (suite *BaseFinderUnitSuite) TestGetBases() { checkBackupEntriesMatch( t, - bb.Backups, + bb.backups, test.backupData, test.expectedBaseReasons) checkManifestEntriesMatch( t, - bb.MergeBases, + bb.mergeBases, test.manifestData, test.expectedBaseReasons) checkManifestEntriesMatch( t, - bb.AssistBases, + bb.assistBases, test.manifestData, test.expectedAssistManifestReasons) }) @@ -920,7 +920,7 @@ func (suite *BaseFinderUnitSuite) TestFetchPrevSnapshots_CustomTags() { bg: &mockModelGetter{data: backupData}, } - bb, err := bf.FindBases( + bb, err := bf.findBases( ctx, testAllUsersAllCats, test.tags) @@ -928,7 +928,7 @@ func (suite *BaseFinderUnitSuite) TestFetchPrevSnapshots_CustomTags() { checkManifestEntriesMatch( t, - bb.MergeBases, + bb.mergeBases, manifestData, test.expectedIdxs) }) diff --git a/src/internal/kopia/inject/inject.go b/src/internal/kopia/inject/inject.go index be1d9a16b..d97e06d31 100644 --- a/src/internal/kopia/inject/inject.go +++ b/src/internal/kopia/inject/inject.go @@ -39,6 +39,6 @@ type ( ctx context.Context, reasons []kopia.Reason, tags map[string]string, - ) (kopia.BackupBases, error) + ) ([]kopia.ManifestEntry, error) } ) diff --git a/src/internal/kopia/wrapper.go b/src/internal/kopia/wrapper.go index e92d2e133..f7830eded 100644 --- a/src/internal/kopia/wrapper.go +++ b/src/internal/kopia/wrapper.go @@ -19,6 +19,7 @@ import ( "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/diagnostics" "github.com/alcionai/corso/src/internal/observe" + "github.com/alcionai/corso/src/internal/operations/inject" "github.com/alcionai/corso/src/internal/stats" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control/repository" @@ -616,6 +617,10 @@ func (w Wrapper) FetchPrevSnapshotManifests( return fetchPrevSnapshotManifests(ctx, w.c, reasons, tags), nil } +func (w Wrapper) NewBaseFinder(bg inject.GetBackuper) (*baseFinder, error) { + return newBaseFinder(w.c, bg) +} + func isErrEntryNotFound(err error) bool { // Calling Child on a directory may return this. if errors.Is(err, fs.ErrEntryNotFound) { diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index 92e95c53b..5240d2c88 100644 --- a/src/internal/operations/backup.go +++ b/src/internal/operations/backup.go @@ -290,9 +290,24 @@ func (op *BackupOperation) do( // should always be 1, since backups are 1:1 with resourceOwners. opStats.resourceCount = 1 + kbf, err := op.kopia.NewBaseFinder(op.store) + if err != nil { + return nil, clues.Stack(err) + } + + type baseFinder struct { + kinject.BaseFinder + kinject.RestoreProducer + } + + bf := baseFinder{ + BaseFinder: kbf, + RestoreProducer: op.kopia, + } + mans, mdColls, canUseMetaData, err := produceManifestsAndMetadata( ctx, - op.kopia, + bf, op.store, reasons, fallbackReasons, op.account.ID(), @@ -467,7 +482,7 @@ func consumeBackupCollections( bc kinject.BackupConsumer, tenantID string, reasons []kopia.Reason, - mans []*kopia.ManifestEntry, + mans []kopia.ManifestEntry, cs []data.BackupCollection, pmr prefixmatcher.StringSetReader, backupID model.StableID, @@ -652,7 +667,7 @@ func getNewPathRefs( func lastCompleteBackups( ctx context.Context, ms *store.Wrapper, - mans []*kopia.ManifestEntry, + mans []kopia.ManifestEntry, ) (map[string]*backup.Backup, int, error) { var ( oldestVersion = version.NoBackup @@ -703,7 +718,7 @@ func mergeDetails( ctx context.Context, ms *store.Wrapper, detailsStore streamstore.Streamer, - mans []*kopia.ManifestEntry, + mans []kopia.ManifestEntry, dataFromBackup kopia.DetailsMergeInfoer, deets *details.Builder, writeStats *kopia.BackupStats, diff --git a/src/internal/operations/backup_test.go b/src/internal/operations/backup_test.go index 13e9a530a..94f0b5c7a 100644 --- a/src/internal/operations/backup_test.go +++ b/src/internal/operations/backup_test.go @@ -533,12 +533,12 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_ConsumeBackupDataCollections table := []struct { name string - inputMan []*kopia.ManifestEntry + inputMan []kopia.ManifestEntry expected []kopia.IncrementalBase }{ { name: "SingleManifestSingleReason", - inputMan: []*kopia.ManifestEntry{ + inputMan: []kopia.ManifestEntry{ { Manifest: manifest1, Reasons: []kopia.Reason{ @@ -557,7 +557,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_ConsumeBackupDataCollections }, { name: "SingleManifestMultipleReasons", - inputMan: []*kopia.ManifestEntry{ + inputMan: []kopia.ManifestEntry{ { Manifest: manifest1, Reasons: []kopia.Reason{ @@ -578,7 +578,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_ConsumeBackupDataCollections }, { name: "MultipleManifestsMultipleReasons", - inputMan: []*kopia.ManifestEntry{ + inputMan: []kopia.ManifestEntry{ { Manifest: manifest1, Reasons: []kopia.Reason{ @@ -731,7 +731,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems name string populatedModels map[model.StableID]backup.Backup populatedDetails map[string]*details.Details - inputMans []*kopia.ManifestEntry + inputMans []kopia.ManifestEntry mdm *mockDetailsMergeInfoer errCheck assert.ErrorAssertionFunc @@ -758,7 +758,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems return res }(), - inputMans: []*kopia.ManifestEntry{ + inputMans: []kopia.ManifestEntry{ { Manifest: makeManifest(suite.T(), "foo", ""), Reasons: []kopia.Reason{ @@ -776,7 +776,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems return res }(), - inputMans: []*kopia.ManifestEntry{ + inputMans: []kopia.ManifestEntry{ { Manifest: makeManifest(suite.T(), backup1.ID, ""), Reasons: []kopia.Reason{ @@ -803,7 +803,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems return res }(), - inputMans: []*kopia.ManifestEntry{ + inputMans: []kopia.ManifestEntry{ { Manifest: makeManifest(suite.T(), backup1.ID, ""), Reasons: []kopia.Reason{ @@ -833,7 +833,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems return res }(), - inputMans: []*kopia.ManifestEntry{ + inputMans: []kopia.ManifestEntry{ { Manifest: makeManifest(suite.T(), backup1.ID, ""), Reasons: []kopia.Reason{ @@ -869,7 +869,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems return res }(), - inputMans: []*kopia.ManifestEntry{ + inputMans: []kopia.ManifestEntry{ { Manifest: makeManifest(suite.T(), backup1.ID, ""), Reasons: []kopia.Reason{ @@ -931,7 +931,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems return res }(), - inputMans: []*kopia.ManifestEntry{ + inputMans: []kopia.ManifestEntry{ { Manifest: makeManifest(suite.T(), backup1.ID, ""), Reasons: []kopia.Reason{ @@ -961,7 +961,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems return res }(), - inputMans: []*kopia.ManifestEntry{ + inputMans: []kopia.ManifestEntry{ { Manifest: makeManifest(suite.T(), backup1.ID, ""), Reasons: []kopia.Reason{ @@ -994,7 +994,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems return res }(), - inputMans: []*kopia.ManifestEntry{ + inputMans: []kopia.ManifestEntry{ { Manifest: makeManifest(suite.T(), backup1.ID, ""), Reasons: []kopia.Reason{ @@ -1027,7 +1027,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems return res }(), - inputMans: []*kopia.ManifestEntry{ + inputMans: []kopia.ManifestEntry{ { Manifest: makeManifest(suite.T(), backup1.ID, ""), Reasons: []kopia.Reason{ @@ -1061,7 +1061,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems return res }(), - inputMans: []*kopia.ManifestEntry{ + inputMans: []kopia.ManifestEntry{ { Manifest: makeManifest(suite.T(), backup1.ID, ""), Reasons: []kopia.Reason{ @@ -1095,7 +1095,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems return res }(), - inputMans: []*kopia.ManifestEntry{ + inputMans: []kopia.ManifestEntry{ { Manifest: makeManifest(suite.T(), backup1.ID, ""), Reasons: []kopia.Reason{ @@ -1146,7 +1146,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsItems return res }(), - inputMans: []*kopia.ManifestEntry{ + inputMans: []kopia.ManifestEntry{ { Manifest: makeManifest(suite.T(), backup1.ID, ""), Reasons: []kopia.Reason{ @@ -1258,7 +1258,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_MergeBackupDetails_AddsFolde Category: itemPath1.Category(), } - inputMans = []*kopia.ManifestEntry{ + inputMans = []kopia.ManifestEntry{ { Manifest: makeManifest(t, backup1.ID, ""), Reasons: []kopia.Reason{ diff --git a/src/internal/operations/manifests.go b/src/internal/operations/manifests.go index 476b99126..2b028c55a 100644 --- a/src/internal/operations/manifests.go +++ b/src/internal/operations/manifests.go @@ -19,16 +19,8 @@ import ( "github.com/alcionai/corso/src/pkg/path" ) -type manifestFetcher interface { - FetchPrevSnapshotManifests( - ctx context.Context, - reasons []kopia.Reason, - tags map[string]string, - ) ([]*kopia.ManifestEntry, error) -} - type manifestRestorer interface { - manifestFetcher + inject.BaseFinder inject.RestoreProducer } @@ -47,14 +39,14 @@ func produceManifestsAndMetadata( reasons, fallbackReasons []kopia.Reason, tenantID string, getMetadata bool, -) ([]*kopia.ManifestEntry, []data.RestoreCollection, bool, error) { +) ([]kopia.ManifestEntry, []data.RestoreCollection, bool, error) { var ( tags = map[string]string{kopia.TagBackupCategory: ""} metadataFiles = graph.AllMetadataFileNames() collections []data.RestoreCollection ) - ms, err := mr.FetchPrevSnapshotManifests(ctx, reasons, tags) + ms, err := mr.FindBases(ctx, reasons, tags) if err != nil { return nil, nil, false, clues.Wrap(err, "looking up prior snapshots") } @@ -70,7 +62,7 @@ func produceManifestsAndMetadata( return ms, nil, false, nil } - fbms, err := mr.FetchPrevSnapshotManifests(ctx, fallbackReasons, tags) + fbms, err := mr.FindBases(ctx, fallbackReasons, tags) if err != nil { return nil, nil, false, clues.Wrap(err, "looking up prior snapshots under alternate id") } @@ -177,9 +169,9 @@ func produceManifestsAndMetadata( // 3. If mans has no entry for a reason, look for both complete and incomplete fallbacks. func unionManifests( reasons []kopia.Reason, - mans []*kopia.ManifestEntry, - fallback []*kopia.ManifestEntry, -) []*kopia.ManifestEntry { + mans []kopia.ManifestEntry, + fallback []kopia.ManifestEntry, +) []kopia.ManifestEntry { if len(fallback) == 0 { return mans } @@ -203,7 +195,9 @@ func unionManifests( } // track the manifests that were collected with the current lookup - for _, m := range mans { + for i := range mans { + m := &mans[i] + for _, r := range m.Reasons { k := r.Service.String() + r.Category.String() t := tups[k] @@ -219,7 +213,8 @@ func unionManifests( } // backfill from the fallback where necessary - for _, m := range fallback { + for i := range fallback { + m := &fallback[i] useReasons := []kopia.Reason{} for _, r := range m.Reasons { @@ -250,15 +245,15 @@ func unionManifests( } // collect the results into a single slice of manifests - ms := map[string]*kopia.ManifestEntry{} + ms := map[string]kopia.ManifestEntry{} for _, m := range tups { if m.complete != nil { - ms[string(m.complete.ID)] = m.complete + ms[string(m.complete.ID)] = *m.complete } if m.incomplete != nil { - ms[string(m.incomplete.ID)] = m.incomplete + ms[string(m.incomplete.ID)] = *m.incomplete } } @@ -269,7 +264,7 @@ func unionManifests( // of manifests, that each manifest's Reason (owner, service, category) is only // included once. If a reason is duplicated by any two manifests, an error is // returned. -func verifyDistinctBases(ctx context.Context, mans []*kopia.ManifestEntry) error { +func verifyDistinctBases(ctx context.Context, mans []kopia.ManifestEntry) error { reasons := map[string]manifest.ID{} for _, man := range mans { @@ -303,7 +298,7 @@ func verifyDistinctBases(ctx context.Context, mans []*kopia.ManifestEntry) error func collectMetadata( ctx context.Context, r inject.RestoreProducer, - man *kopia.ManifestEntry, + man kopia.ManifestEntry, fileNames []string, tenantID string, errs *fault.Bus, diff --git a/src/internal/operations/manifests_test.go b/src/internal/operations/manifests_test.go index fa3e93a6a..4c359edb1 100644 --- a/src/internal/operations/manifests_test.go +++ b/src/internal/operations/manifests_test.go @@ -27,16 +27,16 @@ import ( type mockManifestRestorer struct { mockRestoreProducer - mans []*kopia.ManifestEntry + mans []kopia.ManifestEntry mrErr error // err varname already claimed by mockRestoreProducer } -func (mmr mockManifestRestorer) FetchPrevSnapshotManifests( +func (mmr mockManifestRestorer) FindBases( ctx context.Context, reasons []kopia.Reason, tags map[string]string, -) ([]*kopia.ManifestEntry, error) { - mans := map[string]*kopia.ManifestEntry{} +) ([]kopia.ManifestEntry, error) { + mans := map[string]kopia.ManifestEntry{} for _, r := range reasons { for _, m := range mmr.mans { @@ -49,10 +49,6 @@ func (mmr mockManifestRestorer) FetchPrevSnapshotManifests( } } - if len(mans) == 0 && len(reasons) == 0 { - return mmr.mans, mmr.mrErr - } - return maps.Values(mans), mmr.mrErr } @@ -247,7 +243,7 @@ func (suite *OperationsManifestsUnitSuite) TestCollectMetadata() { mr := mockRestoreProducer{err: test.expectErr} mr.buildRestoreFunc(t, test.manID, paths) - man := &kopia.ManifestEntry{ + man := kopia.ManifestEntry{ Manifest: &snapshot.Manifest{ID: manifest.ID(test.manID)}, Reasons: test.reasons, } @@ -263,12 +259,12 @@ func (suite *OperationsManifestsUnitSuite) TestVerifyDistinctBases() { table := []struct { name string - mans []*kopia.ManifestEntry + mans []kopia.ManifestEntry expect assert.ErrorAssertionFunc }{ { name: "one manifest, one reason", - mans: []*kopia.ManifestEntry{ + mans: []kopia.ManifestEntry{ { Manifest: &snapshot.Manifest{}, Reasons: []kopia.Reason{ @@ -284,7 +280,7 @@ func (suite *OperationsManifestsUnitSuite) TestVerifyDistinctBases() { }, { name: "one incomplete manifest", - mans: []*kopia.ManifestEntry{ + mans: []kopia.ManifestEntry{ { Manifest: &snapshot.Manifest{IncompleteReason: "ir"}, }, @@ -293,7 +289,7 @@ func (suite *OperationsManifestsUnitSuite) TestVerifyDistinctBases() { }, { name: "one manifest, multiple reasons", - mans: []*kopia.ManifestEntry{ + mans: []kopia.ManifestEntry{ { Manifest: &snapshot.Manifest{}, Reasons: []kopia.Reason{ @@ -314,7 +310,7 @@ func (suite *OperationsManifestsUnitSuite) TestVerifyDistinctBases() { }, { name: "one manifest, duplicate reasons", - mans: []*kopia.ManifestEntry{ + mans: []kopia.ManifestEntry{ { Manifest: &snapshot.Manifest{}, Reasons: []kopia.Reason{ @@ -335,7 +331,7 @@ func (suite *OperationsManifestsUnitSuite) TestVerifyDistinctBases() { }, { name: "two manifests, non-overlapping reasons", - mans: []*kopia.ManifestEntry{ + mans: []kopia.ManifestEntry{ { Manifest: &snapshot.Manifest{}, Reasons: []kopia.Reason{ @@ -361,7 +357,7 @@ func (suite *OperationsManifestsUnitSuite) TestVerifyDistinctBases() { }, { name: "two manifests, overlapping reasons", - mans: []*kopia.ManifestEntry{ + mans: []kopia.ManifestEntry{ { Manifest: &snapshot.Manifest{}, Reasons: []kopia.Reason{ @@ -387,7 +383,7 @@ func (suite *OperationsManifestsUnitSuite) TestVerifyDistinctBases() { }, { name: "two manifests, overlapping reasons, one snapshot incomplete", - mans: []*kopia.ManifestEntry{ + mans: []kopia.ManifestEntry{ { Manifest: &snapshot.Manifest{}, Reasons: []kopia.Reason{ @@ -430,13 +426,13 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { did = "detailsid" ) - makeMan := func(pct path.CategoryType, id, incmpl, bid string) *kopia.ManifestEntry { + makeMan := func(pct path.CategoryType, id, incmpl, bid string) kopia.ManifestEntry { tags := map[string]string{} if len(bid) > 0 { tags = map[string]string{"tag:" + kopia.TagBackupID: bid} } - return &kopia.ManifestEntry{ + return kopia.ManifestEntry{ Manifest: &snapshot.Manifest{ ID: manifest.ID(id), IncompleteReason: incmpl, @@ -456,7 +452,6 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { name string mr mockManifestRestorer gb mockGetBackuper - reasons []kopia.Reason getMeta bool assertErr assert.ErrorAssertionFunc assertB assert.BoolAssertionFunc @@ -467,10 +462,9 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { name: "don't get metadata, no mans", mr: mockManifestRestorer{ mockRestoreProducer: mockRestoreProducer{}, - mans: []*kopia.ManifestEntry{}, + mans: []kopia.ManifestEntry{}, }, gb: mockGetBackuper{detailsID: did}, - reasons: []kopia.Reason{}, getMeta: false, assertErr: assert.NoError, assertB: assert.False, @@ -480,10 +474,9 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { name: "don't get metadata", mr: mockManifestRestorer{ mockRestoreProducer: mockRestoreProducer{}, - mans: []*kopia.ManifestEntry{makeMan(path.EmailCategory, "id1", "", "")}, + mans: []kopia.ManifestEntry{makeMan(path.EmailCategory, "id1", "", "")}, }, gb: mockGetBackuper{detailsID: did}, - reasons: []kopia.Reason{}, getMeta: false, assertErr: assert.NoError, assertB: assert.False, @@ -493,10 +486,9 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { name: "don't get metadata, incomplete manifest", mr: mockManifestRestorer{ mockRestoreProducer: mockRestoreProducer{}, - mans: []*kopia.ManifestEntry{makeMan(path.EmailCategory, "id1", "ir", "")}, + mans: []kopia.ManifestEntry{makeMan(path.EmailCategory, "id1", "ir", "")}, }, gb: mockGetBackuper{detailsID: did}, - reasons: []kopia.Reason{}, getMeta: false, assertErr: assert.NoError, assertB: assert.False, @@ -509,7 +501,6 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { mrErr: assert.AnError, }, gb: mockGetBackuper{detailsID: did}, - reasons: []kopia.Reason{}, getMeta: true, assertErr: assert.Error, assertB: assert.False, @@ -519,13 +510,12 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { name: "verify distinct bases fails", mr: mockManifestRestorer{ mockRestoreProducer: mockRestoreProducer{}, - mans: []*kopia.ManifestEntry{ + mans: []kopia.ManifestEntry{ makeMan(path.EmailCategory, "id1", "", ""), makeMan(path.EmailCategory, "id2", "", ""), }, }, gb: mockGetBackuper{detailsID: did}, - reasons: []kopia.Reason{}, getMeta: true, assertErr: assert.NoError, // No error, even though verify failed. assertB: assert.False, @@ -535,10 +525,9 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { name: "no manifests", mr: mockManifestRestorer{ mockRestoreProducer: mockRestoreProducer{}, - mans: []*kopia.ManifestEntry{}, + mans: []kopia.ManifestEntry{}, }, gb: mockGetBackuper{detailsID: did}, - reasons: []kopia.Reason{}, getMeta: true, assertErr: assert.NoError, assertB: assert.True, @@ -548,13 +537,12 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { name: "only incomplete manifests", mr: mockManifestRestorer{ mockRestoreProducer: mockRestoreProducer{}, - mans: []*kopia.ManifestEntry{ + mans: []kopia.ManifestEntry{ makeMan(path.EmailCategory, "id1", "ir", ""), makeMan(path.ContactsCategory, "id2", "ir", ""), }, }, gb: mockGetBackuper{detailsID: did}, - reasons: []kopia.Reason{}, getMeta: true, assertErr: assert.NoError, assertB: assert.True, @@ -568,10 +556,9 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { "id": {data.NotFoundRestoreCollection{Collection: mockColl{id: "id_coll"}}}, }, }, - mans: []*kopia.ManifestEntry{makeMan(path.EmailCategory, "id", "", "")}, + mans: []kopia.ManifestEntry{makeMan(path.EmailCategory, "id", "", "")}, }, gb: mockGetBackuper{detailsID: did}, - reasons: []kopia.Reason{}, getMeta: true, assertErr: assert.Error, assertB: assert.False, @@ -581,10 +568,9 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { name: "backup missing details id", mr: mockManifestRestorer{ mockRestoreProducer: mockRestoreProducer{}, - mans: []*kopia.ManifestEntry{makeMan(path.EmailCategory, "id1", "", "bid")}, + mans: []kopia.ManifestEntry{makeMan(path.EmailCategory, "id1", "", "bid")}, }, gb: mockGetBackuper{}, - reasons: []kopia.Reason{}, getMeta: true, assertErr: assert.NoError, assertB: assert.False, @@ -598,13 +584,12 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { "incmpl_id": {data.NotFoundRestoreCollection{Collection: mockColl{id: "incmpl_id_coll"}}}, }, }, - mans: []*kopia.ManifestEntry{ + mans: []kopia.ManifestEntry{ makeMan(path.EmailCategory, "id", "", "bid"), makeMan(path.EmailCategory, "incmpl_id", "ir", ""), }, }, gb: mockGetBackuper{detailsID: did}, - reasons: []kopia.Reason{}, getMeta: true, assertErr: assert.NoError, assertB: assert.True, @@ -618,10 +603,9 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { "id": {data.NotFoundRestoreCollection{Collection: mockColl{id: "id_coll"}}}, }, }, - mans: []*kopia.ManifestEntry{makeMan(path.EmailCategory, "id", "", "bid")}, + mans: []kopia.ManifestEntry{makeMan(path.EmailCategory, "id", "", "bid")}, }, gb: mockGetBackuper{detailsID: did}, - reasons: []kopia.Reason{}, getMeta: true, assertErr: assert.NoError, assertB: assert.True, @@ -636,13 +620,12 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { "contact": {data.NotFoundRestoreCollection{Collection: mockColl{id: "contact_coll"}}}, }, }, - mans: []*kopia.ManifestEntry{ + mans: []kopia.ManifestEntry{ makeMan(path.EmailCategory, "mail", "", "bid"), makeMan(path.ContactsCategory, "contact", "", "bid"), }, }, gb: mockGetBackuper{detailsID: did}, - reasons: []kopia.Reason{}, getMeta: true, assertErr: assert.NoError, assertB: assert.True, @@ -655,10 +638,9 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { name: "error collecting metadata", mr: mockManifestRestorer{ mockRestoreProducer: mockRestoreProducer{err: assert.AnError}, - mans: []*kopia.ManifestEntry{makeMan(path.EmailCategory, "id1", "", "bid")}, + mans: []kopia.ManifestEntry{makeMan(path.EmailCategory, "id1", "", "bid")}, }, gb: mockGetBackuper{detailsID: did}, - reasons: []kopia.Reason{}, getMeta: true, assertErr: assert.Error, assertB: assert.False, @@ -677,7 +659,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata() { ctx, &test.mr, &test.gb, - test.reasons, nil, + []kopia.Reason{{ResourceOwner: ro}}, nil, tid, test.getMeta) test.assertErr(t, err, clues.ToCore(err)) @@ -739,8 +721,8 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_fallb fbIncomplete = "fb_incmpl" ) - makeMan := func(id, incmpl string, reasons []kopia.Reason) *kopia.ManifestEntry { - return &kopia.ManifestEntry{ + makeMan := func(id, incmpl string, reasons []kopia.Reason) kopia.ManifestEntry { + return kopia.ManifestEntry{ Manifest: &snapshot.Manifest{ ID: manifest.ID(id), IncompleteReason: incmpl, @@ -1005,7 +987,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_fallb }) } - mans := []*kopia.ManifestEntry{} + mans := []kopia.ManifestEntry{} for _, m := range test.man { incomplete := "" @@ -1027,7 +1009,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_fallb mr := mockManifestRestorer{mans: mans} - mans, _, b, err := produceManifestsAndMetadata( + gotMans, _, b, err := produceManifestsAndMetadata( ctx, &mr, nil, @@ -1040,7 +1022,7 @@ func (suite *OperationsManifestsUnitSuite) TestProduceManifestsAndMetadata_fallb manIDs := []string{} - for _, m := range mans { + for _, m := range gotMans { manIDs = append(manIDs, string(m.ID)) reasons := test.expectReasons[string(m.ID)] @@ -1075,12 +1057,12 @@ func (suite *BackupManifestUnitSuite) TestBackupOperation_VerifyDistinctBases() table := []struct { name string - input []*kopia.ManifestEntry + input []kopia.ManifestEntry errCheck assert.ErrorAssertionFunc }{ { name: "SingleManifestMultipleReasons", - input: []*kopia.ManifestEntry{ + input: []kopia.ManifestEntry{ { Manifest: &snapshot.Manifest{ ID: "id1", @@ -1103,7 +1085,7 @@ func (suite *BackupManifestUnitSuite) TestBackupOperation_VerifyDistinctBases() }, { name: "MultipleManifestsDistinctReason", - input: []*kopia.ManifestEntry{ + input: []kopia.ManifestEntry{ { Manifest: &snapshot.Manifest{ ID: "id1", @@ -1133,7 +1115,7 @@ func (suite *BackupManifestUnitSuite) TestBackupOperation_VerifyDistinctBases() }, { name: "MultipleManifestsSameReason", - input: []*kopia.ManifestEntry{ + input: []kopia.ManifestEntry{ { Manifest: &snapshot.Manifest{ ID: "id1", @@ -1163,7 +1145,7 @@ func (suite *BackupManifestUnitSuite) TestBackupOperation_VerifyDistinctBases() }, { name: "MultipleManifestsSameReasonOneIncomplete", - input: []*kopia.ManifestEntry{ + input: []kopia.ManifestEntry{ { Manifest: &snapshot.Manifest{ ID: "id1", @@ -1250,13 +1232,13 @@ func (suite *BackupManifestUnitSuite) TestBackupOperation_CollectMetadata() { table := []struct { name string - inputMan *kopia.ManifestEntry + inputMan kopia.ManifestEntry inputFiles []string expected []path.Path }{ { name: "SingleReasonSingleFile", - inputMan: &kopia.ManifestEntry{ + inputMan: kopia.ManifestEntry{ Manifest: &snapshot.Manifest{}, Reasons: []kopia.Reason{ { @@ -1271,7 +1253,7 @@ func (suite *BackupManifestUnitSuite) TestBackupOperation_CollectMetadata() { }, { name: "SingleReasonMultipleFiles", - inputMan: &kopia.ManifestEntry{ + inputMan: kopia.ManifestEntry{ Manifest: &snapshot.Manifest{}, Reasons: []kopia.Reason{ { @@ -1286,7 +1268,7 @@ func (suite *BackupManifestUnitSuite) TestBackupOperation_CollectMetadata() { }, { name: "MultipleReasonsMultipleFiles", - inputMan: &kopia.ManifestEntry{ + inputMan: kopia.ManifestEntry{ Manifest: &snapshot.Manifest{}, Reasons: []kopia.Reason{ {