From 5bcdaef7694a809a8b4539b02b0e20466a4b9893 Mon Sep 17 00:00:00 2001 From: Danny Date: Wed, 5 Oct 2022 16:04:50 -0400 Subject: [PATCH] GC: Interface: Cache Refactor (#1043) ## Description Code changed to support caching on the default folder of `exchange.Mail` as well as an independent node within the Inbox. ## Type of change - [x] :sunflower: Feature ## Issue(s) * related to Issue #1004 ## Test Plan - [x] :zap: Unit test --- .../connector/exchange/mail_folder_cache.go | 100 +++++++++++++----- .../exchange/mail_folder_cache_test.go | 40 +++++-- .../connector/exchange/service_functions.go | 2 +- src/internal/connector/graph/service.go | 7 +- 4 files changed, 111 insertions(+), 38 deletions(-) diff --git a/src/internal/connector/exchange/mail_folder_cache.go b/src/internal/connector/exchange/mail_folder_cache.go index 7fa0b94f5..034bda7c7 100644 --- a/src/internal/connector/exchange/mail_folder_cache.go +++ b/src/internal/connector/exchange/mail_folder_cache.go @@ -4,14 +4,15 @@ import ( "context" multierror "github.com/hashicorp/go-multierror" - "github.com/microsoftgraph/msgraph-sdk-go/models" - msfolderdelta "github.com/microsoftgraph/msgraph-sdk-go/users/item/mailfolders/delta" + msfolderdelta "github.com/microsoftgraph/msgraph-sdk-go/users/item/mailfolders/item/childfolders/delta" "github.com/pkg/errors" "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/pkg/path" ) +var _ cachedContainer = &mailFolder{} + // cachedContainer is used for local unit tests but also makes it so that this // code can be broken into generic- and service-specific chunks later on to // reuse logic in IDToPath. @@ -21,11 +22,16 @@ type cachedContainer interface { SetPath(*path.Builder) } +// mailFolder structure that implements the cachedContainer interface type mailFolder struct { - models.MailFolderable - p *path.Builder + folder container + p *path.Builder } +//========================================= +// Required Functions to satisfy interfaces +//===================================== + func (mf mailFolder) Path() *path.Builder { return mf.p } @@ -34,15 +40,36 @@ func (mf *mailFolder) SetPath(newPath *path.Builder) { mf.p = newPath } -type mailFolderCache struct { - cache map[string]cachedContainer - gs graph.Service - userID string +func (mf *mailFolder) GetDisplayName() *string { + return mf.folder.GetDisplayName() } -// populateRoot fetches and populates the root folder in the cache so the cache -// knows when to stop resolving the path. -func (mc *mailFolderCache) populateRoot(ctx context.Context) error { +//nolint:revive +func (mf *mailFolder) GetId() *string { + return mf.folder.GetId() +} + +//nolint:revive +func (mf *mailFolder) GetParentFolderId() *string { + return mf.folder.GetParentFolderId() +} + +// mailFolderCache struct used to improve lookup of directories within exchange.Mail +// cache map of cachedContainers where the key = M365ID +// nameLookup map: Key: DisplayName Value: ID +type mailFolderCache struct { + cache map[string]cachedContainer + gs graph.Service + userID, rootID string +} + +// populateMailRoot fetches and populates the "base" directory from user's inbox. +// Action ensures that cache will stop at appropriate level. +// @param directory: M365 ID of the root all intended inquiries. +// Function should only be used directly when it is known that all +// folder inquiries are going to a specific node. In all other cases +// @error iff the struct is not properly instantiated +func (mc *mailFolderCache) populateMailRoot(ctx context.Context, directoryID string) error { wantedOpts := []string{"displayName", "parentFolderId"} opts, err := optionsForMailFoldersItem(wantedOpts) @@ -54,7 +81,7 @@ func (mc *mailFolderCache) populateRoot(ctx context.Context) error { gs. Client(). UsersById(mc.userID). - MailFoldersById(rootFolderAlias). + MailFoldersById(directoryID). Get(ctx, opts) if err != nil { return errors.Wrapf(err, "fetching root folder") @@ -62,18 +89,23 @@ func (mc *mailFolderCache) populateRoot(ctx context.Context) error { // 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") } - mc.cache[*idPtr] = &mailFolder{ - MailFolderable: f, - p: &path.Builder{}, + temp := mailFolder{ + folder: f, + p: &path.Builder{}, } + mc.cache[*idPtr] = &temp + mc.rootID = *idPtr return nil } +// checkRequiredValues is a helper function to ensure that +// all the pointers are set prior to being called. func checkRequiredValues(c container) error { idPtr := c.GetId() if idPtr == nil || len(*idPtr) == 0 { @@ -93,26 +125,33 @@ func checkRequiredValues(c container) error { return nil } -func (mc *mailFolderCache) Populate(ctx context.Context) error { - if mc.cache == nil { - mc.cache = map[string]cachedContainer{} +// Populate utility function for populating the mailFolderCache. +// Number of Graph Queries: 1. +// @param baseID: M365ID of the base of the exchange.Mail.Folder +// Use rootFolderAlias for input if baseID unknown +func (mc *mailFolderCache) Populate(ctx context.Context, baseID string) error { + if len(baseID) == 0 { + return errors.New("populate function requires: M365ID as input") } - if err := mc.populateRoot(ctx); err != nil { + err := mc.Init(ctx, baseID) + if err != nil { return err } - builder := mc. + query := mc. gs. Client(). UsersById(mc.userID). - MailFolders(). + MailFoldersById(mc.rootID).ChildFolders(). Delta() var errs *multierror.Error + // TODO: Cannot use Iterator for delta + // Awaiting resolution: https://github.com/microsoftgraph/msgraph-sdk-go/issues/272 for { - resp, err := builder.Get(ctx, nil) + resp, err := query.Get(ctx, nil) if err != nil { return err } @@ -123,7 +162,9 @@ func (mc *mailFolderCache) Populate(ctx context.Context) error { continue } - mc.cache[*f.GetId()] = &mailFolder{MailFolderable: f} + mc.cache[*f.GetId()] = &mailFolder{ + folder: f, + } } r := resp.GetAdditionalData() @@ -134,7 +175,7 @@ func (mc *mailFolderCache) Populate(ctx context.Context) error { } link := *(n.(*string)) - builder = msfolderdelta.NewDeltaRequestBuilder(link, mc.gs.Adapter()) + query = msfolderdelta.NewDeltaRequestBuilder(link, mc.gs.Adapter()) } return errs.ErrorOrNil() @@ -164,3 +205,14 @@ func (mc *mailFolderCache) IDToPath( return fullPath, nil } + +// Init ensures that the structure's fields are initialized. +// Fields Initialized when cache == nil: +// [mc.cache, mc.rootID] +func (mc *mailFolderCache) Init(ctx context.Context, baseNode string) error { + if mc.cache == nil { + mc.cache = map[string]cachedContainer{} + } + + return mc.populateMailRoot(ctx, baseNode) +} diff --git a/src/internal/connector/exchange/mail_folder_cache_test.go b/src/internal/connector/exchange/mail_folder_cache_test.go index 755465627..c3b0671e5 100644 --- a/src/internal/connector/exchange/mail_folder_cache_test.go +++ b/src/internal/connector/exchange/mail_folder_cache_test.go @@ -22,6 +22,8 @@ const ( //nolint:lll testFolderID = "AAMkAGZmNjNlYjI3LWJlZWYtNGI4Mi04YjMyLTIxYThkNGQ4NmY1MwAuAAAAAADCNgjhM9QmQYWNcI7hCpPrAQDSEBNbUIB9RL6ePDeF3FIYAABl7AqpAAA=" + //nolint:lll + topFolderID = "AAMkAGZmNjNlYjI3LWJlZWYtNGI4Mi04YjMyLTIxYThkNGQ4NmY1MwAuAAAAAADCNgjhM9QmQYWNcI7hCpPrAQDSEBNbUIB9RL6ePDeF3FIYAAAAAAEIAAA=" // Full folder path for the folder above. expectedFolderPath = "toplevel/subFolder/subsubfolder" ) @@ -303,19 +305,35 @@ func TestMailFolderCacheIntegrationSuite(t *testing.T) { } func (suite *MailFolderCacheIntegrationSuite) TestDeltaFetch() { - ctx := context.Background() - t := suite.T() - userID := tester.M365UserID(t) - - mfc := mailFolderCache{ - userID: userID, - gs: suite.gs, + tests := []struct { + name string + root string + }{ + { + name: "Default Root", + root: rootFolderAlias, + }, + { + name: "Node Root", + root: topFolderID, + }, } + ctx := context.Background() + userID := tester.M365UserID(suite.T()) - require.NoError(t, mfc.Populate(ctx)) + for _, test := range tests { + suite.T().Run(test.name, func(t *testing.T) { + mfc := mailFolderCache{ + userID: userID, + gs: suite.gs, + } - p, err := mfc.IDToPath(ctx, testFolderID) - require.NoError(t, err) + require.NoError(t, mfc.Populate(ctx, test.root)) - assert.Equal(t, expectedFolderPath, p.String()) + p, err := mfc.IDToPath(ctx, testFolderID) + require.NoError(t, err) + + assert.Equal(t, expectedFolderPath, p.String()) + }) + } } diff --git a/src/internal/connector/exchange/service_functions.go b/src/internal/connector/exchange/service_functions.go index f10b2ace8..5d8a60332 100644 --- a/src/internal/connector/exchange/service_functions.go +++ b/src/internal/connector/exchange/service_functions.go @@ -395,7 +395,7 @@ func maybeGetAndPopulateFolderResolver( return nil, nil } - if err := res.Populate(ctx); err != nil { + if err := res.Populate(ctx, rootFolderAlias); err != nil { return nil, errors.Wrap(err, "populating directory resolver") } diff --git a/src/internal/connector/graph/service.go b/src/internal/connector/graph/service.go index a9cf9cf26..41e213e6d 100644 --- a/src/internal/connector/graph/service.go +++ b/src/internal/connector/graph/service.go @@ -36,6 +36,9 @@ type ContainerResolver interface { // to that container. The path has a similar format to paths on the local // file system. IDToPath(ctx context.Context, m365ID string) (*path.Builder, error) - // Populate performs any setup logic the resolver may need. - Populate(context.Context) error + // Populate performs initialization steps for the resolver + // @param ctx is necessary param for Graph API tracing + // @param baseFolderID represents the M365ID base that the resolver will + // conclude its search. Default input is "". + Populate(ctx context.Context, baseFolderID string) error }