diff --git a/CHANGELOG.md b/CHANGELOG.md index 14637004d..833d9397f 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/src/internal/kopia/upload.go b/src/internal/kopia/upload.go index 1e1f85e96..df9e40136 100644 --- a/src/internal/kopia/upload.go +++ b/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 { diff --git a/src/internal/kopia/upload_test.go b/src/internal/kopia/upload_test.go index 157e8b80c..56c0f4181 100644 --- a/src/internal/kopia/upload_test.go +++ b/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 { diff --git a/src/internal/kopia/wrapper_test.go b/src/internal/kopia/wrapper_test.go index 684d3fae2..1da2f1a84 100644 --- a/src/internal/kopia/wrapper_test.go +++ b/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} },