From 6651305d4fb2f0edcca6300ceaa64fdf5f107a44 Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Fri, 20 Jan 2023 09:02:23 -0800 Subject: [PATCH] Update OneDrive metadata with folder deletions (#2192) ## Description Track folder and package deletions in OneDrive metadata for incremental backups. Add test for deletions ## 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 - [x] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup ## Issue(s) * #2120 ## Test Plan - [x] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- .../connector/onedrive/collections.go | 18 +++- .../connector/onedrive/collections_test.go | 88 ++++++++++++++++--- 2 files changed, 89 insertions(+), 17 deletions(-) diff --git a/src/internal/connector/onedrive/collections.go b/src/internal/connector/onedrive/collections.go index 6a59104f1..73e1b1ba8 100644 --- a/src/internal/connector/onedrive/collections.go +++ b/src/internal/connector/onedrive/collections.go @@ -197,16 +197,26 @@ func (c *Collections) UpdateCollections( switch { case item.GetFolder() != nil, item.GetPackage() != nil: - // Eventually, deletions of folders will be handled here so we may as well - // start off by saving the path.Path of the item instead of just the - // OneDrive parentRef or such. + if item.GetDeleted() != nil { + // Nested folders also return deleted delta results so we don't have to + // worry about doing a prefix search in the map to remove the subtree of + // the deleted folder/package. + delete(paths, *item.GetId()) + + // TODO(ashmrtn): Create a collection with state Deleted. + + break + } + + // Deletions of folders are handled in this case so we may as well start + // off by saving the path.Path of the item instead of just the OneDrive + // parentRef or such. folderPath, err := collectionPath.Append(*item.GetName(), false) if err != nil { logger.Ctx(ctx).Errorw("failed building collection path", "error", err) return err } - // TODO(ashmrtn): Handle deletions by removing this entry from the map. // TODO(ashmrtn): Handle moves by setting the collection state if the // collection doesn't already exist/have that state. paths[*item.GetId()] = folderPath.String() diff --git a/src/internal/connector/onedrive/collections_test.go b/src/internal/connector/onedrive/collections_test.go index 5a0775edc..a8520d52d 100644 --- a/src/internal/connector/onedrive/collections_test.go +++ b/src/internal/connector/onedrive/collections_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "golang.org/x/exp/maps" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/control" @@ -96,6 +97,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { tests := []struct { testCase string items []models.DriveItemable + inputFolderMap map[string]string scope selectors.OneDriveScope expect assert.ErrorAssertionFunc expectedCollectionPaths []string @@ -109,6 +111,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { items: []models.DriveItemable{ driveItem("item", "item", testBaseDrivePath, false, false, false), }, + inputFolderMap: map[string]string{}, scope: anyFolder, expect: assert.Error, expectedMetadataPaths: map[string]string{}, @@ -118,8 +121,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { items: []models.DriveItemable{ driveItem("file", "file", testBaseDrivePath, true, false, false), }, - scope: anyFolder, - expect: assert.NoError, + inputFolderMap: map[string]string{}, + scope: anyFolder, + expect: assert.NoError, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -137,6 +141,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { items: []models.DriveItemable{ driveItem("folder", "folder", testBaseDrivePath, false, true, false), }, + inputFolderMap: map[string]string{}, scope: anyFolder, expect: assert.NoError, expectedCollectionPaths: []string{}, @@ -154,6 +159,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { items: []models.DriveItemable{ driveItem("package", "package", testBaseDrivePath, false, false, true), }, + inputFolderMap: map[string]string{}, scope: anyFolder, expect: assert.NoError, expectedCollectionPaths: []string{}, @@ -175,8 +181,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { driveItem("fileInFolder", "fileInFolder", testBaseDrivePath+folder, true, false, false), driveItem("fileInPackage", "fileInPackage", testBaseDrivePath+pkg, true, false, false), }, - scope: anyFolder, - expect: assert.NoError, + inputFolderMap: map[string]string{}, + scope: anyFolder, + expect: assert.NoError, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -215,8 +222,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { driveItem("fileInFolder2", "fileInFolder2", testBaseDrivePath+folderSub+folder, true, false, false), driveItem("fileInFolderPackage", "fileInPackage", testBaseDrivePath+pkg, true, false, false), }, - scope: (&selectors.OneDriveBackup{}).Folders([]string{"folder"})[0], - expect: assert.NoError, + inputFolderMap: map[string]string{}, + scope: (&selectors.OneDriveBackup{}).Folders([]string{"folder"})[0], + expect: assert.NoError, expectedCollectionPaths: append( expectedPathAsSlice( suite.T(), @@ -257,12 +265,13 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { driveItem("fileInRoot", "fileInRoot", testBaseDrivePath, true, false, false), driveItem("folder", "folder", testBaseDrivePath, false, true, false), driveItem("subfolder", "subfolder", testBaseDrivePath+folder, false, true, false), - driveItem("folder", "folder", testBaseDrivePath+folderSub, 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), }, + inputFolderMap: map[string]string{}, scope: (&selectors.OneDriveBackup{}). Folders([]string{"/folder/subfolder"}, selectors.PrefixMatch())[0], expect: assert.NoError, @@ -276,7 +285,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { expectedFileCount: 1, expectedContainerCount: 1, expectedMetadataPaths: map[string]string{ - "folder": expectedPathAsSlice( + "folder2": expectedPathAsSlice( suite.T(), tenant, user, @@ -295,8 +304,9 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { driveItem("fileInSubfolder", "fileInSubfolder", testBaseDrivePath+folderSub, true, false, false), driveItem("fileInPackage", "fileInPackage", testBaseDrivePath+pkg, true, false, false), }, - 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, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, @@ -309,6 +319,34 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { // No child folders for subfolder so nothing here. expectedMetadataPaths: map[string]string{}, }, + { + testCase: "deleted folder and package", + items: []models.DriveItemable{ + delItem("folder", testBaseDrivePath, false, true, false), + delItem("package", testBaseDrivePath, false, false, true), + }, + inputFolderMap: map[string]string{ + "folder": expectedPathAsSlice( + suite.T(), + tenant, + user, + testBaseDrivePath+"/folder", + )[0], + "package": expectedPathAsSlice( + suite.T(), + tenant, + user, + testBaseDrivePath+"/package", + )[0], + }, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionPaths: []string{}, + expectedItemCount: 0, + expectedFileCount: 0, + expectedContainerCount: 0, + expectedMetadataPaths: map[string]string{}, + }, } for _, tt := range tests { @@ -316,7 +354,8 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { ctx, flush := tester.NewContext() defer flush() - paths := map[string]string{} + outputFolderMap := map[string]string{} + maps.Copy(outputFolderMap, tt.inputFolderMap) c := NewCollections( tenant, user, @@ -326,7 +365,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { nil, control.Options{}) - err := c.UpdateCollections(ctx, "driveID", "General", tt.items, paths) + err := c.UpdateCollections(ctx, "driveID", "General", tt.items, outputFolderMap) tt.expect(t, err) assert.Equal(t, len(tt.expectedCollectionPaths), len(c.CollectionMap), "collection paths") assert.Equal(t, tt.expectedItemCount, c.NumItems, "item count") @@ -336,7 +375,7 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { assert.Contains(t, c.CollectionMap, collPath) } - assert.Equal(t, tt.expectedMetadataPaths, paths) + assert.Equal(t, tt.expectedMetadataPaths, outputFolderMap) }) } } @@ -361,3 +400,26 @@ func driveItem(id string, name string, path string, isFile, isFolder, isPackage return item } + +// delItem creates a DriveItemable that is marked as deleted. path must be set +// to the base drive path. +func delItem(id string, path string, isFile, isFolder, isPackage bool) models.DriveItemable { + item := models.NewDriveItem() + item.SetId(&id) + item.SetDeleted(models.NewDeleted()) + + parentReference := models.NewItemReference() + parentReference.SetPath(&path) + item.SetParentReference(parentReference) + + switch { + case isFile: + item.SetFile(models.NewFile()) + case isFolder: + item.SetFolder(models.NewFolder()) + case isPackage: + item.SetPackage(models.NewPackage_escaped()) + } + + return item +}