Handle subfolder moves and parent deletions right (#3186)

Properly merge items if a subfolder is moved,
the original parent is deleted and recreated,
and the subfolder is moved back to where it
started

See linked issue for a more detailed example

Manually tested original issue and fix on
OneDrive backup
---

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

- [x] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [ ] ⛔ No

#### Type of change

- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

* closes #3185

#### Test Plan

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

CHANGELOG.md

@@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - ParentPath of json output for Exchange calendar now shows names instead of IDs.
 - Fixed failure when downloading huge amount of attachments
 - Graph API requests that return an ECONNRESET error are now retried.
+- Fixed edge case in incremental backups where moving a subfolder, deleting and recreating the subfolder's original parent folder, and moving the subfolder back to where it started would skip backing up unchanged items in the subfolder.
 
 ### Known Issues
 - Restoring a OneDrive or SharePoint file with the same name as a file with that name as its M365 ID may restore both items.

src/internal/kopia/upload.go

@@ -727,7 +727,7 @@ func inflateCollectionTree(
 	toMerge *mergeDetails,
 ) (map[string]*treeMap, map[string]path.Path, error) {
 	roots := make(map[string]*treeMap)
-	// Contains the old path for collections that have been moved or renamed.
+	// Contains the old path for collections that are not new.
 	// Allows resolving what the new path should be when walking the base
 	// snapshot(s)'s hierarchy. Nil represents a collection that was deleted.
 	updatedPaths := make(map[string]path.Path)
@@ -776,6 +776,14 @@ func inflateCollectionTree(
 			if err := addMergeLocation(s, toMerge); err != nil {
 				return nil, nil, clues.Wrap(err, "adding merge location").WithClues(ictx)
 			}
+		case data.NotMovedState:
+			p := s.PreviousPath().String()
+			if _, ok := updatedPaths[p]; ok {
+				return nil, nil, clues.New("multiple previous state changes to collection").
+					WithClues(ictx)
+			}
+
+			updatedPaths[p] = s.FullPath()
 		}
 
 		if s.FullPath() == nil || len(s.FullPath().Elements()) == 0 {

src/internal/kopia/upload_test.go

@@ -1102,6 +1102,7 @@ func (suite *HierarchyBuilderUnitSuite) TestBuildDirectoryTreeSingleSubtree() {
 			name: "AddsNewItems",
 			inputCollections: func() []data.BackupCollection {
 				mc := exchMock.NewCollection(storePath, locPath, 1)
+				mc.PrevPath = storePath
 				mc.Names[0] = testFileName2
 				mc.Data[0] = testFileData2
 				mc.ColState = data.NotMovedState
@@ -1137,6 +1138,7 @@ func (suite *HierarchyBuilderUnitSuite) TestBuildDirectoryTreeSingleSubtree() {
 			name: "SkipsUpdatedItems",
 			inputCollections: func() []data.BackupCollection {
 				mc := exchMock.NewCollection(storePath, locPath, 1)
+				mc.PrevPath = storePath
 				mc.Names[0] = testFileName
 				mc.Data[0] = testFileData2
 				mc.ColState = data.NotMovedState
@@ -2054,6 +2056,150 @@ func (suite *HierarchyBuilderUnitSuite) TestBuildDirectoryTreeMultipleSubdirecto
 				},
 			),
 		},
+		{
+			// This could happen if a subfolder is moved out of the parent, the parent
+			// is deleted, a new folder at the same location as the parent is created,
+			// and then the subfolder is moved back to the same location.
+			name: "Delete Parent But Child Marked Not Moved Explicit New Parent",
+			inputCollections: func(t *testing.T) []data.BackupCollection {
+				inbox := exchMock.NewCollection(nil, inboxLocPath, 0)
+				inbox.PrevPath = inboxStorePath
+				inbox.ColState = data.DeletedState
+
+				inbox2 := exchMock.NewCollection(inboxStorePath, inboxLocPath, 1)
+				inbox2.PrevPath = nil
+				inbox2.ColState = data.NewState
+				inbox2.Names[0] = workFileName1
+
+				personal := exchMock.NewCollection(personalStorePath, personalLocPath, 0)
+				personal.PrevPath = personalStorePath
+				personal.ColState = data.NotMovedState
+
+				return []data.BackupCollection{inbox, inbox2, personal}
+			},
+			expected: expectedTreeWithChildren(
+				[]string{
+					testTenant,
+					service,
+					testUser,
+					category,
+				},
+				[]*expectedNode{
+					{
+						name: testInboxID,
+						children: []*expectedNode{
+							{
+								name:     workFileName1,
+								children: []*expectedNode{},
+							},
+							{
+								name: personalID,
+								children: []*expectedNode{
+									{
+										name:     personalFileName1,
+										children: []*expectedNode{},
+									},
+									{
+										name:     personalFileName2,
+										children: []*expectedNode{},
+									},
+								},
+							},
+						},
+					},
+				},
+			),
+		},
+		{
+			// This could happen if a subfolder is moved out of the parent, the parent
+			// is deleted, a new folder at the same location as the parent is created,
+			// and then the subfolder is moved back to the same location.
+			name: "Delete Parent But Child Marked Not Moved Implicit New Parent",
+			inputCollections: func(t *testing.T) []data.BackupCollection {
+				inbox := exchMock.NewCollection(nil, inboxLocPath, 0)
+				inbox.PrevPath = inboxStorePath
+				inbox.ColState = data.DeletedState
+
+				// New folder not explicitly listed as it may not have had new items.
+				personal := exchMock.NewCollection(personalStorePath, personalLocPath, 0)
+				personal.PrevPath = personalStorePath
+				personal.ColState = data.NotMovedState
+
+				return []data.BackupCollection{inbox, personal}
+			},
+			expected: expectedTreeWithChildren(
+				[]string{
+					testTenant,
+					service,
+					testUser,
+					category,
+				},
+				[]*expectedNode{
+					{
+						name: testInboxID,
+						children: []*expectedNode{
+							{
+								name: personalID,
+								children: []*expectedNode{
+									{
+										name:     personalFileName1,
+										children: []*expectedNode{},
+									},
+									{
+										name:     personalFileName2,
+										children: []*expectedNode{},
+									},
+								},
+							},
+						},
+					},
+				},
+			),
+		},
+		{
+			// This could happen if a subfolder is moved out of the parent, the parent
+			// is deleted, a new folder at the same location as the parent is created,
+			// and then the subfolder is moved back to the same location.
+			name: "Delete Parent But Child Marked Not Moved Implicit New Parent Child Do Not Merge",
+			inputCollections: func(t *testing.T) []data.BackupCollection {
+				inbox := exchMock.NewCollection(nil, inboxLocPath, 0)
+				inbox.PrevPath = inboxStorePath
+				inbox.ColState = data.DeletedState
+
+				// New folder not explicitly listed as it may not have had new items.
+				personal := exchMock.NewCollection(personalStorePath, personalLocPath, 1)
+				personal.PrevPath = personalStorePath
+				personal.ColState = data.NotMovedState
+				personal.DoNotMerge = true
+				personal.Names[0] = workFileName1
+
+				return []data.BackupCollection{inbox, personal}
+			},
+			expected: expectedTreeWithChildren(
+				[]string{
+					testTenant,
+					service,
+					testUser,
+					category,
+				},
+				[]*expectedNode{
+					{
+						name: testInboxID,
+						children: []*expectedNode{
+							{
+								name: personalID,
+								children: []*expectedNode{
+									{
+										name:     workFileName1,
+										children: []*expectedNode{},
+									},
+								},
+							},
+						},
+					},
+				},
+			),
+		},
 	}
 
 	for _, test := range table {

src/internal/kopia/wrapper_test.go

@@ -620,7 +620,7 @@ func (c mockBackupCollection) FullPath() path.Path {
 }
 
 func (c mockBackupCollection) PreviousPath() path.Path {
-	return nil
+	return c.path
 }
 
 func (c mockBackupCollection) LocationPath() *path.Builder {
@@ -1034,6 +1034,7 @@ func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupExcludeItem() {
 					suite.testPath1,
 					1)
 				c.ColState = data.NotMovedState
+				c.PrevPath = suite.testPath1
 
 				return []data.BackupCollection{c}
 			},