From a1590e0d2ff42fe5c810c558aa6ad452c06765f5 Mon Sep 17 00:00:00 2001 From: Hitesh Pattanayak <48874082+HiteshRepo@users.noreply.github.com> Date: Fri, 22 Dec 2023 22:48:24 +0530 Subject: [PATCH] enables sharepoint to use lists restore handler for lists ops (#4923) enables sharepoint to use lists restore handler for lists ops Changes previously approved in: - https://github.com/alcionai/corso/pull/4854 - https://github.com/alcionai/corso/pull/4910 #### Does this PR need a docs update or release note? - [x] :no_entry: No #### Type of change - [x] :sunflower: Feature #### Issue(s) #4754 #### Test Plan - [x] :muscle: Manual - [x] :zap: Unit test - [x] :green_heart: E2E --- .../m365/collection/site/collection_test.go | 94 --------- .../m365/collection/site/helper_test.go | 22 +++ src/internal/m365/collection/site/restore.go | 60 ++---- .../m365/collection/site/restore_test.go | 184 ++++++++++++++++++ .../m365/service/sharepoint/restore.go | 21 +- .../m365/service/sharepoint/restore_test.go | 62 ++++++ src/pkg/services/m365/api/lists.go | 31 +-- src/pkg/services/m365/api/lists_test.go | 1 + 8 files changed, 311 insertions(+), 164 deletions(-) create mode 100644 src/internal/m365/collection/site/restore_test.go create mode 100644 src/internal/m365/service/sharepoint/restore_test.go diff --git a/src/internal/m365/collection/site/collection_test.go b/src/internal/m365/collection/site/collection_test.go index afcf00880..56c6b0057 100644 --- a/src/internal/m365/collection/site/collection_test.go +++ b/src/internal/m365/collection/site/collection_test.go @@ -17,7 +17,6 @@ import ( "github.com/alcionai/corso/src/internal/m365/collection/site/mock" betaAPI "github.com/alcionai/corso/src/internal/m365/service/sharepoint/api" spMock "github.com/alcionai/corso/src/internal/m365/service/sharepoint/mock" - "github.com/alcionai/corso/src/internal/m365/support" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/internal/tester/tconfig" "github.com/alcionai/corso/src/pkg/account" @@ -241,96 +240,3 @@ func (suite *SharePointCollectionSuite) TestCollection_Items() { }) } } - -func (suite *SharePointCollectionSuite) TestCollection_streamItems() { - var ( - t = suite.T() - statusUpdater = func(*support.ControllerOperationStatus) {} - tenant = "some" - resource = "siteid" - list = "list" - ) - - table := []struct { - name string - category path.CategoryType - items []string - getDir func(t *testing.T) path.Path - }{ - { - name: "no items", - items: []string{}, - category: path.ListsCategory, - getDir: func(t *testing.T) path.Path { - dir, err := path.Build( - tenant, - resource, - path.SharePointService, - path.ListsCategory, - false, - list) - require.NoError(t, err, clues.ToCore(err)) - - return dir - }, - }, - { - name: "with items", - items: []string{"list1", "list2", "list3"}, - category: path.ListsCategory, - getDir: func(t *testing.T) path.Path { - dir, err := path.Build( - tenant, - resource, - path.SharePointService, - path.ListsCategory, - false, - list) - require.NoError(t, err, clues.ToCore(err)) - - return dir - }, - }, - } - for _, test := range table { - suite.Run(test.name, func() { - t.Log("running test", test) - - var ( - errs = fault.New(true) - itemCount int - ) - - ctx, flush := tester.NewContext(t) - defer flush() - - col := &Collection{ - fullPath: test.getDir(t), - category: test.category, - items: test.items, - getter: &mock.ListHandler{}, - stream: make(chan data.Item), - statusUpdater: statusUpdater, - } - - itemMap := func(js []string) map[string]struct{} { - m := make(map[string]struct{}) - for _, j := range js { - m[j] = struct{}{} - } - return m - }(test.items) - - go col.streamItems(ctx, errs) - - for item := range col.stream { - itemCount++ - _, ok := itemMap[item.ID()] - assert.True(t, ok, "should fetch item") - } - - assert.NoError(t, errs.Failure()) - assert.Equal(t, len(test.items), itemCount, "should see all expected items") - }) - } -} diff --git a/src/internal/m365/collection/site/helper_test.go b/src/internal/m365/collection/site/helper_test.go index 344620993..e9f396a10 100644 --- a/src/internal/m365/collection/site/helper_test.go +++ b/src/internal/m365/collection/site/helper_test.go @@ -1,9 +1,16 @@ package site import ( + "testing" + + "github.com/alcionai/clues" msgraphsdk "github.com/microsoftgraph/msgraph-sdk-go" + "github.com/stretchr/testify/require" "github.com/alcionai/corso/src/internal/m365/support" + "github.com/alcionai/corso/src/pkg/account" + "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) // --------------------------------------------------------------------------- @@ -35,3 +42,18 @@ func (ms *MockGraphService) Adapter() *msgraphsdk.GraphRequestAdapter { func (ms *MockGraphService) UpdateStatus(*support.ControllerOperationStatus) { } + +// --------------------------------------------------------------------------- +// Helper functions +// --------------------------------------------------------------------------- + +func createTestService(t *testing.T, credentials account.M365Config) *graph.Service { + adapter, err := graph.CreateAdapter( + credentials.AzureTenantID, + credentials.AzureClientID, + credentials.AzureClientSecret, + count.New()) + require.NoError(t, err, "creating microsoft graph service for exchange", clues.ToCore(err)) + + return graph.NewService(adapter) +} diff --git a/src/internal/m365/collection/site/restore.go b/src/internal/m365/collection/site/restore.go index a6a57cd66..a69e3bc5d 100644 --- a/src/internal/m365/collection/site/restore.go +++ b/src/internal/m365/collection/site/restore.go @@ -8,10 +8,8 @@ import ( "runtime/trace" "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" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/diagnostics" "github.com/alcionai/corso/src/internal/m365/collection/drive" @@ -42,6 +40,7 @@ func ConsumeRestoreCollections( ) (*support.ControllerOperationStatus, error) { var ( lrh = drive.NewSiteRestoreHandler(ac, rcc.Selector.PathService()) + listsRh = NewListsRestoreHandler(rcc.ProtectedResource.ID(), ac.Lists()) restoreMetrics support.CollectionMetrics caches = drive.NewRestoreCaches(backupDriveIDNames) el = errs.Local() @@ -89,7 +88,7 @@ func ConsumeRestoreCollections( case path.ListsCategory: metrics, err = RestoreListCollection( ictx, - ac.Stable, + listsRh, dc, rcc.RestoreConfig.Location, deets, @@ -135,7 +134,7 @@ func ConsumeRestoreCollections( // Restored List can be verified within the Site contents. func restoreListItem( ctx context.Context, - service graph.Servicer, + rh restoreHandler, itemData data.Item, siteID, destName string, ) (details.ItemInfo, error) { @@ -149,57 +148,19 @@ func restoreListItem( listName = itemData.ID() ) - byteArray, err := io.ReadAll(itemData.ToReader()) + bytes, err := io.ReadAll(itemData.ToReader()) if err != nil { return dii, clues.WrapWC(ctx, err, "reading backup data") } - oldList, err := api.BytesToListable(byteArray) - if err != nil { - return dii, clues.WrapWC(ctx, err, "creating item") - } - - if name, ok := ptr.ValOK(oldList.GetDisplayName()); ok { - listName = name - } - - var ( - newName = fmt.Sprintf("%s_%s", destName, listName) - newList = api.ToListable(oldList, newName) - contents = make([]models.ListItemable, 0) - ) - - for _, itm := range oldList.GetItems() { - temp := api.CloneListItem(itm) - contents = append(contents, temp) - } - - newList.SetItems(contents) + newName := fmt.Sprintf("%s_%s", destName, listName) // Restore to List base to M365 back store - restoredList, err := service.Client().Sites().BySiteId(siteID).Lists().Post(ctx, newList, nil) + restoredList, err := rh.PostList(ctx, newName, bytes) if err != nil { return dii, graph.Wrap(ctx, err, "restoring list") } - // Uploading of ListItems is conducted after the List is restored - // Reference: https://learn.microsoft.com/en-us/graph/api/listitem-create?view=graph-rest-1.0&tabs=http - if len(contents) > 0 { - for _, lItem := range contents { - _, err := service.Client(). - Sites(). - BySiteId(siteID). - Lists(). - ByListId(ptr.Val(restoredList.GetId())). - Items(). - Post(ctx, lItem, nil) - if err != nil { - return dii, graph.Wrap(ctx, err, "restoring list items"). - With("restored_list_id", ptr.Val(restoredList.GetId())) - } - } - } - dii.SharePoint = api.ListToSPInfo(restoredList) return dii, nil @@ -207,7 +168,7 @@ func restoreListItem( func RestoreListCollection( ctx context.Context, - service graph.Servicer, + rh restoreHandler, dc data.RestoreCollection, restoreContainerName string, deets *details.Builder, @@ -243,10 +204,15 @@ func RestoreListCollection( itemInfo, err := restoreListItem( ctx, - service, + rh, itemData, siteID, restoreContainerName) + if err != nil && + errors.Is(err, api.ErrCannotCreateWebTemplateExtension) { + continue + } + if err != nil { el.AddRecoverable(ctx, err) continue diff --git a/src/internal/m365/collection/site/restore_test.go b/src/internal/m365/collection/site/restore_test.go new file mode 100644 index 000000000..438c992b2 --- /dev/null +++ b/src/internal/m365/collection/site/restore_test.go @@ -0,0 +1,184 @@ +package site + +import ( + "bytes" + "context" + "fmt" + "io" + "testing" + + "github.com/alcionai/clues" + "github.com/microsoftgraph/msgraph-sdk-go/models" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/alcionai/corso/src/internal/common/ptr" + "github.com/alcionai/corso/src/internal/common/readers" + "github.com/alcionai/corso/src/internal/data" + dataMock "github.com/alcionai/corso/src/internal/data/mock" + spMock "github.com/alcionai/corso/src/internal/m365/service/sharepoint/mock" + "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/internal/tester/tconfig" + "github.com/alcionai/corso/src/pkg/account" + "github.com/alcionai/corso/src/pkg/backup/details" + "github.com/alcionai/corso/src/pkg/control" + "github.com/alcionai/corso/src/pkg/control/testdata" + "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/services/m365/api" + "github.com/alcionai/corso/src/pkg/services/m365/api/graph" +) + +type SharePointRestoreSuite struct { + tester.Suite + siteID string + creds account.M365Config + ac api.Client +} + +func (suite *SharePointRestoreSuite) SetupSuite() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + graph.InitializeConcurrencyLimiter(ctx, false, 4) + + suite.siteID = tconfig.M365SiteID(t) + a := tconfig.NewM365Account(t) + m365, err := a.M365Config() + require.NoError(t, err, clues.ToCore(err)) + + suite.creds = m365 + + ac, err := api.NewClient( + m365, + control.DefaultOptions(), + count.New()) + require.NoError(t, err, clues.ToCore(err)) + + suite.ac = ac +} + +func TestSharePointRestoreSuite(t *testing.T) { + suite.Run(t, &SharePointRestoreSuite{ + Suite: tester.NewIntegrationSuite( + t, + [][]string{tconfig.M365AcctCredEnvs}), + }) +} + +// TestRestoreListCollection verifies Graph Restore API for the List Collection +func (suite *SharePointRestoreSuite) TestListCollection_Restore() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + testName, lrh, destName, mockData := setupDependencies( + suite, + suite.ac, + suite.siteID, + suite.creds, + "genericList") + + deets, err := restoreListItem(ctx, lrh, mockData, suite.siteID, destName) + require.NoError(t, err, clues.ToCore(err)) + assert.Equal(t, fmt.Sprintf("%s_%s", destName, testName), deets.SharePoint.List.Name) + + // Clean-Up + deleteList(ctx, t, suite.siteID, lrh, deets) +} + +func (suite *SharePointRestoreSuite) TestListCollection_Restore_invalidListTemplate() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + _, lrh, destName, mockData := setupDependencies( + suite, + suite.ac, + suite.siteID, + suite.creds, + api.WebTemplateExtensionsListTemplateName) + + _, err := restoreListItem(ctx, lrh, mockData, suite.siteID, destName) + require.Error(t, err) + assert.Contains(t, err.Error(), api.ErrCannotCreateWebTemplateExtension.Error()) +} + +func deleteList( + ctx context.Context, + t *testing.T, + siteID string, + lrh listsRestoreHandler, + deets details.ItemInfo, +) { + var ( + isFound bool + deleteID string + ) + + lists, err := lrh.ac.Client. + Lists(). + GetLists(ctx, siteID, api.CallConfig{}) + assert.NoError(t, err, "getting site lists", clues.ToCore(err)) + + for _, l := range lists { + if ptr.Val(l.GetDisplayName()) == deets.SharePoint.ItemName { + isFound = true + deleteID = ptr.Val(l.GetId()) + + break + } + } + + if isFound { + err := lrh.DeleteList(ctx, deleteID) + assert.NoError(t, err, clues.ToCore(err)) + } +} + +func setupDependencies( + suite tester.Suite, + ac api.Client, + siteID string, + creds account.M365Config, + listTemplate string) ( + string, listsRestoreHandler, string, *dataMock.Item, +) { + t := suite.T() + testName := "MockListing" + + lrh := NewListsRestoreHandler(siteID, ac.Lists()) + + service := createTestService(t, creds) + + listInfo := models.NewListInfo() + listInfo.SetTemplate(ptr.To(listTemplate)) + + listing := spMock.ListDefault("Mock List") + listing.SetDisplayName(&testName) + listing.SetList(listInfo) + + byteArray, err := service.Serialize(listing) + require.NoError(t, err, clues.ToCore(err)) + + destName := testdata.DefaultRestoreConfig("").Location + + listData, err := data.NewPrefetchedItemWithInfo( + io.NopCloser(bytes.NewReader(byteArray)), + testName, + details.ItemInfo{SharePoint: api.ListToSPInfo(listing)}) + require.NoError(t, err, clues.ToCore(err)) + + r, err := readers.NewVersionedRestoreReader(listData.ToReader()) + require.NoError(t, err) + + mockData := &dataMock.Item{ + ItemID: testName, + Reader: r, + } + + return testName, lrh, destName, mockData +} diff --git a/src/internal/m365/service/sharepoint/restore.go b/src/internal/m365/service/sharepoint/restore.go index ccd4a1bc9..bd64b707b 100644 --- a/src/internal/m365/service/sharepoint/restore.go +++ b/src/internal/m365/service/sharepoint/restore.go @@ -48,15 +48,15 @@ func (h *sharepointHandler) ConsumeRestoreCollections( lrh = drive.NewSiteRestoreHandler( h.apiClient, rcc.Selector.PathService()) + listsRh = site.NewListsRestoreHandler( + rcc.ProtectedResource.ID(), + h.apiClient.Lists()) restoreMetrics support.CollectionMetrics - caches = drive.NewRestoreCaches(h.backupDriveIDNames) - el = errs.Local() - ) - err := caches.Populate(ctx, lrh, rcc.ProtectedResource.ID()) - if err != nil { - return nil, nil, clues.Wrap(err, "initializing restore caches") - } + caches = drive.NewRestoreCaches(h.backupDriveIDNames) + + el = errs.Local() + ) // Reorder collections so that the parents directories are created // before the child directories; a requirement for permissions. @@ -81,6 +81,11 @@ func (h *sharepointHandler) ConsumeRestoreCollections( switch dc.FullPath().Category() { case path.LibrariesCategory: + err = caches.Populate(ctx, lrh, rcc.ProtectedResource.ID()) + if err != nil { + return nil, nil, clues.Wrap(err, "initializing restore caches") + } + metrics, err = drive.RestoreCollection( ictx, lrh, @@ -95,7 +100,7 @@ func (h *sharepointHandler) ConsumeRestoreCollections( case path.ListsCategory: metrics, err = site.RestoreListCollection( ictx, - h.apiClient.Stable, + listsRh, dc, rcc.RestoreConfig.Location, deets, diff --git a/src/internal/m365/service/sharepoint/restore_test.go b/src/internal/m365/service/sharepoint/restore_test.go new file mode 100644 index 000000000..0551397b9 --- /dev/null +++ b/src/internal/m365/service/sharepoint/restore_test.go @@ -0,0 +1,62 @@ +package sharepoint + +import ( + "testing" + + "github.com/alcionai/clues" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/alcionai/corso/src/internal/common/idname" + "github.com/alcionai/corso/src/internal/data" + "github.com/alcionai/corso/src/internal/data/mock" + "github.com/alcionai/corso/src/internal/operations/inject" + "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/fault" + "github.com/alcionai/corso/src/pkg/path" + "github.com/alcionai/corso/src/pkg/services/m365/api" +) + +type SharepointRestoreUnitSuite struct { + tester.Suite +} + +func TestSharepointRestoreUnitSuite(t *testing.T) { + suite.Run(t, &SharepointRestoreUnitSuite{Suite: tester.NewUnitSuite(t)}) +} + +func (suite *SharepointRestoreUnitSuite) TestSharePointHandler_ConsumeRestoreCollections_noErrorOnLists() { + t := suite.T() + siteID := "site-id" + + ctx, flush := tester.NewContext(t) + defer flush() + + pr := idname.NewProvider(siteID, siteID) + rcc := inject.RestoreConsumerConfig{ + ProtectedResource: pr, + } + pth, err := path.Builder{}. + Append("lists"). + ToDataLayerPath( + "tenant", + siteID, + path.SharePointService, + path.ListsCategory, + false) + require.NoError(t, err, clues.ToCore(err)) + + dcs := []data.RestoreCollection{ + mock.Collection{Path: pth}, + } + + sh := NewSharePointHandler(api.Client{}, nil) + + _, _, err = sh.ConsumeRestoreCollections( + ctx, + rcc, + dcs, + fault.New(false), + nil) + require.NoError(t, err, "Sharepoint lists restore") +} diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index bd6b2057d..f8f97c2cb 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -16,13 +16,14 @@ import ( var ErrCannotCreateWebTemplateExtension = clues.New("unable to create webTemplateExtension type lists") const ( - AttachmentsColumnName = "Attachments" - EditColumnName = "Edit" - ContentTypeColumnName = "ContentType" - CreatedColumnName = "Created" - ModifiedColumnName = "Modified" - AuthorLookupIDColumnName = "AuthorLookupId" - EditorLookupIDColumnName = "EditorLookupId" + AttachmentsColumnName = "Attachments" + EditColumnName = "Edit" + ContentTypeColumnName = "ContentType" + CreatedColumnName = "Created" + ModifiedColumnName = "Modified" + AuthorLookupIDColumnName = "AuthorLookupId" + EditorLookupIDColumnName = "EditorLookupId" + AppAuthorLookupIDColumnName = "AppAuthorLookupId" ContentTypeColumnDisplayName = "Content Type" @@ -41,6 +42,7 @@ const ( DispNameFieldName = "DispName" LinkTitleFieldNamePart = "LinkTitle" ChildCountFieldNamePart = "ChildCount" + LookupIDFieldNamePart = "LookupId" ReadOnlyOrHiddenFieldNamePrefix = "_" DescoratorFieldNamePrefix = "@" @@ -73,13 +75,11 @@ var legacyColumns = keys.Set{ } var readOnlyFieldNames = keys.Set{ - AttachmentsColumnName: {}, - EditColumnName: {}, - ContentTypeColumnName: {}, - CreatedColumnName: {}, - ModifiedColumnName: {}, - AuthorLookupIDColumnName: {}, - EditorLookupIDColumnName: {}, + AttachmentsColumnName: {}, + EditColumnName: {}, + ContentTypeColumnName: {}, + CreatedColumnName: {}, + ModifiedColumnName: {}, } // --------------------------------------------------------------------------- @@ -546,7 +546,8 @@ func shouldFilterField(key string, value any) bool { strings.HasPrefix(key, ReadOnlyOrHiddenFieldNamePrefix) || strings.HasPrefix(key, DescoratorFieldNamePrefix) || strings.Contains(key, LinkTitleFieldNamePart) || - strings.Contains(key, ChildCountFieldNamePart) + strings.Contains(key, ChildCountFieldNamePart) || + strings.Contains(key, LookupIDFieldNamePart) } func retainPrimaryAddressField(additionalData map[string]any) { diff --git a/src/pkg/services/m365/api/lists_test.go b/src/pkg/services/m365/api/lists_test.go index 9fcfbdf02..df6bd27c5 100644 --- a/src/pkg/services/m365/api/lists_test.go +++ b/src/pkg/services/m365/api/lists_test.go @@ -409,6 +409,7 @@ func (suite *ListsUnitSuite) TestFieldValueSetable() { ReadOnlyOrHiddenFieldNamePrefix + "UIVersionString": "1.0", AuthorLookupIDColumnName: "6", EditorLookupIDColumnName: "6", + AppAuthorLookupIDColumnName: "6", "Item" + ChildCountFieldNamePart: "0", "Folder" + ChildCountFieldNamePart: "0", ModifiedColumnName: "2023-12-13T15:47:51Z",