diff --git a/src/internal/connector/data_collections.go b/src/internal/connector/data_collections.go index 41489ff4a..728e3b52f 100644 --- a/src/internal/connector/data_collections.go +++ b/src/internal/connector/data_collections.go @@ -218,7 +218,7 @@ func (gc *GraphConnector) RestoreDataCollections( case selectors.ServiceExchange: status, err = exchange.RestoreExchangeDataCollections(ctx, creds, gc.Service, dest, dcs, deets, errs) case selectors.ServiceOneDrive: - status, err = onedrive.RestoreCollections(ctx, backupVersion, gc.Service, dest, opts, dcs, deets, errs) + status, err = onedrive.RestoreCollections(ctx, creds, backupVersion, gc.Service, dest, opts, dcs, deets, errs) case selectors.ServiceSharePoint: status, err = sharepoint.RestoreCollections(ctx, backupVersion, creds, gc.Service, dest, dcs, deets, errs) default: diff --git a/src/internal/connector/exchange/api/contacts.go b/src/internal/connector/exchange/api/contacts.go index c9cf824fe..90af25bac 100644 --- a/src/internal/connector/exchange/api/contacts.go +++ b/src/internal/connector/exchange/api/contacts.go @@ -60,7 +60,14 @@ func (c Contacts) DeleteContainer( ctx context.Context, user, folderID string, ) error { - err := c.stable.Client().UsersById(user).ContactFoldersById(folderID).Delete(ctx, nil) + // deletes require unique http clients + // https://github.com/alcionai/corso/issues/2707 + srv, err := newService(c.Credentials) + if err != nil { + return graph.Stack(ctx, err) + } + + err = srv.Client().UsersById(user).ContactFoldersById(folderID).Delete(ctx, nil) if err != nil { return graph.Stack(ctx, err) } diff --git a/src/internal/connector/exchange/api/events.go b/src/internal/connector/exchange/api/events.go index cfb79e80b..421e4a66b 100644 --- a/src/internal/connector/exchange/api/events.go +++ b/src/internal/connector/exchange/api/events.go @@ -62,7 +62,14 @@ func (c Events) DeleteContainer( ctx context.Context, user, calendarID string, ) error { - err := c.stable.Client().UsersById(user).CalendarsById(calendarID).Delete(ctx, nil) + // deletes require unique http clients + // https://github.com/alcionai/corso/issues/2707 + srv, err := newService(c.Credentials) + if err != nil { + return graph.Stack(ctx, err) + } + + err = srv.Client().UsersById(user).CalendarsById(calendarID).Delete(ctx, nil) if err != nil { return graph.Stack(ctx, err) } diff --git a/src/internal/connector/exchange/api/mail.go b/src/internal/connector/exchange/api/mail.go index 890276c54..095ab1525 100644 --- a/src/internal/connector/exchange/api/mail.go +++ b/src/internal/connector/exchange/api/mail.go @@ -89,7 +89,14 @@ func (c Mail) DeleteContainer( ctx context.Context, user, folderID string, ) error { - err := c.stable.Client().UsersById(user).MailFoldersById(folderID).Delete(ctx, nil) + // deletes require unique http clients + // https://github.com/alcionai/corso/issues/2707 + srv, err := newService(c.Credentials) + if err != nil { + return graph.Stack(ctx, err) + } + + err = srv.Client().UsersById(user).MailFoldersById(folderID).Delete(ctx, nil) if err != nil { return graph.Stack(ctx, err) } diff --git a/src/internal/connector/onedrive/drive.go b/src/internal/connector/onedrive/drive.go index 0d0175a0b..f443cb8c7 100644 --- a/src/internal/connector/onedrive/drive.go +++ b/src/internal/connector/onedrive/drive.go @@ -367,6 +367,8 @@ func GetAllFolders( return res, el.Failure() } +// deletes require unique http clients +// https://github.com/alcionai/corso/issues/2707 func DeleteItem( ctx context.Context, gs graph.Servicer, diff --git a/src/internal/connector/onedrive/drive_test.go b/src/internal/connector/onedrive/drive_test.go index 87c346599..1bbc28779 100644 --- a/src/internal/connector/onedrive/drive_test.go +++ b/src/internal/connector/onedrive/drive_test.go @@ -317,7 +317,9 @@ func (suite *OneDriveSuite) TestCreateGetDeleteFolder() { defer func() { for _, id := range folderIDs { - err := DeleteItem(ctx, gs, driveID, id) + // deletes require unique http clients + // https://github.com/alcionai/corso/issues/2707 + err := DeleteItem(ctx, loadTestService(t), driveID, id) if err != nil { logger.Ctx(ctx).Warnw("deleting folder", "id", id, "error", err) } diff --git a/src/internal/connector/onedrive/permission.go b/src/internal/connector/onedrive/permission.go index c1d734f40..938f6d1f5 100644 --- a/src/internal/connector/onedrive/permission.go +++ b/src/internal/connector/onedrive/permission.go @@ -11,6 +11,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/version" + "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/path" ) @@ -88,6 +89,7 @@ func getCollectionMetadata( // 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, @@ -105,6 +107,7 @@ func createRestoreFoldersWithPermissions( err = RestorePermissions( ctx, + creds, service, drivePath.DriveID, id, @@ -169,6 +172,7 @@ func diffPermissions( // the necessary permissions on onedrive objects. func RestorePermissions( ctx context.Context, + creds account.M365Config, service graph.Servicer, driveID string, itemID string, @@ -189,7 +193,17 @@ func RestorePermissions( permAdded, permRemoved := diffPermissions(currentPermissions, meta.Permissions) for _, p := range permRemoved { - err := service.Client(). + // deletes require unique http clients + // https://github.com/alcionai/corso/issues/2707 + // this is bad citizenship, and could end up consuming a lot of + // system resources if servicers leak client connections (sockets, etc). + a, err := graph.CreateAdapter(creds.AzureTenantID, creds.AzureClientID, creds.AzureClientSecret) + if err != nil { + return graph.Wrap(ctx, err, "creating delete client") + } + + err = graph.NewService(a). + Client(). DrivesById(driveID). ItemsById(itemID). PermissionsById(p.ID). diff --git a/src/internal/connector/onedrive/restore.go b/src/internal/connector/onedrive/restore.go index 24a9e6f8e..dc1c870a3 100644 --- a/src/internal/connector/onedrive/restore.go +++ b/src/internal/connector/onedrive/restore.go @@ -18,6 +18,7 @@ import ( "github.com/alcionai/corso/src/internal/diagnostics" "github.com/alcionai/corso/src/internal/observe" "github.com/alcionai/corso/src/internal/version" + "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/fault" @@ -33,6 +34,7 @@ const copyBufferSize = 5 * 1024 * 1024 // RestoreCollections will restore the specified data collections into OneDrive func RestoreCollections( ctx context.Context, + creds account.M365Config, backupVersion int, service graph.Servicer, dest control.RestoreDestination, @@ -80,6 +82,7 @@ func RestoreCollections( metrics, folderMetas, err = RestoreCollection( ictx, + creds, backupVersion, service, dc, @@ -121,6 +124,7 @@ func RestoreCollections( // - error, if any besides recoverable func RestoreCollection( ctx context.Context, + creds account.M365Config, backupVersion int, service graph.Servicer, dc data.RestoreCollection, @@ -177,6 +181,7 @@ func RestoreCollection( // Create restore folders and get the folder ID of the folder the data stream will be restored in restoreFolderID, err := createRestoreFoldersWithPermissions( ctx, + creds, service, drivePath, restoreFolderElements, @@ -209,6 +214,7 @@ func RestoreCollection( itemInfo, skipped, err := restoreItem( ctx, + creds, dc, backupVersion, source, @@ -260,6 +266,7 @@ func RestoreCollection( // returns the item info, a bool (true = restore was skipped), and an error func restoreItem( ctx context.Context, + creds account.M365Config, dc data.RestoreCollection, backupVersion int, source driveSource, @@ -327,6 +334,7 @@ func restoreItem( itemInfo, err := restoreV1File( ctx, source, + creds, service, drivePath, dc, @@ -346,6 +354,7 @@ func restoreItem( itemInfo, err := restoreV6File( ctx, source, + creds, service, drivePath, dc, @@ -392,6 +401,7 @@ type fileFetcher interface { func restoreV1File( ctx context.Context, source driveSource, + creds account.M365Config, service graph.Servicer, drivePath *path.DrivePath, fetcher fileFetcher, @@ -431,6 +441,7 @@ func restoreV1File( err = RestorePermissions( ctx, + creds, service, drivePath.DriveID, itemID, @@ -445,6 +456,7 @@ func restoreV1File( func restoreV6File( ctx context.Context, source driveSource, + creds account.M365Config, service graph.Servicer, drivePath *path.DrivePath, fetcher fileFetcher, @@ -495,11 +507,11 @@ func restoreV6File( err = RestorePermissions( ctx, + creds, service, drivePath.DriveID, itemID, - meta, - ) + meta) if err != nil { return details.ItemInfo{}, clues.Wrap(err, "restoring item permissions") } diff --git a/src/internal/connector/sharepoint/api/pages.go b/src/internal/connector/sharepoint/api/pages.go index cd7c0ec65..6b5800d20 100644 --- a/src/internal/connector/sharepoint/api/pages.go +++ b/src/internal/connector/sharepoint/api/pages.go @@ -155,6 +155,8 @@ func fetchPageOptions() *betasites.ItemPagesRequestBuilderGetRequestConfiguratio // DeleteSitePage removes the selected page from the SharePoint Site // https://learn.microsoft.com/en-us/graph/api/sitepage-delete?view=graph-rest-beta +// deletes require unique http clients +// https://github.com/alcionai/corso/issues/2707 func DeleteSitePage( ctx context.Context, serv *dapi.BetaService, diff --git a/src/internal/connector/sharepoint/collection_test.go b/src/internal/connector/sharepoint/collection_test.go index e32528582..08c9e8c21 100644 --- a/src/internal/connector/sharepoint/collection_test.go +++ b/src/internal/connector/sharepoint/collection_test.go @@ -256,6 +256,7 @@ func (suite *SharePointCollectionSuite) TestRestoreLocation() { 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/list.go b/src/internal/connector/sharepoint/list.go index c0d8c7c58..fd8b42a06 100644 --- a/src/internal/connector/sharepoint/list.go +++ b/src/internal/connector/sharepoint/list.go @@ -389,6 +389,8 @@ func fetchColumnLinks( } // DeleteList removes a list object from a site. +// deletes require unique http clients +// https://github.com/alcionai/corso/issues/2707 func DeleteList( ctx context.Context, gs graph.Servicer, diff --git a/src/internal/connector/sharepoint/restore.go b/src/internal/connector/sharepoint/restore.go index 8bb1a4a32..afa7a2dd8 100644 --- a/src/internal/connector/sharepoint/restore.go +++ b/src/internal/connector/sharepoint/restore.go @@ -69,6 +69,7 @@ func RestoreCollections( case path.LibrariesCategory: metrics, _, err = onedrive.RestoreCollection( ictx, + creds, backupVersion, service, dc, diff --git a/src/internal/operations/backup_integration_test.go b/src/internal/operations/backup_integration_test.go index d7604b008..3ae3d8a72 100644 --- a/src/internal/operations/backup_integration_test.go +++ b/src/internal/operations/backup_integration_test.go @@ -1183,7 +1183,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { genDests = []string{container1, container2} ) - m365, err := acct.M365Config() + creds, err := acct.M365Config() require.NoError(t, err, clues.ToCore(err)) gc, err := connector.NewGraphConnector( @@ -1215,7 +1215,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { acct, path.FilesCategory, selectors.NewOneDriveRestore(owners).Selector, - m365.AzureTenantID, suite.user, driveID, destName, + creds.AzureTenantID, suite.user, driveID, destName, 2, // Use an old backup version so we don't need metadata files. 0, @@ -1299,6 +1299,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { driveItem.SetFile(models.NewFile()) err = onedrive.RestorePermissions( ctx, + creds, gc.Service, driveID, *newFile.GetId(), @@ -1310,8 +1311,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { EntityID: suite.user, }, }, - }, - ) + }) require.NoErrorf(t, err, "add permission to file %v", clues.ToCore(err)) }, itemsRead: 1, // .data file for newitem @@ -1325,14 +1325,14 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { driveItem.SetFile(models.NewFile()) err = onedrive.RestorePermissions( ctx, + creds, gc.Service, driveID, *newFile.GetId(), onedrive.Metadata{ SharingMode: onedrive.SharingModeCustom, Permissions: []onedrive.UserPermission{}, - }, - ) + }) require.NoError(t, err, "add permission to file", clues.ToCore(err)) }, itemsRead: 1, // .data file for newitem @@ -1347,6 +1347,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { driveItem.SetFile(models.NewFile()) err = onedrive.RestorePermissions( ctx, + creds, gc.Service, driveID, targetContainer, @@ -1358,8 +1359,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { EntityID: suite.user, }, }, - }, - ) + }) require.NoError(t, err, "add permission to file", clues.ToCore(err)) }, itemsRead: 0, @@ -1374,14 +1374,14 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { driveItem.SetFile(models.NewFile()) err = onedrive.RestorePermissions( ctx, + creds, gc.Service, driveID, targetContainer, onedrive.Metadata{ SharingMode: onedrive.SharingModeCustom, Permissions: []onedrive.UserPermission{}, - }, - ) + }) require.NoError(t, err, "add permission to file", clues.ToCore(err)) }, itemsRead: 0, @@ -1447,7 +1447,9 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { { name: "delete file", updateUserData: func(t *testing.T) { - err = gc.Service. + // deletes require unique http clients + // https://github.com/alcionai/corso/issues/2707 + err = newDeleteServicer(t). Client(). DrivesById(driveID). ItemsById(ptr.Val(newFile.GetId())). @@ -1506,7 +1508,9 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { name: "delete a folder", updateUserData: func(t *testing.T) { container := containerIDs[container2] - err := gc.Service. + // deletes require unique http clients + // https://github.com/alcionai/corso/issues/2707 + err = newDeleteServicer(t). Client(). DrivesById(driveID). ItemsById(container). @@ -1527,7 +1531,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { acct, path.FilesCategory, selectors.NewOneDriveRestore(owners).Selector, - m365.AzureTenantID, suite.user, driveID, container3, + creds.AzureTenantID, suite.user, driveID, container3, 2, 0, fileDBF) @@ -1568,11 +1572,10 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { incBO.Results.BackupID, kw, ms, - m365.AzureTenantID, + creds.AzureTenantID, suite.user, path.OneDriveService, - categories, - ) + categories) // do some additional checks to ensure the incremental dealt with fewer items. // +2 on read/writes to account for metadata: 1 delta and 1 path. @@ -1611,3 +1614,19 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_sharePoint() { runAndCheckBackup(t, ctx, &bo, mb, false) checkBackupIsInManifests(t, ctx, kw, &bo, sel.Selector, suite.site, path.LibrariesCategory) } + +// --------------------------------------------------------------------------- +// helpers +// --------------------------------------------------------------------------- + +func newDeleteServicer(t *testing.T) graph.Servicer { + acct := tester.NewM365Account(t) + + m365, err := acct.M365Config() + require.NoError(t, err, clues.ToCore(err)) + + a, err := graph.CreateAdapter(acct.ID(), m365.AzureClientID, m365.AzureClientSecret) + require.NoError(t, err, clues.ToCore(err)) + + return graph.NewService(a) +}