ditch userInfo middle struct (#3206)

we don't need a m365.UserInfo when we already have an api.UserInfo

---

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

- [x]  No

#### Type of change

- [x] 🧹 Tech Debt/Cleanup

#### Test Plan

- [x]  Unit test
- [x] 💚 E2E
This commit is contained in:
Keepers 2023-04-25 13:05:07 -06:00 committed by GitHub
parent 8a2e63dcad
commit c95a07660a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 91 additions and 88 deletions

View File

@ -17,7 +17,6 @@ import (
"github.com/alcionai/corso/src/pkg/path"
"github.com/alcionai/corso/src/pkg/repository"
"github.com/alcionai/corso/src/pkg/selectors"
"github.com/alcionai/corso/src/pkg/services/m365"
)
// ------------------------------------------------------------------------------------------------
@ -162,10 +161,7 @@ func createExchangeCmd(cmd *cobra.Command, args []string) error {
sel := exchangeBackupCreateSelectors(utils.UserFV, utils.CategoryDataFV)
// TODO: log/print recoverable errors
errs := fault.New(false)
ins, err := m365.UsersMap(ctx, *acct, errs)
ins, err := utils.UsersMap(ctx, *acct, fault.New(true))
if err != nil {
return Only(ctx, clues.Wrap(err, "Failed to retrieve M365 users"))
}

View File

@ -17,7 +17,6 @@ import (
"github.com/alcionai/corso/src/pkg/path"
"github.com/alcionai/corso/src/pkg/repository"
"github.com/alcionai/corso/src/pkg/selectors"
"github.com/alcionai/corso/src/pkg/services/m365"
)
// ------------------------------------------------------------------------------------------------
@ -144,10 +143,7 @@ func createOneDriveCmd(cmd *cobra.Command, args []string) error {
sel := oneDriveBackupCreateSelectors(utils.UserFV)
// TODO: log/print recoverable errors
errs := fault.New(false)
ins, err := m365.UsersMap(ctx, *acct, errs)
ins, err := utils.UsersMap(ctx, *acct, fault.New(true))
if err != nil {
return Only(ctx, clues.Wrap(err, "Failed to retrieve M365 users"))
}

40
src/cli/utils/users.go Normal file
View File

@ -0,0 +1,40 @@
package utils
import (
"context"
"github.com/alcionai/clues"
"github.com/alcionai/corso/src/internal/common/idname"
"github.com/alcionai/corso/src/pkg/account"
"github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/services/m365/api"
)
// UsersMap retrieves all users in the tenant and returns them in an idname.Cacher
func UsersMap(
ctx context.Context,
acct account.Account,
errs *fault.Bus,
) (idname.Cacher, error) {
au, err := makeUserAPI(acct)
if err != nil {
return nil, clues.Wrap(err, "constructing a graph client")
}
return au.GetAllIDsAndNames(ctx, errs)
}
func makeUserAPI(acct account.Account) (api.Users, error) {
creds, err := acct.M365Config()
if err != nil {
return api.Users{}, clues.Wrap(err, "getting m365 account creds")
}
cli, err := api.NewClient(creds)
if err != nil {
return api.Users{}, clues.Wrap(err, "constructing api client")
}
return cli.Users(), nil
}

View File

@ -3,7 +3,6 @@ package impl
import (
"context"
"os"
"strings"
"time"
"github.com/alcionai/clues"
@ -22,7 +21,6 @@ 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/services/m365"
)
var (
@ -119,21 +117,6 @@ func getGCAndVerifyUser(ctx context.Context, userID string) (*connector.GraphCon
return nil, account.Account{}, clues.Wrap(err, "finding m365 account details")
}
// TODO: log/print recoverable errors
errs := fault.New(false)
ins, err := m365.UsersMap(ctx, acct, errs)
if err != nil {
return nil, account.Account{}, clues.Wrap(err, "getting tenant users")
}
_, idOK := ins.NameOf(strings.ToLower(userID))
_, nameOK := ins.IDOf(strings.ToLower(userID))
if !idOK && !nameOK {
return nil, account.Account{}, clues.New("user not found within tenant")
}
gc, err := connector.NewGraphConnector(
ctx,
acct,
@ -142,6 +125,10 @@ func getGCAndVerifyUser(ctx context.Context, userID string) (*connector.GraphCon
return nil, account.Account{}, clues.Wrap(err, "connecting to graph api")
}
if _, _, err := gc.PopulateOwnerIDAndNamesFrom(ctx, userID, nil); err != nil {
return nil, account.Account{}, clues.Wrap(err, "verifying user")
}
return gc, acct, nil
}

View File

@ -193,7 +193,7 @@ func (suite *DiscoveryIntgSuite) TestUserInfo() {
name: "standard test user",
user: tester.M365UserID(t),
expect: &api.UserInfo{
DiscoveredServices: map[path.ServiceType]struct{}{
ServicesEnabled: map[path.ServiceType]struct{}{
path.ExchangeService: {},
path.OneDriveService: {},
},
@ -208,8 +208,8 @@ func (suite *DiscoveryIntgSuite) TestUserInfo() {
name: "user does not exist",
user: uuid.NewString(),
expect: &api.UserInfo{
DiscoveredServices: map[path.ServiceType]struct{}{},
Mailbox: api.MailboxInfo{},
ServicesEnabled: map[path.ServiceType]struct{}{},
Mailbox: api.MailboxInfo{},
},
expectErr: require.NoError,
},
@ -228,7 +228,7 @@ func (suite *DiscoveryIntgSuite) TestUserInfo() {
return
}
assert.Equal(t, test.expect.DiscoveredServices, result.DiscoveredServices)
assert.Equal(t, test.expect.ServicesEnabled, result.ServicesEnabled)
})
}
}
@ -247,7 +247,7 @@ func (suite *DiscoveryIntgSuite) TestUserWithoutDrive() {
name: "user without drive and exchange",
user: "a53c26f7-5100-4acb-a910-4d20960b2c19", // User: testevents@10rqc2.onmicrosoft.com
expect: &api.UserInfo{
DiscoveredServices: map[path.ServiceType]struct{}{},
ServicesEnabled: map[path.ServiceType]struct{}{},
Mailbox: api.MailboxInfo{
ErrGetMailBoxSetting: []error{api.ErrMailBoxSettingsNotFound},
},
@ -257,7 +257,7 @@ func (suite *DiscoveryIntgSuite) TestUserWithoutDrive() {
name: "user with drive and exchange",
user: userID,
expect: &api.UserInfo{
DiscoveredServices: map[path.ServiceType]struct{}{
ServicesEnabled: map[path.ServiceType]struct{}{
path.ExchangeService: {},
path.OneDriveService: {},
},
@ -277,7 +277,7 @@ func (suite *DiscoveryIntgSuite) TestUserWithoutDrive() {
result, err := discovery.GetUserInfo(ctx, acct, test.user, fault.New(true))
require.NoError(t, err, clues.ToCore(err))
assert.Equal(t, test.expect.DiscoveredServices, result.DiscoveredServices)
assert.Equal(t, test.expect.ServicesEnabled, result.ServicesEnabled)
assert.Equal(t, test.expect.Mailbox.ErrGetMailBoxSetting, result.Mailbox.ErrGetMailBoxSetting)
assert.Equal(t, test.expect.Mailbox.Purpose, result.Mailbox.Purpose)
})

View File

@ -4,6 +4,7 @@ import (
"context"
"fmt"
"net/http"
"strings"
"github.com/alcionai/clues"
abstractions "github.com/microsoft/kiota-abstractions-go"
@ -11,6 +12,7 @@ import (
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/microsoftgraph/msgraph-sdk-go/users"
"github.com/alcionai/corso/src/internal/common/idname"
"github.com/alcionai/corso/src/internal/common/ptr"
"github.com/alcionai/corso/src/internal/connector/graph"
"github.com/alcionai/corso/src/pkg/fault"
@ -41,8 +43,8 @@ type Users struct {
// ---------------------------------------------------------------------------
type UserInfo struct {
DiscoveredServices map[path.ServiceType]struct{}
Mailbox MailboxInfo
ServicesEnabled map[path.ServiceType]struct{}
Mailbox MailboxInfo
}
type MailboxInfo struct {
@ -88,7 +90,7 @@ type WorkingHours struct {
func newUserInfo() *UserInfo {
return &UserInfo{
DiscoveredServices: map[path.ServiceType]struct{}{
ServicesEnabled: map[path.ServiceType]struct{}{
path.ExchangeService: {},
path.OneDriveService: {},
},
@ -98,11 +100,11 @@ func newUserInfo() *UserInfo {
// ServiceEnabled returns true if the UserInfo has an entry for the
// service. If no entry exists, the service is assumed to not be enabled.
func (ui *UserInfo) ServiceEnabled(service path.ServiceType) bool {
if ui == nil || len(ui.DiscoveredServices) == 0 {
if ui == nil || len(ui.ServicesEnabled) == 0 {
return false
}
_, ok := ui.DiscoveredServices[service]
_, ok := ui.ServicesEnabled[service]
return ok
}
@ -225,6 +227,25 @@ func (c Users) GetIDAndName(ctx context.Context, userID string) (string, string,
return ptr.Val(u.GetId()), ptr.Val(u.GetUserPrincipalName()), nil
}
// GetAllIDsAndNames retrieves all users in the tenant and returns them in an idname.Cacher
func (c Users) GetAllIDsAndNames(ctx context.Context, errs *fault.Bus) (idname.Cacher, error) {
all, err := c.GetAll(ctx, errs)
if err != nil {
return nil, clues.Wrap(err, "getting all users")
}
idToName := make(map[string]string, len(all))
for _, u := range all {
id := strings.ToLower(ptr.Val(u.GetId()))
name := strings.ToLower(ptr.Val(u.GetUserPrincipalName()))
idToName[id] = name
}
return idname.NewCache(idToName), nil
}
func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) {
// Assume all services are enabled
// then filter down to only services the user has enabled
@ -252,7 +273,7 @@ func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) {
}
logger.Ctx(ctx).Info("resource owner does not have a mailbox enabled")
delete(userInfo.DiscoveredServices, path.ExchangeService)
delete(userInfo.ServicesEnabled, path.ExchangeService)
}
if _, err := c.GetDrives(ctx, userID); err != nil {
@ -264,7 +285,7 @@ func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) {
logger.Ctx(ctx).Info("resource owner does not have a drive")
delete(userInfo.DiscoveredServices, path.OneDriveService)
delete(userInfo.ServicesEnabled, path.OneDriveService)
}
mbxInfo, err := c.getMailboxSettings(ctx, userID)

View File

@ -2,7 +2,6 @@ package m365
import (
"context"
"strings"
"github.com/alcionai/clues"
"github.com/microsoftgraph/msgraph-sdk-go/models"
@ -12,7 +11,6 @@ import (
"github.com/alcionai/corso/src/internal/connector/discovery"
"github.com/alcionai/corso/src/pkg/account"
"github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/path"
"github.com/alcionai/corso/src/pkg/services/m365/api"
)
@ -35,10 +33,6 @@ type User struct {
Info api.UserInfo
}
type UserInfo struct {
ServicesEnabled ServiceAccess
}
// UsersCompat returns a list of users in the specified M365 tenant.
// TODO(ashmrtn): Remove when upstream consumers of the SDK support the fault
// package.
@ -102,40 +96,12 @@ func parseUser(item models.Userable) (*User, error) {
return u, nil
}
// 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,
) (idname.Cacher, error) {
users, err := Users(ctx, acct, errs)
if err != nil {
return idname.NewCache(nil), err
}
var (
idToName = make(map[string]string, len(users))
nameToID = make(map[string]string, len(users))
)
for _, u := range users {
id, name := strings.ToLower(u.ID), strings.ToLower(u.PrincipalName)
idToName[id] = name
nameToID[name] = id
}
ins := idname.NewCache(idToName)
return ins, nil
}
// UserInfo returns the corso-specific set of user metadata.
func GetUserInfo(
ctx context.Context,
acct account.Account,
userID string,
) (*UserInfo, error) {
) (*api.UserInfo, error) {
uapi, err := makeUserAPI(acct)
if err != nil {
return nil, clues.Wrap(err, "getting user info").WithClues(ctx)
@ -146,13 +112,7 @@ func GetUserInfo(
return nil, err
}
info := UserInfo{
ServicesEnabled: ServiceAccess{
Exchange: ui.ServiceEnabled(path.ExchangeService),
},
}
return &info, nil
return ui, nil
}
// ---------------------------------------------------------------------------

View File

@ -10,6 +10,7 @@ import (
"github.com/alcionai/corso/src/internal/tester"
"github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/path"
"github.com/alcionai/corso/src/pkg/services/m365"
)
@ -66,13 +67,15 @@ func (suite *M365IntegrationSuite) TestGetUserInfo() {
require.NotNil(t, info)
require.NotEmpty(t, info)
expect := &m365.UserInfo{
ServicesEnabled: m365.ServiceAccess{
Exchange: true,
},
expectEnabled := map[path.ServiceType]struct{}{
path.ExchangeService: {},
path.OneDriveService: {},
}
assert.Equal(t, expect, info)
assert.NotEmpty(t, info.ServicesEnabled)
assert.NotEmpty(t, info.Mailbox)
assert.Equal(t, expectEnabled, info.ServicesEnabled)
assert.Equal(t, "user", info.Mailbox.Purpose)
}
func (suite *M365IntegrationSuite) TestSites() {