cache created folders when restoring drive items (#3220)

drive items aren't currently getting cached after creation.  Instead, we do a lookup on the folder name every time we walk the folder hierarchy toward the next child to restore.  Normally this would work fine, but apparently if the folder is named `folder`, then graph api returns nothing, causing the process to fail when repeatedly creating the folder.

---

#### Does this PR need a docs update or release note?

- [x] ✅ Yes, it's included

#### Type of change

- [x] 🐛 Bugfix

#### Test Plan

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

CHANGELOG.md

@@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Graph API requests that return an ECONNRESET error are now retried.
 - Fixed edge case in incremental backups where moving a subfolder, deleting and recreating the subfolder's original parent folder, and moving the subfolder back to where it started would skip backing up unchanged items in the subfolder.
 - SharePoint now correctly displays site urls on `backup list`, instead of the site id.
+- Drives with a directory containing a folder named 'folder' will now restore without error.
 
 ### Known Issues
 - Restoring a OneDrive or SharePoint file with the same name as a file with that name as its M365 ID may restore both items.

src/internal/connector/graph_connector_onedrive_test.go

@@ -106,9 +106,11 @@ func onedriveMetadata(
 }
 
 var (
-	fileName    = "test-file.txt"
+	fileName          = "test-file.txt"
-	folderAName = "folder-a"
+	folderAName       = "folder-a"
-	folderBName = "b"
+	folderBName       = "b"
+	folderNamedFolder = "folder"
+	rootFolder        = "root:"
 
 	fileAData = []byte(strings.Repeat("a", 33))
 	fileBData = []byte(strings.Repeat("b", 65))
@@ -254,7 +256,7 @@ func (c *onedriveCollection) withPermissions(perm permData) *onedriveCollection
 		metaName = ""
 	}
 
-	if name == "root:" {
+	if name == rootFolder {
 		return c
 	}
 
@@ -544,6 +546,11 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsInheritanceR
 	testPermissionsInheritanceRestoreAndBackup(suite, version.Backup)
 }
 
+func (suite *GraphConnectorOneDriveIntegrationSuite) TestRestoreFolderNamedFolderRegression() {
+	// No reason why it couldn't work with previous versions, but this is when it got introduced.
+	testRestoreFolderNamedFolderRegression(suite, version.All8MigrateUserPNToID)
+}
+
 // ---------------------------------------------------------------------------
 // OneDrive regression
 // ---------------------------------------------------------------------------
@@ -600,11 +607,15 @@ func (suite *GraphConnectorOneDriveNightlySuite) TestPermissionsBackupAndNoResto
 }
 
 func (suite *GraphConnectorOneDriveNightlySuite) TestPermissionsInheritanceRestoreAndBackup() {
-	// No reason why it couldn't work with previous versions, but this is when it
+	// No reason why it couldn't work with previous versions, but this is when it got introduced.
-	// got introduced.
 	testPermissionsInheritanceRestoreAndBackup(suite, version.OneDrive4DirIncludesPermissions)
 }
 
+func (suite *GraphConnectorOneDriveNightlySuite) TestRestoreFolderNamedFolderRegression() {
+	// No reason why it couldn't work with previous versions, but this is when it got introduced.
+	testRestoreFolderNamedFolderRegression(suite, version.All8MigrateUserPNToID)
+}
+
 func testRestoreAndBackupMultipleFilesAndFoldersNoPermissions(
 	suite oneDriveSuite,
 	startVersion int,
@@ -618,31 +629,30 @@ func testRestoreAndBackupMultipleFilesAndFoldersNoPermissions(
 		ctx,
 		suite.BackupService(),
 		suite.Service(),
-		suite.BackupResourceOwner(),
+		suite.BackupResourceOwner())
-	)
 
 	rootPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 	}
 	folderAPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 		folderAName,
 	}
 	subfolderBPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 		folderAName,
 		folderBName,
 	}
 	subfolderAPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 		folderAName,
 		folderBName,
 		folderAName,
@@ -650,7 +660,7 @@ func testRestoreAndBackupMultipleFilesAndFoldersNoPermissions(
 	folderBPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 		folderBName,
 	}
 
@@ -744,8 +754,7 @@ func testRestoreAndBackupMultipleFilesAndFoldersNoPermissions(
 				control.Options{
 					RestorePermissions: true,
 					ToggleFeatures:     control.Toggles{},
-				},
+				})
-			)
 		})
 	}
 }
