From b15ce2807be9a6e31486d90979124bdf09a72853 Mon Sep 17 00:00:00 2001 From: Keepers Date: Tue, 3 Jan 2023 12:49:09 -0700 Subject: [PATCH] remove exchangeService for graph.Service (#2002) ## Description the exchangeService struct is a vestigial prop from before we had centralized around the graph.Service, and is no longer needed. These changes will cascade into the migration of graph client usage into an api package for mocking. ## Does this PR need a docs update or release note? - [x] :no_entry: No ## Type of change - [x] :hamster: Trivial/Minor ## Issue(s) * #1967 ## Test Plan - [x] :zap: Unit test - [x] :green_heart: E2E --- .../exchange/exchange_service_test.go | 69 +++++++++++-------- .../connector/exchange/iterators_test.go | 33 ++++----- .../connector/exchange/service_functions.go | 29 +------- 3 files changed, 61 insertions(+), 70 deletions(-) diff --git a/src/internal/connector/exchange/exchange_service_test.go b/src/internal/connector/exchange/exchange_service_test.go index ab5d734ce..353c326dc 100644 --- a/src/internal/connector/exchange/exchange_service_test.go +++ b/src/internal/connector/exchange/exchange_service_test.go @@ -21,7 +21,8 @@ import ( type ExchangeServiceSuite struct { suite.Suite - es *exchangeService + credentials account.M365Config + gs graph.Servicer } func TestExchangeServiceSuite(t *testing.T) { @@ -43,12 +44,15 @@ func (suite *ExchangeServiceSuite) SetupSuite() { a := tester.NewM365Account(t) require.NoError(t, err) + m365, err := a.M365Config() require.NoError(t, err) + service, err := createService(m365) require.NoError(t, err) - suite.es = service + suite.credentials = m365 + suite.gs = service } // TestCreateService verifies that services are created @@ -56,8 +60,8 @@ func (suite *ExchangeServiceSuite) SetupSuite() { // incorrect tenant or password information will NOT generate // an error. func (suite *ExchangeServiceSuite) TestCreateService() { - creds := suite.es.credentials - invalidCredentials := suite.es.credentials + creds := suite.credentials + invalidCredentials := suite.credentials invalidCredentials.AzureClientSecret = "" tests := []struct { @@ -235,7 +239,7 @@ func (suite *ExchangeServiceSuite) TestGraphQueryFunctions() { for _, test := range tests { suite.T().Run(test.name, func(t *testing.T) { - response, err := test.function(ctx, suite.es, userID) + response, err := test.function(ctx, suite.gs, userID) assert.NoError(t, err) assert.NotNil(t, response) }) @@ -259,20 +263,20 @@ func (suite *ExchangeServiceSuite) TestRestoreContact() { folderName = "TestRestoreContact: " + common.FormatSimpleDateTime(now) ) - aFolder, err := CreateContactFolder(ctx, suite.es, userID, folderName) + aFolder, err := CreateContactFolder(ctx, suite.gs, userID, folderName) require.NoError(t, err) folderID := *aFolder.GetId() defer func() { // Remove the folder containing contact prior to exiting test - err = DeleteContactFolder(ctx, suite.es, userID, folderID) + err = DeleteContactFolder(ctx, suite.gs, userID, folderID) assert.NoError(t, err) }() info, err := RestoreExchangeContact(ctx, mockconnector.GetMockContactBytes("Corso TestContact"), - suite.es, + suite.gs, control.Copy, folderID, userID) @@ -292,20 +296,20 @@ func (suite *ExchangeServiceSuite) TestRestoreEvent() { name = "TestRestoreEvent: " + common.FormatSimpleDateTime(time.Now()) ) - calendar, err := CreateCalendar(ctx, suite.es, userID, name) + calendar, err := CreateCalendar(ctx, suite.gs, userID, name) require.NoError(t, err) calendarID := *calendar.GetId() defer func() { // Removes calendar containing events created during the test - err = DeleteCalendar(ctx, suite.es, userID, calendarID) + err = DeleteCalendar(ctx, suite.gs, userID, calendarID) assert.NoError(t, err) }() info, err := RestoreExchangeEvent(ctx, mockconnector.GetMockEventWithAttendeesBytes(name), - suite.es, + suite.gs, control.Copy, calendarID, userID) @@ -315,7 +319,12 @@ func (suite *ExchangeServiceSuite) TestRestoreEvent() { // TestRestoreExchangeObject verifies path.Category usage for restored objects func (suite *ExchangeServiceSuite) TestRestoreExchangeObject() { - service := loadService(suite.T()) + a := tester.NewM365Account(suite.T()) + m365, err := a.M365Config() + require.NoError(suite.T(), err) + + service, err := createService(m365) + require.NoError(suite.T(), err) userID := tester.M365UserID(suite.T()) now := time.Now() @@ -333,7 +342,7 @@ func (suite *ExchangeServiceSuite) TestRestoreExchangeObject() { cleanupFunc: DeleteMailFolder, destination: func(t *testing.T, ctx context.Context) string { folderName := "TestRestoreMailObject: " + common.FormatSimpleDateTime(now) - folder, err := CreateMailFolder(ctx, suite.es, userID, folderName) + folder, err := CreateMailFolder(ctx, suite.gs, userID, folderName) require.NoError(t, err) return *folder.GetId() @@ -346,7 +355,7 @@ func (suite *ExchangeServiceSuite) TestRestoreExchangeObject() { cleanupFunc: DeleteMailFolder, destination: func(t *testing.T, ctx context.Context) string { folderName := "TestRestoreMailwithAttachment: " + common.FormatSimpleDateTime(now) - folder, err := CreateMailFolder(ctx, suite.es, userID, folderName) + folder, err := CreateMailFolder(ctx, suite.gs, userID, folderName) require.NoError(t, err) return *folder.GetId() @@ -359,7 +368,7 @@ func (suite *ExchangeServiceSuite) TestRestoreExchangeObject() { cleanupFunc: DeleteMailFolder, destination: func(t *testing.T, ctx context.Context) string { folderName := "TestRestoreMailwithLargeAttachment: " + common.FormatSimpleDateTime(now) - folder, err := CreateMailFolder(ctx, suite.es, userID, folderName) + folder, err := CreateMailFolder(ctx, suite.gs, userID, folderName) require.NoError(t, err) return *folder.GetId() @@ -372,7 +381,7 @@ func (suite *ExchangeServiceSuite) TestRestoreExchangeObject() { cleanupFunc: DeleteMailFolder, destination: func(t *testing.T, ctx context.Context) string { folderName := "TestRestoreMailwithAttachments: " + common.FormatSimpleDateTime(now) - folder, err := CreateMailFolder(ctx, suite.es, userID, folderName) + folder, err := CreateMailFolder(ctx, suite.gs, userID, folderName) require.NoError(t, err) return *folder.GetId() @@ -385,7 +394,7 @@ func (suite *ExchangeServiceSuite) TestRestoreExchangeObject() { cleanupFunc: DeleteMailFolder, destination: func(t *testing.T, ctx context.Context) string { folderName := "TestRestoreMailwithReferenceAttachment: " + common.FormatSimpleDateTime(now) - folder, err := CreateMailFolder(ctx, suite.es, userID, folderName) + folder, err := CreateMailFolder(ctx, suite.gs, userID, folderName) require.NoError(t, err) return *folder.GetId() @@ -399,7 +408,7 @@ func (suite *ExchangeServiceSuite) TestRestoreExchangeObject() { cleanupFunc: DeleteContactFolder, destination: func(t *testing.T, ctx context.Context) string { folderName := "TestRestoreContactObject: " + common.FormatSimpleDateTime(now) - folder, err := CreateContactFolder(ctx, suite.es, userID, folderName) + folder, err := CreateContactFolder(ctx, suite.gs, userID, folderName) require.NoError(t, err) return *folder.GetId() @@ -412,7 +421,7 @@ func (suite *ExchangeServiceSuite) TestRestoreExchangeObject() { cleanupFunc: DeleteCalendar, destination: func(t *testing.T, ctx context.Context) string { calendarName := "TestRestoreEventObject: " + common.FormatSimpleDateTime(now) - calendar, err := CreateCalendar(ctx, suite.es, userID, calendarName) + calendar, err := CreateCalendar(ctx, suite.gs, userID, calendarName) require.NoError(t, err) return *calendar.GetId() @@ -425,7 +434,7 @@ func (suite *ExchangeServiceSuite) TestRestoreExchangeObject() { cleanupFunc: DeleteCalendar, destination: func(t *testing.T, ctx context.Context) string { calendarName := "TestRestoreEventObject_" + common.FormatSimpleDateTime(now) - calendar, err := CreateCalendar(ctx, suite.es, userID, calendarName) + calendar, err := CreateCalendar(ctx, suite.gs, userID, calendarName) require.NoError(t, err) return *calendar.GetId() @@ -462,9 +471,15 @@ func (suite *ExchangeServiceSuite) TestGetContainerIDFromCache() { ctx, flush := tester.NewContext() defer flush() + a := tester.NewM365Account(suite.T()) + m365, err := a.M365Config() + require.NoError(suite.T(), err) + + connector, err := createService(m365) + require.NoError(suite.T(), err) + var ( user = tester.M365UserID(suite.T()) - connector = loadService(suite.T()) directoryCaches = make(map[path.CategoryType]graph.ContainerResolver) folderName = tester.DefaultTestRestoreDestination().ContainerName tests = []struct { @@ -479,7 +494,7 @@ func (suite *ExchangeServiceSuite) TestGetContainerIDFromCache() { pathFunc1: func(t *testing.T) path.Path { pth, err := path.Builder{}.Append("Griffindor"). Append("Croix").ToDataLayerExchangePathForCategory( - suite.es.credentials.AzureTenantID, + suite.credentials.AzureTenantID, user, path.EmailCategory, false, @@ -491,7 +506,7 @@ func (suite *ExchangeServiceSuite) TestGetContainerIDFromCache() { pathFunc2: func(t *testing.T) path.Path { pth, err := path.Builder{}.Append("Griffindor"). Append("Felicius").ToDataLayerExchangePathForCategory( - suite.es.credentials.AzureTenantID, + suite.credentials.AzureTenantID, user, path.EmailCategory, false, @@ -507,7 +522,7 @@ func (suite *ExchangeServiceSuite) TestGetContainerIDFromCache() { pathFunc1: func(t *testing.T) path.Path { aPath, err := path.Builder{}.Append("HufflePuff"). ToDataLayerExchangePathForCategory( - suite.es.credentials.AzureTenantID, + suite.credentials.AzureTenantID, user, path.ContactsCategory, false, @@ -519,7 +534,7 @@ func (suite *ExchangeServiceSuite) TestGetContainerIDFromCache() { pathFunc2: func(t *testing.T) path.Path { aPath, err := path.Builder{}.Append("Ravenclaw"). ToDataLayerExchangePathForCategory( - suite.es.credentials.AzureTenantID, + suite.credentials.AzureTenantID, user, path.ContactsCategory, false, @@ -535,7 +550,7 @@ func (suite *ExchangeServiceSuite) TestGetContainerIDFromCache() { pathFunc1: func(t *testing.T) path.Path { aPath, err := path.Builder{}.Append("Durmstrang"). ToDataLayerExchangePathForCategory( - suite.es.credentials.AzureTenantID, + suite.credentials.AzureTenantID, user, path.EventsCategory, false, @@ -546,7 +561,7 @@ func (suite *ExchangeServiceSuite) TestGetContainerIDFromCache() { pathFunc2: func(t *testing.T) path.Path { aPath, err := path.Builder{}.Append("Beauxbatons"). ToDataLayerExchangePathForCategory( - suite.es.credentials.AzureTenantID, + suite.credentials.AzureTenantID, user, path.EventsCategory, false, diff --git a/src/internal/connector/exchange/iterators_test.go b/src/internal/connector/exchange/iterators_test.go index 5603e92e8..e1a2cef0c 100644 --- a/src/internal/connector/exchange/iterators_test.go +++ b/src/internal/connector/exchange/iterators_test.go @@ -57,17 +57,6 @@ func (suite *ExchangeIteratorSuite) TestDescendable() { assert.NotNil(t, aDescendable.GetParentFolderId()) } -func loadService(t *testing.T) *exchangeService { - a := tester.NewM365Account(t) - m365, err := a.M365Config() - require.NoError(t, err) - - service, err := createService(m365) - require.NoError(t, err) - - return service -} - // TestCollectionFunctions verifies ability to gather // containers functions are valid for current versioning of msgraph-go-sdk. // Tests for mail have been moved to graph_connector_test.go. @@ -119,24 +108,36 @@ func (suite *ExchangeIteratorSuite) TestCollectionFunctions() { } for _, test := range tests { suite.T().Run(test.name, func(t *testing.T) { - service := loadService(t) + a := tester.NewM365Account(t) + m365, err := a.M365Config() + require.NoError(t, err) + + service, err := createService(m365) + require.NoError(t, err) + response, err := test.queryFunc(ctx, service, userID) require.NoError(t, err) + // Iterator Creation - pageIterator, err := msgraphgocore.NewPageIterator(response, - &service.adapter, + pageIterator, err := msgraphgocore.NewPageIterator( + response, + service.Adapter(), test.transformer) require.NoError(t, err) + // Create collection for iterate test collections := make(map[string]graph.Container) + var errs error + errUpdater := func(id string, err error) { errs = support.WrapAndAppend(id, err, errs) } + // callbackFunc iterates through all models.Messageable and fills exchange.Collection.added[] // with corresponding item IDs. New collections are created for each directory - callbackFunc := test.iterativeFunction( - collections, "", errUpdater) + callbackFunc := test.iterativeFunction(collections, "", errUpdater) + iterateError := pageIterator.Iterate(ctx, callbackFunc) assert.NoError(t, iterateError) assert.NoError(t, errs) diff --git a/src/internal/connector/exchange/service_functions.go b/src/internal/connector/exchange/service_functions.go index c92be996d..3b1747689 100644 --- a/src/internal/connector/exchange/service_functions.go +++ b/src/internal/connector/exchange/service_functions.go @@ -4,7 +4,6 @@ import ( "context" "fmt" - msgraphsdk "github.com/microsoftgraph/msgraph-sdk-go" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/pkg/errors" @@ -16,28 +15,10 @@ import ( var ErrFolderNotFound = errors.New("folder not found") -type exchangeService struct { - client msgraphsdk.GraphServiceClient - adapter msgraphsdk.GraphRequestAdapter - credentials account.M365Config -} - -// ------------------------------------------------------------ -// Functions to comply with graph.Servicer Interface -// ------------------------------------------------------------ - -func (es *exchangeService) Client() *msgraphsdk.GraphServiceClient { - return &es.client -} - -func (es *exchangeService) Adapter() *msgraphsdk.GraphRequestAdapter { - return &es.adapter -} - // createService internal constructor for exchangeService struct returns an error // iff the params for the entry are incorrect (e.g. len(TenantID) == 0, etc.) // NOTE: Incorrect account information will result in errors on subsequent queries. -func createService(credentials account.M365Config) (*exchangeService, error) { +func createService(credentials account.M365Config) (*graph.Service, error) { adapter, err := graph.CreateAdapter( credentials.AzureTenantID, credentials.AzureClientID, @@ -47,13 +28,7 @@ func createService(credentials account.M365Config) (*exchangeService, error) { return nil, errors.Wrap(err, "creating microsoft graph service for exchange") } - service := exchangeService{ - adapter: *adapter, - client: *msgraphsdk.NewGraphServiceClient(adapter), - credentials: credentials, - } - - return &service, nil + return graph.NewService(adapter), nil } // CreateMailFolder makes a mail folder iff a folder of the same name does not exist