From 9520de1a3741e7be38a757e4eba725504df184e4 Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Fri, 19 May 2023 17:05:52 -0700 Subject: [PATCH] More robust folder fetching for container cache (#3464) Update folder cache population * try to fetch missing parent folders * refresh current folder if parent wasn't cached * only populate paths during populatePaths not during IDToFolder Manually tested email folder resolver population with stable reproducer and succeeded --- #### 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: Supportability/Tests - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Issue(s) #### Test Plan - [x] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- CHANGELOG.md | 2 + .../exchange/contact_folder_cache.go | 31 ++- .../connector/exchange/container_resolver.go | 234 ++++++++++++++++-- .../exchange/container_resolver_test.go | 221 ++++++++++++++++- .../exchange/data_collections_test.go | 2 +- .../exchange/event_calendar_cache.go | 6 +- .../connector/exchange/mail_folder_cache.go | 31 ++- src/pkg/services/m365/api/contacts.go | 4 +- src/pkg/services/m365/api/events.go | 4 +- src/pkg/services/m365/api/mail.go | 4 +- 10 files changed, 496 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 475c98407..b36c38580 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added ### Fixed +- Fix Exchange folder cache population error when parent folder isn't found. + ### Known Issues ## [v0.8.0] (beta) - 2023-05-15 diff --git a/src/internal/connector/exchange/contact_folder_cache.go b/src/internal/connector/exchange/contact_folder_cache.go index 79cee76b8..5526bf7b7 100644 --- a/src/internal/connector/exchange/contact_folder_cache.go +++ b/src/internal/connector/exchange/contact_folder_cache.go @@ -11,7 +11,29 @@ import ( "github.com/alcionai/corso/src/pkg/path" ) -var _ graph.ContainerResolver = &contactFolderCache{} +var ( + _ graph.ContainerResolver = &contactFolderCache{} + _ containerRefresher = &contactRefresher{} +) + +type contactRefresher struct { + getter containerGetter + userID string +} + +func (r *contactRefresher) refreshContainer( + ctx context.Context, + id string, +) (graph.CachedContainer, error) { + c, err := r.getter.GetContainerByID(ctx, r.userID, id) + if err != nil { + return nil, clues.Stack(err) + } + + f := graph.NewCacheFolder(c, nil, nil) + + return &f, nil +} type contactFolderCache struct { *containerResolver @@ -34,7 +56,7 @@ func (cfc *contactFolderCache) populateContactRoot( f, path.Builder{}.Append(ptr.Val(f.GetId())), // path of IDs path.Builder{}.Append(baseContainerPath...)) // display location - if err := cfc.addFolder(temp); err != nil { + if err := cfc.addFolder(&temp); err != nil { return clues.Wrap(err, "adding resolver dir").WithClues(ctx) } @@ -77,7 +99,10 @@ func (cfc *contactFolderCache) init( } if cfc.containerResolver == nil { - cfc.containerResolver = newContainerResolver() + cfc.containerResolver = newContainerResolver(&contactRefresher{ + userID: cfc.userID, + getter: cfc.getter, + }) } return cfc.populateContactRoot(ctx, baseNode, baseContainerPath) diff --git a/src/internal/connector/exchange/container_resolver.go b/src/internal/connector/exchange/container_resolver.go index 8ec4f02ff..0e2730449 100644 --- a/src/internal/connector/exchange/container_resolver.go +++ b/src/internal/connector/exchange/container_resolver.go @@ -8,6 +8,7 @@ import ( "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/logger" "github.com/alcionai/corso/src/pkg/path" ) @@ -26,11 +27,18 @@ type containersEnumerator interface { EnumerateContainers( ctx context.Context, userID, baseDirID string, - fn func(graph.CacheFolder) error, + fn func(graph.CachedContainer) error, errs *fault.Bus, ) error } +type containerRefresher interface { + refreshContainer( + ctx context.Context, + dirID string, + ) (graph.CachedContainer, error) +} + // --------------------------------------------------------------------------- // controller // --------------------------------------------------------------------------- @@ -40,59 +48,243 @@ type containersEnumerator interface { // folders if each folder is only a single character. const maxIterations = 300 -func newContainerResolver() *containerResolver { +func newContainerResolver(refresher containerRefresher) *containerResolver { return &containerResolver{ - cache: map[string]graph.CachedContainer{}, + cache: map[string]graph.CachedContainer{}, + refresher: refresher, } } type containerResolver struct { - cache map[string]graph.CachedContainer + cache map[string]graph.CachedContainer + refresher containerRefresher } func (cr *containerResolver) IDToPath( ctx context.Context, folderID string, ) (*path.Builder, *path.Builder, error) { - return cr.idToPath(ctx, folderID, 0) + ctx = clues.Add(ctx, "container_id", folderID) + + c, ok := cr.cache[folderID] + if !ok { + return nil, nil, clues.New("container not cached").WithClues(ctx) + } + + p := c.Path() + if p == nil { + return nil, nil, clues.New("cached container has no path").WithClues(ctx) + } + + return p, c.Location(), nil +} + +// refreshContainer attempts to fetch the container with the given ID from Graph +// API. Returns a graph.CachedContainer if the container was found. If the +// container was deleted, returns nil, true, nil to note the container should +// be removed from the cache. +func (cr *containerResolver) refreshContainer( + ctx context.Context, + id string, +) (graph.CachedContainer, bool, error) { + ctx = clues.Add(ctx, "refresh_container_id", id) + logger.Ctx(ctx).Debug("refreshing container") + + if cr.refresher == nil { + return nil, false, clues.New("nil refresher").WithClues(ctx) + } + + c, err := cr.refresher.refreshContainer(ctx, id) + if err != nil && graph.IsErrDeletedInFlight(err) { + logger.Ctx(ctx).Debug("container deleted") + return nil, true, nil + } else if err != nil { + // This is some other error, just return it. + return nil, false, clues.Wrap(err, "refreshing container").WithClues(ctx) + } + + return c, false, nil +} + +// recoverContainer attempts to fetch a missing container from Graph API and +// populate the path for it. It returns +// - the ID path for the folder +// - the display name path for the folder +// - if the folder was deleted +// - any error that occurred +// +// If the folder is marked as deleted, child folders of this folder should be +// deleted if they haven't been moved to another folder. +func (cr *containerResolver) recoverContainer( + ctx context.Context, + folderID string, + depth int, +) (*path.Builder, *path.Builder, bool, error) { + c, deleted, err := cr.refreshContainer(ctx, folderID) + if err != nil { + return nil, nil, false, clues.Wrap(err, "fetching uncached container") + } + + if deleted { + logger.Ctx(ctx).Debug("fetching uncached container showed it was deleted") + return nil, nil, deleted, err + } + + if err := cr.addFolder(c); err != nil { + return nil, nil, false, clues.Wrap(err, "adding new container").WithClues(ctx) + } + + // Retry populating this container's paths. + // + // TODO(ashmrtn): May want to bump the depth here just so we don't get stuck + // retrying too much if for some reason things keep moving around? + resolved, err := cr.idToPath(ctx, folderID, depth) + if err != nil { + err = clues.Wrap(err, "repopulating uncached container") + } + + return resolved.idPath, resolved.locPath, resolved.deleted, err +} + +type resolvedPath struct { + idPath *path.Builder + locPath *path.Builder + cached bool + deleted bool } func (cr *containerResolver) idToPath( ctx context.Context, folderID string, depth int, -) (*path.Builder, *path.Builder, error) { +) (resolvedPath, error) { ctx = clues.Add(ctx, "container_id", folderID) if depth >= maxIterations { - return nil, nil, clues.New("path contains cycle or is too tall").WithClues(ctx) + return resolvedPath{ + idPath: nil, + locPath: nil, + cached: false, + deleted: false, + }, clues.New("path contains cycle or is too tall").WithClues(ctx) } c, ok := cr.cache[folderID] if !ok { - return nil, nil, clues.New("folder not cached").WithClues(ctx) + pth, loc, deleted, err := cr.recoverContainer(ctx, folderID, depth) + if err != nil { + err = clues.Stack(err) + } + + return resolvedPath{ + idPath: pth, + locPath: loc, + cached: false, + deleted: deleted, + }, err } p := c.Path() if p != nil { - return p, c.Location(), nil + return resolvedPath{ + idPath: p, + locPath: c.Location(), + cached: true, + deleted: false, + }, nil } - parentPath, parentLoc, err := cr.idToPath( + resolved, err := cr.idToPath( ctx, ptr.Val(c.GetParentFolderId()), depth+1) if err != nil { - return nil, nil, clues.Wrap(err, "retrieving parent folder") + return resolvedPath{ + idPath: nil, + locPath: nil, + cached: true, + deleted: false, + }, clues.Wrap(err, "retrieving parent container") } - fullPath := parentPath.Append(ptr.Val(c.GetId())) + if !resolved.cached { + logger.Ctx(ctx).Debug("parent container was refreshed") + + newContainer, shouldDelete, err := cr.refreshContainer(ctx, folderID) + if err != nil { + return resolvedPath{ + idPath: nil, + locPath: nil, + cached: true, + deleted: false, + }, clues.Wrap(err, "refreshing container").WithClues(ctx) + } + + if shouldDelete { + logger.Ctx(ctx).Debug("refreshing container showed it was deleted") + delete(cr.cache, folderID) + + return resolvedPath{ + idPath: nil, + locPath: nil, + cached: true, + deleted: true, + }, nil + } + + // See if the newer version of the current container we got back has + // changed. If it has then it could be that the container was moved prior to + // deleting the parent and we just hit some eventual consistency case in + // Graph. + // + // TODO(ashmrtn): May want to bump the depth here just so we don't get stuck + // retrying too much if for some reason things keep moving around? + if ptr.Val(newContainer.GetParentFolderId()) != ptr.Val(c.GetParentFolderId()) || + ptr.Val(newContainer.GetDisplayName()) != ptr.Val(c.GetDisplayName()) { + delete(cr.cache, folderID) + + if err := cr.addFolder(newContainer); err != nil { + return resolvedPath{ + idPath: nil, + locPath: nil, + cached: false, + deleted: false, + }, clues.Wrap(err, "updating cached container").WithClues(ctx) + } + + return cr.idToPath(ctx, folderID, depth) + } + } + + // If the parent wasn't found and refreshing the current container produced no + // diffs then delete the current container on the assumption that the parent + // was deleted and the current container will later get deleted via eventual + // consistency. If w're wrong then the container will get picked up again on + // the next backup. + if resolved.deleted { + logger.Ctx(ctx).Debug("deleting container since parent was deleted") + delete(cr.cache, folderID) + + return resolvedPath{ + idPath: nil, + locPath: nil, + cached: true, + deleted: true, + }, nil + } + + fullPath := resolved.idPath.Append(ptr.Val(c.GetId())) c.SetPath(fullPath) - locPath := parentLoc.Append(ptr.Val(c.GetDisplayName())) + locPath := resolved.locPath.Append(ptr.Val(c.GetDisplayName())) c.SetLocation(locPath) - return fullPath, locPath, nil + return resolvedPath{ + idPath: fullPath, + locPath: locPath, + cached: true, + deleted: false, + }, nil } // PathInCache is a utility function to return m365ID of a folder if the @@ -139,14 +331,14 @@ func (cr *containerResolver) LocationInCache(pathString string) (string, bool) { // 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 { +func (cr *containerResolver) addFolder(cf graph.CachedContainer) error { // Only require a non-nil non-empty parent if the path isn't already populated. if cf.Path() != nil { - if err := checkIDAndName(cf.Container); err != nil { + if err := checkIDAndName(cf); err != nil { return clues.Wrap(err, "adding item to cache") } } else { - if err := checkRequiredValues(cf.Container); err != nil { + if err := checkRequiredValues(cf); err != nil { return clues.Wrap(err, "adding item to cache") } } @@ -155,7 +347,7 @@ func (cr *containerResolver) addFolder(cf graph.CacheFolder) error { return nil } - cr.cache[ptr.Val(cf.GetId())] = &cf + cr.cache[ptr.Val(cf.GetId())] = cf return nil } @@ -176,7 +368,7 @@ func (cr *containerResolver) AddToCache( ctx context.Context, f graph.Container, ) error { - temp := graph.CacheFolder{ + temp := &graph.CacheFolder{ Container: f, } if err := cr.addFolder(temp); err != nil { @@ -185,7 +377,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())) + _, err := cr.idToPath(ctx, ptr.Val(f.GetId()), 0) if err != nil { return clues.Wrap(err, "adding cache entry") } @@ -208,7 +400,7 @@ func (cr *containerResolver) populatePaths( return el.Failure() } - _, _, err := cr.IDToPath(ctx, ptr.Val(f.GetId())) + _, err := cr.idToPath(ctx, ptr.Val(f.GetId()), 0) 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 de050d25a..a79daa58f 100644 --- a/src/internal/connector/exchange/container_resolver_test.go +++ b/src/internal/connector/exchange/container_resolver_test.go @@ -1,6 +1,7 @@ package exchange import ( + "context" "fmt" stdpath "path" "testing" @@ -232,8 +233,8 @@ func (suite *FolderCacheUnitSuite) TestAddFolder() { for _, test := range table { suite.Run(test.name, func() { - fc := newContainerResolver() - err := fc.addFolder(test.cf) + fc := newContainerResolver(nil) + err := fc.addFolder(&test.cf) test.check(suite.T(), err, clues.ToCore(err)) }) } @@ -293,7 +294,7 @@ func resolverWithContainers(numContainers int, useIDInPath bool) (*containerReso containers[i].expectedLocation = stdpath.Join(containers[i-1].expectedLocation, dn) } - resolver := newContainerResolver() + resolver := newContainerResolver(nil) for _, c := range containers { resolver.cache[c.id] = c @@ -302,6 +303,37 @@ func resolverWithContainers(numContainers int, useIDInPath bool) (*containerReso return resolver, containers } +// --------------------------------------------------------------------------- +// mock container refresher +// --------------------------------------------------------------------------- + +type refreshResult struct { + err error + c graph.CachedContainer +} + +type mockContainerRefresher struct { + // Folder ID -> result + entries map[string]refreshResult +} + +func (r mockContainerRefresher) refreshContainer( + ctx context.Context, + id string, +) (graph.CachedContainer, error) { + rr, ok := r.entries[id] + if !ok { + // May not be this precise error, but it's easy to get a handle on. + return nil, graph.ErrDeletedInFlight + } + + if rr.err != nil { + return nil, rr.err + } + + return rr.c, nil +} + // --------------------------------------------------------------------------- // configured unit suite // --------------------------------------------------------------------------- @@ -326,6 +358,160 @@ func TestConfiguredFolderCacheUnitSuite(t *testing.T) { suite.Run(t, &ConfiguredFolderCacheUnitSuite{Suite: tester.NewUnitSuite(t)}) } +func (suite *ConfiguredFolderCacheUnitSuite) TestRefreshContainer_RefreshParent() { + ctx, flush := tester.NewContext() + defer flush() + + t := suite.T() + + resolver, containers := resolverWithContainers(4, true) + almostLast := containers[len(containers)-2] + last := containers[len(containers)-1] + + refresher := mockContainerRefresher{ + entries: map[string]refreshResult{ + almostLast.id: {c: almostLast}, + last.id: {c: last}, + }, + } + + resolver.refresher = refresher + + delete(resolver.cache, almostLast.id) + + ferrs := fault.New(true) + err := resolver.populatePaths(ctx, ferrs) + require.NoError(t, err, "populating paths", clues.ToCore(err)) + + p, l, err := resolver.IDToPath(ctx, last.id) + require.NoError(t, err, "getting paths", clues.ToCore(err)) + + assert.Equal(t, last.expectedPath, p.String()) + assert.Equal(t, last.expectedLocation, l.String()) +} + +func (suite *ConfiguredFolderCacheUnitSuite) TestRefreshContainer_RefreshParent_NotFoundDeletes() { + ctx, flush := tester.NewContext() + defer flush() + + t := suite.T() + + resolver, containers := resolverWithContainers(4, true) + almostLast := containers[len(containers)-2] + last := containers[len(containers)-1] + + refresher := mockContainerRefresher{ + entries: map[string]refreshResult{ + last.id: {c: last}, + }, + } + + resolver.refresher = refresher + + delete(resolver.cache, almostLast.id) + + ferrs := fault.New(true) + err := resolver.populatePaths(ctx, ferrs) + require.NoError(t, err, "populating paths", clues.ToCore(err)) + + _, _, err = resolver.IDToPath(ctx, last.id) + assert.Error(t, err, "getting paths", clues.ToCore(err)) +} + +func (suite *ConfiguredFolderCacheUnitSuite) TestRefreshContainer_RefreshAncestor_NotFoundDeletes() { + ctx, flush := tester.NewContext() + defer flush() + + t := suite.T() + + resolver, containers := resolverWithContainers(4, true) + gone := containers[0] + child := containers[1] + last := containers[len(containers)-1] + + refresher := mockContainerRefresher{ + entries: map[string]refreshResult{ + child.id: {c: child}, + }, + } + + resolver.refresher = refresher + + delete(resolver.cache, gone.id) + + ferrs := fault.New(true) + err := resolver.populatePaths(ctx, ferrs) + require.NoError(t, err, "populating paths", clues.ToCore(err)) + + _, _, err = resolver.IDToPath(ctx, last.id) + assert.Error(t, err, "getting paths", clues.ToCore(err)) +} + +func (suite *ConfiguredFolderCacheUnitSuite) TestRefreshContainer_RefreshAncestor_NewParent() { + ctx, flush := tester.NewContext() + defer flush() + + t := suite.T() + + resolver, containers := resolverWithContainers(4, true) + other := containers[len(containers)-3] + gone := containers[len(containers)-2] + last := containers[len(containers)-1] + + expected := *last + expected.parentID = other.id + expected.expectedPath = stdpath.Join(other.expectedPath, expected.id) + expected.expectedLocation = stdpath.Join(other.expectedLocation, expected.displayName) + + refresher := mockContainerRefresher{ + entries: map[string]refreshResult{ + last.id: {c: &expected}, + }, + } + + resolver.refresher = refresher + + delete(resolver.cache, gone.id) + + ferrs := fault.New(true) + err := resolver.populatePaths(ctx, ferrs) + require.NoError(t, err, "populating paths", clues.ToCore(err)) + + p, l, err := resolver.IDToPath(ctx, last.id) + require.NoError(t, err, "getting paths", clues.ToCore(err)) + + assert.Equal(t, expected.expectedPath, p.String()) + assert.Equal(t, expected.expectedLocation, l.String()) +} + +func (suite *ConfiguredFolderCacheUnitSuite) TestRefreshContainer_RefreshFolder_FolderDeleted() { + ctx, flush := tester.NewContext() + defer flush() + + t := suite.T() + + resolver, containers := resolverWithContainers(4, true) + parent := containers[len(containers)-2] + last := containers[len(containers)-1] + + refresher := mockContainerRefresher{ + entries: map[string]refreshResult{ + parent.id: {c: parent}, + }, + } + + resolver.refresher = refresher + + delete(resolver.cache, parent.id) + + ferrs := fault.New(true) + err := resolver.populatePaths(ctx, ferrs) + require.NoError(t, err, "populating paths", clues.ToCore(err)) + + _, _, err = resolver.IDToPath(ctx, last.id) + assert.Error(t, err, "getting paths", clues.ToCore(err)) +} + func (suite *ConfiguredFolderCacheUnitSuite) TestDepthLimit() { ctx, flush := tester.NewContext() defer flush() @@ -350,7 +536,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) + _, err := resolver.idToPath(ctx, containers[len(containers)-1].id, 0) test.check(suite.T(), err, clues.ToCore(err)) }) } @@ -384,6 +570,9 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderNoPathsCached ctx, flush := tester.NewContext() defer flush() + err := suite.fc.populatePaths(ctx, fault.New(true)) + require.NoError(suite.T(), err, clues.ToCore(err)) + for _, c := range suite.allContainers { suite.Run(ptr.Val(c.GetDisplayName()), func() { t := suite.T() @@ -396,10 +585,14 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderNoPathsCached } } +// TODO(ashmrtn): Remove this since the same cache can do IDs or locations. func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderNoPathsCached_useID() { ctx, flush := tester.NewContext() defer flush() + err := suite.fcWithID.populatePaths(ctx, fault.New(true)) + require.NoError(suite.T(), err, clues.ToCore(err)) + for _, c := range suite.containersWithID { suite.Run(ptr.Val(c.GetDisplayName()), func() { t := suite.T() @@ -419,6 +612,9 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderCachesPaths() t := suite.T() c := suite.allContainers[len(suite.allContainers)-1] + err := suite.fc.populatePaths(ctx, fault.New(true)) + require.NoError(t, err, clues.ToCore(err)) + p, l, err := suite.fc.IDToPath(ctx, c.id) require.NoError(t, err, clues.ToCore(err)) assert.Equal(t, c.expectedPath, p.String()) @@ -432,6 +628,7 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderCachesPaths() assert.Equal(t, c.expectedLocation, l.String()) } +// TODO(ashmrtn): Remove this since the same cache can do IDs or locations. func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderCachesPaths_useID() { ctx, flush := tester.NewContext() defer flush() @@ -439,6 +636,9 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderCachesPaths_u t := suite.T() c := suite.containersWithID[len(suite.containersWithID)-1] + err := suite.fcWithID.populatePaths(ctx, fault.New(true)) + require.NoError(t, err, clues.ToCore(err)) + p, l, err := suite.fcWithID.IDToPath(ctx, c.id) require.NoError(t, err, clues.ToCore(err)) assert.Equal(t, c.expectedPath, p.String()) @@ -457,12 +657,21 @@ func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderErrorsParentN defer flush() t := suite.T() - last := suite.allContainers[len(suite.allContainers)-1] almostLast := suite.allContainers[len(suite.allContainers)-2] delete(suite.fc.cache, almostLast.id) - _, _, err := suite.fc.IDToPath(ctx, last.id) + err := suite.fc.populatePaths(ctx, fault.New(true)) + assert.Error(t, err, clues.ToCore(err)) +} + +func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolder_Errors_PathsNotBuilt() { + ctx, flush := tester.NewContext() + defer flush() + + t := suite.T() + + _, _, err := suite.fc.IDToPath(ctx, suite.allContainers[len(suite.allContainers)-1].id) assert.Error(t, err, clues.ToCore(err)) } diff --git a/src/internal/connector/exchange/data_collections_test.go b/src/internal/connector/exchange/data_collections_test.go index ef34de5ff..557df264f 100644 --- a/src/internal/connector/exchange/data_collections_test.go +++ b/src/internal/connector/exchange/data_collections_test.go @@ -597,7 +597,7 @@ func (suite *DataCollectionsIntegrationSuite) TestEventsSerializationRegression( bdayID string ) - fn := func(gcf graph.CacheFolder) error { + fn := func(gcf graph.CachedContainer) error { if ptr.Val(gcf.GetDisplayName()) == DefaultCalendar { calID = ptr.Val(gcf.GetId()) } diff --git a/src/internal/connector/exchange/event_calendar_cache.go b/src/internal/connector/exchange/event_calendar_cache.go index ac8e59548..5e99b4b39 100644 --- a/src/internal/connector/exchange/event_calendar_cache.go +++ b/src/internal/connector/exchange/event_calendar_cache.go @@ -27,7 +27,7 @@ func (ecc *eventCalendarCache) init( ctx context.Context, ) error { if ecc.containerResolver == nil { - ecc.containerResolver = newContainerResolver() + ecc.containerResolver = newContainerResolver(nil) } return ecc.populateEventRoot(ctx) @@ -49,7 +49,7 @@ func (ecc *eventCalendarCache) populateEventRoot(ctx context.Context) error { f, path.Builder{}.Append(ptr.Val(f.GetId())), // storage path path.Builder{}.Append(ptr.Val(f.GetDisplayName()))) // display location - if err := ecc.addFolder(temp); err != nil { + if err := ecc.addFolder(&temp); err != nil { return clues.Wrap(err, "initializing calendar resolver").WithClues(ctx) } @@ -98,7 +98,7 @@ 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 err := ecc.addFolder(temp); err != nil { + if err := ecc.addFolder(&temp); err != nil { return clues.Wrap(err, "adding container").WithClues(ctx) } diff --git a/src/internal/connector/exchange/mail_folder_cache.go b/src/internal/connector/exchange/mail_folder_cache.go index c2630a29a..062f91a23 100644 --- a/src/internal/connector/exchange/mail_folder_cache.go +++ b/src/internal/connector/exchange/mail_folder_cache.go @@ -10,7 +10,29 @@ import ( "github.com/alcionai/corso/src/pkg/path" ) -var _ graph.ContainerResolver = &mailFolderCache{} +var ( + _ graph.ContainerResolver = &mailFolderCache{} + _ containerRefresher = &mailRefresher{} +) + +type mailRefresher struct { + getter containerGetter + userID string +} + +func (r *mailRefresher) refreshContainer( + ctx context.Context, + id string, +) (graph.CachedContainer, error) { + c, err := r.getter.GetContainerByID(ctx, r.userID, id) + if err != nil { + return nil, clues.Stack(err) + } + + f := graph.NewCacheFolder(c, nil, nil) + + return &f, nil +} // mailFolderCache struct used to improve lookup of directories within exchange.Mail // cache map of cachedContainers where the key = M365ID @@ -29,7 +51,10 @@ func (mc *mailFolderCache) init( ctx context.Context, ) error { if mc.containerResolver == nil { - mc.containerResolver = newContainerResolver() + mc.containerResolver = newContainerResolver(&mailRefresher{ + userID: mc.userID, + getter: mc.getter, + }) } return mc.populateMailRoot(ctx) @@ -52,7 +77,7 @@ func (mc *mailFolderCache) populateMailRoot(ctx context.Context) error { // the user doesn't see in the regular UI for Exchange. path.Builder{}.Append(), // path of IDs path.Builder{}.Append()) // display location - if err := mc.addFolder(temp); err != nil { + if err := mc.addFolder(&temp); err != nil { return clues.Wrap(err, "adding resolver dir").WithClues(ctx) } diff --git a/src/pkg/services/m365/api/contacts.go b/src/pkg/services/m365/api/contacts.go index b958e2ea1..abcc2ab25 100644 --- a/src/pkg/services/m365/api/contacts.go +++ b/src/pkg/services/m365/api/contacts.go @@ -118,7 +118,7 @@ func (c Contacts) GetContainerByID( func (c Contacts) EnumerateContainers( ctx context.Context, userID, baseDirID string, - fn func(graph.CacheFolder) error, + fn func(graph.CachedContainer) error, errs *fault.Bus, ) error { service, err := c.Service() @@ -166,7 +166,7 @@ func (c Contacts) EnumerateContainers( "container_display_name", ptr.Val(fold.GetDisplayName())) temp := graph.NewCacheFolder(fold, nil, nil) - if err := fn(temp); err != nil { + if err := fn(&temp); err != nil { errs.AddRecoverable(graph.Stack(fctx, err).Label(fault.LabelForceNoBackupCreation)) continue } diff --git a/src/pkg/services/m365/api/events.go b/src/pkg/services/m365/api/events.go index 5cccdc49b..9700e9ffa 100644 --- a/src/pkg/services/m365/api/events.go +++ b/src/pkg/services/m365/api/events.go @@ -192,7 +192,7 @@ func (c Events) GetItem( func (c Events) EnumerateContainers( ctx context.Context, userID, baseDirID string, - fn func(graph.CacheFolder) error, + fn func(graph.CachedContainer) error, errs *fault.Bus, ) error { service, err := c.Service() @@ -239,7 +239,7 @@ func (c Events) EnumerateContainers( cd, path.Builder{}.Append(ptr.Val(cd.GetId())), // storage path path.Builder{}.Append(ptr.Val(cd.GetDisplayName()))) // display location - if err := fn(temp); err != nil { + if err := fn(&temp); err != nil { errs.AddRecoverable(graph.Stack(fctx, err).Label(fault.LabelForceNoBackupCreation)) continue } diff --git a/src/pkg/services/m365/api/mail.go b/src/pkg/services/m365/api/mail.go index c951027be..5fe9e6d4f 100644 --- a/src/pkg/services/m365/api/mail.go +++ b/src/pkg/services/m365/api/mail.go @@ -309,7 +309,7 @@ func (p *mailFolderPager) valuesIn(pl api.PageLinker) ([]models.MailFolderable, func (c Mail) EnumerateContainers( ctx context.Context, userID, baseDirID string, - fn func(graph.CacheFolder) error, + fn func(graph.CachedContainer) error, errs *fault.Bus, ) error { service, err := c.Service() @@ -347,7 +347,7 @@ func (c Mail) EnumerateContainers( "container_name", ptr.Val(v.GetDisplayName())) temp := graph.NewCacheFolder(v, nil, nil) - if err := fn(temp); err != nil { + if err := fn(&temp); err != nil { errs.AddRecoverable(graph.Stack(fctx, err).Label(fault.LabelForceNoBackupCreation)) continue }