diff --git a/src/internal/kopia/wrapper.go b/src/internal/kopia/wrapper.go index ab205ef53..2a6b5c6ed 100644 --- a/src/internal/kopia/wrapper.go +++ b/src/internal/kopia/wrapper.go @@ -2,7 +2,6 @@ package kopia import ( "context" - stdpath "path" "sync" "github.com/hashicorp/go-multierror" @@ -28,7 +27,10 @@ const ( corsoUser = "corso" ) -var errNotConnected = errors.New("not connected to repo") +var ( + errNotConnected = errors.New("not connected to repo") + errNoRestorePath = errors.New("no restore path given") +) type BackupStats struct { SnapshotID string @@ -428,7 +430,7 @@ func (w Wrapper) getEntry( itemPath path.Path, ) (fs.Entry, error) { if itemPath == nil { - return nil, errors.New("no restore path given") + return nil, errors.WithStack(errNoRestorePath) } man, err := snapshot.LoadSnapshot(ctx, w.c, manifest.ID(snapshotID)) @@ -463,21 +465,18 @@ func (w Wrapper) getEntry( func (w Wrapper) collectItems( ctx context.Context, snapshotID string, - itemPath []string, + itemPath path.Path, ) ([]data.Collection, error) { - // TODO(ashmrtn): Remove this extra parsing once selectors pass path.Path to - // this function. - pth, err := path.FromDataLayerPath(stdpath.Join(itemPath...), true) - if err != nil { - return nil, errors.Wrap(err, "parsing to path struct") + if itemPath == nil { + return nil, errors.WithStack(errNoRestorePath) } - parentDir, err := pth.Dir() + parentDir, err := itemPath.Dir() if err != nil { return nil, errors.Wrap(err, "getting parent directory from path") } - e, err := w.getEntry(ctx, snapshotID, pth) + e, err := w.getEntry(ctx, snapshotID, itemPath) if err != nil { return nil, err } @@ -505,7 +504,7 @@ func (w Wrapper) collectItems( func (w Wrapper) RestoreSingleItem( ctx context.Context, snapshotID string, - itemPath []string, + itemPath path.Path, ) (data.Collection, error) { c, err := w.collectItems(ctx, snapshotID, itemPath) if err != nil { @@ -553,8 +552,12 @@ func restoreSingleItem( func (w Wrapper) RestoreMultipleItems( ctx context.Context, snapshotID string, - paths [][]string, + paths []path.Path, ) ([]data.Collection, error) { + if len(paths) == 0 { + return nil, errors.WithStack(errNoRestorePath) + } + var ( dcs = []data.Collection{} errs *multierror.Error diff --git a/src/internal/kopia/wrapper_test.go b/src/internal/kopia/wrapper_test.go index fb3e87378..f1a14482b 100644 --- a/src/internal/kopia/wrapper_test.go +++ b/src/internal/kopia/wrapper_test.go @@ -617,11 +617,9 @@ func (suite *KopiaIntegrationSuite) TestRestoreAfterCompressionChange() { result, err := w.RestoreMultipleItems( ctx, string(stats.SnapshotID), - [][]string{ - // TODO(ashmrtn): Remove when selectors passes paths to wrapper for - // restore. - fp1.Elements(), - fp2.Elements(), + []path.Path{ + fp1, + fp2, }) require.NoError(t, err) @@ -815,10 +813,13 @@ func (suite *KopiaSimpleRepoIntegrationSuite) TearDownTest() { func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupAndRestoreSingleItem() { t := suite.T() + itemPath, err := suite.testPath1.Append(testFileName, true) + require.NoError(t, err) + c, err := suite.w.RestoreSingleItem( suite.ctx, string(suite.snapshotID), - append(testPath, testFileName), + itemPath, ) require.NoError(t, err) @@ -840,30 +841,41 @@ func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupAndRestoreSingleItem() { // TestBackupAndRestoreSingleItem_Errors exercises the public RestoreSingleItem // function. func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupAndRestoreSingleItem_Errors() { + itemPath, err := suite.testPath1.Append(testFileName, true) + require.NoError(suite.T(), err) + + doesntExist, err := path.Builder{}.Append("subdir", "foo").ToDataLayerExchangePathForCategory( + testTenant, + testUser, + path.EmailCategory, + true, + ) + require.NoError(suite.T(), err) + table := []struct { name string snapshotID string - path []string + path path.Path }{ { "EmptyPath", string(suite.snapshotID), - []string{}, + nil, }, { "NoSnapshot", "foo", - append(testPath, testFileName), + itemPath, }, { "TargetNotAFile", string(suite.snapshotID), - testPath[:2], + suite.testPath1, }, { "NonExistentFile", string(suite.snapshotID), - append(testPath, "subdir", "foo"), + doesntExist, }, } @@ -908,10 +920,9 @@ func (suite *KopiaSimpleRepoIntegrationSuite) TestRestoreMultipleItems() { result, err := w.RestoreMultipleItems( ctx, string(stats.SnapshotID), - [][]string{ - // TODO(ashmrtn): Remove when selectors pass path to kopia restore. - fp1.Elements(), - fp2.Elements(), + []path.Path{ + fp1, + fp2, }) require.NoError(t, err) @@ -921,30 +932,46 @@ func (suite *KopiaSimpleRepoIntegrationSuite) TestRestoreMultipleItems() { } func (suite *KopiaSimpleRepoIntegrationSuite) TestRestoreMultipleItems_Errors() { + itemPath, err := suite.testPath1.Append(testFileName, true) + require.NoError(suite.T(), err) + + doesntExist, err := path.Builder{}.Append("subdir", "foo").ToDataLayerExchangePathForCategory( + testTenant, + testUser, + path.EmailCategory, + true, + ) + require.NoError(suite.T(), err) + table := []struct { name string snapshotID string - paths [][]string + paths []path.Path }{ + { + "NilPaths", + string(suite.snapshotID), + nil, + }, { "EmptyPaths", string(suite.snapshotID), - [][]string{{}}, + []path.Path{}, }, { "NoSnapshot", "foo", - [][]string{append(testPath, testFileName)}, + []path.Path{itemPath}, }, { "TargetNotAFile", string(suite.snapshotID), - [][]string{testPath[:2]}, + []path.Path{suite.testPath1}, }, { "NonExistentFile", string(suite.snapshotID), - [][]string{append(testPath, "subdir", "foo")}, + []path.Path{doesntExist}, }, } @@ -966,8 +993,10 @@ func (suite *KopiaSimpleRepoIntegrationSuite) TestDeleteSnapshot() { assert.NoError(t, suite.w.DeleteSnapshot(suite.ctx, string(suite.snapshotID))) // assert the deletion worked - itemPath := append(testPath, testFileName) - _, err := suite.w.RestoreSingleItem(suite.ctx, string(suite.snapshotID), itemPath) + itemPath, err := suite.testPath1.Append(testFileName, true) + require.NoError(t, err) + + _, err = suite.w.RestoreSingleItem(suite.ctx, string(suite.snapshotID), itemPath) assert.Error(t, err, "snapshot should be deleted") } diff --git a/src/internal/operations/restore.go b/src/internal/operations/restore.go index a04917ae2..3d660a323 100644 --- a/src/internal/operations/restore.go +++ b/src/internal/operations/restore.go @@ -2,7 +2,6 @@ package operations import ( "context" - "strings" "time" multierror "github.com/hashicorp/go-multierror" @@ -13,6 +12,7 @@ import ( "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/kopia" "github.com/alcionai/corso/src/internal/model" + "github.com/alcionai/corso/src/internal/path" "github.com/alcionai/corso/src/internal/stats" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/control" @@ -112,22 +112,36 @@ func (op *RestoreOperation) Run(ctx context.Context) (err error) { return errors.New("nothing to restore: no items in the backup match the provided selectors") } - // todo: use path pkg for this fdsPaths := fds.Paths() - paths := make([][]string, len(fdsPaths)) + paths := make([]path.Path, len(fdsPaths)) + + var parseErrs *multierror.Error for i := range fdsPaths { - paths[i] = strings.Split(fdsPaths[i], "/") + p, err := path.FromDataLayerPath(fdsPaths[i], true) + if err != nil { + parseErrs = multierror.Append( + parseErrs, + errors.Wrap(err, "parsing details entry path"), + ) + + continue + } + + paths[i] = p } dcs, err := op.kopia.RestoreMultipleItems(ctx, b.SnapshotID, paths) if err != nil { err = errors.Wrap(err, "retrieving service data") - opStats.readErr = err + + parseErrs = multierror.Append(parseErrs, err) + opStats.readErr = parseErrs.ErrorOrNil() return err } + opStats.readErr = parseErrs.ErrorOrNil() opStats.cs = dcs // restore those collections using graph