From 084b519766f0e6f56181ddb381475e85ae7b66d3 Mon Sep 17 00:00:00 2001 From: Vaibhav Kamra Date: Fri, 28 Apr 2023 08:22:55 -0700 Subject: [PATCH] Introduce a UserCompatNoInfo call (#3257) Instead of returning `Info` in `User`, callers should call the `GetUserInfo` API to get extended user information. This allows the caller to decide whether info is needed *and* control parallelism for these queries. This change introduces a `UserCompatNoInfo` call that allows us to make the switch without breaking existing `UserCompat` references. We will remove this API after callers have switched. --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Test Plan - [ ] :muscle: Manual - [ ] :zap: Unit test - [x] :green_heart: E2E --- src/pkg/services/m365/m365.go | 56 ++++++++++++++++++++++++++++++ src/pkg/services/m365/m365_test.go | 24 +++++++++++++ 2 files changed, 80 insertions(+) diff --git a/src/pkg/services/m365/m365.go b/src/pkg/services/m365/m365.go index ca2c48fa2..f4851e9ef 100644 --- a/src/pkg/services/m365/m365.go +++ b/src/pkg/services/m365/m365.go @@ -33,6 +33,14 @@ type User struct { Info api.UserInfo } +// UserNoInfo is the minimal information required to identify and display a user. +// TODO: Remove this once `UsersCompatNoInfo` is removed +type UserNoInfo struct { + PrincipalName string + ID string + Name string +} + // UsersCompat returns a list of users in the specified M365 tenant. // TODO(ashmrtn): Remove when upstream consumers of the SDK support the fault // package. @@ -47,6 +55,54 @@ func UsersCompat(ctx context.Context, acct account.Account) ([]*User, error) { return users, errs.Failure() } +// UsersCompatNoInfo returns a list of users in the specified M365 tenant. +// TODO: Remove this once `Info` is removed from the `User` struct and callers +// have switched over +func UsersCompatNoInfo(ctx context.Context, acct account.Account) ([]*UserNoInfo, error) { + errs := fault.New(true) + + users, err := usersNoInfo(ctx, acct, errs) + if err != nil { + return nil, err + } + + return users, errs.Failure() +} + +// usersNoInfo returns a list of users in the specified M365 tenant - with no info +// TODO: Remove this once we remove `Info` from `Users` and instead rely on the `GetUserInfo` API +// to get user information +func usersNoInfo(ctx context.Context, acct account.Account, errs *fault.Bus) ([]*UserNoInfo, error) { + uapi, err := makeUserAPI(acct) + if err != nil { + return nil, clues.Wrap(err, "getting users").WithClues(ctx) + } + + users, err := discovery.Users(ctx, uapi, errs) + if err != nil { + return nil, err + } + + ret := make([]*UserNoInfo, 0, len(users)) + + for _, u := range users { + pu, err := parseUser(u) + if err != nil { + return nil, clues.Wrap(err, "formatting user data") + } + + puNoInfo := &UserNoInfo{ + PrincipalName: pu.PrincipalName, + ID: pu.ID, + Name: pu.Name, + } + + ret = append(ret, puNoInfo) + } + + return ret, nil +} + // Users returns a list of users in the specified M365 tenant func Users(ctx context.Context, acct account.Account, errs *fault.Bus) ([]*User, error) { uapi, err := makeUserAPI(acct) diff --git a/src/pkg/services/m365/m365_test.go b/src/pkg/services/m365/m365_test.go index 0353d4e36..46028ee3f 100644 --- a/src/pkg/services/m365/m365_test.go +++ b/src/pkg/services/m365/m365_test.go @@ -52,6 +52,30 @@ func (suite *M365IntegrationSuite) TestUsers() { } } +func (suite *M365IntegrationSuite) TestUsersCompat_HasNoInfo() { + ctx, flush := tester.NewContext() + defer flush() + + var ( + t = suite.T() + acct = tester.NewM365Account(suite.T()) + ) + + users, err := m365.UsersCompatNoInfo(ctx, acct) + assert.NoError(t, err, clues.ToCore(err)) + assert.NotEmpty(t, users) + + for _, u := range users { + suite.Run("user_"+u.ID, func() { + t := suite.T() + + assert.NotEmpty(t, u.ID) + assert.NotEmpty(t, u.PrincipalName) + assert.NotEmpty(t, u.Name) + }) + } +} + func (suite *M365IntegrationSuite) TestGetUserInfo() { ctx, flush := tester.NewContext() defer flush()