From 302dd23ad9d71bd449b5902c6ec15f2f180b412c Mon Sep 17 00:00:00 2001 From: hiteshrepo Date: Mon, 11 Dec 2023 20:01:18 +0530 Subject: [PATCH 1/7] declare lists restore handlers --- src/internal/m365/collection/site/handlers.go | 21 +++++++++++++ .../m365/collection/site/lists_handler.go | 30 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/src/internal/m365/collection/site/handlers.go b/src/internal/m365/collection/site/handlers.go index 72d96d6b58..30bf34a4c0 100644 --- a/src/internal/m365/collection/site/handlers.go +++ b/src/internal/m365/collection/site/handlers.go @@ -13,3 +13,24 @@ type backupHandler interface { type getItemByIDer interface { GetItemByID(ctx context.Context, itemID string) (models.Listable, error) } + +type restoreHandler interface { + PostLister + PostListItemer +} + +type PostLister interface { + PostList( + ctx context.Context, + listName string, + oldListByteArray []byte, + ) (models.Listable, error) +} + +type PostListItemer interface { + PostListItem( + ctx context.Context, + listID string, + oldListByteArray []byte, + ) ([]models.ListItemable, error) +} diff --git a/src/internal/m365/collection/site/lists_handler.go b/src/internal/m365/collection/site/lists_handler.go index 1dab11c584..31513f40fd 100644 --- a/src/internal/m365/collection/site/lists_handler.go +++ b/src/internal/m365/collection/site/lists_handler.go @@ -25,3 +25,33 @@ func NewListsBackupHandler(protectedResource string, ac api.Lists) listsBackupHa func (bh listsBackupHandler) GetItemByID(ctx context.Context, itemID string) (models.Listable, error) { return bh.ac.GetListByID(ctx, bh.protectedResource, itemID) } + +var _ restoreHandler = &listsRestoreHandler{} + +type listsRestoreHandler struct { + ac api.Lists + protectedResource string +} + +func NewListsRestoreHandler(protectedResource string, ac api.Lists) listsRestoreHandler { + return listsRestoreHandler{ + ac: ac, + protectedResource: protectedResource, + } +} + +func (rh listsRestoreHandler) PostList( + ctx context.Context, + listName string, + oldListByteArray []byte, +) (models.Listable, error) { + return rh.ac.PostList(ctx, rh.protectedResource, listName, oldListByteArray) +} + +func (rh listsRestoreHandler) PostListItem( + ctx context.Context, + listID string, + oldListByteArray []byte, +) ([]models.ListItemable, error) { + return rh.ac.PostListItem(ctx, rh.protectedResource, listID, oldListByteArray) +} From 29ffa66067c385a813faf344869564a15b1c4e2a Mon Sep 17 00:00:00 2001 From: hiteshrepo Date: Tue, 12 Dec 2023 09:30:59 +0530 Subject: [PATCH 2/7] add lists restore handler --- .../m365/collection/site/collection_test.go | 65 --------- .../m365/collection/site/mock/list.go | 53 +++++++ src/internal/m365/collection/site/restore.go | 53 ++----- .../m365/collection/site/restore_test.go | 136 ++++++++++++++++++ .../m365/service/sharepoint/restore.go | 18 ++- .../m365/service/sharepoint/restore_test.go | 63 ++++++++ 6 files changed, 274 insertions(+), 114 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 a177a04246..6beb5663e0 100644 --- a/src/internal/m365/collection/site/collection_test.go +++ b/src/internal/m365/collection/site/collection_test.go @@ -7,12 +7,10 @@ import ( "github.com/alcionai/clues" kioser "github.com/microsoft/kiota-serialization-json-go" - "github.com/microsoftgraph/msgraph-sdk-go/sites" "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/data" "github.com/alcionai/corso/src/internal/m365/collection/site/mock" betaAPI "github.com/alcionai/corso/src/internal/m365/service/sharepoint/api" @@ -23,7 +21,6 @@ import ( "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/fault" "github.com/alcionai/corso/src/pkg/path" @@ -282,65 +279,3 @@ func (suite *SharePointCollectionSuite) TestCollection_streamItems() { }) } } - -// TestRestoreListCollection verifies Graph Restore API for the List Collection -func (suite *SharePointCollectionSuite) TestListCollection_Restore() { - t := suite.T() - // https://github.com/microsoftgraph/msgraph-sdk-go/issues/490 - t.Skip("disabled until upstream issue with list restore is fixed.") - - ctx, flush := tester.NewContext(t) - defer flush() - - service := createTestService(t, suite.creds) - listing := spMock.ListDefault("Mock List") - testName := "MockListing" - listing.SetDisplayName(&testName) - byteArray, err := service.Serialize(listing) - require.NoError(t, err, clues.ToCore(err)) - - listData, err := data.NewPrefetchedItemWithInfo( - io.NopCloser(bytes.NewReader(byteArray)), - testName, - details.ItemInfo{SharePoint: ListToSPInfo(listing, int64(len(byteArray)))}) - require.NoError(t, err, clues.ToCore(err)) - - destName := testdata.DefaultRestoreConfig("").Location - - deets, err := restoreListItem(ctx, service, listData, suite.siteID, destName) - assert.NoError(t, err, clues.ToCore(err)) - t.Logf("List created: %s\n", deets.SharePoint.ItemName) - - // Clean-Up - var ( - builder = service.Client().Sites().BySiteId(suite.siteID).Lists() - isFound bool - deleteID string - ) - - for { - resp, err := builder.Get(ctx, nil) - assert.NoError(t, err, "getting site lists", clues.ToCore(err)) - - for _, temp := range resp.GetValue() { - if ptr.Val(temp.GetDisplayName()) == deets.SharePoint.ItemName { - isFound = true - deleteID = ptr.Val(temp.GetId()) - - break - } - } - // Get Next Link - link, ok := ptr.ValOK(resp.GetOdataNextLink()) - if !ok { - break - } - - builder = sites.NewItemListsRequestBuilder(link, service.Adapter()) - } - - if isFound { - err := DeleteList(ctx, service, suite.siteID, deleteID) - assert.NoError(t, err, clues.ToCore(err)) - } -} diff --git a/src/internal/m365/collection/site/mock/list.go b/src/internal/m365/collection/site/mock/list.go index 30b4d50b15..76cf8981a5 100644 --- a/src/internal/m365/collection/site/mock/list.go +++ b/src/internal/m365/collection/site/mock/list.go @@ -2,10 +2,14 @@ package mock import ( "context" + "errors" + "strings" + "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/alcionai/corso/src/internal/common/ptr" + betaAPI "github.com/alcionai/corso/src/internal/m365/service/sharepoint/api" ) type ListHandler struct { @@ -21,3 +25,52 @@ func (lh *ListHandler) GetItemByID(ctx context.Context, itemID string) (models.L return ls, lh.Err } + +type ListRestoreHandler struct { + Err error +} + +func (lh *ListRestoreHandler) PostList( + ctx context.Context, + listName string, + oldListByteArray []byte, +) (models.Listable, error) { + newListName := listName + + oldList, err := betaAPI.CreateListFromBytes(oldListByteArray) + if err != nil { + return nil, errors.New("error while creating old list") + } + + if name, ok := ptr.ValOK(oldList.GetDisplayName()); ok { + nameParts := strings.Split(listName, "_") + if len(nameParts) > 0 { + nameParts[len(nameParts)-1] = name + newListName = strings.Join(nameParts, "_") + } + } + + newList := betaAPI.ToListable(oldList, newListName) + + return newList, lh.Err +} + +func (lh *ListRestoreHandler) PostListItem( + ctx context.Context, + listID string, + oldListByteArray []byte, +) ([]models.ListItemable, error) { + oldList, err := betaAPI.CreateListFromBytes(oldListByteArray) + if err != nil { + return nil, clues.WrapWC(ctx, err, "creating old list to get list items") + } + + contents := make([]models.ListItemable, 0) + + for _, itm := range oldList.GetItems() { + temp := betaAPI.CloneListItem(itm) + contents = append(contents, temp) + } + + return contents, lh.Err +} diff --git a/src/internal/m365/collection/site/restore.go b/src/internal/m365/collection/site/restore.go index c3a85ade3d..ef7d7eac80 100644 --- a/src/internal/m365/collection/site/restore.go +++ b/src/internal/m365/collection/site/restore.go @@ -8,7 +8,6 @@ import ( "runtime/trace" "github.com/alcionai/clues" - "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/alcionai/corso/src/internal/common/dttm" "github.com/alcionai/corso/src/internal/common/idname" @@ -42,6 +41,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 +89,7 @@ func ConsumeRestoreCollections( case path.ListsCategory: metrics, err = RestoreListCollection( ictx, - ac.Stable, + listsRh, dc, rcc.RestoreConfig.Location, deets, @@ -135,7 +135,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) { @@ -154,50 +154,19 @@ func restoreListItem( return dii, clues.WrapWC(ctx, err, "reading backup data") } - oldList, err := betaAPI.CreateListFromBytes(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 = betaAPI.ToListable(oldList, newName) - contents = make([]models.ListItemable, 0) - ) - - for _, itm := range oldList.GetItems() { - temp := betaAPI.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, byteArray) if err != nil { - return dii, graph.Wrap(ctx, err, "restoring list") + return dii, clues.WrapWC(ctx, err, "restoring lists") } // 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())) - } - } + _, err = rh.PostListItem(ctx, ptr.Val(restoredList.GetId()), byteArray) + if err != nil { + return dii, clues.WrapWC(ctx, err, "restoring list items") } dii.SharePoint = ListToSPInfo(restoredList, int64(len(byteArray))) @@ -207,7 +176,7 @@ func restoreListItem( func RestoreListCollection( ctx context.Context, - service graph.Servicer, + rh restoreHandler, dc data.RestoreCollection, restoreContainerName string, deets *details.Builder, @@ -243,7 +212,7 @@ func RestoreListCollection( itemInfo, err := restoreListItem( ctx, - service, + rh, itemData, siteID, restoreContainerName) 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 0000000000..4e2cd62ee4 --- /dev/null +++ b/src/internal/m365/collection/site/restore_test.go @@ -0,0 +1,136 @@ +package site + +import ( + "bytes" + "fmt" + "io" + "testing" + + "github.com/alcionai/clues" + "github.com/microsoftgraph/msgraph-sdk-go/sites" + "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() + + service := createTestService(t, suite.creds) + listing := spMock.ListDefault("Mock List") + testName := "MockListing" + listing.SetDisplayName(&testName) + 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: ListToSPInfo(listing, int64(len(byteArray)))}) + require.NoError(t, err, clues.ToCore(err)) + + r, err := readers.NewVersionedRestoreReader(listData.ToReader()) + require.NoError(t, err) + + mockData := &dataMock.Item{ + ItemID: testName, + Reader: r, + } + + lrh := NewListsRestoreHandler(suite.siteID, suite.ac.Lists()) + 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.ItemName) + + // Clean-Up + var ( + builder = service.Client().Sites().BySiteId(suite.siteID).Lists() + isFound bool + deleteID string + ) + + for { + resp, err := builder.Get(ctx, nil) + assert.NoError(t, err, "getting site lists", clues.ToCore(err)) + + for _, temp := range resp.GetValue() { + if ptr.Val(temp.GetDisplayName()) == deets.SharePoint.ItemName { + isFound = true + deleteID = ptr.Val(temp.GetId()) + + break + } + } + // Get Next Link + link, ok := ptr.ValOK(resp.GetOdataNextLink()) + if !ok { + break + } + + builder = sites.NewItemListsRequestBuilder(link, service.Adapter()) + } + + if isFound { + err := DeleteList(ctx, service, suite.siteID, deleteID) + assert.NoError(t, err, clues.ToCore(err)) + } +} diff --git a/src/internal/m365/service/sharepoint/restore.go b/src/internal/m365/service/sharepoint/restore.go index ab26728af8..5776eca305 100644 --- a/src/internal/m365/service/sharepoint/restore.go +++ b/src/internal/m365/service/sharepoint/restore.go @@ -48,16 +48,13 @@ 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") - } - // Reorder collections so that the parents directories are created // before the child directories; a requirement for permissions. data.SortRestoreCollections(dcs) @@ -81,6 +78,13 @@ func (h *sharepointHandler) ConsumeRestoreCollections( switch dc.FullPath().Category() { case path.LibrariesCategory: + caches := drive.NewRestoreCaches(h.backupDriveIDNames) + + 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 +99,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 0000000000..ca60694b53 --- /dev/null +++ b/src/internal/m365/service/sharepoint/restore_test.go @@ -0,0 +1,63 @@ +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/control" + "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) TestConsumeRestoreCollections_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(control.DefaultOptions(), api.Client{}, nil) + + _, _, err = sh.ConsumeRestoreCollections( + ctx, + rcc, + dcs, + fault.New(false), + nil) + require.NoError(t, err, "Sharepoint lists restore") +} From ed615d89d2a59248a88bd276e5f2de7aefe56e59 Mon Sep 17 00:00:00 2001 From: hiteshrepo Date: Tue, 12 Dec 2023 13:05:59 +0530 Subject: [PATCH 3/7] enable restore path creation for sharepoint lists --- .../operations/pathtransformer/restore_path_transformer.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/internal/operations/pathtransformer/restore_path_transformer.go b/src/internal/operations/pathtransformer/restore_path_transformer.go index 5ee431b53f..5cc7944793 100644 --- a/src/internal/operations/pathtransformer/restore_path_transformer.go +++ b/src/internal/operations/pathtransformer/restore_path_transformer.go @@ -142,7 +142,8 @@ func makeRestorePathsForEntry( // * OneDrive/SharePoint (needs drive information) switch true { case ent.Exchange != nil || - (ent.Groups != nil && ent.Groups.ItemType == details.GroupsChannelMessage): + (ent.Groups != nil && ent.Groups.ItemType == details.GroupsChannelMessage) || + (ent.SharePoint != nil && ent.SharePoint.ItemType == details.SharePointList): // TODO(ashmrtn): Eventually make Events have it's own function to handle // setting the restore destination properly. res.RestorePath, err = basicLocationPath(repoRef, locRef) From 9adf2c2bab0ebd79f5444f96aace17bfd17077b4 Mon Sep 17 00:00:00 2001 From: hiteshrepo Date: Tue, 12 Dec 2023 13:29:48 +0530 Subject: [PATCH 4/7] add delete-lister interface to list restore handler --- src/internal/m365/collection/site/handlers.go | 8 ++++++++ src/internal/m365/collection/site/lists.go | 16 ---------------- .../m365/collection/site/lists_handler.go | 7 +++++++ .../m365/collection/site/restore_test.go | 2 +- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/internal/m365/collection/site/handlers.go b/src/internal/m365/collection/site/handlers.go index 30bf34a4c0..e933779b0c 100644 --- a/src/internal/m365/collection/site/handlers.go +++ b/src/internal/m365/collection/site/handlers.go @@ -17,6 +17,7 @@ type getItemByIDer interface { type restoreHandler interface { PostLister PostListItemer + DeleteLister } type PostLister interface { @@ -34,3 +35,10 @@ type PostListItemer interface { oldListByteArray []byte, ) ([]models.ListItemable, error) } + +type DeleteLister interface { + DeleteList( + ctx context.Context, + listID string, + ) error +} diff --git a/src/internal/m365/collection/site/lists.go b/src/internal/m365/collection/site/lists.go index be336cc5b7..ab4e71011e 100644 --- a/src/internal/m365/collection/site/lists.go +++ b/src/internal/m365/collection/site/lists.go @@ -89,19 +89,3 @@ func PreFetchLists( return listTuples, nil } - -// DeleteList removes a list object from a site. -// deletes require unique http clients -// https://github.com/alcionai/corso/issues/2707 -func DeleteList( - ctx context.Context, - gs graph.Servicer, - siteID, listID string, -) error { - err := gs.Client().Sites().BySiteId(siteID).Lists().ByListId(listID).Delete(ctx, nil) - if err != nil { - return graph.Wrap(ctx, err, "deleting list") - } - - return nil -} diff --git a/src/internal/m365/collection/site/lists_handler.go b/src/internal/m365/collection/site/lists_handler.go index 31513f40fd..40421324f3 100644 --- a/src/internal/m365/collection/site/lists_handler.go +++ b/src/internal/m365/collection/site/lists_handler.go @@ -55,3 +55,10 @@ func (rh listsRestoreHandler) PostListItem( ) ([]models.ListItemable, error) { return rh.ac.PostListItem(ctx, rh.protectedResource, listID, oldListByteArray) } + +func (rh listsRestoreHandler) DeleteList( + ctx context.Context, + listID string, +) error { + return rh.ac.DeleteList(ctx, rh.protectedResource, listID) +} diff --git a/src/internal/m365/collection/site/restore_test.go b/src/internal/m365/collection/site/restore_test.go index 4e2cd62ee4..254184eb34 100644 --- a/src/internal/m365/collection/site/restore_test.go +++ b/src/internal/m365/collection/site/restore_test.go @@ -130,7 +130,7 @@ func (suite *SharePointRestoreSuite) TestListCollection_Restore() { } if isFound { - err := DeleteList(ctx, service, suite.siteID, deleteID) + err := lrh.DeleteList(ctx, deleteID) assert.NoError(t, err, clues.ToCore(err)) } } From 9f112392bb4469995ad792419fe7632311f86c52 Mon Sep 17 00:00:00 2001 From: hiteshrepo Date: Wed, 13 Dec 2023 12:50:33 +0530 Subject: [PATCH 5/7] export list de-serialization functions and handle unset column type --- .../m365/collection/site/mock/list.go | 18 +- src/internal/m365/collection/site/restore.go | 6 + src/pkg/services/m365/api/lists.go | 209 ++++++++++++------ 3 files changed, 161 insertions(+), 72 deletions(-) diff --git a/src/internal/m365/collection/site/mock/list.go b/src/internal/m365/collection/site/mock/list.go index 2f64ed2c1c..5047231a14 100644 --- a/src/internal/m365/collection/site/mock/list.go +++ b/src/internal/m365/collection/site/mock/list.go @@ -2,13 +2,13 @@ package mock import ( "context" - "encoding/json" "errors" "strings" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/alcionai/corso/src/internal/common/ptr" + "github.com/alcionai/corso/src/pkg/services/m365/api" ) type ListHandler struct { @@ -36,9 +36,8 @@ func (lh *ListRestoreHandler) PostList( ) (models.Listable, error) { newListName := listName - oldList := models.NewList() - - if err := json.Unmarshal(oldListByteArray, oldList); err != nil { + oldList, err := api.CreateListFromBytes(oldListByteArray) + if err != nil { return nil, errors.New("error while creating old list") } @@ -50,10 +49,9 @@ func (lh *ListRestoreHandler) PostList( } } - newList := *oldList - newList.SetName(&newListName) + newList := api.ToListable(oldList, newListName) - return &newList, lh.Err + return newList, lh.Err } func (lh *ListRestoreHandler) PostListItem( @@ -61,16 +59,16 @@ func (lh *ListRestoreHandler) PostListItem( listID string, oldListByteArray []byte, ) ([]models.ListItemable, error) { - oldList := models.NewList() - if err := json.Unmarshal(oldListByteArray, oldList); err != nil { + oldList, err := api.CreateListFromBytes(oldListByteArray) + if err != nil { return nil, errors.New("error while creating old list") } contents := make([]models.ListItemable, 0) for _, itm := range oldList.GetItems() { - temp := itm + temp := api.CloneListItem(itm) contents = append(contents, temp) } diff --git a/src/internal/m365/collection/site/restore.go b/src/internal/m365/collection/site/restore.go index ef7d7eac80..b4e40f5cd0 100644 --- a/src/internal/m365/collection/site/restore.go +++ b/src/internal/m365/collection/site/restore.go @@ -166,6 +166,12 @@ func restoreListItem( // Reference: https://learn.microsoft.com/en-us/graph/api/listitem-create?view=graph-rest-1.0&tabs=http _, err = rh.PostListItem(ctx, ptr.Val(restoredList.GetId()), byteArray) if err != nil { + // rollback list creation + err = rh.DeleteList(ctx, ptr.Val(restoredList.GetId())) + if err != nil { + return dii, clues.WrapWC(ctx, err, "deleting restored list") + } + return dii, clues.WrapWC(ctx, err, "restoring list items") } diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index 71e06c57bb..ebe17dd32e 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -151,7 +151,7 @@ func (c Lists) PostList( ) (models.Listable, error) { newListName := listName - oldList, err := createListFromBytes(oldListByteArray) + oldList, err := CreateListFromBytes(oldListByteArray) if err != nil { return nil, clues.WrapWC(ctx, err, "creating old list") } @@ -165,7 +165,7 @@ func (c Lists) PostList( } // this ensure all columns, contentTypes are set to the newList - newList := toListable(oldList, newListName) + newList := ToListable(oldList, newListName) // Restore to List base to M365 back store restoredList, err := c.Stable.Client().Sites().BySiteId(siteID).Lists().Post(ctx, newList, nil) @@ -181,7 +181,7 @@ func (c Lists) PostListItem( siteID, listID string, oldListByteArray []byte, ) ([]models.ListItemable, error) { - oldList, err := createListFromBytes(oldListByteArray) + oldList, err := CreateListFromBytes(oldListByteArray) if err != nil { return nil, clues.WrapWC(ctx, err, "creating old list to get list items") } @@ -189,7 +189,7 @@ func (c Lists) PostListItem( contents := make([]models.ListItemable, 0) for _, itm := range oldList.GetItems() { - temp := cloneListItem(itm) + temp := CloneListItem(itm) contents = append(contents, temp) } @@ -228,7 +228,7 @@ func (c Lists) DeleteList( return nil } -func createListFromBytes(bytes []byte) (models.Listable, error) { +func CreateListFromBytes(bytes []byte) (models.Listable, error) { parsable, err := CreateFromBytes(bytes, models.CreateListFromDiscriminatorValue) if err != nil { return nil, clues.Wrap(err, "deserializing bytes to sharepoint list") @@ -245,7 +245,7 @@ func createListFromBytes(bytes []byte) (models.Listable, error) { // not attached in this method. // ListItems are not included in creation of new list, and have to be restored // in separate call. -func toListable(orig models.Listable, displayName string) models.Listable { +func ToListable(orig models.Listable, displayName string) models.Listable { newList := models.NewList() newList.SetContentTypes(orig.GetContentTypes()) @@ -298,6 +298,42 @@ func toListable(orig models.Listable, displayName string) models.Listable { return newList } +// CloneListItem creates a new `SharePoint.ListItem` and stores the original item's +// M365 data into it set fields. +// - https://learn.microsoft.com/en-us/graph/api/resources/listitem?view=graph-rest-1.0 +func CloneListItem(orig models.ListItemable) models.ListItemable { + newItem := models.NewListItem() + + // list item data + newFieldData := retrieveFieldData(orig.GetFields()) + newItem.SetFields(newFieldData) + + // list item attributes + newItem.SetAdditionalData(orig.GetAdditionalData()) + newItem.SetDescription(orig.GetDescription()) + newItem.SetCreatedBy(orig.GetCreatedBy()) + newItem.SetCreatedDateTime(orig.GetCreatedDateTime()) + newItem.SetLastModifiedBy(orig.GetLastModifiedBy()) + newItem.SetLastModifiedDateTime(orig.GetLastModifiedDateTime()) + newItem.SetOdataType(orig.GetOdataType()) + newItem.SetAnalytics(orig.GetAnalytics()) + newItem.SetContentType(orig.GetContentType()) + newItem.SetVersions(orig.GetVersions()) + + // Requires nil checks to avoid Graph error: 'Invalid request' + lastCreatedByUser := orig.GetCreatedByUser() + if lastCreatedByUser != nil { + newItem.SetCreatedByUser(lastCreatedByUser) + } + + lastModifiedByUser := orig.GetLastModifiedByUser() + if lastCreatedByUser != nil { + newItem.SetLastModifiedByUser(lastModifiedByUser) + } + + return newItem +} + // cloneColumnDefinitionable utility function for encapsulating models.ColumnDefinitionable data // into new object for upload. func cloneColumnDefinitionable(orig models.ColumnDefinitionable) models.ColumnDefinitionable { @@ -324,20 +360,7 @@ func cloneColumnDefinitionable(orig models.ColumnDefinitionable) models.ColumnDe newColumn.SetEnforceUniqueValues(orig.GetEnforceUniqueValues()) // column types - newColumn.SetText(orig.GetText()) - newColumn.SetBoolean(orig.GetBoolean()) - newColumn.SetCalculated(orig.GetCalculated()) - newColumn.SetChoice(orig.GetChoice()) - newColumn.SetContentApprovalStatus(orig.GetContentApprovalStatus()) - newColumn.SetCurrency(orig.GetCurrency()) - newColumn.SetDateTime(orig.GetDateTime()) - newColumn.SetGeolocation(orig.GetGeolocation()) - newColumn.SetHyperlinkOrPicture(orig.GetHyperlinkOrPicture()) - newColumn.SetNumber(orig.GetNumber()) - newColumn.SetLookup(orig.GetLookup()) - newColumn.SetThumbnail(orig.GetThumbnail()) - newColumn.SetTerm(orig.GetTerm()) - newColumn.SetPersonOrGroup(orig.GetPersonOrGroup()) + setColumnType(newColumn, orig) // Requires nil checks to avoid Graph error: 'General exception while processing' defaultValue := orig.GetDefaultValue() @@ -353,40 +376,99 @@ func cloneColumnDefinitionable(orig models.ColumnDefinitionable) models.ColumnDe return newColumn } -// CloneListItem creates a new `SharePoint.ListItem` and stores the original item's -// M365 data into it set fields. -// - https://learn.microsoft.com/en-us/graph/api/resources/listitem?view=graph-rest-1.0 -func cloneListItem(orig models.ListItemable) models.ListItemable { - newItem := models.NewListItem() +func setColumnType(newColumn *models.ColumnDefinition, orig models.ColumnDefinitionable) { + isColumnTypeSet := false - // list item data - newFieldData := retrieveFieldData(orig.GetFields()) - newItem.SetFields(newFieldData) + if orig.GetText() != nil { + newColumn.SetText(orig.GetText()) - // list item attributes - newItem.SetAdditionalData(orig.GetAdditionalData()) - newItem.SetDescription(orig.GetDescription()) - newItem.SetCreatedBy(orig.GetCreatedBy()) - newItem.SetCreatedDateTime(orig.GetCreatedDateTime()) - newItem.SetLastModifiedBy(orig.GetLastModifiedBy()) - newItem.SetLastModifiedDateTime(orig.GetLastModifiedDateTime()) - newItem.SetOdataType(orig.GetOdataType()) - newItem.SetAnalytics(orig.GetAnalytics()) - newItem.SetContentType(orig.GetContentType()) - newItem.SetVersions(orig.GetVersions()) + isColumnTypeSet = true + } - // Requires nil checks to avoid Graph error: 'Invalid request' - lastCreatedByUser := orig.GetCreatedByUser() - if lastCreatedByUser != nil { - newItem.SetCreatedByUser(lastCreatedByUser) + if orig.GetBoolean() != nil { + newColumn.SetBoolean(orig.GetBoolean()) + + isColumnTypeSet = true } - lastModifiedByUser := orig.GetLastModifiedByUser() - if lastCreatedByUser != nil { - newItem.SetLastModifiedByUser(lastModifiedByUser) + if orig.GetCalculated() != nil { + newColumn.SetCalculated(orig.GetCalculated()) + + isColumnTypeSet = true } - return newItem + if orig.GetChoice() != nil { + newColumn.SetChoice(orig.GetChoice()) + + isColumnTypeSet = true + } + + if orig.GetContentApprovalStatus() != nil { + newColumn.SetContentApprovalStatus(orig.GetContentApprovalStatus()) + + isColumnTypeSet = true + } + + if orig.GetCurrency() != nil { + newColumn.SetCurrency(orig.GetCurrency()) + + isColumnTypeSet = true + } + + if orig.GetDateTime() != nil { + newColumn.SetDateTime(orig.GetDateTime()) + + isColumnTypeSet = true + } + + if orig.GetGeolocation() != nil { + newColumn.SetGeolocation(orig.GetGeolocation()) + + isColumnTypeSet = true + } + + if orig.GetHyperlinkOrPicture() != nil { + newColumn.SetHyperlinkOrPicture(orig.GetHyperlinkOrPicture()) + + isColumnTypeSet = true + } + + if orig.GetNumber() != nil { + newColumn.SetNumber(orig.GetNumber()) + + isColumnTypeSet = true + } + + if orig.GetLookup() != nil { + newColumn.SetLookup(orig.GetLookup()) + + isColumnTypeSet = true + } + + if orig.GetThumbnail() != nil { + newColumn.SetThumbnail(orig.GetThumbnail()) + + isColumnTypeSet = true + } + + if orig.GetTerm() != nil { + newColumn.SetTerm(orig.GetTerm()) + + isColumnTypeSet = true + } + + if orig.GetPersonOrGroup() != nil { + newColumn.SetPersonOrGroup(orig.GetPersonOrGroup()) + + isColumnTypeSet = true + } + + // defaulting to text type column + if !isColumnTypeSet { + textColumn := models.NewTextColumn() + + newColumn.SetText(textColumn) + } } // retrieveFieldData utility function to clone raw listItem data from the embedded @@ -396,22 +478,25 @@ func cloneListItem(orig models.ListItemable) models.ListItemable { func retrieveFieldData(orig models.FieldValueSetable) models.FieldValueSetable { fields := models.NewFieldValueSet() additionalData := make(map[string]any) - fieldData := orig.GetAdditionalData() - - // M365 Book keeping values removed during new Item Creation - // Removed Values: - // -- Prefixes -> @odata.context : absolute path to previous list - // . -> @odata.etag : Embedded link to Prior M365 ID - // -- String Match: Read-Only Fields - // -> id : previous un - for key, value := range fieldData { - if strings.HasPrefix(key, "_") || strings.HasPrefix(key, "@") || - key == "Edit" || key == "Created" || key == "Modified" || - strings.Contains(key, "LookupId") || strings.Contains(key, "ChildCount") || strings.Contains(key, "LinkTitle") { - continue - } - additionalData[key] = value + if orig != nil { + fieldData := orig.GetAdditionalData() + + // M365 Book keeping values removed during new Item Creation + // Removed Values: + // -- Prefixes -> @odata.context : absolute path to previous list + // . -> @odata.etag : Embedded link to Prior M365 ID + // -- String Match: Read-Only Fields + // -> id : previous un + for key, value := range fieldData { + if strings.HasPrefix(key, "_") || strings.HasPrefix(key, "@") || + key == "Edit" || key == "Created" || key == "Modified" || + strings.Contains(key, "LookupId") || strings.Contains(key, "ChildCount") || strings.Contains(key, "LinkTitle") { + continue + } + + additionalData[key] = value + } } fields.SetAdditionalData(additionalData) From 923b491bdf779114b75489dc5746c8b67c45a1e0 Mon Sep 17 00:00:00 2001 From: hiteshrepo Date: Wed, 13 Dec 2023 17:24:10 +0530 Subject: [PATCH 6/7] ignore attachments and contenttype while list item creation --- .../m365/collection/site/mock/list.go | 1 - src/pkg/services/m365/api/lists.go | 27 ++++++++++++------- src/pkg/services/m365/api/lists_test.go | 2 +- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/src/internal/m365/collection/site/mock/list.go b/src/internal/m365/collection/site/mock/list.go index 5047231a14..154d084d23 100644 --- a/src/internal/m365/collection/site/mock/list.go +++ b/src/internal/m365/collection/site/mock/list.go @@ -59,7 +59,6 @@ func (lh *ListRestoreHandler) PostListItem( listID string, oldListByteArray []byte, ) ([]models.ListItemable, error) { - oldList, err := api.CreateListFromBytes(oldListByteArray) if err != nil { return nil, errors.New("error while creating old list") diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index ebe17dd32e..9d4603a6e8 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -11,6 +11,12 @@ import ( "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) +var legacyColumns = map[string]struct{}{ + "Attachments": {}, + "Edit": {}, + "Content Type": {}, +} + // --------------------------------------------------------------------------- // controller // --------------------------------------------------------------------------- @@ -262,11 +268,6 @@ func ToListable(orig models.Listable, displayName string) models.Listable { newList.SetParentReference(orig.GetParentReference()) columns := make([]models.ColumnDefinitionable, 0) - leg := map[string]struct{}{ - "Attachments": {}, - "Edit": {}, - "Content Type": {}, - } for _, cd := range orig.GetColumns() { var ( @@ -282,7 +283,7 @@ func ToListable(orig models.Listable, displayName string) models.Listable { readOnly = ro } - _, isLegacy := leg[displayName] + _, isLegacy := legacyColumns[displayName] // Skips columns that cannot be uploaded for models.ColumnDefinitionable: // - ReadOnly, Title, or Legacy columns: Attachments, Edit, or Content Type @@ -488,10 +489,18 @@ func retrieveFieldData(orig models.FieldValueSetable) models.FieldValueSetable { // . -> @odata.etag : Embedded link to Prior M365 ID // -- String Match: Read-Only Fields // -> id : previous un + for key, value := range fieldData { - if strings.HasPrefix(key, "_") || strings.HasPrefix(key, "@") || - key == "Edit" || key == "Created" || key == "Modified" || - strings.Contains(key, "LookupId") || strings.Contains(key, "ChildCount") || strings.Contains(key, "LinkTitle") { + if strings.HasPrefix(key, "_") || + strings.HasPrefix(key, "@") || + key == "Edit" || + key == "Attachments" || + key == "ContentType" || + key == "Created" || + key == "Modified" || + strings.Contains(key, "LinkTitle") || + strings.Contains(key, "LookupId") || + strings.Contains(key, "ChildCount") { continue } diff --git a/src/pkg/services/m365/api/lists_test.go b/src/pkg/services/m365/api/lists_test.go index 5a5aa761cf..f25014e62f 100644 --- a/src/pkg/services/m365/api/lists_test.go +++ b/src/pkg/services/m365/api/lists_test.go @@ -62,7 +62,7 @@ func (suite *ListsUnitSuite) TestCreateListFromBytes() { suite.Run(test.name, func() { t := suite.T() - result, err := createListFromBytes(test.byteArray) + result, err := CreateListFromBytes(test.byteArray) test.checkError(t, err, clues.ToCore(err)) test.isNil(t, result) }) From 80d077ec1d738e665fdddf5d8b407f43321fb5c6 Mon Sep 17 00:00:00 2001 From: hiteshrepo Date: Thu, 14 Dec 2023 00:32:47 +0530 Subject: [PATCH 7/7] handle location fields --- src/pkg/services/m365/api/lists.go | 69 +++++++++++++++++++++++++++--- 1 file changed, 63 insertions(+), 6 deletions(-) diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index 9d4603a6e8..e14abf501a 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -17,6 +17,34 @@ var legacyColumns = map[string]struct{}{ "Content Type": {}, } +var readOnlyFieldNames = map[string]struct{}{ + "Attachments": {}, + "Edit": {}, + "ContentType": {}, + "Created": {}, + "Modified": {}, + "AuthorLookupId": {}, + "EditorLookupId": {}, +} + +var addressFieldNames = map[string]struct{}{ + "address": {}, + "coordinates": {}, + "displayName": {}, + "locationUri": {}, + "uniqueId": {}, +} + +var readonlyAddressFieldNames = map[string]struct{}{ + "CountryOrRegion": {}, + "State": {}, + "City": {}, + "PostalCode": {}, + "Street": {}, + "GeoLoc": {}, + "DispName": {}, +} + // --------------------------------------------------------------------------- // controller // --------------------------------------------------------------------------- @@ -491,15 +519,12 @@ func retrieveFieldData(orig models.FieldValueSetable) models.FieldValueSetable { // -> id : previous un for key, value := range fieldData { + _, isReadOnlyField := readOnlyFieldNames[key] + if strings.HasPrefix(key, "_") || strings.HasPrefix(key, "@") || - key == "Edit" || - key == "Attachments" || - key == "ContentType" || - key == "Created" || - key == "Modified" || + isReadOnlyField || strings.Contains(key, "LinkTitle") || - strings.Contains(key, "LookupId") || strings.Contains(key, "ChildCount") { continue } @@ -508,7 +533,39 @@ func retrieveFieldData(orig models.FieldValueSetable) models.FieldValueSetable { } } + retainPrimaryAddressField(additionalData) + fields.SetAdditionalData(additionalData) return fields } + +func retainPrimaryAddressField(additionalData map[string]interface{}) { + if !hasAddressFields(additionalData) { + return + } + + for k := range readonlyAddressFieldNames { + delete(additionalData, k) + } +} + +func hasAddressFields(additionalData map[string]interface{}) bool { + for _, value := range additionalData { + if nestedFields, ok := value.(map[string]interface{}); ok && + hasRequiredFields(nestedFields, addressFieldNames) && + hasRequiredFields(additionalData, readonlyAddressFieldNames) { + return true + } + } + return false +} + +func hasRequiredFields(data map[string]interface{}, checkFieldNames map[string]struct{}) bool { + for field := range checkFieldNames { + if _, exists := data[field]; !exists { + return false + } + } + return true +}