From eb8f3131f9b15fb74f8fb6f2596fdfec6782d1b4 Mon Sep 17 00:00:00 2001 From: Abin Simon Date: Thu, 7 Sep 2023 12:06:18 +0530 Subject: [PATCH] Misc fixes for handling groups (#4195) A bunch of places where we missed handling groups --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Issue(s) * # #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/m365/graph/metadata/metadata.go | 2 +- .../m365/graph/metadata/metadata_test.go | 5 + src/pkg/backup/details/details.go | 4 +- src/pkg/backup/details/details_test.go | 110 +++++++++++++----- src/pkg/backup/details/groups.go | 6 +- 5 files changed, 90 insertions(+), 37 deletions(-) diff --git a/src/internal/m365/graph/metadata/metadata.go b/src/internal/m365/graph/metadata/metadata.go index d213cd481..f85606587 100644 --- a/src/internal/m365/graph/metadata/metadata.go +++ b/src/internal/m365/graph/metadata/metadata.go @@ -10,7 +10,7 @@ func IsMetadataFile(p path.Path) bool { case path.OneDriveService: return metadata.HasMetaSuffix(p.Item()) - case path.SharePointService: + case path.SharePointService, path.GroupsService: return p.Category() == path.LibrariesCategory && metadata.HasMetaSuffix(p.Item()) default: diff --git a/src/internal/m365/graph/metadata/metadata_test.go b/src/internal/m365/graph/metadata/metadata_test.go index 15b190a19..6419f991c 100644 --- a/src/internal/m365/graph/metadata/metadata_test.go +++ b/src/internal/m365/graph/metadata/metadata_test.go @@ -73,6 +73,11 @@ var ( category: path.PagesCategory, expected: assert.Falsef, }, + { + service: path.GroupsService, + category: path.LibrariesCategory, + expected: assert.Truef, + }, } ) diff --git a/src/pkg/backup/details/details.go b/src/pkg/backup/details/details.go index 4d224f475..a51f5a277 100644 --- a/src/pkg/backup/details/details.go +++ b/src/pkg/backup/details/details.go @@ -47,8 +47,8 @@ func (d *Details) add( // Use the item name and the path for the ShortRef. This ensures that renames // within a directory generate unique ShortRefs. if info.infoType() == OneDriveItem || info.infoType() == SharePointLibrary { - if info.OneDrive == nil && info.SharePoint == nil { - return entry, clues.New("item is not SharePoint or OneDrive type") + if info.OneDrive == nil && info.SharePoint == nil && info.Groups == nil { + return entry, clues.New("item is not Groups, SharePoint or OneDrive type") } // clean metadata suffixes from item refs diff --git a/src/pkg/backup/details/details_test.go b/src/pkg/backup/details/details_test.go index 1d5e57035..591510f70 100644 --- a/src/pkg/backup/details/details_test.go +++ b/src/pkg/backup/details/details_test.go @@ -207,32 +207,51 @@ func exchangeEntry(t *testing.T, id string, size int, it ItemType) Entry { } } -func oneDriveishEntry(t *testing.T, id string, size int, it ItemType) Entry { - service := path.OneDriveService - category := path.FilesCategory - info := ItemInfo{ - OneDrive: &OneDriveInfo{ - ItemName: "bar", - DriveID: "drive-id", - DriveName: "drive-name", - Modified: time.Now(), - ItemType: it, - Size: int64(size), - }, - } +func oneDriveishEntry(t *testing.T, id string, size int, it ItemType, service path.ServiceType) Entry { + var ( + category path.CategoryType + info ItemInfo + ) - if it == SharePointLibrary { - service = path.SharePointService + switch it { + case OneDriveItem: + category = path.FilesCategory + info = ItemInfo{ + OneDrive: &OneDriveInfo{ + ItemName: "bar", + DriveID: "drive-id", + DriveName: "drive-name", + Modified: time.Now(), + ItemType: it, + Size: int64(size), + }, + } + case SharePointLibrary: category = path.LibrariesCategory - info.OneDrive = nil - info.SharePoint = &SharePointInfo{ - ItemName: "bar", - DriveID: "drive-id", - DriveName: "drive-name", - Modified: time.Now(), - ItemType: it, - Size: int64(size), + switch service { + case path.SharePointService: + info = ItemInfo{ + SharePoint: &SharePointInfo{ + ItemName: "bar", + DriveID: "drive-id", + DriveName: "drive-name", + Modified: time.Now(), + ItemType: it, + Size: int64(size), + }, + } + case path.GroupsService: + info = ItemInfo{ + Groups: &GroupsInfo{ + ItemName: "bar", + DriveID: "drive-id", + DriveName: "drive-name", + Modified: time.Now(), + ItemType: it, + Size: int64(size), + }, + } } } @@ -277,21 +296,25 @@ func (suite *DetailsUnitSuite) TestDetailsAdd_NoLocationFolders() { }, { name: "OneDrive File", - entry: oneDriveishEntry(t, itemID, 42, OneDriveItem), + entry: oneDriveishEntry(t, itemID, 42, OneDriveItem, path.OneDriveService), }, { name: "SharePoint File", - entry: oneDriveishEntry(t, itemID, 42, SharePointLibrary), + entry: oneDriveishEntry(t, itemID, 42, SharePointLibrary, path.SharePointService), }, { name: "Legacy SharePoint File", entry: func() Entry { - res := oneDriveishEntry(t, itemID, 42, SharePointLibrary) + res := oneDriveishEntry(t, itemID, 42, SharePointLibrary, path.SharePointService) res.SharePoint.ItemType = OneDriveItem return res }(), }, + { + name: "Group SharePoint File", + entry: oneDriveishEntry(t, itemID, 42, SharePointLibrary, path.GroupsService), + }, } for _, test := range table { @@ -333,11 +356,11 @@ func (suite *DetailsUnitSuite) TestDetailsAdd_LocationFolders() { t := suite.T() exchangeMail1 := exchangeEntry(t, "foo1", 42, ExchangeMail) - oneDrive1 := oneDriveishEntry(t, "foo1", 42, OneDriveItem) - sharePoint1 := oneDriveishEntry(t, "foo1", 42, SharePointLibrary) - sharePointLegacy1 := oneDriveishEntry(t, "foo1", 42, SharePointLibrary) + oneDrive1 := oneDriveishEntry(t, "foo1", 42, OneDriveItem, path.OneDriveService) + sharePoint1 := oneDriveishEntry(t, "foo1", 42, SharePointLibrary, path.SharePointService) + sharePointLegacy1 := oneDriveishEntry(t, "foo1", 42, SharePointLibrary, path.SharePointService) sharePointLegacy1.SharePoint.ItemType = OneDriveItem - + group1 := oneDriveishEntry(t, "foo1", 42, SharePointLibrary, path.GroupsService) // Sleep for a little so we get a larger difference in mod times between the // earlier and later entries. time.Sleep(100 * time.Millisecond) @@ -611,6 +634,33 @@ func (suite *DetailsUnitSuite) TestDetailsAdd_LocationFolders() { res = append(res, e) } + return res + }, + }, + { + name: "One Group SharePoint Item", + entries: func() []Entry { + e := group1 + ei := *group1.Groups + e.Groups = &ei + + return []Entry{e} + }, + expectedDirs: func() []Entry { + res := []Entry{} + + for _, entry := range oneDriveishFolders { + e := entry + ei := *entry.Folder + + e.Folder = &ei + e.Folder.DataType = SharePointLibrary + e.Folder.Size = group1.Groups.Size + e.Folder.Modified = group1.Groups.Modified + + res = append(res, e) + } + return res }, }, diff --git a/src/pkg/backup/details/groups.go b/src/pkg/backup/details/groups.go index 847b36fd6..3d8368600 100644 --- a/src/pkg/backup/details/groups.go +++ b/src/pkg/backup/details/groups.go @@ -17,7 +17,6 @@ func NewGroupsLocationIDer( driveID string, escapedFolders ...string, ) (uniqueLoc, error) { - // TODO(meain): path fixes if err := path.ValidateServiceAndCategory(path.GroupsService, category); err != nil { return uniqueLoc{}, clues.Wrap(err, "making groups LocationIDer") } @@ -26,12 +25,12 @@ func NewGroupsLocationIDer( prefixElems := 1 if driveID != "" { // non sp paths don't have driveID - pb.Append(driveID) + pb = pb.Append(driveID) prefixElems = 2 } - pb.Append(escapedFolders...) + pb = pb.Append(escapedFolders...) return uniqueLoc{pb, prefixElems}, nil } @@ -125,7 +124,6 @@ func (i *GroupsInfo) uniqueLocation(baseLoc *path.Builder) (*uniqueLoc, error) { } func (i *GroupsInfo) updateFolder(f *FolderInfo) error { - // TODO(meain): path updates if any if i.ItemType == SharePointLibrary { return updateFolderWithinDrive(SharePointLibrary, i.DriveName, i.DriveID, f) }