@@ -762,8 +771,7 @@ func testPermissionsRestoreAndBackup(suite oneDriveSuite, startVersion int) {
 		ctx,
 		suite.BackupService(),
 		suite.Service(),
-		suite.BackupResourceOwner(),
+		suite.BackupResourceOwner())
-	)
 
 	fileName2 := "test-file2.txt"
 	folderCName := "folder-c"
@@ -771,32 +779,32 @@ func testPermissionsRestoreAndBackup(suite oneDriveSuite, startVersion int) {
 	rootPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 	}
 	folderAPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 		folderAName,
 	}
 	folderBPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 		folderBName,
 	}
 	// For skipped test
 	// subfolderAPath := []string{
 	// 	"drives",
 	// 	driveID,
-	// 	"root:",
+	// 	rootFolder,
 	// 	folderBName,
 	// 	folderAName,
 	// }
 	folderCPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 		folderCName,
 	}
 
@@ -958,8 +966,7 @@ func testPermissionsRestoreAndBackup(suite oneDriveSuite, startVersion int) {
 				control.Options{
 					RestorePermissions: true,
 					ToggleFeatures:     control.Toggles{},
-				},
+				})
-			)
 		})
 	}
 }
@@ -976,15 +983,14 @@ func testPermissionsBackupAndNoRestore(suite oneDriveSuite, startVersion int) {
 		ctx,
 		suite.BackupService(),
 		suite.Service(),
-		suite.BackupResourceOwner(),
+		suite.BackupResourceOwner())
-	)
 
 	inputCols := []onedriveColInfo{
 		{
 			pathElements: []string{
 				"drives",
 				driveID,
-				"root:",
+				rootFolder,
 			},
 			files: []itemData{
 				{
@@ -1005,7 +1011,7 @@ func testPermissionsBackupAndNoRestore(suite oneDriveSuite, startVersion int) {
 			pathElements: []string{
 				"drives",
 				driveID,
-				"root:",
+				rootFolder,
 			},
 			files: []itemData{
 				{
@@ -1041,8 +1047,7 @@ func testPermissionsBackupAndNoRestore(suite oneDriveSuite, startVersion int) {
 				control.Options{
 					RestorePermissions: false,
 					ToggleFeatures:     control.Toggles{},
-				},
+				})
-			)
 		})
 	}
 }
@@ -1062,8 +1067,7 @@ func testPermissionsInheritanceRestoreAndBackup(suite oneDriveSuite, startVersio
 		ctx,
 		suite.BackupService(),
 		suite.Service(),
-		suite.BackupResourceOwner(),
+		suite.BackupResourceOwner())
-	)
 
 	folderAName := "custom"
 	folderBName := "inherited"
@@ -1072,32 +1076,32 @@ func testPermissionsInheritanceRestoreAndBackup(suite oneDriveSuite, startVersio
 	rootPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 	}
 	folderAPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 		folderAName,
 	}
 	subfolderAAPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 		folderAName,
 		folderAName,
 	}
 	subfolderABPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 		folderAName,
 		folderBName,
 	}
 	subfolderACPath := []string{
 		"drives",
 		driveID,
-		"root:",
+		rootFolder,
 		folderAName,
 		folderCName,
 	}
