add prev-path updates to tree (#4762)

introduces a func to the delta tree that allows a caller to set the previous-path on a folder by ID.

---

#### Does this PR need a docs update or release note?

- [x] ⛔ No

#### Type of change

- [x] 🌻 Feature

#### Issue(s)

* #4689

#### Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E

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
+	for folderID, p := range prevPaths {
-	// delta token was valid because we should see all the changes.
+		// no check for errs.Failure here, despite the addRecoverable below.
-	// if !du.Reset {
+		// it's fine if we run through all of the collection generation even
-	// 	if len(excludedItemIDs) == 0 {
+		// with failures present, and let the backup finish out.
-	// 		continue
+		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)
+			continue
-	// 	if err != nil {
+		}
-	// 		return nil, false, clues.WrapWC(ictx, err, "making exclude prefix")
-	// 	}
 
-	// 	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
+	// TODO(keepers): leaving this code around for now as a guide
-	// deleted. Need to compare by ID because it's possible to make new folders
+	// while implementation progresses.
-	// with the same path as deleted old folders. We shouldn't merge items or
-	// subtrees if that happens though.
 
 	// --- post-processing
 

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
+// this test is expressly aimed exercising coarse combinations of delta enumeration,
-// and broad contracts.
+// previous path management, and post processing. Coarse here means the intent is not
-// More granular testing can be found in the lower level test functions below.
+// 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(
+			mbh.DriveItemEnumeration = test.enumerator
-				mock.Drive(id(drive)).With(
-					mock.Delta(id(delta), nil).With(
-						aPage())))
 
 			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)
 		})
 	}
 }

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:'
+	// contains the complete previous path
-	prev path.Elements
+	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 {
+// setPreviousPath updates the previousPath for the folder with folderID.  If the folder
-	numFiles   int
+// already exists either as a tombstone or in the tree, the previous path on those nodes
-	totalBytes int64
+// 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
+// file handling
-// 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,
-	}
-}
 
 // 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,
+	}
+}

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
 // ---------------------------------------------------------------------------

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)