From 9664846d22cc335779888f7a886392c2dd377159 Mon Sep 17 00:00:00 2001 From: Abin Simon Date: Wed, 23 Aug 2023 15:05:37 +0530 Subject: [PATCH] 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? - [ ] :white_check_mark: Yes, it's included - [x] :clock1: Yes, but in a later PR - [ ] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [x] :broom: Tech Debt/Cleanup #### Issue(s) * https://github.com/alcionai/corso/issues/3990 #### Test Plan - [ ] :muscle: Manual - [ ] :zap: Unit test - [ ] :green_heart: E2E --- .../m365/collection/drive/group_handler.go | 182 ++---------------- .../m365/collection/drive/handler_utils.go | 112 +++++++++++ .../m365/collection/drive/item_handler.go | 54 +----- .../m365/collection/drive/library_handler.go | 99 ++-------- .../collection/drive/library_handler_test.go | 2 +- src/internal/m365/collection/site/restore.go | 2 +- .../m365/service/sharepoint/backup.go | 2 +- .../m365/service/sharepoint/backup_test.go | 5 +- .../m365/service/sharepoint/restore.go | 2 +- .../operations/test/sharepoint_test.go | 2 +- 10 files changed, 155 insertions(+), 307 deletions(-) create mode 100644 src/internal/m365/collection/drive/handler_utils.go diff --git a/src/internal/m365/collection/drive/group_handler.go b/src/internal/m365/collection/drive/group_handler.go index 136d61b2d..d6faad8fb 100644 --- a/src/internal/m365/collection/drive/group_handler.go +++ b/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} -} - -func (h groupBackupHandler) Get( - ctx context.Context, - url string, - headers map[string]string, -) (*http.Response, error) { - return h.ac.Get(ctx, url, headers) -} - -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) + return groupBackupHandler{ + libraryBackupHandler{ + ac: ac, + // Not adding scope here. Anything that needs scope has to + // be from group handler + service: path.GroupsService, + }, + groupID, + scope, + } } 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 - return folders.ToDataLayerPath( - tenantID, - h.groupID, - path.GroupsService, - path.LibrariesCategory, - false) + // TODO(meain): path fixes + return folders.ToDataLayerPath(tenantID, h.groupID, h.service, 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 true + return h.scope.IsAny(selectors.GroupsLibraryFolder) } func (h groupBackupHandler) IncludesDir(dir string) bool { - // TODO(meain) - 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 + return h.scope.Matches(selectors.GroupsLibraryFolder, dir) } diff --git a/src/internal/m365/collection/drive/handler_utils.go b/src/internal/m365/collection/drive/handler_utils.go new file mode 100644 index 000000000..6dc4be66e --- /dev/null +++ b/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 +} diff --git a/src/internal/m365/collection/drive/item_handler.go b/src/internal/m365/collection/drive/item_handler.go index 929649aae..58fccd7a8 100644 --- a/src/internal/m365/collection/drive/item_handler.go +++ b/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 -} diff --git a/src/internal/m365/collection/drive/library_handler.go b/src/internal/m365/collection/drive/library_handler.go index 4649e458c..e06a279db 100644 --- a/src/internal/m365/collection/drive/library_handler.go +++ b/src/internal/m365/collection/drive/library_handler.go @@ -20,12 +20,17 @@ import ( var _ BackupHandler = &libraryBackupHandler{} type libraryBackupHandler struct { - ac api.Drives - scope selectors.SharePointScope + ac api.Drives + scope selectors.SharePointScope + service path.ServiceType } -func NewLibraryBackupHandler(ac api.Drives, scope selectors.SharePointScope) libraryBackupHandler { - return libraryBackupHandler{ac, scope} +func NewLibraryBackupHandler( + 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 { - return libraryRestoreHandler{ac} +func NewLibraryRestoreHandler(ac api.Client, service path.ServiceType) libraryRestoreHandler { + 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 -} diff --git a/src/internal/m365/collection/drive/library_handler_test.go b/src/internal/m365/collection/drive/library_handler_test.go index 1646868e0..93ff8d2ae 100644 --- a/src/internal/m365/collection/drive/library_handler_test.go +++ b/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) diff --git a/src/internal/m365/collection/site/restore.go b/src/internal/m365/collection/site/restore.go index 7b13df1a5..c83dd6290 100644 --- a/src/internal/m365/collection/site/restore.go +++ b/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() diff --git a/src/internal/m365/service/sharepoint/backup.go b/src/internal/m365/service/sharepoint/backup.go index ce7789b64..ad34ef9c9 100644 --- a/src/internal/m365/service/sharepoint/backup.go +++ b/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, diff --git a/src/internal/m365/service/sharepoint/backup_test.go b/src/internal/m365/service/sharepoint/backup_test.go index 2a7c6aad8..8365cb099 100644 --- a/src/internal/m365/service/sharepoint/backup_test.go +++ b/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, diff --git a/src/internal/m365/service/sharepoint/restore.go b/src/internal/m365/service/sharepoint/restore.go index 35e1c67cd..e4336dbd2 100644 --- a/src/internal/m365/service/sharepoint/restore.go +++ b/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() diff --git a/src/internal/operations/test/sharepoint_test.go b/src/internal/operations/test/sharepoint_test.go index dea0c23bf..8ab7d3e4d 100644 --- a/src/internal/operations/test/sharepoint_test.go +++ b/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(