From 66e5e7693d56a31e6fd9b1031d1571d1e244b5fd Mon Sep 17 00:00:00 2001 From: Abin Simon Date: Fri, 10 Feb 2023 23:27:10 +0530 Subject: [PATCH] Use ids for CollectionsMap for OneDrive (#2457) ## Description This will help us with delta backups where we might run into a single folder changing multiple times. ## Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [x] :clock1: Yes, but in a later PR - [ ] :no_entry: No ## Type of change - [x] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup ## Issue(s) * https://github.com/alcionai/corso/pull/2407 ## Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- .../connector/onedrive/collections.go | 17 +- .../connector/onedrive/collections_test.go | 369 ++++++++++-------- .../sharepoint/data_collections_test.go | 35 +- 3 files changed, 251 insertions(+), 170 deletions(-) diff --git a/src/internal/connector/onedrive/collections.go b/src/internal/connector/onedrive/collections.go index a951d682e..9af0e87b3 100644 --- a/src/internal/connector/onedrive/collections.go +++ b/src/internal/connector/onedrive/collections.go @@ -28,7 +28,11 @@ const ( OneDriveSource SharePointSource ) -const restrictedDirectory = "Site Pages" + +const ( + restrictedDirectory = "Site Pages" + rootDrivePattern = "/drives/%s/root:" +) func (ds driveSource) toPathServiceCat() (path.ServiceType, path.CategoryType) { switch ds { @@ -382,11 +386,15 @@ func (c *Collections) UpdateCollections( continue } - if item.GetParentReference() == nil || item.GetParentReference().GetPath() == nil { + if item.GetParentReference() == nil || + item.GetParentReference().GetPath() == nil || + item.GetParentReference().GetId() == nil { return errors.Errorf("item does not have a parent reference. item name : %s", *item.GetName()) } // Create a collection for the parent of this item + collectionID := *item.GetParentReference().GetId() + collectionPath, err := GetCanonicalPath( *item.GetParentReference().GetPath(), c.tenant, @@ -454,8 +462,7 @@ func (c *Collections) UpdateCollections( // TODO(ashmrtn): Figure what when an item was moved (maybe) and add it to // the exclude list. - col, found := c.CollectionMap[collectionPath.String()] - + col, found := c.CollectionMap[collectionID] if !found { // TODO(ashmrtn): Compare old and new path and set collection state // accordingly. @@ -470,7 +477,7 @@ func (c *Collections) UpdateCollections( invalidPrevDelta, ) - c.CollectionMap[collectionPath.String()] = col + c.CollectionMap[collectionID] = col c.NumContainers++ } diff --git a/src/internal/connector/onedrive/collections_test.go b/src/internal/connector/onedrive/collections_test.go index 3cc5dbcb5..da42beec5 100644 --- a/src/internal/connector/onedrive/collections_test.go +++ b/src/internal/connector/onedrive/collections_test.go @@ -2,6 +2,7 @@ package onedrive import ( "context" + "fmt" "strings" "testing" @@ -23,10 +24,6 @@ import ( "github.com/alcionai/corso/src/pkg/selectors" ) -const ( - testBaseDrivePath = "drive/driveID1/root:" -) - func expectedPathAsSlice(t *testing.T, tenant, user string, rest ...string) []string { res := make([]string, 0, len(rest)) @@ -102,12 +99,15 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { pkg = "/package" ) + testBaseDrivePath := fmt.Sprintf(rootDrivePattern, "driveID1") + tests := []struct { testCase string items []models.DriveItemable inputFolderMap map[string]string scope selectors.OneDriveScope expect assert.ErrorAssertionFunc + expectedCollectionIDs []string expectedCollectionPaths []string expectedItemCount int expectedContainerCount int @@ -118,7 +118,8 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "Invalid item", items: []models.DriveItemable{ - driveItem("item", "item", testBaseDrivePath, false, false, false), + driveRootItem("root"), + driveItem("item", "item", testBaseDrivePath, "root", false, false, false), }, inputFolderMap: map[string]string{}, scope: anyFolder, @@ -129,11 +130,13 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "Single File", items: []models.DriveItemable{ - driveItem("file", "file", testBaseDrivePath, true, false, false), + driveRootItem("root"), + driveItem("file", "file", testBaseDrivePath, "root", true, false, false), }, - inputFolderMap: map[string]string{}, - scope: anyFolder, - expect: assert.NoError, + inputFolderMap: map[string]string{}, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionIDs: []string{"root"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -150,11 +153,13 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "Single Folder", items: []models.DriveItemable{ - driveItem("folder", "folder", testBaseDrivePath, false, true, false), + driveRootItem("root"), + driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false), }, - inputFolderMap: map[string]string{}, - scope: anyFolder, - expect: assert.NoError, + inputFolderMap: map[string]string{}, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionIDs: []string{"root"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -176,11 +181,13 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "Single Package", items: []models.DriveItemable{ - driveItem("package", "package", testBaseDrivePath, false, false, true), + driveRootItem("root"), + driveItem("package", "package", testBaseDrivePath, "root", false, false, true), }, - inputFolderMap: map[string]string{}, - scope: anyFolder, - expect: assert.NoError, + inputFolderMap: map[string]string{}, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionIDs: []string{"root"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -202,15 +209,17 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "1 root file, 1 folder, 1 package, 2 files, 3 collections", items: []models.DriveItemable{ - driveItem("fileInRoot", "fileInRoot", testBaseDrivePath, true, false, false), - driveItem("folder", "folder", testBaseDrivePath, false, true, false), - driveItem("package", "package", testBaseDrivePath, false, false, true), - driveItem("fileInFolder", "fileInFolder", testBaseDrivePath+folder, true, false, false), - driveItem("fileInPackage", "fileInPackage", testBaseDrivePath+pkg, true, false, false), + driveRootItem("root"), + driveItem("fileInRoot", "fileInRoot", testBaseDrivePath, "root", true, false, false), + driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false), + driveItem("package", "package", testBaseDrivePath, "root", false, false, true), + driveItem("fileInFolder", "fileInFolder", testBaseDrivePath+folder, "folder", true, false, false), + driveItem("fileInPackage", "fileInPackage", testBaseDrivePath+pkg, "package", true, false, false), }, - inputFolderMap: map[string]string{}, - scope: anyFolder, - expect: assert.NoError, + inputFolderMap: map[string]string{}, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionIDs: []string{"root", "folder", "package"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -241,18 +250,20 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "contains folder selector", items: []models.DriveItemable{ - driveItem("fileInRoot", "fileInRoot", testBaseDrivePath, true, false, false), - driveItem("folder", "folder", testBaseDrivePath, false, true, false), - driveItem("subfolder", "subfolder", testBaseDrivePath+folder, false, true, false), - driveItem("folder2", "folder", testBaseDrivePath+folderSub, false, true, false), - driveItem("package", "package", testBaseDrivePath, false, false, true), - driveItem("fileInFolder", "fileInFolder", testBaseDrivePath+folder, true, false, false), - driveItem("fileInFolder2", "fileInFolder2", testBaseDrivePath+folderSub+folder, true, false, false), - driveItem("fileInFolderPackage", "fileInPackage", testBaseDrivePath+pkg, true, false, false), + driveRootItem("root"), + driveItem("fileInRoot", "fileInRoot", testBaseDrivePath, "root", true, false, false), + driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false), + driveItem("subfolder", "subfolder", testBaseDrivePath+folder, "folder", false, true, false), + driveItem("folder2", "folder", testBaseDrivePath+folderSub, "subfolder", false, true, false), + driveItem("package", "package", testBaseDrivePath, "root", false, false, true), + driveItem("fileInFolder", "fileInFolder", testBaseDrivePath+folder, "folder", true, false, false), + driveItem("fileInFolder2", "fileInFolder2", testBaseDrivePath+folderSub+folder, "folder2", true, false, false), + driveItem("fileInFolderPackage", "fileInPackage", testBaseDrivePath+pkg, "package", true, false, false), }, - inputFolderMap: map[string]string{}, - scope: (&selectors.OneDriveBackup{}).Folders([]string{"folder"})[0], - expect: assert.NoError, + inputFolderMap: map[string]string{}, + scope: (&selectors.OneDriveBackup{}).Folders([]string{"folder"})[0], + expect: assert.NoError, + expectedCollectionIDs: []string{"folder", "subfolder", "folder2"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -285,19 +296,21 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "prefix subfolder selector", items: []models.DriveItemable{ - driveItem("fileInRoot", "fileInRoot", testBaseDrivePath, true, false, false), - driveItem("folder", "folder", testBaseDrivePath, false, true, false), - driveItem("subfolder", "subfolder", testBaseDrivePath+folder, false, true, false), - driveItem("folder2", "folder", testBaseDrivePath+folderSub, false, true, false), - driveItem("package", "package", testBaseDrivePath, false, false, true), - driveItem("fileInFolder", "fileInFolder", testBaseDrivePath+folder, true, false, false), - driveItem("fileInFolder2", "fileInFolder2", testBaseDrivePath+folderSub+folder, true, false, false), - driveItem("fileInPackage", "fileInPackage", testBaseDrivePath+pkg, true, false, false), + driveRootItem("root"), + driveItem("fileInRoot", "fileInRoot", testBaseDrivePath, "root", true, false, false), + driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false), + driveItem("subfolder", "subfolder", testBaseDrivePath+folder, "folder", false, true, false), + driveItem("folder2", "folder", testBaseDrivePath+folderSub, "subfolder", false, true, false), + driveItem("package", "package", testBaseDrivePath, "root", false, false, true), + driveItem("fileInFolder", "fileInFolder", testBaseDrivePath+folder, "folder", true, false, false), + driveItem("fileInFolder2", "fileInFolder2", testBaseDrivePath+folderSub+folder, "folder2", true, false, false), + driveItem("fileInPackage", "fileInPackage", testBaseDrivePath+pkg, "package", true, false, false), }, inputFolderMap: map[string]string{}, scope: (&selectors.OneDriveBackup{}). Folders([]string{"/folder/subfolder"}, selectors.PrefixMatch())[0], - expect: assert.NoError, + expect: assert.NoError, + expectedCollectionIDs: []string{"subfolder", "folder2"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -321,17 +334,19 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "match subfolder selector", items: []models.DriveItemable{ - driveItem("fileInRoot", "fileInRoot", testBaseDrivePath, true, false, false), - driveItem("folder", "folder", testBaseDrivePath, false, true, false), - driveItem("subfolder", "subfolder", testBaseDrivePath+folder, false, true, false), - driveItem("package", "package", testBaseDrivePath, false, false, true), - driveItem("fileInFolder", "fileInFolder", testBaseDrivePath+folder, true, false, false), - driveItem("fileInSubfolder", "fileInSubfolder", testBaseDrivePath+folderSub, true, false, false), - driveItem("fileInPackage", "fileInPackage", testBaseDrivePath+pkg, true, false, false), + driveRootItem("root"), + driveItem("fileInRoot", "fileInRoot", testBaseDrivePath, "root", true, false, false), + driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false), + driveItem("subfolder", "subfolder", testBaseDrivePath+folder, "folder", false, true, false), + driveItem("package", "package", testBaseDrivePath, "root", false, false, true), + driveItem("fileInFolder", "fileInFolder", testBaseDrivePath+folder, "folder", true, false, false), + driveItem("fileInSubfolder", "fileInSubfolder", testBaseDrivePath+folderSub, "subfolder", true, false, false), + driveItem("fileInPackage", "fileInPackage", testBaseDrivePath+pkg, "package", true, false, false), }, - inputFolderMap: map[string]string{}, - scope: (&selectors.OneDriveBackup{}).Folders([]string{"folder/subfolder"})[0], - expect: assert.NoError, + inputFolderMap: map[string]string{}, + scope: (&selectors.OneDriveBackup{}).Folders([]string{"folder/subfolder"})[0], + expect: assert.NoError, + expectedCollectionIDs: []string{"subfolder"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -348,7 +363,8 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "not moved folder tree", items: []models.DriveItemable{ - driveItem("folder", "folder", testBaseDrivePath, false, true, false), + driveRootItem("root"), + driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false), }, inputFolderMap: map[string]string{ "folder": expectedPathAsSlice( @@ -364,8 +380,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { testBaseDrivePath+"/folder/subfolder", )[0], }, - scope: anyFolder, - expect: assert.NoError, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionIDs: []string{"root"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -394,7 +411,8 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "moved folder tree", items: []models.DriveItemable{ - driveItem("folder", "folder", testBaseDrivePath, false, true, false), + driveRootItem("root"), + driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false), }, inputFolderMap: map[string]string{ "folder": expectedPathAsSlice( @@ -410,8 +428,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { testBaseDrivePath+"/a-folder/subfolder", )[0], }, - scope: anyFolder, - expect: assert.NoError, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionIDs: []string{"root"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -440,8 +459,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "moved folder tree and subfolder 1", items: []models.DriveItemable{ - driveItem("folder", "folder", testBaseDrivePath, false, true, false), - driveItem("subfolder", "subfolder", testBaseDrivePath, false, true, false), + driveRootItem("root"), + driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false), + driveItem("subfolder", "subfolder", testBaseDrivePath, "root", false, true, false), }, inputFolderMap: map[string]string{ "folder": expectedPathAsSlice( @@ -457,8 +477,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { testBaseDrivePath+"/a-folder/subfolder", )[0], }, - scope: anyFolder, - expect: assert.NoError, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionIDs: []string{"root"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -487,8 +508,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "moved folder tree and subfolder 2", items: []models.DriveItemable{ - driveItem("subfolder", "subfolder", testBaseDrivePath, false, true, false), - driveItem("folder", "folder", testBaseDrivePath, false, true, false), + driveRootItem("root"), + driveItem("subfolder", "subfolder", testBaseDrivePath, "root", false, true, false), + driveItem("folder", "folder", testBaseDrivePath, "root", false, true, false), }, inputFolderMap: map[string]string{ "folder": expectedPathAsSlice( @@ -504,8 +526,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { testBaseDrivePath+"/a-folder/subfolder", )[0], }, - scope: anyFolder, - expect: assert.NoError, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionIDs: []string{"root"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -534,8 +557,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "deleted folder and package", items: []models.DriveItemable{ - delItem("folder", testBaseDrivePath, false, true, false), - delItem("package", testBaseDrivePath, false, false, true), + driveRootItem("root"), // root is always present, but not necessary here + delItem("folder", testBaseDrivePath, "root", false, true, false), + delItem("package", testBaseDrivePath, "root", false, false, true), }, inputFolderMap: map[string]string{ "folder": expectedPathAsSlice( @@ -563,8 +587,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "delete folder tree move subfolder", items: []models.DriveItemable{ - delItem("folder", testBaseDrivePath, false, true, false), - driveItem("subfolder", "subfolder", testBaseDrivePath, false, true, false), + driveRootItem("root"), + delItem("folder", testBaseDrivePath, "root", false, true, false), + driveItem("subfolder", "subfolder", testBaseDrivePath, "root", false, true, false), }, inputFolderMap: map[string]string{ "folder": expectedPathAsSlice( @@ -580,8 +605,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { testBaseDrivePath+"/folder/subfolder", )[0], }, - scope: anyFolder, - expect: assert.NoError, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionIDs: []string{"root"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -604,7 +630,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { { testCase: "delete file", items: []models.DriveItemable{ - delItem("item", testBaseDrivePath, true, false, false), + delItem("item", testBaseDrivePath, "root", true, false, false), }, inputFolderMap: map[string]string{}, scope: anyFolder, @@ -640,7 +666,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { err := c.UpdateCollections( ctx, - "driveID", + "driveID1", "General", tt.items, tt.inputFolderMap, @@ -649,14 +675,19 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { false, ) tt.expect(t, err) - assert.Equal(t, len(tt.expectedCollectionPaths), len(c.CollectionMap), "collection paths") + assert.Equal(t, len(tt.expectedCollectionIDs), len(c.CollectionMap), "collection ids") assert.Equal(t, tt.expectedItemCount, c.NumItems, "item count") assert.Equal(t, tt.expectedFileCount, c.NumFiles, "file count") assert.Equal(t, tt.expectedContainerCount, c.NumContainers, "container count") - for _, collPath := range tt.expectedCollectionPaths { + + for _, collPath := range tt.expectedCollectionIDs { assert.Contains(t, c.CollectionMap, collPath) } + for _, col := range c.CollectionMap { + assert.Contains(t, tt.expectedCollectionPaths, col.FullPath().String()) + } + assert.Equal(t, tt.expectedMetadataPaths, outputFolderMap) assert.Equal(t, tt.expectedExcludes, excludes) }) @@ -1080,19 +1111,6 @@ func (suite *OneDriveCollectionsSuite) TestGet() { ) require.NoError(suite.T(), err, "making metadata path") - rootFolderPath := expectedPathAsSlice( - suite.T(), - tenant, - user, - testBaseDrivePath, - )[0] - folderPath := expectedPathAsSlice( - suite.T(), - tenant, - user, - testBaseDrivePath+"/folder", - )[0] - empty := "" next := "next" delta := "delta1" @@ -1108,7 +1126,21 @@ func (suite *OneDriveCollectionsSuite) TestGet() { drive2.SetId(&driveID2) drive2.SetName(&driveID2) - driveBasePath2 := "drive/driveID2/root:" + driveBasePath1 := fmt.Sprintf(rootDrivePattern, driveID1) + driveBasePath2 := fmt.Sprintf(rootDrivePattern, driveID2) + + rootFolderPath1 := expectedPathAsSlice( + suite.T(), + tenant, + user, + driveBasePath1, + )[0] + folderPath1 := expectedPathAsSlice( + suite.T(), + tenant, + user, + driveBasePath1+"/folder", + )[0] rootFolderPath2 := expectedPathAsSlice( suite.T(), @@ -1130,7 +1162,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() { 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 + expectedCollections map[string]map[data.CollectionState][]string expectedDeltaURLs map[string]string expectedFolderPaths map[string]map[string]string expectedDelList map[string]struct{} @@ -1143,14 +1175,15 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID1: { { items: []models.DriveItemable{ - delItem("file", testBaseDrivePath, true, false, false), + driveRootItem("root"), // will be present, not needed + delItem("file", driveBasePath1, "root", true, false, false), }, deltaLink: &delta, }, }, }, errCheck: assert.NoError, - expectedCollections: map[string][]string{}, + expectedCollections: map[string]map[data.CollectionState][]string{}, expectedDeltaURLs: map[string]string{ driveID1: delta, }, @@ -1170,20 +1203,21 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID1: { { items: []models.DriveItemable{ - driveItem("file", "file", testBaseDrivePath, true, false, false), + driveRootItem("root"), + driveItem("file", "file", driveBasePath1, "root", true, false, false), }, deltaLink: &delta, }, }, }, errCheck: assert.NoError, - expectedCollections: map[string][]string{ + expectedCollections: map[string]map[data.CollectionState][]string{ expectedPathAsSlice( suite.T(), tenant, user, - testBaseDrivePath, - )[0]: {"file"}, + driveBasePath1, + )[0]: {data.NewState: {"file"}}, }, expectedDeltaURLs: map[string]string{ driveID1: delta, @@ -1202,24 +1236,25 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID1: { { items: []models.DriveItemable{ - driveItem("folder", "folder", testBaseDrivePath, false, true, false), - driveItem("file", "file", testBaseDrivePath+"/folder", true, false, false), + driveRootItem("root"), + driveItem("folder", "folder", driveBasePath1, "root", false, true, false), + driveItem("file", "file", driveBasePath1+"/folder", "folder", true, false, false), }, deltaLink: &delta, }, }, }, errCheck: assert.NoError, - expectedCollections: map[string][]string{ - folderPath: {"file"}, - rootFolderPath: {"folder"}, + expectedCollections: map[string]map[data.CollectionState][]string{ + folderPath1: {data.NewState: {"file"}}, + rootFolderPath1: {data.NewState: {"folder"}}, }, expectedDeltaURLs: map[string]string{ driveID1: delta, }, expectedFolderPaths: map[string]map[string]string{ driveID1: { - "folder": folderPath, + "folder": folderPath1, }, }, expectedDelList: map[string]struct{}{}, @@ -1231,17 +1266,18 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID1: { { items: []models.DriveItemable{ - driveItem("folder", "folder", testBaseDrivePath, false, true, false), - driveItem("file", "file", testBaseDrivePath+"/folder", true, false, false), + driveRootItem("root"), + driveItem("folder", "folder", driveBasePath1, "root", false, true, false), + driveItem("file", "file", driveBasePath1+"/folder", "folder", true, false, false), }, deltaLink: &empty, }, }, }, errCheck: assert.NoError, - expectedCollections: map[string][]string{ - folderPath: {"file"}, - rootFolderPath: {"folder"}, + expectedCollections: map[string]map[data.CollectionState][]string{ + folderPath1: {data.NewState: {"file"}}, + rootFolderPath1: {data.NewState: {"folder"}}, }, expectedDeltaURLs: map[string]string{}, expectedFolderPaths: map[string]map[string]string{}, @@ -1254,31 +1290,33 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID1: { { items: []models.DriveItemable{ - driveItem("folder", "folder", testBaseDrivePath, false, true, false), - driveItem("file", "file", testBaseDrivePath+"/folder", true, false, false), + driveRootItem("root"), + driveItem("folder", "folder", driveBasePath1, "root", false, true, false), + driveItem("file", "file", driveBasePath1+"/folder", "folder", true, false, false), }, nextLink: &next, }, { items: []models.DriveItemable{ - driveItem("folder", "folder", testBaseDrivePath, false, true, false), - driveItem("file2", "file2", testBaseDrivePath+"/folder", true, false, false), + driveRootItem("root"), + driveItem("folder", "folder", driveBasePath1, "root", false, true, false), + driveItem("file2", "file2", driveBasePath1+"/folder", "folder", true, false, false), }, deltaLink: &delta, }, }, }, errCheck: assert.NoError, - expectedCollections: map[string][]string{ - folderPath: {"file", "file2"}, - rootFolderPath: {"folder"}, + expectedCollections: map[string]map[data.CollectionState][]string{ + folderPath1: {data.NewState: {"file", "file2"}}, + rootFolderPath1: {data.NewState: {"folder"}}, }, expectedDeltaURLs: map[string]string{ driveID1: delta, }, expectedFolderPaths: map[string]map[string]string{ driveID1: { - "folder": folderPath, + "folder": folderPath1, }, }, expectedDelList: map[string]struct{}{}, @@ -1293,8 +1331,9 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID1: { { items: []models.DriveItemable{ - driveItem("folder", "folder", testBaseDrivePath, false, true, false), - driveItem("file", "file", testBaseDrivePath+"/folder", true, false, false), + driveRootItem("root"), + driveItem("folder", "folder", driveBasePath1, "root", false, true, false), + driveItem("file", "file", driveBasePath1+"/folder", "folder", true, false, false), }, deltaLink: &delta, }, @@ -1302,19 +1341,20 @@ func (suite *OneDriveCollectionsSuite) TestGet() { driveID2: { { items: []models.DriveItemable{ - driveItem("folder", "folder", driveBasePath2, false, true, false), - driveItem("file", "file", driveBasePath2+"/folder", true, false, false), + driveRootItem("root"), + driveItem("folder", "folder", driveBasePath2, "root", false, true, false), + driveItem("file", "file", driveBasePath2+"/folder", "folder", true, false, false), }, deltaLink: &delta2, }, }, }, errCheck: assert.NoError, - expectedCollections: map[string][]string{ - folderPath: {"file"}, - folderPath2: {"file"}, - rootFolderPath: {"folder"}, - rootFolderPath2: {"folder"}, + expectedCollections: map[string]map[data.CollectionState][]string{ + folderPath1: {data.NewState: {"file"}}, + folderPath2: {data.NewState: {"file"}}, + rootFolderPath1: {data.NewState: {"folder"}}, + rootFolderPath2: {data.NewState: {"folder"}}, }, expectedDeltaURLs: map[string]string{ driveID1: delta, @@ -1322,7 +1362,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() { }, expectedFolderPaths: map[string]map[string]string{ driveID1: { - "folder": folderPath, + "folder": folderPath1, }, driveID2: { "folder": folderPath2, @@ -1356,20 +1396,21 @@ func (suite *OneDriveCollectionsSuite) TestGet() { }, { items: []models.DriveItemable{ - driveItem("file", "file", testBaseDrivePath, true, false, false), + driveRootItem("root"), + driveItem("file", "file", driveBasePath1, "root", true, false, false), }, deltaLink: &delta, }, }, }, errCheck: assert.NoError, - expectedCollections: map[string][]string{ + expectedCollections: map[string]map[data.CollectionState][]string{ expectedPathAsSlice( suite.T(), tenant, user, - testBaseDrivePath, - )[0]: {"file"}, + driveBasePath1, + )[0]: {data.NewState: {"file"}}, }, expectedDeltaURLs: map[string]string{ driveID1: delta, @@ -1383,7 +1424,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() { doNotMergeItems: true, }, { - name: "OneDrive_MultipleCollections_DeltaError", + name: "OneDrive_TwoItemPage_DeltaError", drives: []models.Driveable{drive1}, items: map[string][]deltaPagerResult{ driveID1: { @@ -1392,85 +1433,87 @@ func (suite *OneDriveCollectionsSuite) TestGet() { }, { items: []models.DriveItemable{ - driveItem("file", "file", testBaseDrivePath, true, false, false), + driveRootItem("root"), + driveItem("file", "file", driveBasePath1, "root", true, false, false), }, nextLink: &next, }, { items: []models.DriveItemable{ - driveItem("file", "file", testBaseDrivePath+"/folder", true, false, false), + driveRootItem("root"), + driveItem("folder", "folder", driveBasePath1, "root", false, true, false), + driveItem("file", "file", driveBasePath1+"/folder", "folder", true, false, false), }, deltaLink: &delta, }, }, }, errCheck: assert.NoError, - expectedCollections: map[string][]string{ + expectedCollections: map[string]map[data.CollectionState][]string{ expectedPathAsSlice( suite.T(), tenant, user, - testBaseDrivePath, - )[0]: {"file"}, + driveBasePath1, + )[0]: {data.NewState: {"file", "folder"}}, expectedPathAsSlice( suite.T(), tenant, user, - testBaseDrivePath+"/folder", - )[0]: {"file"}, + driveBasePath1+"/folder", + )[0]: {data.NewState: {"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: {}, + driveID1: {"folder": folderPath1}, }, expectedDelList: map[string]struct{}{}, doNotMergeItems: true, }, { - name: "OneDrive_MultipleCollections_NoDeltaError", + name: "OneDrive_TwoItemPage_NoDeltaError", drives: []models.Driveable{drive1}, items: map[string][]deltaPagerResult{ driveID1: { { items: []models.DriveItemable{ - driveItem("file", "file", testBaseDrivePath, true, false, false), + driveRootItem("root"), + driveItem("file", "file", driveBasePath1, "root", true, false, false), }, nextLink: &next, }, { items: []models.DriveItemable{ - driveItem("file", "file", testBaseDrivePath+"/folder", true, false, false), + driveRootItem("root"), + driveItem("folder", "folder", driveBasePath1, "root", false, true, false), + driveItem("file", "file", driveBasePath1+"/folder", "folder", true, false, false), }, deltaLink: &delta, }, }, }, errCheck: assert.NoError, - expectedCollections: map[string][]string{ + expectedCollections: map[string]map[data.CollectionState][]string{ expectedPathAsSlice( suite.T(), tenant, user, - testBaseDrivePath, - )[0]: {"file"}, + driveBasePath1, + )[0]: {data.NewState: {"file", "folder"}}, expectedPathAsSlice( suite.T(), tenant, user, - testBaseDrivePath+"/folder", - )[0]: {"file"}, + driveBasePath1+"/folder", + )[0]: {data.NewState: {"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: {}, + driveID1: {"folder": folderPath1}, }, expectedDelList: map[string]struct{}{}, doNotMergeItems: false, @@ -1555,7 +1598,7 @@ func (suite *OneDriveCollectionsSuite) TestGet() { itemIDs = append(itemIDs, id) } - assert.ElementsMatch(t, test.expectedCollections[folderPath], itemIDs) + assert.ElementsMatch(t, test.expectedCollections[folderPath][baseCol.State()], itemIDs) assert.Equal(t, test.doNotMergeItems, baseCol.DoNotMergeItems(), "DoNotMergeItems") } @@ -1568,6 +1611,7 @@ func driveItem( id string, name string, parentPath string, + parentID string, isFile, isFolder, isPackage bool, ) models.DriveItemable { item := models.NewDriveItem() @@ -1576,6 +1620,7 @@ func driveItem( parentReference := models.NewItemReference() parentReference.SetPath(&parentPath) + parentReference.SetId(&parentID) item.SetParentReference(parentReference) switch { @@ -1590,11 +1635,22 @@ func driveItem( return item } +func driveRootItem(id string) models.DriveItemable { + name := "root" + item := models.NewDriveItem() + item.SetName(&name) + item.SetId(&id) + item.SetRoot(models.NewRoot()) + + return item +} + // delItem creates a DriveItemable that is marked as deleted. path must be set // to the base drive path. func delItem( id string, parentPath string, + parentID string, isFile, isFolder, isPackage bool, ) models.DriveItemable { item := models.NewDriveItem() @@ -1603,6 +1659,7 @@ func delItem( parentReference := models.NewItemReference() parentReference.SetPath(&parentPath) + parentReference.SetId(&parentID) item.SetParentReference(parentReference) switch { diff --git a/src/internal/connector/sharepoint/data_collections_test.go b/src/internal/connector/sharepoint/data_collections_test.go index 623b5c2e7..75710bc24 100644 --- a/src/internal/connector/sharepoint/data_collections_test.go +++ b/src/internal/connector/sharepoint/data_collections_test.go @@ -20,7 +20,7 @@ import ( // --------------------------------------------------------------------------- const ( - testBaseDrivePath = "drive/driveID1/root:" + testBaseDrivePath = "drives/driveID1/root:" ) type testFolderMatcher struct { @@ -60,6 +60,7 @@ func (suite *SharePointLibrariesSuite) TestUpdateCollections() { items []models.DriveItemable scope selectors.SharePointScope expect assert.ErrorAssertionFunc + expectedCollectionIDs []string expectedCollectionPaths []string expectedItemCount int expectedContainerCount int @@ -68,10 +69,12 @@ func (suite *SharePointLibrariesSuite) TestUpdateCollections() { { testCase: "Single File", items: []models.DriveItemable{ - driveItem("file", testBaseDrivePath, true), + driveRootItem("root"), + driveItem("file", testBaseDrivePath, "root", true), }, - scope: anyFolder, - expect: assert.NoError, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionIDs: []string{"root"}, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -101,26 +104,30 @@ func (suite *SharePointLibrariesSuite) TestUpdateCollections() { &MockGraphService{}, nil, control.Options{}) - err := c.UpdateCollections(ctx, "driveID", "General", test.items, paths, newPaths, excluded, true) + err := c.UpdateCollections(ctx, "driveID1", "General", test.items, paths, newPaths, excluded, true) test.expect(t, err) - assert.Equal(t, len(test.expectedCollectionPaths), len(c.CollectionMap), "collection paths") + assert.Equal(t, len(test.expectedCollectionIDs), len(c.CollectionMap), "collection paths") assert.Equal(t, test.expectedItemCount, c.NumItems, "item count") assert.Equal(t, test.expectedFileCount, c.NumFiles, "file count") assert.Equal(t, test.expectedContainerCount, c.NumContainers, "container count") - for _, collPath := range test.expectedCollectionPaths { + for _, collPath := range test.expectedCollectionIDs { assert.Contains(t, c.CollectionMap, collPath) } + for _, col := range c.CollectionMap { + assert.Contains(t, test.expectedCollectionPaths, col.FullPath().String()) + } }) } } -func driveItem(name string, path string, isFile bool) models.DriveItemable { +func driveItem(name, parentPath, parentID string, isFile bool) models.DriveItemable { item := models.NewDriveItem() item.SetName(&name) item.SetId(&name) parentReference := models.NewItemReference() - parentReference.SetPath(&path) + parentReference.SetPath(&parentPath) + parentReference.SetId(&parentID) item.SetParentReference(parentReference) if isFile { @@ -130,6 +137,16 @@ func driveItem(name string, path string, isFile bool) models.DriveItemable { return item } +func driveRootItem(id string) models.DriveItemable { + name := "root" + item := models.NewDriveItem() + item.SetName(&name) + item.SetId(&id) + item.SetRoot(models.NewRoot()) + + return item +} + type SharePointPagesSuite struct { suite.Suite }