introduce idnamer (#2934)

Adds a common interface: idNamer, which is used
to pass around tuples of an id and a name for some
resource.  Also adds compliance to this iface in
selectors, where a selector's ID and Name are the
DiscreteOwner values.

---

#### Does this PR need a docs update or release note?

- [x] ⛔ No

#### Type of change

- [x] 🌻 Feature

#### Issue(s)

* #2825

#### Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E

src/internal/common/idname.go

@@ -0,0 +1,9 @@
+package common
+
+type IDNamer interface {
+	// the canonical id of the thing, generated and usable
+	//  by whichever system has ownership of it.
+	ID() string
+	// the human-readable name of the thing.
+	Name() string
+}

src/internal/connector/data_collections.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/alcionai/clues"
 
+	"github.com/alcionai/corso/src/internal/common"
 	"github.com/alcionai/corso/src/internal/connector/discovery"
 	"github.com/alcionai/corso/src/internal/connector/discovery/api"
 	"github.com/alcionai/corso/src/internal/connector/exchange"
@@ -34,7 +35,7 @@ import (
 // prior history (ie, incrementals) and run a full backup.
 func (gc *GraphConnector) ProduceBackupCollections(
 	ctx context.Context,
-	ownerID, ownerName string,
+	owner common.IDNamer,
 	sels selectors.Selector,
 	metadata []data.RestoreCollection,
 	ctrlOpts control.Options,

src/internal/connector/data_collections_test.go

@@ -208,7 +208,7 @@ func (suite *ConnectorDataCollectionIntegrationSuite) TestDataCollections_invali
 
 			collections, excludes, err := connector.ProduceBackupCollections(
 				ctx,
-				owners[0], owners[0],
+				test.getSelector(t),
 				test.getSelector(t),
 				nil,
 				control.Options{},
@@ -340,7 +340,7 @@ func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) TestCreateShar
 
 	cols, excludes, err := gc.ProduceBackupCollections(
 		ctx,
-		siteIDs[0], siteIDs[0],
+		sel.Selector,
 		sel.Selector,
 		nil,
 		control.Options{},
@@ -379,7 +379,7 @@ func (suite *ConnectorCreateSharePointCollectionIntegrationSuite) TestCreateShar
 
 	cols, excludes, err := gc.ProduceBackupCollections(
 		ctx,
-		siteIDs[0], siteIDs[0],
+		sel.Selector,
 		sel.Selector,
 		nil,
 		control.Options{},

src/internal/connector/graph_connector_test.go

@@ -459,8 +459,7 @@ func runBackupAndCompare(
 	start := time.Now()
 	dcs, excludes, err := backupGC.ProduceBackupCollections(
 		ctx,
-		backupSel.DiscreteOwner,
-		backupSel.DiscreteOwner,
+		backupSel,
 		backupSel,
 		nil,
 		config.opts,
@@ -1000,8 +999,7 @@ func (suite *GraphConnectorIntegrationSuite) TestMultiFolderBackupDifferentNames
 
 			dcs, excludes, err := backupGC.ProduceBackupCollections(
 				ctx,
-				backupSel.DiscreteOwner,
-				backupSel.DiscreteOwner,
+				backupSel,
 				backupSel,
 				nil,
 				control.Options{
@@ -1154,8 +1152,7 @@ func (suite *GraphConnectorIntegrationSuite) TestBackup_CreatesPrefixCollections
 
 			dcs, excludes, err := backupGC.ProduceBackupCollections(
 				ctx,
-				backupSel.DiscreteOwner,
-				backupSel.DiscreteOwner,
+				backupSel,
 				backupSel,
 				nil,
 				control.Options{

src/internal/connector/mockconnector/mock_data_connector.go

@@ -3,6 +3,7 @@ package mockconnector
 import (
 	"context"
 
+	"github.com/alcionai/corso/src/internal/common"
 	"github.com/alcionai/corso/src/internal/data"
 	"github.com/alcionai/corso/src/pkg/account"
 	"github.com/alcionai/corso/src/pkg/backup/details"
@@ -24,7 +25,7 @@ type GraphConnector struct {
 
 func (gc GraphConnector) ProduceBackupCollections(
 	_ context.Context,
-	_, _ string,
+	_ common.IDNamer,
 	_ selectors.Selector,
 	_ []data.RestoreCollection,
 	_ control.Options,

src/internal/operations/backup.go

@@ -33,8 +33,7 @@ import (
 type BackupOperation struct {
 	operation
 
-	ResourceOwner     string `json:"resourceOwner"`
-	ResourceOwnerName string `json:"resourceOwnerName"`
+	ResourceOwner common.IDNamer
 
 	Results   BackupResults      `json:"results"`
 	Selectors selectors.Selector `json:"selectors"`
@@ -63,22 +62,17 @@ func NewBackupOperation(
 	bp inject.BackupProducer,
 	acct account.Account,
 	selector selectors.Selector,
-	ownerName string,
+	owner common.IDNamer,
 	bus events.Eventer,
 ) (BackupOperation, error) {
 	op := BackupOperation{
-		operation:         newOperation(opts, bus, kw, sw),
-		ResourceOwner:     selector.DiscreteOwner,
-		ResourceOwnerName: ownerName,
-		Selectors:         selector,
-		Version:           "v0",
-		account:           acct,
-		incremental:       useIncrementalBackup(selector, opts),
-		bp:                bp,
-	}
-
-	if len(ownerName) == 0 {
-		op.ResourceOwnerName = op.ResourceOwner
+		operation:     newOperation(opts, bus, kw, sw),
+		ResourceOwner: owner,
+		Selectors:     selector,
+		Version:       "v0",
+		account:       acct,
+		incremental:   useIncrementalBackup(selector, opts),
+		bp:            bp,
 	}
 
 	if err := op.validate(); err != nil {
@@ -89,10 +83,14 @@ func NewBackupOperation(
 }
 
 func (op BackupOperation) validate() error {
-	if len(op.ResourceOwner) == 0 {
+	if op.ResourceOwner == nil {
 		return clues.New("backup requires a resource owner")
 	}
 
+	if len(op.ResourceOwner.ID()) == 0 {
+		return clues.New("backup requires a resource owner with a populated ID")
+	}
+
 	if op.bp == nil {
 		return clues.New("missing backup producer")
 	}
@@ -165,7 +163,7 @@ func (op *BackupOperation) Run(ctx context.Context) (err error) {
 	// Execution
 	// -----
 
-	observe.Message(ctx, "Backing Up", observe.Bullet, clues.Hide(op.ResourceOwner))
+	observe.Message(ctx, "Backing Up", observe.Bullet, clues.Hide(op.ResourceOwner.Name()))
 
 	deets, err := op.do(
 		ctx,
@@ -253,7 +251,6 @@ func (op *BackupOperation) do(
 		ctx,
 		op.bp,
 		op.ResourceOwner,
-		op.ResourceOwnerName,
 		op.Selectors,
 		mdColls,
 		op.Options,
@@ -327,7 +324,7 @@ func useIncrementalBackup(sel selectors.Selector, opts control.Options) bool {
 func produceBackupDataCollections(
 	ctx context.Context,
 	bp inject.BackupProducer,
-	ownerID, ownerName string,
+	resourceOwner common.IDNamer,
 	sel selectors.Selector,
 	metadata []data.RestoreCollection,
 	ctrlOpts control.Options,
@@ -340,7 +337,7 @@ func produceBackupDataCollections(
 		closer()
 	}()
 
-	return bp.ProduceBackupCollections(ctx, ownerID, ownerName, sel, metadata, ctrlOpts, errs)
+	return bp.ProduceBackupCollections(ctx, resourceOwner, sel, metadata, ctrlOpts, errs)
 }
 
 // ---------------------------------------------------------------------------
@@ -717,8 +714,8 @@ func (op *BackupOperation) createBackupModels(
 		op.Status.String(),
 		backupID,
 		op.Selectors,
-		op.ResourceOwner,
-		op.ResourceOwnerName,
+		op.ResourceOwner.ID(),
+		op.ResourceOwner.Name(),
 		op.Results.ReadWrites,
 		op.Results.StartAndEndTime,
 		op.Errors.Errors())

src/internal/operations/backup_integration_test.go

@@ -154,7 +154,7 @@ func newTestBackupOp(
 
 	opts.ToggleFeatures = featureToggles
 
-	bo, err := NewBackupOperation(ctx, opts, kw, sw, gc, acct, sel, sel.DiscreteOwner, bus)
+	bo, err := NewBackupOperation(ctx, opts, kw, sw, gc, acct, sel, sel, bus)
 	if !assert.NoError(t, err, clues.ToCore(err)) {
 		closer()
 		t.FailNow()
@@ -566,6 +566,8 @@ func (suite *BackupOpIntegrationSuite) TestNewBackupOperation() {
 			ctx, flush := tester.NewContext()
 			defer flush()
 
+			sel := selectors.Selector{DiscreteOwner: "test"}
+
 			_, err := NewBackupOperation(
 				ctx,
 				test.opts,
@@ -573,8 +575,8 @@ func (suite *BackupOpIntegrationSuite) TestNewBackupOperation() {
 				test.sw,
 				test.bp,
 				test.acct,
-				selectors.Selector{DiscreteOwner: "test"},
-				"test-name",
+				sel,
+				sel,
 				evmock.NewBus())
 			test.errCheck(suite.T(), err, clues.ToCore(err))
 		})

src/internal/operations/backup_test.go

@@ -415,7 +415,7 @@ func (suite *BackupOpUnitSuite) TestBackupOperation_PersistResults() {
 				gc,
 				acct,
 				sel,
-				sel.DiscreteOwner,
+				sel,
 				evmock.NewBus())
 			require.NoError(t, err, clues.ToCore(err))
 

src/internal/operations/inject/inject.go

@@ -3,6 +3,7 @@ package inject
 import (
 	"context"
 
+	"github.com/alcionai/corso/src/internal/common"
 	"github.com/alcionai/corso/src/internal/data"
 	"github.com/alcionai/corso/src/internal/kopia"
 	"github.com/alcionai/corso/src/pkg/account"
@@ -17,7 +18,7 @@ type (
 	BackupProducer interface {
 		ProduceBackupCollections(
 			ctx context.Context,
-			ownerID, ownerName string,
+			resourceOwner common.IDNamer,
 			sels selectors.Selector,
 			metadata []data.RestoreCollection,
 			ctrlOpts control.Options,

src/internal/operations/restore_test.go

@@ -293,7 +293,7 @@ func setupExchangeBackup(
 		gc,
 		acct,
 		bsel.Selector,
-		bsel.Selector.DiscreteOwner,
+		bsel.Selector,
 		evmock.NewBus())
 	require.NoError(t, err, clues.ToCore(err))
 
@@ -350,7 +350,7 @@ func setupSharePointBackup(
 		gc,
 		acct,
 		spsel.Selector,
-		spsel.Selector.DiscreteOwner,
+		spsel.Selector,
 		evmock.NewBus())
 	require.NoError(t, err, clues.ToCore(err))
 

src/pkg/repository/repository.go

@@ -296,6 +296,9 @@ func (r repository) NewBackup(
 		return operations.BackupOperation{}, errors.Wrap(err, "connecting to m365")
 	}
 
+	// TODO: retrieve display name from gc
+	sel = sel.SetDiscreteOwnerIDName(sel.DiscreteOwner, "")
+
 	return operations.NewBackupOperation(
 		ctx,
 		r.Opts,
@@ -304,7 +307,7 @@ func (r repository) NewBackup(
 		gc,
 		r.Account,
 		sel,
-		sel.DiscreteOwner,
+		sel,
 		r.Bus)
 }
 

src/pkg/selectors/selectors.go

@@ -112,6 +112,8 @@ type Selector struct {
 	// iterate over the results, where each one will populate this field
 	// with a different owner.
 	DiscreteOwner string `json:"discreteOwner,omitempty"`
+	// display name for the DiscreteOwner.
+	DiscreteOwnerName string `json:"discreteOwnerName,omitempty"`
 
 	// A slice of exclusion scopes.  Exclusions apply globally to all
 	// inclusions/filters, with any-match behavior.
@@ -146,6 +148,48 @@ func (s Selector) DiscreteResourceOwners() []string {
 	return split(s.ResourceOwners.Target)
 }
 
+// SetDiscreteOwnerIDName ensures the selector has the correct discrete owner
+// id and name.  Assumes that these values are sourced using the current
+// s.DiscreteOwner as input.  The reason for taking in both the id and name, and
+// not just the name, is so that constructors can input owner aliases in place
+// of  ids, with the expectation that the two will get sorted and re-written
+// later on with this setter.
+//
+// If the id is empty, the original DiscreteOwner value is retained.
+// If the name is empty, the id is duplicated as the name.
+func (s Selector) SetDiscreteOwnerIDName(id, name string) Selector {
+	r := s
+
+	if len(id) == 0 {
+		// assume a the discreteOwner is already set, and don't replace anything.
+		id = s.DiscreteOwner
+	}
+
+	r.DiscreteOwner = id
+	r.DiscreteOwnerName = name
+
+	if len(name) == 0 {
+		r.DiscreteOwnerName = id
+	}
+
+	return r
+}
+
+// ID returns s.discreteOwner, which is assumed to be a stable ID.
+func (s Selector) ID() string {
+	return s.DiscreteOwner
+}
+
+// Name returns s.discreteOwnerName.  If that value is empty, it returns
+// s.DiscreteOwner instead.
+func (s Selector) Name() string {
+	if len(s.DiscreteOwnerName) == 0 {
+		return s.DiscreteOwner
+	}
+
+	return s.DiscreteOwnerName
+}
+
 // isAnyResourceOwner returns true if the selector includes all resource owners.
 func isAnyResourceOwner(s Selector) bool {
 	return s.ResourceOwners.Comparator == filters.Passes
@@ -336,7 +380,7 @@ func pathCategoriesIn[T scopeT, C categoryT](ss []scope) []path.CategoryType {
 }
 
 // ---------------------------------------------------------------------------
-// scope helpers
+// scope constructors
 // ---------------------------------------------------------------------------
 
 type scopeConfig struct {

src/pkg/selectors/selectors_test.go

@@ -248,6 +248,49 @@ func (suite *SelectorSuite) TestSplitByResourceOnwer() {
 	}
 }
 
+func (suite *SelectorSuite) TestIDName() {
+	table := []struct {
+		title                string
+		id, name             string
+		expectID, expectName string
+	}{
+		{"empty", "", "", "", ""},
+		{"only id", "id", "", "id", "id"},
+		{"only name", "", "name", "", "name"},
+		{"both", "id", "name", "id", "name"},
+	}
+	for _, test := range table {
+		suite.Run(test.title, func() {
+			sel := Selector{DiscreteOwner: test.id, DiscreteOwnerName: test.name}
+			assert.Equal(suite.T(), test.expectID, sel.ID())
+			assert.Equal(suite.T(), test.expectName, sel.Name())
+		})
+	}
+}
+
+func (suite *SelectorSuite) TestSetDiscreteOwnerIDName() {
+	table := []struct {
+		title                string
+		initID, initName     string
+		id, name             string
+		expectID, expectName string
+	}{
+		{"empty", "", "", "", "", "", ""},
+		{"only id", "", "", "id", "", "id", "id"},
+		{"only name", "", "", "", "", "", ""},
+		{"both", "", "", "id", "name", "id", "name"},
+		{"both", "init-id", "", "", "name", "init-id", "name"},
+	}
+	for _, test := range table {
+		suite.Run(test.title, func() {
+			sel := Selector{DiscreteOwner: test.initID, DiscreteOwnerName: test.initName}
+			sel = sel.SetDiscreteOwnerIDName(test.id, test.name)
+			assert.Equal(suite.T(), test.expectID, sel.ID())
+			assert.Equal(suite.T(), test.expectName, sel.Name())
+		})
+	}
+}
+
 // TestPathCategories verifies that no scope produces a `path.UnknownCategory`
 func (suite *SelectorSuite) TestPathCategories_includes() {
 	users := []string{"someuser@onmicrosoft.com"}