Simplify group_handler by leveraging library_handler (#4084)

https://github.com/alcionai/corso/pull/4030#discussion_r1296246853

---

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

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

#### Type of change

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

#### Issue(s)

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

#### Test Plan

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

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

@@ -1,15 +1,6 @@
 package drive
 
 import (
-	"context"
-	"net/http"
-	"strings"
-
-	"github.com/microsoftgraph/msgraph-sdk-go/models"
-
-	"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"
@@ -18,183 +9,40 @@ import (
 var _ BackupHandler = &groupBackupHandler{}
 
 type groupBackupHandler struct {
+	libraryBackupHandler
 	groupID string
-	ac      api.Drives
 	scope   selectors.GroupsScope
 }
 
 func NewGroupBackupHandler(groupID string, ac api.Drives, scope selectors.GroupsScope) groupBackupHandler {
-	return groupBackupHandler{groupID, ac, scope}
+	return groupBackupHandler{
-}
+		libraryBackupHandler{
-
+			ac: ac,
-func (h groupBackupHandler) Get(
+			// Not adding scope here. Anything that needs scope has to
-	ctx context.Context,
+			// be from group handler
-	url string,
+			service: path.GroupsService,
-	headers map[string]string,
+		},
-) (*http.Response, error) {
+		groupID,
-	return h.ac.Get(ctx, url, headers)
+		scope,
-}
+	}
-
-func (h groupBackupHandler) PathPrefix(
-	tenantID, resourceOwner, driveID string,
-) (path.Path, error) {
-	return path.Build(
-		tenantID,
-		resourceOwner,
-		path.GroupsService,
-		path.LibrariesCategory, // TODO(meain)
-		false,
-		odConsts.DrivesPathDir,
-		driveID,
-		odConsts.RootPathDir)
 }
 
 func (h groupBackupHandler) CanonicalPath(
 	folders *path.Builder,
 	tenantID, resourceOwner string,
 ) (path.Path, error) {
-	// TODO(meain): path fixes: sharepoint site ids should be in the path
+	// TODO(meain): path fixes
-	return folders.ToDataLayerPath(
+	return folders.ToDataLayerPath(tenantID, h.groupID, h.service, path.LibrariesCategory, false)
-		tenantID,
-		h.groupID,
-		path.GroupsService,
-		path.LibrariesCategory,
-		false)
 }
 
 func (h groupBackupHandler) ServiceCat() (path.ServiceType, path.CategoryType) {
 	return path.GroupsService, path.LibrariesCategory
 }
 
-func (h groupBackupHandler) NewDrivePager(
-	resourceOwner string,
-	fields []string,
-) api.DrivePager {
-	return h.ac.NewSiteDrivePager(resourceOwner, fields)
-}
-
-func (h groupBackupHandler) NewItemPager(
-	driveID, link string,
-	fields []string,
-) api.DriveItemDeltaEnumerator {
-	return h.ac.NewDriveItemDeltaPager(driveID, link, fields)
-}
-
-func (h groupBackupHandler) AugmentItemInfo(
-	dii details.ItemInfo,
-	item models.DriveItemable,
-	size int64,
-	parentPath *path.Builder,
-) details.ItemInfo {
-	return augmentGroupItemInfo(dii, item, size, parentPath)
-}
-
-func (h groupBackupHandler) FormatDisplayPath(
-	driveName string,
-	pb *path.Builder,
-) string {
-	return "/" + driveName + "/" + pb.String()
-}
-
-func (h groupBackupHandler) NewLocationIDer(
-	driveID string,
-	elems ...string,
-) details.LocationIDer {
-	// TODO(meain): path fixes
-	return details.NewSharePointLocationIDer(driveID, elems...)
-}
-
-func (h groupBackupHandler) GetItemPermission(
-	ctx context.Context,
-	driveID, itemID string,
-) (models.PermissionCollectionResponseable, error) {
-	return h.ac.GetItemPermission(ctx, driveID, itemID)
-}
-
-func (h groupBackupHandler) GetItem(
-	ctx context.Context,
-	driveID, itemID string,
-) (models.DriveItemable, error) {
-	return h.ac.GetItem(ctx, driveID, itemID)
-}
-
 func (h groupBackupHandler) IsAllPass() bool {
-	// TODO(meain)
+	return h.scope.IsAny(selectors.GroupsLibraryFolder)
-	return true
 }
 
 func (h groupBackupHandler) IncludesDir(dir string) bool {
-	// TODO(meain)
+	return h.scope.Matches(selectors.GroupsLibraryFolder, dir)
-	return true
-}
-
-// ---------------------------------------------------------------------------
-// Common
-// ---------------------------------------------------------------------------
-
-func augmentGroupItemInfo(
-	dii details.ItemInfo,
-	item models.DriveItemable,
-	size int64,
-	parentPath *path.Builder,
-) details.ItemInfo {
-	var driveName, driveID, creatorEmail, siteID, weburl 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)
-		}
-	}
-
-	gsi := item.GetSharepointIds()
-	if gsi != nil {
-		siteID = ptr.Val(gsi.GetSiteId())
-		weburl = ptr.Val(gsi.GetSiteUrl())
-
-		if len(weburl) == 0 {
-			weburl = constructWebURL(item.GetAdditionalData())
-		}
-	}
-
-	if item.GetParentReference() != nil {
-		driveID = ptr.Val(item.GetParentReference().GetDriveId())
-		driveName = strings.TrimSpace(ptr.Val(item.GetParentReference().GetName()))
-	}
-
-	var pps string
-	if parentPath != nil {
-		pps = parentPath.String()
-	}
-
-	// TODO: Add channel name and ID
-	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,
-		Size:       size,
-		SiteID:     siteID,
-		WebURL:     weburl,
-	}
-
-	dii.Extension = &details.ExtensionData{}
-
-	return dii
 }

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

