continue if collection path is nil (#2912)

Currently code panics if it tries to find FullPath for a drive item and its not present.
Added a continue and skipped the item if FullPath is not present.

Root cause of bug is caused by having a folder in the previous paths map, having
the delta token expire, and seeing the folder be deleted in the delta results

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

#### Test Plan

- [ ] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E

CHANGELOG.md

@@ -9,6 +9,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.
 
 ## [v0.6.1] (beta) - 2023-03-21
 

src/go.sum

@@ -266,8 +266,6 @@ github.com/matttproud/golang_protobuf_extensions v1.0.2 h1:hAHbPm5IJGijwng3PWk09
 github.com/matttproud/golang_protobuf_extensions v1.0.2/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4=
 github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d h1:5PJl274Y63IEHC+7izoQE9x6ikvDFZS2mDVS3drnohI=
 github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE=
-github.com/microsoft/kiota-abstractions-go v0.17.3 h1:RmQ9RXzXFkQh8QnFcQ31QM2ou8MJksQe0/H8shz8CNI=
-github.com/microsoft/kiota-abstractions-go v0.17.3/go.mod h1:0lbPErVO6Rj3HHpntNYW/OFmHhJJ1ewPdsi1xPxYIMc=
 github.com/microsoft/kiota-abstractions-go v0.18.0 h1:H1kQE5hAq/7Q8gENPJ1Y7DuvG9QqKCpglN8D7TJi9qY=
 github.com/microsoft/kiota-abstractions-go v0.18.0/go.mod h1:0lbPErVO6Rj3HHpntNYW/OFmHhJJ1ewPdsi1xPxYIMc=
 github.com/microsoft/kiota-authentication-azure-go v0.6.0 h1:Il9bLO34J6D8DY89xYAXoGh9muvlphayqG4eihyT6B8=

src/internal/connector/onedrive/collection.go

@@ -178,13 +178,13 @@ func (oc *Collection) Add(item models.DriveItemable) bool {
 }
 
 // Remove removes a item from the collection
-func (oc *Collection) Remove(item models.DriveItemable) bool {
+func (oc *Collection) Remove(itemID string) bool {
-	_, found := oc.driveItems[ptr.Val(item.GetId())]
+	_, found := oc.driveItems[itemID]
 	if !found {
 		return false
 	}
 
-	delete(oc.driveItems, ptr.Val(item.GetId()))
+	delete(oc.driveItems, itemID)
 
 	return true
 }

src/internal/connector/onedrive/collections.go

@@ -347,9 +347,14 @@ func (c *Collections) Get(
 		logger.Ctx(ictx).Infow(
 			"persisted metadata for drive",
 			"num_paths_entries", len(paths),
-			"num_deltas_entries", numDeltas)
+			"num_deltas_entries", numDeltas,
+			"delta_reset", delta.Reset)
 
-		if !delta.Reset {
+		// For both cases we don't need to do set difference on folder map if the
+		// delta token was valid because we should see all the changes.
+		if !delta.Reset && len(excluded) == 0 {
+			continue
+		} else if !delta.Reset {
 			p, err := GetCanonicalPath(
 				fmt.Sprintf(rootDrivePattern, driveID),
 				c.tenant,
@@ -376,8 +381,11 @@ func (c *Collections) Get(
 		// Set all folders in previous backup but not in the current
 		// one with state deleted
 		modifiedPaths := map[string]struct{}{}
+
 		for _, p := range c.CollectionMap[driveID] {
-			modifiedPaths[p.FullPath().String()] = struct{}{}
+			if p.FullPath() != nil {
+				modifiedPaths[p.FullPath().String()] = struct{}{}
+			}
 		}
 
 		for fldID, p := range oldPaths {
@@ -493,6 +501,7 @@ func (c *Collections) handleDelete(
 	oldPaths, newPaths map[string]string,
 	isFolder bool,
 	excluded map[string]struct{},
+	invalidPrevDelta bool,
 ) error {
 	if !isFolder {
 		excluded[itemID+DataFileSuffix] = struct{}{}
@@ -526,11 +535,18 @@ func (c *Collections) handleDelete(
 	// the deleted folder/package.
 	delete(newPaths, itemID)
 
-	if prevPath == nil {
+	if prevPath == nil || invalidPrevDelta {
-		// It is possible that an item was created and
+		// It is possible that an item was created and deleted between two delta
-		// deleted between two delta invocations. In
+		// invocations. In that case, it will only produce a single delete entry in
-		// that case, it will only produce a single
+		// the delta response.
-		// delete entry in the delta response.
+		//
+		// It's also possible the item was made and deleted while getting the delta
+		// results or our delta token expired and the folder was seen and now is
+		// marked as deleted. If either of those is the case we should try to delete
+		// the collection with this ID so it doesn't show up with items. For the
+		// latter case, we rely on the set difference in the Get() function to find
+		// folders that need to be marked as deleted and make collections for them.
+		delete(c.CollectionMap[driveID], itemID)
 		return nil
 	}
 
@@ -659,6 +675,7 @@ func (c *Collections) UpdateCollections(
 				newPaths,
 				isFolder,
 				excluded,
+				invalidPrevDelta,
 			); err != nil {
 				return clues.Stack(err).WithClues(ictx)
 			}
@@ -671,6 +688,8 @@ func (c *Collections) UpdateCollections(
 			el.AddRecoverable(clues.Stack(err).
 				WithClues(ictx).
 				Label(fault.LabelForceNoBackupCreation))
+
+			continue
 		}
 
 		// Skip items that don't match the folder selectors we were given.
@@ -763,7 +782,7 @@ func (c *Collections) UpdateCollections(
 					return clues.New("previous collection not found").WithClues(ictx)
 				}
 
-				removed := pcollection.Remove(item)
+				removed := pcollection.Remove(itemID)
 				if !removed {
 					return clues.New("removing from prev collection").WithClues(ictx)
 				}

src/internal/connector/onedrive/collections_test.go

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