diff --git a/src/cmd/factory/impl/common.go b/src/cmd/factory/impl/common.go index 60b524834..4cb0e013d 100644 --- a/src/cmd/factory/impl/common.go +++ b/src/cmd/factory/impl/common.go @@ -137,8 +137,7 @@ func getGCAndVerifyUser(ctx context.Context, userID string) (*connector.GraphCon gc, err := connector.NewGraphConnector( ctx, acct, - connector.Users, - errs) + connector.Users) if err != nil { return nil, account.Account{}, clues.Wrap(err, "connecting to graph api") } diff --git a/src/cmd/purge/purge.go b/src/cmd/purge/purge.go index b7bbff321..239a0d8a6 100644 --- a/src/cmd/purge/purge.go +++ b/src/cmd/purge/purge.go @@ -266,11 +266,7 @@ func getGC(ctx context.Context) (account.Account, *connector.GraphConnector, err return account.Account{}, nil, Only(ctx, clues.Wrap(err, "finding m365 account details")) } - // build a graph connector - // TODO: log/print recoverable errors - errs := fault.New(false) - - gc, err := connector.NewGraphConnector(ctx, acct, connector.Users, errs) + gc, err := connector.NewGraphConnector(ctx, acct, connector.Users) if err != nil { return account.Account{}, nil, Only(ctx, clues.Wrap(err, "connecting to graph api")) } diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index 22126b82f..81f58cd39 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -15,8 +15,7 @@ import ( "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/operations/inject" "github.com/alcionai/corso/src/pkg/account" - "github.com/alcionai/corso/src/pkg/fault" - "github.com/alcionai/corso/src/pkg/services/m365/api" + m365api "github.com/alcionai/corso/src/pkg/services/m365/api" ) // --------------------------------------------------------------------------- @@ -34,7 +33,7 @@ var ( // bookkeeping and interfacing with other component. type GraphConnector struct { Service graph.Servicer - Discovery api.Client + Discovery m365api.Client itemClient graph.Requester // configured to handle large item downloads tenant string @@ -58,8 +57,7 @@ type GraphConnector struct { func NewGraphConnector( ctx context.Context, acct account.Account, - r resource, - errs *fault.Bus, + r Resource, ) (*GraphConnector, error) { creds, err := acct.M365Config() if err != nil { @@ -71,18 +69,18 @@ func NewGraphConnector( return nil, clues.Wrap(err, "creating service connection").WithClues(ctx) } - discovery, err := api.NewClient(creds) + ac, err := m365api.NewClient(creds) if err != nil { return nil, clues.Wrap(err, "creating api client").WithClues(ctx) } - rc, err := r.resourceClient(discovery) + rc, err := r.resourceClient(ac) if err != nil { return nil, clues.Wrap(err, "creating resource client").WithClues(ctx) } gc := GraphConnector{ - Discovery: discovery, + Discovery: ac, IDNameLookup: common.IDsNames{}, Service: service, @@ -177,28 +175,28 @@ func (gc *GraphConnector) incrementMessagesBy(num int) { // Resource Lookup Handling // --------------------------------------------------------------------------- -type resource int +type Resource int const ( - UnknownResource resource = iota + UnknownResource Resource = iota AllResources // unused Users Sites ) -func (r resource) resourceClient(discovery api.Client) (*resourceClient, error) { +func (r Resource) resourceClient(ac m365api.Client) (*resourceClient, error) { switch r { case Users: - return &resourceClient{enum: r, getter: discovery.Users()}, nil + return &resourceClient{enum: r, getter: ac.Users()}, nil case Sites: - return &resourceClient{enum: r, getter: discovery.Sites()}, nil + return &resourceClient{enum: r, getter: ac.Sites()}, nil default: return nil, clues.New("unrecognized owner resource enum").With("resource_enum", r) } } type resourceClient struct { - enum resource + enum Resource getter getIDAndNamer } @@ -215,7 +213,7 @@ var _ getOwnerIDAndNamer = &resourceClient{} type getOwnerIDAndNamer interface { getOwnerIDAndNameFrom( ctx context.Context, - discovery api.Client, + discovery m365api.Client, owner string, ins common.IDNameSwapper, ) ( @@ -233,7 +231,7 @@ type getOwnerIDAndNamer interface { // (PrincipalName for users, WebURL for sites). func (r resourceClient) getOwnerIDAndNameFrom( ctx context.Context, - discovery api.Client, + discovery m365api.Client, owner string, ins common.IDNameSwapper, ) (string, string, error) { diff --git a/src/internal/connector/graph_connector_helper_test.go b/src/internal/connector/graph_connector_helper_test.go index 3a60bc701..951b87104 100644 --- a/src/internal/connector/graph_connector_helper_test.go +++ b/src/internal/connector/graph_connector_helper_test.go @@ -111,7 +111,7 @@ func testElementsMatch[T any]( type configInfo struct { acct account.Account opts control.Options - resource resource + resource Resource service path.ServiceType tenant string resourceOwners []string @@ -147,14 +147,14 @@ type restoreBackupInfo struct { name string service path.ServiceType collections []colInfo - resource resource + resource Resource } type restoreBackupInfoMultiVersion struct { service path.ServiceType collectionsLatest []colInfo collectionsPrevious []colInfo - resource resource + resource Resource backupVersion int } @@ -1282,10 +1282,10 @@ func getSelectorWith( } } -func loadConnector(ctx context.Context, t *testing.T, r resource) *GraphConnector { +func loadConnector(ctx context.Context, t *testing.T, r Resource) *GraphConnector { a := tester.NewM365Account(t) - connector, err := NewGraphConnector(ctx, a, r, fault.New(true)) + connector, err := NewGraphConnector(ctx, a, r) require.NoError(t, err, clues.ToCore(err)) return connector diff --git a/src/internal/connector/graph_connector_onedrive_test.go b/src/internal/connector/graph_connector_onedrive_test.go index 495ddedc4..228c089dc 100644 --- a/src/internal/connector/graph_connector_onedrive_test.go +++ b/src/internal/connector/graph_connector_onedrive_test.go @@ -364,7 +364,7 @@ type suiteInfo interface { // also be a site. BackupResourceOwner() string BackupService() path.ServiceType - Resource() resource + Resource() Resource } type oneDriveSuite interface { @@ -383,7 +383,7 @@ type suiteInfoImpl struct { tertiaryUserID string acct account.Account service path.ServiceType - resourceType resource + resourceType Resource } func (si suiteInfoImpl) Service() graph.Servicer { @@ -418,7 +418,7 @@ func (si suiteInfoImpl) BackupService() path.ServiceType { return si.service } -func (si suiteInfoImpl) Resource() resource { +func (si suiteInfoImpl) Resource() Resource { return si.resourceType } diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 123f3f959..d00583c65 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -1176,7 +1176,7 @@ func (suite *GraphConnectorIntegrationSuite) TestRestoreAndBackup_largeMailAttac func (suite *GraphConnectorIntegrationSuite) TestBackup_CreatesPrefixCollections() { table := []struct { name string - resource resource + resource Resource selectorFunc func(t *testing.T) selectors.Selector service path.ServiceType categories []string diff --git a/src/internal/operations/backup_integration_test.go b/src/internal/operations/backup_integration_test.go index e322bfebe..fcf404dcf 100644 --- a/src/internal/operations/backup_integration_test.go +++ b/src/internal/operations/backup_integration_test.go @@ -41,7 +41,7 @@ import ( "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" - "github.com/alcionai/corso/src/pkg/selectors/testdata" + selTD "github.com/alcionai/corso/src/pkg/selectors/testdata" "github.com/alcionai/corso/src/pkg/store" ) @@ -113,21 +113,7 @@ func prepNewTestBackupOp( connectorResource = connector.Sites } - gc, err := connector.NewGraphConnector( - ctx, - acct, - connectorResource, - fault.New(true)) - if !assert.NoError(t, err, clues.ToCore(err)) { - closer() - t.FailNow() - } - - id, name, err := gc.PopulateOwnerIDAndNamesFrom(ctx, sel.DiscreteOwner, nil) - require.NoError(t, err, clues.ToCore(err)) - - sel.SetDiscreteOwnerIDName(id, name) - + gc := GCWithSelector(t, ctx, acct, connectorResource, sel, nil, closer) bo := newTestBackupOp(t, ctx, kw, ms, gc, acct, sel, bus, featureToggles, closer) return bo, acct, kw, ms, gc, closer @@ -742,18 +728,23 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { container2 = fmt.Sprintf("%s%d_%s", incrementalsDestContainerPrefix, 2, now) container3 = fmt.Sprintf("%s%d_%s", incrementalsDestContainerPrefix, 3, now) containerRename = fmt.Sprintf("%s%d_%s", incrementalsDestContainerPrefix, 4, now) + + // container3 and containerRename don't exist yet. Those will get created + // later on during the tests. Putting their identifiers into the selector + // at this point is harmless. + containers = []string{container1, container2, container3, containerRename} + sel = selectors.NewExchangeBackup(owners) + gc = GCWithSelector(t, ctx, acct, connector.Users, sel.Selector, nil, nil) + ) + + sel.Include( + sel.MailFolders(containers, selectors.PrefixMatch()), + sel.ContactFolders(containers, selectors.PrefixMatch()), ) m365, err := acct.M365Config() require.NoError(t, err, clues.ToCore(err)) - gc, err := connector.NewGraphConnector( - ctx, - acct, - connector.Users, - fault.New(true)) - require.NoError(t, err, clues.ToCore(err)) - ac, err := api.NewClient(m365) require.NoError(t, err, clues.ToCore(err)) @@ -866,16 +857,6 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { } } - // container3 and containerRename don't exist yet. Those will get created - // later on during the tests. Putting their identifiers into the selector - // at this point is harmless. - containers := []string{container1, container2, container3, containerRename} - sel := selectors.NewExchangeBackup(owners) - sel.Include( - sel.MailFolders(containers, selectors.PrefixMatch()), - sel.ContactFolders(containers, selectors.PrefixMatch()), - ) - bo, _, kw, ms, gc, closer := prepNewTestBackupOp(t, ctx, mb, sel.Selector, ffs) defer closer() @@ -1180,12 +1161,12 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { ffs = control.Toggles{} mb = evmock.NewBus() + owners = []string{suite.user} + // `now` has to be formatted with SimpleDateTimeOneDrive as // some onedrive cannot have `:` in file/folder names now = common.FormatNow(common.SimpleTimeTesting) - owners = []string{suite.user} - categories = map[path.CategoryType][]string{ path.FilesCategory: {graph.DeltaURLsFileName, graph.PreviousPathFileName}, } @@ -1194,23 +1175,25 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { container3 = fmt.Sprintf("%s%d_%s", incrementalsDestContainerPrefix, 3, now) genDests = []string{container1, container2} + + // container3 does not exist yet. It will get created later on + // during the tests. + containers = []string{container1, container2, container3} + sel = selectors.NewOneDriveBackup(owners) ) + sel.Include(sel.Folders(containers, selectors.PrefixMatch())) + creds, err := acct.M365Config() require.NoError(t, err, clues.ToCore(err)) - gc, err := connector.NewGraphConnector( - ctx, - acct, - connector.Users, - fault.New(true)) - require.NoError(t, err, clues.ToCore(err)) - - driveID := mustGetDefaultDriveID(t, ctx, gc.Service, suite.user) - - fileDBF := func(id, timeStamp, subject, body string) []byte { - return []byte(id + subject) - } + var ( + gc = GCWithSelector(t, ctx, acct, connector.Users, sel.Selector, nil, nil) + driveID = mustGetDefaultDriveID(t, ctx, gc.Service, suite.user) + fileDBF = func(id, timeStamp, subject, body string) []byte { + return []byte(id + subject) + } + ) // Populate initial test data. // Generate 2 new folders with two items each. Only the first two @@ -1251,12 +1234,6 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { containerIDs[destName] = ptr.Val(resp.GetId()) } - // container3 does not exist yet. It will get created later on - // during the tests. - containers := []string{container1, container2, container3} - sel := selectors.NewOneDriveBackup(owners) - sel.Include(sel.Folders(containers, selectors.PrefixMatch())) - bo, _, kw, ms, gc, closer := prepNewTestBackupOp(t, ctx, mb, sel.Selector, ffs) defer closer() @@ -1615,7 +1592,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_sharePoint() { sel = selectors.NewSharePointBackup([]string{suite.site}) ) - sel.Include(testdata.SharePointBackupFolderScope(sel)) + sel.Include(selTD.SharePointBackupFolderScope(sel)) bo, _, kw, _, _, closer := prepNewTestBackupOp(t, ctx, mb, sel.Selector, control.Toggles{}) defer closer() diff --git a/src/internal/operations/help_test.go b/src/internal/operations/help_test.go new file mode 100644 index 000000000..7860380f8 --- /dev/null +++ b/src/internal/operations/help_test.go @@ -0,0 +1,49 @@ +package operations + +import ( + "context" + "testing" + + "github.com/alcionai/clues" + "github.com/stretchr/testify/assert" + + "github.com/alcionai/corso/src/internal/common" + "github.com/alcionai/corso/src/internal/connector" + "github.com/alcionai/corso/src/pkg/account" + "github.com/alcionai/corso/src/pkg/selectors" +) + +// A QoL builder for live GC instances that updates +// the selector's owner id and name in the process +// to help avoid gotchas. +func GCWithSelector( + t *testing.T, + ctx context.Context, //revive:disable-line:context-as-argument + acct account.Account, + cr connector.Resource, + sel selectors.Selector, + ins common.IDNameSwapper, + onFail func(), +) *connector.GraphConnector { + gc, err := connector.NewGraphConnector(ctx, acct, cr) + if !assert.NoError(t, err, clues.ToCore(err)) { + if onFail != nil { + onFail() + } + + t.FailNow() + } + + id, name, err := gc.PopulateOwnerIDAndNamesFrom(ctx, sel.DiscreteOwner, ins) + if !assert.NoError(t, err, clues.ToCore(err)) { + if onFail != nil { + onFail() + } + + t.FailNow() + } + + sel.SetDiscreteOwnerIDName(id, name) + + return gc +} diff --git a/src/internal/operations/restore_test.go b/src/internal/operations/restore_test.go index 9b00e122e..623367a75 100644 --- a/src/internal/operations/restore_test.go +++ b/src/internal/operations/restore_test.go @@ -27,7 +27,6 @@ import ( "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/control" - "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/selectors" "github.com/alcionai/corso/src/pkg/store" ) @@ -268,27 +267,16 @@ func setupExchangeBackup( var ( users = []string{owner} - bsel = selectors.NewExchangeBackup(users) + sel = selectors.NewExchangeBackup(users) ) - gc, err := connector.NewGraphConnector( - ctx, - acct, - connector.Users, - fault.New(true)) - require.NoError(t, err, clues.ToCore(err)) + sel.DiscreteOwner = owner + sel.Include( + sel.MailFolders([]string{exchange.DefaultMailFolder}, selectors.PrefixMatch()), + sel.ContactFolders([]string{exchange.DefaultContactFolder}, selectors.PrefixMatch()), + sel.EventCalendars([]string{exchange.DefaultCalendar}, selectors.PrefixMatch())) - id, name, err := gc.PopulateOwnerIDAndNamesFrom(ctx, owner, nil) - require.NoError(t, err, clues.ToCore(err)) - - bsel.DiscreteOwner = owner - bsel.Include( - bsel.MailFolders([]string{exchange.DefaultMailFolder}, selectors.PrefixMatch()), - bsel.ContactFolders([]string{exchange.DefaultContactFolder}, selectors.PrefixMatch()), - bsel.EventCalendars([]string{exchange.DefaultCalendar}, selectors.PrefixMatch()), - ) - - bsel.SetDiscreteOwnerIDName(id, name) + gc := GCWithSelector(t, ctx, acct, connector.Users, sel.Selector, nil, nil) bo, err := NewBackupOperation( ctx, @@ -297,8 +285,8 @@ func setupExchangeBackup( sw, gc, acct, - bsel.Selector, - bsel.Selector, + sel.Selector, + sel.Selector, evmock.NewBus()) require.NoError(t, err, clues.ToCore(err)) @@ -329,27 +317,17 @@ func setupSharePointBackup( var ( sites = []string{owner} - spsel = selectors.NewSharePointBackup(sites) + sel = selectors.NewSharePointBackup(sites) ) - gc, err := connector.NewGraphConnector( - ctx, - acct, - connector.Sites, - fault.New(true)) - require.NoError(t, err, clues.ToCore(err)) - - id, name, err := gc.PopulateOwnerIDAndNamesFrom(ctx, owner, nil) - require.NoError(t, err, clues.ToCore(err)) - - spsel.DiscreteOwner = owner // assume a folder name "test" exists in the drive. // this is brittle, and requires us to backfill anytime // the site under test changes, but also prevents explosive // growth from re-backup/restore of restored files. - spsel.Include(spsel.LibraryFolders([]string{"test"}, selectors.PrefixMatch())) + sel.Include(sel.LibraryFolders([]string{"test"}, selectors.PrefixMatch())) + sel.DiscreteOwner = owner - spsel.SetDiscreteOwnerIDName(id, name) + gc := GCWithSelector(t, ctx, acct, connector.Sites, sel.Selector, nil, nil) bo, err := NewBackupOperation( ctx, @@ -358,8 +336,8 @@ func setupSharePointBackup( sw, gc, acct, - spsel.Selector, - spsel.Selector, + sel.Selector, + sel.Selector, evmock.NewBus()) require.NoError(t, err, clues.ToCore(err)) @@ -492,8 +470,7 @@ func (suite *RestoreOpIntegrationSuite) TestRestore_Run_errorNoResults() { gc, err := connector.NewGraphConnector( ctx, suite.acct, - connector.Users, - fault.New(true)) + connector.Users) require.NoError(t, err, clues.ToCore(err)) ro, err := NewRestoreOperation( diff --git a/src/pkg/repository/repository.go b/src/pkg/repository/repository.go index f3db097c3..f995c3bf9 100644 --- a/src/pkg/repository/repository.go +++ b/src/pkg/repository/repository.go @@ -308,7 +308,7 @@ func (r repository) NewBackupWithLookup( sel selectors.Selector, ins common.IDNameSwapper, ) (operations.BackupOperation, error) { - gc, err := connectToM365(ctx, sel, r.Account, fault.New(true)) + gc, err := connectToM365(ctx, sel, r.Account) if err != nil { return operations.BackupOperation{}, errors.Wrap(err, "connecting to m365") } @@ -345,7 +345,7 @@ func (r repository) NewRestore( sel selectors.Selector, dest control.RestoreDestination, ) (operations.RestoreOperation, error) { - gc, err := connectToM365(ctx, sel, r.Account, fault.New(true)) + gc, err := connectToM365(ctx, sel, r.Account) if err != nil { return operations.RestoreOperation{}, errors.Wrap(err, "connecting to m365") } @@ -627,7 +627,6 @@ func connectToM365( ctx context.Context, sel selectors.Selector, acct account.Account, - errs *fault.Bus, ) (*connector.GraphConnector, error) { complete, closer := observe.MessageWithCompletion(ctx, "Connecting to M365") defer func() { @@ -642,7 +641,7 @@ func connectToM365( resource = connector.Sites } - gc, err := connector.NewGraphConnector(ctx, acct, resource, errs) + gc, err := connector.NewGraphConnector(ctx, acct, resource) if err != nil { return nil, err }