@@ -0,0 +1,112 @@
+package drive
+
+import (
+	"strings"
+
+	"github.com/microsoftgraph/msgraph-sdk-go/models"
+
+	"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(
+	dii details.ItemInfo,
+	service path.ServiceType,
+	item models.DriveItemable,
+	size int64,
+	parentPath *path.Builder,
+) details.ItemInfo {
+	var driveName, siteID, driveID, weburl, 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 service == path.SharePointService ||
+		service == path.GroupsService {
+		gsi := item.GetSharepointIds()
+		if gsi != nil {
+			siteID = ptr.Val(gsi.GetSiteId())
+			weburl = ptr.Val(gsi.GetSiteUrl())
+
+			if len(weburl) == 0 {
+				weburl = constructWebURL(item.GetAdditionalData())
+			}
+		}
+	}
+
+	if item.GetParentReference() != nil {
+		driveID = ptr.Val(item.GetParentReference().GetDriveId())
+		driveName = strings.TrimSpace(ptr.Val(item.GetParentReference().GetName()))
+	}
+
+	var pps string
+	if parentPath != nil {
+		pps = parentPath.String()
+	}
+
+	switch service {
+	case path.OneDriveService:
+		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:      creatorEmail,
+			ParentPath: pps,
+			Size:       size,
+		}
+	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:     siteID,
+			Size:       size,
+			WebURL:     weburl,
+		}
+
+	case path.GroupsService:
+		// TODO: Add channel name and ID
+		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:     siteID,
+			Size:       size,
+			WebURL:     weburl,
+		}
+	}
+
+	dii.Extension = &details.ExtensionData{}
+
+	return dii
+}

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

