larryh/corso
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Wire service isolation changes into SDK code (#4186)

Browse Source 
<!-- PR description-->

Wires `IsServiceEnabled` code into `UserHasMailbox`, `UserHasDrive`, and `UserGetMailboxInfo` SDK APIs.

---

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

- [ ] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x] ⛔ No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
*  https://github.com/alcionai/corso/issues/3844

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] 💪 Manual
- [x] ⚡ Unit test
- [x] 💚 E2E
This commit is contained in:
Abhishek Pandey 2023-09-08 13:28:38 +05:30 committed by GitHub
parent 2252625c14
commit 9bedee9c72
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 168 additions and 195 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/internal/m365/service/exchange/enabled.go
View File

@ -64,7 +64,7 @@ func GetMailboxInfo(
mi.ErrGetMailBoxSetting = append(
mi.ErrGetMailBoxSetting,
api.ErrMailBoxSettingsNotFound)
api.ErrMailBoxNotFound)
return mi, nil
}

2
src/internal/m365/service/exchange/enabled_test.go
View File

@ -189,7 +189,7 @@ func (suite *EnabledUnitSuite) TestGetMailboxInfo() {
mi := api.MailboxInfo{}
mi.ErrGetMailBoxSetting = append(
mi.ErrGetMailBoxSetting,
api.ErrMailBoxSettingsNotFound)
api.ErrMailBoxNotFound)
return mi
},

17
src/pkg/services/m365/api/users.go
View File

