From 26149ed85777d190a71cdaa53fc0562ff497f9ba Mon Sep 17 00:00:00 2001 From: Keepers Date: Fri, 23 Jun 2023 16:55:15 -0600 Subject: [PATCH] handle restore collisions in exchange (#3635) adds item collision handling to exchange restores. Currently an incomplete implementation; the replace setting will skip the restore altogether (no-op) as a first pass. The next PR will finish out the replace behavior. --- #### Does this PR need a docs update or release note? - [x] :no_entry: No #### Type of change - [x] :sunflower: Feature #### Issue(s) * #3562 #### Test Plan - [x] :zap: Unit test - [x] :green_heart: E2E --- src/internal/m365/exchange/attachment.go | 6 +- .../m365/exchange/contacts_restore.go | 51 ++++- .../m365/exchange/contacts_restore_test.go | 143 +++++++++++-- src/internal/m365/exchange/events_restore.go | 130 ++++++++++-- .../m365/exchange/events_restore_test.go | 198 ++++++++++++++++-- src/internal/m365/exchange/handlers.go | 34 ++- src/internal/m365/exchange/helper_test.go | 38 ++++ src/internal/m365/exchange/mail_restore.go | 63 +++++- .../m365/exchange/mail_restore_test.go | 160 ++++++++++++-- src/internal/m365/exchange/restore.go | 31 ++- src/internal/m365/exchange/restore_test.go | 9 + src/pkg/services/m365/api/events.go | 6 +- 12 files changed, 753 insertions(+), 116 deletions(-) create mode 100644 src/internal/m365/exchange/helper_test.go diff --git a/src/internal/m365/exchange/attachment.go b/src/internal/m365/exchange/attachment.go index 3f97342ea..ba8153a63 100644 --- a/src/internal/m365/exchange/attachment.go +++ b/src/internal/m365/exchange/attachment.go @@ -53,7 +53,7 @@ func attachmentType(attachment models.Attachmentable) models.AttachmentType { // uploadAttachment will upload the specified message attachment to M365 func uploadAttachment( ctx context.Context, - cli attachmentPoster, + ap attachmentPoster, userID, containerID, parentItemID string, attachment models.Attachmentable, ) error { @@ -102,13 +102,13 @@ func uploadAttachment( return clues.Wrap(err, "serializing attachment content").WithClues(ctx) } - _, err = cli.PostLargeAttachment(ctx, userID, containerID, parentItemID, name, content) + _, err = ap.PostLargeAttachment(ctx, userID, containerID, parentItemID, name, content) return err } // for all other attachments - return cli.PostSmallAttachment(ctx, userID, containerID, parentItemID, attachment) + return ap.PostSmallAttachment(ctx, userID, containerID, parentItemID, attachment) } func getOutlookOdataType(query models.Attachmentable) string { diff --git a/src/internal/m365/exchange/contacts_restore.go b/src/internal/m365/exchange/contacts_restore.go index 82ff1364a..076cd4a22 100644 --- a/src/internal/m365/exchange/contacts_restore.go +++ b/src/internal/m365/exchange/contacts_restore.go @@ -9,7 +9,9 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/m365/graph" "github.com/alcionai/corso/src/pkg/backup/details" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/fault" + "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" ) @@ -18,7 +20,6 @@ var _ itemRestorer = &contactRestoreHandler{} type contactRestoreHandler struct { ac api.Contacts - ip itemPoster[models.Contactable] } func newContactRestoreHandler( @@ -26,7 +27,6 @@ func newContactRestoreHandler( ) contactRestoreHandler { return contactRestoreHandler{ ac: ac.Contacts(), - ip: ac.Contacts(), } } @@ -65,6 +65,27 @@ func (h contactRestoreHandler) restore( ctx context.Context, body []byte, userID, destinationID string, + collisionKeyToItemID map[string]string, + collisionPolicy control.CollisionPolicy, + errs *fault.Bus, +) (*details.ExchangeInfo, error) { + return restoreContact( + ctx, + h.ac, + body, + userID, destinationID, + collisionKeyToItemID, + collisionPolicy, + errs) +} + +func restoreContact( + ctx context.Context, + pi postItemer[models.Contactable], + body []byte, + userID, destinationID string, + collisionKeyToItemID map[string]string, + collisionPolicy control.CollisionPolicy, errs *fault.Bus, ) (*details.ExchangeInfo, error) { contact, err := api.BytesToContactable(body) @@ -73,8 +94,20 @@ func (h contactRestoreHandler) restore( } ctx = clues.Add(ctx, "item_id", ptr.Val(contact.GetId())) + collisionKey := api.ContactCollisionKey(contact) - item, err := h.ip.PostItem(ctx, userID, destinationID, contact) + if _, ok := collisionKeyToItemID[collisionKey]; ok { + log := logger.Ctx(ctx).With("collision_key", clues.Hide(collisionKey)) + log.Debug("item collision") + + // TODO(rkeepers): Replace probably shouldn't no-op. Just a starting point. + if collisionPolicy == control.Skip || collisionPolicy == control.Replace { + log.Debug("skipping item with collision") + return nil, graph.ErrItemAlreadyExistsConflict + } + } + + item, err := pi.PostItem(ctx, userID, destinationID, contact) if err != nil { return nil, graph.Wrap(ctx, err, "restoring mail message") } @@ -84,3 +117,15 @@ func (h contactRestoreHandler) restore( return info, nil } + +func (h contactRestoreHandler) getItemsInContainerByCollisionKey( + ctx context.Context, + userID, containerID string, +) (map[string]string, error) { + m, err := h.ac.GetItemsInContainerByCollisionKey(ctx, userID, containerID) + if err != nil { + return nil, err + } + + return m, nil +} diff --git a/src/internal/m365/exchange/contacts_restore_test.go b/src/internal/m365/exchange/contacts_restore_test.go index de53f59e2..6ed6a3c5e 100644 --- a/src/internal/m365/exchange/contacts_restore_test.go +++ b/src/internal/m365/exchange/contacts_restore_test.go @@ -1,24 +1,46 @@ package exchange import ( + "context" "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/m365/exchange/mock" + "github.com/alcionai/corso/src/internal/m365/graph" "github.com/alcionai/corso/src/internal/tester" - "github.com/alcionai/corso/src/pkg/account" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/control/testdata" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" ) +var _ postItemer[models.Contactable] = &mockContactRestorer{} + +type mockContactRestorer struct { + postItemErr error +} + +func (m mockContactRestorer) PostItem( + ctx context.Context, + userID, containerID string, + body models.Contactable, +) (models.Contactable, error) { + return models.NewContact(), m.postItemErr +} + +// --------------------------------------------------------------------------- +// tests +// --------------------------------------------------------------------------- + type ContactsRestoreIntgSuite struct { tester.Suite - creds account.M365Config - ac api.Client - userID string + its intgTesterSetup } func TestContactsRestoreIntgSuite(t *testing.T) { @@ -30,29 +52,110 @@ func TestContactsRestoreIntgSuite(t *testing.T) { } func (suite *ContactsRestoreIntgSuite) SetupSuite() { - t := suite.T() - - a := tester.NewM365Account(t) - creds, err := a.M365Config() - require.NoError(t, err, clues.ToCore(err)) - - suite.creds = creds - - suite.ac, err = api.NewClient(creds) - require.NoError(t, err, clues.ToCore(err)) - - suite.userID = tester.M365UserID(t) + suite.its = newIntegrationTesterSetup(suite.T()) } // Testing to ensure that cache system works for in multiple different environments func (suite *ContactsRestoreIntgSuite) TestCreateContainerDestination() { runCreateDestinationTest( suite.T(), - newMailRestoreHandler(suite.ac), - path.EmailCategory, - suite.creds.AzureTenantID, - suite.userID, + newContactRestoreHandler(suite.its.ac), + path.ContactsCategory, + suite.its.creds.AzureTenantID, + suite.its.userID, testdata.DefaultRestoreConfig("").Location, []string{"Hufflepuff"}, []string{"Ravenclaw"}) } + +func (suite *ContactsRestoreIntgSuite) TestRestoreContact() { + body := mock.ContactBytes("middlename") + + stub, err := api.BytesToContactable(body) + require.NoError(suite.T(), err, clues.ToCore(err)) + + collisionKey := api.ContactCollisionKey(stub) + + table := []struct { + name string + apiMock postItemer[models.Contactable] + collisionMap map[string]string + onCollision control.CollisionPolicy + expectErr func(*testing.T, error) + }{ + { + name: "no collision: skip", + apiMock: mockContactRestorer{}, + collisionMap: map[string]string{}, + onCollision: control.Copy, + expectErr: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "no collision: copy", + apiMock: mockContactRestorer{}, + collisionMap: map[string]string{}, + onCollision: control.Skip, + expectErr: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "no collision: replace", + apiMock: mockContactRestorer{}, + collisionMap: map[string]string{}, + onCollision: control.Replace, + expectErr: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "collision: skip", + apiMock: mockContactRestorer{}, + collisionMap: map[string]string{collisionKey: "smarf"}, + onCollision: control.Skip, + expectErr: func(t *testing.T, err error) { + assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + }, + }, + { + name: "collision: copy", + apiMock: mockContactRestorer{}, + collisionMap: map[string]string{collisionKey: "smarf"}, + onCollision: control.Copy, + expectErr: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "collision: replace", + apiMock: mockContactRestorer{}, + collisionMap: map[string]string{collisionKey: "smarf"}, + onCollision: control.Replace, + expectErr: func(t *testing.T, err error) { + assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + }, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + _, err := restoreContact( + ctx, + test.apiMock, + body, + suite.its.userID, + "destination", + test.collisionMap, + test.onCollision, + fault.New(true)) + + test.expectErr(t, err) + }) + } +} diff --git a/src/internal/m365/exchange/events_restore.go b/src/internal/m365/exchange/events_restore.go index cb288771e..8ccb1232c 100644 --- a/src/internal/m365/exchange/events_restore.go +++ b/src/internal/m365/exchange/events_restore.go @@ -15,6 +15,7 @@ import ( "github.com/alcionai/corso/src/internal/common/str" "github.com/alcionai/corso/src/internal/m365/graph" "github.com/alcionai/corso/src/pkg/backup/details" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -25,17 +26,13 @@ var _ itemRestorer = &eventRestoreHandler{} type eventRestoreHandler struct { ac api.Events - ip itemPoster[models.Eventable] } func newEventRestoreHandler( ac api.Client, ) eventRestoreHandler { - ace := ac.Events() - return eventRestoreHandler{ - ac: ace, - ip: ace, + ac: ac.Events(), } } @@ -74,6 +71,32 @@ func (h eventRestoreHandler) restore( ctx context.Context, body []byte, userID, destinationID string, + collisionKeyToItemID map[string]string, + collisionPolicy control.CollisionPolicy, + errs *fault.Bus, +) (*details.ExchangeInfo, error) { + return restoreEvent( + ctx, + h.ac, + body, + userID, destinationID, + collisionKeyToItemID, + collisionPolicy, + errs) +} + +type eventRestorer interface { + postItemer[models.Eventable] + eventInstanceAndAttachmenter +} + +func restoreEvent( + ctx context.Context, + er eventRestorer, + body []byte, + userID, destinationID string, + collisionKeyToItemID map[string]string, + collisionPolicy control.CollisionPolicy, errs *fault.Bus, ) (*details.ExchangeInfo, error) { event, err := api.BytesToEventable(body) @@ -82,6 +105,18 @@ func (h eventRestoreHandler) restore( } ctx = clues.Add(ctx, "item_id", ptr.Val(event.GetId())) + collisionKey := api.EventCollisionKey(event) + + if _, ok := collisionKeyToItemID[collisionKey]; ok { + log := logger.Ctx(ctx).With("collision_key", clues.Hide(collisionKey)) + log.Debug("item collision") + + // TODO(rkeepers): Replace probably shouldn't no-op. Just a starting point. + if collisionPolicy == control.Skip || collisionPolicy == control.Replace { + log.Debug("skipping item with collision") + return nil, graph.ErrItemAlreadyExistsConflict + } + } event = toEventSimplified(event) @@ -94,14 +129,14 @@ func (h eventRestoreHandler) restore( event.SetAttachments(nil) } - item, err := h.ip.PostItem(ctx, userID, destinationID, event) + item, err := er.PostItem(ctx, userID, destinationID, event) if err != nil { return nil, graph.Wrap(ctx, err, "restoring calendar item") } err = uploadAttachments( ctx, - h.ac, + er, attachments, userID, destinationID, @@ -121,7 +156,7 @@ func (h eventRestoreHandler) restore( // Fix up event instances in case we have a recurring event err = updateRecurringEvents( ctx, - h.ac, + er, userID, destinationID, ptr.Val(item.GetId()), @@ -140,7 +175,7 @@ func (h eventRestoreHandler) restore( func updateRecurringEvents( ctx context.Context, - ac api.Events, + eiaa eventInstanceAndAttachmenter, userID, containerID, itemID string, event models.Eventable, errs *fault.Bus, @@ -155,12 +190,12 @@ func updateRecurringEvents( cancelledOccurrences := event.GetAdditionalData()["cancelledOccurrences"] exceptionOccurrences := event.GetAdditionalData()["exceptionOccurrences"] - err := updateCancelledOccurrences(ctx, ac, userID, itemID, cancelledOccurrences) + err := updateCancelledOccurrences(ctx, eiaa, userID, itemID, cancelledOccurrences) if err != nil { return clues.Wrap(err, "update cancelled occurrences") } - err = updateExceptionOccurrences(ctx, ac, userID, containerID, itemID, exceptionOccurrences, errs) + err = updateExceptionOccurrences(ctx, eiaa, userID, containerID, itemID, exceptionOccurrences, errs) if err != nil { return clues.Wrap(err, "update exception occurrences") } @@ -168,12 +203,30 @@ func updateRecurringEvents( return nil } +type eventInstanceAndAttachmenter interface { + attachmentGetDeletePoster + DeleteItem( + ctx context.Context, + userID, itemID string, + ) error + GetItemInstances( + ctx context.Context, + userID, itemID string, + startDate, endDate string, + ) ([]models.Eventable, error) + PatchItem( + ctx context.Context, + userID, eventID string, + body models.Eventable, + ) (models.Eventable, error) +} + // updateExceptionOccurrences take events that have exceptions, uses // the originalStart date to find the instance and modify it to match // the backup by updating the instance to match the backed up one func updateExceptionOccurrences( ctx context.Context, - ac api.Events, + eiaa eventInstanceAndAttachmenter, userID string, containerID string, itemID string, @@ -210,7 +263,7 @@ func updateExceptionOccurrences( // Get all instances on the day of the instance which should // just the one we need to modify - instances, err := ac.GetItemInstances(ictx, userID, itemID, startStr, endStr) + instances, err := eiaa.GetItemInstances(ictx, userID, itemID, startStr, endStr) if err != nil { return clues.Wrap(err, "getting instances") } @@ -225,7 +278,7 @@ func updateExceptionOccurrences( evt = toEventSimplified(evt) - _, err = ac.PatchItem(ictx, userID, ptr.Val(instances[0].GetId()), evt) + _, err = eiaa.PatchItem(ictx, userID, ptr.Val(instances[0].GetId()), evt) if err != nil { return clues.Wrap(err, "updating event instance") } @@ -238,7 +291,14 @@ func updateExceptionOccurrences( return clues.Wrap(err, "parsing event instance") } - err = updateAttachments(ictx, ac, userID, containerID, ptr.Val(instances[0].GetId()), evt, errs) + err = updateAttachments( + ictx, + eiaa, + userID, + containerID, + ptr.Val(instances[0].GetId()), + evt, + errs) if err != nil { return clues.Wrap(err, "updating event instance attachments") } @@ -247,6 +307,20 @@ func updateExceptionOccurrences( return nil } +type attachmentGetDeletePoster interface { + attachmentPoster + GetAttachments( + ctx context.Context, + immutableIDs bool, + userID string, + itemID string, + ) ([]models.Attachmentable, error) + DeleteAttachment( + ctx context.Context, + userID, calendarID, eventID, attachmentID string, + ) error +} + // updateAttachments updates the attachments of an event to match what // is present in the backed up event. Ideally we could make use of the // id of the series master event's attachments to see if we had @@ -259,14 +333,14 @@ func updateExceptionOccurrences( // would be better use Post[Small|Large]Attachment. func updateAttachments( ctx context.Context, - client api.Events, + agdp attachmentGetDeletePoster, userID, containerID, eventID string, event models.Eventable, errs *fault.Bus, ) error { el := errs.Local() - attachments, err := client.GetAttachments(ctx, false, userID, eventID) + attachments, err := agdp.GetAttachments(ctx, false, userID, eventID) if err != nil { return clues.Wrap(err, "getting attachments") } @@ -304,7 +378,7 @@ func updateAttachments( } if !found { - err = client.DeleteAttachment(ctx, userID, containerID, eventID, id) + err = agdp.DeleteAttachment(ctx, userID, containerID, eventID, id) if err != nil { logger.CtxErr(ctx, err).With("attachment_name", name).Info("attachment delete failed") el.AddRecoverable(ctx, clues.Wrap(err, "deleting event attachment"). @@ -342,7 +416,7 @@ func updateAttachments( } if !found { - err = uploadAttachment(ctx, client, userID, containerID, eventID, att) + err = uploadAttachment(ctx, agdp, userID, containerID, eventID, att) if err != nil { return clues.Wrap(err, "uploading attachment"). With("attachment_id", id) @@ -358,7 +432,7 @@ func updateAttachments( // that and uses the to get the event instance at that date to delete. func updateCancelledOccurrences( ctx context.Context, - ac api.Events, + eiaa eventInstanceAndAttachmenter, userID string, itemID string, cancelledOccurrences any, @@ -395,7 +469,7 @@ func updateCancelledOccurrences( // Get all instances on the day of the instance which should // just the one we need to modify - instances, err := ac.GetItemInstances(ctx, userID, itemID, startStr, endStr) + instances, err := eiaa.GetItemInstances(ctx, userID, itemID, startStr, endStr) if err != nil { return clues.Wrap(err, "getting instances") } @@ -408,7 +482,7 @@ func updateCancelledOccurrences( With("instances_count", len(instances), "search_start", startStr, "search_end", endStr) } - err = ac.DeleteItem(ctx, userID, ptr.Val(instances[0].GetId())) + err = eiaa.DeleteItem(ctx, userID, ptr.Val(instances[0].GetId())) if err != nil { return clues.Wrap(err, "deleting event instance") } @@ -416,3 +490,15 @@ func updateCancelledOccurrences( return nil } + +func (h eventRestoreHandler) getItemsInContainerByCollisionKey( + ctx context.Context, + userID, containerID string, +) (map[string]string, error) { + m, err := h.ac.GetItemsInContainerByCollisionKey(ctx, userID, containerID) + if err != nil { + return nil, err + } + + return m, nil +} diff --git a/src/internal/m365/exchange/events_restore_test.go b/src/internal/m365/exchange/events_restore_test.go index 156d191d1..ddd9983b8 100644 --- a/src/internal/m365/exchange/events_restore_test.go +++ b/src/internal/m365/exchange/events_restore_test.go @@ -1,24 +1,101 @@ package exchange import ( + "context" "testing" "github.com/alcionai/clues" + "github.com/google/uuid" + "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/m365/exchange/mock" + "github.com/alcionai/corso/src/internal/m365/graph" "github.com/alcionai/corso/src/internal/tester" - "github.com/alcionai/corso/src/pkg/account" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/control/testdata" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" ) +var _ eventRestorer = &mockEventRestorer{} + +type mockEventRestorer struct { + postItemErr error + postAttachmentErr error +} + +func (m mockEventRestorer) PostItem( + ctx context.Context, + userID, containerID string, + body models.Eventable, +) (models.Eventable, error) { + return models.NewEvent(), m.postItemErr +} + +func (m mockEventRestorer) PostSmallAttachment( + _ context.Context, + _, _, _ string, + _ models.Attachmentable, +) error { + return m.postAttachmentErr +} + +func (m mockEventRestorer) PostLargeAttachment( + _ context.Context, + _, _, _, _ string, + _ []byte, +) (string, error) { + return uuid.NewString(), m.postAttachmentErr +} + +func (m mockEventRestorer) DeleteAttachment( + ctx context.Context, + userID, calendarID, eventID, attachmentID string, +) error { + return nil +} + +func (m mockEventRestorer) DeleteItem( + ctx context.Context, + userID, itemID string, +) error { + return nil +} + +func (m mockEventRestorer) GetAttachments( + _ context.Context, + _ bool, + _, _ string, +) ([]models.Attachmentable, error) { + return []models.Attachmentable{}, nil +} + +func (m mockEventRestorer) GetItemInstances( + _ context.Context, + _, _, _, _ string, +) ([]models.Eventable, error) { + return []models.Eventable{}, nil +} + +func (m mockEventRestorer) PatchItem( + _ context.Context, + _, _ string, + _ models.Eventable, +) (models.Eventable, error) { + return models.NewEvent(), nil +} + +// --------------------------------------------------------------------------- +// tests +// --------------------------------------------------------------------------- + type EventsRestoreIntgSuite struct { tester.Suite - creds account.M365Config - ac api.Client - userID string + its intgTesterSetup } func TestEventsRestoreIntgSuite(t *testing.T) { @@ -30,29 +107,110 @@ func TestEventsRestoreIntgSuite(t *testing.T) { } func (suite *EventsRestoreIntgSuite) SetupSuite() { - t := suite.T() - - a := tester.NewM365Account(t) - creds, err := a.M365Config() - require.NoError(t, err, clues.ToCore(err)) - - suite.creds = creds - - suite.ac, err = api.NewClient(creds) - require.NoError(t, err, clues.ToCore(err)) - - suite.userID = tester.M365UserID(t) + suite.its = newIntegrationTesterSetup(suite.T()) } // Testing to ensure that cache system works for in multiple different environments func (suite *EventsRestoreIntgSuite) TestCreateContainerDestination() { runCreateDestinationTest( suite.T(), - newMailRestoreHandler(suite.ac), - path.EmailCategory, - suite.creds.AzureTenantID, - suite.userID, + newEventRestoreHandler(suite.its.ac), + path.EventsCategory, + suite.its.creds.AzureTenantID, + suite.its.userID, testdata.DefaultRestoreConfig("").Location, []string{"Durmstrang"}, []string{"Beauxbatons"}) } + +func (suite *EventsRestoreIntgSuite) TestRestoreEvent() { + body := mock.EventBytes("subject") + + stub, err := api.BytesToEventable(body) + require.NoError(suite.T(), err, clues.ToCore(err)) + + collisionKey := api.EventCollisionKey(stub) + + table := []struct { + name string + apiMock eventRestorer + collisionMap map[string]string + onCollision control.CollisionPolicy + expectErr func(*testing.T, error) + }{ + { + name: "no collision: skip", + apiMock: mockEventRestorer{}, + collisionMap: map[string]string{}, + onCollision: control.Copy, + expectErr: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "no collision: copy", + apiMock: mockEventRestorer{}, + collisionMap: map[string]string{}, + onCollision: control.Skip, + expectErr: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "no collision: replace", + apiMock: mockEventRestorer{}, + collisionMap: map[string]string{}, + onCollision: control.Replace, + expectErr: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "collision: skip", + apiMock: mockEventRestorer{}, + collisionMap: map[string]string{collisionKey: "smarf"}, + onCollision: control.Skip, + expectErr: func(t *testing.T, err error) { + assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + }, + }, + { + name: "collision: copy", + apiMock: mockEventRestorer{}, + collisionMap: map[string]string{collisionKey: "smarf"}, + onCollision: control.Copy, + expectErr: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "collision: replace", + apiMock: mockEventRestorer{}, + collisionMap: map[string]string{collisionKey: "smarf"}, + onCollision: control.Replace, + expectErr: func(t *testing.T, err error) { + assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + }, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + _, err := restoreEvent( + ctx, + test.apiMock, + body, + suite.its.userID, + "destination", + test.collisionMap, + test.onCollision, + fault.New(true)) + + test.expectErr(t, err) + }) + } +} diff --git a/src/internal/m365/exchange/handlers.go b/src/internal/m365/exchange/handlers.go index 9eb7d1fe1..243ad11fd 100644 --- a/src/internal/m365/exchange/handlers.go +++ b/src/internal/m365/exchange/handlers.go @@ -7,6 +7,7 @@ import ( "github.com/alcionai/corso/src/internal/m365/graph" "github.com/alcionai/corso/src/pkg/backup/details" + "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" @@ -60,6 +61,7 @@ func BackupHandlers(ac api.Client) map[path.CategoryType]backupHandler { type restoreHandler interface { itemRestorer containerAPI + getItemsByCollisionKeyser newContainerCache(userID string) graph.ContainerResolver formatRestoreDestination( destinationContainerName string, @@ -75,19 +77,12 @@ type itemRestorer interface { ctx context.Context, body []byte, userID, destinationID string, + collisionKeyToItemID map[string]string, + collisionPolicy control.CollisionPolicy, errs *fault.Bus, ) (*details.ExchangeInfo, error) } -// runs the actual graph API post request. -type itemPoster[T any] interface { - PostItem( - ctx context.Context, - userID, dirID string, - body T, - ) (T, error) -} - // produces structs that interface with the graph/cache_container // CachedContainer interface. type containerAPI interface { @@ -129,3 +124,24 @@ func restoreHandlers( path.EventsCategory: newEventRestoreHandler(ac), } } + +type getItemsByCollisionKeyser interface { + // GetItemsInContainerByCollisionKey looks up all items currently in + // the container, and returns them in a map[collisionKey]itemID. + // The collision key is uniquely defined by each category of data. + // Collision key checks are used during restore to handle the on- + // collision restore configurations that cause the item restore to get + // skipped, replaced, or copied. + getItemsInContainerByCollisionKey( + ctx context.Context, + userID, containerID string, + ) (map[string]string, error) +} + +type postItemer[T any] interface { + PostItem( + ctx context.Context, + userID, containerID string, + body T, + ) (T, error) +} diff --git a/src/internal/m365/exchange/helper_test.go b/src/internal/m365/exchange/helper_test.go new file mode 100644 index 000000000..222179d6c --- /dev/null +++ b/src/internal/m365/exchange/helper_test.go @@ -0,0 +1,38 @@ +package exchange + +import ( + "testing" + + "github.com/alcionai/clues" + "github.com/stretchr/testify/require" + + "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/account" + "github.com/alcionai/corso/src/pkg/services/m365/api" +) + +type intgTesterSetup struct { + ac api.Client + creds account.M365Config + userID string +} + +func newIntegrationTesterSetup(t *testing.T) intgTesterSetup { + its := intgTesterSetup{} + + ctx, flush := tester.NewContext(t) + defer flush() + + a := tester.NewM365Account(t) + creds, err := a.M365Config() + require.NoError(t, err, clues.ToCore(err)) + + its.creds = creds + + its.ac, err = api.NewClient(creds) + require.NoError(t, err, clues.ToCore(err)) + + its.userID = tester.GetM365UserID(ctx) + + return its +} diff --git a/src/internal/m365/exchange/mail_restore.go b/src/internal/m365/exchange/mail_restore.go index ce0979859..1ddb3e52b 100644 --- a/src/internal/m365/exchange/mail_restore.go +++ b/src/internal/m365/exchange/mail_restore.go @@ -10,7 +10,9 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/m365/graph" "github.com/alcionai/corso/src/pkg/backup/details" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/fault" + "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" ) @@ -19,17 +21,13 @@ var _ itemRestorer = &mailRestoreHandler{} type mailRestoreHandler struct { ac api.Mail - ip itemPoster[models.Messageable] } func newMailRestoreHandler( ac api.Client, ) mailRestoreHandler { - acm := ac.Mail() - return mailRestoreHandler{ - ac: acm, - ip: acm, + ac: ac.Mail(), } } @@ -72,6 +70,32 @@ func (h mailRestoreHandler) restore( ctx context.Context, body []byte, userID, destinationID string, + collisionKeyToItemID map[string]string, + collisionPolicy control.CollisionPolicy, + errs *fault.Bus, +) (*details.ExchangeInfo, error) { + return restoreMail( + ctx, + h.ac, + body, + userID, destinationID, + collisionKeyToItemID, + collisionPolicy, + errs) +} + +type mailRestorer interface { + postItemer[models.Messageable] + attachmentPoster +} + +func restoreMail( + ctx context.Context, + mr mailRestorer, + body []byte, + userID, destinationID string, + collisionKeyToItemID map[string]string, + collisionPolicy control.CollisionPolicy, errs *fault.Bus, ) (*details.ExchangeInfo, error) { msg, err := api.BytesToMessageable(body) @@ -80,20 +104,33 @@ func (h mailRestoreHandler) restore( } ctx = clues.Add(ctx, "item_id", ptr.Val(msg.GetId())) + collisionKey := api.MailCollisionKey(msg) + + if _, ok := collisionKeyToItemID[collisionKey]; ok { + log := logger.Ctx(ctx).With("collision_key", clues.Hide(collisionKey)) + log.Debug("item collision") + + // TODO(rkeepers): Replace probably shouldn't no-op. Just a starting point. + if collisionPolicy == control.Skip || collisionPolicy == control.Replace { + log.Debug("skipping item with collision") + return nil, graph.ErrItemAlreadyExistsConflict + } + } + msg = setMessageSVEPs(toMessage(msg)) attachments := msg.GetAttachments() // Item.Attachments --> HasAttachments doesn't always have a value populated when deserialized msg.SetAttachments([]models.Attachmentable{}) - item, err := h.ip.PostItem(ctx, userID, destinationID, msg) + item, err := mr.PostItem(ctx, userID, destinationID, msg) if err != nil { return nil, graph.Wrap(ctx, err, "restoring mail message") } err = uploadAttachments( ctx, - h.ac, + mr, attachments, userID, destinationID, @@ -138,3 +175,15 @@ func setMessageSVEPs(msg models.Messageable) models.Messageable { return msg } + +func (h mailRestoreHandler) getItemsInContainerByCollisionKey( + ctx context.Context, + userID, containerID string, +) (map[string]string, error) { + m, err := h.ac.GetItemsInContainerByCollisionKey(ctx, userID, containerID) + if err != nil { + return nil, err + } + + return m, nil +} diff --git a/src/internal/m365/exchange/mail_restore_test.go b/src/internal/m365/exchange/mail_restore_test.go index 9d71de800..fc2837c85 100644 --- a/src/internal/m365/exchange/mail_restore_test.go +++ b/src/internal/m365/exchange/mail_restore_test.go @@ -1,24 +1,64 @@ package exchange import ( + "context" "testing" "github.com/alcionai/clues" + "github.com/google/uuid" + "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/m365/exchange/mock" + "github.com/alcionai/corso/src/internal/m365/graph" "github.com/alcionai/corso/src/internal/tester" - "github.com/alcionai/corso/src/pkg/account" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/control/testdata" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" ) +var _ mailRestorer = &mockMailRestorer{} + +type mockMailRestorer struct { + postItemErr error + postAttachmentErr error +} + +func (m mockMailRestorer) PostItem( + ctx context.Context, + userID, containerID string, + body models.Messageable, +) (models.Messageable, error) { + return models.NewMessage(), m.postItemErr +} + +func (m mockMailRestorer) PostSmallAttachment( + _ context.Context, + _, _, _ string, + _ models.Attachmentable, +) error { + return m.postAttachmentErr +} + +func (m mockMailRestorer) PostLargeAttachment( + _ context.Context, + _, _, _, _ string, + _ []byte, +) (string, error) { + return uuid.NewString(), m.postAttachmentErr +} + +// --------------------------------------------------------------------------- +// tests +// --------------------------------------------------------------------------- + type MailRestoreIntgSuite struct { tester.Suite - creds account.M365Config - ac api.Client - userID string + its intgTesterSetup } func TestMailRestoreIntgSuite(t *testing.T) { @@ -30,29 +70,109 @@ func TestMailRestoreIntgSuite(t *testing.T) { } func (suite *MailRestoreIntgSuite) SetupSuite() { - t := suite.T() - - a := tester.NewM365Account(t) - creds, err := a.M365Config() - require.NoError(t, err, clues.ToCore(err)) - - suite.creds = creds - - suite.ac, err = api.NewClient(creds) - require.NoError(t, err, clues.ToCore(err)) - - suite.userID = tester.M365UserID(t) + suite.its = newIntegrationTesterSetup(suite.T()) } -// Testing to ensure that cache system works for in multiple different environments func (suite *MailRestoreIntgSuite) TestCreateContainerDestination() { runCreateDestinationTest( suite.T(), - newMailRestoreHandler(suite.ac), + newMailRestoreHandler(suite.its.ac), path.EmailCategory, - suite.creds.AzureTenantID, - suite.userID, + suite.its.creds.AzureTenantID, + suite.its.userID, testdata.DefaultRestoreConfig("").Location, []string{"Griffindor", "Croix"}, []string{"Griffindor", "Felicius"}) } + +func (suite *MailRestoreIntgSuite) TestRestoreMail() { + body := mock.MessageBytes("subject") + + stub, err := api.BytesToMessageable(body) + require.NoError(suite.T(), err, clues.ToCore(err)) + + collisionKey := api.MailCollisionKey(stub) + + table := []struct { + name string + apiMock mailRestorer + collisionMap map[string]string + onCollision control.CollisionPolicy + expectErr func(*testing.T, error) + }{ + { + name: "no collision: skip", + apiMock: mockMailRestorer{}, + collisionMap: map[string]string{}, + onCollision: control.Copy, + expectErr: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "no collision: copy", + apiMock: mockMailRestorer{}, + collisionMap: map[string]string{}, + onCollision: control.Skip, + expectErr: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "no collision: replace", + apiMock: mockMailRestorer{}, + collisionMap: map[string]string{}, + onCollision: control.Replace, + expectErr: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "collision: skip", + apiMock: mockMailRestorer{}, + collisionMap: map[string]string{collisionKey: "smarf"}, + onCollision: control.Skip, + expectErr: func(t *testing.T, err error) { + assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + }, + }, + { + name: "collision: copy", + apiMock: mockMailRestorer{}, + collisionMap: map[string]string{collisionKey: "smarf"}, + onCollision: control.Copy, + expectErr: func(t *testing.T, err error) { + assert.NoError(t, err, clues.ToCore(err)) + }, + }, + { + name: "collision: replace", + apiMock: mockMailRestorer{}, + collisionMap: map[string]string{collisionKey: "smarf"}, + onCollision: control.Replace, + expectErr: func(t *testing.T, err error) { + assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + }, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + _, err := restoreMail( + ctx, + test.apiMock, + body, + suite.its.userID, + "destination", + test.collisionMap, + test.onCollision, + fault.New(true)) + + test.expectErr(t, err) + }) + } +} diff --git a/src/internal/m365/exchange/restore.go b/src/internal/m365/exchange/restore.go index 77ffbe9f8..accc78d27 100644 --- a/src/internal/m365/exchange/restore.go +++ b/src/internal/m365/exchange/restore.go @@ -41,9 +41,7 @@ func ConsumeRestoreCollections( directoryCache = make(map[path.CategoryType]graph.ContainerResolver) handlers = restoreHandlers(ac) metrics support.CollectionMetrics - // TODO policy to be updated from external source after completion of refactoring - policy = control.Copy - el = errs.Local() + el = errs.Local() ) ctx = clues.Add(ctx, "resource_owner", clues.Hide(userID)) @@ -87,16 +85,22 @@ func ConsumeRestoreCollections( } directoryCache[category] = gcc - ictx = clues.Add(ictx, "restore_destination_id", containerID) + collisionKeyToItemID, err := handler.getItemsInContainerByCollisionKey(ctx, userID, containerID) + if err != nil { + el.AddRecoverable(ctx, clues.Wrap(err, "building item collision cache")) + continue + } + temp, err := restoreCollection( ictx, handler, dc, userID, containerID, - policy, + collisionKeyToItemID, + restoreCfg.OnCollision, deets, errs) @@ -127,7 +131,8 @@ func restoreCollection( ir itemRestorer, dc data.RestoreCollection, userID, destinationID string, - policy control.CollisionPolicy, + collisionKeyToItemID map[string]string, + collisionPolicy control.CollisionPolicy, deets *details.Builder, errs *fault.Bus, ) (support.CollectionMetrics, error) { @@ -172,9 +177,19 @@ func restoreCollection( body := buf.Bytes() - info, err := ir.restore(ictx, body, userID, destinationID, errs) + info, err := ir.restore( + ictx, + body, + userID, + destinationID, + collisionKeyToItemID, + collisionPolicy, + errs) if err != nil { - el.AddRecoverable(ictx, err) + if !graph.IsErrItemAlreadyExistsConflict(err) { + el.AddRecoverable(ictx, err) + } + continue } diff --git a/src/internal/m365/exchange/restore_test.go b/src/internal/m365/exchange/restore_test.go index 370e6e879..3c1aa51d2 100644 --- a/src/internal/m365/exchange/restore_test.go +++ b/src/internal/m365/exchange/restore_test.go @@ -13,6 +13,7 @@ import ( exchMock "github.com/alcionai/corso/src/internal/m365/exchange/mock" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/account" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/control/testdata" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" @@ -74,6 +75,8 @@ func (suite *RestoreIntgSuite) TestRestoreContact() { ctx, exchMock.ContactBytes("Corso TestContact"), userID, folderID, + nil, + control.Copy, fault.New(true)) assert.NoError(t, err, clues.ToCore(err)) assert.NotNil(t, info, "contact item info") @@ -141,6 +144,8 @@ func (suite *RestoreIntgSuite) TestRestoreEvent() { ctx, test.bytes, userID, calendarID, + nil, + control.Copy, fault.New(true)) assert.NoError(t, err, clues.ToCore(err)) assert.NotNil(t, info, "event item info") @@ -367,6 +372,8 @@ func (suite *RestoreIntgSuite) TestRestoreExchangeObject() { ctx, test.bytes, userID, destination, + nil, + control.Copy, fault.New(true)) assert.NoError(t, err, clues.ToCore(err)) assert.NotNil(t, info, "item info was not populated") @@ -396,6 +403,8 @@ func (suite *RestoreIntgSuite) TestRestoreAndBackupEvent_recurringInstancesWithA ctx, bytes, userID, calendarID, + nil, + control.Copy, fault.New(true)) require.NoError(t, err, clues.ToCore(err)) assert.NotNil(t, info, "event item info") diff --git a/src/pkg/services/m365/api/events.go b/src/pkg/services/m365/api/events.go index c57d4c078..4ed1b83b8 100644 --- a/src/pkg/services/m365/api/events.go +++ b/src/pkg/services/m365/api/events.go @@ -380,8 +380,7 @@ func parseableToMap(att serialization.Parsable) (map[string]any, error) { func (c Events) GetAttachments( ctx context.Context, immutableIDs bool, - userID string, - itemID string, + userID, itemID string, ) ([]models.Attachmentable, error) { config := &users.ItemEventsItemAttachmentsRequestBuilderGetRequestConfiguration{ QueryParameters: &users.ItemEventsItemAttachmentsRequestBuilderGetQueryParameters{ @@ -424,8 +423,7 @@ func (c Events) DeleteAttachment( func (c Events) GetItemInstances( ctx context.Context, - userID, itemID string, - startDate, endDate string, + userID, itemID, startDate, endDate string, ) ([]models.Eventable, error) { config := &users.ItemEventsItemInstancesRequestBuilderGetRequestConfiguration{ QueryParameters: &users.ItemEventsItemInstancesRequestBuilderGetQueryParameters{