Fixup item deletion edge cases (#2920)

Mostly centers around handling item deletion
when the item is both created and deleted
during item enumeration

---

#### 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 #2917

#### Test Plan

- [ ] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2023-03-22 16:31:46 -07:00 committed by GitHub
parent 7a7933e54c
commit d1ce661b8f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 206 additions and 0 deletions

View File

@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Incremental OneDrive backups could panic if the delta token expired and a folder was seen and deleted in the course of item enumeration for the backup. - Incremental OneDrive backups could panic if the delta token expired and a folder was seen and deleted in the course of item enumeration for the backup.
- Incorrectly moving subfolder hierarchy from a deleted folder to a new folder at the same path during OneDrive incremental backup. - Incorrectly moving subfolder hierarchy from a deleted folder to a new folder at the same path during OneDrive incremental backup.
- Handle calendar events with no body. - Handle calendar events with no body.
- Items not being deleted if they were created and deleted during item enumeration of a OneDrive backup.
## [v0.6.1] (beta) - 2023-03-21 ## [v0.6.1] (beta) - 2023-03-21

View File

@ -495,9 +495,27 @@ func (c *Collections) handleDelete(
oldPaths, newPaths map[string]string, oldPaths, newPaths map[string]string,
isFolder bool, isFolder bool,
excluded map[string]struct{}, excluded map[string]struct{},
itemCollection map[string]map[string]string,
invalidPrevDelta bool, invalidPrevDelta bool,
) error { ) error {
if !isFolder { if !isFolder {
// Try to remove the item from the Collection if an entry exists for this
// item. This handles cases where an item was created and deleted during the
// same delta query.
if parentID, ok := itemCollection[driveID][itemID]; ok {
if col := c.CollectionMap[driveID][parentID]; col != nil {
col.Remove(itemID)
}
delete(itemCollection[driveID], itemID)
}
// Don't need to add to exclude list if the delta is invalid since the
// exclude list only matters if we're merging with a base.
if invalidPrevDelta {
return nil
}
excluded[itemID+DataFileSuffix] = struct{}{} excluded[itemID+DataFileSuffix] = struct{}{}
excluded[itemID+MetaFileSuffix] = struct{}{} excluded[itemID+MetaFileSuffix] = struct{}{}
// Exchange counts items streamed through it which includes deletions so // Exchange counts items streamed through it which includes deletions so
@ -669,6 +687,7 @@ func (c *Collections) UpdateCollections(
newPaths, newPaths,
isFolder, isFolder,
excluded, excluded,
itemCollection,
invalidPrevDelta, invalidPrevDelta,
); err != nil { ); err != nil {
return clues.Stack(err).WithClues(ictx) return clues.Stack(err).WithClues(ictx)

View File

@ -2009,6 +2009,192 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() {
expectedDelList: map[string]map[string]struct{}{}, expectedDelList: map[string]map[string]struct{}{},
doNotMergeItems: true, doNotMergeItems: true,
}, },
{
name: "One Drive Delta Error Random Item Delete",
drives: []models.Driveable{drive1},
items: map[string][]deltaPagerResult{
driveID1: {
{
err: getDeltaError(),
},
{
items: []models.DriveItemable{
driveRootItem("root"),
delItem("file", driveBasePath1, "root", true, false, false),
},
deltaLink: &delta,
},
},
},
errCheck: assert.NoError,
prevFolderPaths: map[string]map[string]string{
driveID1: {
"root": rootFolderPath1,
},
},
expectedCollections: map[string]map[data.CollectionState][]string{
rootFolderPath1: {data.NewState: {}},
},
expectedDeltaURLs: map[string]string{
driveID1: delta,
},
expectedFolderPaths: map[string]map[string]string{
driveID1: {
"root": rootFolderPath1,
},
},
expectedDelList: map[string]map[string]struct{}{},
doNotMergeItems: true,
},
{
name: "One Drive Folder Made And Deleted",
drives: []models.Driveable{drive1},
items: map[string][]deltaPagerResult{
driveID1: {
{
items: []models.DriveItemable{
driveRootItem("root"),
driveItem("folder", "folder", driveBasePath1, "root", false, true, false),
driveItem("file", "file", driveBasePath1+"/folder", "folder", true, false, false),
},
nextLink: &next,
},
{
items: []models.DriveItemable{
driveRootItem("root"),
delItem("folder", driveBasePath1, "root", false, true, false),
delItem("file", driveBasePath1, "root", true, false, false),
},
deltaLink: &delta2,
},
},
},
errCheck: assert.NoError,
prevFolderPaths: map[string]map[string]string{
driveID1: {},
},
expectedCollections: map[string]map[data.CollectionState][]string{
rootFolderPath1: {data.NewState: {}},
},
expectedDeltaURLs: map[string]string{
driveID1: delta2,
},
expectedFolderPaths: map[string]map[string]string{
driveID1: {
"root": rootFolderPath1,
},
},
expectedDelList: map[string]map[string]struct{}{
rootFolderPath1: getDelList("file"),
},
},
{
name: "One Drive Item Made And Deleted",
drives: []models.Driveable{drive1},
items: map[string][]deltaPagerResult{
driveID1: {
{
items: []models.DriveItemable{
driveRootItem("root"),
driveItem("folder", "folder", driveBasePath1, "root", false, true, false),
driveItem("file", "file", driveBasePath1+"/folder", "folder", true, false, false),
},
nextLink: &next,
},
{
items: []models.DriveItemable{
driveRootItem("root"),
delItem("file", driveBasePath1, "root", true, false, false),
},
deltaLink: &delta,
},
},
},
errCheck: assert.NoError,
prevFolderPaths: map[string]map[string]string{
driveID1: {},
},
expectedCollections: map[string]map[data.CollectionState][]string{
rootFolderPath1: {data.NewState: {}},
folderPath1: {data.NewState: {"folder"}},
},
expectedDeltaURLs: map[string]string{
driveID1: delta,
},
expectedFolderPaths: map[string]map[string]string{
driveID1: {
"root": rootFolderPath1,
"folder": folderPath1,
},
},
expectedDelList: map[string]map[string]struct{}{
rootFolderPath1: getDelList("file"),
},
},
{
name: "One Drive Random Folder Delete",
drives: []models.Driveable{drive1},
items: map[string][]deltaPagerResult{
driveID1: {
{
items: []models.DriveItemable{
driveRootItem("root"),
delItem("folder", driveBasePath1, "root", false, true, false),
},
deltaLink: &delta,
},
},
},
errCheck: assert.NoError,
prevFolderPaths: map[string]map[string]string{
driveID1: {},
},
expectedCollections: map[string]map[data.CollectionState][]string{
rootFolderPath1: {data.NewState: {}},
},
expectedDeltaURLs: map[string]string{
driveID1: delta,
},
expectedFolderPaths: map[string]map[string]string{
driveID1: {
"root": rootFolderPath1,
},
},
expectedDelList: map[string]map[string]struct{}{},
},
{
name: "One Drive Random Item Delete",
drives: []models.Driveable{drive1},
items: map[string][]deltaPagerResult{
driveID1: {
{
items: []models.DriveItemable{
driveRootItem("root"),
delItem("file", driveBasePath1, "root", true, false, false),
},
deltaLink: &delta,
},
},
},
errCheck: assert.NoError,
prevFolderPaths: map[string]map[string]string{
driveID1: {},
},
expectedCollections: map[string]map[data.CollectionState][]string{
rootFolderPath1: {data.NewState: {}},
},
expectedDeltaURLs: map[string]string{
driveID1: delta,
},
expectedFolderPaths: map[string]map[string]string{
driveID1: {
"root": rootFolderPath1,
},
},
expectedDelList: map[string]map[string]struct{}{
rootFolderPath1: getDelList("file"),
},
},
} }
for _, test := range table { for _, test := range table {
suite.Run(test.name, func() { suite.Run(test.name, func() {