From df086bd6f4385393f299c9f722faa00b6022424b Mon Sep 17 00:00:00 2001 From: ashmrtn Date: Wed, 21 Dec 2022 15:32:47 -0800 Subject: [PATCH] Move OneDrive path helper func to path package (#1880) ## Description Later code in details package will need access to this function so it can update item details. However, importing the onedrive package in the details package leads to an import cycle. Moving it to the path package breaks the cycle while still allowing the code to be accessed. ## 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 - [ ] :robot: Test - [ ] :computer: CI/Deployment - [x] :hamster: Trivial/Minor ## Issue(s) * #1800 ## Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/connector/onedrive/collection.go | 2 +- .../connector/onedrive/collection_test.go | 3 +- .../connector/onedrive/collections.go | 12 +----- src/internal/connector/onedrive/restore.go | 30 ++------------ src/pkg/path/onedrive.go | 40 +++++++++++++++++++ .../path/onedrive_test.go} | 18 ++++----- 6 files changed, 57 insertions(+), 48 deletions(-) create mode 100644 src/pkg/path/onedrive.go rename src/{internal/connector/onedrive/restore_test.go => pkg/path/onedrive_test.go} (71%) diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go index 930c2c528..638876b66 100644 --- a/src/internal/connector/onedrive/collection.go +++ b/src/internal/connector/onedrive/collection.go @@ -182,7 +182,7 @@ func (oc *Collection) populateItems(ctx context.Context) { // Retrieve the OneDrive folder path to set later in // `details.OneDriveInfo` - parentPathString, err := getDriveFolderPath(oc.folderPath) + parentPathString, err := path.GetDriveFolderPath(oc.folderPath) if err != nil { oc.reportAsCompleted(ctx, 0, 0, err) return diff --git a/src/internal/connector/onedrive/collection_test.go b/src/internal/connector/onedrive/collection_test.go index 0abe52448..a5c815f14 100644 --- a/src/internal/connector/onedrive/collection_test.go +++ b/src/internal/connector/onedrive/collection_test.go @@ -18,6 +18,7 @@ import ( "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" + "github.com/alcionai/corso/src/pkg/path" ) type CollectionUnitTestSuite struct { @@ -102,7 +103,7 @@ func (suite *CollectionUnitTestSuite) TestCollection() { folderPath, err := GetCanonicalPath("drive/driveID1/root:/dir1/dir2/dir3", "tenant", "owner", test.source) require.NoError(t, err) - driveFolderPath, err := getDriveFolderPath(folderPath) + driveFolderPath, err := path.GetDriveFolderPath(folderPath) require.NoError(t, err) coll := NewCollection( diff --git a/src/internal/connector/onedrive/collections.go b/src/internal/connector/onedrive/collections.go index 7e4aa3a4e..f7e3d9290 100644 --- a/src/internal/connector/onedrive/collections.go +++ b/src/internal/connector/onedrive/collections.go @@ -189,19 +189,9 @@ func GetCanonicalPath(p, tenant, resourceOwner string, source driveSource) (path return result, nil } -// Returns the path to the folder within the drive (i.e. under `root:`) -func getDriveFolderPath(p path.Path) (string, error) { - drivePath, err := toOneDrivePath(p) - if err != nil { - return "", err - } - - return path.Builder{}.Append(drivePath.folders...).String(), nil -} - func includePath(ctx context.Context, m folderMatcher, folderPath path.Path) bool { // Check if the folder is allowed by the scope. - folderPathString, err := getDriveFolderPath(folderPath) + folderPathString, err := path.GetDriveFolderPath(folderPath) if err != nil { logger.Ctx(ctx).Error(err) return true diff --git a/src/internal/connector/onedrive/restore.go b/src/internal/connector/onedrive/restore.go index 74a9095a2..67cfac802 100644 --- a/src/internal/connector/onedrive/restore.go +++ b/src/internal/connector/onedrive/restore.go @@ -25,28 +25,6 @@ const ( copyBufferSize = 5 * 1024 * 1024 ) -// drivePath is used to represent path components -// of an item within the drive i.e. -// Given `drives/b!X_8Z2zuXpkKkXZsr7gThk9oJpuj0yXVGnK5_VjRRPK-q725SX_8ZQJgFDK8PlFxA/root:/Folder1/Folder2/file` -// -// driveID is `b!X_8Z2zuXpkKkXZsr7gThk9oJpuj0yXVGnK5_VjRRPK-q725SX_8ZQJgFDK8PlFxA` and -// folders[] is []{"Folder1", "Folder2"} -type drivePath struct { - driveID string - folders []string -} - -func toOneDrivePath(p path.Path) (*drivePath, error) { - folders := p.Folders() - - // Must be at least `drives//root:` - if len(folders) < 3 { - return nil, errors.Errorf("folder path doesn't match expected format for OneDrive items: %s", p.Folder()) - } - - return &drivePath{driveID: folders[1], folders: folders[3:]}, nil -} - // RestoreCollections will restore the specified data collections into OneDrive func RestoreCollections( ctx context.Context, @@ -107,7 +85,7 @@ func RestoreCollection( directory = dc.FullPath() ) - drivePath, err := toOneDrivePath(directory) + drivePath, err := path.ToOneDrivePath(directory) if err != nil { errUpdater(directory.String(), err) return metrics, false @@ -118,13 +96,13 @@ func RestoreCollection( // i.e. Restore into `/root://` restoreFolderElements := []string{restoreContainerName} - restoreFolderElements = append(restoreFolderElements, drivePath.folders...) + restoreFolderElements = append(restoreFolderElements, drivePath.Folders...) trace.Log(ctx, "gc:oneDrive:restoreCollection", directory.String()) logger.Ctx(ctx).Debugf("Restore target for %s is %v", dc.FullPath(), restoreFolderElements) // Create restore folders and get the folder ID of the folder the data stream will be restored in - restoreFolderID, err := createRestoreFolders(ctx, service, drivePath.driveID, restoreFolderElements) + restoreFolderID, err := createRestoreFolders(ctx, service, drivePath.DriveID, restoreFolderElements) if err != nil { errUpdater(directory.String(), errors.Wrapf(err, "failed to create folders %v", restoreFolderElements)) return metrics, false @@ -150,7 +128,7 @@ func RestoreCollection( itemInfo, err := restoreItem(ctx, service, itemData, - drivePath.driveID, + drivePath.DriveID, restoreFolderID, copyBuffer, source) diff --git a/src/pkg/path/onedrive.go b/src/pkg/path/onedrive.go new file mode 100644 index 000000000..cf960933f --- /dev/null +++ b/src/pkg/path/onedrive.go @@ -0,0 +1,40 @@ +package path + +import ( + "github.com/pkg/errors" +) + +// drivePath is used to represent path components +// of an item within the drive i.e. +// Given `drives/b!X_8Z2zuXpkKkXZsr7gThk9oJpuj0yXVGnK5_VjRRPK-q725SX_8ZQJgFDK8PlFxA/root:/Folder1/Folder2/file` +// +// driveID is `b!X_8Z2zuXpkKkXZsr7gThk9oJpuj0yXVGnK5_VjRRPK-q725SX_8ZQJgFDK8PlFxA` and +// folders[] is []{"Folder1", "Folder2"} +type DrivePath struct { + DriveID string + Folders []string +} + +func ToOneDrivePath(p Path) (*DrivePath, error) { + folders := p.Folders() + + // Must be at least `drives//root:` + if len(folders) < 3 { + return nil, errors.Errorf( + "folder path doesn't match expected format for OneDrive items: %s", + p.Folder(), + ) + } + + return &DrivePath{DriveID: folders[1], Folders: folders[3:]}, nil +} + +// Returns the path to the folder within the drive (i.e. under `root:`) +func GetDriveFolderPath(p Path) (string, error) { + drivePath, err := ToOneDrivePath(p) + if err != nil { + return "", err + } + + return Builder{}.Append(drivePath.Folders...).String(), nil +} diff --git a/src/internal/connector/onedrive/restore_test.go b/src/pkg/path/onedrive_test.go similarity index 71% rename from src/internal/connector/onedrive/restore_test.go rename to src/pkg/path/onedrive_test.go index 2c5bff1f8..d4638256f 100644 --- a/src/internal/connector/onedrive/restore_test.go +++ b/src/pkg/path/onedrive_test.go @@ -1,4 +1,4 @@ -package onedrive +package path_test import ( "testing" @@ -10,19 +10,19 @@ import ( "github.com/alcionai/corso/src/pkg/path" ) -type OneDriveRestoreSuite struct { +type OneDrivePathSuite struct { suite.Suite } -func TestOneDriveRestoreSuite(t *testing.T) { - suite.Run(t, new(OneDriveRestoreSuite)) +func TestOneDrivePathSuite(t *testing.T) { + suite.Run(t, new(OneDrivePathSuite)) } -func (suite *OneDriveRestoreSuite) Test_toOneDrivePath() { +func (suite *OneDrivePathSuite) Test_ToOneDrivePath() { tests := []struct { name string pathElements []string - expected *drivePath + expected *path.DrivePath errCheck assert.ErrorAssertionFunc }{ { @@ -33,13 +33,13 @@ func (suite *OneDriveRestoreSuite) Test_toOneDrivePath() { { name: "Root path", pathElements: []string{"drive", "driveID", "root:"}, - expected: &drivePath{driveID: "driveID", folders: []string{}}, + expected: &path.DrivePath{DriveID: "driveID", Folders: []string{}}, errCheck: assert.NoError, }, { name: "Deeper path", pathElements: []string{"drive", "driveID", "root:", "folder1", "folder2"}, - expected: &drivePath{driveID: "driveID", folders: []string{"folder1", "folder2"}}, + expected: &path.DrivePath{DriveID: "driveID", Folders: []string{"folder1", "folder2"}}, errCheck: assert.NoError, }, } @@ -48,7 +48,7 @@ func (suite *OneDriveRestoreSuite) Test_toOneDrivePath() { p, err := path.Builder{}.Append(tt.pathElements...).ToDataLayerOneDrivePath("tenant", "user", false) require.NoError(suite.T(), err) - got, err := toOneDrivePath(p) + got, err := path.ToOneDrivePath(p) tt.errCheck(t, err) if err != nil { return