From d9c42f790c391f09e72622aed741f402bec5361c Mon Sep 17 00:00:00 2001 From: Keepers Date: Thu, 30 Nov 2023 17:12:57 -0700 Subject: [PATCH] introduce flat deletions to delta tree (#4719) there are certain enumeration cases where tombstoning isn't the correct behavior, and we want to delete a folder from the tree entirely. This primarily occurrs when we have a create->delete pair of markers either within or across enumerations. --- #### Does this PR need a docs update or release note? - [x] :no_entry: No #### Type of change - [ ] :sunflower: Feature #### Issue(s) * #4689 #### Test Plan - [x] :zap: Unit test --- .../m365/collection/drive/collections_test.go | 29 +- .../m365/collection/drive/collections_tree.go | 345 +++++- .../collection/drive/collections_tree_test.go | 1094 ++++++++++++++--- .../m365/collection/drive/delta_tree.go | 68 +- .../m365/collection/drive/delta_tree_test.go | 314 ++--- .../m365/collection/drive/limiter_test.go | 20 + src/pkg/count/keys.go | 28 +- src/pkg/logger/logger.go | 1 + 8 files changed, 1476 insertions(+), 423 deletions(-) diff --git a/src/internal/m365/collection/drive/collections_test.go b/src/internal/m365/collection/drive/collections_test.go index 077eedc4d..b1c939eeb 100644 --- a/src/internal/m365/collection/drive/collections_test.go +++ b/src/internal/m365/collection/drive/collections_test.go @@ -172,7 +172,7 @@ func malwareItem( } func driveRootItem(id string) models.DriveItemable { - name := "root" + name := rootName item := models.NewDriveItem() item.SetName(&name) item.SetId(&id) @@ -279,8 +279,8 @@ const ( malware = "malware" nav = "nav" pkg = "package" - rootName = "root" - rootID = "root_id" + rootID = odConsts.RootID + rootName = odConsts.RootPathDir subfolder = "subfolder" tenant = "t" user = "u" @@ -1692,22 +1692,29 @@ func (suite *CollectionsUnitSuite) TestGet_treeCannotBeUsedWhileIncomplete() { ctx, flush := tester.NewContext(t) defer flush() - drive := models.NewDrive() - drive.SetId(ptr.To("id")) - drive.SetName(ptr.To("name")) + drv := models.NewDrive() + drv.SetId(ptr.To("id")) + drv.SetName(ptr.To("name")) mbh := mock.DefaultOneDriveBH(user) opts := control.DefaultOptions() opts.ToggleFeatures.UseDeltaTree = true - mockDrivePager := &apiMock.Pager[models.Driveable]{ - ToReturn: []apiMock.PagerResult[models.Driveable]{ - {Values: []models.Driveable{drive}}, + mbh.DrivePagerV = pagerForDrives(drv) + mbh.DriveItemEnumeration = mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + "id": { + Pages: []mock.NextPage{{ + Items: []models.DriveItemable{ + driveRootItem(rootID), // will be present, not needed + delItem(id(file), parent(1), rootID, isFile), + }, + }}, + DeltaUpdate: pagers.DeltaUpdate{URL: id(delta)}, + }, }, } - mbh.DrivePagerV = mockDrivePager - c := collWithMBH(mbh) c.ctrl = opts diff --git a/src/internal/m365/collection/drive/collections_tree.go b/src/internal/m365/collection/drive/collections_tree.go index c28064c11..d11841be8 100644 --- a/src/internal/m365/collection/drive/collections_tree.go +++ b/src/internal/m365/collection/drive/collections_tree.go @@ -9,15 +9,21 @@ import ( "github.com/alcionai/corso/src/internal/common/prefixmatcher" "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/data" + odConsts "github.com/alcionai/corso/src/internal/m365/service/onedrive/consts" bupMD "github.com/alcionai/corso/src/pkg/backup/metadata" "github.com/alcionai/corso/src/pkg/count" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" + "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" "github.com/alcionai/corso/src/pkg/services/m365/api/graph" "github.com/alcionai/corso/src/pkg/services/m365/api/pagers" ) +// --------------------------------------------------------------------------- +// Processing +// --------------------------------------------------------------------------- + // this file is used to separate the collections handling between the previous // (list-based) design, and the in-progress (tree-based) redesign. // see: https://github.com/alcionai/corso/issues/4688 @@ -29,6 +35,13 @@ func (c *Collections) getTree( errs *fault.Bus, ) ([]data.BackupCollection, bool, error) { ctx = clues.AddTraceName(ctx, "GetTree") + limiter := newPagerLimiter(c.ctrl) + + logger.Ctx(ctx).Infow( + "running backup: getting collection data using tree structure", + "limits", c.ctrl.PreviewLimits, + "effective_limits", limiter.effectiveLimits(), + "preview_mode", limiter.enabled()) // extract the previous backup's metadata like: deltaToken urls and previousPath maps. // We'll need these to reconstruct / ensure the correct state of the world, after @@ -97,8 +110,10 @@ func (c *Collections) getTree( ictx, drv, prevPathsByDriveID[driveID], + deltasByDriveID[driveID], + limiter, cl, - el.Local()) + el) if err != nil { el.AddRecoverable(ictx, clues.Stack(err)) continue @@ -140,40 +155,52 @@ func (c *Collections) getTree( return collections, canUsePrevBackup, nil } +var errTreeNotImplemented = clues.New("backup tree not implemented") + func (c *Collections) makeDriveCollections( ctx context.Context, - d models.Driveable, + drv models.Driveable, prevPaths map[string]string, + prevDeltaLink string, + limiter *pagerLimiter, counter *count.Bus, errs *fault.Bus, ) ([]data.BackupCollection, map[string]string, pagers.DeltaUpdate, error) { - cl := c.counter.Local() + ppfx, err := c.handler.PathPrefix(c.tenantID, ptr.Val(drv.GetId())) + if err != nil { + return nil, nil, pagers.DeltaUpdate{}, clues.Wrap(err, "generating backup tree prefix") + } - cl.Add(count.PrevPaths, int64(len(prevPaths))) - logger.Ctx(ctx).Infow( - "previous metadata for drive", - "count_old_prev_paths", len(prevPaths)) + var ( + tree = newFolderyMcFolderFace(ppfx) + stats = &driveEnumerationStats{} + ) - // TODO(keepers): leaving this code around for now as a guide - // while implementation progresses. + counter.Add(count.PrevPaths, int64(len(prevPaths))) - // --- pager aggregation + // --- delta item aggregation - // du, newPrevPaths, err := c.PopulateDriveCollections( - // ctx, - // d, - // tree, - // cl.Local(), - // errs) - // if err != nil { - // return nil, false, clues.Stack(err) - // } + du, err := c.populateTree( + ctx, + tree, + limiter, + stats, + drv, + prevDeltaLink, + counter, + errs) + if err != nil { + return nil, nil, pagers.DeltaUpdate{}, clues.Stack(err) + } // numDriveItems := c.NumItems - numPrevItems // numPrevItems = c.NumItems // cl.Add(count.NewPrevPaths, int64(len(newPrevPaths))) + // TODO(keepers): leaving this code around for now as a guide + // while implementation progresses. + // --- prev path incorporation // For both cases we don't need to do set difference on folder map if the @@ -227,7 +254,285 @@ func (c *Collections) makeDriveCollections( // } // } - return nil, nil, pagers.DeltaUpdate{}, clues.New("not yet implemented") + // this is a dumb hack to satisfy the linter. + if ctx == nil { + return nil, nil, du, nil + } + + return nil, nil, du, errTreeNotImplemented +} + +// populateTree constructs a new tree and populates it with items +// retrieved by enumerating the delta query for the drive. +func (c *Collections) populateTree( + ctx context.Context, + tree *folderyMcFolderFace, + limiter *pagerLimiter, + stats *driveEnumerationStats, + drv models.Driveable, + prevDeltaLink string, + counter *count.Bus, + errs *fault.Bus, +) (pagers.DeltaUpdate, error) { + ctx = clues.Add(ctx, "invalid_prev_delta", len(prevDeltaLink) == 0) + + var ( + driveID = ptr.Val(drv.GetId()) + el = errs.Local() + ) + + // TODO(keepers): to end in a correct state, we'll eventually need to run this + // query multiple times over, until it ends in an empty change set. + pager := c.handler.EnumerateDriveItemsDelta( + ctx, + driveID, + prevDeltaLink, + api.CallConfig{ + Select: api.DefaultDriveItemProps(), + }) + + for page, reset, done := pager.NextPage(); !done; page, reset, done = pager.NextPage() { + if el.Failure() != nil { + break + } + + counter.Inc(count.PagesEnumerated) + + if reset { + counter.Inc(count.PagerResets) + tree.Reset() + c.resetStats() + + *stats = driveEnumerationStats{} + } + + err := c.enumeratePageOfItems( + ctx, + tree, + limiter, + stats, + drv, + page, + counter, + errs) + if err != nil { + el.AddRecoverable(ctx, clues.Stack(err)) + } + + // Stop enumeration early if we've reached the item or page limit. Do this + // at the end of the loop so we don't request another page in the + // background. + // + // We don't want to break on just the container limit here because it's + // possible that there's more items in the current (final) container that + // we're processing. We need to see the next page to determine if we've + // reached the end of the container. Note that this doesn't take into + // account the number of items in the current container, so it's possible it + // will fetch more data when it doesn't really need to. + if limiter.atPageLimit(stats) || limiter.atItemLimit(stats) { + break + } + } + + // Always cancel the pager so that even if we exit early from the loop above + // we don't deadlock. Cancelling a pager that's already completed is + // essentially a noop. + pager.Cancel() + + du, err := pager.Results() + if err != nil { + return du, clues.Stack(err) + } + + logger.Ctx(ctx).Infow("enumerated collection delta", "stats", counter.Values()) + + return du, el.Failure() +} + +func (c *Collections) enumeratePageOfItems( + ctx context.Context, + tree *folderyMcFolderFace, + limiter *pagerLimiter, + stats *driveEnumerationStats, + drv models.Driveable, + page []models.DriveItemable, + counter *count.Bus, + errs *fault.Bus, +) error { + ctx = clues.Add(ctx, "page_lenth", len(page)) + el := errs.Local() + + for i, item := range page { + if el.Failure() != nil { + break + } + + var ( + isFolder = item.GetFolder() != nil || item.GetPackageEscaped() != nil + itemID = ptr.Val(item.GetId()) + err error + skipped *fault.Skipped + ) + + ictx := clues.Add( + ctx, + "item_id", itemID, + "item_name", clues.Hide(ptr.Val(item.GetName())), + "item_index", i, + "item_is_folder", isFolder, + "item_is_package", item.GetPackageEscaped() != nil) + + if isFolder { + // check if the preview needs to exit before adding each folder + if !tree.ContainsFolder(itemID) && limiter.atLimit(stats, len(tree.folderIDToNode)) { + break + } + + skipped, err = c.addFolderToTree(ictx, tree, drv, item, stats, counter) + } else { + skipped, err = c.addFileToTree(ictx, tree, drv, item, stats, counter) + } + + if skipped != nil { + el.AddSkip(ctx, skipped) + } + + if err != nil { + el.AddRecoverable(ictx, clues.Wrap(err, "adding folder")) + } + + // Check if we reached the item or size limit while processing this page. + // The check after this loop will get us out of the pager. + // We don't want to check all limits because it's possible we've reached + // the container limit but haven't reached the item limit or really added + // items to the last container we found. + if limiter.atItemLimit(stats) { + break + } + } + + stats.numPages++ + + return clues.Stack(el.Failure()).OrNil() +} + +func (c *Collections) addFolderToTree( + ctx context.Context, + tree *folderyMcFolderFace, + drv models.Driveable, + folder models.DriveItemable, + stats *driveEnumerationStats, + counter *count.Bus, +) (*fault.Skipped, error) { + var ( + driveID = ptr.Val(drv.GetId()) + folderID = ptr.Val(folder.GetId()) + folderName = ptr.Val(folder.GetName()) + isDeleted = folder.GetDeleted() != nil + isMalware = folder.GetMalware() != nil + isPkg = folder.GetPackageEscaped() != nil + parent = folder.GetParentReference() + parentID string + notSelected bool + ) + + if parent != nil { + parentID = ptr.Val(parent.GetId()) + } + + defer func() { + switch { + case notSelected: + counter.Inc(count.TotalContainersSkipped) + case isMalware: + counter.Inc(count.TotalMalwareProcessed) + case isDeleted: + counter.Inc(count.TotalDeleteFoldersProcessed) + case isPkg: + counter.Inc(count.TotalPackagesProcessed) + default: + counter.Inc(count.TotalFoldersProcessed) + } + }() + + // FIXME(keepers): if we don't track this as previously visited, + // we could add a skip multiple times, every time we visit the + // folder again at the top of the page. + if isMalware { + skip := fault.ContainerSkip( + fault.SkipMalware, + driveID, + folderID, + folderName, + graph.ItemInfo(folder)) + + logger.Ctx(ctx).Infow("malware detected") + + return skip, nil + } + + if isDeleted { + err := tree.SetTombstone(ctx, folderID) + return nil, clues.Stack(err).OrNil() + } + + collectionPath, err := c.makeFolderCollectionPath(ctx, driveID, folder) + if err != nil { + return nil, clues.Stack(err).Label(fault.LabelForceNoBackupCreation, count.BadCollPath) + } + + // Skip items that don't match the folder selectors we were given. + notSelected = shouldSkip(ctx, collectionPath, c.handler, ptr.Val(drv.GetName())) + if notSelected { + logger.Ctx(ctx).Debugw("path not selected", "skipped_path", collectionPath.String()) + return nil, nil + } + + err = tree.SetFolder(ctx, parentID, folderID, folderName, isPkg) + + return nil, clues.Stack(err).OrNil() +} + +func (c *Collections) makeFolderCollectionPath( + ctx context.Context, + driveID string, + folder models.DriveItemable, +) (path.Path, error) { + if folder.GetRoot() != nil { + pb := odConsts.DriveFolderPrefixBuilder(driveID) + collectionPath, err := c.handler.CanonicalPath(pb, c.tenantID) + + return collectionPath, clues.WrapWC(ctx, err, "making canonical root path").OrNil() + } + + if folder.GetParentReference() == nil || folder.GetParentReference().GetPath() == nil { + return nil, clues.NewWC(ctx, "no parent reference in folder").Label(count.MissingParent) + } + + // Append folder name to path since we want the path for the collection, not + // the path for the parent of the collection. + name := ptr.Val(folder.GetName()) + if len(name) == 0 { + return nil, clues.NewWC(ctx, "missing folder name") + } + + folderPath := path.Split(ptr.Val(folder.GetParentReference().GetPath())) + folderPath = append(folderPath, name) + pb := path.Builder{}.Append(folderPath...) + collectionPath, err := c.handler.CanonicalPath(pb, c.tenantID) + + return collectionPath, clues.WrapWC(ctx, err, "making folder collection path").OrNil() +} + +func (c *Collections) addFileToTree( + ctx context.Context, + tree *folderyMcFolderFace, + drv models.Driveable, + item models.DriveItemable, + stats *driveEnumerationStats, + counter *count.Bus, +) (*fault.Skipped, error) { + return nil, clues.New("not yet implemented") } // quality-of-life wrapper that transforms each tombstone in the map diff --git a/src/internal/m365/collection/drive/collections_tree_test.go b/src/internal/m365/collection/drive/collections_tree_test.go index 3cb9b1fa3..58b01d92b 100644 --- a/src/internal/m365/collection/drive/collections_tree_test.go +++ b/src/internal/m365/collection/drive/collections_tree_test.go @@ -14,6 +14,7 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/data" dataMock "github.com/alcionai/corso/src/internal/data/mock" + odConsts "github.com/alcionai/corso/src/internal/m365/service/onedrive/consts" "github.com/alcionai/corso/src/internal/m365/service/onedrive/mock" "github.com/alcionai/corso/src/internal/m365/support" "github.com/alcionai/corso/src/internal/tester" @@ -22,9 +23,9 @@ import ( "github.com/alcionai/corso/src/pkg/count" countTD "github.com/alcionai/corso/src/pkg/count/testdata" "github.com/alcionai/corso/src/pkg/fault" - "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api/graph" apiMock "github.com/alcionai/corso/src/pkg/services/m365/api/mock" + "github.com/alcionai/corso/src/pkg/services/m365/api/pagers" ) // --------------------------------------------------------------------------- @@ -43,25 +44,25 @@ func collWithMBH(mbh BackupHandler) *Collections { count.New()) } -func fullOrPrevPath( - t *testing.T, - coll data.BackupCollection, -) path.Path { - var collPath path.Path +// func fullOrPrevPath( +// t *testing.T, +// coll data.BackupCollection, +// ) path.Path { +// var collPath path.Path - if coll.State() != data.DeletedState { - collPath = coll.FullPath() - } else { - collPath = coll.PreviousPath() - } +// if coll.State() != data.DeletedState { +// collPath = coll.FullPath() +// } else { +// collPath = coll.PreviousPath() +// } - require.False( - t, - len(collPath.Elements()) < 4, - "malformed or missing collection path") +// require.False( +// t, +// len(collPath.Elements()) < 4, +// "malformed or missing collection path") - return collPath -} +// return collPath +// } func pagerForDrives(drives ...models.Driveable) *apiMock.Pager[models.Driveable] { return &apiMock.Pager[models.Driveable]{ @@ -100,28 +101,28 @@ func makePrevMetadataColls( } } -func compareMetadata( - t *testing.T, - mdColl data.Collection, - expectDeltas map[string]string, - expectPrevPaths map[string]map[string]string, -) { - ctx, flush := tester.NewContext(t) - defer flush() +// func compareMetadata( +// t *testing.T, +// mdColl data.Collection, +// expectDeltas map[string]string, +// expectPrevPaths map[string]map[string]string, +// ) { +// ctx, flush := tester.NewContext(t) +// defer flush() - colls := []data.RestoreCollection{ - dataMock.NewUnversionedRestoreCollection(t, data.NoFetchRestoreCollection{Collection: mdColl}), - } +// colls := []data.RestoreCollection{ +// dataMock.NewUnversionedRestoreCollection(t, data.NoFetchRestoreCollection{Collection: mdColl}), +// } - deltas, prevs, _, err := deserializeAndValidateMetadata( - ctx, - colls, - count.New(), - fault.New(true)) - require.NoError(t, err, "deserializing metadata", clues.ToCore(err)) - assert.Equal(t, expectDeltas, deltas, "delta urls") - assert.Equal(t, expectPrevPaths, prevs, "previous paths") -} +// deltas, prevs, _, err := deserializeAndValidateMetadata( +// ctx, +// colls, +// count.New(), +// fault.New(true)) +// require.NoError(t, err, "deserializing metadata", clues.ToCore(err)) +// assert.Equal(t, expectDeltas, deltas, "delta urls") +// assert.Equal(t, expectPrevPaths, prevs, "previous paths") +// } // for comparisons done by collection state type stateAssertion struct { @@ -129,7 +130,7 @@ type stateAssertion struct { // should never get set by the user. // this flag gets flipped when calling assertions.compare. // any unseen collection will error on requireNoUnseenCollections - sawCollection bool + // sawCollection bool } // for comparisons done by a given collection path @@ -172,65 +173,79 @@ func newCollAssertion( type collectionAssertions map[string]collectionAssertion // ensure the provided collection matches expectations as set by the test. -func (cas collectionAssertions) compare( - t *testing.T, - coll data.BackupCollection, - excludes *prefixmatcher.StringSetMatchBuilder, -) { - ctx, flush := tester.NewContext(t) - defer flush() +// func (cas collectionAssertions) compare( +// t *testing.T, +// coll data.BackupCollection, +// excludes *prefixmatcher.StringSetMatchBuilder, +// ) { +// ctx, flush := tester.NewContext(t) +// defer flush() - var ( - itemCh = coll.Items(ctx, fault.New(true)) - itemIDs = []string{} - ) +// var ( +// itemCh = coll.Items(ctx, fault.New(true)) +// itemIDs = []string{} +// ) - p := fullOrPrevPath(t, coll) +// p := fullOrPrevPath(t, coll) - for itm := range itemCh { - itemIDs = append(itemIDs, itm.ID()) - } +// for itm := range itemCh { +// itemIDs = append(itemIDs, itm.ID()) +// } - expect := cas[p.String()] - expectState := expect.states[coll.State()] - expectState.sawCollection = true +// expect := cas[p.String()] +// expectState := expect.states[coll.State()] +// expectState.sawCollection = true - assert.ElementsMatchf( - t, - expectState.itemIDs, - itemIDs, - "expected all items to match in collection with:\nstate %q\npath %q", - coll.State(), - p) +// assert.ElementsMatchf( +// t, +// expectState.itemIDs, +// itemIDs, +// "expected all items to match in collection with:\nstate %q\npath %q", +// coll.State(), +// p) - expect.doNotMerge( - t, - coll.DoNotMergeItems(), - "expected collection to have the appropariate doNotMerge flag") +// expect.doNotMerge( +// t, +// coll.DoNotMergeItems(), +// "expected collection to have the appropariate doNotMerge flag") - if result, ok := excludes.Get(p.String()); ok { - assert.Equal( - t, - expect.excludedItems, - result, - "excluded items") - } -} +// if result, ok := excludes.Get(p.String()); ok { +// assert.Equal( +// t, +// expect.excludedItems, +// result, +// "excluded items") +// } +// } // ensure that no collections in the expected set are still flagged // as sawCollection == false. -func (cas collectionAssertions) requireNoUnseenCollections( - t *testing.T, -) { - for p, withPath := range cas { - for _, state := range withPath.states { - require.True( - t, - state.sawCollection, - "results should have contained collection:\n\t%q\t\n%q", - state, p) - } +// func (cas collectionAssertions) requireNoUnseenCollections( +// t *testing.T, +// ) { +// for p, withPath := range cas { +// for _, state := range withPath.states { +// require.True( +// t, +// state.sawCollection, +// "results should have contained collection:\n\t%q\t\n%q", +// state, p) +// } +// } +// } + +func rootAnd(items ...models.DriveItemable) []models.DriveItemable { + return append([]models.DriveItemable{driveItem(rootID, rootName, parent(0), "", isFolder)}, items...) +} + +func pagesOf(pages ...[]models.DriveItemable) []mock.NextPage { + mnp := []mock.NextPage{} + + for _, page := range pages { + mnp = append(mnp, mock.NextPage{Items: page}) } + + return mnp } // --------------------------------------------------------------------------- @@ -349,72 +364,19 @@ func (suite *CollectionsTreeUnitSuite) TestCollections_MakeMetadataCollections() } } -// TODO(keepers): implement tree version of populateDriveCollections tests - -// TODO(keepers): implement tree version of TestGet single-drive tests - -func (suite *CollectionsTreeUnitSuite) TestCollections_MakeDriveCollections() { - drive1 := models.NewDrive() - drive1.SetId(ptr.To(idx(drive, 1))) - drive1.SetName(ptr.To(namex(drive, 1))) - - table := []struct { - name string - c *Collections - drive models.Driveable - prevPaths map[string]string - expectErr require.ErrorAssertionFunc - expectCounts countTD.Expected - }{ - { - name: "not yet implemented", - c: collWithMBH(mock.DefaultOneDriveBH(user)), - drive: drive1, - expectErr: require.Error, - expectCounts: countTD.Expected{ - count.PrevPaths: 0, - }, - }, - } - for _, test := range table { - suite.Run(test.name, func() { - t := suite.T() - - ctx, flush := tester.NewContext(t) - defer flush() - - colls, paths, delta, err := test.c.makeDriveCollections( - ctx, - test.drive, - test.prevPaths, - test.c.counter, - fault.New(true)) - - // TODO(keepers): awaiting implementation - test.expectErr(t, err, clues.ToCore(err)) - assert.Empty(t, colls) - assert.Empty(t, paths) - assert.Empty(t, delta.URL) - - test.expectCounts.Compare(t, test.c.counter) - }) - } -} - -// TODO(keepers): implement tree version of TestGet multi-drive tests - +// This test is primarily aimed at multi-drive handling. +// More complicated single-drive tests can be found in _MakeDriveCollections. func (suite *CollectionsTreeUnitSuite) TestCollections_GetTree() { - metadataPath, err := path.BuildMetadata( - tenant, - user, - path.OneDriveService, - path.FilesCategory, - false) - require.NoError(suite.T(), err, "making metadata path", clues.ToCore(err)) - - drive1 := models.NewDrive() - drive1.SetId(ptr.To(idx(drive, 1))) - drive1.SetName(ptr.To(namex(drive, 1))) + // metadataPath, err := path.BuildMetadata( + // tenant, + // user, + // path.OneDriveService, + // path.FilesCategory, + // false) + // require.NoError(suite.T(), err, "making metadata path", clues.ToCore(err)) + drv := models.NewDrive() + drv.SetId(ptr.To(id(drive))) + drv.SetName(ptr.To(name(drive))) type expected struct { canUsePrevBackup assert.BoolAssertionFunc @@ -437,7 +399,15 @@ func (suite *CollectionsTreeUnitSuite) TestCollections_GetTree() { }{ { name: "not yet implemented", - drivePager: pagerForDrives(drive1), + drivePager: pagerForDrives(drv), + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + id(drive): { + Pages: pagesOf(rootAnd()), + DeltaUpdate: pagers.DeltaUpdate{URL: id(delta)}, + }, + }, + }, expect: expected{ canUsePrevBackup: assert.False, collAssertions: collectionAssertions{ @@ -471,7 +441,7 @@ func (suite *CollectionsTreeUnitSuite) TestCollections_GetTree() { errs = fault.New(true) ) - colls, canUsePrevBackup, err := c.getTree( + _, _, err := c.getTree( ctx, prevMetadata, globalExcludes, @@ -479,32 +449,810 @@ func (suite *CollectionsTreeUnitSuite) TestCollections_GetTree() { test.expect.err(t, err, clues.ToCore(err)) // TODO(keepers): awaiting implementation - assert.Empty(t, colls) - assert.Equal(t, test.expect.skips, len(errs.Skipped())) - test.expect.canUsePrevBackup(t, canUsePrevBackup) - test.expect.counts.Compare(t, c.counter) + // assert.Empty(t, colls) + // assert.Equal(t, test.expect.skips, len(errs.Skipped())) + // test.expect.canUsePrevBackup(t, canUsePrevBackup) + // test.expect.counts.Compare(t, c.counter) - if err != nil { - return - } + // if err != nil { + // return + // } - for _, coll := range colls { - collPath := fullOrPrevPath(t, coll) + // for _, coll := range colls { + // collPath := fullOrPrevPath(t, coll) - if collPath.String() == metadataPath.String() { - compareMetadata( - t, - coll, - test.expect.deltas, - test.expect.prevPaths) + // if collPath.String() == metadataPath.String() { + // compareMetadata( + // t, + // coll, + // test.expect.deltas, + // test.expect.prevPaths) - continue - } + // continue + // } - test.expect.collAssertions.compare(t, coll, globalExcludes) - } + // test.expect.collAssertions.compare(t, coll, globalExcludes) + // } - test.expect.collAssertions.requireNoUnseenCollections(t) + // test.expect.collAssertions.requireNoUnseenCollections(t) }) } } + +// This test is primarily aimed exercising the full breadth of single-drive delta enumeration +// and broad contracts. +// More granular testing can be found in the lower level test functions below. +func (suite *CollectionsTreeUnitSuite) TestCollections_MakeDriveCollections() { + drv := models.NewDrive() + drv.SetId(ptr.To(id(drive))) + drv.SetName(ptr.To(name(drive))) + + table := []struct { + name string + drive models.Driveable + drivePager *apiMock.Pager[models.Driveable] + enumerator mock.EnumerateItemsDeltaByDrive + prevPaths map[string]string + expectErr require.ErrorAssertionFunc + expectCounts countTD.Expected + }{ + { + name: "not yet implemented", + drive: drv, + drivePager: pagerForDrives(drv), + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + id(drive): { + Pages: pagesOf(rootAnd()), + DeltaUpdate: pagers.DeltaUpdate{URL: id(delta)}, + }, + }, + }, + expectErr: require.Error, + expectCounts: countTD.Expected{ + count.PrevPaths: 0, + }, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + mbh := mock.DefaultDriveBHWith(user, test.drivePager, test.enumerator) + c := collWithMBH(mbh) + + colls, paths, du, err := c.makeDriveCollections( + ctx, + test.drive, + test.prevPaths, + idx(delta, "prev"), + newPagerLimiter(control.DefaultOptions()), + c.counter, + fault.New(true)) + + // TODO(keepers): awaiting implementation + test.expectErr(t, err, clues.ToCore(err)) + assert.Empty(t, colls) + assert.Empty(t, paths) + assert.Equal(t, id(delta), du.URL) + + test.expectCounts.Compare(t, c.counter) + }) + } +} + +// This test focuses on the population of a tree using delta enumeration data, +// and is not concerned with unifying previous paths or post-processing collections. +func (suite *CollectionsTreeUnitSuite) TestCollections_PopulateTree() { + drv := models.NewDrive() + drv.SetId(ptr.To(id(drive))) + drv.SetName(ptr.To(name(drive))) + + type expected struct { + counts countTD.Expected + err require.ErrorAssertionFunc + treeSize int + treeContainsFolderIDs []string + treeContainsTombstoneIDs []string + } + + table := []struct { + name string + enumerator mock.EnumerateItemsDeltaByDrive + tree *folderyMcFolderFace + limiter *pagerLimiter + expect expected + }{ + { + name: "nil page", + tree: newFolderyMcFolderFace(nil), + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + id(drive): { + Pages: nil, + DeltaUpdate: pagers.DeltaUpdate{URL: id(delta)}, + }, + }, + }, + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{}, + err: require.NoError, + treeSize: 0, + treeContainsFolderIDs: []string{}, + treeContainsTombstoneIDs: []string{}, + }, + }, + { + name: "root only", + tree: newFolderyMcFolderFace(nil), + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + id(drive): { + Pages: pagesOf(rootAnd()), + DeltaUpdate: pagers.DeltaUpdate{URL: id(delta)}, + }, + }, + }, + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{ + count.TotalFoldersProcessed: 1, + count.PagesEnumerated: 1, + }, + err: require.NoError, + treeSize: 1, + treeContainsFolderIDs: []string{ + rootID, + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + { + name: "root only on two pages", + tree: newFolderyMcFolderFace(nil), + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + id(drive): { + Pages: pagesOf(rootAnd(), rootAnd()), + DeltaUpdate: pagers.DeltaUpdate{URL: id(delta)}, + }, + }, + }, + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{ + count.TotalFoldersProcessed: 2, + count.PagesEnumerated: 2, + }, + err: require.NoError, + treeSize: 1, + treeContainsFolderIDs: []string{ + rootID, + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + { + name: "many folders in a hierarchy across multiple pages", + tree: newFolderyMcFolderFace(nil), + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + id(drive): { + Pages: pagesOf( + rootAnd(driveItem(id(folder), name(folder), parent(0), rootID, isFolder)), + rootAnd(driveItem(idx(folder, "sib"), namex(folder, "sib"), parent(0), rootID, isFolder)), + rootAnd( + driveItem(id(folder), name(folder), parent(0), rootID, isFolder), + driveItem(idx(folder, "chld"), namex(folder, "chld"), parent(0), id(folder), isFolder))), + DeltaUpdate: pagers.DeltaUpdate{URL: id(delta)}, + }, + }, + }, + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{ + count.TotalFoldersProcessed: 7, + count.PagesEnumerated: 3, + }, + err: require.NoError, + treeSize: 4, + treeContainsFolderIDs: []string{ + rootID, + id(folder), + idx(folder, "sib"), + idx(folder, "chld"), + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + { + // technically you won't see this behavior from graph deltas, since deletes always + // precede creates/updates. But it's worth checking that we can handle it anyways. + name: "create, delete on next page", + tree: newFolderyMcFolderFace(nil), + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + id(drive): { + Pages: pagesOf( + rootAnd(driveItem(id(folder), name(folder), parent(0), rootID, isFolder)), + rootAnd(delItem(id(folder), parent(0), rootID, isFolder))), + DeltaUpdate: pagers.DeltaUpdate{URL: id(delta)}, + }, + }, + }, + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{ + count.TotalFoldersProcessed: 3, + count.TotalDeleteFoldersProcessed: 1, + count.PagesEnumerated: 2, + }, + err: require.NoError, + treeSize: 2, + treeContainsFolderIDs: []string{ + rootID, + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + { + // technically you won't see this behavior from graph deltas, since deletes always + // precede creates/updates. But it's worth checking that we can handle it anyways. + name: "move, delete on next page", + tree: treeWithFolders(), + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + id(drive): { + Pages: pagesOf( + rootAnd( + driveItem(idx(folder, "parent"), namex(folder, "parent"), parent(0), rootID, isFolder), + driveItem(id(folder), namex(folder, "moved"), parent(0), idx(folder, "parent"), isFolder)), + rootAnd(delItem(id(folder), parent(0), idx(folder, "parent"), isFolder))), + DeltaUpdate: pagers.DeltaUpdate{URL: id(delta)}, + }, + }, + }, + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{ + count.TotalFoldersProcessed: 4, + count.TotalDeleteFoldersProcessed: 1, + count.PagesEnumerated: 2, + }, + err: require.NoError, + treeSize: 3, + treeContainsFolderIDs: []string{ + rootID, + idx(folder, "parent"), + }, + treeContainsTombstoneIDs: []string{ + id(folder), + }, + }, + }, + { + name: "at limit before enumeration", + tree: treeWithRoot(), + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + id(drive): { + Pages: pagesOf( + rootAnd(driveItem(id(folder), name(folder), parent(0), rootID, isFolder)), + rootAnd(driveItem(idx(folder, "sib"), namex(folder, "sib"), parent(0), rootID, isFolder)), + rootAnd( + driveItem(id(folder), name(folder), parent(0), rootID, isFolder), + driveItem(idx(folder, "chld"), namex(folder, "chld"), parent(0), id(folder), isFolder))), + DeltaUpdate: pagers.DeltaUpdate{URL: id(delta)}, + }, + }, + }, + limiter: newPagerLimiter(minimumLimitOpts()), + expect: expected{ + counts: countTD.Expected{ + count.TotalDeleteFoldersProcessed: 0, + count.TotalFoldersProcessed: 1, + count.PagesEnumerated: 1, + }, + err: require.NoError, + treeSize: 1, + treeContainsFolderIDs: []string{ + rootID, + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + { + name: "hit limit during enumeration", + tree: newFolderyMcFolderFace(nil), + enumerator: mock.EnumerateItemsDeltaByDrive{ + DrivePagers: map[string]*mock.DriveItemsDeltaPager{ + id(drive): { + Pages: pagesOf( + rootAnd(driveItem(id(folder), name(folder), parent(0), rootID, isFolder)), + rootAnd(driveItem(idx(folder, "sib"), namex(folder, "sib"), parent(0), rootID, isFolder)), + rootAnd( + driveItem(id(folder), name(folder), parent(0), rootID, isFolder), + driveItem(idx(folder, "chld"), namex(folder, "chld"), parent(0), id(folder), isFolder))), + DeltaUpdate: pagers.DeltaUpdate{URL: id(delta)}, + }, + }, + }, + limiter: newPagerLimiter(minimumLimitOpts()), + expect: expected{ + counts: countTD.Expected{ + count.TotalDeleteFoldersProcessed: 0, + count.TotalFoldersProcessed: 1, + count.PagesEnumerated: 1, + }, + err: require.NoError, + treeSize: 1, + treeContainsFolderIDs: []string{ + rootID, + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + mockDrivePager := &apiMock.Pager[models.Driveable]{ + ToReturn: []apiMock.PagerResult[models.Driveable]{ + {Values: []models.Driveable{drv}}, + }, + } + + mbh := mock.DefaultDriveBHWith(user, mockDrivePager, test.enumerator) + c := collWithMBH(mbh) + counter := count.New() + + _, err := c.populateTree( + ctx, + test.tree, + test.limiter, + &driveEnumerationStats{}, + drv, + id(delta), + counter, + fault.New(true)) + test.expect.err(t, err, clues.ToCore(err)) + assert.Equal(t, test.expect.treeSize, test.tree.CountFolders(), "count folders in tree") + test.expect.counts.Compare(t, counter) + + for _, id := range test.expect.treeContainsFolderIDs { + require.NotNil(t, test.tree.folderIDToNode[id], "node exists") + } + + for _, id := range test.expect.treeContainsTombstoneIDs { + require.NotNil(t, test.tree.tombstones[id], "tombstone exists") + } + }) + } +} + +// This test focuses on folder assertions when enumerating a page of items. +// File-specific assertions are focused in the _folders test variant. +func (suite *CollectionsTreeUnitSuite) TestCollections_EnumeratePageOfItems_folders() { + drv := models.NewDrive() + drv.SetId(ptr.To(id(drive))) + drv.SetName(ptr.To(name(drive))) + + type expected struct { + counts countTD.Expected + err require.ErrorAssertionFunc + treeSize int + treeContainsFolderIDs []string + treeContainsTombstoneIDs []string + } + + table := []struct { + name string + tree *folderyMcFolderFace + page []models.DriveItemable + limiter *pagerLimiter + expect expected + }{ + { + name: "nil page", + tree: treeWithRoot(), + page: nil, + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{}, + err: require.NoError, + treeSize: 1, + treeContainsFolderIDs: []string{ + rootID, + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + { + name: "empty page", + tree: treeWithRoot(), + page: []models.DriveItemable{}, + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{}, + err: require.NoError, + treeSize: 1, + treeContainsFolderIDs: []string{ + rootID, + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + { + name: "root only", + tree: newFolderyMcFolderFace(nil), + page: rootAnd(), + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{ + count.TotalFoldersProcessed: 1, + }, + err: require.NoError, + treeSize: 1, + treeContainsFolderIDs: []string{ + rootID, + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + { + name: "many folders in a hierarchy", + tree: treeWithRoot(), + page: rootAnd( + driveItem(id(folder), name(folder), parent(0), rootID, isFolder), + driveItem(idx(folder, "sib"), namex(folder, "sib"), parent(0), rootID, isFolder), + driveItem(idx(folder, "chld"), namex(folder, "chld"), parent(0), id(folder), isFolder)), + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{ + count.TotalFoldersProcessed: 4, + }, + err: require.NoError, + treeSize: 4, + treeContainsFolderIDs: []string{ + rootID, + id(folder), + idx(folder, "sib"), + idx(folder, "chld"), + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + { + name: "already hit folder limit", + tree: treeWithRoot(), + page: rootAnd( + driveItem(id(folder), name(folder), parent(0), rootID, isFolder), + driveItem(idx(folder, "sib"), namex(folder, "sib"), parent(0), rootID, isFolder), + driveItem(idx(folder, "chld"), namex(folder, "chld"), parent(0), id(folder), isFolder)), + limiter: newPagerLimiter(minimumLimitOpts()), + expect: expected{ + counts: countTD.Expected{ + count.TotalFoldersProcessed: 1, + }, + err: require.NoError, + treeSize: 1, + treeContainsFolderIDs: []string{ + rootID, + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + { + name: "create->delete", + tree: treeWithRoot(), + page: rootAnd( + driveItem(id(folder), name(folder), parent(0), rootID, isFolder), + delItem(id(folder), parent(0), rootID, isFolder)), + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{ + count.TotalFoldersProcessed: 2, + count.TotalDeleteFoldersProcessed: 1, + }, + err: require.NoError, + treeSize: 2, + treeContainsFolderIDs: []string{ + rootID, + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + { + name: "move->delete", + tree: treeWithFolders(), + page: rootAnd( + driveItem(idx(folder, "parent"), namex(folder, "parent"), parent(0), rootID, isFolder), + driveItem(id(folder), namex(folder, "moved"), parent(0), idx(folder, "parent"), isFolder), + delItem(id(folder), parent(0), idx(folder, "parent"), isFolder)), + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{ + count.TotalFoldersProcessed: 3, + count.TotalDeleteFoldersProcessed: 1, + }, + err: require.NoError, + treeSize: 3, + treeContainsFolderIDs: []string{ + rootID, + idx(folder, "parent"), + }, + treeContainsTombstoneIDs: []string{ + id(folder), + }, + }, + }, + { + name: "delete->create", + tree: treeWithRoot(), + page: rootAnd( + delItem(id(folder), parent(0), rootID, isFolder), + driveItem(id(folder), name(folder), parent(0), rootID, isFolder)), + limiter: newPagerLimiter(control.DefaultOptions()), + expect: expected{ + counts: countTD.Expected{ + count.TotalFoldersProcessed: 2, + count.TotalDeleteFoldersProcessed: 1, + }, + err: require.NoError, + treeSize: 2, + treeContainsFolderIDs: []string{ + rootID, + id(folder), + }, + treeContainsTombstoneIDs: []string{}, + }, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + c := collWithMBH(mock.DefaultOneDriveBH(user)) + counter := count.New() + + err := c.enumeratePageOfItems( + ctx, + test.tree, + test.limiter, + &driveEnumerationStats{}, + drv, + test.page, + counter, + fault.New(true)) + test.expect.err(t, err, clues.ToCore(err)) + assert.Equal(t, test.expect.treeSize, test.tree.CountFolders(), "count folders in tree") + test.expect.counts.Compare(t, counter) + + for _, id := range test.expect.treeContainsFolderIDs { + assert.NotNil(t, test.tree.folderIDToNode[id], "node exists") + } + + for _, id := range test.expect.treeContainsTombstoneIDs { + assert.NotNil(t, test.tree.tombstones[id], "tombstone exists") + } + }) + } +} + +func (suite *CollectionsTreeUnitSuite) TestCollections_AddFolderToTree() { + drv := models.NewDrive() + drv.SetId(ptr.To(id(drive))) + drv.SetName(ptr.To(name(drive))) + + fld := driveItem(id(folder), name(folder), parent(0), rootID, isFolder) + subFld := driveItem(id(folder), name(folder), parent(drv, namex(folder, "parent")), idx(folder, "parent"), isFolder) + pack := driveItem(id(pkg), name(pkg), parent(0), rootID, isPackage) + del := delItem(id(folder), parent(0), rootID, isFolder) + mal := malwareItem(idx(folder, "mal"), namex(folder, "mal"), parent(0), rootID, isFolder) + + type expected struct { + counts countTD.Expected + err require.ErrorAssertionFunc + treeSize int + treeContainsFolder assert.BoolAssertionFunc + skipped assert.ValueAssertionFunc + } + + table := []struct { + name string + tree *folderyMcFolderFace + folder models.DriveItemable + expect expected + }{ + { + name: "add folder", + tree: treeWithRoot(), + folder: fld, + expect: expected{ + err: require.NoError, + counts: countTD.Expected{count.TotalFoldersProcessed: 1}, + treeSize: 2, + treeContainsFolder: assert.True, + skipped: assert.Nil, + }, + }, + { + name: "re-add folder that already exists", + tree: treeWithFolders(), + folder: subFld, + expect: expected{ + err: require.NoError, + counts: countTD.Expected{count.TotalFoldersProcessed: 1}, + treeSize: 3, + treeContainsFolder: assert.True, + skipped: assert.Nil, + }, + }, + { + name: "add package", + tree: treeWithRoot(), + folder: pack, + expect: expected{ + err: require.NoError, + counts: countTD.Expected{count.TotalPackagesProcessed: 1}, + treeSize: 2, + treeContainsFolder: assert.True, + skipped: assert.Nil, + }, + }, + { + name: "tombstone a folder in a populated tree", + tree: treeWithFolders(), + folder: del, + expect: expected{ + err: require.NoError, + counts: countTD.Expected{count.TotalDeleteFoldersProcessed: 1}, + treeSize: 3, + treeContainsFolder: assert.True, + skipped: assert.Nil, + }, + }, + { + name: "tombstone new folder in unpopulated tree", + tree: newFolderyMcFolderFace(nil), + folder: del, + expect: expected{ + err: require.NoError, + counts: countTD.Expected{count.TotalDeleteFoldersProcessed: 1}, + treeSize: 1, + treeContainsFolder: assert.True, + skipped: assert.Nil, + }, + }, + { + name: "re-add tombstone that already exists", + tree: treeWithTombstone(), + folder: del, + expect: expected{ + err: require.NoError, + counts: countTD.Expected{count.TotalDeleteFoldersProcessed: 1}, + treeSize: 2, + treeContainsFolder: assert.True, + skipped: assert.Nil, + }, + }, + { + name: "add malware", + tree: treeWithRoot(), + folder: mal, + expect: expected{ + err: require.NoError, + counts: countTD.Expected{count.TotalMalwareProcessed: 1}, + treeSize: 1, + treeContainsFolder: assert.False, + skipped: assert.NotNil, + }, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + c := collWithMBH(mock.DefaultOneDriveBH(user)) + counter := count.New() + des := &driveEnumerationStats{} + + skipped, err := c.addFolderToTree( + ctx, + test.tree, + drv, + test.folder, + des, + counter) + test.expect.err(t, err, clues.ToCore(err)) + test.expect.skipped(t, skipped) + test.expect.counts.Compare(t, counter) + assert.Equal(t, test.expect.treeSize, test.tree.CountFolders(), "folders in tree") + test.expect.treeContainsFolder(t, test.tree.ContainsFolder(ptr.Val(test.folder.GetId()))) + }) + } +} + +func (suite *CollectionsTreeUnitSuite) TestCollections_MakeFolderCollectionPath() { + basePath, err := odConsts.DriveFolderPrefixBuilder(id(drive)).ToDataLayerOneDrivePath(tenant, user, false) + require.NoError(suite.T(), err, clues.ToCore(err)) + + folderPath, err := basePath.Append(false, name(folder)) + require.NoError(suite.T(), err, clues.ToCore(err)) + + table := []struct { + name string + folder models.DriveItemable + expect string + expectErr require.ErrorAssertionFunc + }{ + { + name: "root", + folder: driveRootItem(rootID), + expect: basePath.String(), + expectErr: require.NoError, + }, + { + name: "folder", + folder: driveItem(id(folder), name(folder), parent(0), rootID, isFolder), + expect: folderPath.String(), + expectErr: require.NoError, + }, + { + name: "folder without parent ref", + folder: models.NewDriveItem(), + expect: folderPath.String(), + expectErr: require.Error, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + c := collWithMBH(mock.DefaultOneDriveBH(user)) + + p, err := c.makeFolderCollectionPath(ctx, id(drive), test.folder) + test.expectErr(t, err, clues.ToCore(err)) + + if err == nil { + assert.Equal(t, test.expect, p.String()) + } + }) + } +} + +func (suite *CollectionsTreeUnitSuite) TestCollections_AddFileToTree() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + c := collWithMBH(mock.DefaultOneDriveBH(user)) + + skipped, err := c.addFileToTree( + ctx, + nil, + nil, + nil, + nil, + nil) + require.ErrorContains(t, err, "not yet implemented", clues.ToCore(err)) + require.Nil(t, skipped) +} diff --git a/src/internal/m365/collection/drive/delta_tree.go b/src/internal/m365/collection/drive/delta_tree.go index 3d4acf97c..53d51c393 100644 --- a/src/internal/m365/collection/drive/delta_tree.go +++ b/src/internal/m365/collection/drive/delta_tree.go @@ -36,6 +36,9 @@ type folderyMcFolderFace struct { // it's just a sensible place to store the data, since we're // already pushing file additions through the api. excludeFileIDs map[string]struct{} + + // true if Reset() was called + hadReset bool } func newFolderyMcFolderFace( @@ -49,6 +52,18 @@ func newFolderyMcFolderFace( } } +// Reset erases all data contained in the tree. This is intended for +// tracking a delta enumeration reset, not for tree re-use, and will +// cause the tree to flag itself as dirty in order to appropriately +// post-process the data. +func (face *folderyMcFolderFace) Reset() { + face.hadReset = true + face.root = nil + face.folderIDToNode = map[string]*nodeyMcNodeFace{} + face.tombstones = map[string]*nodeyMcNodeFace{} + face.excludeFileIDs = map[string]struct{}{} +} + type nodeyMcNodeFace struct { // required for mid-enumeration folder moves, else we have to walk // the tree completely to remove the node from its old parent. @@ -71,14 +86,12 @@ type nodeyMcNodeFace struct { func newNodeyMcNodeFace( parent *nodeyMcNodeFace, id, name string, - prev path.Elements, isPackage bool, ) *nodeyMcNodeFace { return &nodeyMcNodeFace{ parent: parent, id: id, name: name, - prev: prev, children: map[string]*nodeyMcNodeFace{}, items: map[string]time.Time{}, isPackage: isPackage, @@ -89,6 +102,21 @@ func newNodeyMcNodeFace( // folder handling // --------------------------------------------------------------------------- +// ContainsFolder returns true if the given folder id is present as either +// a live node or a tombstone. +func (face *folderyMcFolderFace) ContainsFolder(id string) bool { + _, stillKicking := face.folderIDToNode[id] + _, alreadyBuried := face.tombstones[id] + + return stillKicking || alreadyBuried +} + +// CountNodes returns a count that is the sum of live folders and +// tombstones recorded in the tree. +func (face *folderyMcFolderFace) CountFolders() int { + return len(face.tombstones) + len(face.folderIDToNode) +} + // SetFolder adds a node with the following details to the tree. // If the node already exists with the given ID, the name and parent // values are updated to match (isPackage is assumed not to change). @@ -118,7 +146,7 @@ func (face *folderyMcFolderFace) SetFolder( // only set the root node once. if name == odConsts.RootPathDir { if face.root == nil { - root := newNodeyMcNodeFace(nil, id, name, nil, isPackage) + root := newNodeyMcNodeFace(nil, id, name, isPackage) face.root = root face.folderIDToNode[id] = root } @@ -178,9 +206,7 @@ func (face *folderyMcFolderFace) SetFolder( nodey.parent = parent } else { // change type 1: new addition - // the previous location is always nil, since previous path additions get their - // own setter func. - nodey = newNodeyMcNodeFace(parent, id, name, nil, isPackage) + nodey = newNodeyMcNodeFace(parent, id, name, isPackage) } // ensure the parent points to this node, and that the node is registered @@ -194,16 +220,11 @@ func (face *folderyMcFolderFace) SetFolder( func (face *folderyMcFolderFace) SetTombstone( ctx context.Context, id string, - loc path.Elements, ) error { if len(id) == 0 { return clues.NewWC(ctx, "missing tombstone folder ID") } - if len(loc) == 0 { - return clues.NewWC(ctx, "missing tombstone location") - } - // since we run mutiple enumerations, it's possible to see a folder added on the // first enumeration that then gets deleted on the next. This means that the folder // was deleted while the first enumeration was in flight, and will show up even if @@ -225,29 +246,8 @@ func (face *folderyMcFolderFace) SetTombstone( return nil } - zombey, alreadyBuried := face.tombstones[id] - if alreadyBuried { - if zombey.prev.String() != loc.String() { - // logging for sanity - logger.Ctx(ctx).Infow( - "attempted to tombstone two paths with the same ID", - "first_tombstone_path", zombey.prev, - "second_tombstone_path", loc) - } - - // since we're storing drive data by folder name in kopia, not id, we need - // to make sure to preserve the original tombstone location. If we get a - // conflicting set of locations in the same delta enumeration, we can always - // treat the original one as the canonical one. IE: what we're deleting is - // the original location as it exists in kopia. So even if we get a newer - // location in the drive enumeration, the original location is the one that - // kopia uses, and the one we need to tombstone. - // - // this should also be asserted in the second step, where we compare the delta - // changes to the backup previous paths metadata. - face.tombstones[id] = zombey - } else { - face.tombstones[id] = newNodeyMcNodeFace(nil, id, "", loc, false) + if _, alreadyBuried := face.tombstones[id]; !alreadyBuried { + face.tombstones[id] = newNodeyMcNodeFace(nil, id, "", false) } return nil diff --git a/src/internal/m365/collection/drive/delta_tree_test.go b/src/internal/m365/collection/drive/delta_tree_test.go index 2197f3763..9c4d4a2c1 100644 --- a/src/internal/m365/collection/drive/delta_tree_test.go +++ b/src/internal/m365/collection/drive/delta_tree_test.go @@ -8,11 +8,49 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - odConsts "github.com/alcionai/corso/src/internal/m365/service/onedrive/consts" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/path" ) +// --------------------------------------------------------------------------- +// helpers +// --------------------------------------------------------------------------- + +var loc = path.NewElements("root:/foo/bar/baz/qux/fnords/smarf/voi/zumba/bangles/howdyhowdyhowdy") + +func treeWithRoot() *folderyMcFolderFace { + tree := newFolderyMcFolderFace(nil) + rootey := newNodeyMcNodeFace(nil, rootID, rootName, false) + tree.root = rootey + tree.folderIDToNode[rootID] = rootey + + return tree +} + +func treeWithTombstone() *folderyMcFolderFace { + tree := treeWithRoot() + tree.tombstones[id(folder)] = newNodeyMcNodeFace(nil, id(folder), "", false) + + return tree +} + +func treeWithFolders() *folderyMcFolderFace { + tree := treeWithRoot() + + o := newNodeyMcNodeFace(tree.root, idx(folder, "parent"), namex(folder, "parent"), true) + tree.folderIDToNode[o.id] = o + + f := newNodeyMcNodeFace(o, id(folder), name(folder), false) + tree.folderIDToNode[f.id] = f + o.children[f.id] = f + + return tree +} + +// --------------------------------------------------------------------------- +// tests +// --------------------------------------------------------------------------- + type DeltaTreeUnitSuite struct { tester.Suite } @@ -24,7 +62,7 @@ func TestDeltaTreeUnitSuite(t *testing.T) { func (suite *DeltaTreeUnitSuite) TestNewFolderyMcFolderFace() { var ( t = suite.T() - p, err = path.BuildPrefix("t", "r", path.OneDriveService, path.FilesCategory) + p, err = path.BuildPrefix(tenant, user, path.OneDriveService, path.FilesCategory) ) require.NoError(t, err, clues.ToCore(err)) @@ -41,14 +79,13 @@ func (suite *DeltaTreeUnitSuite) TestNewNodeyMcNodeFace() { var ( t = suite.T() parent = &nodeyMcNodeFace{} - loc = path.NewElements("root:/foo/bar/baz/qux/fnords/smarf/voi/zumba/bangles/howdyhowdyhowdy") ) - nodeFace := newNodeyMcNodeFace(parent, "id", "name", loc, true) + nodeFace := newNodeyMcNodeFace(parent, "id", "name", true) assert.Equal(t, parent, nodeFace.parent) assert.Equal(t, "id", nodeFace.id) assert.Equal(t, "name", nodeFace.name) - assert.Equal(t, loc, nodeFace.prev) + assert.NotEqual(t, loc, nodeFace.prev) assert.True(t, nodeFace.isPackage) assert.NotNil(t, nodeFace.children) assert.NotNil(t, nodeFace.items) @@ -57,139 +94,99 @@ func (suite *DeltaTreeUnitSuite) TestNewNodeyMcNodeFace() { // note that this test is focused on the SetFolder function, // and intentionally does not verify the resulting node tree func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_SetFolder() { - treeWithRoot := func() *folderyMcFolderFace { - rootey := newNodeyMcNodeFace(nil, odConsts.RootID, odConsts.RootPathDir, nil, false) - tree := newFolderyMcFolderFace(nil) - tree.root = rootey - tree.folderIDToNode[odConsts.RootID] = rootey - - return tree - } - - treeWithTombstone := func() *folderyMcFolderFace { - tree := treeWithRoot() - tree.tombstones["folder"] = newNodeyMcNodeFace(nil, "folder", "", path.NewElements(""), false) - - return tree - } - - treeWithFolders := func() *folderyMcFolderFace { - tree := treeWithRoot() - - o := newNodeyMcNodeFace(tree.root, "other", "o", nil, true) - tree.folderIDToNode[o.id] = o - - f := newNodeyMcNodeFace(o, "folder", "f", nil, false) - tree.folderIDToNode[f.id] = f - o.children[f.id] = f - - return tree - } - table := []struct { - tname string - tree *folderyMcFolderFace - parentID string - id string - name string - isPackage bool - expectErr assert.ErrorAssertionFunc - expectPrev assert.ValueAssertionFunc + tname string + tree *folderyMcFolderFace + parentID string + id string + name string + isPackage bool + expectErr assert.ErrorAssertionFunc }{ { tname: "add root", tree: &folderyMcFolderFace{ folderIDToNode: map[string]*nodeyMcNodeFace{}, }, - id: odConsts.RootID, - name: odConsts.RootPathDir, - isPackage: true, - expectErr: assert.NoError, - expectPrev: assert.Nil, + id: rootID, + name: rootName, + isPackage: true, + expectErr: assert.NoError, }, { - tname: "root already exists", - tree: treeWithRoot(), - id: odConsts.RootID, - name: odConsts.RootPathDir, - expectErr: assert.NoError, - expectPrev: assert.Nil, + tname: "root already exists", + tree: treeWithRoot(), + id: rootID, + name: rootName, + expectErr: assert.NoError, }, { - tname: "add folder", - tree: treeWithRoot(), - parentID: odConsts.RootID, - id: "folder", - name: "nameyMcNameFace", - expectErr: assert.NoError, - expectPrev: assert.Nil, + tname: "add folder", + tree: treeWithRoot(), + parentID: rootID, + id: id(folder), + name: name(folder), + expectErr: assert.NoError, }, { - tname: "add package", - tree: treeWithRoot(), - parentID: odConsts.RootID, - id: "folder", - name: "nameyMcNameFace", - isPackage: true, - expectErr: assert.NoError, - expectPrev: assert.Nil, + tname: "add package", + tree: treeWithRoot(), + parentID: rootID, + id: id(folder), + name: name(folder), + isPackage: true, + expectErr: assert.NoError, }, { - tname: "missing ID", - tree: treeWithRoot(), - parentID: odConsts.RootID, - name: "nameyMcNameFace", - isPackage: true, - expectErr: assert.Error, - expectPrev: assert.Nil, + tname: "missing ID", + tree: treeWithRoot(), + parentID: rootID, + name: name(folder), + isPackage: true, + expectErr: assert.Error, }, { - tname: "missing name", - tree: treeWithRoot(), - parentID: odConsts.RootID, - id: "folder", - isPackage: true, - expectErr: assert.Error, - expectPrev: assert.Nil, + tname: "missing name", + tree: treeWithRoot(), + parentID: rootID, + id: id(folder), + isPackage: true, + expectErr: assert.Error, }, { - tname: "missing parentID", - tree: treeWithRoot(), - id: "folder", - name: "nameyMcNameFace", - isPackage: true, - expectErr: assert.Error, - expectPrev: assert.Nil, + tname: "missing parentID", + tree: treeWithRoot(), + id: id(folder), + name: name(folder), + isPackage: true, + expectErr: assert.Error, }, { - tname: "already tombstoned", - tree: treeWithTombstone(), - parentID: odConsts.RootID, - id: "folder", - name: "nameyMcNameFace", - expectErr: assert.NoError, - expectPrev: assert.NotNil, + tname: "already tombstoned", + tree: treeWithTombstone(), + parentID: rootID, + id: id(folder), + name: name(folder), + expectErr: assert.NoError, }, { tname: "add folder before parent", tree: &folderyMcFolderFace{ folderIDToNode: map[string]*nodeyMcNodeFace{}, }, - parentID: odConsts.RootID, - id: "folder", - name: "nameyMcNameFace", - isPackage: true, - expectErr: assert.Error, - expectPrev: assert.Nil, + parentID: rootID, + id: id(folder), + name: name(folder), + isPackage: true, + expectErr: assert.Error, }, { - tname: "folder already exists", - tree: treeWithFolders(), - parentID: "other", - id: "folder", - name: "nameyMcNameFace", - expectErr: assert.NoError, - expectPrev: assert.Nil, + tname: "folder already exists", + tree: treeWithFolders(), + parentID: idx(folder, "parent"), + id: id(folder), + name: name(folder), + expectErr: assert.NoError, }, } for _, test := range table { @@ -213,7 +210,6 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_SetFolder() { result := test.tree.folderIDToNode[test.id] require.NotNil(t, result) - test.expectPrev(t, result.prev) assert.Equal(t, test.id, result.id) assert.Equal(t, test.name, result.name) assert.Equal(t, test.isPackage, result.isPackage) @@ -230,65 +226,38 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_SetFolder() { } func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_AddTombstone() { - loc := path.NewElements("root:/foo/bar/baz/qux/fnords/smarf/voi/zumba/bangles/howdyhowdyhowdy") - treeWithTombstone := func() *folderyMcFolderFace { - tree := newFolderyMcFolderFace(nil) - tree.tombstones["id"] = newNodeyMcNodeFace(nil, "id", "", loc, false) - - return tree - } - table := []struct { name string id string - loc path.Elements tree *folderyMcFolderFace expectErr assert.ErrorAssertionFunc }{ { name: "add tombstone", - id: "id", - loc: loc, + id: id(folder), tree: newFolderyMcFolderFace(nil), expectErr: assert.NoError, }, { name: "duplicate tombstone", - id: "id", - loc: loc, + id: id(folder), tree: treeWithTombstone(), expectErr: assert.NoError, }, { name: "missing ID", - loc: loc, - tree: newFolderyMcFolderFace(nil), - expectErr: assert.Error, - }, - { - name: "missing loc", - id: "id", - tree: newFolderyMcFolderFace(nil), - expectErr: assert.Error, - }, - { - name: "empty loc", - id: "id", - loc: path.Elements{}, tree: newFolderyMcFolderFace(nil), expectErr: assert.Error, }, { name: "conflict: folder alive", - id: "id", - loc: loc, + id: id(folder), tree: treeWithTombstone(), expectErr: assert.NoError, }, { - name: "already tombstoned with different path", - id: "id", - loc: append(path.Elements{"foo"}, loc...), + name: "already tombstoned", + id: id(folder), tree: treeWithTombstone(), expectErr: assert.NoError, }, @@ -300,7 +269,7 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_AddTombstone() { ctx, flush := tester.NewContext(t) defer flush() - err := test.tree.SetTombstone(ctx, test.id, test.loc) + err := test.tree.SetTombstone(ctx, test.id) test.expectErr(t, err, clues.ToCore(err)) if err != nil { @@ -309,8 +278,6 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_AddTombstone() { result := test.tree.tombstones[test.id] require.NotNil(t, result) - require.NotEmpty(t, result.prev) - assert.Equal(t, loc, result.prev) }) } } @@ -431,22 +398,17 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_SetFolder_correctTree() ctx, flush := tester.NewContext(t) defer flush() - tree := newFolderyMcFolderFace(nil) - rootID := odConsts.RootID + tree := treeWithRoot() set := func( - parentID, id, name string, + parentID, fid, fname string, isPackage bool, ) { - err := tree.SetFolder(ctx, parentID, id, name, isPackage) + err := tree.SetFolder(ctx, parentID, fid, fname, isPackage) require.NoError(t, err, clues.ToCore(err)) } - assert.Nil(t, tree.root) - assert.Empty(t, tree.folderIDToNode) - - // add the root - set("", rootID, odConsts.RootPathDir, false) + // assert the root exists assert.NotNil(t, tree.root) assert.Equal(t, rootID, tree.root.id) @@ -455,18 +417,18 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_SetFolder_correctTree() an(tree.root).compare(t, tree, true) // add a child at the root - set(tree.root.id, "lefty", "l", false) + set(rootID, id("lefty"), name("l"), false) - lefty := tree.folderIDToNode["lefty"] + lefty := tree.folderIDToNode[id("lefty")] an( tree.root, an(lefty)). compare(t, tree, true) // add another child at the root - set(tree.root.id, "righty", "r", false) + set(rootID, id("righty"), name("r"), false) - righty := tree.folderIDToNode["righty"] + righty := tree.folderIDToNode[id("righty")] an( tree.root, an(lefty), @@ -474,9 +436,9 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_SetFolder_correctTree() compare(t, tree, true) // add a child to lefty - set(lefty.id, "bloaty", "bl", false) + set(lefty.id, id("bloaty"), name("bl"), false) - bloaty := tree.folderIDToNode["bloaty"] + bloaty := tree.folderIDToNode[id("bloaty")] an( tree.root, an(lefty, an(bloaty)), @@ -484,9 +446,9 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_SetFolder_correctTree() compare(t, tree, true) // add another child to lefty - set(lefty.id, "brightly", "br", false) + set(lefty.id, id("brightly"), name("br"), false) - brightly := tree.folderIDToNode["brightly"] + brightly := tree.folderIDToNode[id("brightly")] an( tree.root, an(lefty, an(bloaty), an(brightly)), @@ -522,40 +484,34 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_SetFolder_correctTombst ctx, flush := tester.NewContext(t) defer flush() - tree := newFolderyMcFolderFace(nil) - rootID := odConsts.RootID + tree := treeWithRoot() set := func( - parentID, id, name string, + parentID, fid, fname string, isPackage bool, ) { - err := tree.SetFolder(ctx, parentID, id, name, isPackage) + err := tree.SetFolder(ctx, parentID, fid, fname, isPackage) require.NoError(t, err, clues.ToCore(err)) } tomb := func( - id string, + tid string, loc path.Elements, ) { - err := tree.SetTombstone(ctx, id, loc) + err := tree.SetTombstone(ctx, tid) require.NoError(t, err, clues.ToCore(err)) } - assert.Nil(t, tree.root) - assert.Empty(t, tree.folderIDToNode) - // create a simple tree // root > branchy > [leafy, bob] - set("", rootID, odConsts.RootPathDir, false) + set(tree.root.id, id("branchy"), name("br"), false) + branchy := tree.folderIDToNode[id("branchy")] - set(tree.root.id, "branchy", "br", false) - branchy := tree.folderIDToNode["branchy"] + set(branchy.id, id("leafy"), name("l"), false) + set(branchy.id, id("bob"), name("bobbers"), false) - set(branchy.id, "leafy", "l", false) - set(branchy.id, "bob", "bobbers", false) - - leafy := tree.folderIDToNode["leafy"] - bob := tree.folderIDToNode["bob"] + leafy := tree.folderIDToNode[id("leafy")] + bob := tree.folderIDToNode[id("bob")] an( tree.root, diff --git a/src/internal/m365/collection/drive/limiter_test.go b/src/internal/m365/collection/drive/limiter_test.go index 47f070489..2fa1317e7 100644 --- a/src/internal/m365/collection/drive/limiter_test.go +++ b/src/internal/m365/collection/drive/limiter_test.go @@ -26,6 +26,26 @@ import ( "github.com/alcionai/corso/src/pkg/services/m365/api/pagers" ) +// --------------------------------------------------------------------------- +// helpers +// --------------------------------------------------------------------------- + +func minimumLimitOpts() control.Options { + minLimitOpts := control.DefaultOptions() + minLimitOpts.PreviewLimits.Enabled = true + minLimitOpts.PreviewLimits.MaxBytes = 1 + minLimitOpts.PreviewLimits.MaxContainers = 1 + minLimitOpts.PreviewLimits.MaxItems = 1 + minLimitOpts.PreviewLimits.MaxItemsPerContainer = 1 + minLimitOpts.PreviewLimits.MaxPages = 1 + + return minLimitOpts +} + +// --------------------------------------------------------------------------- +// tests +// --------------------------------------------------------------------------- + type LimiterUnitSuite struct { tester.Suite } diff --git a/src/pkg/count/keys.go b/src/pkg/count/keys.go index a3a02e999..2e2945918 100644 --- a/src/pkg/count/keys.go +++ b/src/pkg/count/keys.go @@ -9,10 +9,8 @@ const ( ThrottledAPICalls Key = "throttled-api-calls" ) -// Tracked during backup +// backup amounts reported by kopia const ( - // amounts reported by kopia - PersistedCachedFiles Key = "persisted-cached-files" PersistedDirectories Key = "persisted-directories" PersistedFiles Key = "persisted-files" @@ -24,9 +22,10 @@ const ( PersistenceErrors Key = "persistence-errors" PersistenceExpectedErrors Key = "persistence-expected-errors" PersistenceIgnoredErrors Key = "persistence-ignored-errors" +) - // amounts reported by data providers - +// backup amounts reported by data providers +const ( Channels Key = "channels" CollectionMoved Key = "collection-moved" CollectionNew Key = "collection-state-new" @@ -66,10 +65,27 @@ const ( StreamItemsErrored Key = "stream-items-errored" StreamItemsFound Key = "stream-items-found" StreamItemsRemoved Key = "stream-items-removed" + TotalContainersSkipped Key = "total-containers-skipped" URLCacheMiss Key = "url-cache-miss" URLCacheRefresh Key = "url-cache-refresh" +) - // miscellaneous +// Total___Processed counts are used to track raw processing numbers +// for values that may have a similar, but different, end result count. +// For example: a delta query may add the same folder to many different pages. +// instead of adding logic to catch folder duplications and only count new +// entries, we can increment TotalFoldersProcessed for every duplication, +// and use a separate Key (Folders) for the end count of folders produced +// at the end of the delta enumeration. +const ( + TotalDeleteFoldersProcessed Key = "total-delete-folders-processed" + TotalFoldersProcessed Key = "total-folders-processed" + TotalMalwareProcessed Key = "total-malware-processed" + TotalPackagesProcessed Key = "total-packages-processed" +) + +// miscellaneous +const ( RequiresUserPnToIDMigration Key = "requires-user-pn-to-id-migration" ) diff --git a/src/pkg/logger/logger.go b/src/pkg/logger/logger.go index 00a0b34e3..25843c6c9 100644 --- a/src/pkg/logger/logger.go +++ b/src/pkg/logger/logger.go @@ -491,6 +491,7 @@ func CtxStack(ctx context.Context, skip int) *zap.SugaredLogger { // CtxErr retrieves the logger embedded in the context // and packs all of the structured data in the error inside it. func CtxErr(ctx context.Context, err error) *zap.SugaredLogger { + // TODO(keepers): only log the err values, not the ctx. return Ctx(ctx). With( "error", err,