From d390aaae4060b897a4dfd252ee9f3b1e4f27063c Mon Sep 17 00:00:00 2001
From: ashmrtn <ashmrtnz@alcion.ai>
Date: Wed, 22 Mar 2023 15:19:58 -0700
Subject: [PATCH] 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] :white_check_mark: Yes, it's included
- [ ] :clock1: Yes, but in a later PR
- [ ] :no_entry: No

#### Type of change

- [ ] :sunflower: Feature
- [x] :bug: Bugfix
- [ ] :world_map: Documentation
- [ ] :robot: Supportability/Tests
- [ ] :computer: CI/Deployment
- [ ] :broom: Tech Debt/Cleanup

#### Issue(s)

* closes #2915

#### Test Plan

- [ ] :muscle: Manual
- [x] :zap: Unit test
- [ ] :green_heart: E2E
---
 CHANGELOG.md                                  |  1 +
 .../connector/onedrive/collections.go         | 22 +++++++------------
 .../connector/onedrive/collections_test.go    |  9 ++++++--
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7e15da9f1..519a70083 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
 
diff --git a/src/internal/connector/onedrive/collections.go b/src/internal/connector/onedrive/collections.go
index a482de3d7..411154891 100644
--- a/src/internal/connector/onedrive/collections.go
+++ b/src/internal/connector/onedrive/collections.go
@@ -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
 			}
 
diff --git a/src/internal/connector/onedrive/collections_test.go b/src/internal/connector/onedrive/collections_test.go
index 25448abd6..c12d1b390 100644
--- a/src/internal/connector/onedrive/collections_test.go
+++ b/src/internal/connector/onedrive/collections_test.go
@@ -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,