diff --git a/src/internal/connector/exchange/contact_folder_cache.go b/src/internal/connector/exchange/contact_folder_cache.go index a9e7f93a2..b88378e15 100644 --- a/src/internal/connector/exchange/contact_folder_cache.go +++ b/src/internal/connector/exchange/contact_folder_cache.go @@ -15,9 +15,9 @@ import ( var _ graph.ContainerResolver = &contactFolderCache{} type contactFolderCache struct { - cache map[string]graph.CachedContainer - gs graph.Service - userID, rootID string + *containerResolver + gs graph.Service + userID string } func (cfc *contactFolderCache) populateContactRoot( @@ -44,18 +44,14 @@ func (cfc *contactFolderCache) populateContactRoot( "fetching root contact folder: "+support.ConnectorStackErrorTrace(err)) } - idPtr := f.GetId() - - if idPtr == nil || len(*idPtr) == 0 { - return errors.New("root folder has no ID") - } - temp := cacheFolder{ Container: f, p: path.Builder{}.Append(baseContainerPath...), } - cfc.cache[*idPtr] = &temp - cfc.rootID = *idPtr + + if err := cfc.addFolder(temp); err != nil { + return errors.Wrap(err, "adding cache root") + } return nil } @@ -84,7 +80,7 @@ func (cfc *contactFolderCache) Populate( query, err := cfc. gs.Client(). UsersById(cfc.userID). - ContactFoldersById(cfc.rootID). + ContactFoldersById(baseID). ChildFolders(). Get(ctx, nil) if err != nil { @@ -109,7 +105,11 @@ func (cfc *contactFolderCache) Populate( } for _, entry := range containers { - err = cfc.AddToCache(ctx, entry) + temp := cacheFolder{ + Container: entry, + } + + err = cfc.addFolder(temp) if err != nil { errs = support.WrapAndAppend( "cache build in cfc.Populate", @@ -118,6 +118,14 @@ func (cfc *contactFolderCache) Populate( } } + if err := cfc.populatePaths(ctx); err != nil { + errs = support.WrapAndAppend( + "contacts resolver", + err, + errs, + ) + } + return errs } @@ -130,90 +138,9 @@ func (cfc *contactFolderCache) init( return errors.New("m365 folderID required for base folder") } - if cfc.cache == nil { - cfc.cache = map[string]graph.CachedContainer{} + if cfc.containerResolver == nil { + cfc.containerResolver = newContainerResolver() } return cfc.populateContactRoot(ctx, baseNode, baseContainerPath) } - -func (cfc *contactFolderCache) IDToPath( - ctx context.Context, - folderID string, -) (*path.Builder, error) { - c, ok := cfc.cache[folderID] - if !ok { - return nil, errors.Errorf("contact folder %s not cached", folderID) - } - - p := c.Path() - if p != nil { - return p, nil - } - - parentPath, err := cfc.IDToPath(ctx, *c.GetParentFolderId()) - if err != nil { - return nil, errors.Wrap(err, "retrieving parent folder") - } - - fullPath := parentPath.Append(*c.GetDisplayName()) - c.SetPath(fullPath) - - return fullPath, nil -} - -// PathInCache utility function to return m365ID of folder if the pathString -// matches the path of a container within the cache. A boolean function -// accompanies the call to indicate whether the lookup was successful. -func (cfc *contactFolderCache) PathInCache(pathString string) (string, bool) { - if len(pathString) == 0 || cfc.cache == nil { - return "", false - } - - for _, contain := range cfc.cache { - if contain.Path() == nil { - continue - } - - if contain.Path().String() == pathString { - return *contain.GetId(), true - } - } - - return "", false -} - -// AddToCache places container into internal cache field. -// @returns error iff input does not possess accessible values. -func (cfc *contactFolderCache) AddToCache(ctx context.Context, f graph.Container) error { - if err := checkRequiredValues(f); err != nil { - return err - } - - if _, ok := cfc.cache[*f.GetId()]; ok { - return nil - } - - cfc.cache[*f.GetId()] = &cacheFolder{ - Container: f, - } - - // Populate the path for this entry so calls to PathInCache succeed no matter - // when they're made. - _, err := cfc.IDToPath(ctx, *f.GetId()) - if err != nil { - return errors.Wrap(err, "adding cache entry") - } - - return nil -} - -func (cfc *contactFolderCache) Items() []graph.CachedContainer { - res := make([]graph.CachedContainer, 0, len(cfc.cache)) - - for _, c := range cfc.cache { - res = append(res, c) - } - - return res -} diff --git a/src/internal/connector/exchange/container_resolver.go b/src/internal/connector/exchange/container_resolver.go new file mode 100644 index 000000000..f8b097d75 --- /dev/null +++ b/src/internal/connector/exchange/container_resolver.go @@ -0,0 +1,136 @@ +package exchange + +import ( + "context" + + "github.com/hashicorp/go-multierror" + "github.com/pkg/errors" + + "github.com/alcionai/corso/src/internal/connector/graph" + "github.com/alcionai/corso/src/pkg/path" +) + +func newContainerResolver() *containerResolver { + return &containerResolver{ + cache: map[string]graph.CachedContainer{}, + } +} + +type containerResolver struct { + cache map[string]graph.CachedContainer +} + +func (cr *containerResolver) IDToPath( + ctx context.Context, + folderID string, +) (*path.Builder, error) { + c, ok := cr.cache[folderID] + if !ok { + return nil, errors.Errorf("folder %s not cached", folderID) + } + + p := c.Path() + if p != nil { + return p, nil + } + + parentPath, err := cr.IDToPath(ctx, *c.GetParentFolderId()) + if err != nil { + return nil, errors.Wrap(err, "retrieving parent folder") + } + + fullPath := parentPath.Append(*c.GetDisplayName()) + c.SetPath(fullPath) + + return fullPath, nil +} + +// PathInCache utility function to return m365ID of folder if the pathString +// matches the path of a container within the cache. A boolean function +// accompanies the call to indicate whether the lookup was successful. +func (cr *containerResolver) PathInCache(pathString string) (string, bool) { + if len(pathString) == 0 || cr == nil { + return "", false + } + + for _, contain := range cr.cache { + if contain.Path() == nil { + continue + } + + if contain.Path().String() == pathString { + return *contain.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 cacheFolder) error { + // Only require a non-nil non-empty parent if the path isn't already + // populated. + if cf.p != nil { + if err := checkIDAndName(cf.Container); err != nil { + return errors.Wrap(err, "adding item to cache") + } + } else { + if err := checkRequiredValues(cf.Container); err != nil { + return errors.Wrap(err, "adding item to cache") + } + } + + if _, ok := cr.cache[*cf.GetId()]; ok { + return nil + } + + cr.cache[*cf.GetId()] = &cf + + return nil +} + +func (cr *containerResolver) Items() []graph.CachedContainer { + res := make([]graph.CachedContainer, 0, len(cr.cache)) + + for _, c := range cr.cache { + res = append(res, c) + } + + return res +} + +// AddToCache adds container to map in field 'cache' +// @returns error iff the required values are not accessible. +func (cr *containerResolver) AddToCache(ctx context.Context, f graph.Container) error { + temp := cacheFolder{ + Container: f, + } + + if err := cr.addFolder(temp); err != nil { + return errors.Wrap(err, "adding cache folder") + } + + // Populate the path for this entry so calls to PathInCache succeed no matter + // when they're made. + _, err := cr.IDToPath(ctx, *f.GetId()) + if err != nil { + return errors.Wrap(err, "adding cache entry") + } + + return nil +} + +func (cr *containerResolver) populatePaths(ctx context.Context) error { + var errs *multierror.Error + + // Populate all folder paths. + for _, f := range cr.Items() { + _, err := cr.IDToPath(ctx, *f.GetId()) + if err != nil { + errs = multierror.Append(errs, errors.Wrap(err, "populating path")) + } + } + + return errs.ErrorOrNil() +} diff --git a/src/internal/connector/exchange/container_resolver_test. b/src/internal/connector/exchange/container_resolver_test. new file mode 100644 index 000000000..3282d2c33 --- /dev/null +++ b/src/internal/connector/exchange/container_resolver_test. @@ -0,0 +1,400 @@ +package exchange + +import ( + stdpath "path" + "strings" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/path" +) + +type mockContainer struct { + id *string + name *string + parentID *string +} + +//nolint:revive +func (m mockContainer) GetId() *string { + return m.id +} + +func (m mockContainer) GetDisplayName() *string { + return m.name +} + +//nolint:revive +func (m mockContainer) GetParentFolderId() *string { + return m.parentID +} + +type FolderCacheUnitSuite struct { + suite.Suite +} + +func TestFolderCacheUnitSuite(t *testing.T) { + suite.Run(t, new(FolderCacheUnitSuite)) +} + +type containerCheckTestInfo struct { + name string + c mockContainer + check assert.ErrorAssertionFunc +} + +var ( + testID = uuid.NewString() + testName = "foo" + testParentID = uuid.NewString() + emptyString = "" + + containerCheckTests = []containerCheckTestInfo{ + { + name: "NilID", + c: mockContainer{ + id: nil, + name: &testName, + parentID: &testParentID, + }, + check: assert.Error, + }, + { + name: "NilDisplayName", + c: mockContainer{ + id: &testID, + name: nil, + parentID: &testParentID, + }, + check: assert.Error, + }, + { + name: "EmptyID", + c: mockContainer{ + id: &emptyString, + name: &testName, + parentID: &testParentID, + }, + check: assert.Error, + }, + { + name: "EmptyDisplayName", + c: mockContainer{ + id: &testID, + name: &emptyString, + parentID: &testParentID, + }, + check: assert.Error, + }, + { + name: "AllValues", + c: mockContainer{ + id: &testID, + name: &testName, + parentID: &testParentID, + }, + check: assert.NoError, + }, + } +) + +func (suite *FolderCacheUnitSuite) TestCheckIDAndName() { + for _, test := range containerCheckTests { + suite.T().Run(test.name, func(t *testing.T) { + test.check(t, checkIDAndName(test.c)) + }) + } +} + +func (suite *FolderCacheUnitSuite) TestCheckRequiredValues() { + table := []containerCheckTestInfo{ + { + name: "NilParentFolderID", + c: mockContainer{ + id: &testID, + name: &testName, + parentID: nil, + }, + check: assert.Error, + }, + { + name: "EmptyParentFolderID", + c: mockContainer{ + id: &testID, + name: &testName, + parentID: &emptyString, + }, + check: assert.Error, + }, + } + + table = append(table, containerCheckTests...) + + for _, test := range table { + suite.T().Run(test.name, func(t *testing.T) { + test.check(t, checkRequiredValues(test.c)) + }) + } +} + +func (suite *FolderCacheUnitSuite) TestAddFolder() { + table := []struct { + name string + cf cacheFolder + check assert.ErrorAssertionFunc + }{ + { + name: "NoParentNoPath", + cf: cacheFolder{ + Container: &mockContainer{ + id: &testID, + name: &testName, + parentID: nil, + }, + p: nil, + }, + check: assert.Error, + }, + { + name: "NoParentPath", + cf: cacheFolder{ + Container: &mockContainer{ + id: &testID, + name: &testName, + parentID: nil, + }, + p: path.Builder{}.Append("foo"), + }, + check: assert.NoError, + }, + { + name: "NoName", + cf: cacheFolder{ + Container: &mockContainer{ + id: &testID, + name: nil, + parentID: &testParentID, + }, + p: path.Builder{}.Append("foo"), + }, + check: assert.Error, + }, + { + name: "NoID", + cf: cacheFolder{ + Container: &mockContainer{ + id: nil, + name: &testName, + parentID: &testParentID, + }, + p: path.Builder{}.Append("foo"), + }, + check: assert.Error, + }, + { + name: "NoPath", + cf: cacheFolder{ + Container: &mockContainer{ + id: &testID, + name: &testName, + parentID: &testParentID, + }, + p: nil, + }, + check: assert.NoError, + }, + } + + for _, test := range table { + suite.T().Run(test.name, func(t *testing.T) { + fc := newContainerResolver() + test.check(t, fc.addFolder(test.cf)) + }) + } +} + +func newMockCachedContainer(name string) *mockCachedContainer { + return &mockCachedContainer{ + id: uuid.NewString(), + parentID: uuid.NewString(), + displayName: name, + } +} + +type mockCachedContainer struct { + id string + parentID string + displayName string + p *path.Builder + expectedPath string +} + +//nolint:revive +func (m mockCachedContainer) GetId() *string { + return &m.id +} + +//nolint:revive +func (m mockCachedContainer) GetParentFolderId() *string { + return &m.parentID +} + +func (m mockCachedContainer) GetDisplayName() *string { + return &m.displayName +} + +func (m mockCachedContainer) Path() *path.Builder { + return m.p +} + +func (m *mockCachedContainer) SetPath(newPath *path.Builder) { + m.p = newPath +} + +// TestConfiguredFolderCacheUnitSuite cannot run its tests in parallel. +type ConfiguredFolderCacheUnitSuite struct { + suite.Suite + + fc *containerResolver + + allContainers []*mockCachedContainer +} + +func (suite *ConfiguredFolderCacheUnitSuite) SetupTest() { + suite.allContainers = []*mockCachedContainer{} + + for i := 0; i < 4; i++ { + suite.allContainers = append( + suite.allContainers, + newMockCachedContainer(strings.Repeat("sub", i)+"folder"), + ) + } + + // Base case for the recursive lookup. + suite.allContainers[0].p = path.Builder{}.Append(suite.allContainers[0].displayName) + suite.allContainers[0].expectedPath = suite.allContainers[0].displayName + + for i := 1; i < len(suite.allContainers); i++ { + suite.allContainers[i].parentID = suite.allContainers[i-1].id + suite.allContainers[i].expectedPath = stdpath.Join( + suite.allContainers[i-1].expectedPath, + suite.allContainers[i].displayName, + ) + } + + suite.fc = newContainerResolver() + + for _, c := range suite.allContainers { + suite.fc.cache[c.id] = c + } +} + +func TestConfiguredFolderCacheUnitSuite(t *testing.T) { + suite.Run(t, new(ConfiguredFolderCacheUnitSuite)) +} + +func (suite *ConfiguredFolderCacheUnitSuite) TestPopulatePaths() { + ctx, flush := tester.NewContext() + defer flush() + + t := suite.T() + + require.NoError(t, suite.fc.populatePaths(ctx)) + + items := suite.fc.Items() + gotPaths := make([]string, 0, len(items)) + + for _, i := range items { + gotPaths = append(gotPaths, i.Path().String()) + } + + expectedPaths := make([]string, 0, len(suite.allContainers)) + for _, c := range suite.allContainers { + expectedPaths = append(expectedPaths, c.expectedPath) + } + + assert.ElementsMatch(t, expectedPaths, gotPaths) +} + +func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderNoPathsCached() { + ctx, flush := tester.NewContext() + defer flush() + + for _, c := range suite.allContainers { + suite.T().Run(*c.GetDisplayName(), func(t *testing.T) { + p, err := suite.fc.IDToPath(ctx, c.id) + require.NoError(t, err) + + assert.Equal(t, c.expectedPath, p.String()) + }) + } +} + +func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderCachesPaths() { + ctx, flush := tester.NewContext() + defer flush() + + t := suite.T() + c := suite.allContainers[len(suite.allContainers)-1] + + p, err := suite.fc.IDToPath(ctx, c.id) + require.NoError(t, err) + + assert.Equal(t, c.expectedPath, p.String()) + + c.parentID = "foo" + + p, err = suite.fc.IDToPath(ctx, c.id) + require.NoError(t, err) + + assert.Equal(t, c.expectedPath, p.String()) +} + +func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderErrorsParentNotFound() { + ctx, flush := tester.NewContext() + 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) + assert.Error(t, err) +} + +func (suite *ConfiguredFolderCacheUnitSuite) TestLookupCachedFolderErrorsNotFound() { + ctx, flush := tester.NewContext() + defer flush() + + t := suite.T() + + _, err := suite.fc.IDToPath(ctx, "foo") + assert.Error(t, err) +} + +func (suite *ConfiguredFolderCacheUnitSuite) TestAddToCache() { + ctx, flush := tester.NewContext() + defer flush() + + t := suite.T() + + last := suite.allContainers[len(suite.allContainers)-1] + + m := newMockCachedContainer("testAddFolder") + + m.parentID = last.id + m.expectedPath = stdpath.Join(last.expectedPath, m.displayName) + + require.NoError(t, suite.fc.AddToCache(ctx, m)) + + p, err := suite.fc.IDToPath(ctx, m.id) + require.NoError(t, err) + assert.Equal(t, m.expectedPath, p.String()) +} diff --git a/src/internal/connector/exchange/event_calendar_cache.go b/src/internal/connector/exchange/event_calendar_cache.go index 3999cf8e7..79ea316f4 100644 --- a/src/internal/connector/exchange/event_calendar_cache.go +++ b/src/internal/connector/exchange/event_calendar_cache.go @@ -15,9 +15,9 @@ import ( var _ graph.ContainerResolver = &eventCalendarCache{} type eventCalendarCache struct { - cache map[string]graph.CachedContainer - gs graph.Service - userID, rootID string + *containerResolver + gs graph.Service + userID string } // Populate utility function for populating eventCalendarCache. @@ -28,8 +28,8 @@ func (ecc *eventCalendarCache) Populate( baseID string, baseContainerPath ...string, ) error { - if ecc.cache == nil { - ecc.cache = map[string]graph.CachedContainer{} + if ecc.containerResolver == nil { + ecc.containerResolver = newContainerResolver() } options, err := optionsForCalendars([]string{"name"}) @@ -75,10 +75,25 @@ func (ecc *eventCalendarCache) Populate( return err } - for _, containerr := range directories { - if err := ecc.AddToCache(ctx, containerr); err != nil { + for _, container := range directories { + if err := checkIDAndName(container); err != nil { iterateErr = support.WrapAndAppend( - "failure adding "+*containerr.GetDisplayName(), + "adding folder to cache", + err, + iterateErr, + ) + + continue + } + + temp := cacheFolder{ + Container: container, + p: path.Builder{}.Append(*container.GetDisplayName()), + } + + if err := ecc.addFolder(temp); err != nil { + iterateErr = support.WrapAndAppend( + "failure adding "+*container.GetDisplayName(), err, iterateErr) } @@ -87,69 +102,28 @@ func (ecc *eventCalendarCache) Populate( return iterateErr } -func (ecc *eventCalendarCache) IDToPath( - ctx context.Context, - calendarID string, -) (*path.Builder, error) { - c, ok := ecc.cache[calendarID] - if !ok { - return nil, errors.Errorf("calendar %s not cached", calendarID) - } - - p := c.Path() - if p == nil { - // Shouldn't happen - p := path.Builder{}.Append(*c.GetDisplayName()) - c.SetPath(p) - } - - return p, nil -} - -// AddToCache places container into internal cache field. For EventCalendars -// this means that the object has to be transformed prior to calling -// this function. +// 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) error { if err := checkIDAndName(f); err != nil { - return err + return errors.Wrap(err, "adding cache folder") } - if _, ok := ecc.cache[*f.GetId()]; ok { - return nil - } - - ecc.cache[*f.GetId()] = &cacheFolder{ + temp := cacheFolder{ Container: f, p: path.Builder{}.Append(*f.GetDisplayName()), } + if err := ecc.addFolder(temp); err != nil { + return errors.Wrap(err, "adding cache folder") + } + + // Populate the path for this entry so calls to PathInCache succeed no matter + // when they're made. + _, err := ecc.IDToPath(ctx, *f.GetId()) + if err != nil { + return errors.Wrap(err, "adding cache entry") + } + return nil } - -func (ecc *eventCalendarCache) PathInCache(pathString string) (string, bool) { - if len(pathString) == 0 || ecc.cache == nil { - return "", false - } - - for _, containerr := range ecc.cache { - if containerr.Path() == nil { - continue - } - - if containerr.Path().String() == pathString { - return *containerr.GetId(), true - } - } - - return "", false -} - -func (ecc *eventCalendarCache) Items() []graph.CachedContainer { - res := make([]graph.CachedContainer, 0, len(ecc.cache)) - - for _, c := range ecc.cache { - res = append(res, c) - } - - return res -} diff --git a/src/internal/connector/exchange/folder_resolver_test.go b/src/internal/connector/exchange/folder_resolver_test.go index b9f5e4a56..6adfb3ceb 100644 --- a/src/internal/connector/exchange/folder_resolver_test.go +++ b/src/internal/connector/exchange/folder_resolver_test.go @@ -50,72 +50,77 @@ func (suite *CacheResolverSuite) TestPopulate() { ctx, flush := tester.NewContext() defer flush() - ecc := eventCalendarCache{ - userID: tester.M365UserID(suite.T()), - gs: suite.gs, + eventFunc := func(t *testing.T) graph.ContainerResolver { + return &eventCalendarCache{ + userID: tester.M365UserID(t), + gs: suite.gs, + } } - cfc := contactFolderCache{ - userID: tester.M365UserID(suite.T()), - gs: suite.gs, + contactFunc := func(t *testing.T) graph.ContainerResolver { + return &contactFolderCache{ + userID: tester.M365UserID(t), + gs: suite.gs, + } } tests := []struct { name, folderName, root, basePath string - resolver graph.ContainerResolver + resolverFunc func(t *testing.T) graph.ContainerResolver canFind assert.BoolAssertionFunc }{ { - name: "Default Event Cache", - folderName: DefaultCalendar, - root: DefaultCalendar, - basePath: DefaultCalendar, - resolver: &ecc, - canFind: assert.True, + name: "Default Event Cache", + folderName: DefaultCalendar, + root: DefaultCalendar, + basePath: DefaultCalendar, + resolverFunc: eventFunc, + canFind: assert.True, }, { - name: "Default Event Folder Hidden", - root: DefaultCalendar, - folderName: DefaultContactFolder, - canFind: assert.False, - resolver: &ecc, + name: "Default Event Folder Hidden", + root: DefaultCalendar, + folderName: DefaultContactFolder, + canFind: assert.False, + resolverFunc: eventFunc, }, { - name: "Name Not in Cache", - folderName: "testFooBarWhoBar", - root: DefaultCalendar, - canFind: assert.False, - resolver: &ecc, + name: "Name Not in Cache", + folderName: "testFooBarWhoBar", + root: DefaultCalendar, + canFind: assert.False, + resolverFunc: eventFunc, }, { - name: "Default Contact Cache", - folderName: DefaultContactFolder, - root: DefaultContactFolder, - basePath: DefaultContactFolder, - canFind: assert.True, - resolver: &cfc, + name: "Default Contact Cache", + folderName: DefaultContactFolder, + root: DefaultContactFolder, + basePath: DefaultContactFolder, + canFind: assert.True, + resolverFunc: contactFunc, }, { - name: "Default Contact Hidden", - folderName: DefaultContactFolder, - root: DefaultContactFolder, - canFind: assert.False, - resolver: &cfc, + name: "Default Contact Hidden", + folderName: DefaultContactFolder, + root: DefaultContactFolder, + canFind: assert.False, + resolverFunc: contactFunc, }, { - name: "Name Not in Cache", - folderName: "testFooBarWhoBar", - root: DefaultContactFolder, - canFind: assert.False, - resolver: &cfc, + name: "Name Not in Cache", + folderName: "testFooBarWhoBar", + root: DefaultContactFolder, + canFind: assert.False, + resolverFunc: contactFunc, }, } for _, test := range tests { suite.T().Run(test.name, func(t *testing.T) { - require.NoError(t, test.resolver.Populate(ctx, test.root, test.basePath)) - _, isFound := test.resolver.PathInCache(test.folderName) + resolver := test.resolverFunc(t) + + require.NoError(t, resolver.Populate(ctx, test.root, test.basePath)) + _, isFound := resolver.PathInCache(test.folderName) test.canFind(t, isFound) - assert.Greater(t, len(ecc.cache), 0) }) } } diff --git a/src/internal/connector/exchange/mail_folder_cache.go b/src/internal/connector/exchange/mail_folder_cache.go index 884c8a46a..3d2fbdc4c 100644 --- a/src/internal/connector/exchange/mail_folder_cache.go +++ b/src/internal/connector/exchange/mail_folder_cache.go @@ -18,9 +18,9 @@ var _ graph.ContainerResolver = &mailFolderCache{} // cache map of cachedContainers where the key = M365ID // nameLookup map: Key: DisplayName Value: ID type mailFolderCache struct { - cache map[string]graph.CachedContainer - gs graph.Service - userID, rootID string + *containerResolver + gs graph.Service + userID string } // populateMailRoot fetches and populates the "base" directory from user's inbox. @@ -51,19 +51,14 @@ func (mc *mailFolderCache) populateMailRoot( return errors.Wrap(err, "fetching root folder"+support.ConnectorStackErrorTrace(err)) } - // Root only needs the ID because we hide it's name for Mail. - idPtr := f.GetId() - - if idPtr == nil || len(*idPtr) == 0 { - return errors.New("root folder has no ID") - } - temp := cacheFolder{ Container: f, p: path.Builder{}.Append(baseContainerPath...), } - mc.cache[*idPtr] = &temp - mc.rootID = *idPtr + + if err := mc.addFolder(temp); err != nil { + return errors.Wrap(err, "initializing mail resolver") + } return nil } @@ -86,7 +81,7 @@ func (mc *mailFolderCache) Populate( gs. Client(). UsersById(mc.userID). - MailFoldersById(mc.rootID).ChildFolders(). + MailFoldersById(baseID).ChildFolders(). Delta() var errs *multierror.Error @@ -100,7 +95,14 @@ func (mc *mailFolderCache) Populate( } for _, f := range resp.GetValue() { - if err := mc.AddToCache(ctx, f); err != nil { + temp := cacheFolder{ + Container: f, + } + + // Use addFolder instead of AddToCache to be conservative about path + // population. The fetch order of the folders could cause failures while + // trying to resolve paths, so put it off until we've gotten all folders. + if err := mc.addFolder(temp); err != nil { errs = multierror.Append(errs, errors.Wrap(err, "delta fetch")) continue } @@ -117,37 +119,16 @@ func (mc *mailFolderCache) Populate( query = msfolderdelta.NewDeltaRequestBuilder(link, mc.gs.Adapter()) } + if err := mc.populatePaths(ctx); err != nil { + errs = multierror.Append(errs, errors.Wrap(err, "mail resolver")) + } + return errs.ErrorOrNil() } -func (mc *mailFolderCache) IDToPath( - ctx context.Context, - folderID string, -) (*path.Builder, error) { - c, ok := mc.cache[folderID] - if !ok { - return nil, errors.Errorf("folder %s not cached", folderID) - } - - p := c.Path() - if p != nil { - return p, nil - } - - parentPath, err := mc.IDToPath(ctx, *c.GetParentFolderId()) - if err != nil { - return nil, errors.Wrap(err, "retrieving parent folder") - } - - fullPath := parentPath.Append(*c.GetDisplayName()) - c.SetPath(fullPath) - - return fullPath, nil -} - // init ensures that the structure's fields are initialized. // Fields Initialized when cache == nil: -// [mc.cache, mc.rootID] +// [mc.cache] func (mc *mailFolderCache) init( ctx context.Context, baseNode string, @@ -157,64 +138,9 @@ func (mc *mailFolderCache) init( return errors.New("m365 folder ID required for base folder") } - if mc.cache == nil { - mc.cache = map[string]graph.CachedContainer{} + if mc.containerResolver == nil { + mc.containerResolver = newContainerResolver() } return mc.populateMailRoot(ctx, baseNode, baseContainerPath) } - -// AddToCache adds container to map in field 'cache' -// @returns error iff the required values are not accessible. -func (mc *mailFolderCache) AddToCache(ctx context.Context, f graph.Container) error { - if err := checkRequiredValues(f); err != nil { - return errors.Wrap(err, "object not added to cache") - } - - if _, ok := mc.cache[*f.GetId()]; ok { - return nil - } - - mc.cache[*f.GetId()] = &cacheFolder{ - Container: f, - } - - // Populate the path for this entry so calls to PathInCache succeed no matter - // when they're made. - _, err := mc.IDToPath(ctx, *f.GetId()) - if err != nil { - return errors.Wrap(err, "adding cache entry") - } - - return nil -} - -// PathInCache utility function to return m365ID of folder if the pathString -// matches the path of a container within the cache. -func (mc *mailFolderCache) PathInCache(pathString string) (string, bool) { - if len(pathString) == 0 || mc.cache == nil { - return "", false - } - - for _, folder := range mc.cache { - if folder.Path() == nil { - continue - } - - if folder.Path().String() == pathString { - return *folder.GetId(), true - } - } - - return "", false -} - -func (mc *mailFolderCache) Items() []graph.CachedContainer { - res := make([]graph.CachedContainer, 0, len(mc.cache)) - - for _, c := range mc.cache { - res = append(res, c) - } - - return res -} diff --git a/src/internal/connector/exchange/mail_folder_cache_test.go b/src/internal/connector/exchange/mail_folder_cache_test.go index bd0d78e83..6bd4f3d77 100644 --- a/src/internal/connector/exchange/mail_folder_cache_test.go +++ b/src/internal/connector/exchange/mail_folder_cache_test.go @@ -2,17 +2,14 @@ package exchange import ( stdpath "path" - "strings" "testing" - "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/tester" - "github.com/alcionai/corso/src/pkg/path" ) const ( @@ -27,275 +24,6 @@ const ( expectedFolderPath = "toplevel/subFolder/subsubfolder" ) -type mockContainer struct { - id *string - name *string - parentID *string -} - -//nolint:revive -func (m mockContainer) GetId() *string { - return m.id -} - -func (m mockContainer) GetDisplayName() *string { - return m.name -} - -//nolint:revive -func (m mockContainer) GetParentFolderId() *string { - return m.parentID -} - -type MailFolderCacheUnitSuite struct { - suite.Suite -} - -func TestMailFolderCacheUnitSuite(t *testing.T) { - suite.Run(t, new(MailFolderCacheUnitSuite)) -} - -func (suite *MailFolderCacheUnitSuite) TestCheckRequiredValues() { - id := uuid.NewString() - name := "foo" - parentID := uuid.NewString() - emptyString := "" - - table := []struct { - name string - c mockContainer - check assert.ErrorAssertionFunc - }{ - { - name: "NilID", - c: mockContainer{ - id: nil, - name: &name, - parentID: &parentID, - }, - check: assert.Error, - }, - { - name: "NilDisplayName", - c: mockContainer{ - id: &id, - name: nil, - parentID: &parentID, - }, - check: assert.Error, - }, - { - name: "NilParentFolderID", - c: mockContainer{ - id: &id, - name: &name, - parentID: nil, - }, - check: assert.Error, - }, - { - name: "EmptyID", - c: mockContainer{ - id: &emptyString, - name: &name, - parentID: &parentID, - }, - check: assert.Error, - }, - { - name: "EmptyDisplayName", - c: mockContainer{ - id: &id, - name: &emptyString, - parentID: &parentID, - }, - check: assert.Error, - }, - { - name: "EmptyParentFolderID", - c: mockContainer{ - id: &id, - name: &name, - parentID: &emptyString, - }, - check: assert.Error, - }, - { - name: "AllValues", - c: mockContainer{ - id: &id, - name: &name, - parentID: &parentID, - }, - check: assert.NoError, - }, - } - - for _, test := range table { - suite.T().Run(test.name, func(t *testing.T) { - test.check(t, graph.CheckRequiredValues(test.c)) - }) - } -} - -func newMockCachedContainer(name string) *mockCachedContainer { - return &mockCachedContainer{ - id: uuid.NewString(), - parentID: uuid.NewString(), - displayName: name, - } -} - -type mockCachedContainer struct { - id string - parentID string - displayName string - p *path.Builder - expectedPath string -} - -//nolint:revive -func (m mockCachedContainer) GetId() *string { - return &m.id -} - -//nolint:revive -func (m mockCachedContainer) GetParentFolderId() *string { - return &m.parentID -} - -func (m mockCachedContainer) GetDisplayName() *string { - return &m.displayName -} - -func (m mockCachedContainer) Path() *path.Builder { - return m.p -} - -func (m *mockCachedContainer) SetPath(newPath *path.Builder) { - m.p = newPath -} - -// TestConfiguredMailFolderCacheUnitSuite cannot run its tests in parallel. -type ConfiguredMailFolderCacheUnitSuite struct { - suite.Suite - - mc mailFolderCache - - allContainers []*mockCachedContainer -} - -func (suite *ConfiguredMailFolderCacheUnitSuite) SetupTest() { - suite.allContainers = []*mockCachedContainer{} - - for i := 0; i < 4; i++ { - suite.allContainers = append( - suite.allContainers, - newMockCachedContainer(strings.Repeat("sub", i)+"folder"), - ) - } - - // Base case for the recursive lookup. - suite.allContainers[0].p = path.Builder{}.Append(suite.allContainers[0].displayName) - suite.allContainers[0].expectedPath = suite.allContainers[0].displayName - - for i := 1; i < len(suite.allContainers); i++ { - suite.allContainers[i].parentID = suite.allContainers[i-1].id - suite.allContainers[i].expectedPath = stdpath.Join( - suite.allContainers[i-1].expectedPath, - suite.allContainers[i].displayName, - ) - } - - suite.mc = mailFolderCache{cache: map[string]graph.CachedContainer{}} - - for _, c := range suite.allContainers { - suite.mc.cache[c.id] = c - } -} - -func TestConfiguredMailFolderCacheUnitSuite(t *testing.T) { - suite.Run(t, new(ConfiguredMailFolderCacheUnitSuite)) -} - -func (suite *ConfiguredMailFolderCacheUnitSuite) TestLookupCachedFolderNoPathsCached() { - ctx, flush := tester.NewContext() - defer flush() - - for _, c := range suite.allContainers { - suite.T().Run(*c.GetDisplayName(), func(t *testing.T) { - p, err := suite.mc.IDToPath(ctx, c.id) - require.NoError(t, err) - - assert.Equal(t, c.expectedPath, p.String()) - }) - } -} - -func (suite *ConfiguredMailFolderCacheUnitSuite) TestLookupCachedFolderCachesPaths() { - ctx, flush := tester.NewContext() - defer flush() - - t := suite.T() - c := suite.allContainers[len(suite.allContainers)-1] - - p, err := suite.mc.IDToPath(ctx, c.id) - require.NoError(t, err) - - assert.Equal(t, c.expectedPath, p.String()) - - c.parentID = "foo" - - p, err = suite.mc.IDToPath(ctx, c.id) - require.NoError(t, err) - - assert.Equal(t, c.expectedPath, p.String()) -} - -func (suite *ConfiguredMailFolderCacheUnitSuite) TestLookupCachedFolderErrorsParentNotFound() { - ctx, flush := tester.NewContext() - defer flush() - - t := suite.T() - last := suite.allContainers[len(suite.allContainers)-1] - almostLast := suite.allContainers[len(suite.allContainers)-2] - - delete(suite.mc.cache, almostLast.id) - - _, err := suite.mc.IDToPath(ctx, last.id) - assert.Error(t, err) -} - -func (suite *ConfiguredMailFolderCacheUnitSuite) TestLookupCachedFolderErrorsNotFound() { - ctx, flush := tester.NewContext() - defer flush() - - t := suite.T() - - _, err := suite.mc.IDToPath(ctx, "foo") - assert.Error(t, err) -} - -func (suite *ConfiguredMailFolderCacheUnitSuite) TestAddToCache() { - ctx, flush := tester.NewContext() - defer flush() - - t := suite.T() - - last := suite.allContainers[len(suite.allContainers)-1] - - m := newMockCachedContainer("testAddFolder") - - m.parentID = last.id - m.expectedPath = stdpath.Join(last.expectedPath, m.displayName) - - require.NoError(t, suite.mc.AddToCache(ctx, m)) - - p, err := suite.mc.IDToPath(ctx, m.id) - require.NoError(t, err) - assert.Equal(t, m.expectedPath, p.String()) -} - type MailFolderCacheIntegrationSuite struct { suite.Suite gs graph.Service