diff --git a/CHANGELOG.md b/CHANGELOG.md index 9687ee84d..0babdb791 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Retry transient 400 "invalidRequest" errors during onedrive & sharepoint backup. - Backup attachments associated with group mailbox items. - Groups and Teams backups no longer fail when a resource has no display name. +- Contacts in-place restore failed if the restore destination was empty. ### Changed - When running `backup details` on an empty backup returns a more helpful error message. @@ -21,6 +22,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Known issues - Backing up a group mailbox item may fail if it has a very large number of attachments (500+). - Event description for exchange exports might look slightly different for certain events. +- Exchange in-place restore may restore items in well-known folders to different folders if the user has well-known folder names change based on locale and has updated the locale since the backup was created. +- In-place Exchange contacts restore will merge items in folders named "Contacts" or "contacts" into the default folder. ## [v0.18.0] (beta) - 2024-01-02 diff --git a/src/internal/m365/collection/exchange/contacts_restore.go b/src/internal/m365/collection/exchange/contacts_restore.go index 40fccb037..52c5225b9 100644 --- a/src/internal/m365/collection/exchange/contacts_restore.go +++ b/src/internal/m365/collection/exchange/contacts_restore.go @@ -19,7 +19,10 @@ import ( "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) -var _ itemRestorer = &contactRestoreHandler{} +var ( + _ itemRestorer = &contactRestoreHandler{} + _ restoreHandler = &contactRestoreHandler{} +) type contactRestoreHandler struct { ac api.Contacts @@ -41,11 +44,25 @@ func (h contactRestoreHandler) NewContainerCache(userID string) graph.ContainerR } } +func (h contactRestoreHandler) ShouldSetContainerToDefaultRoot( + restoreFolderPath string, + collectionPath path.Path, +) bool { + return len(collectionPath.Folders()) == 1 && + restoreFolderPath == h.DefaultRootContainer() +} + func (h contactRestoreHandler) FormatRestoreDestination( destinationContainerName string, - _ path.Path, // contact folders cannot be nested + collectionFullPath path.Path, // contact folders cannot be nested ) *path.Builder { - return path.Builder{}.Append(destinationContainerName) + // User passed in some location to restore to, use that. + if len(destinationContainerName) > 0 { + return path.Builder{}.Append(destinationContainerName) + } + + // FIXME(ashmrtn): Make sure this plays ok with nested folder creation. + return path.Builder{}.Append(collectionFullPath.Folders()...) } func (h contactRestoreHandler) CreateContainer( diff --git a/src/internal/m365/collection/exchange/events_restore.go b/src/internal/m365/collection/exchange/events_restore.go index 01330d380..9aeada8bb 100644 --- a/src/internal/m365/collection/exchange/events_restore.go +++ b/src/internal/m365/collection/exchange/events_restore.go @@ -20,7 +20,10 @@ import ( "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) -var _ itemRestorer = &eventRestoreHandler{} +var ( + _ itemRestorer = &eventRestoreHandler{} + _ restoreHandler = &eventRestoreHandler{} +) type eventRestoreHandler struct { ac api.Events @@ -42,6 +45,13 @@ func (h eventRestoreHandler) NewContainerCache(userID string) graph.ContainerRes } } +func (h eventRestoreHandler) ShouldSetContainerToDefaultRoot( + restoreFolderPath string, + collectionPath path.Path, +) bool { + return false +} + func (h eventRestoreHandler) FormatRestoreDestination( destinationContainerName string, _ path.Path, // ignored because calendars cannot be nested diff --git a/src/internal/m365/collection/exchange/handlers.go b/src/internal/m365/collection/exchange/handlers.go index e11cedb73..2da773a3d 100644 --- a/src/internal/m365/collection/exchange/handlers.go +++ b/src/internal/m365/collection/exchange/handlers.go @@ -66,6 +66,10 @@ type restoreHandler interface { containerAPI getItemsByCollisionKeyser NewContainerCache(userID string) graph.ContainerResolver + ShouldSetContainerToDefaultRoot( + restoreFolderPath string, + collectionPath path.Path, + ) bool FormatRestoreDestination( destinationContainerName string, collectionFullPath path.Path, diff --git a/src/internal/m365/collection/exchange/mail_restore.go b/src/internal/m365/collection/exchange/mail_restore.go index a50b733ad..8bb6309da 100644 --- a/src/internal/m365/collection/exchange/mail_restore.go +++ b/src/internal/m365/collection/exchange/mail_restore.go @@ -20,7 +20,10 @@ import ( "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) -var _ itemRestorer = &mailRestoreHandler{} +var ( + _ itemRestorer = &mailRestoreHandler{} + _ restoreHandler = &mailRestoreHandler{} +) type mailRestoreHandler struct { ac api.Mail @@ -42,6 +45,13 @@ func (h mailRestoreHandler) NewContainerCache(userID string) graph.ContainerReso } } +func (h mailRestoreHandler) ShouldSetContainerToDefaultRoot( + restoreFolderPath string, + collectionPath path.Path, +) bool { + return false +} + func (h mailRestoreHandler) FormatRestoreDestination( destinationContainerName string, collectionFullPath path.Path, diff --git a/src/internal/m365/restore_test.go b/src/internal/m365/restore_test.go new file mode 100644 index 000000000..8ed749a72 --- /dev/null +++ b/src/internal/m365/restore_test.go @@ -0,0 +1,176 @@ +package m365 + +import ( + "testing" + + "github.com/alcionai/clues" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/alcionai/corso/src/internal/data" + exchMock "github.com/alcionai/corso/src/internal/m365/service/exchange/mock" + "github.com/alcionai/corso/src/internal/operations/inject" + "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/internal/tester/tconfig" + "github.com/alcionai/corso/src/internal/version" + "github.com/alcionai/corso/src/pkg/control" + controlTD "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" + "github.com/alcionai/corso/src/pkg/selectors" + "github.com/alcionai/corso/src/pkg/services/m365/api" +) + +type RestoreIntgSuite struct { + tester.Suite +} + +func TestRestoreIntgSuite(t *testing.T) { + suite.Run(t, &RestoreIntgSuite{ + Suite: tester.NewIntegrationSuite( + t, + [][]string{tconfig.M365AcctCredEnvs}), + }) +} + +// TestRestoreCollections_HandlesEmptyRestoreLocation checks to make sure that +// even if the restore location is empty we fallback to using the collection +// path as the folder, resulting in an in-place restore. It doesn't attempt to +// retore any items because that would bloat the data set in the test user. +func (suite *RestoreIntgSuite) TestRestoreCollections_HandlesEmptyRestoreLocation() { + acct := tconfig.NewM365Account(suite.T()) + + table := []struct { + service path.ServiceType + category path.CategoryType + selector func(*testing.T) selectors.Selector + defaultPathFolders func() []string + secondaryPathFolders func(location string) []string + }{ + { + service: path.ExchangeService, + category: path.EmailCategory, + selector: func(t *testing.T) selectors.Selector { + sel := selectors.NewExchangeRestore([]string{tconfig.M365UserID(t)}) + sel.Include(sel.Mails(selectors.Any(), selectors.Any())) + + return sel.Selector + }, + defaultPathFolders: func() []string { + return []string{api.MailInbox} + }, + secondaryPathFolders: func(location string) []string { + return []string{location} + }, + }, + { + service: path.ExchangeService, + category: path.EventsCategory, + selector: func(t *testing.T) selectors.Selector { + sel := selectors.NewExchangeRestore([]string{tconfig.M365UserID(t)}) + sel.Include(sel.Events(selectors.Any(), selectors.Any())) + + return sel.Selector + }, + defaultPathFolders: func() []string { + return []string{api.DefaultCalendar} + }, + secondaryPathFolders: func(location string) []string { + return []string{location} + }, + }, + { + service: path.ExchangeService, + category: path.ContactsCategory, + selector: func(t *testing.T) selectors.Selector { + sel := selectors.NewExchangeRestore([]string{tconfig.M365UserID(t)}) + sel.Include(sel.Contacts(selectors.Any(), selectors.Any())) + + return sel.Selector + }, + defaultPathFolders: func() []string { + return []string{api.DefaultContacts} + }, + secondaryPathFolders: func(location string) []string { + return []string{location} + }, + }, + } + + for _, test := range table { + suite.Run(test.service.HumanString()+test.category.HumanString(), func() { + t := suite.T() + + ctx, flush := tester.NewContext(t) + defer flush() + + controller, err := NewController( + ctx, + acct, + test.service, + control.DefaultOptions(), + count.New()) + require.NoError(t, err, clues.ToCore(err)) + + handler, err := controller.NewServiceHandler(test.service) + require.NoError(t, err, clues.ToCore(err)) + + restoreConfig := controlTD.DefaultRestoreConfig("restore_in_place") + restoreConfig.OnCollision = control.Copy + + // Create 2 empty collections so we don't bloat the data set. + path1, err := path.Build( + tconfig.M365TenantID(t), + tconfig.M365UserID(t), + test.service, + test.category, + false, + test.defaultPathFolders()...) + require.NoError(t, err, clues.ToCore(err)) + + path2, err := path.Build( + tconfig.M365TenantID(t), + tconfig.M365UserID(t), + test.service, + test.category, + false, + test.secondaryPathFolders(restoreConfig.Location)...) + require.NoError(t, err, clues.ToCore(err)) + + cols := []data.RestoreCollection{ + data.NoFetchRestoreCollection{ + Collection: exchMock.NewCollection( + path1, + path1, + 0), + }, + data.NoFetchRestoreCollection{ + Collection: exchMock.NewCollection( + path2, + path2, + 0), + }, + } + + restoreConfig.Location = "" + + sel := test.selector(t) + + _, _, err = handler.ConsumeRestoreCollections( + ctx, + inject.RestoreConsumerConfig{ + BackupVersion: version.Backup, + Options: control.DefaultOptions(), + ProtectedResource: sel, + RestoreConfig: restoreConfig, + Selector: sel, + }, + cols, + fault.New(true), + count.New()) + assert.NoError(t, err, clues.ToCore(err)) + }) + } +} diff --git a/src/internal/m365/service/exchange/restore.go b/src/internal/m365/service/exchange/restore.go index b42619b20..3a72aebea 100644 --- a/src/internal/m365/service/exchange/restore.go +++ b/src/internal/m365/service/exchange/restore.go @@ -12,6 +12,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/count" "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/graph" ) @@ -77,19 +78,43 @@ func (h *exchangeHandler) ConsumeRestoreCollections( directoryCache[category] = gcr } - containerID, gcc, err := exchange.CreateDestination( - ictx, - handler, - handler.FormatRestoreDestination(rcc.RestoreConfig.Location, dc.FullPath()), - resourceID, - directoryCache[category], - errs) - if err != nil { - el.AddRecoverable(ictx, err) - continue + restoreFolderPath := handler.FormatRestoreDestination(rcc.RestoreConfig.Location, dc.FullPath()) + + ictx = clues.Add(ictx, "restore_folder_path", restoreFolderPath) + + var containerID string + + // Only attempt to create a new folder if it's not the default contacts + // folder. Contacts is weird in that it allows creating sub folders with the + // same name as the default contacts folder, but which are technically + // nested folders. + // + // Without this check we'll end up restoring to Contacts/Contacts instead of + // Contacts if in-place restore is requested and the root Contacts folder + // had items. + if handler.ShouldSetContainerToDefaultRoot(restoreFolderPath.String(), dc.FullPath()) { + logger.Ctx(ictx).Info("using default contact folder") + + containerID = handler.DefaultRootContainer() + } else { + logger.Ctx(ictx).Info("creating restore folder") + + newContainerID, gcc, err := exchange.CreateDestination( + ictx, + handler, + restoreFolderPath, + resourceID, + directoryCache[category], + errs) + if err != nil { + el.AddRecoverable(ictx, err) + continue + } + + directoryCache[category] = gcc + containerID = newContainerID } - directoryCache[category] = gcc ictx = clues.Add(ictx, "restore_destination_id", containerID) collisionKeyToItemID, err := handler.GetItemsInContainerByCollisionKey(ictx, resourceID, containerID) diff --git a/website/docs/support/known-issues.md b/website/docs/support/known-issues.md index 52ac0ccd3..76ad7dc50 100644 --- a/website/docs/support/known-issues.md +++ b/website/docs/support/known-issues.md @@ -37,3 +37,9 @@ Below is a list of known Corso issues and limitations: * Restoring the data into a different Group from the one it was backed up from isn't currently supported. * Backing up a group mailbox item may fail if it has a large number of attachments (500+). + +* Exchange in-place restore may restore items in well-known folders to different + folders if the user has well-known folder names change based on locale and has + updated the locale since the backup was created. + +* In-place Exchange contacts restore will merge items in folders named "Contacts" or "contacts" into the default folder.