From 38f56cccbab6718a8eec82202fca8bf0adb4aa3c Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Fri, 3 Feb 2023 15:29:55 -0800 Subject: [PATCH] Alternative way to handle 2-level calendars hierarchy (#2397) ## Description All calendars except the default are nested under a "Other Calendars" folder. Having a non-default calendar named the same as the default calendar does not cause problems when fetching the default calendar by name. Only the default calendar will be returned in that situation. This fixes the bug where we had multiple collections for the same path but representing different folders. Also updates the restore execution path to handle the new nested folder structure. Backup, incremental backup, and restore flows tested manually ## Does this PR need a docs update or release note? - [x] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [ ] :no_entry: No ## Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup ## Issue(s) * #2388 ## Test Plan - [x] :muscle: Manual - [ ] :zap: Unit test - [ ] :green_heart: E2E --- CHANGELOG.md | 3 +++ .../connector/exchange/container_resolver_test.go | 15 +++++++++------ .../connector/exchange/data_collections_test.go | 4 ++-- .../connector/exchange/event_calendar_cache.go | 12 ++++++++++-- src/internal/connector/exchange/exchange_vars.go | 1 + .../connector/exchange/service_restore.go | 6 +++++- 6 files changed, 30 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53ec01a36..bf2cd7f0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `--restore-permissions` flag to toggle restoration of OneDrive permissions - Add versions to backups so that we can understand/handle older backup formats +### Fixed +- Backing up a calendar that has the same name as the default calendar + ### Known Issues - When the same user has permissions to a file and the containing diff --git a/src/internal/connector/exchange/container_resolver_test.go b/src/internal/connector/exchange/container_resolver_test.go index d7c6651f1..be0704f46 100644 --- a/src/internal/connector/exchange/container_resolver_test.go +++ b/src/internal/connector/exchange/container_resolver_test.go @@ -501,10 +501,11 @@ func (suite *FolderCacheIntegrationSuite) TestCreateContainerDestination() { directoryCaches = make(map[path.CategoryType]graph.ContainerResolver) folderName = tester.DefaultTestRestoreDestination().ContainerName tests = []struct { - name string - pathFunc1 func(t *testing.T) path.Path - pathFunc2 func(t *testing.T) path.Path - category path.CategoryType + name string + pathFunc1 func(t *testing.T) path.Path + pathFunc2 func(t *testing.T) path.Path + category path.CategoryType + folderPrefix string }{ { name: "Mail Cache Test", @@ -587,6 +588,7 @@ func (suite *FolderCacheIntegrationSuite) TestCreateContainerDestination() { require.NoError(t, err) return aPath }, + folderPrefix: calendarOthersFolder, }, } ) @@ -617,8 +619,9 @@ func (suite *FolderCacheIntegrationSuite) TestCreateContainerDestination() { _, err = resolver.IDToPath(ctx, secondID) require.NoError(t, err) - _, ok := resolver.PathInCache(folderName) - require.True(t, ok) + p := stdpath.Join(test.folderPrefix, folderName) + _, ok := resolver.PathInCache(p) + require.True(t, ok, "looking for path in cache: %s", p) }) } } diff --git a/src/internal/connector/exchange/data_collections_test.go b/src/internal/connector/exchange/data_collections_test.go index 70f413239..07eef5e7a 100644 --- a/src/internal/connector/exchange/data_collections_test.go +++ b/src/internal/connector/exchange/data_collections_test.go @@ -537,9 +537,9 @@ func (suite *DataCollectionsIntegrationSuite) TestEventsSerializationRegression( }, { name: "Birthday Calendar", - expected: "Birthdays", + expected: calendarOthersFolder + "/Birthdays", scope: selectors.NewExchangeBackup(users).EventCalendars( - []string{"Birthdays"}, + []string{calendarOthersFolder + "/Birthdays"}, selectors.PrefixMatch(), )[0], }, diff --git a/src/internal/connector/exchange/event_calendar_cache.go b/src/internal/connector/exchange/event_calendar_cache.go index e497a272a..0377433ee 100644 --- a/src/internal/connector/exchange/event_calendar_cache.go +++ b/src/internal/connector/exchange/event_calendar_cache.go @@ -64,7 +64,15 @@ func (ecc *eventCalendarCache) Populate( return errors.Wrap(err, "initializing") } - err := ecc.enumer.EnumerateContainers(ctx, ecc.userID, "", ecc.addFolder) + err := ecc.enumer.EnumerateContainers( + ctx, + ecc.userID, + "", + func(cf graph.CacheFolder) error { + cf.SetPath(path.Builder{}.Append(calendarOthersFolder, *cf.GetDisplayName())) + return ecc.addFolder(cf) + }, + ) if err != nil { return errors.Wrap(err, "enumerating containers") } @@ -83,7 +91,7 @@ func (ecc *eventCalendarCache) AddToCache(ctx context.Context, f graph.Container return errors.Wrap(err, "validating container") } - temp := graph.NewCacheFolder(f, path.Builder{}.Append(*f.GetDisplayName())) + temp := graph.NewCacheFolder(f, path.Builder{}.Append(calendarOthersFolder, *f.GetDisplayName())) if err := ecc.addFolder(temp); err != nil { return errors.Wrap(err, "adding container") diff --git a/src/internal/connector/exchange/exchange_vars.go b/src/internal/connector/exchange/exchange_vars.go index e45de0bf0..988d20330 100644 --- a/src/internal/connector/exchange/exchange_vars.go +++ b/src/internal/connector/exchange/exchange_vars.go @@ -38,4 +38,5 @@ const ( rootFolderAlias = "msgfolderroot" DefaultContactFolder = "Contacts" DefaultCalendar = "Calendar" + calendarOthersFolder = "Other Calendars" ) diff --git a/src/internal/connector/exchange/service_restore.go b/src/internal/connector/exchange/service_restore.go index 45e2ff1c4..e6fa592f7 100644 --- a/src/internal/connector/exchange/service_restore.go +++ b/src/internal/connector/exchange/service_restore.go @@ -645,7 +645,11 @@ func establishEventsRestoreLocation( user string, isNewCache bool, ) (string, error) { - cached, ok := ecc.PathInCache(folders[0]) + // Need to prefix with the "Other Calendars" folder so lookup happens properly. + cached, ok := ecc.PathInCache(path.Builder{}.Append( + calendarOthersFolder, + folders[0], + ).String()) if ok { return cached, nil }