diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bf239842..a840f7587 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/src/internal/connector/graph_connector_onedrive_test.go b/src/internal/connector/graph_connector_onedrive_test.go index 529407205..2072d150e 100644 --- a/src/internal/connector/graph_connector_onedrive_test.go +++ b/src/internal/connector/graph_connector_onedrive_test.go @@ -106,9 +106,11 @@ func onedriveMetadata( } var ( - fileName = "test-file.txt" - folderAName = "folder-a" - folderBName = "b" + fileName = "test-file.txt" + folderAName = "folder-a" + 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 - // got introduced. + // No reason why it couldn't work with previous versions, but this is when it 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, diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 4c3c29faa..92d4dccb6 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/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. diff --git a/src/internal/connector/onedrive/api/drive.go b/src/internal/connector/onedrive/api/drive.go index f72cdf10f..3567ece4c 100644 --- a/src/internal/connector/onedrive/api/drive.go +++ b/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 +} diff --git a/src/internal/connector/onedrive/drive.go b/src/internal/connector/onedrive/drive.go index fd0c8859a..99487c66b 100644 --- a/src/internal/connector/onedrive/drive.go +++ b/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, diff --git a/src/internal/connector/onedrive/drive_test.go b/src/internal/connector/onedrive/drive_test.go index fde067adf..28310e8ed 100644 --- a/src/internal/connector/onedrive/drive_test.go +++ b/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 { - return fm.scope.Matches(selectors.OneDriveFolder, path) +func (fm testFolderMatcher) Matches(p string) bool { + return fm.scope.Matches(selectors.OneDriveFolder, p) } func (suite *OneDriveSuite) TestOneDriveNewCollections() { diff --git a/src/internal/connector/onedrive/folder_cache.go b/src/internal/connector/onedrive/folder_cache.go new file mode 100644 index 000000000..696d42819 --- /dev/null +++ b/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 +} diff --git a/src/internal/connector/onedrive/item_test.go b/src/internal/connector/onedrive/item_test.go index 992b446d1..60c5e6866 100644 --- a/src/internal/connector/onedrive/item_test.go +++ b/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) - require.ErrorIs(t, err, errFolderNotFound, clues.ToCore(err)) + _, err = api.GetFolderByName(ctx, srv, test.driveID, ptr.Val(newFolder.GetId()), newItemName) + 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") - require.ErrorIs(t, err, errFolderNotFound, clues.ToCore(err)) + _, err = api.GetFolderByName(ctx, srv, test.driveID, ptr.Val(root.GetId()), "FolderDoesNotExist") + 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)) }) } diff --git a/src/internal/connector/onedrive/permission.go b/src/internal/connector/onedrive/permission.go index d59461a00..deb5109be 100644 --- a/src/internal/connector/onedrive/permission.go +++ b/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 diff --git a/src/internal/connector/onedrive/restore.go b/src/internal/connector/onedrive/restore.go index 6eee0314b..3a3e137d4 100644 --- a/src/internal/connector/onedrive/restore.go +++ b/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 `/root://` - - restoreFolderElements := []string{restoreContainerName} - restoreFolderElements = append(restoreFolderElements, drivePath.Folders...) + // i.e. Restore into `/` + // the drive into which this folder gets restored is tracked separately in drivePath. + restoreFolderElements := path.Builder{}.Append(restoreContainerName).Append(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, - restoreFolders []string, + driveID, driveRootID string, + restoreFolders *path.Builder, + fc *folderCache, ) (string, error) { - driveRoot, err := service.Client().DrivesById(driveID).Root().Get(ctx, nil) - if err != nil { - return "", graph.Wrap(ctx, err, "getting drive root") - } + var ( + location = &path.Builder{} + parentFolderID = driveRootID + folders = restoreFolders.Elements() + ) - parentFolderID := ptr.Val(driveRoot.GetId()) - ctx = clues.Add(ctx, "drive_root_id", parentFolderID) + for _, folder := range folders { + 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") - - for _, folder := range restoreFolders { - folderItem, err := getFolder(ctx, service, driveID, parentFolderID, folder) - if err == nil { - parentFolderID = ptr.Val(folderItem.GetId()) + if fl, ok := fc.get(location); ok { + parentFolderID = ptr.Val(fl.GetId()) + // folder was already created, move on to the child continue } - if !errors.Is(err, errFolderNotFound) { - return "", clues.Wrap(err, "folder not found").With("folder_id", folder).WithClues(ctx) + folderItem, err := api.GetFolderByName(ictx, service, driveID, parentFolderID, folder) + 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 diff --git a/src/internal/connector/sharepoint/collection_test.go b/src/internal/connector/sharepoint/collection_test.go index 33103acee..7520d2ff4 100644 --- a/src/internal/connector/sharepoint/collection_test.go +++ b/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)) -} diff --git a/src/internal/connector/sharepoint/restore.go b/src/internal/connector/sharepoint/restore.go index 2bea0fb35..9bd6a82ff 100644 --- a/src/internal/connector/sharepoint/restore.go +++ b/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