Factor out common code for getting kopia items (#298)

* Factor out common code for getting kopia items

Both directory and single item restore in kopia need to do common tasks
like getting the item in question. Factor out that common code and
adjust tests to prep for directory restore.

src/internal/kopia/wrapper.go

@@ -273,6 +273,74 @@ func (w Wrapper) makeSnapshotWithRoot(
 	return &res, nil
 }
 
+// getEntry returns the item that the restore operation is rooted at. For
+// single-item restores, this is the kopia file the data is sourced from. For
+// restores of directories or subtrees it is the directory at the root of the
+// subtree.
+func (w Wrapper) getEntry(
+	ctx context.Context,
+	snapshotID string,
+	itemPath []string,
+) (fs.Entry, error) {
+	if len(itemPath) == 0 {
+		return nil, errors.New("no restore path given")
+	}
+
+	manifest, err := snapshot.LoadSnapshot(ctx, w.c, manifest.ID(snapshotID))
+	if err != nil {
+		return nil, errors.Wrap(err, "getting snapshot handle")
+	}
+
+	rootDirEntry, err := snapshotfs.SnapshotRoot(w.c, manifest)
+	if err != nil {
+		return nil, errors.Wrap(err, "getting root directory")
+	}
+
+	// GetNestedEntry handles nil properly.
+	e, err := snapshotfs.GetNestedEntry(ctx, rootDirEntry, itemPath[1:])
+	if err != nil {
+		return nil, errors.Wrap(err, "getting nested object handle")
+	}
+
+	return e, nil
+}
+
+// collectItems is a generic helper function that pulls data from kopia for the
+// given item in the snapshot with ID snapshotID. If isDirectory is true, it
+// returns a slice of DataCollections with data from directories in the subtree
+// rooted at itemPath. If isDirectory is false it returns a DataCollection (in a
+// slice) with a single item corresponding to the requested item. If the item
+// does not exist or a file is found when a directory is expected (or the
+// opposite) it returns an error.
+func (w Wrapper) collectItems(
+	ctx context.Context,
+	snapshotID string,
+	itemPath []string,
+	isDirectory bool,
+) ([]connector.DataCollection, error) {
+	e, err := w.getEntry(ctx, snapshotID, itemPath)
+	if err != nil {
+		return nil, err
+	}
+
+	// The paths passed below is the path up to (but not including) the
+	// file/directory passed.
+	if isDirectory {
+		return nil, errors.New("directory restore not implemented")
+	}
+
+	f, ok := e.(fs.File)
+	if !ok {
+		return nil, errors.New("requested object is not a file")
+	}
+	c, err := w.restoreSingleItem(ctx, f, itemPath[:len(itemPath)-1])
+	if err != nil {
+		return nil, err
+	}
+
+	return []connector.DataCollection{c}, nil
+}
+
 // RestoreSingleItem looks up the item at the given path in the snapshot with id
 // snapshotID. The path should be the full path of the item from the root.
 // If the item is a file in kopia then it returns a DataCollection with the item
@@ -285,18 +353,12 @@ func (w Wrapper) RestoreSingleItem(
 	snapshotID string,
 	itemPath []string,
 ) (connector.DataCollection, error) {
-	manifest, err := snapshot.LoadSnapshot(ctx, w.c, manifest.ID(snapshotID))
+	c, err := w.collectItems(ctx, snapshotID, itemPath, false)
 	if err != nil {
-		return nil, errors.Wrap(err, "getting snapshot handle")
+		return nil, err
 	}
 
-	rootDirEntry, err := snapshotfs.SnapshotRoot(w.c, manifest)
+	return c[0], nil
-	if err != nil {
-		return nil, errors.Wrap(err, "getting root directory")
-	}
-
-	// Fine if rootDirEntry is nil, will be checked in called function.
-	return w.restoreSingleItem(ctx, rootDirEntry, itemPath[1:])
 }
 
 // restoreSingleItem looks up the item at the given path starting from rootDir
@@ -308,27 +370,14 @@ func (w Wrapper) RestoreSingleItem(
 // sourced from.
 func (w Wrapper) restoreSingleItem(
 	ctx context.Context,
-	rootDir fs.Entry,
+	f fs.File,
 	itemPath []string,
 ) (connector.DataCollection, error) {
-	e, err := snapshotfs.GetNestedEntry(ctx, rootDir, itemPath)
-	if err != nil {
-		return nil, errors.Wrap(err, "getting object handle")
-	}
-
-	f, ok := e.(fs.File)
-	if !ok {
-		return nil, errors.New("not a file")
-	}
-
 	r, err := f.Open(ctx)
 	if err != nil {
 		return nil, errors.Wrap(err, "opening file")
 	}
 
-	pathWithRoot := []string{rootDir.Name()}
-	pathWithRoot = append(pathWithRoot, itemPath[:len(itemPath)-1]...)
-
 	return &kopiaDataCollection{
 		streams: []connector.DataStream{
 			&kopiaDataStream{
@@ -336,6 +385,6 @@ func (w Wrapper) restoreSingleItem(
 				reader: r,
 			},
 		},
-		path: pathWithRoot,
+		path: itemPath,
 	}, nil
 }

src/internal/kopia/wrapper_test.go

@@ -8,7 +8,6 @@ import (
 	"testing"
 
 	"github.com/kopia/kopia/fs"
-	"github.com/kopia/kopia/fs/virtualfs"
 	"github.com/kopia/kopia/repo/manifest"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -356,30 +355,29 @@ func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupAndRestoreSingleItem() {
 // function.
 func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupAndRestoreSingleItem_Errors() {
 	table := []struct {
-		name           string
+		name       string
-		snapshotIDFunc func(manifest.ID) manifest.ID
+		snapshotID string
-		path           []string
+		path       []string
 	}{
+		{
+			"EmptyPath",
+			string(suite.snapshotID),
+			[]string{},
+		},
 		{
 			"NoSnapshot",
-			func(manifest.ID) manifest.ID {
+			"foo",
-				return manifest.ID("foo")
-			},
 			append(testPath, testFileUUID),
 		},
 		{
 			"TargetNotAFile",
-			func(m manifest.ID) manifest.ID {
+			string(suite.snapshotID),
-				return m
-			},
 			testPath[:2],
 		},
 		{
 			"NonExistentFile",
-			func(m manifest.ID) manifest.ID {
+			string(suite.snapshotID),
-				return m
+			append(testPath, "subdir", "foo"),
-			},
-			append(testPath, "foo"),
 		},
 	}
 
@@ -387,44 +385,7 @@ func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupAndRestoreSingleItem_Err
 		suite.T().Run(test.name, func(t *testing.T) {
 			_, err := suite.w.RestoreSingleItem(
 				suite.ctx,
-				string(test.snapshotIDFunc(suite.snapshotID)),
+				test.snapshotID,
-				test.path,
-			)
-			require.Error(t, err)
-		})
-	}
-}
-
-// TestBackupAndRestoreSingleItem_Errors2 exercises some edge cases in the
-// package-private restoreSingleItem function. It helps ensure kopia behaves the
-// way we expect.
-func (suite *KopiaSimpleRepoIntegrationSuite) TestBackupAndRestoreSingleItem_Errors2() {
-	table := []struct {
-		name        string
-		rootDirFunc func(*testing.T, context.Context, *Wrapper) fs.Entry
-		path        []string
-	}{
-		{
-			"FileAsRoot",
-			func(t *testing.T, ctx context.Context, w *Wrapper) fs.Entry {
-				return virtualfs.StreamingFileFromReader(testFileUUID, bytes.NewReader(testFileData))
-			},
-			append(testPath[1:], testFileUUID),
-		},
-		{
-			"NoRootDir",
-			func(t *testing.T, ctx context.Context, w *Wrapper) fs.Entry {
-				return nil
-			},
-			append(testPath[1:], testFileUUID),
-		},
-	}
-
-	for _, test := range table {
-		suite.T().Run(test.name, func(t *testing.T) {
-			_, err := suite.w.restoreSingleItem(
-				suite.ctx,
-				test.rootDirFunc(t, suite.ctx, suite.w),
 				test.path,
 			)
 			require.Error(t, err)