diff --git a/src/internal/common/idname.go b/src/internal/common/idname.go new file mode 100644 index 000000000..fdc767226 --- /dev/null +++ b/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 +} diff --git a/src/internal/connector/data_collections.go b/src/internal/connector/data_collections.go index f7f2ba143..26a9a8a49 100644 --- a/src/internal/connector/data_collections.go +++ b/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, diff --git a/src/internal/connector/data_collections_test.go b/src/internal/connector/data_collections_test.go index 1d4b55373..a002fdc80 100644 --- a/src/internal/connector/data_collections_test.go +++ b/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{}, diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index dc8115134..5c45b3fa9 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/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{ diff --git a/src/internal/connector/mockconnector/mock_data_connector.go b/src/internal/connector/mockconnector/mock_data_connector.go index 453345a2c..6c5850bdb 100644 --- a/src/internal/connector/mockconnector/mock_data_connector.go +++ b/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, diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index 5f6259355..9d5af2860 100644 --- a/src/internal/operations/backup.go +++ b/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()) diff --git a/src/internal/operations/backup_integration_test.go b/src/internal/operations/backup_integration_test.go index 1da1ac6ff..c87a3ec71 100644 --- a/src/internal/operations/backup_integration_test.go +++ b/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)) }) diff --git a/src/internal/operations/backup_test.go b/src/internal/operations/backup_test.go index 96f9de9c7..8a519c456 100644 --- a/src/internal/operations/backup_test.go +++ b/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)) diff --git a/src/internal/operations/inject/inject.go b/src/internal/operations/inject/inject.go index dfc0c8cab..fa9339f50 100644 --- a/src/internal/operations/inject/inject.go +++ b/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, diff --git a/src/internal/operations/restore_test.go b/src/internal/operations/restore_test.go index 33ac4a881..adc0fe55e 100644 --- a/src/internal/operations/restore_test.go +++ b/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)) diff --git a/src/pkg/repository/repository.go b/src/pkg/repository/repository.go index 5c96e27e1..1e340f9dc 100644 --- a/src/pkg/repository/repository.go +++ b/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) } diff --git a/src/pkg/selectors/selectors.go b/src/pkg/selectors/selectors.go index caf1f5cb1..17247b39b 100644 --- a/src/pkg/selectors/selectors.go +++ b/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 { diff --git a/src/pkg/selectors/selectors_test.go b/src/pkg/selectors/selectors_test.go index 8651d1823..28ed90198 100644 --- a/src/pkg/selectors/selectors_test.go +++ b/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"}