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?

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

## Type of change

- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [ ] 🐹 Trivial/Minor

## Issue(s)

* #1914 

## Test Plan

- [ ] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2022-12-22 17:40:26 -08:00 committed by GitHub
parent bc56d38970
commit c3dbd5e0a8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 190 additions and 0 deletions

View File

@ -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(

View File

@ -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 {