diff --git a/src/internal/m365/collection/drive/collections_tree.go b/src/internal/m365/collection/drive/collections_tree.go index b58389cb2..a8ea5fb29 100644 --- a/src/internal/m365/collection/drive/collections_tree.go +++ b/src/internal/m365/collection/drive/collections_tree.go @@ -204,32 +204,33 @@ func (c *Collections) makeDriveCollections( // 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 - // delta token was valid because we should see all the changes. - // if !du.Reset { - // if len(excludedItemIDs) == 0 { - // continue - // } + for folderID, p := range prevPaths { + // no check for errs.Failure here, despite the addRecoverable below. + // it's fine if we run through all of the collection generation even + // with failures present, and let the backup finish out. + prevPath, err := path.FromDataLayerPath(p, false) + if err != nil { + errs.AddRecoverable(ctx, clues.WrapWC(ctx, err, "invalid previous path"). + With("folderID", folderID, "prev_path", p). + Label(fault.LabelForceNoBackupCreation)) - // p, err := c.handler.CanonicalPath(odConsts.DriveFolderPrefixBuilder(driveID), c.tenantID) - // if err != nil { - // return nil, false, clues.WrapWC(ictx, err, "making exclude prefix") - // } + continue + } - // ssmb.Add(p.String(), excludedItemIDs) + err = tree.setPreviousPath(folderID, prevPath) + if err != nil { + errs.AddRecoverable(ctx, clues.WrapWC(ctx, err, "setting previous path"). + With("folderID", folderID, "prev_path", p). + Label(fault.LabelForceNoBackupCreation)) - // continue - // } + continue + } + } - // Set all folders in previous backup but not in the current one with state - // deleted. Need to compare by ID because it's possible to make new folders - // with the same path as deleted old folders. We shouldn't merge items or - // subtrees if that happens though. + // TODO(keepers): leaving this code around for now as a guide + // while implementation progresses. // --- post-processing diff --git a/src/internal/m365/collection/drive/collections_tree_test.go b/src/internal/m365/collection/drive/collections_tree_test.go index 519c86e6f..f73c738cf 100644 --- a/src/internal/m365/collection/drive/collections_tree_test.go +++ b/src/internal/m365/collection/drive/collections_tree_test.go @@ -244,9 +244,11 @@ func (suite *CollectionsTreeUnitSuite) TestCollections_GetTree() { } } -// 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. +// this test is expressly aimed exercising coarse combinations of delta enumeration, +// previous path management, and post processing. Coarse here means the intent is not +// to evaluate every possible combination of inputs and outputs. More granular tests +// at lower levels are better for verifing fine-grained concerns. This test only needs +// to ensure we stitch the parts together correctly. func (suite *CollectionsTreeUnitSuite) TestCollections_MakeDriveCollections() { drv := models.NewDrive() drv.SetId(ptr.To(id(drive))) @@ -255,17 +257,118 @@ func (suite *CollectionsTreeUnitSuite) TestCollections_MakeDriveCollections() { table := []struct { name string drive models.Driveable - drivePager *apiMock.Pager[models.Driveable] + enumerator mock.EnumerateDriveItemsDelta prevPaths map[string]string expectCounts countTD.Expected }{ { - name: "not yet implemented", + name: "only root in delta, no prev paths", drive: drv, + enumerator: mock.DriveEnumerator( + mock.Drive(id(drive)).With( + mock.Delta(id(delta), nil).With( + aPage()))), + prevPaths: map[string]string{}, expectCounts: countTD.Expected{ count.PrevPaths: 0, }, }, + { + name: "only root in delta, with prev paths", + drive: drv, + enumerator: mock.DriveEnumerator( + mock.Drive(id(drive)).With( + mock.Delta(id(delta), nil).With( + aPage()))), + prevPaths: map[string]string{ + id(folder): fullPath(id(folder)), + }, + expectCounts: countTD.Expected{ + count.PrevPaths: 1, + }, + }, + { + name: "some items, no prev paths", + drive: drv, + enumerator: mock.DriveEnumerator( + mock.Drive(id(drive)).With( + mock.Delta(id(delta), nil).With( + aPage(folderAtRoot(), fileAt(folder))))), + prevPaths: map[string]string{}, + expectCounts: countTD.Expected{ + count.PrevPaths: 0, + }, + }, + { + name: "some items, with prev paths", + drive: drv, + enumerator: mock.DriveEnumerator( + mock.Drive(id(drive)).With( + mock.Delta(id(delta), nil).With( + aPage(folderAtRoot(), fileAt(folder))))), + prevPaths: map[string]string{ + id(folder): fullPath(id(folder)), + }, + expectCounts: countTD.Expected{ + count.PrevPaths: 1, + }, + }, + { + name: "tree had delta reset, only root after, no prev paths", + drive: drv, + enumerator: mock.DriveEnumerator( + mock.Drive(id(drive)).With( + mock.DeltaWReset(id(delta), nil).With( + aReset(), + aPage()))), + prevPaths: map[string]string{}, + expectCounts: countTD.Expected{ + count.PrevPaths: 0, + }, + }, + { + name: "tree had delta reset, only root after, with prev paths", + drive: drv, + enumerator: mock.DriveEnumerator( + mock.Drive(id(drive)).With( + mock.DeltaWReset(id(delta), nil).With( + aReset(), + aPage()))), + prevPaths: map[string]string{ + id(folder): fullPath(id(folder)), + }, + expectCounts: countTD.Expected{ + count.PrevPaths: 1, + }, + }, + { + name: "tree had delta reset, enumerate items after, no prev paths", + drive: drv, + enumerator: mock.DriveEnumerator( + mock.Drive(id(drive)).With( + mock.DeltaWReset(id(delta), nil).With( + aReset(), + aPage(folderAtRoot(), fileAt(folder))))), + prevPaths: map[string]string{}, + expectCounts: countTD.Expected{ + count.PrevPaths: 0, + }, + }, + { + name: "tree had delta reset, enumerate items after, with prev paths", + drive: drv, + enumerator: mock.DriveEnumerator( + mock.Drive(id(drive)).With( + mock.DeltaWReset(id(delta), nil).With( + aReset(), + aPage(folderAtRoot(), fileAt(folder))))), + prevPaths: map[string]string{ + id(folder): fullPath(id(folder)), + }, + expectCounts: countTD.Expected{ + count.PrevPaths: 1, + }, + }, } for _, test := range table { suite.Run(test.name, func() { @@ -276,10 +379,7 @@ func (suite *CollectionsTreeUnitSuite) TestCollections_MakeDriveCollections() { mbh := mock.DefaultOneDriveBH(user) mbh.DrivePagerV = pagerForDrives(drv) - mbh.DriveItemEnumeration = mock.DriveEnumerator( - mock.Drive(id(drive)).With( - mock.Delta(id(delta), nil).With( - aPage()))) + mbh.DriveItemEnumeration = test.enumerator c := collWithMBH(mbh) @@ -292,13 +392,11 @@ func (suite *CollectionsTreeUnitSuite) TestCollections_MakeDriveCollections() { c.counter, fault.New(true)) - // TODO(keepers): awaiting implementation + // TODO(keepers): implementation is incomplete + // an error check is the best we can get at the moment. require.ErrorIs(t, err, errGetTreeNotImplemented, clues.ToCore(err)) - // assert.Empty(t, colls) - // assert.Empty(t, paths) - // assert.Empty(t, delta.URL) - // test.expectCounts.Compare(t, c.counter) + test.expectCounts.Compare(t, c.counter) }) } } diff --git a/src/internal/m365/collection/drive/delta_tree.go b/src/internal/m365/collection/drive/delta_tree.go index 4bfad289b..a888da1bd 100644 --- a/src/internal/m365/collection/drive/delta_tree.go +++ b/src/internal/m365/collection/drive/delta_tree.go @@ -81,8 +81,8 @@ type nodeyMcNodeFace struct { id string // single directory name, not a path name string - // only contains the folders starting at and including '/root:' - prev path.Elements + // contains the complete previous path + prev path.Path // folderID -> node children map[string]*nodeyMcNodeFace // file item ID -> file metadata @@ -124,12 +124,6 @@ func (face *folderyMcFolderFace) containsFolder(id string) bool { return stillKicking || alreadyBuried } -// countLiveFolders returns a count of the number of folders held in the tree. -// Tombstones are not included in the count. Only live folders. -func (face *folderyMcFolderFace) countLiveFolders() int { - return len(face.folderIDToNode) -} - func (face *folderyMcFolderFace) getNode(id string) *nodeyMcNodeFace { if zombey, alreadyBuried := face.tombstones[id]; alreadyBuried { return zombey @@ -269,44 +263,54 @@ func (face *folderyMcFolderFace) setTombstone( return nil } -type countAndSize struct { - numFiles int - totalBytes int64 +// setPreviousPath updates the previousPath for the folder with folderID. If the folder +// already exists either as a tombstone or in the tree, the previous path on those nodes +// gets updated. Otherwise the previous path update usually gets dropped, because we +// assume no changes have occurred. +// If the tree was Reset() at any point, any previous path that does not still exist in +// the tree- either as a tombstone or a live node- is assumed to have been deleted between +// deltas, and gets turned into a tombstone. +func (face *folderyMcFolderFace) setPreviousPath( + folderID string, + prev path.Path, +) error { + if len(folderID) == 0 { + return clues.New("missing folder id") + } + + if prev == nil { + return clues.New("missing previous path") + } + + if zombey, isDie := face.tombstones[folderID]; isDie { + zombey.prev = prev + return nil + } + + if nodey, exists := face.folderIDToNode[folderID]; exists { + nodey.prev = prev + return nil + } + + // if no reset occurred, then we assume all previous folder entries are still + // valid and continue to exist, even without a reference in the tree. However, + // if the delta was reset, then it's possible for a folder to be have been deleted + // and the only way we'd know is if the previous paths map says the folder exists + // but we haven't seen it again in this enumeration. + if !face.hadReset { + return nil + } + + zombey := newNodeyMcNodeFace(nil, folderID, "", false) + zombey.prev = prev + face.tombstones[folderID] = zombey + + return nil } -// countLiveFilesAndSizes returns a count of the number of files in the tree -// and the sum of all of their sizes. Only includes files that are not -// children of tombstoned containers. If running an incremental backup, a -// live file may be either a creation or an update. -func (face *folderyMcFolderFace) countLiveFilesAndSizes() countAndSize { - return countFilesAndSizes(face.root) -} - -func countFilesAndSizes(nodey *nodeyMcNodeFace) countAndSize { - if nodey == nil { - return countAndSize{} - } - - var ( - fileCount int - sumContentSize int64 - ) - - for _, child := range nodey.children { - countSize := countFilesAndSizes(child) - fileCount += countSize.numFiles - sumContentSize += countSize.totalBytes - } - - for _, file := range nodey.files { - sumContentSize += file.contentSize - } - - return countAndSize{ - numFiles: fileCount + len(nodey.files), - totalBytes: sumContentSize, - } -} +// --------------------------------------------------------------------------- +// file handling +// --------------------------------------------------------------------------- // addFile places the file in the correct parent node. If the // file was already added to the tree and is getting relocated, @@ -369,3 +373,52 @@ func (face *folderyMcFolderFace) deleteFile(id string) { face.deletedFileIDs[id] = struct{}{} } + +// --------------------------------------------------------------------------- +// quantification +// --------------------------------------------------------------------------- + +// countLiveFolders returns a count of the number of folders held in the tree. +// Tombstones are not included in the count. Only live folders. +func (face *folderyMcFolderFace) countLiveFolders() int { + return len(face.folderIDToNode) +} + +type countAndSize struct { + numFiles int + totalBytes int64 +} + +// countLiveFilesAndSizes returns a count of the number of files in the tree +// and the sum of all of their sizes. Only includes files that are not +// children of tombstoned containers. If running an incremental backup, a +// live file may be either a creation or an update. +func (face *folderyMcFolderFace) countLiveFilesAndSizes() countAndSize { + return countFilesAndSizes(face.root) +} + +func countFilesAndSizes(nodey *nodeyMcNodeFace) countAndSize { + if nodey == nil { + return countAndSize{} + } + + var ( + fileCount int + sumContentSize int64 + ) + + for _, child := range nodey.children { + countSize := countFilesAndSizes(child) + fileCount += countSize.numFiles + sumContentSize += countSize.totalBytes + } + + for _, file := range nodey.files { + sumContentSize += file.contentSize + } + + return countAndSize{ + numFiles: fileCount + len(nodey.files), + totalBytes: sumContentSize, + } +} diff --git a/src/internal/m365/collection/drive/delta_tree_test.go b/src/internal/m365/collection/drive/delta_tree_test.go index f3d8b39a4..2a8934feb 100644 --- a/src/internal/m365/collection/drive/delta_tree_test.go +++ b/src/internal/m365/collection/drive/delta_tree_test.go @@ -250,6 +250,109 @@ func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_AddTombstone() { } } +func (suite *DeltaTreeUnitSuite) TestFolderyMcFolderFace_SetPreviousPath() { + pathWith := func(loc path.Elements) path.Path { + p, err := path.Build(tenant, user, path.OneDriveService, path.FilesCategory, false, loc...) + require.NoError(suite.T(), err, clues.ToCore(err)) + + return p + } + + table := []struct { + name string + id string + prev path.Path + tree *folderyMcFolderFace + expectErr assert.ErrorAssertionFunc + expectLive bool + expectTombstone bool + }{ + { + name: "no changes become a no-op", + id: id(folder), + prev: pathWith(loc), + tree: newFolderyMcFolderFace(nil, rootID), + expectErr: assert.NoError, + expectLive: false, + expectTombstone: false, + }, + { + name: "added folders after reset", + id: id(folder), + prev: pathWith(loc), + tree: treeWithFoldersAfterReset(), + expectErr: assert.NoError, + expectLive: true, + expectTombstone: false, + }, + { + name: "create tombstone after reset", + id: id(folder), + prev: pathWith(loc), + tree: treeAfterReset(), + expectErr: assert.NoError, + expectLive: false, + expectTombstone: true, + }, + { + name: "missing ID", + prev: pathWith(loc), + tree: newFolderyMcFolderFace(nil, rootID), + expectErr: assert.Error, + expectLive: false, + expectTombstone: false, + }, + { + name: "missing prev", + id: id(folder), + tree: newFolderyMcFolderFace(nil, rootID), + expectErr: assert.Error, + expectLive: false, + expectTombstone: false, + }, + { + name: "update live folder", + id: id(folder), + prev: pathWith(loc), + tree: treeWithFolders(), + expectErr: assert.NoError, + expectLive: true, + expectTombstone: false, + }, + { + name: "update tombstone", + id: id(folder), + prev: pathWith(loc), + tree: treeWithTombstone(), + expectErr: assert.NoError, + expectLive: false, + expectTombstone: true, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + err := test.tree.setPreviousPath(test.id, test.prev) + test.expectErr(t, err, clues.ToCore(err)) + + if test.expectLive { + require.Contains(t, test.tree.folderIDToNode, test.id) + assert.Equal(t, test.prev, test.tree.folderIDToNode[test.id].prev) + } else { + require.NotContains(t, test.tree.folderIDToNode, test.id) + } + + if test.expectTombstone { + require.Contains(t, test.tree.tombstones, test.id) + assert.Equal(t, test.prev, test.tree.tombstones[test.id].prev) + } else { + require.NotContains(t, test.tree.tombstones, test.id) + } + }) + } +} + // --------------------------------------------------------------------------- // tree structure assertions tests // --------------------------------------------------------------------------- diff --git a/src/internal/m365/collection/drive/helper_test.go b/src/internal/m365/collection/drive/helper_test.go index 369b3078f..727a32163 100644 --- a/src/internal/m365/collection/drive/helper_test.go +++ b/src/internal/m365/collection/drive/helper_test.go @@ -789,6 +789,20 @@ func treeWithRoot() *folderyMcFolderFace { return tree } +func treeAfterReset() *folderyMcFolderFace { + tree := newFolderyMcFolderFace(nil, rootID) + tree.reset() + + return tree +} + +func treeWithFoldersAfterReset() *folderyMcFolderFace { + tree := treeWithFolders() + tree.hadReset = true + + return tree +} + func treeWithTombstone() *folderyMcFolderFace { tree := treeWithRoot() tree.tombstones[id(folder)] = newNodeyMcNodeFace(nil, id(folder), "", false)