From c74585eafcc9e730e3c322b00814ffee1ed4978c Mon Sep 17 00:00:00 2001 From: Keepers Date: Thu, 7 Sep 2023 12:30:46 -0600 Subject: [PATCH] remove redundant type check (#4199) info.itemType already looks through each service entry to find the item type. A service nil check is redundant. Adds a qol func for asking if an itemInfo is a driveish item. --- #### Does this PR need a docs update or release note? - [x] :no_entry: No #### Type of change - [x] :bug: Bugfix #### Issue(s) * #3990 #### Test Plan - [x] :muscle: Manual - [x] :zap: Unit test - [x] :green_heart: E2E --- src/cmd/factory/impl/common.go | 5 +- src/cmd/factory/impl/exchange.go | 7 +- src/cmd/factory/impl/onedrive.go | 3 +- src/cmd/factory/impl/sharepoint.go | 3 +- src/internal/m365/backup_test.go | 17 ++-- src/internal/m365/controller.go | 12 ++- src/internal/m365/controller_test.go | 45 ++++------ src/internal/m365/helper_test.go | 6 +- src/internal/m365/onedrive_test.go | 55 ++++-------- src/internal/m365/stub/stub.go | 2 - src/internal/operations/help_test.go | 4 +- src/internal/operations/restore_test.go | 2 - src/internal/operations/test/exchange_test.go | 3 +- src/internal/operations/test/helper_test.go | 18 +--- src/internal/operations/test/onedrive_test.go | 8 +- .../operations/test/restore_helper_test.go | 11 --- .../operations/test/sharepoint_test.go | 2 - src/pkg/backup/details/details.go | 6 +- src/pkg/backup/details/iteminfo.go | 11 +++ src/pkg/backup/details/iteminfo_test.go | 86 +++++++++++++++++++ src/pkg/repository/repository.go | 9 +- src/pkg/repository/repository_test.go | 10 ++- 22 files changed, 175 insertions(+), 150 deletions(-) create mode 100644 src/pkg/backup/details/iteminfo_test.go diff --git a/src/cmd/factory/impl/common.go b/src/cmd/factory/impl/common.go index bec56762f..9f9eee19d 100644 --- a/src/cmd/factory/impl/common.go +++ b/src/cmd/factory/impl/common.go @@ -17,7 +17,6 @@ import ( "github.com/alcionai/corso/src/internal/common/str" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/m365" - "github.com/alcionai/corso/src/internal/m365/resource" exchMock "github.com/alcionai/corso/src/internal/m365/service/exchange/mock" odStub "github.com/alcionai/corso/src/internal/m365/service/onedrive/stub" m365Stub "github.com/alcionai/corso/src/internal/m365/stub" @@ -121,7 +120,6 @@ func generateAndRestoreItems( func getControllerAndVerifyResourceOwner( ctx context.Context, - resourceCat resource.Category, resourceOwner string, pst path.ServiceType, ) ( @@ -147,7 +145,7 @@ func getControllerAndVerifyResourceOwner( return nil, account.Account{}, nil, clues.Wrap(err, "finding m365 account details") } - ctrl, err := m365.NewController(ctx, acct, resourceCat, pst, control.Options{}) + ctrl, err := m365.NewController(ctx, acct, pst, control.Options{}) if err != nil { return nil, account.Account{}, nil, clues.Wrap(err, "connecting to graph api") } @@ -430,7 +428,6 @@ func generateAndRestoreDriveItems( config := m365Stub.ConfigInfo{ Opts: opts, - Resource: resource.Users, Service: service, Tenant: tenantID, ResourceOwners: []string{protectedResource.ID()}, diff --git a/src/cmd/factory/impl/exchange.go b/src/cmd/factory/impl/exchange.go index d4513fe4a..a8ee887eb 100644 --- a/src/cmd/factory/impl/exchange.go +++ b/src/cmd/factory/impl/exchange.go @@ -5,7 +5,6 @@ import ( . "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/cli/utils" - "github.com/alcionai/corso/src/internal/m365/resource" exchMock "github.com/alcionai/corso/src/internal/m365/service/exchange/mock" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" @@ -53,7 +52,7 @@ func handleExchangeEmailFactory(cmd *cobra.Command, args []string) error { return nil } - ctrl, _, _, err := getControllerAndVerifyResourceOwner(ctx, resource.Users, User, path.ExchangeService) + ctrl, _, _, err := getControllerAndVerifyResourceOwner(ctx, User, path.ExchangeService) if err != nil { return Only(ctx, err) } @@ -100,7 +99,7 @@ func handleExchangeCalendarEventFactory(cmd *cobra.Command, args []string) error return nil } - ctrl, _, _, err := getControllerAndVerifyResourceOwner(ctx, resource.Users, User, path.ExchangeService) + ctrl, _, _, err := getControllerAndVerifyResourceOwner(ctx, User, path.ExchangeService) if err != nil { return Only(ctx, err) } @@ -149,7 +148,7 @@ func handleExchangeContactFactory(cmd *cobra.Command, args []string) error { return nil } - ctrl, _, _, err := getControllerAndVerifyResourceOwner(ctx, resource.Users, User, path.ExchangeService) + ctrl, _, _, err := getControllerAndVerifyResourceOwner(ctx, User, path.ExchangeService) if err != nil { return Only(ctx, err) } diff --git a/src/cmd/factory/impl/onedrive.go b/src/cmd/factory/impl/onedrive.go index 6ccc98977..1cc771af9 100644 --- a/src/cmd/factory/impl/onedrive.go +++ b/src/cmd/factory/impl/onedrive.go @@ -7,7 +7,6 @@ import ( . "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/cli/utils" - "github.com/alcionai/corso/src/internal/m365/resource" "github.com/alcionai/corso/src/pkg/count" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" @@ -37,7 +36,7 @@ func handleOneDriveFileFactory(cmd *cobra.Command, args []string) error { return nil } - ctrl, acct, inp, err := getControllerAndVerifyResourceOwner(ctx, resource.Users, User, path.OneDriveService) + ctrl, acct, inp, err := getControllerAndVerifyResourceOwner(ctx, User, path.OneDriveService) if err != nil { return Only(ctx, err) } diff --git a/src/cmd/factory/impl/sharepoint.go b/src/cmd/factory/impl/sharepoint.go index ab9d3fc92..7f4313538 100644 --- a/src/cmd/factory/impl/sharepoint.go +++ b/src/cmd/factory/impl/sharepoint.go @@ -7,7 +7,6 @@ import ( . "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/cli/utils" - "github.com/alcionai/corso/src/internal/m365/resource" "github.com/alcionai/corso/src/pkg/count" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" @@ -37,7 +36,7 @@ func handleSharePointLibraryFileFactory(cmd *cobra.Command, args []string) error return nil } - ctrl, acct, inp, err := getControllerAndVerifyResourceOwner(ctx, resource.Sites, Site, path.SharePointService) + ctrl, acct, inp, err := getControllerAndVerifyResourceOwner(ctx, Site, path.SharePointService) if err != nil { return Only(ctx, err) } diff --git a/src/internal/m365/backup_test.go b/src/internal/m365/backup_test.go index 068d05891..75efc4d1d 100644 --- a/src/internal/m365/backup_test.go +++ b/src/internal/m365/backup_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/suite" inMock "github.com/alcionai/corso/src/internal/common/idname/mock" - "github.com/alcionai/corso/src/internal/m365/resource" "github.com/alcionai/corso/src/internal/m365/service/exchange" odConsts "github.com/alcionai/corso/src/internal/m365/service/onedrive/consts" "github.com/alcionai/corso/src/internal/m365/service/sharepoint" @@ -69,7 +68,7 @@ func (suite *DataCollectionIntgSuite) TestExchangeDataCollection() { selUsers := []string{suite.user} - ctrl := newController(ctx, suite.T(), resource.Users, path.ExchangeService) + ctrl := newController(ctx, suite.T(), path.ExchangeService) tests := []struct { name string getSelector func(t *testing.T) selectors.Selector @@ -175,7 +174,7 @@ func (suite *DataCollectionIntgSuite) TestDataCollections_invalidResourceOwner() defer flush() owners := []string{"snuffleupagus"} - ctrl := newController(ctx, suite.T(), resource.Users, path.ExchangeService) + ctrl := newController(ctx, suite.T(), path.ExchangeService) tests := []struct { name string getSelector func(t *testing.T) selectors.Selector @@ -263,7 +262,7 @@ func (suite *DataCollectionIntgSuite) TestSharePointDataCollection() { defer flush() selSites := []string{suite.site} - ctrl := newController(ctx, suite.T(), resource.Sites, path.SharePointService) + ctrl := newController(ctx, suite.T(), path.SharePointService) tests := []struct { name string expected int @@ -361,7 +360,7 @@ func (suite *SPCollectionIntgSuite) SetupSuite() { ctx, flush := tester.NewContext(suite.T()) defer flush() - suite.connector = newController(ctx, suite.T(), resource.Sites, path.SharePointService) + suite.connector = newController(ctx, suite.T(), path.SharePointService) suite.user = tconfig.M365UserID(suite.T()) tester.LogTimeOfTest(suite.T()) @@ -375,7 +374,7 @@ func (suite *SPCollectionIntgSuite) TestCreateSharePointCollection_Libraries() { var ( siteID = tconfig.M365SiteID(t) - ctrl = newController(ctx, t, resource.Sites, path.SharePointService) + ctrl = newController(ctx, t, path.SharePointService) siteIDs = []string{siteID} ) @@ -425,7 +424,7 @@ func (suite *SPCollectionIntgSuite) TestCreateSharePointCollection_Lists() { var ( siteID = tconfig.M365SiteID(t) - ctrl = newController(ctx, t, resource.Sites, path.SharePointService) + ctrl = newController(ctx, t, path.SharePointService) siteIDs = []string{siteID} ) @@ -492,7 +491,7 @@ func (suite *GroupsCollectionIntgSuite) SetupSuite() { ctx, flush := tester.NewContext(t) defer flush() - suite.connector = newController(ctx, t, resource.Sites, path.GroupsService) + suite.connector = newController(ctx, t, path.GroupsService) suite.user = tconfig.M365UserID(t) acct := tconfig.NewM365Account(t) @@ -512,7 +511,7 @@ func (suite *GroupsCollectionIntgSuite) TestCreateGroupsCollection_SharePoint() var ( groupID = tconfig.M365GroupID(t) - ctrl = newController(ctx, t, resource.Groups, path.GroupsService) + ctrl = newController(ctx, t, path.GroupsService) groupIDs = []string{groupID} ) diff --git a/src/internal/m365/controller.go b/src/internal/m365/controller.go index 0b8854be2..3a4c8be19 100644 --- a/src/internal/m365/controller.go +++ b/src/internal/m365/controller.go @@ -59,7 +59,6 @@ type Controller struct { func NewController( ctx context.Context, acct account.Account, - rc resource.Category, pst path.ServiceType, co control.Options, ) (*Controller, error) { @@ -75,6 +74,17 @@ func NewController( return nil, clues.Wrap(err, "creating api client").WithClues(ctx) } + rc := resource.UnknownResource + + switch pst { + case path.ExchangeService, path.OneDriveService: + rc = resource.Users + case path.GroupsService: + rc = resource.Groups + case path.SharePointService: + rc = resource.Sites + } + rCli, err := getResourceClient(rc, ac) if err != nil { return nil, clues.Wrap(err, "creating resource client").WithClues(ctx) diff --git a/src/internal/m365/controller_test.go b/src/internal/m365/controller_test.go index cf290e320..4aeefb6a9 100644 --- a/src/internal/m365/controller_test.go +++ b/src/internal/m365/controller_test.go @@ -366,7 +366,7 @@ func (suite *ControllerIntegrationSuite) SetupSuite() { ctx, flush := tester.NewContext(t) defer flush() - suite.ctrl = newController(ctx, t, resource.Users, path.ExchangeService) + suite.ctrl = newController(ctx, t, path.ExchangeService) suite.user = tconfig.M365UserID(t) suite.secondaryUser = tconfig.SecondaryM365UserID(t) @@ -472,7 +472,7 @@ func runRestore( start := time.Now() - restoreCtrl := newController(ctx, t, sci.Resource, path.ExchangeService) + restoreCtrl := newController(ctx, t, path.ExchangeService) restoreSel := getSelectorWith(t, sci.Service, sci.ResourceOwners, true) rcc := inject.RestoreConsumerConfig{ @@ -541,7 +541,7 @@ func runBackupAndCompare( nameToID[ro] = ro } - backupCtrl := newController(ctx, t, sci.Resource, path.ExchangeService) + backupCtrl := newController(ctx, t, path.ExchangeService) backupCtrl.IDNameLookup = inMock.NewCache(idToName, nameToID) backupSel := backupSelectorForExpected(t, sci.Service, expectedDests) @@ -597,7 +597,6 @@ func runRestoreBackupTest( cfg := stub.ConfigInfo{ Opts: opts, - Resource: test.resourceCat, Service: test.service, Tenant: tenant, ResourceOwners: resourceOwners, @@ -643,7 +642,6 @@ func runRestoreTestWithVersion( cfg := stub.ConfigInfo{ Opts: opts, - Resource: test.resourceCat, Service: test.service, Tenant: tenant, ResourceOwners: resourceOwners, @@ -681,7 +679,6 @@ func runRestoreBackupTestVersions( cfg := stub.ConfigInfo{ Opts: opts, - Resource: test.resourceCat, Service: test.service, Tenant: tenant, ResourceOwners: resourceOwners, @@ -725,9 +722,8 @@ func (suite *ControllerIntegrationSuite) TestRestoreAndBackup_core() { table := []restoreBackupInfo{ { - name: "EmailsWithAttachments", - service: path.ExchangeService, - resourceCat: resource.Users, + name: "EmailsWithAttachments", + service: path.ExchangeService, collections: []stub.ColInfo{ { PathElements: []string{api.MailInbox}, @@ -752,9 +748,8 @@ func (suite *ControllerIntegrationSuite) TestRestoreAndBackup_core() { }, }, { - name: "MultipleEmailsMultipleFolders", - service: path.ExchangeService, - resourceCat: resource.Users, + name: "MultipleEmailsMultipleFolders", + service: path.ExchangeService, collections: []stub.ColInfo{ { PathElements: []string{api.MailInbox}, @@ -828,9 +823,8 @@ func (suite *ControllerIntegrationSuite) TestRestoreAndBackup_core() { }, }, { - name: "MultipleContactsSingleFolder", - service: path.ExchangeService, - resourceCat: resource.Users, + name: "MultipleContactsSingleFolder", + service: path.ExchangeService, collections: []stub.ColInfo{ { PathElements: []string{"Contacts"}, @@ -856,9 +850,8 @@ func (suite *ControllerIntegrationSuite) TestRestoreAndBackup_core() { }, }, { - name: "MultipleContactsMultipleFolders", - service: path.ExchangeService, - resourceCat: resource.Users, + name: "MultipleContactsMultipleFolders", + service: path.ExchangeService, collections: []stub.ColInfo{ { PathElements: []string{"Work"}, @@ -987,9 +980,8 @@ func (suite *ControllerIntegrationSuite) TestRestoreAndBackup_core() { func (suite *ControllerIntegrationSuite) TestMultiFolderBackupDifferentNames() { table := []restoreBackupInfo{ { - name: "Contacts", - service: path.ExchangeService, - resourceCat: resource.Users, + name: "Contacts", + service: path.ExchangeService, collections: []stub.ColInfo{ { PathElements: []string{"Work"}, @@ -1093,7 +1085,7 @@ func (suite *ControllerIntegrationSuite) TestMultiFolderBackupDifferentNames() { restoreCfg.Location, ) - restoreCtrl := newController(ctx, t, test.resourceCat, path.ExchangeService) + restoreCtrl := newController(ctx, t, path.ExchangeService) rcc := inject.RestoreConsumerConfig{ BackupVersion: version.Backup, @@ -1127,7 +1119,7 @@ func (suite *ControllerIntegrationSuite) TestMultiFolderBackupDifferentNames() { // Run a backup and compare its output with what we put in. - backupCtrl := newController(ctx, t, test.resourceCat, path.ExchangeService) + backupCtrl := newController(ctx, t, path.ExchangeService) backupSel := backupSelectorForExpected(t, test.service, expectedDests) t.Log("Selective backup of", backupSel) @@ -1175,9 +1167,8 @@ func (suite *ControllerIntegrationSuite) TestRestoreAndBackup_largeMailAttachmen subjectText := "Test message for restore with large attachment" test := restoreBackupInfo{ - name: "EmailsWithLargeAttachments", - service: path.ExchangeService, - resourceCat: resource.Users, + name: "EmailsWithLargeAttachments", + service: path.ExchangeService, collections: []stub.ColInfo{ { PathElements: []string{api.MailInbox}, @@ -1278,7 +1269,7 @@ func (suite *ControllerIntegrationSuite) TestBackup_CreatesPrefixCollections() { defer flush() var ( - backupCtrl = newController(ctx, t, test.resourceCat, path.ExchangeService) + backupCtrl = newController(ctx, t, test.service) backupSel = test.selectorFunc(t) errs = fault.New(true) start = time.Now() diff --git a/src/internal/m365/helper_test.go b/src/internal/m365/helper_test.go index 7f93c51e0..4d8f8ad79 100644 --- a/src/internal/m365/helper_test.go +++ b/src/internal/m365/helper_test.go @@ -19,7 +19,6 @@ import ( "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/m365/collection/drive" "github.com/alcionai/corso/src/internal/m365/collection/drive/metadata" - "github.com/alcionai/corso/src/internal/m365/resource" odStub "github.com/alcionai/corso/src/internal/m365/service/onedrive/stub" m365Stub "github.com/alcionai/corso/src/internal/m365/stub" "github.com/alcionai/corso/src/internal/tester/tconfig" @@ -106,14 +105,12 @@ type restoreBackupInfo struct { name string service path.ServiceType collections []m365Stub.ColInfo - resourceCat resource.Category } type restoreBackupInfoMultiVersion struct { service path.ServiceType collectionsLatest []m365Stub.ColInfo collectionsPrevious []m365Stub.ColInfo - resourceCat resource.Category backupVersion int } @@ -1197,12 +1194,11 @@ func getSelectorWith( func newController( ctx context.Context, t *testing.T, - r resource.Category, pst path.ServiceType, ) *Controller { a := tconfig.NewM365Account(t) - controller, err := NewController(ctx, a, r, pst, control.Options{}) + controller, err := NewController(ctx, a, pst, control.Options{}) require.NoError(t, err, clues.ToCore(err)) return controller diff --git a/src/internal/m365/onedrive_test.go b/src/internal/m365/onedrive_test.go index d53bfb2e8..6617f1bfa 100644 --- a/src/internal/m365/onedrive_test.go +++ b/src/internal/m365/onedrive_test.go @@ -16,7 +16,6 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/m365/collection/drive/metadata" "github.com/alcionai/corso/src/internal/m365/graph" - "github.com/alcionai/corso/src/internal/m365/resource" odConsts "github.com/alcionai/corso/src/internal/m365/service/onedrive/consts" "github.com/alcionai/corso/src/internal/m365/service/onedrive/stub" "github.com/alcionai/corso/src/internal/tester" @@ -91,7 +90,6 @@ type suiteInfo interface { // also be a site. ResourceOwner() string Service() path.ServiceType - Resource() resource.Category } type oneDriveSuite interface { @@ -100,17 +98,16 @@ type oneDriveSuite interface { } type suiteInfoImpl struct { - ac api.Client - controller *Controller - resourceOwner string - resourceCategory resource.Category - secondaryUser string - secondaryUserID string - service path.ServiceType - tertiaryUser string - tertiaryUserID string - user string - userID string + ac api.Client + controller *Controller + resourceOwner string + secondaryUser string + secondaryUserID string + service path.ServiceType + tertiaryUser string + tertiaryUserID string + user string + userID string } func NewSuiteInfoImpl( @@ -119,22 +116,16 @@ func NewSuiteInfoImpl( resourceOwner string, service path.ServiceType, ) suiteInfoImpl { - rsc := resource.Users - if service == path.SharePointService { - rsc = resource.Sites - } - - ctrl := newController(ctx, t, rsc, path.OneDriveService) + ctrl := newController(ctx, t, path.OneDriveService) return suiteInfoImpl{ - ac: ctrl.AC, - controller: ctrl, - resourceOwner: resourceOwner, - resourceCategory: rsc, - secondaryUser: tconfig.SecondaryM365UserID(t), - service: service, - tertiaryUser: tconfig.TertiaryM365UserID(t), - user: tconfig.M365UserID(t), + ac: ctrl.AC, + controller: ctrl, + resourceOwner: resourceOwner, + secondaryUser: tconfig.SecondaryM365UserID(t), + service: service, + tertiaryUser: tconfig.TertiaryM365UserID(t), + user: tconfig.M365UserID(t), } } @@ -166,10 +157,6 @@ func (si suiteInfoImpl) Service() path.ServiceType { return si.service } -func (si suiteInfoImpl) Resource() resource.Category { - return si.resourceCategory -} - // --------------------------------------------------------------------------- // SharePoint Libraries // --------------------------------------------------------------------------- @@ -512,7 +499,6 @@ func testRestoreAndBackupMultipleFilesAndFoldersNoPermissions( testData := restoreBackupInfoMultiVersion{ service: suite.Service(), - resourceCat: suite.Resource(), backupVersion: vn, collectionsPrevious: input, collectionsLatest: expected, @@ -761,7 +747,6 @@ func testPermissionsRestoreAndBackup(suite oneDriveSuite, startVersion int) { testData := restoreBackupInfoMultiVersion{ service: suite.Service(), - resourceCat: suite.Resource(), backupVersion: vn, collectionsPrevious: input, collectionsLatest: expected, @@ -851,7 +836,6 @@ func testRestoreNoPermissionsAndBackup(suite oneDriveSuite, startVersion int) { testData := restoreBackupInfoMultiVersion{ service: suite.Service(), - resourceCat: suite.Resource(), backupVersion: vn, collectionsPrevious: input, collectionsLatest: expected, @@ -1056,7 +1040,6 @@ func testPermissionsInheritanceRestoreAndBackup(suite oneDriveSuite, startVersio testData := restoreBackupInfoMultiVersion{ service: suite.Service(), - resourceCat: suite.Resource(), backupVersion: vn, collectionsPrevious: input, collectionsLatest: expected, @@ -1251,7 +1234,6 @@ func testLinkSharesInheritanceRestoreAndBackup(suite oneDriveSuite, startVersion testData := restoreBackupInfoMultiVersion{ service: suite.Service(), - resourceCat: suite.Resource(), backupVersion: vn, collectionsPrevious: input, collectionsLatest: expected, @@ -1368,7 +1350,6 @@ func testRestoreFolderNamedFolderRegression( testData := restoreBackupInfoMultiVersion{ service: suite.Service(), - resourceCat: suite.Resource(), backupVersion: vn, collectionsPrevious: input, collectionsLatest: expected, diff --git a/src/internal/m365/stub/stub.go b/src/internal/m365/stub/stub.go index c6e4245b3..84bcfe61f 100644 --- a/src/internal/m365/stub/stub.go +++ b/src/internal/m365/stub/stub.go @@ -10,7 +10,6 @@ import ( dataMock "github.com/alcionai/corso/src/internal/data/mock" "github.com/alcionai/corso/src/internal/m365/collection/drive/metadata" "github.com/alcionai/corso/src/internal/m365/mock" - "github.com/alcionai/corso/src/internal/m365/resource" exchMock "github.com/alcionai/corso/src/internal/m365/service/exchange/mock" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/path" @@ -43,7 +42,6 @@ type ItemInfo struct { type ConfigInfo struct { Opts control.Options - Resource resource.Category Service path.ServiceType Tenant string ResourceOwners []string diff --git a/src/internal/operations/help_test.go b/src/internal/operations/help_test.go index bd8509a1a..8bf863b64 100644 --- a/src/internal/operations/help_test.go +++ b/src/internal/operations/help_test.go @@ -9,7 +9,6 @@ import ( "github.com/alcionai/corso/src/internal/common/idname" "github.com/alcionai/corso/src/internal/m365" - "github.com/alcionai/corso/src/internal/m365/resource" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/selectors" @@ -22,12 +21,11 @@ func ControllerWithSelector( t *testing.T, ctx context.Context, //revive:disable-line:context-as-argument acct account.Account, - cr resource.Category, sel selectors.Selector, ins idname.Cacher, onFail func(), ) (*m365.Controller, selectors.Selector) { - ctrl, err := m365.NewController(ctx, acct, cr, sel.PathService(), control.DefaultOptions()) + ctrl, err := m365.NewController(ctx, acct, sel.PathService(), control.DefaultOptions()) if !assert.NoError(t, err, clues.ToCore(err)) { if onFail != nil { onFail() diff --git a/src/internal/operations/restore_test.go b/src/internal/operations/restore_test.go index 3f84c32b1..d8d1767e5 100644 --- a/src/internal/operations/restore_test.go +++ b/src/internal/operations/restore_test.go @@ -19,7 +19,6 @@ import ( "github.com/alcionai/corso/src/internal/m365" "github.com/alcionai/corso/src/internal/m365/graph" "github.com/alcionai/corso/src/internal/m365/mock" - "github.com/alcionai/corso/src/internal/m365/resource" exchMock "github.com/alcionai/corso/src/internal/m365/service/exchange/mock" "github.com/alcionai/corso/src/internal/operations/inject" "github.com/alcionai/corso/src/internal/stats" @@ -344,7 +343,6 @@ func (suite *RestoreOpIntegrationSuite) TestRestore_Run_errorNoBackup() { ctrl, err := m365.NewController( ctx, suite.acct, - resource.Users, rsel.PathService(), control.DefaultOptions()) require.NoError(t, err, clues.ToCore(err)) diff --git a/src/internal/operations/test/exchange_test.go b/src/internal/operations/test/exchange_test.go index 6be354d42..6219cc38a 100644 --- a/src/internal/operations/test/exchange_test.go +++ b/src/internal/operations/test/exchange_test.go @@ -20,7 +20,6 @@ import ( evmock "github.com/alcionai/corso/src/internal/events/mock" "github.com/alcionai/corso/src/internal/m365/collection/exchange" "github.com/alcionai/corso/src/internal/m365/graph" - "github.com/alcionai/corso/src/internal/m365/resource" exchMock "github.com/alcionai/corso/src/internal/m365/service/exchange/mock" exchTD "github.com/alcionai/corso/src/internal/m365/service/exchange/testdata" "github.com/alcionai/corso/src/internal/tester" @@ -264,7 +263,7 @@ func testExchangeContinuousBackups(suite *ExchangeBackupIntgSuite, toggles contr ) opts.ToggleFeatures = toggles - ctrl, sels := ControllerWithSelector(t, ctx, acct, resource.Users, sel.Selector, nil, nil) + ctrl, sels := ControllerWithSelector(t, ctx, acct, sel.Selector, nil, nil) sel.DiscreteOwner = sels.ID() sel.DiscreteOwnerName = sels.Name() diff --git a/src/internal/operations/test/helper_test.go b/src/internal/operations/test/helper_test.go index b6e79bcd8..2d74dfd2e 100644 --- a/src/internal/operations/test/helper_test.go +++ b/src/internal/operations/test/helper_test.go @@ -20,7 +20,6 @@ import ( "github.com/alcionai/corso/src/internal/kopia" "github.com/alcionai/corso/src/internal/m365" "github.com/alcionai/corso/src/internal/m365/graph" - "github.com/alcionai/corso/src/internal/m365/resource" exchMock "github.com/alcionai/corso/src/internal/m365/service/exchange/mock" odConsts "github.com/alcionai/corso/src/internal/m365/service/onedrive/consts" "github.com/alcionai/corso/src/internal/model" @@ -132,22 +131,12 @@ func prepNewTestBackupOp( bod.sw = store.NewWrapper(bod.kms) - var connectorResource resource.Category - - switch sel.PathService() { - case path.SharePointService: - connectorResource = resource.Sites - case path.GroupsService: - connectorResource = resource.Groups - default: - connectorResource = resource.Users - } - bod.ctrl, bod.sel = ControllerWithSelector( t, ctx, bod.acct, - connectorResource, sel, nil, + sel, + nil, bod.close) bo := newTestBackupOp( @@ -543,12 +532,11 @@ func ControllerWithSelector( t *testing.T, ctx context.Context, //revive:disable-line:context-as-argument acct account.Account, - rc resource.Category, sel selectors.Selector, ins idname.Cacher, onFail func(*testing.T, context.Context), ) (*m365.Controller, selectors.Selector) { - ctrl, err := m365.NewController(ctx, acct, rc, sel.PathService(), control.DefaultOptions()) + ctrl, err := m365.NewController(ctx, acct, sel.PathService(), control.DefaultOptions()) if !assert.NoError(t, err, clues.ToCore(err)) { if onFail != nil { onFail(t, ctx) diff --git a/src/internal/operations/test/onedrive_test.go b/src/internal/operations/test/onedrive_test.go index 9646b4ec9..a571400ea 100644 --- a/src/internal/operations/test/onedrive_test.go +++ b/src/internal/operations/test/onedrive_test.go @@ -23,7 +23,6 @@ import ( "github.com/alcionai/corso/src/internal/m365/collection/drive" "github.com/alcionai/corso/src/internal/m365/collection/drive/metadata" "github.com/alcionai/corso/src/internal/m365/graph" - "github.com/alcionai/corso/src/internal/m365/resource" "github.com/alcionai/corso/src/internal/model" "github.com/alcionai/corso/src/internal/streamstore" "github.com/alcionai/corso/src/internal/tester" @@ -140,7 +139,6 @@ func (suite *OneDriveBackupIntgSuite) TestBackup_Run_incrementalOneDrive() { suite, suite.its.user.ID, suite.its.user.ID, - resource.Users, path.OneDriveService, path.FilesCategory, ic, @@ -152,7 +150,6 @@ func (suite *OneDriveBackupIntgSuite) TestBackup_Run_incrementalOneDrive() { func runDriveIncrementalTest( suite tester.Suite, owner, permissionsUser string, - rc resource.Category, service path.ServiceType, category path.CategoryType, includeContainers func([]string) selectors.Selector, @@ -195,7 +192,7 @@ func runDriveIncrementalTest( creds, err := acct.M365Config() require.NoError(t, err, clues.ToCore(err)) - ctrl, sel := ControllerWithSelector(t, ctx, acct, rc, sel, nil, nil) + ctrl, sel := ControllerWithSelector(t, ctx, acct, sel, nil, nil) ac := ctrl.AC.Drives() rh := getRestoreHandler(ctrl.AC) @@ -684,7 +681,7 @@ func runDriveIncrementalTest( } for _, test := range table { suite.Run(test.name, func() { - cleanCtrl, err := m365.NewController(ctx, acct, rc, sel.PathService(), control.DefaultOptions()) + cleanCtrl, err := m365.NewController(ctx, acct, sel.PathService(), control.DefaultOptions()) require.NoError(t, err, clues.ToCore(err)) bod.ctrl = cleanCtrl @@ -800,7 +797,6 @@ func (suite *OneDriveBackupIntgSuite) TestBackup_Run_oneDriveOwnerMigration() { ctrl, err := m365.NewController( ctx, acct, - resource.Users, path.OneDriveService, control.DefaultOptions()) require.NoError(t, err, clues.ToCore(err)) diff --git a/src/internal/operations/test/restore_helper_test.go b/src/internal/operations/test/restore_helper_test.go index 95f3f3861..32b73c8a7 100644 --- a/src/internal/operations/test/restore_helper_test.go +++ b/src/internal/operations/test/restore_helper_test.go @@ -13,7 +13,6 @@ import ( evmock "github.com/alcionai/corso/src/internal/events/mock" "github.com/alcionai/corso/src/internal/kopia" "github.com/alcionai/corso/src/internal/m365" - "github.com/alcionai/corso/src/internal/m365/resource" "github.com/alcionai/corso/src/internal/model" "github.com/alcionai/corso/src/internal/operations" "github.com/alcionai/corso/src/internal/streamstore" @@ -107,20 +106,10 @@ func prepNewTestRestoreOp( rod.sw = store.NewWrapper(rod.kms) - connectorResource := resource.Users - - switch sel.Service { - case selectors.ServiceSharePoint: - connectorResource = resource.Sites - case selectors.ServiceGroups: - connectorResource = resource.Groups - } - rod.ctrl, rod.sel = ControllerWithSelector( t, ctx, rod.acct, - connectorResource, sel, nil, rod.close) diff --git a/src/internal/operations/test/sharepoint_test.go b/src/internal/operations/test/sharepoint_test.go index 525e83ede..65f9c6bb7 100644 --- a/src/internal/operations/test/sharepoint_test.go +++ b/src/internal/operations/test/sharepoint_test.go @@ -15,7 +15,6 @@ import ( evmock "github.com/alcionai/corso/src/internal/events/mock" "github.com/alcionai/corso/src/internal/m365/collection/drive" "github.com/alcionai/corso/src/internal/m365/graph" - "github.com/alcionai/corso/src/internal/m365/resource" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/internal/tester/tconfig" "github.com/alcionai/corso/src/internal/version" @@ -81,7 +80,6 @@ func (suite *SharePointBackupIntgSuite) TestBackup_Run_incrementalSharePoint() { suite, suite.its.site.ID, suite.its.user.ID, - resource.Sites, path.SharePointService, path.LibrariesCategory, ic, diff --git a/src/pkg/backup/details/details.go b/src/pkg/backup/details/details.go index a51f5a277..ab2b57191 100644 --- a/src/pkg/backup/details/details.go +++ b/src/pkg/backup/details/details.go @@ -46,11 +46,7 @@ func (d *Details) add( // Use the item name and the path for the ShortRef. This ensures that renames // within a directory generate unique ShortRefs. - if info.infoType() == OneDriveItem || info.infoType() == SharePointLibrary { - if info.OneDrive == nil && info.SharePoint == nil && info.Groups == nil { - return entry, clues.New("item is not Groups, SharePoint or OneDrive type") - } - + if info.isDriveItem() { // clean metadata suffixes from item refs entry.ItemRef = withoutMetadataSuffix(entry.ItemRef) } diff --git a/src/pkg/backup/details/iteminfo.go b/src/pkg/backup/details/iteminfo.go index 25a77d5c2..180572db3 100644 --- a/src/pkg/backup/details/iteminfo.go +++ b/src/pkg/backup/details/iteminfo.go @@ -182,3 +182,14 @@ func (i ItemInfo) updateFolder(f *FolderInfo) error { return clues.New("unsupported type") } } + +// true if the info represents an item backed by the drive api. +func (i ItemInfo) isDriveItem() bool { + iit := i.infoType() + + if !(iit == OneDriveItem || iit == SharePointLibrary) { + return false + } + + return !(i.OneDrive == nil && i.SharePoint == nil && i.Groups == nil) +} diff --git a/src/pkg/backup/details/iteminfo_test.go b/src/pkg/backup/details/iteminfo_test.go new file mode 100644 index 000000000..1073edb27 --- /dev/null +++ b/src/pkg/backup/details/iteminfo_test.go @@ -0,0 +1,86 @@ +package details + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" + + "github.com/alcionai/corso/src/internal/tester" +) + +type ItemInfoUnitSuite struct { + tester.Suite +} + +func TestItemInfoUnitSuite(t *testing.T) { + suite.Run(t, &ItemInfoUnitSuite{Suite: tester.NewUnitSuite(t)}) +} + +func (suite *ItemInfoUnitSuite) TestItemInfo_IsDriveItem() { + table := []struct { + name string + ii ItemInfo + expect assert.BoolAssertionFunc + }{ + { + name: "onedrive item", + ii: ItemInfo{ + OneDrive: &OneDriveInfo{ + ItemType: OneDriveItem, + }, + }, + expect: assert.True, + }, + { + name: "sharepoint library", + ii: ItemInfo{ + SharePoint: &SharePointInfo{ + ItemType: SharePointLibrary, + }, + }, + expect: assert.True, + }, + { + name: "sharepoint page", + ii: ItemInfo{ + SharePoint: &SharePointInfo{ + ItemType: SharePointPage, + }, + }, + expect: assert.False, + }, + { + name: "groups library", + ii: ItemInfo{ + Groups: &GroupsInfo{ + ItemType: SharePointLibrary, + }, + }, + expect: assert.True, + }, + { + name: "groups channel message", + ii: ItemInfo{ + Groups: &GroupsInfo{ + ItemType: GroupsChannelMessage, + }, + }, + expect: assert.False, + }, + { + name: "exchange anything", + ii: ItemInfo{ + Groups: &GroupsInfo{ + ItemType: ExchangeMail, + }, + }, + expect: assert.False, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + test.expect(suite.T(), test.ii.isDriveItem()) + }) + } +} diff --git a/src/pkg/repository/repository.go b/src/pkg/repository/repository.go index fe607a4c5..4cd361e07 100644 --- a/src/pkg/repository/repository.go +++ b/src/pkg/repository/repository.go @@ -16,7 +16,6 @@ import ( "github.com/alcionai/corso/src/internal/kopia" "github.com/alcionai/corso/src/internal/m365" "github.com/alcionai/corso/src/internal/m365/collection/drive/metadata" - "github.com/alcionai/corso/src/internal/m365/resource" "github.com/alcionai/corso/src/internal/model" "github.com/alcionai/corso/src/internal/observe" "github.com/alcionai/corso/src/internal/operations" @@ -768,13 +767,7 @@ func connectToM365( defer close(progressBar) } - // retrieve data from the producer - rc := resource.Users - if pst == path.SharePointService { - rc = resource.Sites - } - - ctrl, err := m365.NewController(ctx, acct, rc, pst, co) + ctrl, err := m365.NewController(ctx, acct, pst, co) if err != nil { return nil, err } diff --git a/src/pkg/repository/repository_test.go b/src/pkg/repository/repository_test.go index 8f9725dc2..bd123d24c 100644 --- a/src/pkg/repository/repository_test.go +++ b/src/pkg/repository/repository_test.go @@ -259,7 +259,7 @@ func (suite *RepositoryIntegrationSuite) TestNewBackup() { userID := tconfig.M365UserID(t) - bo, err := r.NewBackup(ctx, selectors.Selector{DiscreteOwner: userID}) + bo, err := r.NewBackup(ctx, selectors.NewExchangeBackup([]string{userID}).Selector) require.NoError(t, err, clues.ToCore(err)) require.NotNil(t, bo) } @@ -284,7 +284,11 @@ func (suite *RepositoryIntegrationSuite) TestNewRestore() { ctrlRepo.Retention{}) require.NoError(t, err, clues.ToCore(err)) - ro, err := r.NewRestore(ctx, "backup-id", selectors.Selector{DiscreteOwner: "test"}, restoreCfg) + ro, err := r.NewRestore( + ctx, + "backup-id", + selectors.NewExchangeBackup([]string{"test"}).Selector, + restoreCfg) require.NoError(t, err, clues.ToCore(err)) require.NotNil(t, ro) } @@ -331,7 +335,7 @@ func (suite *RepositoryIntegrationSuite) TestNewBackupAndDelete() { ro, err := r.NewRestore( ctx, backupID, - selectors.Selector{DiscreteOwner: userID}, + selectors.NewExchangeBackup([]string{userID}).Selector, restoreCfg) require.NoError(t, err, clues.ToCore(err)) require.NotNil(t, ro)