create a new http client on every delete (#2970)

in order to avoid errors cascading from graph API's
populated 204 responses containing `0/r/n/r/n`,
in particular in response to deletes, this change
hacks in the creation of a unique http client for
each delete call.

This is dangerous to do, since we already know
that generating clients per calls can leak
consumption of system resources such as sockets.
This change presumes that runtime deletes are
minimally, if ever, called during standard backup
and restore.  The permission deletion in onedrive
is the stand-out exception that might need some
additional investigation.

---

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

- [x] ⛔ No

#### Type of change

- [x] 🤖 Supportability/Tests

#### Issue(s)

* #2707

#### Test Plan

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

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:

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)
 	}

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)
 	}

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)
 	}

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,

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)
 			}

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).

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")
 	}

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,

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))
 }

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,

src/internal/connector/sharepoint/restore.go

@@ -69,6 +69,7 @@ func RestoreCollections(
 		case path.LibrariesCategory:
 			metrics, _, err = onedrive.RestoreCollection(
 				ictx,
+				creds,
 				backupVersion,
 				service,
 				dc,

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)
+}