OneDrive invalid delta set difference (#2918)

When finding the set difference if the delta token has
expired do so by comparing folder IDs instead of paths.
This will keep from accidentally re-rooting subfolders
from a deleted folder onto a new folder at the same
path. This is safe because we're enumerating all files
and folders, so if the subtree wasn't actually deleted
it should be seen

---

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

#### Test Plan

- [ ] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2023-03-22 15:19:58 -07:00 committed by GitHub
parent 01a6304a62
commit d390aaae40
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 16 additions and 16 deletions

View File

@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- Fixed permissions restore in latest backup version
- 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.
## [v0.6.1] (beta) - 2023-03-21

View File

@ -378,24 +378,18 @@ func (c *Collections) Get(
continue
}
// Set all folders in previous backup but not in the current
// one with state deleted
modifiedPaths := map[string]struct{}{}
// Set all folders in previous backup but not in the current one with state
// deleted. Need to compare by ID because it's possible to make new folders
// with the same path as deleted old folders. We shouldn't merge items or
// subtrees if that happens though.
foundFolders := map[string]struct{}{}
for _, p := range c.CollectionMap[driveID] {
if p.FullPath() != nil {
modifiedPaths[p.FullPath().String()] = struct{}{}
}
for id := range c.CollectionMap[driveID] {
foundFolders[id] = struct{}{}
}
for fldID, p := range oldPaths {
if _, ok := paths[fldID]; ok {
continue
}
if _, ok := modifiedPaths[p]; ok {
// Original folder was deleted and new folder with the
// same name/path was created in its place
if _, ok := foundFolders[fldID]; ok {
continue
}

View File

@ -1850,8 +1850,13 @@ func (suite *OneDriveCollectionsUnitSuite) TestGet() {
},
},
expectedCollections: map[string]map[data.CollectionState][]string{
rootFolderPath1: {data.NewState: {}},
expectedPath1("/folder"): {data.NewState: {"folder2", "file"}},
rootFolderPath1: {data.NewState: {}},
expectedPath1("/folder"): {
// Old folder path should be marked as deleted since it should compare
// by ID.
data.DeletedState: {},
data.NewState: {"folder2", "file"},
},
},
expectedDeltaURLs: map[string]string{
driveID1: delta,