Integration test for permissions changes (#2965)

This adds basic integration tests for backing up permissions in OneDrive.
*https://github.com/alcionai/corso/issues/2790 could be a good follow up to this.*

---

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

- [ ] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x] ⛔ No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [x] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* fixes https://github.com/alcionai/corso/issues/2772

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] 💪 Manual
- [ ] ⚡ Unit test
- [x] 💚 E2E

src/internal/connector/onedrive/permission.go

@@ -8,7 +8,6 @@ import (
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
 	"golang.org/x/exp/slices"
 
-	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/connector/graph"
 	"github.com/alcionai/corso/src/internal/data"
 	"github.com/alcionai/corso/src/internal/version"
@@ -93,7 +92,6 @@ func createRestoreFoldersWithPermissions(
 	drivePath *path.DrivePath,
 	restoreFolders []string,
 	folderMetadata Metadata,
-	permissionIDMappings map[string]string,
 ) (string, error) {
 	id, err := CreateRestoreFolders(ctx, service, drivePath.DriveID, restoreFolders)
 	if err != nil {
@@ -105,13 +103,12 @@ func createRestoreFoldersWithPermissions(
 		return id, nil
 	}
 
-	err = restorePermissions(
+	err = RestorePermissions(
 		ctx,
 		service,
 		drivePath.DriveID,
 		id,
-		folderMetadata,
+		folderMetadata)
-		permissionIDMappings)
 
 	return id, err
 }
@@ -126,7 +123,6 @@ func isSame(first, second []string) bool {
 
 func diffPermissions(
 	before, after []UserPermission,
-	permissionIDMappings map[string]string,
 ) ([]UserPermission, []UserPermission) {
 	var (
 		added   = []UserPermission{}
@@ -168,16 +164,15 @@ func diffPermissions(
 	return added, removed
 }
 
-// restorePermissions takes in the permissions that were added and the
+// RestorePermissions takes in the permissions that were added and the
 // removed(ones present in parent but not in child) and adds/removes
 // the necessary permissions on onedrive objects.
-func restorePermissions(
+func RestorePermissions(
 	ctx context.Context,
 	service graph.Servicer,
 	driveID string,
 	itemID string,
 	meta Metadata,
-	permissionIDMappings map[string]string,
 ) error {
 	if meta.SharingMode == SharingModeInherited {
 		return nil
@@ -191,7 +186,7 @@ func restorePermissions(
 		return graph.Wrap(ctx, err, "fetching current permissions")
 	}
 
-	permAdded, permRemoved := diffPermissions(currentPermissions, meta.Permissions, permissionIDMappings)
+	permAdded, permRemoved := diffPermissions(currentPermissions, meta.Permissions)
 
 	for _, p := range permRemoved {
 		err := service.Client().
@@ -244,12 +239,10 @@ func restorePermissions(
 
 		pbody.SetRecipients([]models.DriveRecipientable{rec})
 
-		np, err := service.Client().DrivesById(driveID).ItemsById(itemID).Invite().Post(ctx, pbody, nil)
+		_, err := service.Client().DrivesById(driveID).ItemsById(itemID).Invite().Post(ctx, pbody, nil)
 		if err != nil {
 			return graph.Wrap(ctx, err, "setting permissions")
 		}
-
-		permissionIDMappings[p.ID] = ptr.Val(np.GetValue()[0].GetId())
 	}
 
 	return nil

src/internal/connector/onedrive/restore.go

@@ -45,10 +45,6 @@ func RestoreCollections(
 		restoreMetrics support.CollectionMetrics
 		metrics        support.CollectionMetrics
 		folderMetas    map[string]Metadata
-
-		// permissionIDMappings is used to map between old and new id
-		// of permissions as we restore them
-		permissionIDMappings = map[string]string{}
 	)
 
 	ctx = clues.Add(
@@ -82,7 +78,7 @@ func RestoreCollections(
 				"path", dc.FullPath()) // TODO: pii
 		)
 
-		metrics, folderMetas, permissionIDMappings, err = RestoreCollection(
+		metrics, folderMetas, err = RestoreCollection(
 			ictx,
 			backupVersion,
 			service,
@@ -91,7 +87,6 @@ func RestoreCollections(
 			OneDriveSource,
 			dest.ContainerName,
 			deets,
-			permissionIDMappings,
 			opts.RestorePermissions,
 			errs)
 		if err != nil {
@@ -122,7 +117,8 @@ func RestoreCollections(
 // RestoreCollection handles restoration of an individual collection.
 // returns:
 // - the collection's item and byte count metrics
-// - the context cancellation state (true if the context is canceled)
+// - the updated metadata map that include metadata for folders in this collection
+// - error, if any besides recoverable
 func RestoreCollection(
 	ctx context.Context,
 	backupVersion int,
@@ -132,10 +128,9 @@ func RestoreCollection(
 	source driveSource,
 	restoreContainerName string,
 	deets *details.Builder,
-	permissionIDMappings map[string]string,
 	restorePerms bool,
 	errs *fault.Bus,
-) (support.CollectionMetrics, map[string]Metadata, map[string]string, error) {
+) (support.CollectionMetrics, map[string]Metadata, error) {
 	var (
 		metrics     = support.CollectionMetrics{}
 		copyBuffer  = make([]byte, copyBufferSize)
@@ -149,7 +144,7 @@ func RestoreCollection(
 
 	drivePath, err := path.ToOneDrivePath(directory)
 	if err != nil {
-		return metrics, folderMetas, permissionIDMappings, clues.Wrap(err, "creating drive path").WithClues(ctx)
+		return metrics, folderMetas, clues.Wrap(err, "creating drive path").WithClues(ctx)
 	}
 
 	// Assemble folder hierarchy we're going to restore into (we recreate the folder hierarchy
@@ -176,7 +171,7 @@ func RestoreCollection(
 		backupVersion,
 		restorePerms)
 	if err != nil {
-		return metrics, folderMetas, permissionIDMappings, clues.Wrap(err, "getting permissions").WithClues(ctx)
+		return metrics, folderMetas, clues.Wrap(err, "getting permissions").WithClues(ctx)
 	}
 
 	// Create restore folders and get the folder ID of the folder the data stream will be restored in
@@ -185,10 +180,9 @@ func RestoreCollection(
 		service,
 		drivePath,
 		restoreFolderElements,
-		colMeta,
+		colMeta)
-		permissionIDMappings)
 	if err != nil {
-		return metrics, folderMetas, permissionIDMappings, clues.Wrap(err, "creating folders for restore")
+		return metrics, folderMetas, clues.Wrap(err, "creating folders for restore")
 	}
 
 	items := dc.Items(ctx, errs)
@@ -200,11 +194,11 @@ func RestoreCollection(
 
 		select {
 		case <-ctx.Done():
-			return metrics, folderMetas, permissionIDMappings, err
+			return metrics, folderMetas, err
 
 		case itemData, ok := <-items:
 			if !ok {
-				return metrics, folderMetas, permissionIDMappings, nil
+				return metrics, folderMetas, nil
 			}
 
 			itemPath, err := dc.FullPath().Append(itemData.UUID(), true)
@@ -223,7 +217,6 @@ func RestoreCollection(
 				restoreFolderID,
 				copyBuffer,
 				folderMetas,
-				permissionIDMappings,
 				restorePerms,
 				itemData,
 				itemPath)
@@ -260,7 +253,7 @@ func RestoreCollection(
 		}
 	}
 
-	return metrics, folderMetas, permissionIDMappings, el.Failure()
+	return metrics, folderMetas, el.Failure()
 }
 
 // restores an item, according to correct backup version behavior.
@@ -275,7 +268,6 @@ func restoreItem(
 	restoreFolderID string,
 	copyBuffer []byte,
 	folderMetas map[string]Metadata,
-	permissionIDMappings map[string]string,
 	restorePerms bool,
 	itemData data.Stream,
 	itemPath path.Path,
@@ -340,7 +332,6 @@ func restoreItem(
 			dc,
 			restoreFolderID,
 			copyBuffer,
-			permissionIDMappings,
 			restorePerms,
 			itemData)
 		if err != nil {
@@ -360,7 +351,6 @@ func restoreItem(
 		dc,
 		restoreFolderID,
 		copyBuffer,
-		permissionIDMappings,
 		restorePerms,
 		itemData)
 	if err != nil {
@@ -407,7 +397,6 @@ func restoreV1File(
 	fetcher fileFetcher,
 	restoreFolderID string,
 	copyBuffer []byte,
-	permissionIDMappings map[string]string,
 	restorePerms bool,
 	itemData data.Stream,
 ) (details.ItemInfo, error) {
@@ -440,13 +429,12 @@ func restoreV1File(
 		return details.ItemInfo{}, clues.Wrap(err, "restoring file")
 	}
 
-	err = restorePermissions(
+	err = RestorePermissions(
 		ctx,
 		service,
 		drivePath.DriveID,
 		itemID,
-		meta,
+		meta)
-		permissionIDMappings)
 	if err != nil {
 		return details.ItemInfo{}, clues.Wrap(err, "restoring item permissions")
 	}
@@ -462,7 +450,6 @@ func restoreV6File(
 	fetcher fileFetcher,
 	restoreFolderID string,
 	copyBuffer []byte,
-	permissionIDMappings map[string]string,
 	restorePerms bool,
 	itemData data.Stream,
 ) (details.ItemInfo, error) {
@@ -506,13 +493,12 @@ func restoreV6File(
 		return itemInfo, nil
 	}
 
-	err = restorePermissions(
+	err = RestorePermissions(
 		ctx,
 		service,
 		drivePath.DriveID,
 		itemID,
 		meta,
-		permissionIDMappings,
 	)
 	if err != nil {
 		return details.ItemInfo{}, clues.Wrap(err, "restoring item permissions")

src/internal/connector/sharepoint/restore.go

@@ -67,7 +67,7 @@ func RestoreCollections(
 
 		switch dc.FullPath().Category() {
 		case path.LibrariesCategory:
-			metrics, _, _, err = onedrive.RestoreCollection(
+			metrics, _, err = onedrive.RestoreCollection(
 				ictx,
 				backupVersion,
 				service,
@@ -76,7 +76,6 @@ func RestoreCollections(
 				onedrive.SharePointSource,
 				dest.ContainerName,
 				deets,
-				map[string]string{},
 				false,
 				errs)
 		case path.ListsCategory:

src/internal/operations/backup_integration_test.go

@@ -455,6 +455,29 @@ func toDataLayerPath(
 	return p
 }
 
+func mustGetDefaultDriveID(
+	t *testing.T,
+	ctx context.Context, //revive:disable-line:context-as-argument
+	service graph.Servicer,
+	userID string,
+) string {
+	d, err := service.Client().UsersById(userID).Drive().Get(ctx, nil)
+	if err != nil {
+		err = graph.Wrap(
+			ctx,
+			err,
+			"retrieving default user drive").
+			With("user", userID)
+	}
+
+	require.Nil(t, clues.ToCore(err))
+
+	id := ptr.Val(d.GetId())
+	require.NotEmpty(t, id, "drive ID not set")
+
+	return id
+}
+
 // ---------------------------------------------------------------------------
 // integration tests
 // ---------------------------------------------------------------------------
@@ -1132,31 +1155,6 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() {
 		fault.New(true))
 	require.NoError(t, err, clues.ToCore(err))
 
-	// TODO: whomever can figure out a way to declare this outside of this func
-	// and not have the linter complain about unused is welcome to do so.
-	mustGetDefaultDriveID := func(
-		t *testing.T,
-		ctx context.Context, //revive:disable-line:context-as-argument
-		service graph.Servicer,
-		userID string,
-	) string {
-		d, err := service.Client().UsersById(userID).Drive().Get(ctx, nil)
-		if err != nil {
-			err = graph.Wrap(
-				ctx,
-				err,
-				"retrieving default user drive").
-				With("user", userID)
-		}
-
-		require.NoError(t, err)
-
-		id := ptr.Val(d.GetId())
-		require.NotEmpty(t, id, "drive ID not set")
-
-		return id
-	}
-
 	driveID := mustGetDefaultDriveID(t, ctx, gc.Service, suite.user)
 
 	fileDBF := func(id, timeStamp, subject, body string) []byte {
@@ -1254,6 +1252,102 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() {
 			itemsRead:    1, // .data file for newitem
 			itemsWritten: 3, // .data and .meta for newitem, .dirmeta for parent
 		},
+		{
+			name: "add permission to new file",
+			updateUserData: func(t *testing.T) {
+				driveItem := models.NewDriveItem()
+				driveItem.SetName(&newFileName)
+				driveItem.SetFile(models.NewFile())
+				err = onedrive.RestorePermissions(
+					ctx,
+					gc.Service,
+					driveID,
+					*newFile.GetId(),
+					onedrive.Metadata{
+						SharingMode: onedrive.SharingModeCustom,
+						Permissions: []onedrive.UserPermission{
+							{
+								Roles:    []string{"write"},
+								EntityID: suite.user,
+							},
+						},
+					},
+				)
+				require.NoErrorf(t, err, "add permission to file %v", clues.ToCore(err))
+			},
+			itemsRead:    1, // .data file for newitem
+			itemsWritten: 2, // .meta for newitem, .dirmeta for parent (.data is not written as it is not updated)
+		},
+		{
+			name: "remove permission from new file",
+			updateUserData: func(t *testing.T) {
+				driveItem := models.NewDriveItem()
+				driveItem.SetName(&newFileName)
+				driveItem.SetFile(models.NewFile())
+				err = onedrive.RestorePermissions(
+					ctx,
+					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
+			itemsWritten: 2, // .meta for newitem, .dirmeta for parent (.data is not written as it is not updated)
+		},
+		{
+			name: "add permission to container",
+			updateUserData: func(t *testing.T) {
+				targetContainer := containerIDs[container1]
+				driveItem := models.NewDriveItem()
+				driveItem.SetName(&newFileName)
+				driveItem.SetFile(models.NewFile())
+				err = onedrive.RestorePermissions(
+					ctx,
+					gc.Service,
+					driveID,
+					targetContainer,
+					onedrive.Metadata{
+						SharingMode: onedrive.SharingModeCustom,
+						Permissions: []onedrive.UserPermission{
+							{
+								Roles:    []string{"write"},
+								EntityID: suite.user,
+							},
+						},
+					},
+				)
+				require.NoError(t, err, "add permission to file", clues.ToCore(err))
+			},
+			itemsRead:    0,
+			itemsWritten: 1, // .dirmeta for collection
+		},
+		{
+			name: "remove permission from container",
+			updateUserData: func(t *testing.T) {
+				targetContainer := containerIDs[container1]
+				driveItem := models.NewDriveItem()
+				driveItem.SetName(&newFileName)
+				driveItem.SetFile(models.NewFile())
+				err = onedrive.RestorePermissions(
+					ctx,
+					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,
+			itemsWritten: 1, // .dirmeta for collection
+		},
 		{
 			name: "update contents of a file",
 			updateUserData: func(t *testing.T) {