From 1cbb34e403d278aa1c8d7676c177690d98e34ce0 Mon Sep 17 00:00:00 2001 From: Vaibhav Kamra Date: Thu, 13 Oct 2022 10:24:42 -0700 Subject: [PATCH] Scope OneDrive backup - for testing (#1109) ## Description ## Type of change - [x] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :hamster: Trivial/Minor ## Issue(s) * closes #1108 * closes #1137 ## Test Plan - [x] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/connector/graph_connector.go | 1 + .../connector/onedrive/collection_test.go | 2 +- .../connector/onedrive/collections.go | 89 ++++++------ .../connector/onedrive/collections_test.go | 130 ++++++++++++------ 4 files changed, 130 insertions(+), 92 deletions(-) diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index d1dbf7360..aae2115d6 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -522,6 +522,7 @@ func (gc *GraphConnector) OneDriveDataCollections( odcs, err := onedrive.NewCollections( gc.credentials.TenantID, user, + scope, &gc.graphService, gc.UpdateStatus, ).Get(ctx) diff --git a/src/internal/connector/onedrive/collection_test.go b/src/internal/connector/onedrive/collection_test.go index dd2c8b3ee..27183e1d6 100644 --- a/src/internal/connector/onedrive/collection_test.go +++ b/src/internal/connector/onedrive/collection_test.go @@ -117,7 +117,7 @@ func (suite *OneDriveCollectionSuite) TestOneDriveCollectionReadError() { wg := sync.WaitGroup{} wg.Add(1) - folderPath, err := getCanonicalPath("folderPath", "a-tenant", "a-user") + folderPath, err := getCanonicalPath("drive/driveID1/root:/folderPath", "a-tenant", "a-user") require.NoError(t, err) coll := NewCollection(folderPath, "fakeDriveID", suite, suite.testStatusUpdater(&wg, &collStatus)) diff --git a/src/internal/connector/onedrive/collections.go b/src/internal/connector/onedrive/collections.go index 246a82ec0..33b6b7a88 100644 --- a/src/internal/connector/onedrive/collections.go +++ b/src/internal/connector/onedrive/collections.go @@ -2,7 +2,6 @@ package onedrive import ( "context" - stdpath "path" "strings" "github.com/microsoftgraph/msgraph-sdk-go/models" @@ -11,7 +10,9 @@ import ( "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" + "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" + "github.com/alcionai/corso/src/pkg/selectors" ) // Collections is used to retrieve OneDrive data for a @@ -19,28 +20,30 @@ import ( type Collections struct { tenant string user string + scope selectors.OneDriveScope // collectionMap allows lookup of the data.Collection // for a OneDrive folder collectionMap map[string]data.Collection service graph.Service statusUpdater support.StatusUpdater - // Track stats from drive enumeration - numItems int - numDirs int - numFiles int - numPackages int + // Track stats from drive enumeration. Represents the items backed up. + numItems int + numFiles int + numContainers int } func NewCollections( tenant string, user string, + scope selectors.OneDriveScope, service graph.Service, statusUpdater support.StatusUpdater, ) *Collections { return &Collections{ tenant: tenant, user: user, + scope: scope, collectionMap: map[string]data.Collection{}, service: service, statusUpdater: statusUpdater, @@ -96,11 +99,6 @@ func getDriveFolderPath(p path.Path) (string, error) { // A new collection is created for every OneDrive folder (or package) func (c *Collections) updateCollections(ctx context.Context, driveID string, items []models.DriveItemable) error { for _, item := range items { - err := c.stats(item) - if err != nil { - return err - } - if item.GetRoot() != nil { // Skip the root item continue @@ -120,43 +118,38 @@ func (c *Collections) updateCollections(ctx context.Context, driveID string, ite return err } - if _, found := c.collectionMap[collectionPath.String()]; !found { - c.collectionMap[collectionPath.String()] = NewCollection( - collectionPath, - driveID, - c.service, - c.statusUpdater, - ) + // Skip items that don't match the folder selectors we were given. + if !includePath(ctx, c.scope, collectionPath) { + logger.Ctx(ctx).Infof("Skipping path %s", collectionPath.String()) + continue } switch { case item.GetFolder() != nil, item.GetPackage() != nil: - // For folders and packages we also create a collection to represent those + // Leave this here so we don't fall into the default case. // TODO: This is where we might create a "special file" to represent these in the backup repository // e.g. a ".folderMetadataFile" - itemPath, err := getCanonicalPath( - stdpath.Join( - *item.GetParentReference().GetPath(), - *item.GetName(), - ), - c.tenant, - c.user, - ) - if err != nil { - return err - } - if _, found := c.collectionMap[itemPath.String()]; !found { - c.collectionMap[itemPath.String()] = NewCollection( - itemPath, + case item.GetFile() != nil: + col, found := c.collectionMap[collectionPath.String()] + if !found { + col = NewCollection( + collectionPath, driveID, c.service, c.statusUpdater, ) + + c.collectionMap[collectionPath.String()] = col + c.numContainers++ + c.numItems++ } - case item.GetFile() != nil: - collection := c.collectionMap[collectionPath.String()].(*Collection) + + collection := col.(*Collection) collection.Add(*item.GetId()) + c.numFiles++ + c.numItems++ + default: return errors.Errorf("item type not supported. item name : %s", *item.GetName()) } @@ -165,19 +158,21 @@ func (c *Collections) updateCollections(ctx context.Context, driveID string, ite return nil } -func (c *Collections) stats(item models.DriveItemable) error { - switch { - case item.GetFolder() != nil: - c.numDirs++ - case item.GetPackage() != nil: - c.numPackages++ - case item.GetFile() != nil: - c.numFiles++ - default: - return errors.Errorf("item type not supported. item name : %s", *item.GetName()) +func includePath(ctx context.Context, scope selectors.OneDriveScope, folderPath path.Path) bool { + // Check if the folder is allowed by the scope. + folderPathString, err := getDriveFolderPath(folderPath) + if err != nil { + logger.Ctx(ctx).Error(err) + return true } - c.numItems++ + // Hack for the edge case where we're looking at the root folder and can + // select any folder. Right now the root folder has an empty folder path. + if len(folderPathString) == 0 && scope.IsAny(selectors.OneDriveFolder) { + return true + } - return nil + logger.Ctx(ctx).Infof("Checking path %q", folderPathString) + + return scope.Matches(selectors.OneDriveFolder, folderPathString) } diff --git a/src/internal/connector/onedrive/collections_test.go b/src/internal/connector/onedrive/collections_test.go index 6b3cbc312..299a63576 100644 --- a/src/internal/connector/onedrive/collections_test.go +++ b/src/internal/connector/onedrive/collections_test.go @@ -10,6 +10,11 @@ import ( "github.com/stretchr/testify/suite" "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/selectors" +) + +const ( + testBaseDrivePath = "drive/driveID1/root:" ) func expectedPathAsSlice(t *testing.T, tenant, user string, rest ...string) []string { @@ -34,94 +39,132 @@ func TestOneDriveCollectionsSuite(t *testing.T) { } func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { + anyFolder := (&selectors.OneDriveBackup{}).Folders(selectors.Any(), selectors.Any())[0] tenant := "tenant" user := "user" + tests := []struct { testCase string items []models.DriveItemable + scope selectors.OneDriveScope expect assert.ErrorAssertionFunc expectedCollectionPaths []string expectedItemCount int - expectedFolderCount int - expectedPackageCount int + expectedContainerCount int expectedFileCount int }{ { testCase: "Invalid item", items: []models.DriveItemable{ - driveItem("item", "/root", false, false, false), + driveItem("item", testBaseDrivePath, false, false, false), }, + scope: anyFolder, expect: assert.Error, }, { testCase: "Single File", items: []models.DriveItemable{ - driveItem("file", "/root", true, false, false), + driveItem("file", testBaseDrivePath, true, false, false), }, + scope: anyFolder, expect: assert.NoError, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, user, - "root", + testBaseDrivePath, ), - expectedItemCount: 1, - expectedFileCount: 1, + expectedItemCount: 2, + expectedFileCount: 1, + expectedContainerCount: 1, }, { testCase: "Single Folder", items: []models.DriveItemable{ - driveItem("folder", "/root", false, true, false), + driveItem("folder", testBaseDrivePath, false, true, false), }, - expect: assert.NoError, - expectedCollectionPaths: expectedPathAsSlice( - suite.T(), - tenant, - user, - "/root", - "/root/folder", - ), - expectedItemCount: 1, - expectedFolderCount: 1, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionPaths: []string{}, }, { testCase: "Single Package", items: []models.DriveItemable{ - driveItem("package", "/root", false, false, true), + driveItem("package", testBaseDrivePath, false, false, true), }, - expect: assert.NoError, - expectedCollectionPaths: expectedPathAsSlice( - suite.T(), - tenant, - user, - "/root", - "/root/package", - ), - expectedItemCount: 1, - expectedPackageCount: 1, + scope: anyFolder, + expect: assert.NoError, + expectedCollectionPaths: []string{}, }, { testCase: "1 root file, 1 folder, 1 package, 2 files, 3 collections", items: []models.DriveItemable{ - driveItem("fileInRoot", "/root", true, false, false), - driveItem("folder", "/root", false, true, false), - driveItem("package", "/root", false, false, true), - driveItem("fileInFolder", "/root/folder", true, false, false), - driveItem("fileInPackage", "/root/package", true, false, false), + driveItem("fileInRoot", testBaseDrivePath, true, false, false), + driveItem("folder", testBaseDrivePath, false, true, false), + driveItem("package", testBaseDrivePath, false, false, true), + driveItem("fileInFolder", testBaseDrivePath+"/folder", true, false, false), + driveItem("fileInPackage", testBaseDrivePath+"/package", true, false, false), }, + scope: anyFolder, expect: assert.NoError, expectedCollectionPaths: expectedPathAsSlice( suite.T(), tenant, user, - "/root", - "/root/folder", - "/root/package", + testBaseDrivePath, + testBaseDrivePath+"/folder", + testBaseDrivePath+"/package", ), - expectedItemCount: 5, - expectedFileCount: 3, - expectedFolderCount: 1, - expectedPackageCount: 1, + expectedItemCount: 6, + expectedFileCount: 3, + expectedContainerCount: 3, + }, + { + testCase: "match folder selector", + items: []models.DriveItemable{ + driveItem("fileInRoot", testBaseDrivePath, true, false, false), + driveItem("folder", testBaseDrivePath, false, true, false), + driveItem("subfolder", testBaseDrivePath+"/folder", false, true, false), + driveItem("folder", testBaseDrivePath+"/folder/subfolder", false, true, false), + driveItem("package", testBaseDrivePath, false, false, true), + driveItem("fileInFolder", testBaseDrivePath+"/folder", true, false, false), + driveItem("fileInFolder2", testBaseDrivePath+"/folder/subfolder/folder", true, false, false), + driveItem("fileInPackage", testBaseDrivePath+"/package", true, false, false), + }, + scope: (&selectors.OneDriveBackup{}).Folders(selectors.Any(), []string{"folder"})[0], + expect: assert.NoError, + expectedCollectionPaths: expectedPathAsSlice( + suite.T(), + tenant, + user, + testBaseDrivePath+"/folder", + ), + expectedItemCount: 2, + expectedFileCount: 1, + expectedContainerCount: 1, + }, + { + testCase: "match subfolder selector", + items: []models.DriveItemable{ + driveItem("fileInRoot", testBaseDrivePath, true, false, false), + driveItem("folder", testBaseDrivePath, false, true, false), + driveItem("subfolder", testBaseDrivePath+"/folder", false, true, false), + driveItem("package", testBaseDrivePath, false, false, true), + driveItem("fileInFolder", testBaseDrivePath+"/folder", true, false, false), + driveItem("fileInSubfolder", testBaseDrivePath+"/folder/subfolder", true, false, false), + driveItem("fileInPackage", testBaseDrivePath+"/package", true, false, false), + }, + scope: (&selectors.OneDriveBackup{}).Folders(selectors.Any(), []string{"folder/subfolder"})[0], + expect: assert.NoError, + expectedCollectionPaths: expectedPathAsSlice( + suite.T(), + tenant, + user, + testBaseDrivePath+"/folder/subfolder", + ), + expectedItemCount: 2, + expectedFileCount: 1, + expectedContainerCount: 1, }, } @@ -130,14 +173,13 @@ func (suite *OneDriveCollectionsSuite) TestUpdateCollections() { ctx, flush := tester.NewContext() defer flush() - c := NewCollections(tenant, user, &MockGraphService{}, nil) + c := NewCollections(tenant, user, tt.scope, &MockGraphService{}, nil) err := c.updateCollections(ctx, "driveID", tt.items) tt.expect(t, err) assert.Equal(t, len(tt.expectedCollectionPaths), len(c.collectionMap)) assert.Equal(t, tt.expectedItemCount, c.numItems) assert.Equal(t, tt.expectedFileCount, c.numFiles) - assert.Equal(t, tt.expectedFolderCount, c.numDirs) - assert.Equal(t, tt.expectedPackageCount, c.numPackages) + assert.Equal(t, tt.expectedContainerCount, c.numContainers) for _, collPath := range tt.expectedCollectionPaths { assert.Contains(t, c.collectionMap, collPath) }