From 92e109d4eb3eda30dbbea10ae25b3c2caa36bce8 Mon Sep 17 00:00:00 2001 From: ryanfkeepers Date: Mon, 14 Nov 2022 16:09:03 -0700 Subject: [PATCH] extract scope from qp Since scopes are service specific, we cannot easily house them within the graph QueryParam struct, unless we bloat the struct with all types. Alternatively, we could add a generic "scope" with parsers, much like the Selector itself. But really, the most simple solution is to only pass the scope within the tree of service funcs that use it. --- src/cmd/purge/purge.go | 26 ++--- src/internal/connector/data_collections.go | 45 +++----- .../connector/exchange/service_functions.go | 60 +++++------ .../exchange/service_functions_test.go | 102 ++++-------------- .../connector/exchange/service_iterators.go | 19 ++-- src/internal/connector/graph/service.go | 9 +- .../connector/sharepoint/service_iterators.go | 2 + 7 files changed, 88 insertions(+), 175 deletions(-) diff --git a/src/cmd/purge/purge.go b/src/cmd/purge/purge.go index b451fb398..546d9b099 100644 --- a/src/cmd/purge/purge.go +++ b/src/cmd/purge/purge.go @@ -240,12 +240,12 @@ func purgeMailFolders( uid string, ) error { getter := func(gs graph.Service, uid, prefix string) ([]purgable, error) { - params, err := exchangeQueryParamFactory(uid, path.EmailCategory) + params, scope, err := exchangeQueryParamFactory(uid, path.EmailCategory) if err != nil { return nil, err } - allFolders, err := exchange.GetAllMailFolders(ctx, *params, gs) + allFolders, err := exchange.GetAllMailFolders(ctx, *params, gs, scope) if err != nil { return nil, err } @@ -276,12 +276,12 @@ func purgeCalendarFolders( uid string, ) error { getter := func(gs graph.Service, uid, prefix string) ([]purgable, error) { - params, err := exchangeQueryParamFactory(uid, path.EventsCategory) + params, scope, err := exchangeQueryParamFactory(uid, path.EventsCategory) if err != nil { return nil, err } - allCalendars, err := exchange.GetAllCalendars(ctx, *params, gs) + allCalendars, err := exchange.GetAllCalendars(ctx, *params, gs, scope) if err != nil { return nil, err } @@ -312,12 +312,12 @@ func purgeContactFolders( uid string, ) error { getter := func(gs graph.Service, uid, prefix string) ([]purgable, error) { - params, err := exchangeQueryParamFactory(uid, path.ContactsCategory) + params, scope, err := exchangeQueryParamFactory(uid, path.ContactsCategory) if err != nil { return nil, err } - allContainers, err := exchange.GetAllContactFolders(ctx, *params, gs) + allContainers, err := exchange.GetAllContactFolders(ctx, *params, gs, scope) if err != nil { return nil, err } @@ -518,7 +518,10 @@ func containerFilter(nameContains string, containers []graph.CachedContainer) [] return result } -func exchangeQueryParamFactory(user string, category path.CategoryType) (*graph.QueryParams, error) { +func exchangeQueryParamFactory( + user string, + category path.CategoryType, +) (*graph.QueryParams, selectors.ExchangeScope, error) { var scope selectors.ExchangeScope switch category { @@ -529,18 +532,17 @@ func exchangeQueryParamFactory(user string, category path.CategoryType) (*graph. case path.EventsCategory: scope = selectors.NewExchangeBackup().EventCalendars([]string{user}, selectors.Any())[0] default: - return nil, fmt.Errorf("category %s not supported", category) + return nil, scope, fmt.Errorf("category %s not supported", category) } params := &graph.QueryParams{ - User: user, - Scope: scope, - FailFast: false, + ResourceOwner: user, + FailFast: false, Credentials: account.M365Config{ M365: credentials.GetM365(), AzureTenantID: common.First(tenant, os.Getenv(account.AzureTenantID)), }, } - return params, nil + return params, scope, nil } diff --git a/src/internal/connector/data_collections.go b/src/internal/connector/data_collections.go index 8c7606ba4..62bcef8f4 100644 --- a/src/internal/connector/data_collections.go +++ b/src/internal/connector/data_collections.go @@ -114,23 +114,17 @@ func (gc *GraphConnector) createExchangeCollections( collections := make(map[string]*exchange.Collection) qp := graph.QueryParams{ - User: user, - Scope: scope, - FailFast: gc.failFast, - Credentials: gc.credentials, + Category: scope.Category().PathType(), + ResourceOwner: user, + FailFast: gc.failFast, + Credentials: gc.credentials, } - itemCategory := qp.Scope.Category().PathType() - - foldersComplete, closer := observe.MessageWithCompletion(fmt.Sprintf("∙ %s - %s:", itemCategory.String(), user)) + foldersComplete, closer := observe.MessageWithCompletion(fmt.Sprintf("∙ %s - %s:", qp.Category, user)) defer closer() defer close(foldersComplete) - resolver, err := exchange.PopulateExchangeContainerResolver( - ctx, - qp, - qp.Scope.Category().PathType(), - ) + resolver, err := exchange.PopulateExchangeContainerResolver(ctx, qp) if err != nil { return nil, errors.Wrap(err, "getting folder cache") } @@ -140,7 +134,8 @@ func (gc *GraphConnector) createExchangeCollections( qp, collections, gc.UpdateStatus, - resolver) + resolver, + scope) if err != nil { return nil, errors.Wrap(err, "filling collections") @@ -264,26 +259,17 @@ func (gc *GraphConnector) createSharePointCollections( collections := make(map[string]*sharepoint.Collection) qp := graph.QueryParams{ - // TODO: Resource owner, not user/site. - User: site, - // TODO: generic scope handling in query params. - // - or, break scope out of QP. - // Scope: scope, - FailFast: gc.failFast, - Credentials: gc.credentials, + Category: scope.Category().PathType(), + ResourceOwner: site, + FailFast: gc.failFast, + Credentials: gc.credentials, } - itemCategory := qp.Scope.Category().PathType() - - foldersComplete, closer := observe.MessageWithCompletion(fmt.Sprintf("∙ %s - %s:", itemCategory.String(), site)) + foldersComplete, closer := observe.MessageWithCompletion(fmt.Sprintf("∙ %s - %s:", qp.Category, site)) defer closer() defer close(foldersComplete) - resolver, err := exchange.PopulateExchangeContainerResolver( - ctx, - qp, - qp.Scope.Category().PathType(), - ) + resolver, err := exchange.PopulateExchangeContainerResolver(ctx, qp) if err != nil { return nil, errors.Wrap(err, "getting folder cache") } @@ -293,7 +279,8 @@ func (gc *GraphConnector) createSharePointCollections( qp, collections, gc.UpdateStatus, - resolver) + resolver, + scope) if err != nil { return nil, errors.Wrap(err, "filling collections") diff --git a/src/internal/connector/exchange/service_functions.go b/src/internal/connector/exchange/service_functions.go index 136cb1e8d..8b97973d4 100644 --- a/src/internal/connector/exchange/service_functions.go +++ b/src/internal/connector/exchange/service_functions.go @@ -137,10 +137,11 @@ func GetAllMailFolders( ctx context.Context, qp graph.QueryParams, gs graph.Service, + scope selectors.ExchangeScope, ) ([]graph.CachedContainer, error) { containers := make([]graph.CachedContainer, 0) - resolver, err := PopulateExchangeContainerResolver(ctx, qp, path.EmailCategory) + resolver, err := PopulateExchangeContainerResolver(ctx, qp) if err != nil { return nil, errors.Wrap(err, "building directory resolver in GetAllMailFolders") } @@ -151,7 +152,7 @@ func GetAllMailFolders( continue } - if qp.Scope.Matches(selectors.ExchangeMailFolder, directory) { + if scope.Matches(selectors.ExchangeMailFolder, directory) { containers = append(containers, c) } } @@ -166,10 +167,11 @@ func GetAllCalendars( ctx context.Context, qp graph.QueryParams, gs graph.Service, + scope selectors.ExchangeScope, ) ([]graph.CachedContainer, error) { containers := make([]graph.CachedContainer, 0) - resolver, err := PopulateExchangeContainerResolver(ctx, qp, path.EventsCategory) + resolver, err := PopulateExchangeContainerResolver(ctx, qp) if err != nil { return nil, errors.Wrap(err, "building calendar resolver in GetAllCalendars") } @@ -177,7 +179,7 @@ func GetAllCalendars( for _, c := range resolver.Items() { directory := c.Path().String() - if qp.Scope.Matches(selectors.ExchangeEventCalendar, directory) { + if scope.Matches(selectors.ExchangeEventCalendar, directory) { containers = append(containers, c) } } @@ -193,12 +195,13 @@ func GetAllContactFolders( ctx context.Context, qp graph.QueryParams, gs graph.Service, + scope selectors.ExchangeScope, ) ([]graph.CachedContainer, error) { var query string containers := make([]graph.CachedContainer, 0) - resolver, err := PopulateExchangeContainerResolver(ctx, qp, path.ContactsCategory) + resolver, err := PopulateExchangeContainerResolver(ctx, qp) if err != nil { return nil, errors.Wrap(err, "building directory resolver in GetAllContactFolders") } @@ -212,7 +215,7 @@ func GetAllContactFolders( query = directory } - if qp.Scope.Matches(selectors.ExchangeContactFolder, query) { + if scope.Matches(selectors.ExchangeContactFolder, query) { containers = append(containers, c) } } @@ -220,25 +223,6 @@ func GetAllContactFolders( return containers, err } -func GetContainers( - ctx context.Context, - qp graph.QueryParams, - gs graph.Service, -) ([]graph.CachedContainer, error) { - category := qp.Scope.Category().PathType() - - switch category { - case path.ContactsCategory: - return GetAllContactFolders(ctx, qp, gs) - case path.EmailCategory: - return GetAllMailFolders(ctx, qp, gs) - case path.EventsCategory: - return GetAllCalendars(ctx, qp, gs) - default: - return nil, fmt.Errorf("path.Category %s not supported", category) - } -} - // PopulateExchangeContainerResolver gets a folder resolver if one is available for // this category of data. If one is not available, returns nil so that other // logic in the caller can complete as long as they check if the resolver is not @@ -246,7 +230,6 @@ func GetContainers( func PopulateExchangeContainerResolver( ctx context.Context, qp graph.QueryParams, - category path.CategoryType, ) (graph.ContainerResolver, error) { var ( res graph.ContainerResolver @@ -258,30 +241,30 @@ func PopulateExchangeContainerResolver( return nil, err } - switch category { + switch qp.Category { case path.EmailCategory: res = &mailFolderCache{ - userID: qp.User, + userID: qp.ResourceOwner, gs: service, } cacheRoot = rootFolderAlias case path.ContactsCategory: res = &contactFolderCache{ - userID: qp.User, + userID: qp.ResourceOwner, gs: service, } cacheRoot = DefaultContactFolder case path.EventsCategory: res = &eventCalendarCache{ - userID: qp.User, + userID: qp.ResourceOwner, gs: service, } cacheRoot = DefaultCalendar default: - return nil, fmt.Errorf("ContainerResolver not present for %s type", category) + return nil, fmt.Errorf("ContainerResolver not present for %s type", qp.Category) } if err := res.Populate(ctx, cacheRoot); err != nil { @@ -291,8 +274,13 @@ func PopulateExchangeContainerResolver( return res, nil } -func pathAndMatch(qp graph.QueryParams, category path.CategoryType, c graph.CachedContainer) (path.Path, bool) { +func pathAndMatch( + qp graph.QueryParams, + c graph.CachedContainer, + scope selectors.ExchangeScope, +) (path.Path, bool) { var ( + category = scope.Category().PathType() directory string pb = c.Path() ) @@ -304,7 +292,7 @@ func pathAndMatch(qp graph.QueryParams, category path.CategoryType, c graph.Cach dirPath, err := pb.ToDataLayerExchangePathForCategory( qp.Credentials.AzureTenantID, - qp.User, + qp.ResourceOwner, category, false, ) @@ -320,11 +308,11 @@ func pathAndMatch(qp graph.QueryParams, category path.CategoryType, c graph.Cach switch category { case path.EmailCategory: - return dirPath, qp.Scope.Matches(selectors.ExchangeMailFolder, directory) + return dirPath, scope.Matches(selectors.ExchangeMailFolder, directory) case path.ContactsCategory: - return dirPath, qp.Scope.Matches(selectors.ExchangeContactFolder, directory) + return dirPath, scope.Matches(selectors.ExchangeContactFolder, directory) case path.EventsCategory: - return dirPath, qp.Scope.Matches(selectors.ExchangeEventCalendar, directory) + return dirPath, scope.Matches(selectors.ExchangeEventCalendar, directory) default: return nil, false } diff --git a/src/internal/connector/exchange/service_functions_test.go b/src/internal/connector/exchange/service_functions_test.go index 649f36163..962a7d575 100644 --- a/src/internal/connector/exchange/service_functions_test.go +++ b/src/internal/connector/exchange/service_functions_test.go @@ -115,13 +115,14 @@ func (suite *ServiceFunctionsIntegrationSuite) TestGetAllCalendars() { } for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { + scope := test.getScope(t) params := graph.QueryParams{ - User: test.user, - Scope: test.getScope(t), - FailFast: false, - Credentials: suite.creds, + Category: scope.Category().PathType(), + ResourceOwner: test.user, + FailFast: false, + Credentials: suite.creds, } - cals, err := GetAllCalendars(ctx, params, gs) + cals, err := GetAllCalendars(ctx, params, gs, scope) test.expectErr(t, err) test.expectCount(t, len(cals), 0) }) @@ -199,13 +200,14 @@ func (suite *ServiceFunctionsIntegrationSuite) TestGetAllContactFolders() { } for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { + scope := test.getScope(t) params := graph.QueryParams{ - User: test.user, - Scope: test.getScope(t), - FailFast: false, - Credentials: suite.creds, + Category: scope.Category().PathType(), + ResourceOwner: test.user, + FailFast: false, + Credentials: suite.creds, } - cals, err := GetAllContactFolders(ctx, params, gs) + cals, err := GetAllContactFolders(ctx, params, gs, scope) test.expectErr(t, err) test.expectCount(t, len(cals), 0) }) @@ -283,84 +285,16 @@ func (suite *ServiceFunctionsIntegrationSuite) TestGetAllMailFolders() { } for _, test := range table { suite.T().Run(test.name, func(t *testing.T) { + scope := test.getScope(t) params := graph.QueryParams{ - User: test.user, - Scope: test.getScope(t), - FailFast: false, - Credentials: suite.creds, + Category: scope.Category().PathType(), + ResourceOwner: test.user, + FailFast: false, + Credentials: suite.creds, } - cals, err := GetAllMailFolders(ctx, params, gs) + cals, err := GetAllMailFolders(ctx, params, gs, scope) test.expectErr(t, err) test.expectCount(t, len(cals), 0) }) } } - -func (suite *ServiceFunctionsIntegrationSuite) TestCollectContainers() { - ctx, flush := tester.NewContext() - defer flush() - - failFast := false - containerCount := 1 - t := suite.T() - user := tester.M365UserID(t) - a := tester.NewM365Account(t) - service := loadService(t) - credentials, err := a.M365Config() - require.NoError(t, err) - - tests := []struct { - name, contains string - getScope func() selectors.ExchangeScope - expectedCount assert.ComparisonAssertionFunc - }{ - { - name: "All Events", - contains: "Birthdays", - expectedCount: assert.Greater, - getScope: func() selectors.ExchangeScope { - return selectors. - NewExchangeBackup(). - EventCalendars([]string{user}, selectors.Any())[0] - }, - }, { - name: "Default Calendar", - contains: DefaultCalendar, - expectedCount: assert.Equal, - getScope: func() selectors.ExchangeScope { - return selectors. - NewExchangeBackup(). - EventCalendars([]string{user}, []string{DefaultCalendar}, selectors.PrefixMatch())[0] - }, - }, { - name: "Default Mail", - contains: DefaultMailFolder, - expectedCount: assert.Equal, - getScope: func() selectors.ExchangeScope { - return selectors. - NewExchangeBackup(). - MailFolders([]string{user}, []string{DefaultMailFolder}, selectors.PrefixMatch())[0] - }, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - qp := graph.QueryParams{ - User: user, - Scope: test.getScope(), - FailFast: failFast, - Credentials: credentials, - } - collections, err := GetContainers(ctx, qp, service) - assert.NoError(t, err) - test.expectedCount(t, len(collections), containerCount) - - keys := make([]string, 0, len(collections)) - for _, k := range collections { - keys = append(keys, *k.GetDisplayName()) - } - assert.Contains(t, keys, test.contains) - }) - } -} diff --git a/src/internal/connector/exchange/service_iterators.go b/src/internal/connector/exchange/service_iterators.go index 5ce694ba3..f3d8f0b41 100644 --- a/src/internal/connector/exchange/service_iterators.go +++ b/src/internal/connector/exchange/service_iterators.go @@ -13,6 +13,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/pkg/path" + "github.com/alcionai/corso/src/pkg/selectors" ) // FilterContainersAndFillCollections is a utility function @@ -26,21 +27,21 @@ func FilterContainersAndFillCollections( collections map[string]*Collection, statusUpdater support.StatusUpdater, resolver graph.ContainerResolver, + scope selectors.ExchangeScope, ) error { var ( - category = qp.Scope.Category().PathType() - collectionType = CategoryToOptionIdentifier(category) + collectionType = CategoryToOptionIdentifier(scope.Category().PathType()) errs error ) for _, c := range resolver.Items() { - dirPath, ok := pathAndMatch(qp, category, c) + dirPath, ok := pathAndMatch(qp, c, scope) if ok { // Create only those that match service, err := createService(qp.Credentials, qp.FailFast) if err != nil { errs = support.WrapAndAppend( - qp.User+" FilterContainerAndFillCollection", + qp.ResourceOwner+" FilterContainerAndFillCollection", err, errs) @@ -50,7 +51,7 @@ func FilterContainersAndFillCollections( } edc := NewCollection( - qp.User, + qp.ResourceOwner, dirPath, collectionType, service, @@ -61,10 +62,10 @@ func FilterContainersAndFillCollections( } for directoryID, col := range collections { - fetchFunc, err := getFetchIDFunc(category) + fetchFunc, err := getFetchIDFunc(scope.Category().PathType()) if err != nil { errs = support.WrapAndAppend( - qp.User, + qp.ResourceOwner, err, errs) @@ -75,10 +76,10 @@ func FilterContainersAndFillCollections( continue } - jobs, err := fetchFunc(ctx, col.service, qp.User, directoryID) + jobs, err := fetchFunc(ctx, col.service, qp.ResourceOwner, directoryID) if err != nil { errs = support.WrapAndAppend( - qp.User, + qp.ResourceOwner, err, errs, ) diff --git a/src/internal/connector/graph/service.go b/src/internal/connector/graph/service.go index 0ecdfb7af..7281dbe94 100644 --- a/src/internal/connector/graph/service.go +++ b/src/internal/connector/graph/service.go @@ -7,14 +7,13 @@ import ( "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/path" - "github.com/alcionai/corso/src/pkg/selectors" ) type QueryParams struct { - User string - Scope selectors.ExchangeScope - Credentials account.M365Config - FailFast bool + Category path.CategoryType + ResourceOwner string + Credentials account.M365Config + FailFast bool } type Service interface { diff --git a/src/internal/connector/sharepoint/service_iterators.go b/src/internal/connector/sharepoint/service_iterators.go index a1f682c26..d7fa23484 100644 --- a/src/internal/connector/sharepoint/service_iterators.go +++ b/src/internal/connector/sharepoint/service_iterators.go @@ -5,6 +5,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/support" + "github.com/alcionai/corso/src/pkg/selectors" ) // FilterContainersAndFillCollections is a utility function @@ -17,6 +18,7 @@ func FilterContainersAndFillCollections( collections map[string]*Collection, statusUpdater support.StatusUpdater, resolver graph.ContainerResolver, + scope selectors.SharePointScope, ) error { return nil }