From 91e0a2783104b7f9ed49d2a5b88fb8d790432a13 Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Tue, 28 Mar 2023 18:19:59 -0700 Subject: [PATCH] Populate location in Exchange container resolver (#2828) Always generate both a path of IDs and a path of display names for folders in the container resolver. Changes in internal/connector/exchange/service_functions.go keep the behavior of the overall system from changing --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [x] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [x] :broom: Tech Debt/Cleanup #### Issue(s) * #2486 merge after: * #2808 #### Test Plan - [x] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- .../exchange/contact_folder_cache.go | 5 +- .../connector/exchange/container_resolver.go | 61 +++++++-------- .../exchange/container_resolver_test.go | 75 +++++++------------ .../exchange/event_calendar_cache.go | 27 ++----- .../exchange/folder_resolver_test.go | 13 ++-- .../connector/exchange/mail_folder_cache.go | 4 +- .../exchange/mail_folder_cache_test.go | 4 +- .../connector/exchange/service_functions.go | 16 ++-- .../exchange/service_iterators_test.go | 27 +++++-- .../connector/exchange/service_restore.go | 17 ++--- .../connector/graph/cache_container.go | 14 ++-- .../operations/backup_integration_test.go | 4 +- 12 files changed, 124 insertions(+), 143 deletions(-) diff --git a/src/internal/connector/exchange/contact_folder_cache.go b/src/internal/connector/exchange/contact_folder_cache.go index 2e7b313c8..79cee76b8 100644 --- a/src/internal/connector/exchange/contact_folder_cache.go +++ b/src/internal/connector/exchange/contact_folder_cache.go @@ -5,6 +5,7 @@ import ( "github.com/alcionai/clues" + "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" @@ -31,7 +32,7 @@ func (cfc *contactFolderCache) populateContactRoot( temp := graph.NewCacheFolder( f, - path.Builder{}.Append(baseContainerPath...), // storage path + path.Builder{}.Append(ptr.Val(f.GetId())), // path of IDs path.Builder{}.Append(baseContainerPath...)) // display location if err := cfc.addFolder(temp); err != nil { return clues.Wrap(err, "adding resolver dir").WithClues(ctx) @@ -59,7 +60,7 @@ func (cfc *contactFolderCache) Populate( return clues.Wrap(err, "enumerating containers") } - if err := cfc.populatePaths(ctx, false, errs); err != nil { + if err := cfc.populatePaths(ctx, errs); err != nil { return clues.Wrap(err, "populating paths") } diff --git a/src/internal/connector/exchange/container_resolver.go b/src/internal/connector/exchange/container_resolver.go index db93d213a..8ec4f02ff 100644 --- a/src/internal/connector/exchange/container_resolver.go +++ b/src/internal/connector/exchange/container_resolver.go @@ -53,16 +53,14 @@ type containerResolver struct { func (cr *containerResolver) IDToPath( ctx context.Context, folderID string, - useIDInPath bool, ) (*path.Builder, *path.Builder, error) { - return cr.idToPath(ctx, folderID, 0, useIDInPath) + return cr.idToPath(ctx, folderID, 0) } func (cr *containerResolver) idToPath( ctx context.Context, folderID string, depth int, - useIDInPath bool, ) (*path.Builder, *path.Builder, error) { ctx = clues.Add(ctx, "container_id", folderID) @@ -83,33 +81,23 @@ func (cr *containerResolver) idToPath( parentPath, parentLoc, err := cr.idToPath( ctx, ptr.Val(c.GetParentFolderId()), - depth+1, - useIDInPath) + depth+1) if err != nil { return nil, nil, clues.Wrap(err, "retrieving parent folder") } - toAppend := ptr.Val(c.GetDisplayName()) - if useIDInPath { - toAppend = ptr.Val(c.GetId()) - } - - fullPath := parentPath.Append(toAppend) + fullPath := parentPath.Append(ptr.Val(c.GetId())) c.SetPath(fullPath) - var locPath *path.Builder - - if parentLoc != nil { - locPath = parentLoc.Append(ptr.Val(c.GetDisplayName())) - c.SetLocation(locPath) - } + locPath := parentLoc.Append(ptr.Val(c.GetDisplayName())) + c.SetLocation(locPath) return fullPath, locPath, nil } -// PathInCache utility function to return m365ID of folder if the path.Folders -// matches the directory of a container within the cache. A boolean result -// is provided to indicate whether the lookup was successful. +// PathInCache is a utility function to return m365ID of a folder if the +// path.Folders matches the directory of a container within the cache. A boolean +// result is provided to indicate whether the lookup was successful. func (cr *containerResolver) PathInCache(pathString string) (string, bool) { if len(pathString) == 0 || cr == nil { return "", false @@ -128,6 +116,27 @@ func (cr *containerResolver) PathInCache(pathString string) (string, bool) { return "", false } +// LocationInCache is a utility function to return m365ID of a folder if the +// path.Folders matches the directory of a container within the cache. A boolean +// result is provided to indicate whether the lookup was successful. +func (cr *containerResolver) LocationInCache(pathString string) (string, bool) { + if len(pathString) == 0 || cr == nil { + return "", false + } + + for _, cc := range cr.cache { + if cc.Location() == nil { + continue + } + + if cc.Location().String() == pathString { + return ptr.Val(cc.GetId()), true + } + } + + return "", false +} + // addFolder adds a folder to the cache with the given ID. If the item is // already in the cache does nothing. The path for the item is not modified. func (cr *containerResolver) addFolder(cf graph.CacheFolder) error { @@ -166,7 +175,6 @@ func (cr *containerResolver) Items() []graph.CachedContainer { func (cr *containerResolver) AddToCache( ctx context.Context, f graph.Container, - useIDInPath bool, ) error { temp := graph.CacheFolder{ Container: f, @@ -177,7 +185,7 @@ func (cr *containerResolver) AddToCache( // Populate the path for this entry so calls to PathInCache succeed no matter // when they're made. - _, _, err := cr.IDToPath(ctx, ptr.Val(f.GetId()), useIDInPath) + _, _, err := cr.IDToPath(ctx, ptr.Val(f.GetId())) if err != nil { return clues.Wrap(err, "adding cache entry") } @@ -185,15 +193,8 @@ func (cr *containerResolver) AddToCache( return nil } -// DestinationNameToID returns an empty string. This is only supported by exchange -// calendars at this time. -func (cr *containerResolver) DestinationNameToID(dest string) string { - return "" -} - func (cr *containerResolver) populatePaths( ctx context.Context, - useIDInPath bool, errs *fault.Bus, ) error { var ( @@ -207,7 +208,7 @@ func (cr *containerResolver) populatePaths( return el.Failure() } - _, _, err := cr.IDToPath(ctx, ptr.Val(f.GetId()), useIDInPath) + _, _, err := cr.IDToPath(ctx, ptr.Val(f.GetId())) if err != nil { err = clues.Wrap(err, "populating path") el.AddRecoverable(err) diff --git a/src/internal/connector/exchange/container_resolver_test.go b/src/internal/connector/exchange/container_resolver_test.go index 389182ebb..572162263 100644 --- a/src/internal/connector/exchange/container_resolver_test.go +++ b/src/internal/connector/exchange/container_resolver_test.go @@ -33,10 +33,10 @@ type mockContainer struct { l *path.Builder } -//nolint:revive +//revive:disable-next-line:var-naming func (m mockContainer) GetId() *string { return m.id } -//nolint:revive +//revive:disable-next-line:var-naming func (m mockContainer) GetParentFolderId() *string { return m.parentID } func (m mockContainer) GetDisplayName() *string { return m.displayName } func (m mockContainer) Location() *path.Builder { return m.l } @@ -277,11 +277,7 @@ func resolverWithContainers(numContainers int, useIDInPath bool) (*containerReso // Base case for the recursive lookup. dn := containers[0].displayName - - apndP := dn - if useIDInPath { - apndP = containers[0].id - } + apndP := containers[0].id containers[0].p = path.Builder{}.Append(apndP) containers[0].expectedPath = apndP @@ -290,11 +286,7 @@ func resolverWithContainers(numContainers int, useIDInPath bool) (*containerReso for i := 1; i < len(containers); i++ { dn := containers[i].displayName - - apndP := dn - if useIDInPath { - apndP = containers[i].id - } + apndP := containers[i].id containers[i].parentID = containers[i-1].id containers[i].expectedPath = stdpath.Join(containers[i-1].expectedPath, apndP) @@ -358,7 +350,7 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestDepthLimit() { for _, test := range table { suite.Run(test.name, func() { resolver, containers := resolverWithContainers(test.numContainers, false) - _, _, err := resolver.IDToPath(ctx, containers[len(containers)-1].id, false) + _, _, err := resolver.IDToPath(ctx, containers[len(containers)-1].id) test.check(suite.T(), err, clues.ToCore(err)) }) } @@ -370,7 +362,7 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestPopulatePaths() { t := suite.T() - err := suite.fc.populatePaths(ctx, false, fault.New(true)) + err := suite.fc.populatePaths(ctx, fault.New(true)) require.NoError(t, err, clues.ToCore(err)) items := suite.fc.Items() @@ -396,7 +388,7 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderNoPathsCached suite.Run(ptr.Val(c.GetDisplayName()), func() { t := suite.T() - p, l, err := suite.fc.IDToPath(ctx, c.id, false) + p, l, err := suite.fc.IDToPath(ctx, c.id) require.NoError(t, err, clues.ToCore(err)) assert.Equal(t, c.expectedPath, p.String()) assert.Equal(t, c.expectedLocation, l.String()) @@ -412,7 +404,7 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderNoPathsCached suite.Run(ptr.Val(c.GetDisplayName()), func() { t := suite.T() - p, l, err := suite.fcWithID.IDToPath(ctx, c.id, true) + p, l, err := suite.fcWithID.IDToPath(ctx, c.id) require.NoError(t, err, clues.ToCore(err)) assert.Equal(t, c.expectedPath, p.String()) assert.Equal(t, c.expectedLocation, l.String()) @@ -427,14 +419,14 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderCachesPaths() t := suite.T() c := suite.allContainers[len(suite.allContainers)-1] - p, l, err := suite.fc.IDToPath(ctx, c.id, false) + p, l, err := suite.fc.IDToPath(ctx, c.id) require.NoError(t, err, clues.ToCore(err)) assert.Equal(t, c.expectedPath, p.String()) assert.Equal(t, c.expectedLocation, l.String()) c.parentID = "foo" - p, l, err = suite.fc.IDToPath(ctx, c.id, false) + p, l, err = suite.fc.IDToPath(ctx, c.id) require.NoError(t, err, clues.ToCore(err)) assert.Equal(t, c.expectedPath, p.String()) assert.Equal(t, c.expectedLocation, l.String()) @@ -447,14 +439,14 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderCachesPaths_u t := suite.T() c := suite.containersWithID[len(suite.containersWithID)-1] - p, l, err := suite.fcWithID.IDToPath(ctx, c.id, true) + p, l, err := suite.fcWithID.IDToPath(ctx, c.id) require.NoError(t, err, clues.ToCore(err)) assert.Equal(t, c.expectedPath, p.String()) assert.Equal(t, c.expectedLocation, l.String()) c.parentID = "foo" - p, l, err = suite.fcWithID.IDToPath(ctx, c.id, true) + p, l, err = suite.fcWithID.IDToPath(ctx, c.id) require.NoError(t, err, clues.ToCore(err)) assert.Equal(t, c.expectedPath, p.String()) assert.Equal(t, c.expectedLocation, l.String()) @@ -470,7 +462,7 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderErrorsParentN delete(suite.fc.cache, almostLast.id) - _, _, err := suite.fc.IDToPath(ctx, last.id, false) + _, _, err := suite.fc.IDToPath(ctx, last.id) assert.Error(t, err, clues.ToCore(err)) } @@ -480,7 +472,7 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderErrorsNotFoun t := suite.T() - _, _, err := suite.fc.IDToPath(ctx, "foo", false) + _, _, err := suite.fc.IDToPath(ctx, "foo") assert.Error(t, err, clues.ToCore(err)) } @@ -496,20 +488,16 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestAddToCache() { ) m.parentID = last.id - m.expectedPath = stdpath.Join(last.expectedPath, m.displayName) - m.expectedLocation = stdpath.Join(last.expectedPath, m.displayName) + m.expectedPath = stdpath.Join(last.expectedPath, m.id) + m.expectedLocation = stdpath.Join(last.expectedLocation, m.displayName) - require.Empty(t, suite.fc.DestinationNameToID(dest), "destination not yet added to cache") - - err := suite.fc.AddToCache(ctx, m, false) + err := suite.fc.AddToCache(ctx, m) require.NoError(t, err, clues.ToCore(err)) - require.Empty(t, suite.fc.DestinationNameToID(dest), - "destination id from cache, still empty, because this is not a calendar") - p, l, err := suite.fc.IDToPath(ctx, m.id, false) + p, l, err := suite.fc.IDToPath(ctx, m.id) require.NoError(t, err, clues.ToCore(err)) - assert.Equal(t, m.expectedPath, p.String()) - assert.Equal(t, m.expectedLocation, l.String()) + assert.Equal(t, m.expectedPath, p.String(), "ID path") + assert.Equal(t, m.expectedLocation, l.String(), "location path") } // --------------------------------------------------------------------------- @@ -568,7 +556,6 @@ func (suite *FolderCacheIntegrationSuite) TestCreateContainerDestination() { pathFunc2 func(t *testing.T) path.Path category path.CategoryType folderPrefix string - useIDForPath bool }{ { name: "Mail Cache Test", @@ -627,9 +614,8 @@ func (suite *FolderCacheIntegrationSuite) TestCreateContainerDestination() { }, }, { - name: "Event Cache Test", - category: path.EventsCategory, - useIDForPath: true, + name: "Event Cache Test", + category: path.EventsCategory, pathFunc1: func(t *testing.T) path.Path { pth, err := path.Build( suite.credentials.AzureTenantID, @@ -673,28 +659,25 @@ func (suite *FolderCacheIntegrationSuite) TestCreateContainerDestination() { resolver := directoryCaches[test.category] - _, _, err = resolver.IDToPath(ctx, folderID, test.useIDForPath) + _, _, err = resolver.IDToPath(ctx, folderID) assert.NoError(t, err, clues.ToCore(err)) - parentContainer := folderName - if test.useIDForPath { - parentContainer = folderID - } - secondID, err := CreateContainerDestination( ctx, m365, test.pathFunc2(t), - parentContainer, + folderName, directoryCaches, fault.New(true)) require.NoError(t, err, clues.ToCore(err)) - _, _, err = resolver.IDToPath(ctx, secondID, test.useIDForPath) + p, l, err := resolver.IDToPath(ctx, secondID) require.NoError(t, err, clues.ToCore(err)) - p := stdpath.Join(test.folderPrefix, parentContainer) - _, ok := resolver.PathInCache(p) + _, ok := resolver.LocationInCache(l.String()) + require.True(t, ok, "looking for location in cache: %s", l) + + _, ok = resolver.PathInCache(p.String()) require.True(t, ok, "looking for path in cache: %s", p) }) } diff --git a/src/internal/connector/exchange/event_calendar_cache.go b/src/internal/connector/exchange/event_calendar_cache.go index 69fc5f2f2..ac8e59548 100644 --- a/src/internal/connector/exchange/event_calendar_cache.go +++ b/src/internal/connector/exchange/event_calendar_cache.go @@ -15,10 +15,9 @@ var _ graph.ContainerResolver = &eventCalendarCache{} type eventCalendarCache struct { *containerResolver - enumer containersEnumerator - getter containerGetter - userID string - newAdditions map[string]string + enumer containersEnumerator + getter containerGetter + userID string } // init ensures that the structure's fields are initialized. @@ -80,7 +79,7 @@ func (ecc *eventCalendarCache) Populate( return clues.Wrap(err, "enumerating containers") } - if err := ecc.populatePaths(ctx, true, errs); err != nil { + if err := ecc.populatePaths(ctx, errs); err != nil { return clues.Wrap(err, "establishing calendar paths") } @@ -89,7 +88,7 @@ func (ecc *eventCalendarCache) Populate( // AddToCache adds container to map in field 'cache' // @returns error iff the required values are not accessible. -func (ecc *eventCalendarCache) AddToCache(ctx context.Context, f graph.Container, useIDInPath bool) error { +func (ecc *eventCalendarCache) AddToCache(ctx context.Context, f graph.Container) error { if err := checkIDAndName(f); err != nil { return clues.Wrap(err, "validating container").WithClues(ctx) } @@ -99,30 +98,16 @@ func (ecc *eventCalendarCache) AddToCache(ctx context.Context, f graph.Container path.Builder{}.Append(ptr.Val(f.GetId())), // storage path path.Builder{}.Append(ptr.Val(f.GetDisplayName()))) // display location - if len(ecc.newAdditions) == 0 { - ecc.newAdditions = map[string]string{} - } - - ecc.newAdditions[ptr.Val(f.GetDisplayName())] = ptr.Val(f.GetId()) - if err := ecc.addFolder(temp); err != nil { - delete(ecc.newAdditions, ptr.Val(f.GetDisplayName())) return clues.Wrap(err, "adding container").WithClues(ctx) } // Populate the path for this entry so calls to PathInCache succeed no matter // when they're made. - _, _, err := ecc.IDToPath(ctx, ptr.Val(f.GetId()), true) + _, _, err := ecc.IDToPath(ctx, ptr.Val(f.GetId())) if err != nil { - delete(ecc.newAdditions, ptr.Val(f.GetDisplayName())) return clues.Wrap(err, "setting path to container id") } return nil } - -// DestinationNameToID returns an empty string. This is only supported by exchange -// calendars at this time. -func (ecc *eventCalendarCache) DestinationNameToID(dest string) string { - return ecc.newAdditions[dest] -} diff --git a/src/internal/connector/exchange/folder_resolver_test.go b/src/internal/connector/exchange/folder_resolver_test.go index 01a852bf2..76c32a4fd 100644 --- a/src/internal/connector/exchange/folder_resolver_test.go +++ b/src/internal/connector/exchange/folder_resolver_test.go @@ -8,7 +8,6 @@ import ( "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" - "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/exchange/api" "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/tester" @@ -47,9 +46,6 @@ func (suite *CacheResolverSuite) TestPopulate() { ac, err := api.NewClient(suite.credentials) require.NoError(suite.T(), err, clues.ToCore(err)) - cal, err := ac.Events().GetContainerByID(ctx, tester.M365UserID(suite.T()), DefaultCalendar) - require.NoError(suite.T(), err, clues.ToCore(err)) - eventFunc := func(t *testing.T) graph.ContainerResolver { return &eventCalendarCache{ userID: tester.M365UserID(t), @@ -72,8 +68,9 @@ func (suite *CacheResolverSuite) TestPopulate() { canFind assert.BoolAssertionFunc }{ { - name: "Default Event Cache", - folderInCache: ptr.Val(cal.GetId()), + name: "Default Event Cache", + // Fine as long as this isn't running against a migrated Exchange server. + folderInCache: DefaultCalendar, root: DefaultCalendar, basePath: DefaultCalendar, resolverFunc: eventFunc, @@ -124,8 +121,8 @@ func (suite *CacheResolverSuite) TestPopulate() { err := resolver.Populate(ctx, fault.New(true), test.root, test.basePath) require.NoError(t, err, clues.ToCore(err)) - _, isFound := resolver.PathInCache(test.folderInCache) - test.canFind(t, isFound) + _, isFound := resolver.LocationInCache(test.folderInCache) + test.canFind(t, isFound, "folder path", test.folderInCache) }) } } diff --git a/src/internal/connector/exchange/mail_folder_cache.go b/src/internal/connector/exchange/mail_folder_cache.go index cb014482f..c2630a29a 100644 --- a/src/internal/connector/exchange/mail_folder_cache.go +++ b/src/internal/connector/exchange/mail_folder_cache.go @@ -50,7 +50,7 @@ func (mc *mailFolderCache) populateMailRoot(ctx context.Context) error { // Root folder doesn't store any mail messages so it isn't given any paths. // Giving it a path/location would cause us to add extra path elements that // the user doesn't see in the regular UI for Exchange. - path.Builder{}.Append(), // storage path + path.Builder{}.Append(), // path of IDs path.Builder{}.Append()) // display location if err := mc.addFolder(temp); err != nil { return clues.Wrap(err, "adding resolver dir").WithClues(ctx) @@ -79,7 +79,7 @@ func (mc *mailFolderCache) Populate( return clues.Wrap(err, "enumerating containers") } - if err := mc.populatePaths(ctx, false, errs); err != nil { + if err := mc.populatePaths(ctx, errs); err != nil { return clues.Wrap(err, "populating paths") } diff --git a/src/internal/connector/exchange/mail_folder_cache_test.go b/src/internal/connector/exchange/mail_folder_cache_test.go index 42dc407d1..0ae08ce88 100644 --- a/src/internal/connector/exchange/mail_folder_cache_test.go +++ b/src/internal/connector/exchange/mail_folder_cache_test.go @@ -96,14 +96,14 @@ func (suite *MailFolderCacheIntegrationSuite) TestDeltaFetch() { err = mfc.Populate(ctx, fault.New(true), test.root, test.path...) require.NoError(t, err, clues.ToCore(err)) - p, l, err := mfc.IDToPath(ctx, testFolderID, true) + p, l, err := mfc.IDToPath(ctx, testFolderID) require.NoError(t, err, clues.ToCore(err)) t.Logf("Path: %s\n", p.String()) t.Logf("Location: %s\n", l.String()) expectedPath := stdpath.Join(append(test.path, expectedFolderPath)...) assert.Equal(t, expectedPath, p.String()) - identifier, ok := mfc.PathInCache(p.String()) + identifier, ok := mfc.LocationInCache(p.String()) assert.True(t, ok) assert.NotEmpty(t, identifier) }) diff --git a/src/internal/connector/exchange/service_functions.go b/src/internal/connector/exchange/service_functions.go index d19b03f52..0827fbb05 100644 --- a/src/internal/connector/exchange/service_functions.go +++ b/src/internal/connector/exchange/service_functions.go @@ -106,11 +106,7 @@ func includeContainer( // Clause ensures that DefaultContactFolder is inspected properly if category == path.ContactsCategory && ptr.Val(c.GetDisplayName()) == DefaultContactFolder { - pb = pb.Append(DefaultContactFolder) - - if loc != nil { - loc = loc.Append(DefaultContactFolder) - } + loc = loc.Append(DefaultContactFolder) } dirPath, err := pb.ToDataLayerExchangePathForCategory( @@ -139,18 +135,24 @@ func includeContainer( directory = locPath.Folder(false) } - var ok bool + var ( + ok bool + pathRes path.Path + ) switch category { case path.EmailCategory: ok = scope.Matches(selectors.ExchangeMailFolder, directory) + pathRes = locPath case path.ContactsCategory: ok = scope.Matches(selectors.ExchangeContactFolder, directory) + pathRes = locPath case path.EventsCategory: ok = scope.Matches(selectors.ExchangeEventCalendar, directory) + pathRes = dirPath default: return nil, nil, false } - return dirPath, locPath, ok + return pathRes, locPath, ok } diff --git a/src/internal/connector/exchange/service_iterators_test.go b/src/internal/connector/exchange/service_iterators_test.go index 580598bf6..f14601b8c 100644 --- a/src/internal/connector/exchange/service_iterators_test.go +++ b/src/internal/connector/exchange/service_iterators_test.go @@ -78,7 +78,7 @@ func (m mockResolver) Items() []graph.CachedContainer { return m.items } -func (m mockResolver) AddToCache(ctx context.Context, gc graph.Container, b bool) error { +func (m mockResolver) AddToCache(ctx context.Context, gc graph.Container) error { if len(m.added) == 0 { m.added = map[string]string{} } @@ -88,10 +88,11 @@ func (m mockResolver) AddToCache(ctx context.Context, gc graph.Container, b bool return nil } func (m mockResolver) DestinationNameToID(dest string) string { return m.added[dest] } -func (m mockResolver) IDToPath(context.Context, string, bool) (*path.Builder, *path.Builder, error) { +func (m mockResolver) IDToPath(context.Context, string) (*path.Builder, *path.Builder, error) { return nil, nil, nil } func (m mockResolver) PathInCache(string) (string, bool) { return "", false } +func (m mockResolver) LocationInCache(string) (string, bool) { return "", false } func (m mockResolver) Populate(context.Context, *fault.Bus, string, ...string) error { return nil } // --------------------------------------------------------------------------- @@ -145,12 +146,14 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections() { container1 = mockContainer{ id: strPtr("1"), displayName: strPtr("display_name_1"), - p: path.Builder{}.Append("display_name_1"), + p: path.Builder{}.Append("1"), + l: path.Builder{}.Append("display_name_1"), } container2 = mockContainer{ id: strPtr("2"), displayName: strPtr("display_name_2"), - p: path.Builder{}.Append("display_name_2"), + p: path.Builder{}.Append("2"), + l: path.Builder{}.Append("display_name_2"), } ) @@ -445,7 +448,8 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_repea container1 = mockContainer{ id: strPtr("1"), displayName: strPtr("display_name_1"), - p: path.Builder{}.Append("display_name_1"), + p: path.Builder{}.Append("1"), + l: path.Builder{}.Append("display_name_1"), } resolver = newMockResolver(container1) ) @@ -562,6 +566,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre id: strPtr("1"), displayName: strPtr("new"), p: path.Builder{}.Append("1", "new"), + l: path.Builder{}.Append("1", "new"), }), dps: DeltaPaths{}, expect: map[string]endState{ @@ -577,6 +582,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre id: strPtr("1"), displayName: strPtr("not_moved"), p: path.Builder{}.Append("1", "not_moved"), + l: path.Builder{}.Append("1", "not_moved"), }), dps: DeltaPaths{ "1": DeltaPath{ @@ -597,6 +603,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre id: strPtr("1"), displayName: strPtr("moved"), p: path.Builder{}.Append("1", "moved"), + l: path.Builder{}.Append("1", "moved"), }), dps: DeltaPaths{ "1": DeltaPath{ @@ -631,6 +638,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre id: strPtr("2"), displayName: strPtr("new"), p: path.Builder{}.Append("2", "new"), + l: path.Builder{}.Append("2", "new"), }), dps: DeltaPaths{ "1": DeltaPath{ @@ -652,6 +660,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre id: strPtr("2"), displayName: strPtr("same"), p: path.Builder{}.Append("2", "same"), + l: path.Builder{}.Append("2", "same"), }), dps: DeltaPaths{ "1": DeltaPath{ @@ -675,11 +684,13 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre id: strPtr("1"), displayName: strPtr("moved"), p: path.Builder{}.Append("1", "moved"), + l: path.Builder{}.Append("1", "moved"), }, mockContainer{ id: strPtr("2"), displayName: strPtr("prev"), p: path.Builder{}.Append("2", "prev"), + l: path.Builder{}.Append("2", "prev"), }, ), dps: DeltaPaths{ @@ -702,6 +713,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre id: strPtr("1"), displayName: strPtr("not_moved"), p: path.Builder{}.Append("1", "not_moved"), + l: path.Builder{}.Append("1", "not_moved"), }), dps: DeltaPaths{ "1": DeltaPath{ @@ -726,6 +738,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre id: strPtr("1"), displayName: strPtr("same"), p: path.Builder{}.Append("1", "same"), + l: path.Builder{}.Append("1", "same"), }), dps: DeltaPaths{ "1": DeltaPath{ @@ -751,21 +764,25 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre id: strPtr("1"), displayName: strPtr("new"), p: path.Builder{}.Append("1", "new"), + l: path.Builder{}.Append("1", "new"), }, mockContainer{ id: strPtr("2"), displayName: strPtr("not_moved"), p: path.Builder{}.Append("2", "not_moved"), + l: path.Builder{}.Append("2", "not_moved"), }, mockContainer{ id: strPtr("3"), displayName: strPtr("moved"), p: path.Builder{}.Append("3", "moved"), + l: path.Builder{}.Append("3", "moved"), }, mockContainer{ id: strPtr("4"), displayName: strPtr("moved"), p: path.Builder{}.Append("4", "moved"), + l: path.Builder{}.Append("4", "moved"), }, ), dps: DeltaPaths{ diff --git a/src/internal/connector/exchange/service_restore.go b/src/internal/connector/exchange/service_restore.go index f8f7cdb29..d323bbbfd 100644 --- a/src/internal/connector/exchange/service_restore.go +++ b/src/internal/connector/exchange/service_restore.go @@ -555,11 +555,6 @@ func CreateContainerDestination( caches[category] = ecc newCache = true directoryCache = ecc - } else if did := directoryCache.DestinationNameToID(dest); len(did) > 0 { - // calendars are cached by ID in the resolver, not name, so once we have - // created the destination calendar, we need to look up its id and use - // that for resolver lookups instead of the display name. - dest = did } folders := append([]string{dest}, directory.Folders()...) @@ -602,7 +597,7 @@ func establishMailRestoreLocation( for _, folder := range folders { pb = *pb.Append(folder) - cached, ok := mfc.PathInCache(pb.String()) + cached, ok := mfc.LocationInCache(pb.String()) if ok { folderID = cached continue @@ -628,7 +623,7 @@ func establishMailRestoreLocation( } // NOOP if the folder is already in the cache. - if err = mfc.AddToCache(ctx, temp, false); err != nil { + if err = mfc.AddToCache(ctx, temp); err != nil { return "", clues.Wrap(err, "adding folder to cache") } } @@ -651,7 +646,7 @@ func establishContactsRestoreLocation( isNewCache bool, errs *fault.Bus, ) (string, error) { - cached, ok := cfc.PathInCache(folders[0]) + cached, ok := cfc.LocationInCache(folders[0]) if ok { return cached, nil } @@ -670,7 +665,7 @@ func establishContactsRestoreLocation( return "", clues.Wrap(err, "populating contact cache") } - if err = cfc.AddToCache(ctx, temp, false); err != nil { + if err = cfc.AddToCache(ctx, temp); err != nil { return "", clues.Wrap(err, "adding contact folder to cache") } } @@ -688,7 +683,7 @@ func establishEventsRestoreLocation( errs *fault.Bus, ) (string, error) { // Need to prefix with the "Other Calendars" folder so lookup happens properly. - cached, ok := ecc.PathInCache(folders[0]) + cached, ok := ecc.LocationInCache(folders[0]) if ok { return cached, nil } @@ -708,7 +703,7 @@ func establishEventsRestoreLocation( } displayable := api.CalendarDisplayable{Calendarable: temp} - if err = ecc.AddToCache(ctx, displayable, true); err != nil { + if err = ecc.AddToCache(ctx, displayable); err != nil { return "", clues.Wrap(err, "adding new calendar to cache") } } diff --git a/src/internal/connector/graph/cache_container.go b/src/internal/connector/graph/cache_container.go index 9675612cc..fa5a979bb 100644 --- a/src/internal/connector/graph/cache_container.go +++ b/src/internal/connector/graph/cache_container.go @@ -58,7 +58,7 @@ type ContainerResolver interface { // IDToPath takes an m365 container ID and converts it to a hierarchical path // to that container. The path has a similar format to paths on the local // file system. - IDToPath(ctx context.Context, m365ID string, useIDInPath bool) (*path.Builder, *path.Builder, error) + IDToPath(ctx context.Context, m365ID string) (*path.Builder, *path.Builder, error) // Populate performs initialization steps for the resolver // @param ctx is necessary param for Graph API tracing @@ -71,13 +71,13 @@ type ContainerResolver interface { // matches the path of a container within the cache. // @returns bool represents if m365ID was found. PathInCache(pathString string) (string, bool) + // LocationInCache performs a look up of a path reprensentation + // and returns the m365ID of directory iff the pathString + // matches the logical path of a container within the cache. + // @returns bool represents if m365ID was found. + LocationInCache(pathString string) (string, bool) - AddToCache(ctx context.Context, m365Container Container, useIDInPath bool) error - - // DestinationNameToID returns the ID of the destination container. Dest is - // assumed to be a display name. The ID is only populated if the destination - // was added using `AddToCache()`. Returns an empty string if not found. - DestinationNameToID(dest string) string + AddToCache(ctx context.Context, m365Container Container) error // Items returns the containers in the cache. Items() []CachedContainer diff --git a/src/internal/operations/backup_integration_test.go b/src/internal/operations/backup_integration_test.go index dbaa272dc..d26f80233 100644 --- a/src/internal/operations/backup_integration_test.go +++ b/src/internal/operations/backup_integration_test.go @@ -784,7 +784,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { p, err := path.FromDataLayerPath(dest.deets.Entries[0].RepoRef, true) require.NoError(t, err, clues.ToCore(err)) - id, ok := cr.PathInCache(p.Folder(false)) + id, ok := cr.LocationInCache(p.Folder(false)) require.True(t, ok, "dir %s found in %s cache", p.Folder(false), category) d := dataset[category].dests[destName] @@ -897,7 +897,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { p, err := path.FromDataLayerPath(deets.Entries[0].RepoRef, true) require.NoError(t, err, clues.ToCore(err)) - id, ok := cr.PathInCache(p.Folder(false)) + id, ok := cr.LocationInCache(p.Folder(false)) require.Truef(t, ok, "dir %s found in %s cache", p.Folder(false), category) dataset[category].dests[container3] = contDeets{id, deets}