From e2775aeb9517a54c313ba4bfc2b3641c88b233e5 Mon Sep 17 00:00:00 2001 From: Keepers Date: Tue, 13 Dec 2022 16:06:27 -0700 Subject: [PATCH] Refactor service failfast (#1789) ## Description Configuration and attenion to the graphService failFast is haphazard and has shared ownership. This change removes that property from the service, along with the ErrPolicy func, in favor of passing around a control.Options struct. ## Type of change - [x] :hamster: Trivial/Minor ## Issue(s) * #1725 * #302 ## Test Plan - [x] :zap: Unit test --- src/cmd/getM365/getItem.go | 4 +-- src/internal/connector/data_collections.go | 27 ++++++++++++++----- .../connector/data_collections_test.go | 25 +++++++++-------- .../exchange/exchange_data_collection.go | 7 ++++- .../exchange/exchange_service_test.go | 4 +-- .../exchange/folder_resolver_test.go | 2 +- .../connector/exchange/iterators_test.go | 2 +- .../exchange/mail_folder_cache_test.go | 2 +- .../connector/exchange/service_functions.go | 10 ++----- .../connector/exchange/service_iterators.go | 9 ++++--- src/internal/connector/graph/service.go | 3 --- src/internal/connector/graph_connector.go | 15 ++++------- .../connector/graph_connector_test.go | 11 ++++---- src/internal/connector/onedrive/collection.go | 6 ++++- .../connector/onedrive/collection_test.go | 21 ++++++++++----- .../connector/onedrive/collections.go | 6 +++++ .../connector/onedrive/collections_test.go | 10 ++++++- src/internal/connector/onedrive/drive_test.go | 2 ++ src/internal/connector/onedrive/item_test.go | 4 --- .../connector/onedrive/service_test.go | 8 ------ .../connector/sharepoint/data_collections.go | 19 +++++++------ .../sharepoint/data_collections_test.go | 4 ++- .../connector/sharepoint/helper_test.go | 8 ------ src/internal/operations/backup.go | 5 ++-- 24 files changed, 117 insertions(+), 97 deletions(-) diff --git a/src/cmd/getM365/getItem.go b/src/cmd/getM365/getItem.go index 32d291d2b..8d4f66c2c 100644 --- a/src/cmd/getM365/getItem.go +++ b/src/cmd/getM365/getItem.go @@ -81,9 +81,7 @@ func handleGetCommand(cmd *cobra.Command, args []string) error { return err } - err = runDisplayM365JSON( - ctx, - gc) + err = runDisplayM365JSON(ctx, gc.Service()) if err != nil { return Only(ctx, errors.Wrapf(err, "unable to create mock from M365: %s", m365ID)) } diff --git a/src/internal/connector/data_collections.go b/src/internal/connector/data_collections.go index 792fe1373..31fa8db27 100644 --- a/src/internal/connector/data_collections.go +++ b/src/internal/connector/data_collections.go @@ -16,6 +16,7 @@ import ( "github.com/alcionai/corso/src/internal/data" D "github.com/alcionai/corso/src/internal/diagnostics" "github.com/alcionai/corso/src/internal/observe" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -33,6 +34,7 @@ func (gc *GraphConnector) DataCollections( ctx context.Context, sels selectors.Selector, metadata []data.Collection, + ctrlOpts control.Options, ) ([]data.Collection, error) { ctx, end := D.Span(ctx, "gc:dataCollections", D.Index("service", sels.Service.String())) defer end() @@ -44,11 +46,18 @@ func (gc *GraphConnector) DataCollections( switch sels.Service { case selectors.ServiceExchange: - return gc.ExchangeDataCollection(ctx, sels, metadata) + return gc.ExchangeDataCollection(ctx, sels, metadata, ctrlOpts) case selectors.ServiceOneDrive: - return gc.OneDriveDataCollections(ctx, sels) + return gc.OneDriveDataCollections(ctx, sels, ctrlOpts) case selectors.ServiceSharePoint: - colls, err := sharepoint.DataCollections(ctx, sels, gc.GetSiteIDs(), gc.credentials.AzureTenantID, gc) + colls, err := sharepoint.DataCollections( + ctx, + sels, + gc.GetSiteIDs(), + gc.credentials.AzureTenantID, + gc, + gc, + ctrlOpts) if err != nil { return nil, err } @@ -118,6 +127,7 @@ func (gc *GraphConnector) createExchangeCollections( ctx context.Context, scope selectors.ExchangeScope, deltas map[string]string, + ctrlOpts control.Options, ) ([]data.Collection, error) { var ( errs *multierror.Error @@ -132,7 +142,6 @@ func (gc *GraphConnector) createExchangeCollections( qp := graph.QueryParams{ Category: scope.Category().PathType(), ResourceOwner: user, - FailFast: gc.failFast, Credentials: gc.credentials, } @@ -152,7 +161,8 @@ func (gc *GraphConnector) createExchangeCollections( gc.UpdateStatus, resolver, scope, - deltas) + deltas, + ctrlOpts) if err != nil { return nil, errors.Wrap(err, "filling collections") @@ -179,6 +189,7 @@ func (gc *GraphConnector) ExchangeDataCollection( ctx context.Context, selector selectors.Selector, metadata []data.Collection, + ctrlOpts control.Options, ) ([]data.Collection, error) { eb, err := selector.ToExchangeBackup() if err != nil { @@ -198,7 +209,7 @@ func (gc *GraphConnector) ExchangeDataCollection( for _, scope := range scopes { // Creates a map of collections based on scope - dcs, err := gc.createExchangeCollections(ctx, scope, deltas) + dcs, err := gc.createExchangeCollections(ctx, scope, deltas, control.Options{}) if err != nil { user := scope.Get(selectors.ExchangeUser) return nil, support.WrapAndAppend(user[0], err, errs) @@ -231,6 +242,7 @@ func (fm odFolderMatcher) Matches(dir string) bool { func (gc *GraphConnector) OneDriveDataCollections( ctx context.Context, selector selectors.Selector, + ctrlOpts control.Options, ) ([]data.Collection, error) { odb, err := selector.ToOneDriveBackup() if err != nil { @@ -253,8 +265,9 @@ func (gc *GraphConnector) OneDriveDataCollections( user, onedrive.OneDriveSource, odFolderMatcher{scope}, - &gc.graphService, + gc, gc.UpdateStatus, + ctrlOpts, ).Get(ctx) if err != nil { return nil, support.WrapAndAppend(user, err, errs) diff --git a/src/internal/connector/data_collections_test.go b/src/internal/connector/data_collections_test.go index 81175888b..d5c4e1dfb 100644 --- a/src/internal/connector/data_collections_test.go +++ b/src/internal/connector/data_collections_test.go @@ -13,6 +13,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -104,7 +105,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestExchangeDataCollection for _, test := range tests { suite.T().Run(test.name, func(t *testing.T) { - collection, err := connector.ExchangeDataCollection(ctx, test.getSelector(t), nil) + collection, err := connector.ExchangeDataCollection(ctx, test.getSelector(t), nil, control.Options{}) require.NoError(t, err) // Categories with delta endpoints will produce a collection for metadata // as well as the actual data pulled. @@ -157,7 +158,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestInvalidUserForDataColl for _, test := range tests { suite.T().Run(test.name, func(t *testing.T) { - collections, err := connector.DataCollections(ctx, test.getSelector(t), nil) + collections, err := connector.DataCollections(ctx, test.getSelector(t), nil, control.Options{}) assert.Error(t, err) assert.Empty(t, collections) }) @@ -194,7 +195,9 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestSharePointDataCollecti test.getSelector(t), []string{suite.site}, connector.credentials.AzureTenantID, - connector) + connector, + connector, + control.Options{}) require.NoError(t, err) // we don't know an exact count of drives this will produce, @@ -283,7 +286,7 @@ func (suite *ConnectorCreateExchangeCollectionIntegrationSuite) TestMailFetch() for _, test := range tests { suite.T().Run(test.name, func(t *testing.T) { - collections, err := gc.createExchangeCollections(ctx, test.scope, nil) + collections, err := gc.createExchangeCollections(ctx, test.scope, nil, control.Options{}) require.NoError(t, err) for _, c := range collections { @@ -335,7 +338,7 @@ func (suite *ConnectorCreateExchangeCollectionIntegrationSuite) TestDelta() { for _, test := range tests { suite.T().Run(test.name, func(t *testing.T) { // get collections without providing any delta history (ie: full backup) - collections, err := gc.createExchangeCollections(ctx, test.scope, nil) + collections, err := gc.createExchangeCollections(ctx, test.scope, nil, control.Options{}) require.NoError(t, err) assert.Less(t, 1, len(collections), "retrieved metadata and data collections") @@ -354,7 +357,7 @@ func (suite *ConnectorCreateExchangeCollectionIntegrationSuite) TestDelta() { // now do another backup with the previous delta tokens, // which should only contain the difference. - collections, err = gc.createExchangeCollections(ctx, test.scope, deltas) + collections, err = gc.createExchangeCollections(ctx, test.scope, deltas, control.Options{}) require.NoError(t, err) // TODO(keepers): this isn't a very useful test at the moment. It needs to @@ -384,7 +387,7 @@ func (suite *ConnectorCreateExchangeCollectionIntegrationSuite) TestMailSerializ connector := loadConnector(ctx, t, Users) sel := selectors.NewExchangeBackup() sel.Include(sel.MailFolders([]string{suite.user}, []string{exchange.DefaultMailFolder}, selectors.PrefixMatch())) - collection, err := connector.createExchangeCollections(ctx, sel.Scopes()[0], nil) + collection, err := connector.createExchangeCollections(ctx, sel.Scopes()[0], nil, control.Options{}) require.NoError(t, err) for _, edc := range collection { @@ -427,7 +430,7 @@ func (suite *ConnectorCreateExchangeCollectionIntegrationSuite) TestContactSeria scope := selectors. NewExchangeBackup(). ContactFolders([]string{suite.user}, []string{exchange.DefaultContactFolder}, selectors.PrefixMatch())[0] - collections, err := connector.createExchangeCollections(ctx, scope, nil) + collections, err := connector.createExchangeCollections(ctx, scope, nil, control.Options{}) require.NoError(t, err) return collections @@ -494,7 +497,7 @@ func (suite *ConnectorCreateExchangeCollectionIntegrationSuite) TestEventsSerial getCollection: func(t *testing.T) []data.Collection { sel := selectors.NewExchangeBackup() sel.Include(sel.EventCalendars([]string{suite.user}, []string{exchange.DefaultCalendar}, selectors.PrefixMatch())) - collections, err := connector.createExchangeCollections(ctx, sel.Scopes()[0], nil) + collections, err := connector.createExchangeCollections(ctx, sel.Scopes()[0], nil, control.Options{}) require.NoError(t, err) return collections @@ -506,7 +509,7 @@ func (suite *ConnectorCreateExchangeCollectionIntegrationSuite) TestEventsSerial getCollection: func(t *testing.T) []data.Collection { sel := selectors.NewExchangeBackup() sel.Include(sel.EventCalendars([]string{suite.user}, []string{"Birthdays"}, selectors.PrefixMatch())) - collections, err := connector.createExchangeCollections(ctx, sel.Scopes()[0], nil) + collections, err := connector.createExchangeCollections(ctx, sel.Scopes()[0], nil, control.Options{}) require.NoError(t, err) return collections @@ -588,6 +591,6 @@ func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) TestCreateShar selectors.PrefixMatch(), )) - _, err := gc.DataCollections(ctx, sel.Selector, nil) + _, err := gc.DataCollections(ctx, sel.Selector, nil, control.Options{}) require.NoError(t, err) } diff --git a/src/internal/connector/exchange/exchange_data_collection.go b/src/internal/connector/exchange/exchange_data_collection.go index 130c0f164..862e3c255 100644 --- a/src/internal/connector/exchange/exchange_data_collection.go +++ b/src/internal/connector/exchange/exchange_data_collection.go @@ -23,6 +23,7 @@ import ( "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/observe" "github.com/alcionai/corso/src/pkg/backup/details" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" ) @@ -60,6 +61,8 @@ type Collection struct { // FullPath is the slice representation of the action context passed down through the hierarchy. // The original request can be gleaned from the slice. (e.g. {, , "emails"}) fullPath path.Path + + ctrl control.Options } // NewExchangeDataCollection creates an ExchangeDataCollection with fullPath is annotated @@ -69,6 +72,7 @@ func NewCollection( collectionType optionIdentifier, service graph.Servicer, statusUpdater support.StatusUpdater, + ctrlOpts control.Options, ) Collection { collection := Collection{ user: user, @@ -78,6 +82,7 @@ func NewCollection( statusUpdater: statusUpdater, fullPath: fullPath, collectionType: collectionType, + ctrl: ctrlOpts, } return collection @@ -168,7 +173,7 @@ func (col *Collection) populateByOptionIdentifier( } for _, identifier := range col.jobs { - if col.service.ErrPolicy() && errs != nil { + if col.ctrl.FailFast && errs != nil { break } semaphoreCh <- struct{}{} diff --git a/src/internal/connector/exchange/exchange_service_test.go b/src/internal/connector/exchange/exchange_service_test.go index 608478db9..cb8fb3860 100644 --- a/src/internal/connector/exchange/exchange_service_test.go +++ b/src/internal/connector/exchange/exchange_service_test.go @@ -45,7 +45,7 @@ func (suite *ExchangeServiceSuite) SetupSuite() { require.NoError(t, err) m365, err := a.M365Config() require.NoError(t, err) - service, err := createService(m365, false) + service, err := createService(m365) require.NoError(t, err) suite.es = service @@ -79,7 +79,7 @@ func (suite *ExchangeServiceSuite) TestCreateService() { for _, test := range tests { suite.T().Run(test.name, func(t *testing.T) { t.Log(test.credentials.AzureClientSecret) - _, err := createService(test.credentials, false) + _, err := createService(test.credentials) test.checkErr(t, err) }) } diff --git a/src/internal/connector/exchange/folder_resolver_test.go b/src/internal/connector/exchange/folder_resolver_test.go index 4914988fb..23247d8be 100644 --- a/src/internal/connector/exchange/folder_resolver_test.go +++ b/src/internal/connector/exchange/folder_resolver_test.go @@ -40,7 +40,7 @@ func (suite *CacheResolverSuite) SetupSuite() { m365, err := a.M365Config() require.NoError(t, err) - service, err := createService(m365, false) + service, err := createService(m365) require.NoError(t, err) suite.gs = service diff --git a/src/internal/connector/exchange/iterators_test.go b/src/internal/connector/exchange/iterators_test.go index a686a04c1..07d7b2278 100644 --- a/src/internal/connector/exchange/iterators_test.go +++ b/src/internal/connector/exchange/iterators_test.go @@ -151,7 +151,7 @@ func loadService(t *testing.T) *exchangeService { m365, err := a.M365Config() require.NoError(t, err) - service, err := createService(m365, false) + service, err := createService(m365) require.NoError(t, err) return service diff --git a/src/internal/connector/exchange/mail_folder_cache_test.go b/src/internal/connector/exchange/mail_folder_cache_test.go index 3b1911e5c..b8a508d35 100644 --- a/src/internal/connector/exchange/mail_folder_cache_test.go +++ b/src/internal/connector/exchange/mail_folder_cache_test.go @@ -41,7 +41,7 @@ func (suite *MailFolderCacheIntegrationSuite) SetupSuite() { m365, err := a.M365Config() require.NoError(t, err) - service, err := createService(m365, false) + service, err := createService(m365) require.NoError(t, err) suite.gs = service diff --git a/src/internal/connector/exchange/service_functions.go b/src/internal/connector/exchange/service_functions.go index 0df4127d7..7a920630e 100644 --- a/src/internal/connector/exchange/service_functions.go +++ b/src/internal/connector/exchange/service_functions.go @@ -19,7 +19,6 @@ var ErrFolderNotFound = errors.New("folder not found") type exchangeService struct { client msgraphsdk.GraphServiceClient adapter msgraphsdk.GraphRequestAdapter - failFast bool // if true service will exit sequence upon encountering an error credentials account.M365Config } @@ -35,14 +34,10 @@ func (es *exchangeService) Adapter() *msgraphsdk.GraphRequestAdapter { return &es.adapter } -func (es *exchangeService) ErrPolicy() bool { - return es.failFast -} - // createService internal constructor for exchangeService struct returns an error // iff the params for the entry are incorrect (e.g. len(TenantID) == 0, etc.) // NOTE: Incorrect account information will result in errors on subsequent queries. -func createService(credentials account.M365Config, shouldFailFast bool) (*exchangeService, error) { +func createService(credentials account.M365Config) (*exchangeService, error) { adapter, err := graph.CreateAdapter( credentials.AzureTenantID, credentials.AzureClientID, @@ -55,7 +50,6 @@ func createService(credentials account.M365Config, shouldFailFast bool) (*exchan service := exchangeService{ adapter: *adapter, client: *msgraphsdk.NewGraphServiceClient(adapter), - failFast: shouldFailFast, credentials: credentials, } @@ -142,7 +136,7 @@ func PopulateExchangeContainerResolver( var ( res graph.ContainerResolver cacheRoot string - service, err = createService(qp.Credentials, qp.FailFast) + service, err = createService(qp.Credentials) ) if err != nil { diff --git a/src/internal/connector/exchange/service_iterators.go b/src/internal/connector/exchange/service_iterators.go index a7364e898..43069fbba 100644 --- a/src/internal/connector/exchange/service_iterators.go +++ b/src/internal/connector/exchange/service_iterators.go @@ -15,6 +15,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -79,6 +80,7 @@ func FilterContainersAndFillCollections( resolver graph.ContainerResolver, scope selectors.ExchangeScope, oldDeltas map[string]string, + ctrlOpts control.Options, ) error { var ( errs error @@ -94,14 +96,14 @@ func FilterContainersAndFillCollections( } // Create only those that match - service, err := createService(qp.Credentials, qp.FailFast) + service, err := createService(qp.Credentials) if err != nil { errs = support.WrapAndAppend( qp.ResourceOwner+" FilterContainerAndFillCollection", err, errs) - if qp.FailFast { + if ctrlOpts.FailFast { return errs } } @@ -112,6 +114,7 @@ func FilterContainersAndFillCollections( collectionType, service, statusUpdater, + ctrlOpts, ) collections[*c.GetId()] = &edc @@ -122,7 +125,7 @@ func FilterContainersAndFillCollections( err, errs) - if qp.FailFast { + if ctrlOpts.FailFast { return errs } diff --git a/src/internal/connector/graph/service.go b/src/internal/connector/graph/service.go index db0b059a0..348fbad87 100644 --- a/src/internal/connector/graph/service.go +++ b/src/internal/connector/graph/service.go @@ -23,7 +23,6 @@ type QueryParams struct { Category path.CategoryType ResourceOwner string Credentials account.M365Config - FailFast bool } type Servicer interface { @@ -33,8 +32,6 @@ type Servicer interface { // Adapter() returns GraphRequest adapter used to process large requests, create batches // and page iterators Adapter() *msgraphsdk.GraphRequestAdapter - // ErrPolicy returns if the service is implementing a Fast-Fail policy or not - ErrPolicy() bool } // Idable represents objects that implement msgraph-sdk-go/models.entityable diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index a8298d474..746adc843 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -73,10 +73,6 @@ func (gs graphService) Adapter() *msgraphsdk.GraphRequestAdapter { return &gs.adapter } -func (gs graphService) ErrPolicy() bool { - return gs.failFast -} - type resource int const ( @@ -99,7 +95,7 @@ func NewGraphConnector(ctx context.Context, acct account.Account, r resource) (* credentials: m365, } - aService, err := gc.createService(false) + aService, err := gc.createService() if err != nil { return nil, errors.Wrap(err, "creating service connection") } @@ -126,7 +122,7 @@ func NewGraphConnector(ctx context.Context, acct account.Account, r resource) (* } // createService constructor for graphService component -func (gc *GraphConnector) createService(shouldFailFast bool) (*graphService, error) { +func (gc *GraphConnector) createService() (*graphService, error) { adapter, err := graph.CreateAdapter( gc.credentials.AzureTenantID, gc.credentials.AzureClientID, @@ -137,9 +133,8 @@ func (gc *GraphConnector) createService(shouldFailFast bool) (*graphService, err } connector := graphService{ - adapter: *adapter, - client: *msgraphsdk.NewGraphServiceClient(adapter), - failFast: shouldFailFast, + adapter: *adapter, + client: *msgraphsdk.NewGraphServiceClient(adapter), } return &connector, nil @@ -156,7 +151,7 @@ func (gc *GraphConnector) setTenantUsers(ctx context.Context) error { ctx, end := D.Span(ctx, "gc:setTenantUsers") defer end() - users, err := discovery.Users(ctx, gc.graphService, gc.tenant) + users, err := discovery.Users(ctx, gc, gc.tenant) if err != nil { return err } diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 3afd82603..651b0555e 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -16,6 +16,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -170,7 +171,7 @@ func (suite *GraphConnectorIntegrationSuite) TestSetTenantUsers() { ctx, flush := tester.NewContext() defer flush() - service, err := newConnector.createService(false) + service, err := newConnector.createService() require.NoError(suite.T(), err) newConnector.graphService = *service @@ -193,7 +194,7 @@ func (suite *GraphConnectorIntegrationSuite) TestSetTenantSites() { ctx, flush := tester.NewContext() defer flush() - service, err := newConnector.createService(false) + service, err := newConnector.createService() require.NoError(suite.T(), err) newConnector.graphService = *service @@ -387,7 +388,7 @@ func runRestoreBackupTest( t.Logf("Selective backup of %s\n", backupSel) start = time.Now() - dcs, err := backupGC.DataCollections(ctx, backupSel, nil) + dcs, err := backupGC.DataCollections(ctx, backupSel, nil, control.Options{}) require.NoError(t, err) t.Logf("Backup enumeration complete in %v\n", time.Since(start)) @@ -412,7 +413,7 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackup() { driveID := mustGetDefaultDriveID( suite.T(), ctx, - suite.connector.Service(), + suite.connector, suite.user, ) @@ -855,7 +856,7 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames backupSel := backupSelectorForExpected(t, test.service, expectedDests) t.Log("Selective backup of", backupSel) - dcs, err := backupGC.DataCollections(ctx, backupSel, nil) + dcs, err := backupGC.DataCollections(ctx, backupSel, nil, control.Options{}) require.NoError(t, err) t.Log("Backup enumeration complete") diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go index fd36468d5..a381e3b25 100644 --- a/src/internal/connector/onedrive/collection.go +++ b/src/internal/connector/onedrive/collection.go @@ -14,6 +14,7 @@ import ( "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/observe" "github.com/alcionai/corso/src/pkg/backup/details" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" ) @@ -54,6 +55,7 @@ type Collection struct { service graph.Servicer statusUpdater support.StatusUpdater itemReader itemReaderFunc + ctrl control.Options } // itemReadFunc returns a reader for the specified item @@ -70,6 +72,7 @@ func NewCollection( service graph.Servicer, statusUpdater support.StatusUpdater, source driveSource, + ctrlOpts control.Options, ) *Collection { c := &Collection{ folderPath: folderPath, @@ -79,6 +82,7 @@ func NewCollection( service: service, data: make(chan data.Stream, collectionChannelBufferSize), statusUpdater: statusUpdater, + ctrl: ctrlOpts, } // Allows tests to set a mock populator @@ -199,7 +203,7 @@ func (oc *Collection) populateItems(ctx context.Context) { } for _, itemID := range oc.driveItemIDs { - if oc.service.ErrPolicy() && errs != nil { + if oc.ctrl.FailFast && errs != nil { break } diff --git a/src/internal/connector/onedrive/collection_test.go b/src/internal/connector/onedrive/collection_test.go index 27614aba4..4de617c33 100644 --- a/src/internal/connector/onedrive/collection_test.go +++ b/src/internal/connector/onedrive/collection_test.go @@ -17,6 +17,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/pkg/backup/details" + "github.com/alcionai/corso/src/pkg/control" ) type CollectionUnitTestSuite struct { @@ -34,10 +35,6 @@ func (suite *CollectionUnitTestSuite) Adapter() *msgraphsdk.GraphRequestAdapter return nil } -func (suite *CollectionUnitTestSuite) ErrPolicy() bool { - return false -} - func TestCollectionUnitTestSuite(t *testing.T) { suite.Run(t, new(CollectionUnitTestSuite)) } @@ -108,7 +105,13 @@ func (suite *CollectionUnitTestSuite) TestCollection() { driveFolderPath, err := getDriveFolderPath(folderPath) require.NoError(t, err) - coll := NewCollection(folderPath, "drive-id", suite, suite.testStatusUpdater(&wg, &collStatus), test.source) + coll := NewCollection( + folderPath, + "drive-id", + suite, + suite.testStatusUpdater(&wg, &collStatus), + test.source, + control.Options{}) require.NotNil(t, coll) assert.Equal(t, folderPath, coll.FullPath()) @@ -173,7 +176,13 @@ func (suite *CollectionUnitTestSuite) TestCollectionReadError() { folderPath, err := GetCanonicalPath("drive/driveID1/root:/folderPath", "a-tenant", "a-user", test.source) require.NoError(t, err) - coll := NewCollection(folderPath, "fakeDriveID", suite, suite.testStatusUpdater(&wg, &collStatus), test.source) + coll := NewCollection( + folderPath, + "fakeDriveID", + suite, + suite.testStatusUpdater(&wg, &collStatus), + test.source, + control.Options{}) coll.Add("testItemID") readError := errors.New("Test error") diff --git a/src/internal/connector/onedrive/collections.go b/src/internal/connector/onedrive/collections.go index ac83be844..ac7d1ec4c 100644 --- a/src/internal/connector/onedrive/collections.go +++ b/src/internal/connector/onedrive/collections.go @@ -12,6 +12,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/observe" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" ) @@ -39,6 +40,8 @@ type Collections struct { service graph.Servicer statusUpdater support.StatusUpdater + ctrl control.Options + // collectionMap allows lookup of the data.Collection // for a OneDrive folder CollectionMap map[string]data.Collection @@ -56,6 +59,7 @@ func NewCollections( matcher folderMatcher, service graph.Servicer, statusUpdater support.StatusUpdater, + ctrlOpts control.Options, ) *Collections { return &Collections{ tenant: tenant, @@ -65,6 +69,7 @@ func NewCollections( CollectionMap: map[string]data.Collection{}, service: service, statusUpdater: statusUpdater, + ctrl: ctrlOpts, } } @@ -139,6 +144,7 @@ func (c *Collections) UpdateCollections(ctx context.Context, driveID string, ite c.service, c.statusUpdater, c.source, + c.ctrl, ) c.CollectionMap[collectionPath.String()] = col diff --git a/src/internal/connector/onedrive/collections_test.go b/src/internal/connector/onedrive/collections_test.go index 78148efcc..ad39d772e 100644 --- a/src/internal/connector/onedrive/collections_test.go +++ b/src/internal/connector/onedrive/collections_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -255,7 +256,14 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { ctx, flush := tester.NewContext() defer flush() - c := NewCollections(tenant, user, OneDriveSource, testFolderMatcher{tt.scope}, &MockGraphService{}, nil) + c := NewCollections( + tenant, + user, + OneDriveSource, + testFolderMatcher{tt.scope}, + &MockGraphService{}, + nil, + control.Options{}) err := c.UpdateCollections(ctx, "driveID", tt.items) tt.expect(t, err) diff --git a/src/internal/connector/onedrive/drive_test.go b/src/internal/connector/onedrive/drive_test.go index fa3964099..67f957a61 100644 --- a/src/internal/connector/onedrive/drive_test.go +++ b/src/internal/connector/onedrive/drive_test.go @@ -10,6 +10,7 @@ import ( "github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -153,6 +154,7 @@ func (suite *OneDriveSuite) TestOneDriveNewCollections() { testFolderMatcher{scope}, service, service.updateStatus, + control.Options{}, ).Get(ctx) assert.NoError(t, err) diff --git a/src/internal/connector/onedrive/item_test.go b/src/internal/connector/onedrive/item_test.go index 47a240a66..44b2d8ce7 100644 --- a/src/internal/connector/onedrive/item_test.go +++ b/src/internal/connector/onedrive/item_test.go @@ -34,10 +34,6 @@ func (suite *ItemIntegrationSuite) Adapter() *msgraphsdk.GraphRequestAdapter { return suite.adapter } -func (suite *ItemIntegrationSuite) ErrPolicy() bool { - return false -} - func TestItemIntegrationSuite(t *testing.T) { if err := tester.RunOnAny( tester.CorsoCITests, diff --git a/src/internal/connector/onedrive/service_test.go b/src/internal/connector/onedrive/service_test.go index da67ecfc1..00a791f1e 100644 --- a/src/internal/connector/onedrive/service_test.go +++ b/src/internal/connector/onedrive/service_test.go @@ -22,10 +22,6 @@ func (ms *MockGraphService) Adapter() *msgraphsdk.GraphRequestAdapter { return nil } -func (ms *MockGraphService) ErrPolicy() bool { - return false -} - // TODO(ashmrtn): Merge with similar structs in graph and exchange packages. type oneDriveService struct { client msgraphsdk.GraphServiceClient @@ -42,10 +38,6 @@ func (ods *oneDriveService) Adapter() *msgraphsdk.GraphRequestAdapter { return &ods.adapter } -func (ods *oneDriveService) ErrPolicy() bool { - return false -} - func NewOneDriveService(credentials account.M365Config) (*oneDriveService, error) { adapter, err := graph.CreateAdapter( credentials.AzureTenantID, diff --git a/src/internal/connector/sharepoint/data_collections.go b/src/internal/connector/sharepoint/data_collections.go index c248dc580..c1c38c67e 100644 --- a/src/internal/connector/sharepoint/data_collections.go +++ b/src/internal/connector/sharepoint/data_collections.go @@ -11,6 +11,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/observe" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" @@ -20,12 +21,6 @@ type statusUpdater interface { UpdateStatus(status *support.ConnectorOperationStatus) } -type connector interface { - statusUpdater - - Service() graph.Servicer -} - // DataCollections returns a set of DataCollection which represents the SharePoint data // for the specified user func DataCollections( @@ -33,7 +28,9 @@ func DataCollections( selector selectors.Selector, siteIDs []string, tenantID string, - con connector, + serv graph.Servicer, + su statusUpdater, + ctrlOpts control.Options, ) ([]data.Collection, error) { b, err := selector.ToSharePointBackup() if err != nil { @@ -43,7 +40,6 @@ func DataCollections( var ( scopes = b.DiscreteScopes(siteIDs) collections = []data.Collection{} - serv = con.Service() errs error ) @@ -70,7 +66,8 @@ func DataCollections( tenantID, site, scope, - con) + su, + ctrlOpts) if err != nil { return nil, support.WrapAndAppend(site, err, errs) } @@ -93,6 +90,7 @@ func collectLibraries( tenantID, siteID string, scope selectors.SharePointScope, updater statusUpdater, + ctrlOpts control.Options, ) ([]data.Collection, error) { var ( collections = []data.Collection{} @@ -107,7 +105,8 @@ func collectLibraries( onedrive.SharePointSource, folderMatcher{scope}, serv, - updater.UpdateStatus) + updater.UpdateStatus, + ctrlOpts) odcs, err := colls.Get(ctx) if err != nil { diff --git a/src/internal/connector/sharepoint/data_collections_test.go b/src/internal/connector/sharepoint/data_collections_test.go index 02f88c771..ab884e995 100644 --- a/src/internal/connector/sharepoint/data_collections_test.go +++ b/src/internal/connector/sharepoint/data_collections_test.go @@ -9,6 +9,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/onedrive" "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -92,7 +93,8 @@ func (suite *SharePointLibrariesSuite) TestUpdateCollections() { onedrive.SharePointSource, testFolderMatcher{test.scope}, &MockGraphService{}, - nil) + nil, + control.Options{}) err := c.UpdateCollections(ctx, "driveID", test.items) test.expect(t, err) assert.Equal(t, len(test.expectedCollectionPaths), len(c.CollectionMap), "collection paths") diff --git a/src/internal/connector/sharepoint/helper_test.go b/src/internal/connector/sharepoint/helper_test.go index 89f387957..e0dcc5ddf 100644 --- a/src/internal/connector/sharepoint/helper_test.go +++ b/src/internal/connector/sharepoint/helper_test.go @@ -35,10 +35,6 @@ func (ms *MockGraphService) Adapter() *msgraphsdk.GraphRequestAdapter { return nil } -func (ms *MockGraphService) ErrPolicy() bool { - return false -} - func (ts *testService) Client() *msgraphsdk.GraphServiceClient { return &ts.client } @@ -47,10 +43,6 @@ func (ts *testService) Adapter() *msgraphsdk.GraphRequestAdapter { return &ts.adapter } -func (ts *testService) ErrPolicy() bool { - return false -} - // --------------------------------------------------------------------------- // Helper Functions // --------------------------------------------------------------------------- diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index ebedf7407..c36762aaf 100644 --- a/src/internal/operations/backup.go +++ b/src/internal/operations/backup.go @@ -139,7 +139,7 @@ func (op *BackupOperation) Run(ctx context.Context) (err error) { return opStats.readErr } - cs, err := produceBackupDataCollections(ctx, gc, op.Selectors, mdColls) + cs, err := produceBackupDataCollections(ctx, gc, op.Selectors, mdColls, control.Options{}) if err != nil { opStats.readErr = errors.Wrap(err, "retrieving data to backup") return opStats.readErr @@ -316,6 +316,7 @@ func produceBackupDataCollections( gc *connector.GraphConnector, sel selectors.Selector, metadata []data.Collection, + ctrlOpts control.Options, ) ([]data.Collection, error) { complete, closer := observe.MessageWithCompletion("Discovering items to backup:") defer func() { @@ -324,7 +325,7 @@ func produceBackupDataCollections( closer() }() - return gc.DataCollections(ctx, sel, metadata) + return gc.DataCollections(ctx, sel, metadata, ctrlOpts) } // calls kopia to backup the collections of data