From c3dbd5e0a84ec7767e2adb5d249a8394460c3f1c Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Thu, 22 Dec 2022 17:40:26 -0800 Subject: [PATCH] Handle DoNotMergeItems in hierarchy merge (#1922) ## Description The DoNotMergeItems flag denotes situations where we want to propagate changes to the hierarchy but do not want to source items from the base for a specific directory. As of now, the only time we expect to encounter this situation is when a delta token expires in M365 and we need to pull all the items for the container again. By setting DoNotMergeItems, a collection can propagate things like rename to its subtree while avoiding zombie items that would have appeared if there was a deletion in the container and the container was enumerated again ## Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No ## Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :hamster: Trivial/Minor ## Issue(s) * #1914 ## Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/kopia/upload.go | 9 ++ src/internal/kopia/upload_test.go | 181 ++++++++++++++++++++++++++++++ 2 files changed, 190 insertions(+) diff --git a/src/internal/kopia/upload.go b/src/internal/kopia/upload.go index 9c8d87424..a4cae93d8 100644 --- a/src/internal/kopia/upload.go +++ b/src/internal/kopia/upload.go @@ -759,6 +759,15 @@ func traverseBaseDir( return errors.Errorf("unable to get tree node for path %s", currentPath) } + // Now that we have the node we need to check if there is a collection + // marked DoNotMerge. If there is, skip adding a reference to this base dir + // in the node. That allows us to propagate subtree operations (e.x. move) + // while selectively skipping merging old and new versions for some + // directories. The expected usecase for this is delta token expiry in M365. + if node.collection != nil && node.collection.DoNotMergeItems() { + return nil + } + curP, err := path.FromDataLayerPath(currentPath.String(), false) if err != nil { return errors.Errorf( diff --git a/src/internal/kopia/upload_test.go b/src/internal/kopia/upload_test.go index fb70c557d..dcc6d39a2 100644 --- a/src/internal/kopia/upload_test.go +++ b/src/internal/kopia/upload_test.go @@ -1189,6 +1189,10 @@ func (suite *HierarchyBuilderUnitSuite) TestBuildDirectoryTreeMultipleSubdirecto []string{testTenant, service, testUser, category, testInboxDir}, false, ) + inboxFileName1 := testFileName4 + inboxFileData1 := testFileData4 + inboxFileName2 := testFileName5 + inboxFileData2 := testFileData5 personalPath := makePath( suite.T(), @@ -1213,6 +1217,7 @@ func (suite *HierarchyBuilderUnitSuite) TestBuildDirectoryTreeMultipleSubdirecto // - user1 // - email // - Inbox + // - file4 // - personal // - file1 // - file2 @@ -1230,6 +1235,11 @@ func (suite *HierarchyBuilderUnitSuite) TestBuildDirectoryTreeMultipleSubdirecto virtualfs.NewStaticDirectory( encodeElements(testInboxDir)[0], []fs.Entry{ + virtualfs.StreamingFileWithModTimeFromReader( + encodeElements(inboxFileName1)[0], + time.Time{}, + bytes.NewReader(inboxFileData1), + ), virtualfs.NewStaticDirectory( encodeElements(personalDir)[0], []fs.Entry{ @@ -1292,6 +1302,10 @@ func (suite *HierarchyBuilderUnitSuite) TestBuildDirectoryTreeMultipleSubdirecto { name: testInboxDir + "2", children: []*expectedNode{ + { + name: inboxFileName1, + children: []*expectedNode{}, + }, { name: personalDir, children: []*expectedNode{ @@ -1354,6 +1368,10 @@ func (suite *HierarchyBuilderUnitSuite) TestBuildDirectoryTreeMultipleSubdirecto { name: testInboxDir + "2", children: []*expectedNode{ + { + name: inboxFileName1, + children: []*expectedNode{}, + }, { name: personalDir, children: []*expectedNode{ @@ -1444,6 +1462,10 @@ func (suite *HierarchyBuilderUnitSuite) TestBuildDirectoryTreeMultipleSubdirecto { name: testInboxDir, children: []*expectedNode{ + { + name: inboxFileName1, + children: []*expectedNode{}, + }, { name: personalDir, children: []*expectedNode{ @@ -1487,6 +1509,10 @@ func (suite *HierarchyBuilderUnitSuite) TestBuildDirectoryTreeMultipleSubdirecto { name: testInboxDir, children: []*expectedNode{ + { + name: inboxFileName1, + children: []*expectedNode{}, + }, { name: personalDir, children: []*expectedNode{ @@ -1541,6 +1567,10 @@ func (suite *HierarchyBuilderUnitSuite) TestBuildDirectoryTreeMultipleSubdirecto { name: testInboxDir, children: []*expectedNode{ + { + name: inboxFileName1, + children: []*expectedNode{}, + }, { name: workDir, children: []*expectedNode{ @@ -1571,6 +1601,157 @@ func (suite *HierarchyBuilderUnitSuite) TestBuildDirectoryTreeMultipleSubdirecto }, ), }, + { + name: "MoveParentDeleteFileNoMergeSubtreeMerge", + inputCollections: func(t *testing.T) []data.Collection { + newInboxPath := makePath( + t, + []string{testTenant, service, testUser, category, personalDir}, + false, + ) + + // This path is implicitly updated because we update the inbox path. If + // we didn't update it here then it would end up at the old location + // still. + newWorkPath := makePath( + t, + []string{testTenant, service, testUser, category, personalDir, workDir}, + false, + ) + + inbox := mockconnector.NewMockExchangeCollection(newInboxPath, 1) + inbox.PrevPath = inboxPath + inbox.ColState = data.MovedState + inbox.DoNotMerge = true + // First file in inbox is implicitly deleted as we're not merging items + // and it's not listed. + inbox.Names[0] = inboxFileName2 + inbox.Data[0] = inboxFileData2 + + work := mockconnector.NewMockExchangeCollection(newWorkPath, 1) + work.PrevPath = workPath + work.ColState = data.MovedState + work.Names[0] = testFileName6 + work.Data[0] = testFileData6 + + return []data.Collection{inbox, work} + }, + expected: expectedTreeWithChildren( + []string{ + testTenant, + service, + testUser, + category, + }, + []*expectedNode{ + { + name: personalDir, + children: []*expectedNode{ + { + name: inboxFileName2, + children: []*expectedNode{}, + data: inboxFileData2, + }, + { + name: personalDir, + children: []*expectedNode{ + { + name: personalFileName1, + children: []*expectedNode{}, + }, + { + name: personalFileName2, + children: []*expectedNode{}, + }, + }, + }, + { + name: workDir, + children: []*expectedNode{ + { + name: workFileName, + children: []*expectedNode{}, + }, + { + name: testFileName6, + children: []*expectedNode{}, + data: testFileData6, + }, + }, + }, + }, + }, + }, + ), + }, + { + name: "NoMoveParentDeleteFileNoMergeSubtreeMerge", + inputCollections: func(t *testing.T) []data.Collection { + inbox := mockconnector.NewMockExchangeCollection(inboxPath, 1) + inbox.PrevPath = inboxPath + inbox.ColState = data.NotMovedState + inbox.DoNotMerge = true + // First file in inbox is implicitly deleted as we're not merging items + // and it's not listed. + inbox.Names[0] = inboxFileName2 + inbox.Data[0] = inboxFileData2 + + work := mockconnector.NewMockExchangeCollection(workPath, 1) + work.PrevPath = workPath + work.ColState = data.NotMovedState + work.Names[0] = testFileName6 + work.Data[0] = testFileData6 + + return []data.Collection{inbox, work} + }, + expected: expectedTreeWithChildren( + []string{ + testTenant, + service, + testUser, + category, + }, + []*expectedNode{ + { + name: testInboxDir, + children: []*expectedNode{ + { + name: inboxFileName2, + children: []*expectedNode{}, + data: inboxFileData2, + }, + { + name: personalDir, + children: []*expectedNode{ + { + name: personalFileName1, + children: []*expectedNode{}, + }, + { + name: personalFileName2, + children: []*expectedNode{}, + }, + }, + }, + { + name: workDir, + children: []*expectedNode{ + { + name: workFileName, + children: []*expectedNode{}, + }, + { + name: testFileName6, + children: []*expectedNode{}, + data: testFileData6, + }, + }, + }, + }, + }, + }, + ), + }, } for _, test := range table {