From 71d2c3deb1fd42cbeeee8d3d2dae181303b6768b Mon Sep 17 00:00:00 2001 From: Abin Simon Date: Fri, 10 Mar 2023 11:14:23 +0530 Subject: [PATCH] Augment paths for OneDrive restore (#2718) This adds the missing .dirmeta files which are needed to make sure that permissions are being restored properly. By adding a dirmeta entry for each folder that is part of the items, we make sure that all folders get their own collection which in turn will help us properly restore permissions for all folders. **This also makes it so that meta items are not returned from older backups.** *Although the newer version can fetch permissions file using the Fetch API, we still have to populate the items as it cannot fetch items from previous directories which is needed to make sure we can restore permissions correctly.* --- #### 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 - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Issue(s) * fixes https://github.com/alcionai/corso/issues/2697 #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/connector/onedrive/restore.go | 67 ++++++++ .../connector/onedrive/restore_test.go | 158 ++++++++++++++++++ src/internal/operations/restore.go | 21 +-- src/pkg/backup/details/details.go | 6 +- src/pkg/repository/repository.go | 2 + 5 files changed, 238 insertions(+), 16 deletions(-) create mode 100644 src/internal/connector/onedrive/restore_test.go diff --git a/src/internal/connector/onedrive/restore.go b/src/internal/connector/onedrive/restore.go index ea34387b9..911f6fe1d 100644 --- a/src/internal/connector/onedrive/restore.go +++ b/src/internal/connector/onedrive/restore.go @@ -599,3 +599,70 @@ func getMetadata(metar io.ReadCloser) (Metadata, error) { return meta, nil } + +// Augment restore path to add extra files(meta) needed for restore as +// well as do any other ordering operations on the paths +func AugmentRestorePaths(backupVersion int, paths []path.Path) ([]path.Path, error) { + colPaths := map[string]path.Path{} + + for _, p := range paths { + for { + np, err := p.Dir() + if err != nil { + return nil, err + } + + onedrivePath, err := path.ToOneDrivePath(np) + if err != nil { + return nil, err + } + + if len(onedrivePath.Folders) == 0 { + break + } + + colPaths[np.String()] = np + p = np + } + } + + // Adds dirmeta files as we need to make sure collections for all + // directories involved are created and not just the final one. No + // need to add `.meta` files (metadata for files) as they will + // anyways be looked up automatically. + // TODO: Stop populating .dirmeta for newer versions once we can + // get files from parent directory via `Fetch` in a collection. + // As of now look up metadata for parent directories from a + // collection. + for _, p := range colPaths { + el := p.Elements() + if backupVersion >= version.OneDrive4DirIncludesPermissions { + mPath, err := p.Append(el[len(el)-1]+".dirmeta", true) + if err != nil { + return nil, err + } + + paths = append(paths, mPath) + } else if backupVersion >= version.OneDrive1DataAndMetaFiles { + pp, err := p.Dir() + if err != nil { + return nil, err + } + mPath, err := pp.Append(el[len(el)-1]+".dirmeta", true) + if err != nil { + return nil, err + } + paths = append(paths, mPath) + } + } + + // This sort is done primarily to order `.meta` files after `.data` + // files. This is only a necessity for OneDrive as we are storing + // metadata for files/folders in separate meta files and we the + // data to be restored before we can restore the metadata. + sort.Slice(paths, func(i, j int) bool { + return paths[i].String() < paths[j].String() + }) + + return paths, nil +} diff --git a/src/internal/connector/onedrive/restore_test.go b/src/internal/connector/onedrive/restore_test.go new file mode 100644 index 000000000..f83c9a122 --- /dev/null +++ b/src/internal/connector/onedrive/restore_test.go @@ -0,0 +1,158 @@ +package onedrive + +import ( + "testing" + + "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/internal/version" + "github.com/alcionai/corso/src/pkg/path" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +type RestoreUnitSuite struct { + tester.Suite +} + +func TestRestoreUnitSuite(t *testing.T) { + suite.Run(t, &RestoreUnitSuite{Suite: tester.NewUnitSuite(t)}) +} + +func (suite *RestoreUnitSuite) TestAugmentRestorePaths() { + table := []struct { + name string + version int + input []string + output []string + }{ + { + name: "no change v0", + version: 0, + input: []string{ + "file.txt.data", + "file.txt", // v0 does not have `.data` + }, + output: []string{ + "file.txt", // ordering artifact of sorting + "file.txt.data", + }, + }, + { + name: "one folder v0", + version: 0, + input: []string{ + "folder/file.txt.data", + "folder/file.txt", + }, + output: []string{ + "folder/file.txt", + "folder/file.txt.data", + }, + }, + { + name: "no change v1", + version: version.OneDrive1DataAndMetaFiles, + input: []string{ + "file.txt.data", + }, + output: []string{ + "file.txt.data", + }, + }, + { + name: "one folder v1", + version: version.OneDrive1DataAndMetaFiles, + input: []string{ + "folder/file.txt.data", + }, + output: []string{ + "folder.dirmeta", + "folder/file.txt.data", + }, + }, + { + name: "nested folders v1", + version: version.OneDrive1DataAndMetaFiles, + input: []string{ + "folder/file.txt.data", + "folder/folder2/file.txt.data", + }, + output: []string{ + "folder.dirmeta", + "folder/file.txt.data", + "folder/folder2.dirmeta", + "folder/folder2/file.txt.data", + }, + }, + { + name: "no change v4", + version: version.OneDrive4DirIncludesPermissions, + input: []string{ + "file.txt.data", + }, + output: []string{ + "file.txt.data", + }, + }, + { + name: "one folder v4", + version: version.OneDrive4DirIncludesPermissions, + input: []string{ + "folder/file.txt.data", + }, + output: []string{ + "folder/file.txt.data", + "folder/folder.dirmeta", + }, + }, + { + name: "nested folders v4", + version: version.OneDrive4DirIncludesPermissions, + input: []string{ + "folder/file.txt.data", + "folder/folder2/file.txt.data", + }, + output: []string{ + "folder/file.txt.data", + "folder/folder.dirmeta", + "folder/folder2/file.txt.data", + "folder/folder2/folder2.dirmeta", + }, + }, + } + + for _, test := range table { + suite.Run(test.name, func() { + t := suite.T() + + _, flush := tester.NewContext() + defer flush() + + base := "id/onedrive/user/files/drives/driveID/root:/" + + inPaths := []path.Path{} + for _, ps := range test.input { + p, err := path.FromDataLayerPath(base+ps, true) + require.NoError(t, err, "creating path") + + inPaths = append(inPaths, p) + } + + outPaths := []path.Path{} + for _, ps := range test.output { + p, err := path.FromDataLayerPath(base+ps, true) + require.NoError(t, err, "creating path") + + outPaths = append(outPaths, p) + } + + actual, err := AugmentRestorePaths(test.version, inPaths) + require.NoError(t, err, "augmenting paths") + + // Ordering of paths matter here as we need dirmeta files + // to show up before file in dir + assert.Equal(t, outPaths, actual, "augmented paths") + }) + } +} diff --git a/src/internal/operations/restore.go b/src/internal/operations/restore.go index e4432d4d8..36046f531 100644 --- a/src/internal/operations/restore.go +++ b/src/internal/operations/restore.go @@ -3,7 +3,6 @@ package operations import ( "context" "fmt" - "sort" "time" "github.com/alcionai/clues" @@ -12,6 +11,7 @@ import ( "github.com/alcionai/corso/src/internal/common" "github.com/alcionai/corso/src/internal/common/crash" + "github.com/alcionai/corso/src/internal/connector/onedrive" "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" D "github.com/alcionai/corso/src/internal/diagnostics" @@ -193,7 +193,7 @@ func (op *RestoreOperation) do( return nil, errors.Wrap(err, "getting backup and details") } - paths, err := formatDetailsForRestoration(ctx, op.Selectors, deets, op.Errors) + paths, err := formatDetailsForRestoration(ctx, bup.Version, op.Selectors, deets, op.Errors) if err != nil { return nil, errors.Wrap(err, "formatting paths from details") } @@ -319,6 +319,7 @@ func (op *RestoreOperation) persistResults( // selector specifications. func formatDetailsForRestoration( ctx context.Context, + backupVersion int, sel selectors.Selector, deets *details.Details, errs *fault.Bus, @@ -354,16 +355,12 @@ func formatDetailsForRestoration( shortRefs[i] = p.ShortRef() } - // TODO(meain): Move this to onedrive specific component, but as - // of now the paths can technically be from multiple services - - // This sort is done primarily to order `.meta` files after `.data` - // files. This is only a necessity for OneDrive as we are storing - // metadata for files/folders in separate meta files and we the - // data to be restored before we can restore the metadata. - sort.Slice(paths, func(i, j int) bool { - return paths[i].String() < paths[j].String() - }) + if sel.Service == selectors.ServiceOneDrive { + paths, err = onedrive.AugmentRestorePaths(backupVersion, paths) + if err != nil { + return nil, clues.Wrap(err, "augmenting paths") + } + } logger.Ctx(ctx).With("short_refs", shortRefs).Infof("found %d details entries to restore", len(shortRefs)) diff --git a/src/pkg/backup/details/details.go b/src/pkg/backup/details/details.go index 1f8072844..204dd1fc0 100644 --- a/src/pkg/backup/details/details.go +++ b/src/pkg/backup/details/details.go @@ -38,12 +38,10 @@ type DetailsModel struct { // Print writes the DetailModel Entries to StdOut, in the format // requested by the caller. func (dm DetailsModel) PrintEntries(ctx context.Context) { - sl := dm.FilterMetaFiles() - if print.JSONFormat() { - printJSON(ctx, sl) + printJSON(ctx, dm) } else { - printTable(ctx, sl) + printTable(ctx, dm) } } diff --git a/src/pkg/repository/repository.go b/src/pkg/repository/repository.go index d436d78e9..b88f8578a 100644 --- a/src/pkg/repository/repository.go +++ b/src/pkg/repository/repository.go @@ -373,6 +373,8 @@ func (r repository) BackupDetails( } } + deets.DetailsModel = deets.FilterMetaFiles() + return &deets, b, errs }