diff --git a/src/cli/backup/backup.go b/src/cli/backup/backup.go index f407f084f..feb2fe594 100644 --- a/src/cli/backup/backup.go +++ b/src/cli/backup/backup.go @@ -195,6 +195,8 @@ func runBackups( r repository.Repository, serviceName, resourceOwnerType string, selectorSet []selectors.Selector, + resourceOwnersIDToName map[string]string, + resourceOwnersNameToID map[string]string, ) error { var ( bIDs []model.StableID @@ -204,21 +206,21 @@ func runBackups( for _, discSel := range selectorSet { var ( owner = discSel.DiscreteOwner - bctx = clues.Add(ctx, "resource_owner", owner) + ictx = clues.Add(ctx, "resource_owner", owner) ) - bo, err := r.NewBackup(bctx, discSel) + bo, err := r.NewBackup(ictx, discSel, resourceOwnersIDToName, resourceOwnersNameToID) if err != nil { - errs = append(errs, clues.Wrap(err, owner).WithClues(bctx)) - Errf(bctx, "%v\n", err) + errs = append(errs, clues.Wrap(err, owner).WithClues(ictx)) + Errf(ictx, "%v\n", err) continue } - err = bo.Run(bctx) + err = bo.Run(ictx) if err != nil { - errs = append(errs, clues.Wrap(err, owner).WithClues(bctx)) - Errf(bctx, "%v\n", err) + errs = append(errs, clues.Wrap(err, owner).WithClues(ictx)) + Errf(ictx, "%v\n", err) continue } diff --git a/src/cli/backup/exchange.go b/src/cli/backup/exchange.go index 81c3cad64..aa1818be3 100644 --- a/src/cli/backup/exchange.go +++ b/src/cli/backup/exchange.go @@ -7,6 +7,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" + "golang.org/x/exp/maps" "github.com/alcionai/corso/src/cli/options" . "github.com/alcionai/corso/src/cli/print" @@ -164,14 +165,14 @@ func createExchangeCmd(cmd *cobra.Command, args []string) error { // TODO: log/print recoverable errors errs := fault.New(false) - users, err := m365.UserPNs(ctx, *acct, errs) + idToPN, pnToID, err := m365.UsersMap(ctx, *acct, errs) if err != nil { return Only(ctx, clues.Wrap(err, "Failed to retrieve M365 user(s)")) } selectorSet := []selectors.Selector{} - for _, discSel := range sel.SplitByResourceOwner(users) { + for _, discSel := range sel.SplitByResourceOwner(maps.Keys(pnToID)) { selectorSet = append(selectorSet, discSel.Selector) } @@ -180,7 +181,7 @@ func createExchangeCmd(cmd *cobra.Command, args []string) error { r, "Exchange", "user", selectorSet, - ) + idToPN, pnToID) } func exchangeBackupCreateSelectors(userIDs, cats []string) *selectors.ExchangeBackup { diff --git a/src/cli/backup/exchange_e2e_test.go b/src/cli/backup/exchange_e2e_test.go index f25e0ecfd..cd9de2a84 100644 --- a/src/cli/backup/exchange_e2e_test.go +++ b/src/cli/backup/exchange_e2e_test.go @@ -300,7 +300,11 @@ func (suite *PreparedBackupExchangeE2ESuite) SetupSuite() { suite.backupOps = make(map[path.CategoryType]string) - users := []string{suite.m365UserID} + var ( + users = []string{suite.m365UserID} + idToName = map[string]string{suite.m365UserID: "todo-name-" + suite.m365UserID} + nameToID = map[string]string{"todo-name-" + suite.m365UserID: suite.m365UserID} + ) for _, set := range backupDataSets { var ( @@ -321,7 +325,7 @@ func (suite *PreparedBackupExchangeE2ESuite) SetupSuite() { sel.Include(scopes) - bop, err := suite.repo.NewBackup(ctx, sel.Selector) + bop, err := suite.repo.NewBackup(ctx, sel.Selector, idToName, nameToID) require.NoError(t, err, clues.ToCore(err)) err = bop.Run(ctx) @@ -546,7 +550,7 @@ func (suite *BackupDeleteExchangeE2ESuite) SetupSuite() { sel := selectors.NewExchangeBackup(users) sel.Include(sel.MailFolders([]string{exchange.DefaultMailFolder}, selectors.PrefixMatch())) - suite.backupOp, err = suite.repo.NewBackup(ctx, sel.Selector) + suite.backupOp, err = suite.repo.NewBackup(ctx, sel.Selector, nil, nil) require.NoError(t, err, clues.ToCore(err)) err = suite.backupOp.Run(ctx) diff --git a/src/cli/backup/onedrive.go b/src/cli/backup/onedrive.go index befaacf76..fd5a35630 100644 --- a/src/cli/backup/onedrive.go +++ b/src/cli/backup/onedrive.go @@ -7,6 +7,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" + "golang.org/x/exp/maps" "github.com/alcionai/corso/src/cli/options" . "github.com/alcionai/corso/src/cli/print" @@ -148,14 +149,14 @@ func createOneDriveCmd(cmd *cobra.Command, args []string) error { // TODO: log/print recoverable errors errs := fault.New(false) - users, err := m365.UserPNs(ctx, *acct, errs) + idToName, nameToID, err := m365.UsersMap(ctx, *acct, errs) if err != nil { return Only(ctx, clues.Wrap(err, "Failed to retrieve M365 users")) } selectorSet := []selectors.Selector{} - for _, discSel := range sel.SplitByResourceOwner(users) { + for _, discSel := range sel.SplitByResourceOwner(maps.Keys(idToName)) { selectorSet = append(selectorSet, discSel.Selector) } @@ -164,7 +165,7 @@ func createOneDriveCmd(cmd *cobra.Command, args []string) error { r, "OneDrive", "user", selectorSet, - ) + idToName, nameToID) } func validateOneDriveBackupCreateFlags(users []string) error { diff --git a/src/cli/backup/onedrive_e2e_test.go b/src/cli/backup/onedrive_e2e_test.go index 515002f31..0e84c6684 100644 --- a/src/cli/backup/onedrive_e2e_test.go +++ b/src/cli/backup/onedrive_e2e_test.go @@ -205,14 +205,18 @@ func (suite *BackupDeleteOneDriveE2ESuite) SetupSuite() { }) require.NoError(t, err, clues.ToCore(err)) - m365UserID := tester.M365UserID(t) - users := []string{m365UserID} + var ( + m365UserID = tester.M365UserID(t) + users = []string{m365UserID} + idToName = map[string]string{m365UserID: "todo-name-" + m365UserID} + nameToID = map[string]string{"todo-name-" + m365UserID: m365UserID} + ) // some tests require an existing backup sel := selectors.NewOneDriveBackup(users) sel.Include(sel.Folders(selectors.Any())) - suite.backupOp, err = suite.repo.NewBackup(ctx, sel.Selector) + suite.backupOp, err = suite.repo.NewBackup(ctx, sel.Selector, idToName, nameToID) require.NoError(t, err, clues.ToCore(err)) err = suite.backupOp.Run(ctx) diff --git a/src/cli/backup/sharepoint.go b/src/cli/backup/sharepoint.go index de32575bb..77da2f37c 100644 --- a/src/cli/backup/sharepoint.go +++ b/src/cli/backup/sharepoint.go @@ -154,6 +154,7 @@ func createSharePointCmd(cmd *cobra.Command, args []string) error { // TODO: log/print recoverable errors errs := fault.New(false) + // TODO: discovery of sharepoint sites instead of early GC construction. gc, err := connector.NewGraphConnector(ctx, graph.HTTPClient(graph.NoTimeout()), *acct, connector.Sites, errs) if err != nil { return Only(ctx, clues.Wrap(err, "Failed to connect to Microsoft APIs")) @@ -175,7 +176,7 @@ func createSharePointCmd(cmd *cobra.Command, args []string) error { r, "SharePoint", "site", selectorSet, - ) + nil, nil) } func validateSharePointBackupCreateFlags(sites, weburls, cats []string) error { diff --git a/src/cli/backup/sharepoint_e2e_test.go b/src/cli/backup/sharepoint_e2e_test.go index 94289a7d5..ef96ca522 100644 --- a/src/cli/backup/sharepoint_e2e_test.go +++ b/src/cli/backup/sharepoint_e2e_test.go @@ -156,14 +156,18 @@ func (suite *BackupDeleteSharePointE2ESuite) SetupSuite() { suite.repo, err = repository.Initialize(ctx, suite.acct, suite.st, control.Options{}) require.NoError(t, err, clues.ToCore(err)) - m365SiteID := tester.M365SiteID(t) - sites := []string{m365SiteID} + var ( + m365SiteID = tester.M365SiteID(t) + sites = []string{m365SiteID} + idToName = map[string]string{m365SiteID: "todo-name-" + m365SiteID} + nameToID = map[string]string{"todo-name-" + m365SiteID: m365SiteID} + ) // some tests require an existing backup sel := selectors.NewSharePointBackup(sites) sel.Include(sel.LibraryFolders(selectors.Any())) - suite.backupOp, err = suite.repo.NewBackup(ctx, sel.Selector) + suite.backupOp, err = suite.repo.NewBackup(ctx, sel.Selector, idToName, nameToID) require.NoError(t, err, clues.ToCore(err)) err = suite.backupOp.Run(ctx) diff --git a/src/cli/restore/exchange_e2e_test.go b/src/cli/restore/exchange_e2e_test.go index 2c03fd6bb..7c8787771 100644 --- a/src/cli/restore/exchange_e2e_test.go +++ b/src/cli/restore/exchange_e2e_test.go @@ -73,7 +73,12 @@ func (suite *RestoreExchangeE2ESuite) SetupSuite() { suite.vpr, suite.cfgFP = tester.MakeTempTestConfigClone(t, force) suite.m365UserID = tester.M365UserID(t) - users := []string{suite.m365UserID} + + var ( + users = []string{suite.m365UserID} + idToName = map[string]string{suite.m365UserID: "todo-name-" + suite.m365UserID} + nameToID = map[string]string{"todo-name-" + suite.m365UserID: suite.m365UserID} + ) // init the repo first suite.repo, err = repository.Initialize(ctx, suite.acct, suite.st, control.Options{}) @@ -100,7 +105,7 @@ func (suite *RestoreExchangeE2ESuite) SetupSuite() { sel.Include(scopes) - bop, err := suite.repo.NewBackup(ctx, sel.Selector) + bop, err := suite.repo.NewBackup(ctx, sel.Selector, idToName, nameToID) require.NoError(t, err, clues.ToCore(err)) err = bop.Run(ctx) diff --git a/src/cmd/factory/impl/common.go b/src/cmd/factory/impl/common.go index 4a8ee1b52..5aafa4de8 100644 --- a/src/cmd/factory/impl/common.go +++ b/src/cmd/factory/impl/common.go @@ -8,6 +8,7 @@ import ( "github.com/alcionai/clues" "github.com/google/uuid" + "golang.org/x/exp/maps" "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/internal/common" @@ -126,12 +127,12 @@ func getGCAndVerifyUser(ctx context.Context, userID string) (*connector.GraphCon errs := fault.New(false) normUsers := map[string]struct{}{} - users, err := m365.UserPNs(ctx, acct, errs) + idToName, _, err := m365.UsersMap(ctx, acct, errs) if err != nil { return nil, account.Account{}, clues.Wrap(err, "getting tenant users") } - for _, k := range users { + for _, k := range maps.Keys(idToName) { normUsers[strings.ToLower(k)] = struct{}{} } diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index d62cd2bd8..b9119c5a5 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -42,9 +42,17 @@ type GraphConnector struct { itemClient *http.Client // configured to handle large item downloads tenant string - Sites map[string]string // webURL -> siteID and siteID -> webURL credentials account.M365Config + // TODO: remove in favor of the maps below. + Sites map[string]string // webURL -> siteID and siteID -> webURL + + // maps of resource owner ids to names, and names to ids. + // not guaranteed to be populated, only here as a post-population + // reference for processes that choose to populate the values. + ResourceOwnerIDToName map[string]string + ResourceOwnerNameToID map[string]string + // wg is used to track completion of GC tasks wg *sync.WaitGroup region *trace.Region @@ -101,6 +109,63 @@ func NewGraphConnector( return &gc, nil } +// PopulateOwnerIDAndNamesFrom takes the provided owner identifier and produces +// the owner's name and ID from that value. Returns an error if the owner is +// not recognized by the current tenant. +// +// The id-name maps are optional. Some processes will look up all owners in +// the tenant before reaching this step. In that case, the data gets handed +// down for this func to consume instead of performing further queries. The +// maps get stored inside the gc instance for later re-use. +// +// TODO: If the maps are nil or empty, this func will perform a lookup on the given +// owner, and populate each map with that owner's id and name for downstream +// guarantees about that data being present. Optional performance enhancement +// idea: downstream from here, we should _only_ need the given user's id and name, +// and could store minimal map copies with that info instead of the whole tenant. +func (gc *GraphConnector) PopulateOwnerIDAndNamesFrom( + owner string, // input value, can be either id or name + idToName, nameToID map[string]string, // optionally pre-populated lookups +) (string, string, error) { + // ensure the maps exist, even if they aren't populated so that + // getOwnerIDAndNameFrom can populate any values it looks up. + if len(idToName) == 0 { + idToName = map[string]string{} + } + + if len(nameToID) == 0 { + nameToID = map[string]string{} + } + + // move this to GC method + id, name, err := getOwnerIDAndNameFrom(owner, idToName, nameToID) + if err != nil { + return "", "", errors.Wrap(err, "resolving resource owner details") + } + + gc.ResourceOwnerIDToName = idToName + gc.ResourceOwnerNameToID = nameToID + + return id, name, nil +} + +func getOwnerIDAndNameFrom( + owner string, + idToName, nameToID map[string]string, +) (string, string, error) { + if n, ok := idToName[owner]; ok { + return owner, n, nil + } else if i, ok := nameToID[owner]; ok { + return i, owner, nil + } + + // TODO: look-up user by owner, either id or name, + // and populate with maps as a result. Only + // return owner, owner as a very last resort. + + return owner, owner, nil +} + // createService constructor for graphService component func (gc *GraphConnector) createService() (*graph.Service, error) { adapter, err := graph.CreateAdapter( diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index 80de50a53..782c85243 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -123,11 +123,11 @@ func (suite *GraphConnectorUnitSuite) TestUnionSiteIDsAndWebURLs() { } for _, test := range table { suite.Run(test.name, func() { - t := suite.T() - ctx, flush := tester.NewContext() defer flush() + t := suite.T() + result, err := gc.UnionSiteIDsAndWebURLs(ctx, test.ids, test.urls, fault.New(true)) assert.NoError(t, err, clues.ToCore(err)) assert.ElementsMatch(t, test.expect, result) @@ -135,6 +135,113 @@ func (suite *GraphConnectorUnitSuite) TestUnionSiteIDsAndWebURLs() { } } +func (suite *GraphConnectorUnitSuite) TestPopulateOwnerIDAndNamesFrom() { + const ( + ownerID = "owner-id" + ownerName = "owner-name" + ) + + var ( + itn = map[string]string{ownerID: ownerName} + nti = map[string]string{ownerName: ownerID} + ) + + table := []struct { + name string + owner string + idToName map[string]string + nameToID map[string]string + expectID string + expectName string + }{ + { + name: "nil maps", + owner: ownerID, + idToName: nil, + nameToID: nil, + expectID: ownerID, + expectName: ownerID, + }, + { + name: "only id map with owner id", + owner: ownerID, + idToName: itn, + nameToID: nil, + expectID: ownerID, + expectName: ownerName, + }, + { + name: "only name map with owner id", + owner: ownerID, + idToName: nil, + nameToID: nti, + expectID: ownerID, + expectName: ownerID, + }, + { + name: "only id map with owner name", + owner: ownerName, + idToName: itn, + nameToID: nil, + expectID: ownerName, + expectName: ownerName, + }, + { + name: "only name map with owner name", + owner: ownerName, + idToName: nil, + nameToID: nti, + expectID: ownerID, + expectName: ownerName, + }, + { + name: "both maps with owner id", + owner: ownerID, + idToName: itn, + nameToID: nti, + expectID: ownerID, + expectName: ownerName, + }, + { + name: "both maps with owner name", + owner: ownerName, + idToName: itn, + nameToID: nti, + expectID: ownerID, + expectName: ownerName, + }, + { + name: "non-matching maps with owner id", + owner: ownerID, + idToName: map[string]string{"foo": "bar"}, + nameToID: map[string]string{"fnords": "smarf"}, + expectID: ownerID, + expectName: ownerID, + }, + { + name: "non-matching with owner name", + owner: ownerName, + idToName: map[string]string{"foo": "bar"}, + nameToID: map[string]string{"fnords": "smarf"}, + expectID: ownerName, + expectName: ownerName, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + var ( + t = suite.T() + gc = &GraphConnector{} + ) + + id, name, err := gc.PopulateOwnerIDAndNamesFrom(test.owner, test.idToName, test.nameToID) + require.NoError(t, err, clues.ToCore(err)) + assert.Equal(t, test.expectID, id) + assert.Equal(t, test.expectName, name) + }) + } +} + func (suite *GraphConnectorUnitSuite) TestGraphConnector_Wait() { ctx, flush := tester.NewContext() defer flush() diff --git a/src/pkg/repository/loadtest/repository_load_test.go b/src/pkg/repository/loadtest/repository_load_test.go index 72093a27f..cd8f24140 100644 --- a/src/pkg/repository/loadtest/repository_load_test.go +++ b/src/pkg/repository/loadtest/repository_load_test.go @@ -120,7 +120,7 @@ func runLoadTest( ) { //revive:enable:context-as-argument t.Run(prefix+"_load_test_main", func(t *testing.T) { - b, err := r.NewBackup(ctx, bupSel) + b, err := r.NewBackup(ctx, bupSel, nil, nil) require.NoError(t, err, clues.ToCore(err)) runBackupLoadTest(t, ctx, &b, service, usersUnderTest) @@ -447,8 +447,7 @@ func (suite *LoadExchangeSuite) TestExchange() { "all_users", "exchange", suite.usersUnderTest, sel, sel, // same selection for backup and restore - true, - ) + true) } // single user, lots of data @@ -500,8 +499,7 @@ func (suite *IndividualLoadExchangeSuite) TestExchange() { "single_user", "exchange", suite.usersUnderTest, sel, sel, // same selection for backup and restore - true, - ) + true) } // ------------------------------------------------------------------------------------------------ @@ -553,8 +551,7 @@ func (suite *LoadOneDriveSuite) TestOneDrive() { "all_users", "one_drive", suite.usersUnderTest, sel, sel, // same selection for backup and restore - false, - ) + false) } type IndividualLoadOneDriveSuite struct { @@ -601,8 +598,7 @@ func (suite *IndividualLoadOneDriveSuite) TestOneDrive() { "single_user", "one_drive", suite.usersUnderTest, sel, sel, // same selection for backup and restore - false, - ) + false) } // ------------------------------------------------------------------------------------------------ @@ -654,8 +650,7 @@ func (suite *LoadSharePointSuite) TestSharePoint() { "all_sites", "share_point", suite.sitesUnderTest, sel, sel, // same selection for backup and restore - false, - ) + false) } type IndividualLoadSharePointSuite struct { @@ -703,6 +698,5 @@ func (suite *IndividualLoadSharePointSuite) TestSharePoint() { "single_site", "share_point", suite.sitesUnderTest, sel, sel, // same selection for backup and restore - false, - ) + false) } diff --git a/src/pkg/repository/repository.go b/src/pkg/repository/repository.go index 60cdccda8..fa0c21aa8 100644 --- a/src/pkg/repository/repository.go +++ b/src/pkg/repository/repository.go @@ -54,6 +54,7 @@ type Repository interface { NewBackup( ctx context.Context, self selectors.Selector, + ownerIDToName, ownerNameToID map[string]string, ) (operations.BackupOperation, error) NewRestore( ctx context.Context, @@ -283,17 +284,25 @@ func (r *repository) Close(ctx context.Context) error { } // NewBackup generates a BackupOperation runner. +// ownerIDToName and ownerNameToID are optional populations, in case the caller has +// already generated those values. func (r repository) NewBackup( ctx context.Context, sel selectors.Selector, + ownerIDToName, ownerNameToID map[string]string, ) (operations.BackupOperation, error) { gc, err := connectToM365(ctx, sel, r.Account, fault.New(true)) if err != nil { return operations.BackupOperation{}, errors.Wrap(err, "connecting to m365") } + ownerID, ownerName, err := gc.PopulateOwnerIDAndNamesFrom(sel.DiscreteOwner, ownerIDToName, ownerNameToID) + if err != nil { + return operations.BackupOperation{}, errors.Wrap(err, "resolving resource owner details") + } + // TODO: retrieve display name from gc - sel = sel.SetDiscreteOwnerIDName(sel.DiscreteOwner, "") + sel = sel.SetDiscreteOwnerIDName(ownerID, ownerName) return operations.NewBackupOperation( ctx, diff --git a/src/pkg/repository/repository_test.go b/src/pkg/repository/repository_test.go index cdea26d27..30663537a 100644 --- a/src/pkg/repository/repository_test.go +++ b/src/pkg/repository/repository_test.go @@ -198,7 +198,7 @@ func (suite *RepositoryIntegrationSuite) TestNewBackup() { r, err := repository.Initialize(ctx, acct, st, control.Options{}) require.NoError(t, err, clues.ToCore(err)) - bo, err := r.NewBackup(ctx, selectors.Selector{DiscreteOwner: "test"}) + bo, err := r.NewBackup(ctx, selectors.Selector{DiscreteOwner: "test"}, nil, nil) require.NoError(t, err, clues.ToCore(err)) require.NotNil(t, bo) } diff --git a/src/pkg/services/m365/m365.go b/src/pkg/services/m365/m365.go index e078152cf..c76639721 100644 --- a/src/pkg/services/m365/m365.go +++ b/src/pkg/services/m365/m365.go @@ -55,34 +55,29 @@ func Users(ctx context.Context, acct account.Account, errs *fault.Bus) ([]*User, return ret, nil } -func UserIDs(ctx context.Context, acct account.Account, errs *fault.Bus) ([]string, error) { +// UsersMap retrieves all users in the tenant, and returns two maps: one id-to-principalName, +// and one principalName-to-id. +func UsersMap( + ctx context.Context, + acct account.Account, + errs *fault.Bus, +) (map[string]string, map[string]string, error) { users, err := Users(ctx, acct, errs) if err != nil { - return nil, err + return nil, nil, err } - ret := make([]string, 0, len(users)) + var ( + idToName = make(map[string]string, len(users)) + nameToID = make(map[string]string, len(users)) + ) + for _, u := range users { - ret = append(ret, u.ID) + idToName[u.ID] = u.PrincipalName + nameToID[u.PrincipalName] = u.ID } - return ret, nil -} - -// UserPNs retrieves all user principleNames in the tenant. Principle Names -// can be used analogous userIDs in graph API queries. -func UserPNs(ctx context.Context, acct account.Account, errs *fault.Bus) ([]string, error) { - users, err := Users(ctx, acct, errs) - if err != nil { - return nil, err - } - - ret := make([]string, 0, len(users)) - for _, u := range users { - ret = append(ret, u.PrincipalName) - } - - return ret, nil + return idToName, nameToID, nil } type Site struct {