@@ -1214,6 +1218,117 @@ func testPermissionsInheritanceRestoreAndBackup(suite oneDriveSuite, startVersio
 			}
 
 			runRestoreBackupTestVersions(
+				t,
+				suite.Account(),
+				testData,
+				suite.Tenant(),
+				[]string{suite.BackupResourceOwner()},
+				control.Options{
+					RestorePermissions: true,
+					ToggleFeatures:     control.Toggles{},
+				})
+		})
+	}
+}
+
+func testRestoreFolderNamedFolderRegression(
+	suite oneDriveSuite,
+	startVersion int,
+) {
+	ctx, flush := tester.NewContext()
+	defer flush()
+
+	// Get the default drive ID for the test user.
+	driveID := mustGetDefaultDriveID(
+		suite.T(),
+		ctx,
+		suite.BackupService(),
+		suite.Service(),
+		suite.BackupResourceOwner())
+
+	rootPath := []string{
+		"drives",
+		driveID,
+		rootFolder,
+	}
+	folderFolderPath := []string{
+		"drives",
+		driveID,
+		rootFolder,
+		folderNamedFolder,
+	}
+	subfolderPath := []string{
+		"drives",
+		driveID,
+		rootFolder,
+		folderNamedFolder,
+		folderBName,
+	}
+
+	cols := []onedriveColInfo{
+		{
+			pathElements: rootPath,
+			files: []itemData{
+				{
+					name: fileName,
+					data: fileAData,
+				},
+			},
+			folders: []itemData{
+				{
+					name: folderNamedFolder,
+				},
+				{
+					name: folderBName,
+				},
+			},
+		},
+		{
+			pathElements: folderFolderPath,
+			files: []itemData{
+				{
+					name: fileName,
+					data: fileBData,
+				},
+			},
+			folders: []itemData{
+				{
+					name: folderBName,
+				},
+			},
+		},
+		{
+			pathElements: subfolderPath,
+			files: []itemData{
+				{
+					name: fileName,
+					data: fileCData,
+				},
+			},
+			folders: []itemData{
+				{
+					name: folderNamedFolder,
+				},
+			},
+		},
+	}
+
+	expected := testDataForInfo(suite.T(), suite.BackupService(), cols, version.Backup)
+
+	for vn := startVersion; vn <= version.Backup; vn++ {
+		suite.Run(fmt.Sprintf("Version%d", vn), func() {
+			t := suite.T()
+			input := testDataForInfo(t, suite.BackupService(), cols, vn)
+
+			testData := restoreBackupInfoMultiVersion{
+				service:             suite.BackupService(),
+				resource:            suite.Resource(),
+				backupVersion:       vn,
+				collectionsPrevious: input,
+				collectionsLatest:   expected,
+			}
+
+			runRestoreTestWithVerion(
 				t,
 				suite.Account(),
 				testData,

src/internal/connector/graph_connector_test.go

@@ -606,6 +606,43 @@ func runRestoreBackupTest(
 		test.collections)
 }
 
+// runRestoreTest restores with data using the test's backup version
+func runRestoreTestWithVerion(
+	t *testing.T,
+	acct account.Account,
+	test restoreBackupInfoMultiVersion,
+	tenant string,
+	resourceOwners []string,
+	opts control.Options,
+) {
+	ctx, flush := tester.NewContext()
+	defer flush()
+
+	config := configInfo{
+		acct:           acct,
+		opts:           opts,
+		resource:       test.resource,
+		service:        test.service,
+		tenant:         tenant,
+		resourceOwners: resourceOwners,
+		dest:           tester.DefaultTestRestoreDestination(),
+	}
+
+	totalItems, _, collections, _ := getCollectionsAndExpected(
+		t,
+		config,
+		test.collectionsPrevious,
+		test.backupVersion)
+
+	runRestore(
+		t,
+		ctx,
+		config,
+		test.backupVersion,
+		collections,
+		totalItems)
+}
+
 // runRestoreBackupTestVersions restores with data from an older
 // version of the backup and check the restored data against the
 // something that would be in the form of a newer backup.

src/internal/connector/onedrive/api/drive.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/alcionai/clues"
 	abstractions "github.com/microsoft/kiota-abstractions-go"
+	"github.com/microsoftgraph/msgraph-sdk-go/drive"
 	"github.com/microsoftgraph/msgraph-sdk-go/drives"
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
 	"github.com/microsoftgraph/msgraph-sdk-go/sites"
@@ -323,3 +324,51 @@ func GetDriveByID(
 
 	return d, nil
 }
+
+func GetDriveRoot(
+	ctx context.Context,
+	srv graph.Servicer,
+	driveID string,
+) (models.DriveItemable, error) {
+	root, err := srv.Client().DrivesById(driveID).Root().Get(ctx, nil)
+	if err != nil {
+		return nil, graph.Wrap(ctx, err, "getting drive root")
+	}
+
+	return root, nil
+}
+
+const itemByPathRawURLFmt = "https://graph.microsoft.com/v1.0/drives/%s/items/%s:/%s"
+
+var ErrFolderNotFound = clues.New("folder not found")
+
+// GetFolderByName will lookup the specified folder by name within the parentFolderID folder.
+func GetFolderByName(
+	ctx context.Context,
+	service graph.Servicer,
+	driveID, parentFolderID, folder string,
+) (models.DriveItemable, error) {
+	// The `Children().Get()` API doesn't yet support $filter, so using that to find a folder
+	// will be sub-optimal.
+	// Instead, we leverage OneDrive path-based addressing -
+	// https://learn.microsoft.com/en-us/graph/onedrive-addressing-driveitems#path-based-addressing
+	// - which allows us to lookup an item by its path relative to the parent ID
+	rawURL := fmt.Sprintf(itemByPathRawURLFmt, driveID, parentFolderID, folder)
+	builder := drive.NewItemsDriveItemItemRequestBuilder(rawURL, service.Adapter())
+
+	foundItem, err := builder.Get(ctx, nil)
+	if err != nil {
+		if graph.IsErrDeletedInFlight(err) {
+			return nil, graph.Stack(ctx, clues.Stack(ErrFolderNotFound, err))
+		}
+
+		return nil, graph.Wrap(ctx, err, "getting folder")
+	}
+
+	// Check if the item found is a folder, fail the call if not
+	if foundItem.GetFolder() == nil {
+		return nil, graph.Wrap(ctx, ErrFolderNotFound, "item is not a folder")
+	}
+
+	return foundItem, nil
+}

src/internal/connector/onedrive/drive.go

@@ -18,8 +18,6 @@ import (
 	"github.com/alcionai/corso/src/pkg/logger"
 )
 
-var errFolderNotFound = clues.New("folder not found")
-
 const (
 	maxDrivesRetries = 3
 
@@ -27,7 +25,6 @@ const (
 	// graph response
 	nextLinkKey           = "@odata.nextLink"
 	itemChildrenRawURLFmt = "https://graph.microsoft.com/v1.0/drives/%s/items/%s/children"
-	itemByPathRawURLFmt   = "https://graph.microsoft.com/v1.0/drives/%s/items/%s:/%s"
 	itemNotFoundErrorCode = "itemNotFound"
 )
 
@@ -195,42 +192,6 @@ func collectItems(
 	return DeltaUpdate{URL: newDeltaURL, Reset: invalidPrevDelta}, newPaths, excluded, nil
 }
 
-// getFolder will lookup the specified folder name under `parentFolderID`
-func getFolder(
-	ctx context.Context,
-	service graph.Servicer,
-	driveID, parentFolderID, folderName string,
-) (models.DriveItemable, error) {
-	// The `Children().Get()` API doesn't yet support $filter, so using that to find a folder
-	// will be sub-optimal.
-	// Instead, we leverage OneDrive path-based addressing -
-	// https://learn.microsoft.com/en-us/graph/onedrive-addressing-driveitems#path-based-addressing
-	// - which allows us to lookup an item by its path relative to the parent ID
-	rawURL := fmt.Sprintf(itemByPathRawURLFmt, driveID, parentFolderID, folderName)
-	builder := drive.NewItemsDriveItemItemRequestBuilder(rawURL, service.Adapter())
-
-	var (
-		foundItem models.DriveItemable
-		err       error
-	)
-
-	foundItem, err = builder.Get(ctx, nil)
-	if err != nil {
-		if graph.IsErrDeletedInFlight(err) {
-			return nil, graph.Stack(ctx, clues.Stack(errFolderNotFound, err))
-		}
-
-		return nil, graph.Wrap(ctx, err, "getting folder")
-	}
-
-	// Check if the item found is a folder, fail the call if not
-	if foundItem.GetFolder() == nil {
-		return nil, graph.Stack(ctx, errFolderNotFound)
-	}
-
-	return foundItem, nil
-}
-
 // Create a new item in the specified folder
 func CreateItem(
 	ctx context.Context,

src/internal/connector/onedrive/drive_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/alcionai/corso/src/pkg/control"
 	"github.com/alcionai/corso/src/pkg/fault"
 	"github.com/alcionai/corso/src/pkg/logger"
+	"github.com/alcionai/corso/src/pkg/path"
 	"github.com/alcionai/corso/src/pkg/selectors"
 )
 
@@ -282,7 +283,7 @@ type OneDriveSuite struct {
 	userID string
 }
 
-func TestOneDriveDriveSuite(t *testing.T) {
+func TestOneDriveSuite(t *testing.T) {
 	suite.Run(t, &OneDriveSuite{
 		Suite: tester.NewIntegrationSuite(
 			t,
@@ -329,15 +330,20 @@ func (suite *OneDriveSuite) TestCreateGetDeleteFolder() {
 		}
 	}()
 
-	folderID, err := CreateRestoreFolders(ctx, gs, driveID, folderElements)
+	rootFolder, err := api.GetDriveRoot(ctx, gs, driveID)
+	require.NoError(t, err, clues.ToCore(err))
+
+	restoreFolders := path.Builder{}.Append(folderElements...)
+
+	folderID, err := CreateRestoreFolders(ctx, gs, driveID, ptr.Val(rootFolder.GetId()), restoreFolders, NewFolderCache())
 	require.NoError(t, err, clues.ToCore(err))
 
 	folderIDs = append(folderIDs, folderID)
 
 	folderName2 := "Corso_Folder_Test_" + common.FormatNow(common.SimpleTimeTesting)
-	folderElements = append(folderElements, folderName2)
+	restoreFolders = restoreFolders.Append(folderName2)
 
-	folderID, err = CreateRestoreFolders(ctx, gs, driveID, folderElements)
+	folderID, err = CreateRestoreFolders(ctx, gs, driveID, ptr.Val(rootFolder.GetId()), restoreFolders, NewFolderCache())
 	require.NoError(t, err, clues.ToCore(err))
 
 	folderIDs = append(folderIDs, folderID)
@@ -390,8 +396,8 @@ func (fm testFolderMatcher) IsAny() bool {
 	return fm.scope.IsAny(selectors.OneDriveFolder)
 }
 
-func (fm testFolderMatcher) Matches(path string) bool {
+func (fm testFolderMatcher) Matches(p string) bool {
-	return fm.scope.Matches(selectors.OneDriveFolder, path)
+	return fm.scope.Matches(selectors.OneDriveFolder, p)
 }
 
 func (suite *OneDriveSuite) TestOneDriveNewCollections() {

src/internal/connector/onedrive/folder_cache.go

@@ -0,0 +1,28 @@
+package onedrive
+
+import (
+	"github.com/microsoftgraph/msgraph-sdk-go/models"
+
+	"github.com/alcionai/corso/src/pkg/path"
+)
+
+// TODO: refactor to comply with graph/cache_container
+
+type folderCache struct {
+	cache map[string]models.DriveItemable
+}
+
+func NewFolderCache() *folderCache {
+	return &folderCache{
+		cache: map[string]models.DriveItemable{},
+	}
+}
+
+func (c *folderCache) get(loc *path.Builder) (models.DriveItemable, bool) {
+	mdi, ok := c.cache[loc.String()]
+	return mdi, ok
+}
+
+func (c *folderCache) set(loc *path.Builder, mdi models.DriveItemable) {
+	c.cache[loc.String()] = mdi
+}

src/internal/connector/onedrive/item_test.go

@@ -155,7 +155,7 @@ func (suite *ItemIntegrationSuite) TestItemWriter() {
 			require.NoError(t, err, clues.ToCore(err))
 
 			// Test Requirement 2: "Test Folder" should exist
-			folder, err := getFolder(ctx, srv, test.driveID, ptr.Val(root.GetId()), "Test Folder")
+			folder, err := api.GetFolderByName(ctx, srv, test.driveID, ptr.Val(root.GetId()), "Test Folder")
 			require.NoError(t, err, clues.ToCore(err))
 
 			newFolderName := "testfolder_" + common.FormatNow(common.SimpleTimeTesting)
@@ -184,8 +184,8 @@ func (suite *ItemIntegrationSuite) TestItemWriter() {
 
 			// HACK: Leveraging this to test getFolder behavior for a file. `getFolder()` on the
 			// newly created item should fail because it's a file not a folder
-			_, err = getFolder(ctx, srv, test.driveID, ptr.Val(newFolder.GetId()), newItemName)
+			_, err = api.GetFolderByName(ctx, srv, test.driveID, ptr.Val(newFolder.GetId()), newItemName)
-			require.ErrorIs(t, err, errFolderNotFound, clues.ToCore(err))
+			require.ErrorIs(t, err, api.ErrFolderNotFound, clues.ToCore(err))
 
 			// Initialize a 100KB mockDataProvider
 			td, writeSize := mockDataReader(int64(100 * 1024))
@@ -237,11 +237,11 @@ func (suite *ItemIntegrationSuite) TestDriveGetFolder() {
 			require.NoError(t, err, clues.ToCore(err))
 
 			// Lookup a folder that doesn't exist
-			_, err = getFolder(ctx, srv, test.driveID, ptr.Val(root.GetId()), "FolderDoesNotExist")
+			_, err = api.GetFolderByName(ctx, srv, test.driveID, ptr.Val(root.GetId()), "FolderDoesNotExist")
-			require.ErrorIs(t, err, errFolderNotFound, clues.ToCore(err))
+			require.ErrorIs(t, err, api.ErrFolderNotFound, clues.ToCore(err))
 
 			// Lookup a folder that does exist
-			_, err = getFolder(ctx, srv, test.driveID, ptr.Val(root.GetId()), "")
+			_, err = api.GetFolderByName(ctx, srv, test.driveID, ptr.Val(root.GetId()), "")
 			require.NoError(t, err, clues.ToCore(err))
 		})
 	}

src/internal/connector/onedrive/permission.go

@@ -85,50 +85,6 @@ func getCollectionMetadata(
 	return meta, nil
 }
 
-// createRestoreFoldersWithPermissions creates the restore folder hierarchy in
-// the specified drive and returns the folder ID of the last folder entry in the
-// hierarchy. Permissions are only applied to the last folder in the hierarchy.
-// Passing nil for the permissions results in just creating the folder(s).
-func createRestoreFoldersWithPermissions(
-	ctx context.Context,
-	creds account.M365Config,
-	service graph.Servicer,
-	drivePath *path.DrivePath,
-	restoreFolders []string,
-	folderPath path.Path,
-	folderMetadata Metadata,
-	folderMetas map[string]Metadata,
-	permissionIDMappings map[string]string,
-	restorePerms bool,
-) (string, error) {
-	id, err := CreateRestoreFolders(ctx, service, drivePath.DriveID, restoreFolders)
-	if err != nil {
-		return "", err
-	}
-
-	if len(drivePath.Folders) == 0 {
-		// No permissions for root folder
-		return id, nil
-	}
-
-	if !restorePerms {
-		return id, nil
-	}
-
-	err = RestorePermissions(
-		ctx,
-		creds,
-		service,
-		drivePath.DriveID,
-		id,
-		folderPath,
-		folderMetadata,
-		folderMetas,
-		permissionIDMappings)
-
-	return id, err
-}
-
 // isSamePermission checks equality of two UserPermission objects
 func isSamePermission(p1, p2 UserPermission) bool {
 	// EntityID can be empty for older backups and Email can be empty

src/internal/connector/onedrive/restore.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/connector/graph"
+	"github.com/alcionai/corso/src/internal/connector/onedrive/api"
 	"github.com/alcionai/corso/src/internal/connector/onedrive/metadata"
 	"github.com/alcionai/corso/src/internal/connector/support"
 	"github.com/alcionai/corso/src/internal/data"
@@ -52,6 +53,8 @@ func RestoreCollections(
 		// permissionIDMappings is used to map between old and new id
 		// of permissions as we restore them
 		permissionIDMappings = map[string]string{}
+		fc                   = NewFolderCache()
+		rootIDCache          = map[string]string{}
 	)
 
 	ctx = clues.Add(
@@ -90,6 +93,8 @@ func RestoreCollections(
 			dc,
 			folderMetas,
 			permissionIDMappings,
+			fc,
+			rootIDCache,
 			OneDriveSource,
 			dest.ContainerName,
 			deets,
@@ -129,6 +134,8 @@ func RestoreCollection(
 	dc data.RestoreCollection,
 	folderMetas map[string]Metadata,
 	permissionIDMappings map[string]string,
+	fc *folderCache,
+	rootIDCache map[string]string, // map of drive id -> root folder ID
 	source driveSource,
 	restoreContainerName string,
 	deets *details.Builder,
@@ -150,12 +157,24 @@ func RestoreCollection(
 		return metrics, clues.Wrap(err, "creating drive path").WithClues(ctx)
 	}
 
+	if rootIDCache == nil {
+		rootIDCache = map[string]string{}
+	}
+
+	if _, ok := rootIDCache[drivePath.DriveID]; !ok {
+		root, err := api.GetDriveRoot(ctx, service, drivePath.DriveID)
+		if err != nil {
+			return metrics, clues.Wrap(err, "getting drive root id")
+		}
+
+		rootIDCache[drivePath.DriveID] = ptr.Val(root.GetId())
+	}
+
 	// Assemble folder hierarchy we're going to restore into (we recreate the folder hierarchy
 	// from the backup under this the restore folder instead of root)
-	// i.e. Restore into `<drive>/root:/<restoreContainerName>/<original folder path>`
+	// i.e. Restore into `<restoreContainerName>/<original folder path>`
-
+	// the drive into which this folder gets restored is tracked separately in drivePath.
-	restoreFolderElements := []string{restoreContainerName}
+	restoreFolderElements := path.Builder{}.Append(restoreContainerName).Append(drivePath.Folders...)
-	restoreFolderElements = append(restoreFolderElements, drivePath.Folders...)
 
 	ctx = clues.Add(
 		ctx,
@@ -183,10 +202,12 @@ func RestoreCollection(
 		creds,
 		service,
 		drivePath,
+		rootIDCache[drivePath.DriveID],
 		restoreFolderElements,
 		dc.FullPath(),
 		colMeta,
 		folderMetas,
+		fc,
 		permissionIDMappings,
 		restorePerms)
 	if err != nil {
@@ -541,43 +562,112 @@ func restoreV6File(
 	return itemInfo, nil
 }
 
+// createRestoreFoldersWithPermissions creates the restore folder hierarchy in
+// the specified drive and returns the folder ID of the last folder entry in the
+// hierarchy. Permissions are only applied to the last folder in the hierarchy.
+// Passing nil for the permissions results in just creating the folder(s).
+// folderCache is mutated, as a side effect of populating the items.
+func createRestoreFoldersWithPermissions(
+	ctx context.Context,
+	creds account.M365Config,
+	service graph.Servicer,
+	drivePath *path.DrivePath,
+	driveRootID string,
+	restoreFolders *path.Builder,
+	folderPath path.Path,
+	folderMetadata Metadata,
+	folderMetas map[string]Metadata,
+	fc *folderCache,
+	permissionIDMappings map[string]string,
+	restorePerms bool,
+) (string, error) {
+	id, err := CreateRestoreFolders(
+		ctx,
+		service,
+		drivePath.DriveID,
+		driveRootID,
+		restoreFolders,
+		fc)
+	if err != nil {
+		return "", err
+	}
+
+	if len(drivePath.Folders) == 0 {
+		// No permissions for root folder
+		return id, nil
+	}
+
+	if !restorePerms {
+		return id, nil
+	}
+
+	err = RestorePermissions(
+		ctx,
+		creds,
+		service,
+		drivePath.DriveID,
+		id,
+		folderPath,
+		folderMetadata,
+		folderMetas,
+		permissionIDMappings)
+
+	return id, err
+}
+
 // CreateRestoreFolders creates the restore folder hierarchy in the specified
 // drive and returns the folder ID of the last folder entry in the hierarchy.
+// folderCache is mutated, as a side effect of populating the items.
 func CreateRestoreFolders(
 	ctx context.Context,
 	service graph.Servicer,
-	driveID string,
+	driveID, driveRootID string,
-	restoreFolders []string,
+	restoreFolders *path.Builder,
+	fc *folderCache,
 ) (string, error) {
-	driveRoot, err := service.Client().DrivesById(driveID).Root().Get(ctx, nil)
+	var (
-	if err != nil {
+		location       = &path.Builder{}
-		return "", graph.Wrap(ctx, err, "getting drive root")
+		parentFolderID = driveRootID
-	}
+		folders        = restoreFolders.Elements()
+	)
 
-	parentFolderID := ptr.Val(driveRoot.GetId())
+	for _, folder := range folders {
-	ctx = clues.Add(ctx, "drive_root_id", parentFolderID)
+		location = location.Append(folder)
+		ictx := clues.Add(
+			ctx,
+			"creating_restore_folder", folder,
+			"restore_folder_location", location,
+			"parent_of_restore_folder", parentFolderID)
 
-	logger.Ctx(ctx).Debug("found drive root")
+		if fl, ok := fc.get(location); ok {
-
+			parentFolderID = ptr.Val(fl.GetId())
-	for _, folder := range restoreFolders {
+			// folder was already created, move on to the child
-		folderItem, err := getFolder(ctx, service, driveID, parentFolderID, folder)
-		if err == nil {
-			parentFolderID = ptr.Val(folderItem.GetId())
 			continue
 		}
 
-		if !errors.Is(err, errFolderNotFound) {
+		folderItem, err := api.GetFolderByName(ictx, service, driveID, parentFolderID, folder)
-			return "", clues.Wrap(err, "folder not found").With("folder_id", folder).WithClues(ctx)
+		if err != nil && !errors.Is(err, api.ErrFolderNotFound) {
+			return "", clues.Wrap(err, "getting folder by display name").WithClues(ctx)
 		}
 
+		// folder found, moving to next child
+		if err == nil {
+			parentFolderID = ptr.Val(folderItem.GetId())
+			fc.set(location, folderItem)
+
+			continue
+		}
+
+		// create the folder if not found
 		folderItem, err = CreateItem(ctx, service, driveID, parentFolderID, newItem(folder, true))
 		if err != nil {
 			return "", clues.Wrap(err, "creating folder")
 		}
 
 		parentFolderID = ptr.Val(folderItem.GetId())
+		fc.set(location, folderItem)
 
-		logger.Ctx(ctx).Debugw("resolved restore destination", "dest_id", parentFolderID)
+		logger.Ctx(ctx).Debug("resolved restore destination")
 	}
 
 	return parentFolderID, nil

src/internal/connector/sharepoint/collection_test.go

@@ -14,7 +14,6 @@ import (
 
 	"github.com/alcionai/corso/src/internal/common"
 	"github.com/alcionai/corso/src/internal/common/ptr"
-	"github.com/alcionai/corso/src/internal/connector/onedrive"
 	"github.com/alcionai/corso/src/internal/connector/sharepoint/api"
 	spMock "github.com/alcionai/corso/src/internal/connector/sharepoint/mock"
 	"github.com/alcionai/corso/src/internal/connector/support"
@@ -233,30 +232,3 @@ func (suite *SharePointCollectionSuite) TestListCollection_Restore() {
 		assert.NoError(t, err, clues.ToCore(err))
 	}
 }
-
-// TestRestoreLocation temporary test for greater restore operation
-// TODO delete after full functionality tested in GraphConnector
-func (suite *SharePointCollectionSuite) TestRestoreLocation() {
-	ctx, flush := tester.NewContext()
-	defer flush()
-
-	t := suite.T()
-
-	service := createTestService(t, suite.creds)
-	rootFolder := "General_" + common.FormatNow(common.SimpleTimeTesting)
-	folderID, err := createRestoreFolders(ctx, service, suite.siteID, []string{rootFolder})
-	require.NoError(t, err, clues.ToCore(err))
-	t.Log("FolderID: " + folderID)
-
-	_, err = createRestoreFolders(ctx, service, suite.siteID, []string{rootFolder, "Tsao"})
-	require.NoError(t, err, clues.ToCore(err))
-
-	// CleanUp
-	siteDrive, err := service.Client().SitesById(suite.siteID).Drive().Get(ctx, nil)
-	require.NoError(t, err, clues.ToCore(err))
-
-	driveID := ptr.Val(siteDrive.GetId())
-
-	err = onedrive.DeleteItem(ctx, service, driveID, folderID)
-	assert.NoError(t, err, clues.ToCore(err))
-}

src/internal/connector/sharepoint/restore.go

@@ -63,6 +63,7 @@ func RestoreCollections(
 				"category", category,
 				"destination", clues.Hide(dest.ContainerName),
 				"resource_owner", clues.Hide(dc.FullPath().ResourceOwner()))
+			driveFolderCache = onedrive.NewFolderCache()
 		)
 
 		switch dc.FullPath().Category() {
@@ -75,11 +76,14 @@ func RestoreCollections(
 				dc,
 				map[string]onedrive.Metadata{}, // Currently permission data is not stored for sharepoint
 				map[string]string{},
+				driveFolderCache,
+				nil,
 				onedrive.SharePointSource,
 				dest.ContainerName,
 				deets,
 				false,
 				errs)
+
 		case path.ListsCategory:
 			metrics, err = RestoreListCollection(
 				ictx,
@@ -88,6 +92,7 @@ func RestoreCollections(
 				dest.ContainerName,
 				deets,
 				errs)
+
 		case path.PagesCategory:
 			metrics, err = RestorePageCollection(
 				ictx,
@@ -96,6 +101,7 @@ func RestoreCollections(
 				dest.ContainerName,
 				deets,
 				errs)
+
 		default:
 			return nil, clues.Wrap(clues.New(category.String()), "category not supported").With("category", category)
 		}
@@ -117,23 +123,6 @@ func RestoreCollections(
 	return status, err
 }
 
-// createRestoreFolders creates the restore folder hierarchy in the specified drive and returns the folder ID
-// of the last folder entry given in the hierarchy
-func createRestoreFolders(
-	ctx context.Context,
-	service graph.Servicer,
-	siteID string,
-	restoreFolders []string,
-) (string, error) {
-	// Get Main Drive for Site, Documents
-	mainDrive, err := service.Client().SitesById(siteID).Drive().Get(ctx, nil)
-	if err != nil {
-		return "", graph.Wrap(ctx, err, "getting site drive root")
-	}
-
-	return onedrive.CreateRestoreFolders(ctx, service, ptr.Val(mainDrive.GetId()), restoreFolders)
-}
-
 // restoreListItem utility function restores a List to the siteID.
 // The name is changed to to Corso_Restore_{timeStame}_name
 // API Reference: https://learn.microsoft.com/en-us/graph/api/list-create?view=graph-rest-1.0&tabs=http