Consolidate container resolver code (#1365)

## Description

As the exchange container resolvers are pretty close implementation-wise, consolidate the generic code. Also move some of the tests around to better fit what file has the code being tested

## Type of change

<!--- Please check the type of change your PR introduces: --->
- [x] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [ ] 🐹 Trivial/Minor

## Issue(s)

* closes #1362 

## Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E

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
+	*containerResolver
 	gs     graph.Service
-	userID, rootID string
+	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 {
+	if cfc.containerResolver == nil {
-		cfc.cache = map[string]graph.CachedContainer{}
+		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
-}

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()
+}

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())
+}

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
+	*containerResolver
 	gs     graph.Service
-	userID, rootID string
+	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 {
+	if ecc.containerResolver == nil {
-		ecc.cache = map[string]graph.CachedContainer{}
+		ecc.containerResolver = newContainerResolver()
 	}
 
 	options, err := optionsForCalendars([]string{"name"})
@@ -75,10 +75,25 @@ func (ecc *eventCalendarCache) Populate(
 		return err
 	}
 
-	for _, containerr := range directories {
+	for _, container := range directories {
-		if err := ecc.AddToCache(ctx, containerr); err != nil {
+		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(
+// AddToCache adds container to map in field 'cache'
-	ctx context.Context,
+// @returns error iff the required values are not accessible.
-	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.
 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 {
+	temp := cacheFolder{
-		return nil
-	}
-
-	ecc.cache[*f.GetId()] = &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
-}

src/internal/connector/exchange/folder_resolver_test.go

@@ -50,19 +50,23 @@ func (suite *CacheResolverSuite) TestPopulate() {
 	ctx, flush := tester.NewContext()
 	defer flush()
 
-	ecc := eventCalendarCache{
+	eventFunc := func(t *testing.T) graph.ContainerResolver {
-		userID: tester.M365UserID(suite.T()),
+		return &eventCalendarCache{
+			userID: tester.M365UserID(t),
 			gs:     suite.gs,
 		}
+	}
 
-	cfc := contactFolderCache{
+	contactFunc := func(t *testing.T) graph.ContainerResolver {
-		userID: tester.M365UserID(suite.T()),
+		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
 	}{
 		{
@@ -70,7 +74,7 @@ func (suite *CacheResolverSuite) TestPopulate() {
 			folderName:   DefaultCalendar,
 			root:         DefaultCalendar,
 			basePath:     DefaultCalendar,
-			resolver:   &ecc,
+			resolverFunc: eventFunc,
 			canFind:      assert.True,
 		},
 		{
@@ -78,14 +82,14 @@ func (suite *CacheResolverSuite) TestPopulate() {
 			root:         DefaultCalendar,
 			folderName:   DefaultContactFolder,
 			canFind:      assert.False,
-			resolver:   &ecc,
+			resolverFunc: eventFunc,
 		},
 		{
 			name:         "Name Not in Cache",
 			folderName:   "testFooBarWhoBar",
 			root:         DefaultCalendar,
 			canFind:      assert.False,
-			resolver:   &ecc,
+			resolverFunc: eventFunc,
 		},
 		{
 			name:         "Default Contact Cache",
@@ -93,29 +97,30 @@ func (suite *CacheResolverSuite) TestPopulate() {
 			root:         DefaultContactFolder,
 			basePath:     DefaultContactFolder,
 			canFind:      assert.True,
-			resolver:   &cfc,
+			resolverFunc: contactFunc,
 		},
 		{
 			name:         "Default Contact Hidden",
 			folderName:   DefaultContactFolder,
 			root:         DefaultContactFolder,
 			canFind:      assert.False,
-			resolver:   &cfc,
+			resolverFunc: contactFunc,
 		},
 		{
 			name:         "Name Not in Cache",
 			folderName:   "testFooBarWhoBar",
 			root:         DefaultContactFolder,
 			canFind:      assert.False,
-			resolver:   &cfc,
+			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))
+			resolver := test.resolverFunc(t)
-			_, isFound := test.resolver.PathInCache(test.folderName)
+
+			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)
 		})
 	}
 }

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
+	*containerResolver
 	gs     graph.Service
-	userID, rootID string
+	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 {
+	if mc.containerResolver == nil {
-		mc.cache = map[string]graph.CachedContainer{}
+		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
-}

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