From cf1c80271f7a3416db7fb83febbc0c29bcc6662f Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Wed, 1 Feb 2023 13:34:16 -0800 Subject: [PATCH] OneDrive tests for Get() (#2342) ## Description Create a test case that allows checking most of the Get call (getting item data is not mocked right now). This allows better checking on things like: * having multiple drives to backup * having items split across multiple delta query pages * errors returned by the API * aggregation of delta URLs and folder paths in metadata Eventually these tests could help with checking: * consumption of metadata for incremental backups * consumption of delta tokens for incremental backups Long-term, we may want to merge these with the UpdateCollections tests as there is overlap between the two. We may also want to consider a more stable API for checking the returned items. Right now it partly relies on being able to cast to a onedrive.Collection struct instead of using `Items()` ## 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 - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [x] :robot: Test - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup ## Issue(s) * #2264 ## Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- .../connector/onedrive/collections.go | 29 +- .../connector/onedrive/collections_test.go | 423 +++++++++++++++++- 2 files changed, 439 insertions(+), 13 deletions(-) diff --git a/src/internal/connector/onedrive/collections.go b/src/internal/connector/onedrive/collections.go index e3d60fe2b..200e51e23 100644 --- a/src/internal/connector/onedrive/collections.go +++ b/src/internal/connector/onedrive/collections.go @@ -67,6 +67,12 @@ type Collections struct { // Not the most ideal, but allows us to change the pager function for testing // as needed. This will allow us to mock out some scenarios during testing. + drivePagerFunc func( + source driveSource, + servicer graph.Servicer, + resourceOwner string, + fields []string, + ) (drivePager, error) itemPagerFunc func( servicer graph.Servicer, driveID, link string, @@ -89,16 +95,17 @@ func NewCollections( ctrlOpts control.Options, ) *Collections { return &Collections{ - itemClient: itemClient, - tenant: tenant, - resourceOwner: resourceOwner, - source: source, - matcher: matcher, - CollectionMap: map[string]data.Collection{}, - itemPagerFunc: defaultItemPager, - service: service, - statusUpdater: statusUpdater, - ctrl: ctrlOpts, + itemClient: itemClient, + tenant: tenant, + resourceOwner: resourceOwner, + source: source, + matcher: matcher, + CollectionMap: map[string]data.Collection{}, + drivePagerFunc: PagerForSource, + itemPagerFunc: defaultItemPager, + service: service, + statusUpdater: statusUpdater, + ctrl: ctrlOpts, } } @@ -250,7 +257,7 @@ func (c *Collections) Get( } // Enumerate drives for the specified resourceOwner - pager, err := PagerForSource(c.source, c.service, c.resourceOwner, nil) + pager, err := c.drivePagerFunc(c.source, c.service, c.resourceOwner, nil) if err != nil { return nil, nil, err } diff --git a/src/internal/connector/onedrive/collections_test.go b/src/internal/connector/onedrive/collections_test.go index 39e094cd6..21dae9549 100644 --- a/src/internal/connector/onedrive/collections_test.go +++ b/src/internal/connector/onedrive/collections_test.go @@ -1,9 +1,11 @@ package onedrive import ( + "context" "strings" "testing" + "github.com/google/uuid" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -11,6 +13,7 @@ import ( "golang.org/x/exp/maps" "github.com/alcionai/corso/src/internal/connector/graph" + gapi "github.com/alcionai/corso/src/internal/connector/graph/api" "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/tester" @@ -969,7 +972,419 @@ func (suite *OneDriveCollectionsSuite) TestDeserializeMetadata() { } } -func driveItem(id string, name string, parentPath string, isFile, isFolder, isPackage bool) models.DriveItemable { +type mockDeltaPageLinker struct { + link *string + delta *string +} + +func (pl *mockDeltaPageLinker) GetOdataNextLink() *string { + return pl.link +} + +func (pl *mockDeltaPageLinker) GetOdataDeltaLink() *string { + return pl.delta +} + +type deltaPagerResult struct { + items []models.DriveItemable + nextLink *string + deltaLink *string + err error +} + +type mockItemPager struct { + // DriveID -> set of return values for queries for that drive. + toReturn []deltaPagerResult + getIdx int +} + +func (p *mockItemPager) GetPage(context.Context) (gapi.DeltaPageLinker, error) { + if len(p.toReturn) <= p.getIdx { + return nil, assert.AnError + } + + idx := p.getIdx + p.getIdx++ + + return &mockDeltaPageLinker{ + p.toReturn[idx].nextLink, + p.toReturn[idx].deltaLink, + }, p.toReturn[idx].err +} + +func (p *mockItemPager) SetNext(string) {} + +func (p *mockItemPager) ValuesIn(gapi.DeltaPageLinker) ([]models.DriveItemable, error) { + idx := p.getIdx + if idx > 0 { + // Return values lag by one since we increment in GetPage(). + idx-- + } + + if len(p.toReturn) <= idx { + return nil, assert.AnError + } + + return p.toReturn[idx].items, nil +} + +func (suite *OneDriveCollectionsSuite) TestGet() { + anyFolder := (&selectors.OneDriveBackup{}).Folders(selectors.Any())[0] + + tenant := "a-tenant" + user := "a-user" + + metadataPath, err := path.Builder{}.ToServiceCategoryMetadataPath( + tenant, + user, + path.OneDriveService, + path.FilesCategory, + false, + ) + require.NoError(suite.T(), err, "making metadata path") + + folderPath := expectedPathAsSlice( + suite.T(), + tenant, + user, + testBaseDrivePath+"/folder", + )[0] + + empty := "" + next := "next" + delta := "delta1" + delta2 := "delta2" + + driveID1 := uuid.NewString() + drive1 := models.NewDrive() + drive1.SetId(&driveID1) + drive1.SetName(&driveID1) + + driveID2 := uuid.NewString() + drive2 := models.NewDrive() + drive2.SetId(&driveID2) + drive2.SetName(&driveID2) + + driveBasePath2 := "drive/driveID2/root:" + + folderPath2 := expectedPathAsSlice( + suite.T(), + tenant, + user, + driveBasePath2+"/folder", + )[0] + + table := []struct { + name string + drives []models.Driveable + items map[string][]deltaPagerResult + errCheck assert.ErrorAssertionFunc + // Collection name -> set of item IDs. We can't check item data because + // that's not mocked out. Metadata is checked separately. + expectedCollections map[string][]string + expectedDeltaURLs map[string]string + expectedFolderPaths map[string]map[string]string + expectedDelList map[string]struct{} + }{ + { + name: "OneDrive_OneItemPage_DelFileOnly_NoFolders_NoErrors", + drives: []models.Driveable{drive1}, + items: map[string][]deltaPagerResult{ + driveID1: { + { + items: []models.DriveItemable{ + delItem("file", testBaseDrivePath, true, false, false), + }, + deltaLink: &delta, + }, + }, + }, + errCheck: assert.NoError, + expectedCollections: map[string][]string{}, + expectedDeltaURLs: map[string]string{ + driveID1: delta, + }, + expectedFolderPaths: map[string]map[string]string{ + // We need an empty map here so deserializing metadata knows the delta + // token for this drive is valid. + driveID1: {}, + }, + expectedDelList: map[string]struct{}{ + "file": {}, + }, + }, + { + name: "OneDrive_OneItemPage_NoFolders_NoErrors", + drives: []models.Driveable{drive1}, + items: map[string][]deltaPagerResult{ + driveID1: { + { + items: []models.DriveItemable{ + driveItem("file", "file", testBaseDrivePath, true, false, false), + }, + deltaLink: &delta, + }, + }, + }, + errCheck: assert.NoError, + expectedCollections: map[string][]string{ + expectedPathAsSlice( + suite.T(), + tenant, + user, + testBaseDrivePath, + )[0]: {"file"}, + }, + expectedDeltaURLs: map[string]string{ + driveID1: delta, + }, + expectedFolderPaths: map[string]map[string]string{ + // We need an empty map here so deserializing metadata knows the delta + // token for this drive is valid. + driveID1: {}, + }, + expectedDelList: map[string]struct{}{}, + }, + { + name: "OneDrive_OneItemPage_NoErrors", + drives: []models.Driveable{drive1}, + items: map[string][]deltaPagerResult{ + driveID1: { + { + items: []models.DriveItemable{ + driveItem("folder", "folder", testBaseDrivePath, false, true, false), + driveItem("file", "file", testBaseDrivePath+"/folder", true, false, false), + }, + deltaLink: &delta, + }, + }, + }, + errCheck: assert.NoError, + expectedCollections: map[string][]string{ + folderPath: {"file"}, + }, + expectedDeltaURLs: map[string]string{ + driveID1: delta, + }, + expectedFolderPaths: map[string]map[string]string{ + driveID1: { + "folder": folderPath, + }, + }, + expectedDelList: map[string]struct{}{}, + }, + { + name: "OneDrive_OneItemPage_EmptyDelta_NoErrors", + drives: []models.Driveable{drive1}, + items: map[string][]deltaPagerResult{ + driveID1: { + { + items: []models.DriveItemable{ + driveItem("folder", "folder", testBaseDrivePath, false, true, false), + driveItem("file", "file", testBaseDrivePath+"/folder", true, false, false), + }, + deltaLink: &empty, + }, + }, + }, + errCheck: assert.NoError, + expectedCollections: map[string][]string{ + folderPath: {"file"}, + }, + expectedDeltaURLs: map[string]string{}, + expectedFolderPaths: map[string]map[string]string{}, + expectedDelList: map[string]struct{}{}, + }, + { + name: "OneDrive_TwoItemPages_NoErrors", + drives: []models.Driveable{drive1}, + items: map[string][]deltaPagerResult{ + driveID1: { + { + items: []models.DriveItemable{ + driveItem("folder", "folder", testBaseDrivePath, false, true, false), + driveItem("file", "file", testBaseDrivePath+"/folder", true, false, false), + }, + nextLink: &next, + }, + { + items: []models.DriveItemable{ + driveItem("folder", "folder", testBaseDrivePath, false, true, false), + driveItem("file2", "file2", testBaseDrivePath+"/folder", true, false, false), + }, + deltaLink: &delta, + }, + }, + }, + errCheck: assert.NoError, + expectedCollections: map[string][]string{ + folderPath: {"file", "file2"}, + }, + expectedDeltaURLs: map[string]string{ + driveID1: delta, + }, + expectedFolderPaths: map[string]map[string]string{ + driveID1: { + "folder": folderPath, + }, + }, + expectedDelList: map[string]struct{}{}, + }, + { + name: "TwoDrives_OneItemPageEach_NoErrors", + drives: []models.Driveable{ + drive1, + drive2, + }, + items: map[string][]deltaPagerResult{ + driveID1: { + { + items: []models.DriveItemable{ + driveItem("folder", "folder", testBaseDrivePath, false, true, false), + driveItem("file", "file", testBaseDrivePath+"/folder", true, false, false), + }, + deltaLink: &delta, + }, + }, + driveID2: { + { + items: []models.DriveItemable{ + driveItem("folder", "folder", driveBasePath2, false, true, false), + driveItem("file", "file", driveBasePath2+"/folder", true, false, false), + }, + deltaLink: &delta2, + }, + }, + }, + errCheck: assert.NoError, + expectedCollections: map[string][]string{ + folderPath: {"file"}, + folderPath2: {"file"}, + }, + expectedDeltaURLs: map[string]string{ + driveID1: delta, + driveID2: delta2, + }, + expectedFolderPaths: map[string]map[string]string{ + driveID1: { + "folder": folderPath, + }, + driveID2: { + "folder": folderPath2, + }, + }, + expectedDelList: map[string]struct{}{}, + }, + { + name: "OneDrive_OneItemPage_Errors", + drives: []models.Driveable{drive1}, + items: map[string][]deltaPagerResult{ + driveID1: { + { + err: assert.AnError, + }, + }, + }, + errCheck: assert.Error, + expectedCollections: nil, + expectedDeltaURLs: nil, + expectedFolderPaths: nil, + expectedDelList: nil, + }, + } + for _, test := range table { + suite.T().Run(test.name, func(t *testing.T) { + ctx, flush := tester.NewContext() + defer flush() + + drivePagerFunc := func( + source driveSource, + servicer graph.Servicer, + resourceOwner string, + fields []string, + ) (drivePager, error) { + return &mockDrivePager{ + toReturn: []pagerResult{ + { + drives: test.drives, + }, + }, + }, nil + } + + itemPagerFunc := func( + servicer graph.Servicer, + driveID, link string, + ) itemPager { + return &mockItemPager{ + toReturn: test.items[driveID], + } + } + + c := NewCollections( + graph.HTTPClient(graph.NoTimeout()), + tenant, + user, + OneDriveSource, + testFolderMatcher{anyFolder}, + &MockGraphService{}, + func(*support.ConnectorOperationStatus) {}, + control.Options{}, + ) + c.drivePagerFunc = drivePagerFunc + c.itemPagerFunc = itemPagerFunc + + // TODO(ashmrtn): Allow passing previous metadata. + cols, _, err := c.Get(ctx, nil) + test.errCheck(t, err) + + if err != nil { + return + } + + for _, baseCol := range cols { + folderPath := baseCol.FullPath().String() + if folderPath == metadataPath.String() { + deltas, paths, err := deserializeMetadata(ctx, []data.Collection{baseCol}) + if !assert.NoError(t, err, "deserializing metadata") { + continue + } + + assert.Equal(t, test.expectedDeltaURLs, deltas) + assert.Equal(t, test.expectedFolderPaths, paths) + + continue + } + + // TODO(ashmrtn): We should really be getting items in the collection + // via the Items() channel, but we don't have a way to mock out the + // actual item fetch yet (mostly wiring issues). The lack of that makes + // this check a bit more bittle since internal details can change. + col, ok := baseCol.(*Collection) + require.True(t, ok, "getting onedrive.Collection handle") + + itemIDs := make([]string, 0, len(col.driveItems)) + + for id := range col.driveItems { + itemIDs = append(itemIDs, id) + } + + assert.ElementsMatch(t, test.expectedCollections[folderPath], itemIDs) + } + + // TODO(ashmrtn): Uncomment this when we begin return the set of items to + // remove from the upcoming backup. + // assert.Equal(t, test.expectedDelList, delList) + }) + } +} + +func driveItem( + id string, + name string, + parentPath string, + isFile, isFolder, isPackage bool, +) models.DriveItemable { item := models.NewDriveItem() item.SetName(&name) item.SetId(&id) @@ -992,7 +1407,11 @@ func driveItem(id string, name string, parentPath string, isFile, isFolder, isPa // delItem creates a DriveItemable that is marked as deleted. path must be set // to the base drive path. -func delItem(id string, parentPath string, isFile, isFolder, isPackage bool) models.DriveItemable { +func delItem( + id string, + parentPath string, + isFile, isFolder, isPackage bool, +) models.DriveItemable { item := models.NewDriveItem() item.SetId(&id) item.SetDeleted(models.NewDeleted())