cleaning up some gc/selector test construction (#3175)

Duplicate of https://github.com/alcionai/corso/pull/3049, fixing a git rebase screwup.

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,
+		connector.Users)
-		errs)
 	if err != nil {
 		return nil, account.Account{}, clues.Wrap(err, "connecting to graph api")
 	}

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
+	gc, err := connector.NewGraphConnector(ctx, acct, connector.Users)
-	// TODO: log/print recoverable errors
-	errs := fault.New(false)
-
-	gc, err := connector.NewGraphConnector(ctx, acct, connector.Users, errs)
 	if err != nil {
 		return account.Account{}, nil, Only(ctx, clues.Wrap(err, "connecting to graph api"))
 	}

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"
+	m365api "github.com/alcionai/corso/src/pkg/services/m365/api"
-	"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,
+	r Resource,
-	errs *fault.Bus,
 ) (*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) {

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

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
 }
 

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

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(
+	gc := GCWithSelector(t, ctx, acct, connectorResource, sel, nil, closer)
-		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)
-
 	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(
+	var (
-		ctx,
+		gc      = GCWithSelector(t, ctx, acct, connector.Users, sel.Selector, nil, nil)
-		acct,
+		driveID = mustGetDefaultDriveID(t, ctx, gc.Service, suite.user)
-		connector.Users,
+		fileDBF = func(id, timeStamp, subject, body string) []byte {
-		fault.New(true))
+			return []byte(id + subject)
-	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)
-	}
 
 	// 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()

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

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(
+	sel.DiscreteOwner = owner
-		ctx,
+	sel.Include(
-		acct,
+		sel.MailFolders([]string{exchange.DefaultMailFolder}, selectors.PrefixMatch()),
-		connector.Users,
+		sel.ContactFolders([]string{exchange.DefaultContactFolder}, selectors.PrefixMatch()),
-		fault.New(true))
+		sel.EventCalendars([]string{exchange.DefaultCalendar}, selectors.PrefixMatch()))
-	require.NoError(t, err, clues.ToCore(err))
 
-	id, name, err := gc.PopulateOwnerIDAndNamesFrom(ctx, owner, nil)
+	gc := GCWithSelector(t, ctx, acct, connector.Users, sel.Selector, nil, 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)
 
 	bo, err := NewBackupOperation(
 		ctx,
@@ -297,8 +285,8 @@ func setupExchangeBackup(
 		sw,
 		gc,
 		acct,
-		bsel.Selector,
+		sel.Selector,
-		bsel.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,
+		sel.Selector,
-		spsel.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,
+		connector.Users)
-		fault.New(true))
 	require.NoError(t, err, clues.ToCore(err))
 
 	ro, err := NewRestoreOperation(

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
 	}