@ -9,6 +9,7 @@ import (
msgraphgocore "github.com/microsoftgraph/msgraph-sdk-go-core"
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/microsoftgraph/msgraph-sdk-go/users"
"github.com/pkg/errors"
"github.com/alcionai/corso/src/internal/common/idname"
"github.com/alcionai/corso/src/internal/common/ptr"
@ -20,7 +21,7 @@ import (
// Variables
var (
ErrMailBoxSettingsNotFound = clues.New("mailbox settings not found")
ErrMailBoxNotFound = clues.New("mailbox not found")
ErrMailBoxSettingsAccessDenied = clues.New("mailbox settings access denied")
)
@ -215,7 +216,7 @@ func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) {
}
if !mailFolderFound {
mi.ErrGetMailBoxSetting = append(mi.ErrGetMailBoxSetting, ErrMailBoxSettingsNotFound)
mi.ErrGetMailBoxSetting = append(mi.ErrGetMailBoxSetting, ErrMailBoxNotFound)
userInfo.Mailbox = mi
return userInfo, nil
@ -268,6 +269,18 @@ func EvaluateMailboxError(err error) error {
return err
}
// IsAnyErrMailboxNotFound inspects the secondary errors inside MailboxInfo and
// determines whether the resource has a mailbox.
func IsAnyErrMailboxNotFound(errs []error) bool {
for _, err := range errs {
if errors.Is(err, ErrMailBoxNotFound) {
return true
}
}
return false
}
func (c Users) GetMailboxSettings(
ctx context.Context,
userID string,

37
src/pkg/services/m365/api/users_test.go
View File

@ -117,6 +117,43 @@ func (suite *UsersUnitSuite) TestEvaluateMailboxError() {
}
}
func (suite *UsersUnitSuite) TestIsAnyErrMailboxNotFound() {
table := []struct {
name string
errs []error
expect bool
}{
{
name: "no errors",
errs: nil,
expect: false,
},
{
name: "mailbox not found error",
errs: []error{
clues.New("an error"),
api.ErrMailBoxNotFound,
clues.New("an error"),
},
expect: true,
},
{
name: "other errors",
errs: []error{
clues.New("an error"),
api.ErrMailBoxSettingsAccessDenied,
clues.New("an error"),
},
expect: false,
},
}
for _, test := range table {
suite.Run(test.name, func() {
assert.Equal(suite.T(), test.expect, api.IsAnyErrMailboxNotFound(test.errs))
})
}
}
type UsersIntgSuite struct {
tester.Suite
its intgTesterSetup

5
src/pkg/services/m365/m365.go
View File

@ -4,7 +4,6 @@ import (
"context"
"github.com/alcionai/clues"
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/alcionai/corso/src/pkg/account"
"github.com/alcionai/corso/src/pkg/control"
@ -17,10 +16,6 @@ import (
// interfaces & structs
// ---------------------------------------------------------------------------
type getDefaultDriver interface {
GetDefaultDrive(ctx context.Context, userID string) (models.Driveable, error)
}
type getAller[T any] interface {
GetAll(ctx context.Context, errs *fault.Bus) ([]T, error)
}

42
src/pkg/services/m365/users.go
View File

@ -7,7 +7,8 @@ import (
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/alcionai/corso/src/internal/common/ptr"
"github.com/alcionai/corso/src/internal/m365/graph"
"github.com/alcionai/corso/src/internal/m365/service/exchange"
"github.com/alcionai/corso/src/internal/m365/service/onedrive"
"github.com/alcionai/corso/src/pkg/account"
"github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/path"
@ -66,16 +67,20 @@ func UserHasMailbox(ctx context.Context, acct account.Account, userID string) (b
return false, clues.Stack(err).WithClues(ctx)
}
_, err = ac.Users().GetMailInbox(ctx, userID)
if err != nil {
if err := api.EvaluateMailboxError(err); err != nil {
return false, clues.Stack(err)
}
return exchange.IsServiceEnabled(ctx, ac.Users(), userID)
}
return false, nil
func UserGetMailboxInfo(
ctx context.Context,
acct account.Account,
userID string,
) (api.MailboxInfo, error) {
ac, err := makeAC(ctx, acct, path.ExchangeService)
if err != nil {
return api.MailboxInfo{}, clues.Stack(err).WithClues(ctx)
}
return true, nil
return exchange.GetMailboxInfo(ctx, ac.Users(), userID)
}
// UserHasDrives returns true if the user has any drives
@ -86,26 +91,7 @@ func UserHasDrives(ctx context.Context, acct account.Account, userID string) (bo
return false, clues.Stack(err).WithClues(ctx)
}
return checkUserHasDrives(ctx, ac.Users(), userID)
}
func checkUserHasDrives(ctx context.Context, dgdd getDefaultDriver, userID string) (bool, error) {
_, err := dgdd.GetDefaultDrive(ctx, userID)
if err != nil {
// we consider this a non-error case, since it
// answers the question the caller is asking.
if clues.HasLabel(err, graph.LabelsMysiteNotFound) || clues.HasLabel(err, graph.LabelsNoSharePointLicense) {
return false, nil
}
if graph.IsErrUserNotFound(err) {
return false, clues.Stack(graph.ErrResourceOwnerNotFound, err)
}
return false, clues.Stack(err)
}
return true, nil
return onedrive.IsServiceEnabled(ctx, ac.Users(), userID)
}
// usersNoInfo returns a list of users in the specified M365 tenant - with no info

258
src/pkg/services/m365/users_test.go
View File

@ -1,18 +1,14 @@
package m365
import (
"context"
"testing"
"github.com/alcionai/clues"
"github.com/google/uuid"
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/microsoftgraph/msgraph-sdk-go/models/odataerrors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/alcionai/corso/src/internal/common/ptr"
"github.com/alcionai/corso/src/internal/m365/graph"
"github.com/alcionai/corso/src/internal/tester"
"github.com/alcionai/corso/src/internal/tester/tconfig"
@ -94,34 +90,115 @@ func (suite *userIntegrationSuite) TestUsersCompat_HasNoInfo() {
func (suite *userIntegrationSuite) TestUserHasMailbox() {
t := suite.T()
acct := tconfig.NewM365Account(t)
userID := tconfig.M365UserID(t)
ctx, flush := tester.NewContext(t)
defer flush()
table := []struct {
name string
user string
expect bool
}{
{
name: "user with no mailbox",
user: "a53c26f7-5100-4acb-a910-4d20960b2c19", // User: testevents@10rqc2.onmicrosoft.com
expect: false,
},
{
name: "user with mailbox",
user: userID,
expect: true,
},
}
for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()
var (
acct = tconfig.NewM365Account(t)
uid = tconfig.M365UserID(t)
)
ctx, flush := tester.NewContext(t)
defer flush()
enabled, err := UserHasMailbox(ctx, acct, uid)
require.NoError(t, err, clues.ToCore(err))
assert.True(t, enabled)
enabled, err := UserHasMailbox(ctx, acct, test.user)
require.NoError(t, err, clues.ToCore(err))
assert.Equal(t, test.expect, enabled)
})
}
}
func (suite *userIntegrationSuite) TestUserHasDrive() {
t := suite.T()
acct := tconfig.NewM365Account(t)
userID := tconfig.M365UserID(t)
ctx, flush := tester.NewContext(t)
defer flush()
table := []struct {
name string
user string
expect bool
}{
{
name: "user without drive",
user: "a53c26f7-5100-4acb-a910-4d20960b2c19", // User: testevents@10rqc2.onmicrosoft.com
expect: false,
},
{
name: "user with drive",
user: userID,
expect: true,
},
}
for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()
var (
acct = tconfig.NewM365Account(t)
uid = tconfig.M365UserID(t)
)
ctx, flush := tester.NewContext(t)
defer flush()
enabled, err := UserHasDrives(ctx, acct, uid)
require.NoError(t, err, clues.ToCore(err))
assert.True(t, enabled)
enabled, err := UserHasDrives(ctx, acct, test.user)
require.NoError(t, err, clues.ToCore(err))
assert.Equal(t, test.expect, enabled)
})
}
}
func (suite *userIntegrationSuite) TestUserGetMailboxInfo() {
t := suite.T()
acct := tconfig.NewM365Account(t)
table := []struct {
name string
user string
expect func(t *testing.T, info api.MailboxInfo)
expectErr require.ErrorAssertionFunc
}{
{
name: "shared mailbox",
user: "bb1a2049-3fc1-4fdc-93b8-7a14f63dd0db", // User: neha-test-shared-mailbox@10rqc2.onmicrosoft.com
expect: func(t *testing.T, info api.MailboxInfo) {
require.NotNil(t, info)
assert.Equal(t, "shared", info.Purpose)
},
expectErr: require.NoError,
},
{
name: "user with no mailbox",
user: "a53c26f7-5100-4acb-a910-4d20960b2c19", // User: testevents@10rqc2.onmicrosoft.com
expect: func(t *testing.T, info api.MailboxInfo) {
require.NotNil(t, info)
assert.Contains(t, info.ErrGetMailBoxSetting, api.ErrMailBoxNotFound)
},
expectErr: require.NoError,
},
}
for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()
ctx, flush := tester.NewContext(t)
defer flush()
info, err := UserGetMailboxInfo(ctx, acct, test.user)
test.expectErr(t, err, clues.ToCore(err))
test.expect(t, info)
})
}
}
func (suite *userIntegrationSuite) TestUsers_InvalidCredentials() {
@ -228,7 +305,7 @@ func (suite *userIntegrationSuite) TestGetUserInfo_userWithoutDrive() {
expect: &api.UserInfo{
ServicesEnabled: map[path.ServiceType]struct{}{},
Mailbox: api.MailboxInfo{
ErrGetMailBoxSetting: []error{api.ErrMailBoxSettingsNotFound},
ErrGetMailBoxSetting: []error{api.ErrMailBoxNotFound},
},
},
},
@ -262,138 +339,3 @@ func (suite *userIntegrationSuite) TestGetUserInfo_userWithoutDrive() {
})
}
}
// ---------------------------------------------------------------------------
// Unit
// ---------------------------------------------------------------------------
type userUnitSuite struct {
tester.Suite
}
func TestUserUnitSuite(t *testing.T) {
suite.Run(t, &userUnitSuite{Suite: tester.NewUnitSuite(t)})
}
type mockDGDD struct {
response models.Driveable
err error
}
func (m mockDGDD) GetDefaultDrive(context.Context, string) (models.Driveable, error) {
return m.response, m.err
}
func (suite *userUnitSuite) TestCheckUserHasDrives() {
table := []struct {
name string
mock func(context.Context) getDefaultDriver
expect assert.BoolAssertionFunc
expectErr func(*testing.T, error)
}{
{
name: "ok",
mock: func(ctx context.Context) getDefaultDriver {
return mockDGDD{models.NewDrive(), nil}
},
expect: assert.True,
expectErr: func(t *testing.T, err error) {
assert.NoError(t, err, clues.ToCore(err))
},
},
{
name: "mysite not found",
mock: func(ctx context.Context) getDefaultDriver {
odErr := odataerrors.NewODataError()
merr := odataerrors.NewMainError()
merr.SetCode(ptr.To("code"))
merr.SetMessage(ptr.To(string(graph.MysiteNotFound)))
odErr.SetErrorEscaped(merr)
return mockDGDD{nil, graph.Stack(ctx, odErr)}
},
expect: assert.False,
expectErr: func(t *testing.T, err error) {
assert.NoError(t, err, clues.ToCore(err))
},
},
{
name: "mysite URL not found",
mock: func(ctx context.Context) getDefaultDriver {
odErr := odataerrors.NewODataError()
merr := odataerrors.NewMainError()
merr.SetCode(ptr.To("code"))
merr.SetMessage(ptr.To(string(graph.MysiteURLNotFound)))
odErr.SetErrorEscaped(merr)
return mockDGDD{nil, graph.Stack(ctx, odErr)}
},
expect: assert.False,
expectErr: func(t *testing.T, err error) {
assert.NoError(t, err, clues.ToCore(err))
},
},
{
name: "no sharepoint license",
mock: func(ctx context.Context) getDefaultDriver {
odErr := odataerrors.NewODataError()
merr := odataerrors.NewMainError()
merr.SetCode(ptr.To("code"))
merr.SetMessage(ptr.To(string(graph.NoSPLicense)))
odErr.SetErrorEscaped(merr)
return mockDGDD{nil, graph.Stack(ctx, odErr)}
},
expect: assert.False,
expectErr: func(t *testing.T, err error) {
assert.NoError(t, err, clues.ToCore(err))
},
},
{
name: "user not found",
mock: func(ctx context.Context) getDefaultDriver {
odErr := odataerrors.NewODataError()
merr := odataerrors.NewMainError()
merr.SetCode(ptr.To(string(graph.RequestResourceNotFound)))
merr.SetMessage(ptr.To("message"))
odErr.SetErrorEscaped(merr)
return mockDGDD{nil, graph.Stack(ctx, odErr)}
},
expect: assert.False,
expectErr: func(t *testing.T, err error) {
assert.Error(t, err, clues.ToCore(err))
},
},
{
name: "arbitrary error",
mock: func(ctx context.Context) getDefaultDriver {
odErr := odataerrors.NewODataError()
merr := odataerrors.NewMainError()
merr.SetCode(ptr.To("code"))
merr.SetMessage(ptr.To("message"))
odErr.SetErrorEscaped(merr)
return mockDGDD{nil, graph.Stack(ctx, odErr)}
},
expect: assert.False,
expectErr: func(t *testing.T, err error) {
assert.Error(t, err, clues.ToCore(err))
},
},
}
for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()
ctx, flush := tester.NewContext(t)
defer flush()
dgdd := test.mock(ctx)
ok, err := checkUserHasDrives(ctx, dgdd, "foo")
test.expect(t, ok, "has drives flag")
test.expectErr(t, err)
})
}
}