Refactor drive handler logic (#4477)

Do a couple of high-level things with drive handlers:
* pull out generic functionality into a base handler
* use handler-specific implementations of AugmentItemInfo
* remove old switch-based augmentItemInfo

Overall goal is to reduce places we use service type
comparisons to handle behavior differences

---

#### 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

- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)

* #4254

#### Test Plan

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

src/internal/m365/collection/drive/collections_test.go

@@ -740,7 +740,13 @@ func (suite *OneDriveCollectionsUnitSuite) TestUpdateCollections() {
 			maps.Copy(currPrevPaths, test.inputFolderMap)
 
 			c := NewCollections(
-				&itemBackupHandler{api.Drives{}, user, test.scope},
+				&itemBackupHandler{
+					baseItemHandler: baseItemHandler{
+						ac: api.Drives{},
+					},
+					userID: user,
+					scope:  test.scope,
+				},
 				tenant,
 				idname.NewProvider(user, user),
 				nil,
@@ -2462,7 +2468,13 @@ func (suite *OneDriveCollectionsUnitSuite) TestAddURLCacheToDriveCollections() {
 			// Add a few collections
 			for i := 0; i < collCount; i++ {
 				coll, err := NewCollection(
-					&itemBackupHandler{api.Drives{}, "test-user", anyFolder},
+					&itemBackupHandler{
+						baseItemHandler: baseItemHandler{
+							ac: api.Drives{},
+						},
+						userID: "test-user",
+						scope:  anyFolder,
+					},
 					idname.NewProvider("", ""),
 					nil,
 					nil,

src/internal/m365/collection/drive/group_handler.go

@@ -2,8 +2,12 @@ package drive
 
 import (
 	"github.com/alcionai/clues"
+	"github.com/microsoftgraph/msgraph-sdk-go/models"
 
+	"github.com/alcionai/corso/src/internal/common/idname"
+	"github.com/alcionai/corso/src/internal/common/ptr"
 	odConsts "github.com/alcionai/corso/src/internal/m365/service/onedrive/consts"
+	"github.com/alcionai/corso/src/pkg/backup/details"
 	"github.com/alcionai/corso/src/pkg/path"
 	"github.com/alcionai/corso/src/pkg/selectors"
 	"github.com/alcionai/corso/src/pkg/services/m365/api"
@@ -24,7 +28,9 @@ func NewGroupBackupHandler(
 ) groupBackupHandler {
 	return groupBackupHandler{
 		libraryBackupHandler{
-			ac:     ac,
+			baseLibraryHandler: baseLibraryHandler{
+				ac: ac,
+			},
 			siteID: siteID,
 			// Not adding scope here. Anything that needs scope has to
 			// be from group handler
@@ -96,6 +102,39 @@ func (h groupBackupHandler) SitePathPrefix(tenantID string) (path.Path, error) {
 		h.siteID)
 }
 
+func (h groupBackupHandler) AugmentItemInfo(
+	dii details.ItemInfo,
+	resource idname.Provider,
+	item models.DriveItemable,
+	size int64,
+	parentPath *path.Builder,
+) details.ItemInfo {
+	var pps string
+
+	if parentPath != nil {
+		pps = parentPath.String()
+	}
+
+	driveName, driveID := getItemDriveInfo(item)
+
+	dii.Extension = &details.ExtensionData{}
+	dii.Groups = &details.GroupsInfo{
+		Created:    ptr.Val(item.GetCreatedDateTime()),
+		DriveID:    driveID,
+		DriveName:  driveName,
+		ItemName:   ptr.Val(item.GetName()),
+		ItemType:   details.SharePointLibrary,
+		Modified:   ptr.Val(item.GetLastModifiedDateTime()),
+		Owner:      getItemCreator(item),
+		ParentPath: pps,
+		SiteID:     resource.ID(),
+		Size:       size,
+		WebURL:     resource.Name(),
+	}
+
+	return dii
+}
+
 func (h groupBackupHandler) IsAllPass() bool {
 	return h.scope.IsAny(selectors.GroupsLibraryFolder)
 }

src/internal/m365/collection/drive/handler_utils.go

@@ -5,96 +5,38 @@ import (
 
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
 
-	"github.com/alcionai/corso/src/internal/common/idname"
 	"github.com/alcionai/corso/src/internal/common/ptr"
-	"github.com/alcionai/corso/src/pkg/backup/details"
-	"github.com/alcionai/corso/src/pkg/path"
 )
 
-func augmentItemInfo(
+func getItemCreator(item models.DriveItemable) string {
-	dii details.ItemInfo,
+	if item.GetCreatedBy() == nil || item.GetCreatedBy().GetUser() == nil {
-	resource idname.Provider,
+		return ""
-	service path.ServiceType,
-	item models.DriveItemable,
-	size int64,
-	parentPath *path.Builder,
-) details.ItemInfo {
-	var driveName, driveID, creatorEmail string
-
-	// TODO: we rely on this info for details/restore lookups,
-	// so if it's nil we have an issue, and will need an alternative
-	// way to source the data.
-
-	if item.GetCreatedBy() != nil && item.GetCreatedBy().GetUser() != nil {
-		// User is sometimes not available when created via some
-		// external applications (like backup/restore solutions)
-		additionalData := item.GetCreatedBy().GetUser().GetAdditionalData()
-
-		ed, ok := additionalData["email"]
-		if !ok {
-			ed = additionalData["displayName"]
-		}
-
-		if ed != nil {
-			creatorEmail = *ed.(*string)
-		}
 	}
 
-	if item.GetParentReference() != nil {
+	// User is sometimes not available when created via some
-		driveID = ptr.Val(item.GetParentReference().GetDriveId())
+	// external applications (like backup/restore solutions)
-		driveName = strings.TrimSpace(ptr.Val(item.GetParentReference().GetName()))
+	additionalData := item.GetCreatedBy().GetUser().GetAdditionalData()
+
+	ed, ok := additionalData["email"]
+	if !ok {
+		ed = additionalData["displayName"]
 	}
 
-	var pps string
+	if ed == nil {
-	if parentPath != nil {
+		return ""
-		pps = parentPath.String()
 	}
 
-	switch service {
+	// TODO(ashmrtn): Replace with str package with fallbacks.
-	case path.OneDriveService:
+	return *ed.(*string)
-		dii.OneDrive = &details.OneDriveInfo{
+}
-			Created:    ptr.Val(item.GetCreatedDateTime()),
+
-			DriveID:    driveID,
+func getItemDriveInfo(item models.DriveItemable) (string, string) {
-			DriveName:  driveName,
+	if item.GetParentReference() == nil {
-			ItemName:   ptr.Val(item.GetName()),
+		return "", ""
-			ItemType:   details.OneDriveItem,
+	}
-			Modified:   ptr.Val(item.GetLastModifiedDateTime()),
+
-			Owner:      creatorEmail,
+	driveName := strings.TrimSpace(ptr.Val(item.GetParentReference().GetName()))
-			ParentPath: pps,
+	driveID := ptr.Val(item.GetParentReference().GetDriveId())
-			Size:       size,
+
-		}
+	return driveName, driveID
-	case path.SharePointService:
-		dii.SharePoint = &details.SharePointInfo{
-			Created:    ptr.Val(item.GetCreatedDateTime()),
-			DriveID:    driveID,
-			DriveName:  driveName,
-			ItemName:   ptr.Val(item.GetName()),
-			ItemType:   details.SharePointLibrary,
-			Modified:   ptr.Val(item.GetLastModifiedDateTime()),
-			Owner:      creatorEmail,
-			ParentPath: pps,
-			SiteID:     resource.ID(),
-			Size:       size,
-			WebURL:     resource.Name(),
-		}
-
-	case path.GroupsService:
-		dii.Groups = &details.GroupsInfo{
-			Created:    ptr.Val(item.GetCreatedDateTime()),
-			DriveID:    driveID,
-			DriveName:  driveName,
-			ItemName:   ptr.Val(item.GetName()),
-			ItemType:   details.SharePointLibrary,
-			Modified:   ptr.Val(item.GetLastModifiedDateTime()),
-			Owner:      creatorEmail,
-			ParentPath: pps,
-			SiteID:     resource.ID(),
-			Size:       size,
-			WebURL:     resource.Name(),
-		}
-	}
-
-	dii.Extension = &details.ExtensionData{}
-
-	return dii
 }

src/internal/m365/collection/drive/item_collector_test.go

@@ -266,7 +266,13 @@ func (suite *OneDriveIntgSuite) TestOneDriveNewCollections() {
 			)
 
 			colls := NewCollections(
-				&itemBackupHandler{suite.ac.Drives(), test.user, scope},
+				&itemBackupHandler{
+					baseItemHandler: baseItemHandler{
+						ac: suite.ac.Drives(),
+					},
+					userID: test.user,
+					scope:  scope,
+				},
 				creds.AzureTenantID,
 				idname.NewProvider(test.user, test.user),
 				service.updateStatus,

src/internal/m365/collection/drive/item_handler.go

@@ -9,6 +9,7 @@ import (
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
 
 	"github.com/alcionai/corso/src/internal/common/idname"
+	"github.com/alcionai/corso/src/internal/common/ptr"
 	odConsts "github.com/alcionai/corso/src/internal/m365/service/onedrive/consts"
 	"github.com/alcionai/corso/src/pkg/backup/details"
 	"github.com/alcionai/corso/src/pkg/control"
@@ -21,16 +22,69 @@ import (
 // backup
 // ---------------------------------------------------------------------------
 
+type baseItemHandler struct {
+	ac api.Drives
+}
+
+func (h baseItemHandler) NewDrivePager(
+	resourceOwner string,
+	fields []string,
+) api.Pager[models.Driveable] {
+	return h.ac.NewUserDrivePager(resourceOwner, fields)
+}
+
+// AugmentItemInfo will populate a details.OneDriveInfo struct
+// with properties from the drive item.  ItemSize is specified
+// separately for restore processes because the local itemable
+// doesn't have its size value updated as a side effect of creation,
+// and kiota drops any SetSize update.
+func (h baseItemHandler) AugmentItemInfo(
+	dii details.ItemInfo,
+	resource idname.Provider,
+	item models.DriveItemable,
+	size int64,
+	parentPath *path.Builder,
+) details.ItemInfo {
+	var pps string
+
+	if parentPath != nil {
+		pps = parentPath.String()
+	}
+
+	driveName, driveID := getItemDriveInfo(item)
+
+	dii.Extension = &details.ExtensionData{}
+	dii.OneDrive = &details.OneDriveInfo{
+		Created:    ptr.Val(item.GetCreatedDateTime()),
+		DriveID:    driveID,
+		DriveName:  driveName,
+		ItemName:   ptr.Val(item.GetName()),
+		ItemType:   details.OneDriveItem,
+		Modified:   ptr.Val(item.GetLastModifiedDateTime()),
+		Owner:      getItemCreator(item),
+		ParentPath: pps,
+		Size:       size,
+	}
+
+	return dii
+}
+
 var _ BackupHandler = &itemBackupHandler{}
 
 type itemBackupHandler struct {
-	ac     api.Drives
+	baseItemHandler
 	userID string
 	scope  selectors.OneDriveScope
 }
 
 func NewItemBackupHandler(ac api.Drives, userID string, scope selectors.OneDriveScope) *itemBackupHandler {
-	return &itemBackupHandler{ac, userID, scope}
+	return &itemBackupHandler{
+		baseItemHandler: baseItemHandler{
+			ac: ac,
+		},
+		userID: userID,
+		scope:  scope,
+	}
 }
 
 func (h itemBackupHandler) Get(
@@ -82,22 +136,6 @@ func (h itemBackupHandler) ServiceCat() (path.ServiceType, path.CategoryType) {
 	return path.OneDriveService, path.FilesCategory
 }
 
-func (h itemBackupHandler) NewDrivePager(
-	resourceOwner string, fields []string,
-) api.Pager[models.Driveable] {
-	return h.ac.NewUserDrivePager(resourceOwner, fields)
-}
-
-func (h itemBackupHandler) AugmentItemInfo(
-	dii details.ItemInfo,
-	resource idname.Provider,
-	item models.DriveItemable,
-	size int64,
-	parentPath *path.Builder,
-) details.ItemInfo {
-	return augmentItemInfo(dii, resource, path.OneDriveService, item, size, parentPath)
-}
-
 func (h itemBackupHandler) FormatDisplayPath(
 	_ string, // drive name not displayed for onedrive
 	pb *path.Builder,
@@ -149,11 +187,15 @@ func (h itemBackupHandler) EnumerateDriveItemsDelta(
 var _ RestoreHandler = &itemRestoreHandler{}
 
 type itemRestoreHandler struct {
-	ac api.Drives
+	baseItemHandler
 }
 
 func NewRestoreHandler(ac api.Client) *itemRestoreHandler {
-	return &itemRestoreHandler{ac.Drives()}
+	return &itemRestoreHandler{
+		baseItemHandler: baseItemHandler{
+			ac: ac.Drives(),
+		},
+	}
 }
 
 func (h itemRestoreHandler) PostDrive(
@@ -163,27 +205,6 @@ func (h itemRestoreHandler) PostDrive(
 	return nil, clues.New("creating drives in oneDrive is not supported")
 }
 
-func (h itemRestoreHandler) NewDrivePager(
-	resourceOwner string, fields []string,
-) api.Pager[models.Driveable] {
-	return h.ac.NewUserDrivePager(resourceOwner, fields)
-}
-
-// AugmentItemInfo will populate a details.OneDriveInfo struct
-// with properties from the drive item.  ItemSize is specified
-// separately for restore processes because the local itemable
-// doesn't have its size value updated as a side effect of creation,
-// and kiota drops any SetSize update.
-func (h itemRestoreHandler) AugmentItemInfo(
-	dii details.ItemInfo,
-	resource idname.Provider,
-	item models.DriveItemable,
-	size int64,
-	parentPath *path.Builder,
-) details.ItemInfo {
-	return augmentItemInfo(dii, resource, path.OneDriveService, item, size, parentPath)
-}
-
 func (h itemRestoreHandler) DeleteItem(
 	ctx context.Context,
 	driveID, itemID string,

src/internal/m365/collection/drive/library_handler.go

@@ -9,6 +9,7 @@ import (
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
 
 	"github.com/alcionai/corso/src/internal/common/idname"
+	"github.com/alcionai/corso/src/internal/common/ptr"
 	odConsts "github.com/alcionai/corso/src/internal/m365/service/onedrive/consts"
 	"github.com/alcionai/corso/src/pkg/backup/details"
 	"github.com/alcionai/corso/src/pkg/control"
@@ -17,10 +18,54 @@ import (
 	"github.com/alcionai/corso/src/pkg/services/m365/api"
 )
 
+type baseLibraryHandler struct {
+	ac api.Drives
+}
+
+func (h baseLibraryHandler) NewDrivePager(
+	resourceOwner string,
+	fields []string,
+) api.Pager[models.Driveable] {
+	return h.ac.NewSiteDrivePager(resourceOwner, fields)
+}
+
+func (h baseLibraryHandler) AugmentItemInfo(
+	dii details.ItemInfo,
+	resource idname.Provider,
+	item models.DriveItemable,
+	size int64,
+	parentPath *path.Builder,
+) details.ItemInfo {
+	var pps string
+
+	if parentPath != nil {
+		pps = parentPath.String()
+	}
+
+	driveName, driveID := getItemDriveInfo(item)
+
+	dii.Extension = &details.ExtensionData{}
+	dii.SharePoint = &details.SharePointInfo{
+		Created:    ptr.Val(item.GetCreatedDateTime()),
+		DriveID:    driveID,
+		DriveName:  driveName,
+		ItemName:   ptr.Val(item.GetName()),
+		ItemType:   details.SharePointLibrary,
+		Modified:   ptr.Val(item.GetLastModifiedDateTime()),
+		Owner:      getItemCreator(item),
+		ParentPath: pps,
+		SiteID:     resource.ID(),
+		Size:       size,
+		WebURL:     resource.Name(),
+	}
+
+	return dii
+}
+
 var _ BackupHandler = &libraryBackupHandler{}
 
 type libraryBackupHandler struct {
-	ac      api.Drives
+	baseLibraryHandler
 	siteID  string
 	scope   selectors.SharePointScope
 	service path.ServiceType
@@ -32,7 +77,14 @@ func NewLibraryBackupHandler(
 	scope selectors.SharePointScope,
 	service path.ServiceType,
 ) libraryBackupHandler {
-	return libraryBackupHandler{ac, siteID, scope, service}
+	return libraryBackupHandler{
+		baseLibraryHandler: baseLibraryHandler{
+			ac: ac,
+		},
+		siteID:  siteID,
+		scope:   scope,
+		service: service,
+	}
 }
 
 func (h libraryBackupHandler) Get(
@@ -84,23 +136,6 @@ func (h libraryBackupHandler) ServiceCat() (path.ServiceType, path.CategoryType)
 	return h.service, path.LibrariesCategory
 }
 
-func (h libraryBackupHandler) NewDrivePager(
-	resourceOwner string,
-	fields []string,
-) api.Pager[models.Driveable] {
-	return h.ac.NewSiteDrivePager(resourceOwner, fields)
-}
-
-func (h libraryBackupHandler) AugmentItemInfo(
-	dii details.ItemInfo,
-	resource idname.Provider,
-	item models.DriveItemable,
-	size int64,
-	parentPath *path.Builder,
-) details.ItemInfo {
-	return augmentItemInfo(dii, resource, h.service, item, size, parentPath)
-}
-
 func (h libraryBackupHandler) FormatDisplayPath(
 	driveName string,
 	pb *path.Builder,
@@ -152,12 +187,19 @@ func (h libraryBackupHandler) EnumerateDriveItemsDelta(
 var _ RestoreHandler = &libraryRestoreHandler{}
 
 type libraryRestoreHandler struct {
+	baseLibraryHandler
 	ac      api.Client
 	service path.ServiceType
 }
 
 func NewLibraryRestoreHandler(ac api.Client, service path.ServiceType) libraryRestoreHandler {
-	return libraryRestoreHandler{ac, service}
+	return libraryRestoreHandler{
+		baseLibraryHandler: baseLibraryHandler{
+			ac: ac.Drives(),
+		},
+		ac:      ac,
+		service: service,
+	}
 }
 
 func (h libraryRestoreHandler) PostDrive(
@@ -167,23 +209,6 @@ func (h libraryRestoreHandler) PostDrive(
 	return h.ac.Lists().PostDrive(ctx, siteID, driveName)
 }
 
-func (h libraryRestoreHandler) NewDrivePager(
-	resourceOwner string,
-	fields []string,
-) api.Pager[models.Driveable] {
-	return h.ac.Drives().NewSiteDrivePager(resourceOwner, fields)
-}
-
-func (h libraryRestoreHandler) AugmentItemInfo(
-	dii details.ItemInfo,
-	resource idname.Provider,
-	item models.DriveItemable,
-	size int64,
-	parentPath *path.Builder,
-) details.ItemInfo {
-	return augmentItemInfo(dii, resource, h.service, item, size, parentPath)
-}
-
 func (h libraryRestoreHandler) DeleteItem(
 	ctx context.Context,
 	driveID, itemID string,