@@ -3,13 +3,11 @@ package drive
 import (
 	"context"
 	"net/http"
-	"strings"
 
 	"github.com/alcionai/clues"
 	"github.com/microsoftgraph/msgraph-sdk-go/drives"
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
 
-	"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"
@@ -85,7 +83,7 @@ func (h itemBackupHandler) AugmentItemInfo(
 	size int64,
 	parentPath *path.Builder,
 ) details.ItemInfo {
-	return augmentItemInfo(dii, item, size, parentPath)
+	return augmentItemInfo(dii, path.OneDriveService, item, size, parentPath)
 }
 
 func (h itemBackupHandler) FormatDisplayPath(
@@ -162,7 +160,7 @@ func (h itemRestoreHandler) AugmentItemInfo(
 	size int64,
 	parentPath *path.Builder,
 ) details.ItemInfo {
-	return augmentItemInfo(dii, item, size, parentPath)
+	return augmentItemInfo(dii, path.OneDriveService, item, size, parentPath)
 }
 
 func (h itemRestoreHandler) DeleteItem(
@@ -236,51 +234,3 @@ func (h itemRestoreHandler) GetRootFolder(
 ) (models.DriveItemable, error) {
 	return h.ac.GetRootFolder(ctx, driveID)
 }
-
-// ---------------------------------------------------------------------------
-// Common
-// ---------------------------------------------------------------------------
-
-func augmentItemInfo(
-	dii details.ItemInfo,
-	item models.DriveItemable,
-	size int64,
-	parentPath *path.Builder,
-) details.ItemInfo {
-	var email, driveName, driveID string
-
-	if item.GetCreatedBy() != nil && item.GetCreatedBy().GetUser() != nil {
-		// User is sometimes not available when created via some
-		// external applications (like backup/restore solutions)
-		ed, ok := item.GetCreatedBy().GetUser().GetAdditionalData()["email"]
-		if ok {
-			email = *ed.(*string)
-		}
-	}
-
-	if item.GetParentReference() != nil {
-		driveID = ptr.Val(item.GetParentReference().GetDriveId())
-		driveName = strings.TrimSpace(ptr.Val(item.GetParentReference().GetName()))
-	}
-
-	var pps string
-	if parentPath != nil {
-		pps = parentPath.String()
-	}
-
-	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:      email,
-		ParentPath: pps,
-		Size:       size,
-	}
-
-	dii.Extension = &details.ExtensionData{}
-
-	return dii
-}

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

@@ -20,12 +20,17 @@ import (
 var _ BackupHandler = &libraryBackupHandler{}
 
 type libraryBackupHandler struct {
-	ac    api.Drives
+	ac      api.Drives
-	scope selectors.SharePointScope
+	scope   selectors.SharePointScope
+	service path.ServiceType
 }
 
-func NewLibraryBackupHandler(ac api.Drives, scope selectors.SharePointScope) libraryBackupHandler {
+func NewLibraryBackupHandler(
-	return libraryBackupHandler{ac, scope}
+	ac api.Drives,
+	scope selectors.SharePointScope,
+	service path.ServiceType,
+) libraryBackupHandler {
+	return libraryBackupHandler{ac, scope, service}
 }
 
 func (h libraryBackupHandler) Get(
@@ -42,7 +47,7 @@ func (h libraryBackupHandler) PathPrefix(
 	return path.Build(
 		tenantID,
 		resourceOwner,
-		path.SharePointService,
+		h.service,
 		path.LibrariesCategory,
 		false,
 		odConsts.DrivesPathDir,
@@ -54,7 +59,7 @@ func (h libraryBackupHandler) CanonicalPath(
 	folders *path.Builder,
 	tenantID, resourceOwner string,
 ) (path.Path, error) {
-	return folders.ToDataLayerSharePointPath(tenantID, resourceOwner, path.LibrariesCategory, false)
+	return folders.ToDataLayerPath(tenantID, resourceOwner, h.service, path.LibrariesCategory, false)
 }
 
 func (h libraryBackupHandler) ServiceCat() (path.ServiceType, path.CategoryType) {
@@ -81,7 +86,7 @@ func (h libraryBackupHandler) AugmentItemInfo(
 	size int64,
 	parentPath *path.Builder,
 ) details.ItemInfo {
-	return augmentLibraryItemInfo(dii, item, size, parentPath)
+	return augmentItemInfo(dii, h.service, item, size, parentPath)
 }
 
 // constructWebURL is a helper function for recreating the webURL
@@ -128,6 +133,7 @@ func (h libraryBackupHandler) NewLocationIDer(
 	driveID string,
 	elems ...string,
 ) details.LocationIDer {
+	// TODO(meain): path related changes for groups
 	return details.NewSharePointLocationIDer(driveID, elems...)
 }
 
@@ -160,11 +166,12 @@ func (h libraryBackupHandler) IncludesDir(dir string) bool {
 var _ RestoreHandler = &libraryRestoreHandler{}
 
 type libraryRestoreHandler struct {
-	ac api.Client
+	ac      api.Client
+	service path.ServiceType
 }
 
-func NewLibraryRestoreHandler(ac api.Client) libraryRestoreHandler {
+func NewLibraryRestoreHandler(ac api.Client, service path.ServiceType) libraryRestoreHandler {
-	return libraryRestoreHandler{ac}
+	return libraryRestoreHandler{ac, service}
 }
 
 func (h libraryRestoreHandler) PostDrive(
@@ -187,7 +194,7 @@ func (h libraryRestoreHandler) AugmentItemInfo(
 	size int64,
 	parentPath *path.Builder,
 ) details.ItemInfo {
-	return augmentLibraryItemInfo(dii, item, size, parentPath)
+	return augmentItemInfo(dii, h.service, item, size, parentPath)
 }
 
 func (h libraryRestoreHandler) DeleteItem(
@@ -261,73 +268,3 @@ func (h libraryRestoreHandler) GetRootFolder(
 ) (models.DriveItemable, error) {
 	return h.ac.Drives().GetRootFolder(ctx, driveID)
 }
-
-// ---------------------------------------------------------------------------
-// Common
-// ---------------------------------------------------------------------------
-
-func augmentLibraryItemInfo(
-	dii details.ItemInfo,
-	item models.DriveItemable,
-	size int64,
-	parentPath *path.Builder,
-) details.ItemInfo {
-	var driveName, siteID, driveID, weburl, 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)
-		}
-	}
-
-	gsi := item.GetSharepointIds()
-	if gsi != nil {
-		siteID = ptr.Val(gsi.GetSiteId())
-		weburl = ptr.Val(gsi.GetSiteUrl())
-
-		if len(weburl) == 0 {
-			weburl = constructWebURL(item.GetAdditionalData())
-		}
-	}
-
-	if item.GetParentReference() != nil {
-		driveID = ptr.Val(item.GetParentReference().GetDriveId())
-		driveName = strings.TrimSpace(ptr.Val(item.GetParentReference().GetName()))
-	}
-
-	var pps string
-	if parentPath != nil {
-		pps = parentPath.String()
-	}
-
-	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:     siteID,
-		Size:       size,
-		WebURL:     weburl,
-	}
-
-	dii.Extension = &details.ExtensionData{}
-
-	return dii
-}

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

@@ -36,7 +36,7 @@ func (suite *LibraryBackupHandlerUnitSuite) TestCanonicalPath() {
 	for _, test := range table {
 		suite.Run(test.name, func() {
 			t := suite.T()
-			h := libraryBackupHandler{}
+			h := libraryBackupHandler{service: path.SharePointService}
 			p := path.Builder{}.Append("prefix")
 
 			result, err := h.CanonicalPath(p, tenantID, resourceOwner)

src/internal/m365/collection/site/restore.go

@@ -41,7 +41,7 @@ func ConsumeRestoreCollections(
 	ctr *count.Bus,
 ) (*support.ControllerOperationStatus, error) {
 	var (
-		lrh            = drive.NewLibraryRestoreHandler(ac)
+		lrh            = drive.NewLibraryRestoreHandler(ac, rcc.Selector.PathService())
 		restoreMetrics support.CollectionMetrics
 		caches         = drive.NewRestoreCaches(backupDriveIDNames)
 		el             = errs.Local()

src/internal/m365/service/sharepoint/backup.go

@@ -80,7 +80,7 @@ func ProduceBackupCollections(
 			spcs, canUsePreviousBackup, err = site.CollectLibraries(
 				ctx,
 				bpc,
-				drive.NewLibraryBackupHandler(ac.Drives(), scope),
+				drive.NewLibraryBackupHandler(ac.Drives(), scope, bpc.Selector.PathService()),
 				creds.AzureTenantID,
 				ssmb,
 				su,

src/internal/m365/service/sharepoint/backup_test.go

@@ -50,7 +50,8 @@ func (suite *LibrariesBackupUnitSuite) TestUpdateCollections() {
 	)
 
 	pb := path.Builder{}.Append(testBaseDrivePath.Elements()...)
-	ep, err := drive.NewLibraryBackupHandler(api.Drives{}, nil).CanonicalPath(pb, tenantID, siteID)
+	ep, err := drive.NewLibraryBackupHandler(api.Drives{}, nil, path.SharePointService).
+		CanonicalPath(pb, tenantID, siteID)
 	require.NoError(suite.T(), err, clues.ToCore(err))
 
 	tests := []struct {
@@ -100,7 +101,7 @@ func (suite *LibrariesBackupUnitSuite) TestUpdateCollections() {
 			)
 
 			c := drive.NewCollections(
-				drive.NewLibraryBackupHandler(api.Drives{}, test.scope),
+				drive.NewLibraryBackupHandler(api.Drives{}, test.scope, path.SharePointService),
 				tenantID,
 				siteID,
 				nil,

src/internal/m365/service/sharepoint/restore.go

@@ -33,7 +33,7 @@ func ConsumeRestoreCollections(
 	ctr *count.Bus,
 ) (*support.ControllerOperationStatus, error) {
 	var (
-		lrh            = drive.NewLibraryRestoreHandler(ac)
+		lrh            = drive.NewLibraryRestoreHandler(ac, rcc.Selector.PathService())
 		restoreMetrics support.CollectionMetrics
 		caches         = drive.NewRestoreCaches(backupDriveIDNames)
 		el             = errs.Local()

src/internal/operations/test/sharepoint_test.go

@@ -74,7 +74,7 @@ func (suite *SharePointBackupIntgSuite) TestBackup_Run_incrementalSharePoint() {
 	}
 
 	grh := func(ac api.Client) drive.RestoreHandler {
-		return drive.NewLibraryRestoreHandler(ac)
+		return drive.NewLibraryRestoreHandler(ac, path.SharePointService)
 	}
 
 	runDriveIncrementalTest(