From fbcbd5318c5eec2c26773182bf1ce0cd25a99db0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 12 Jun 2022 14:38:19 +0200 Subject: [PATCH 1/2] repository: extract LoadTree/SaveTree The repository has no real idea what a Tree is. So these methods never belonged there. --- cmd/restic/cmd_diff.go | 8 ++-- cmd/restic/cmd_dump.go | 6 +-- cmd/restic/cmd_recover.go | 4 +- internal/archiver/archiver.go | 4 +- internal/archiver/archiver_test.go | 8 ++-- internal/archiver/testing.go | 2 +- internal/checker/checker_test.go | 8 ++-- internal/dump/common_test.go | 2 +- internal/fuse/dir.go | 4 +- internal/fuse/fuse_test.go | 2 +- internal/repository/repository.go | 35 ------------------ internal/repository/repository_test.go | 45 ----------------------- internal/restic/find.go | 8 ++-- internal/restic/find_test.go | 2 +- internal/restic/repository.go | 11 ++++-- internal/restic/tree.go | 45 +++++++++++++++++++++++ internal/restic/tree_stream.go | 8 ++-- internal/restic/tree_test.go | 51 ++++++++++++++++++++++++-- internal/restorer/restorer.go | 2 +- internal/restorer/restorer_test.go | 2 +- internal/walker/walker.go | 13 ++----- internal/walker/walker_test.go | 13 ++++++- 22 files changed, 150 insertions(+), 133 deletions(-) diff --git a/cmd/restic/cmd_diff.go b/cmd/restic/cmd_diff.go index 8099bf296..5fdd28d97 100644 --- a/cmd/restic/cmd_diff.go +++ b/cmd/restic/cmd_diff.go @@ -160,7 +160,7 @@ func updateBlobs(repo restic.Repository, blobs restic.BlobSet, stats *DiffStat) func (c *Comparer) printDir(ctx context.Context, mode string, stats *DiffStat, blobs restic.BlobSet, prefix string, id restic.ID) error { debug.Log("print %v tree %v", mode, id) - tree, err := c.repo.LoadTree(ctx, id) + tree, err := restic.LoadTree(ctx, c.repo, id) if err != nil { return err } @@ -187,7 +187,7 @@ func (c *Comparer) printDir(ctx context.Context, mode string, stats *DiffStat, b func (c *Comparer) collectDir(ctx context.Context, blobs restic.BlobSet, id restic.ID) error { debug.Log("print tree %v", id) - tree, err := c.repo.LoadTree(ctx, id) + tree, err := restic.LoadTree(ctx, c.repo, id) if err != nil { return err } @@ -231,12 +231,12 @@ func uniqueNodeNames(tree1, tree2 *restic.Tree) (tree1Nodes, tree2Nodes map[stri func (c *Comparer) diffTree(ctx context.Context, stats *DiffStatsContainer, prefix string, id1, id2 restic.ID) error { debug.Log("diffing %v to %v", id1, id2) - tree1, err := c.repo.LoadTree(ctx, id1) + tree1, err := restic.LoadTree(ctx, c.repo, id1) if err != nil { return err } - tree2, err := c.repo.LoadTree(ctx, id2) + tree2, err := restic.LoadTree(ctx, c.repo, id2) if err != nil { return err } diff --git a/cmd/restic/cmd_dump.go b/cmd/restic/cmd_dump.go index b4ce299cd..993072f9c 100644 --- a/cmd/restic/cmd_dump.go +++ b/cmd/restic/cmd_dump.go @@ -87,7 +87,7 @@ func printFromTree(ctx context.Context, tree *restic.Tree, repo restic.Repositor case l == 1 && dump.IsFile(node): return d.WriteNode(ctx, node) case l > 1 && dump.IsDir(node): - subtree, err := repo.LoadTree(ctx, *node.Subtree) + subtree, err := restic.LoadTree(ctx, repo, *node.Subtree) if err != nil { return errors.Wrapf(err, "cannot load subtree for %q", item) } @@ -96,7 +96,7 @@ func printFromTree(ctx context.Context, tree *restic.Tree, repo restic.Repositor if err := checkStdoutArchive(); err != nil { return err } - subtree, err := repo.LoadTree(ctx, *node.Subtree) + subtree, err := restic.LoadTree(ctx, repo, *node.Subtree) if err != nil { return err } @@ -168,7 +168,7 @@ func runDump(opts DumpOptions, gopts GlobalOptions, args []string) error { return err } - tree, err := repo.LoadTree(ctx, *sn.Tree) + tree, err := restic.LoadTree(ctx, repo, *sn.Tree) if err != nil { Exitf(2, "loading tree for snapshot %q failed: %v", snapshotIDString, err) } diff --git a/cmd/restic/cmd_recover.go b/cmd/restic/cmd_recover.go index 1eeba54e3..f81ef1f10 100644 --- a/cmd/restic/cmd_recover.go +++ b/cmd/restic/cmd_recover.go @@ -75,7 +75,7 @@ func runRecover(gopts GlobalOptions) error { Verbosef("load %d trees\n", len(trees)) bar := newProgressMax(!gopts.Quiet, uint64(len(trees)), "trees loaded") for id := range trees { - tree, err := repo.LoadTree(gopts.ctx, id) + tree, err := restic.LoadTree(gopts.ctx, repo, id) if err != nil { Warnf("unable to load tree %v: %v\n", id.Str(), err) continue @@ -138,7 +138,7 @@ func runRecover(gopts GlobalOptions) error { var treeID restic.ID wg.Go(func() error { var err error - treeID, err = repo.SaveTree(ctx, tree) + treeID, err = restic.SaveTree(ctx, repo, tree) if err != nil { return errors.Fatalf("unable to save new tree to the repository: %v", err) } diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 7d28fcb30..fe99e3704 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -218,7 +218,7 @@ func (arch *Archiver) loadSubtree(ctx context.Context, node *restic.Node) (*rest return nil, nil } - tree, err := arch.Repo.LoadTree(ctx, *node.Subtree) + tree, err := restic.LoadTree(ctx, arch.Repo, *node.Subtree) if err != nil { debug.Log("unable to load tree %v: %v", node.Subtree.Str(), err) // a tree in the repository is not readable -> warn the user @@ -762,7 +762,7 @@ func (arch *Archiver) loadParentTree(ctx context.Context, snapshotID restic.ID) } debug.Log("load parent tree %v", *sn.Tree) - tree, err := arch.Repo.LoadTree(ctx, *sn.Tree) + tree, err := restic.LoadTree(ctx, arch.Repo, *sn.Tree) if err != nil { debug.Log("unable to load tree %v: %v", *sn.Tree, err) _ = arch.error("/", nil, arch.wrapLoadTreeError(*sn.Tree, err)) diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 1ae126014..6367a19cb 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -431,7 +431,7 @@ func (repo *blobCountingRepo) SaveBlob(ctx context.Context, t restic.BlobType, b } func (repo *blobCountingRepo) SaveTree(ctx context.Context, t *restic.Tree) (restic.ID, error) { - id, err := repo.Repository.SaveTree(ctx, t) + id, err := restic.SaveTree(ctx, repo.Repository, t) h := restic.BlobHandle{ID: id, Type: restic.TreeBlob} repo.m.Lock() repo.saved[h]++ @@ -875,7 +875,7 @@ func TestArchiverSaveDir(t *testing.T) { node.Name = targetNodeName tree := &restic.Tree{Nodes: []*restic.Node{node}} - treeID, err := repo.SaveTree(ctx, tree) + treeID, err := restic.SaveTree(ctx, repo, tree) if err != nil { t.Fatal(err) } @@ -1123,7 +1123,7 @@ func TestArchiverSaveTree(t *testing.T) { t.Fatal(err) } - treeID, err := repo.SaveTree(ctx, tree) + treeID, err := restic.SaveTree(ctx, repo, tree) if err != nil { t.Fatal(err) } @@ -2076,7 +2076,7 @@ func snapshot(t testing.TB, repo restic.Repository, fs fs.FS, parent restic.ID, t.Fatal(err) } - tree, err := repo.LoadTree(ctx, *snapshot.Tree) + tree, err := restic.LoadTree(ctx, repo, *snapshot.Tree) if err != nil { t.Fatal(err) } diff --git a/internal/archiver/testing.go b/internal/archiver/testing.go index d8ad0a9e7..4021c5bc6 100644 --- a/internal/archiver/testing.go +++ b/internal/archiver/testing.go @@ -250,7 +250,7 @@ func TestEnsureFileContent(ctx context.Context, t testing.TB, repo restic.Reposi func TestEnsureTree(ctx context.Context, t testing.TB, prefix string, repo restic.Repository, treeID restic.ID, dir TestDir) { t.Helper() - tree, err := repo.LoadTree(ctx, treeID) + tree, err := restic.LoadTree(ctx, repo, treeID) if err != nil { t.Fatal(err) return diff --git a/internal/checker/checker_test.go b/internal/checker/checker_test.go index a4245d574..3ab177c7d 100644 --- a/internal/checker/checker_test.go +++ b/internal/checker/checker_test.go @@ -403,7 +403,7 @@ func (r *loadTreesOnceRepository) LoadTree(ctx context.Context, id restic.ID) (* return nil, errors.Errorf("trying to load tree with id %v twice", id) } r.loadedTrees.Insert(id) - return r.Repository.LoadTree(ctx, id) + return restic.LoadTree(ctx, r.Repository, id) } func TestCheckerNoDuplicateTreeDecodes(t *testing.T) { @@ -443,7 +443,7 @@ func (r *delayRepository) LoadTree(ctx context.Context, id restic.ID) (*restic.T if id == r.DelayTree { <-r.UnblockChannel } - return r.Repository.LoadTree(ctx, id) + return restic.LoadTree(ctx, r.Repository, id) } func (r *delayRepository) LookupBlobSize(id restic.ID, t restic.BlobType) (uint, bool) { @@ -479,7 +479,7 @@ func TestCheckerBlobTypeConfusion(t *testing.T) { wg, wgCtx := errgroup.WithContext(ctx) repo.StartPackUploader(wgCtx, wg) - id, err := repo.SaveTree(ctx, damagedTree) + id, err := restic.SaveTree(ctx, repo, damagedTree) test.OK(t, repo.Flush(ctx)) test.OK(t, err) @@ -509,7 +509,7 @@ func TestCheckerBlobTypeConfusion(t *testing.T) { Nodes: []*restic.Node{malNode, dirNode}, } - rootID, err := repo.SaveTree(ctx, rootTree) + rootID, err := restic.SaveTree(ctx, repo, rootTree) test.OK(t, err) test.OK(t, repo.Flush(ctx)) diff --git a/internal/dump/common_test.go b/internal/dump/common_test.go index 22d059751..7892a4fa9 100644 --- a/internal/dump/common_test.go +++ b/internal/dump/common_test.go @@ -88,7 +88,7 @@ func WriteTest(t *testing.T, format string, cd CheckDump) { sn, _, err := arch.Snapshot(ctx, []string{"."}, archiver.SnapshotOptions{}) rtest.OK(t, err) - tree, err := repo.LoadTree(ctx, *sn.Tree) + tree, err := restic.LoadTree(ctx, repo, *sn.Tree) rtest.OK(t, err) dst := &bytes.Buffer{} diff --git a/internal/fuse/dir.go b/internal/fuse/dir.go index 399348312..dcacaa96a 100644 --- a/internal/fuse/dir.go +++ b/internal/fuse/dir.go @@ -55,7 +55,7 @@ func replaceSpecialNodes(ctx context.Context, repo restic.Repository, node *rest return []*restic.Node{node}, nil } - tree, err := repo.LoadTree(ctx, *node.Subtree) + tree, err := restic.LoadTree(ctx, repo, *node.Subtree) if err != nil { return nil, err } @@ -88,7 +88,7 @@ func (d *dir) open(ctx context.Context) error { debug.Log("open dir %v (%v)", d.node.Name, d.node.Subtree) - tree, err := d.root.repo.LoadTree(ctx, *d.node.Subtree) + tree, err := restic.LoadTree(ctx, d.root.repo, *d.node.Subtree) if err != nil { debug.Log(" error loading tree %v: %v", d.node.Subtree, err) return err diff --git a/internal/fuse/fuse_test.go b/internal/fuse/fuse_test.go index 690df770a..df24f77af 100644 --- a/internal/fuse/fuse_test.go +++ b/internal/fuse/fuse_test.go @@ -59,7 +59,7 @@ func loadFirstSnapshot(t testing.TB, repo restic.Repository) *restic.Snapshot { } func loadTree(t testing.TB, repo restic.Repository, id restic.ID) *restic.Tree { - tree, err := repo.LoadTree(context.TODO(), id) + tree, err := restic.LoadTree(context.TODO(), repo, id) rtest.OK(t, err) return tree } diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 84193a4f3..863bb9d7b 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -834,41 +834,6 @@ func (r *Repository) SaveBlob(ctx context.Context, t restic.BlobType, buf []byte return newID, known, size, err } -// LoadTree loads a tree from the repository. -func (r *Repository) LoadTree(ctx context.Context, id restic.ID) (*restic.Tree, error) { - debug.Log("load tree %v", id) - - buf, err := r.LoadBlob(ctx, restic.TreeBlob, id, nil) - if err != nil { - return nil, err - } - - t := &restic.Tree{} - err = json.Unmarshal(buf, t) - if err != nil { - return nil, err - } - - return t, nil -} - -// SaveTree stores a tree into the repository and returns the ID. The ID is -// checked against the index. The tree is only stored when the index does not -// contain the ID. -func (r *Repository) SaveTree(ctx context.Context, t *restic.Tree) (restic.ID, error) { - buf, err := json.Marshal(t) - if err != nil { - return restic.ID{}, errors.Wrap(err, "MarshalJSON") - } - - // append a newline so that the data is always consistent (json.Encoder - // adds a newline after each object) - buf = append(buf, '\n') - - id, _, _, err := r.SaveBlob(ctx, restic.TreeBlob, buf, restic.ID{}, false) - return id, err -} - type BackendLoadFn func(ctx context.Context, h restic.Handle, length int, offset int64, fn func(rd io.Reader) error) error // StreamPack loads the listed blobs from the specified pack file. The plaintext blob is passed to diff --git a/internal/repository/repository_test.go b/internal/repository/repository_test.go index 38d3117a5..97e7e55e7 100644 --- a/internal/repository/repository_test.go +++ b/internal/repository/repository_test.go @@ -16,7 +16,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/klauspost/compress/zstd" - "github.com/restic/restic/internal/archiver" "github.com/restic/restic/internal/crypto" "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" @@ -137,50 +136,6 @@ func benchmarkSaveAndEncrypt(t *testing.B, version uint) { } } -func TestLoadTree(t *testing.T) { - repository.TestAllVersions(t, testLoadTree) -} - -func testLoadTree(t *testing.T, version uint) { - repo, cleanup := repository.TestRepositoryWithVersion(t, version) - defer cleanup() - - if rtest.BenchArchiveDirectory == "" { - t.Skip("benchdir not set, skipping") - } - - // archive a few files - sn := archiver.TestSnapshot(t, repo, rtest.BenchArchiveDirectory, nil) - rtest.OK(t, repo.Flush(context.Background())) - - _, err := repo.LoadTree(context.TODO(), *sn.Tree) - rtest.OK(t, err) -} - -func BenchmarkLoadTree(t *testing.B) { - repository.BenchmarkAllVersions(t, benchmarkLoadTree) -} - -func benchmarkLoadTree(t *testing.B, version uint) { - repo, cleanup := repository.TestRepositoryWithVersion(t, version) - defer cleanup() - - if rtest.BenchArchiveDirectory == "" { - t.Skip("benchdir not set, skipping") - } - - // archive a few files - sn := archiver.TestSnapshot(t, repo, rtest.BenchArchiveDirectory, nil) - rtest.OK(t, repo.Flush(context.Background())) - - t.ResetTimer() - - for i := 0; i < t.N; i++ { - _, err := repo.LoadTree(context.TODO(), *sn.Tree) - rtest.OK(t, err) - } -} - func TestLoadBlob(t *testing.T) { repository.TestAllVersions(t, testLoadBlob) } diff --git a/internal/restic/find.go b/internal/restic/find.go index 695bbbb91..6544f2b3d 100644 --- a/internal/restic/find.go +++ b/internal/restic/find.go @@ -8,16 +8,16 @@ import ( "golang.org/x/sync/errgroup" ) -// TreeLoader loads a tree from a repository. -type TreeLoader interface { - LoadTree(context.Context, ID) (*Tree, error) +// Loader loads a blob from a repository. +type Loader interface { + LoadBlob(context.Context, BlobType, ID, []byte) ([]byte, error) LookupBlobSize(id ID, tpe BlobType) (uint, bool) Connections() uint } // FindUsedBlobs traverses the tree ID and adds all seen blobs (trees and data // blobs) to the set blobs. Already seen tree blobs will not be visited again. -func FindUsedBlobs(ctx context.Context, repo TreeLoader, treeIDs IDs, blobs BlobSet, p *progress.Counter) error { +func FindUsedBlobs(ctx context.Context, repo Loader, treeIDs IDs, blobs BlobSet, p *progress.Counter) error { var lock sync.Mutex wg, ctx := errgroup.WithContext(ctx) diff --git a/internal/restic/find_test.go b/internal/restic/find_test.go index 46f607299..b415501dc 100644 --- a/internal/restic/find_test.go +++ b/internal/restic/find_test.go @@ -162,7 +162,7 @@ func TestMultiFindUsedBlobs(t *testing.T) { type ForbiddenRepo struct{} -func (r ForbiddenRepo) LoadTree(ctx context.Context, id restic.ID) (*restic.Tree, error) { +func (r ForbiddenRepo) LoadBlob(context.Context, restic.BlobType, restic.ID, []byte) ([]byte, error) { return nil, errors.New("should not be called") } diff --git a/internal/restic/repository.go b/internal/restic/repository.go index 1daeb0e4f..996f11260 100644 --- a/internal/restic/repository.go +++ b/internal/restic/repository.go @@ -43,20 +43,18 @@ type Repository interface { StartPackUploader(ctx context.Context, wg *errgroup.Group) Flush(context.Context) error - SaveUnpacked(context.Context, FileType, []byte) (ID, error) SaveJSONUnpacked(context.Context, FileType, interface{}) (ID, error) LoadJSONUnpacked(ctx context.Context, t FileType, id ID, dest interface{}) error + // LoadUnpacked loads and decrypts the file with the given type and ID, // using the supplied buffer (which must be empty). If the buffer is nil, a // new buffer will be allocated and returned. LoadUnpacked(ctx context.Context, buf []byte, t FileType, id ID) (data []byte, err error) + SaveUnpacked(context.Context, FileType, []byte) (ID, error) LoadBlob(context.Context, BlobType, ID, []byte) ([]byte, error) SaveBlob(context.Context, BlobType, []byte, ID, bool) (ID, bool, int, error) - - LoadTree(context.Context, ID) (*Tree, error) - SaveTree(context.Context, *Tree) (ID, error) } // Lister allows listing files in a backend. @@ -71,6 +69,11 @@ type LoadJSONUnpackeder interface { LoadJSONUnpacked(ctx context.Context, t FileType, id ID, dest interface{}) error } +// LoaderUnpacked allows loading a blob not stored in a pack file +type LoaderUnpacked interface { + LoadUnpacked(ctx context.Context, buf []byte, t FileType, id ID) (data []byte, err error) +} + // SaverUnpacked allows saving a blob not stored in a pack file type SaverUnpacked interface { // Connections returns the maximum number of concurrent backend operations diff --git a/internal/restic/tree.go b/internal/restic/tree.go index 09ea44dfa..33d1ec577 100644 --- a/internal/restic/tree.go +++ b/internal/restic/tree.go @@ -1,6 +1,8 @@ package restic import ( + "context" + "encoding/json" "fmt" "sort" @@ -98,3 +100,46 @@ func (t *Tree) Subtrees() (trees IDs) { return trees } + +type BlobLoader interface { + LoadBlob(context.Context, BlobType, ID, []byte) ([]byte, error) +} + +// LoadTree loads a tree from the repository. +func LoadTree(ctx context.Context, r BlobLoader, id ID) (*Tree, error) { + debug.Log("load tree %v", id) + + buf, err := r.LoadBlob(ctx, TreeBlob, id, nil) + if err != nil { + return nil, err + } + + t := &Tree{} + err = json.Unmarshal(buf, t) + if err != nil { + return nil, err + } + + return t, nil +} + +type BlobSaver interface { + SaveBlob(context.Context, BlobType, []byte, ID, bool) (ID, bool, int, error) +} + +// SaveTree stores a tree into the repository and returns the ID. The ID is +// checked against the index. The tree is only stored when the index does not +// contain the ID. +func SaveTree(ctx context.Context, r BlobSaver, t *Tree) (ID, error) { + buf, err := json.Marshal(t) + if err != nil { + return ID{}, errors.Wrap(err, "MarshalJSON") + } + + // append a newline so that the data is always consistent (json.Encoder + // adds a newline after each object) + buf = append(buf, '\n') + + id, _, _, err := r.SaveBlob(ctx, TreeBlob, buf, ID{}, false) + return id, err +} diff --git a/internal/restic/tree_stream.go b/internal/restic/tree_stream.go index 39f1c6646..4110a5e8d 100644 --- a/internal/restic/tree_stream.go +++ b/internal/restic/tree_stream.go @@ -29,11 +29,11 @@ type trackedID struct { } // loadTreeWorker loads trees from repo and sends them to out. -func loadTreeWorker(ctx context.Context, repo TreeLoader, +func loadTreeWorker(ctx context.Context, repo Loader, in <-chan trackedID, out chan<- trackedTreeItem) { for treeID := range in { - tree, err := repo.LoadTree(ctx, treeID.ID) + tree, err := LoadTree(ctx, repo, treeID.ID) debug.Log("load tree %v (%v) returned err: %v", tree, treeID, err) job := trackedTreeItem{TreeItem: TreeItem{ID: treeID.ID, Error: err, Tree: tree}, rootIdx: treeID.rootIdx} @@ -45,7 +45,7 @@ func loadTreeWorker(ctx context.Context, repo TreeLoader, } } -func filterTrees(ctx context.Context, repo TreeLoader, trees IDs, loaderChan chan<- trackedID, hugeTreeLoaderChan chan<- trackedID, +func filterTrees(ctx context.Context, repo Loader, trees IDs, loaderChan chan<- trackedID, hugeTreeLoaderChan chan<- trackedID, in <-chan trackedTreeItem, out chan<- TreeItem, skip func(tree ID) bool, p *progress.Counter) { var ( @@ -154,7 +154,7 @@ func filterTrees(ctx context.Context, repo TreeLoader, trees IDs, loaderChan cha // is guaranteed to always be called from the same goroutine. To shutdown the started // goroutines, either read all items from the channel or cancel the context. Then `Wait()` // on the errgroup until all goroutines were stopped. -func StreamTrees(ctx context.Context, wg *errgroup.Group, repo TreeLoader, trees IDs, skip func(tree ID) bool, p *progress.Counter) <-chan TreeItem { +func StreamTrees(ctx context.Context, wg *errgroup.Group, repo Loader, trees IDs, skip func(tree ID) bool, p *progress.Counter) <-chan TreeItem { loaderChan := make(chan trackedID) hugeTreeChan := make(chan trackedID, 10) loadedTreeChan := make(chan trackedTreeItem) diff --git a/internal/restic/tree_test.go b/internal/restic/tree_test.go index 0f9b9a9d3..3ed3e7938 100644 --- a/internal/restic/tree_test.go +++ b/internal/restic/tree_test.go @@ -9,6 +9,7 @@ import ( "strconv" "testing" + "github.com/restic/restic/internal/archiver" "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" @@ -95,7 +96,7 @@ func TestNodeComparison(t *testing.T) { rtest.Assert(t, !node.Equals(n2), "nodes are equal") } -func TestLoadTree(t *testing.T) { +func TestEmptyLoadTree(t *testing.T) { repo, cleanup := repository.TestRepository(t) defer cleanup() @@ -103,14 +104,14 @@ func TestLoadTree(t *testing.T) { repo.StartPackUploader(context.TODO(), &wg) // save tree tree := restic.NewTree(0) - id, err := repo.SaveTree(context.TODO(), tree) + id, err := restic.SaveTree(context.TODO(), repo, tree) rtest.OK(t, err) // save packs rtest.OK(t, repo.Flush(context.Background())) // load tree again - tree2, err := repo.LoadTree(context.TODO(), id) + tree2, err := restic.LoadTree(context.TODO(), repo, id) rtest.OK(t, err) rtest.Assert(t, tree.Equals(tree2), @@ -138,3 +139,47 @@ func BenchmarkBuildTree(b *testing.B) { } } } + +func TestLoadTree(t *testing.T) { + repository.TestAllVersions(t, testLoadTree) +} + +func testLoadTree(t *testing.T, version uint) { + repo, cleanup := repository.TestRepositoryWithVersion(t, version) + defer cleanup() + + if rtest.BenchArchiveDirectory == "" { + t.Skip("benchdir not set, skipping") + } + + // archive a few files + sn := archiver.TestSnapshot(t, repo, rtest.BenchArchiveDirectory, nil) + rtest.OK(t, repo.Flush(context.Background())) + + _, err := restic.LoadTree(context.TODO(), repo, *sn.Tree) + rtest.OK(t, err) +} + +func BenchmarkLoadTree(t *testing.B) { + repository.BenchmarkAllVersions(t, benchmarkLoadTree) +} + +func benchmarkLoadTree(t *testing.B, version uint) { + repo, cleanup := repository.TestRepositoryWithVersion(t, version) + defer cleanup() + + if rtest.BenchArchiveDirectory == "" { + t.Skip("benchdir not set, skipping") + } + + // archive a few files + sn := archiver.TestSnapshot(t, repo, rtest.BenchArchiveDirectory, nil) + rtest.OK(t, repo.Flush(context.Background())) + + t.ResetTimer() + + for i := 0; i < t.N; i++ { + _, err := restic.LoadTree(context.TODO(), repo, *sn.Tree) + rtest.OK(t, err) + } +} diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index 5a3f91368..17231cbba 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -53,7 +53,7 @@ type treeVisitor struct { // target is the path in the file system, location within the snapshot. func (res *Restorer) traverseTree(ctx context.Context, target, location string, treeID restic.ID, visitor treeVisitor) (hasRestored bool, err error) { debug.Log("%v %v %v", target, location, treeID) - tree, err := res.repo.LoadTree(ctx, treeID) + tree, err := restic.LoadTree(ctx, res.repo, treeID) if err != nil { debug.Log("error loading tree %v: %v", treeID, err) return hasRestored, res.Error(location, err) diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index 512c144ab..441f28b41 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -111,7 +111,7 @@ func saveDir(t testing.TB, repo restic.Repository, nodes map[string]Node, inode } } - id, err := repo.SaveTree(ctx, tree) + id, err := restic.SaveTree(ctx, repo, tree) if err != nil { t.Fatal(err) } diff --git a/internal/walker/walker.go b/internal/walker/walker.go index 3c8e723a8..4c4e7f5ab 100644 --- a/internal/walker/walker.go +++ b/internal/walker/walker.go @@ -10,11 +10,6 @@ import ( "github.com/restic/restic/internal/restic" ) -// TreeLoader loads a tree from a repository. -type TreeLoader interface { - LoadTree(context.Context, restic.ID) (*restic.Tree, error) -} - // ErrSkipNode is returned by WalkFunc when a dir node should not be walked. var ErrSkipNode = errors.New("skip this node") @@ -38,8 +33,8 @@ type WalkFunc func(parentTreeID restic.ID, path string, node *restic.Node, nodeE // Walk calls walkFn recursively for each node in root. If walkFn returns an // error, it is passed up the call stack. The trees in ignoreTrees are not // walked. If walkFn ignores trees, these are added to the set. -func Walk(ctx context.Context, repo TreeLoader, root restic.ID, ignoreTrees restic.IDSet, walkFn WalkFunc) error { - tree, err := repo.LoadTree(ctx, root) +func Walk(ctx context.Context, repo restic.BlobLoader, root restic.ID, ignoreTrees restic.IDSet, walkFn WalkFunc) error { + tree, err := restic.LoadTree(ctx, repo, root) _, err = walkFn(root, "/", nil, err) if err != nil { @@ -60,7 +55,7 @@ func Walk(ctx context.Context, repo TreeLoader, root restic.ID, ignoreTrees rest // walk recursively traverses the tree, ignoring subtrees when the ID of the // subtree is in ignoreTrees. If err is nil and ignore is true, the subtree ID // will be added to ignoreTrees by walk. -func walk(ctx context.Context, repo TreeLoader, prefix string, parentTreeID restic.ID, tree *restic.Tree, ignoreTrees restic.IDSet, walkFn WalkFunc) (ignore bool, err error) { +func walk(ctx context.Context, repo restic.BlobLoader, prefix string, parentTreeID restic.ID, tree *restic.Tree, ignoreTrees restic.IDSet, walkFn WalkFunc) (ignore bool, err error) { var allNodesIgnored = true if len(tree.Nodes) == 0 { @@ -104,7 +99,7 @@ func walk(ctx context.Context, repo TreeLoader, prefix string, parentTreeID rest continue } - subtree, err := repo.LoadTree(ctx, *node.Subtree) + subtree, err := restic.LoadTree(ctx, repo, *node.Subtree) ignore, err := walkFn(parentTreeID, p, node, err) if err != nil { if err == ErrSkipNode { diff --git a/internal/walker/walker_test.go b/internal/walker/walker_test.go index eb4545d63..90ca7c2b8 100644 --- a/internal/walker/walker_test.go +++ b/internal/walker/walker_test.go @@ -67,13 +67,22 @@ func buildTreeMap(tree TestTree, m TreeMap) restic.ID { // TreeMap returns the trees from the map on LoadTree. type TreeMap map[restic.ID]*restic.Tree -func (t TreeMap) LoadTree(ctx context.Context, id restic.ID) (*restic.Tree, error) { +func (t TreeMap) LoadBlob(ctx context.Context, tpe restic.BlobType, id restic.ID, buf []byte) ([]byte, error) { + if tpe != restic.TreeBlob { + return nil, errors.New("can only load trees") + } tree, ok := t[id] if !ok { return nil, errors.New("tree not found") } - return tree, nil + tbuf, err := json.Marshal(tree) + if err != nil { + panic(err) + } + tbuf = append(tbuf, '\n') + + return tbuf, nil } func (t TreeMap) Connections() uint { From 89d3ce852b5b909db2c5c971a261b5a8c2a500b3 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 12 Jun 2022 14:38:19 +0200 Subject: [PATCH 2/2] repository: extract Load/StoreJSONUnpacked A Load/Store method for each data type is much clearer. As a result the repository no longer needs a method to load / store json. --- cmd/restic/cmd_cat.go | 7 ++--- cmd/restic/cmd_copy.go | 2 +- cmd/restic/cmd_recover.go | 2 +- cmd/restic/cmd_tag.go | 2 +- cmd/restic/find.go | 2 +- internal/archiver/archiver.go | 2 +- internal/checker/checker_test.go | 7 ++--- internal/migrations/upgrade_repo_v2.go | 2 +- internal/repository/index_parallel.go | 2 +- internal/repository/repository.go | 29 ++--------------- internal/repository/repository_test.go | 42 ++----------------------- internal/restic/config.go | 9 ++++-- internal/restic/config_test.go | 43 ++++++++++++++++---------- internal/restic/json.go | 32 +++++++++++++++++++ internal/restic/lock.go | 4 +-- internal/restic/lock_test.go | 2 +- internal/restic/repository.go | 23 +++++--------- internal/restic/snapshot.go | 11 +++++-- internal/restic/snapshot_find.go | 4 +-- internal/restic/snapshot_test.go | 26 ++++++++++++++++ internal/restic/testing.go | 2 +- internal/restorer/restorer_test.go | 2 +- 22 files changed, 130 insertions(+), 127 deletions(-) create mode 100644 internal/restic/json.go diff --git a/cmd/restic/cmd_cat.go b/cmd/restic/cmd_cat.go index 86d538e70..991df86a2 100644 --- a/cmd/restic/cmd_cat.go +++ b/cmd/restic/cmd_cat.go @@ -79,7 +79,7 @@ func runCat(gopts GlobalOptions, args []string) error { Println(string(buf)) return nil case "index": - buf, err := repo.LoadUnpacked(gopts.ctx, nil, restic.IndexFile, id) + buf, err := repo.LoadUnpacked(gopts.ctx, restic.IndexFile, id, nil) if err != nil { return err } @@ -87,13 +87,12 @@ func runCat(gopts GlobalOptions, args []string) error { Println(string(buf)) return nil case "snapshot": - sn := &restic.Snapshot{} - err = repo.LoadJSONUnpacked(gopts.ctx, restic.SnapshotFile, id, sn) + sn, err := restic.LoadSnapshot(gopts.ctx, repo, id) if err != nil { return err } - buf, err := json.MarshalIndent(&sn, "", " ") + buf, err := json.MarshalIndent(sn, "", " ") if err != nil { return err } diff --git a/cmd/restic/cmd_copy.go b/cmd/restic/cmd_copy.go index 03b34ecca..b2c2c00ef 100644 --- a/cmd/restic/cmd_copy.go +++ b/cmd/restic/cmd_copy.go @@ -154,7 +154,7 @@ func runCopy(opts CopyOptions, gopts GlobalOptions, args []string) error { if sn.Original == nil { sn.Original = sn.ID() } - newID, err := dstRepo.SaveJSONUnpacked(ctx, restic.SnapshotFile, sn) + newID, err := restic.SaveSnapshot(ctx, dstRepo, sn) if err != nil { return err } diff --git a/cmd/restic/cmd_recover.go b/cmd/restic/cmd_recover.go index f81ef1f10..9f6d2061d 100644 --- a/cmd/restic/cmd_recover.go +++ b/cmd/restic/cmd_recover.go @@ -166,7 +166,7 @@ func createSnapshot(ctx context.Context, name, hostname string, tags []string, r sn.Tree = tree - id, err := repo.SaveJSONUnpacked(ctx, restic.SnapshotFile, sn) + id, err := restic.SaveSnapshot(ctx, repo, sn) if err != nil { return errors.Fatalf("unable to save snapshot: %v", err) } diff --git a/cmd/restic/cmd_tag.go b/cmd/restic/cmd_tag.go index b05cd6e55..1b99a4d56 100644 --- a/cmd/restic/cmd_tag.go +++ b/cmd/restic/cmd_tag.go @@ -82,7 +82,7 @@ func changeTags(ctx context.Context, repo *repository.Repository, sn *restic.Sna } // Save the new snapshot. - id, err := repo.SaveJSONUnpacked(ctx, restic.SnapshotFile, sn) + id, err := restic.SaveSnapshot(ctx, repo, sn) if err != nil { return false, err } diff --git a/cmd/restic/find.go b/cmd/restic/find.go index f93f37278..5107ef599 100644 --- a/cmd/restic/find.go +++ b/cmd/restic/find.go @@ -8,7 +8,7 @@ import ( ) // FindFilteredSnapshots yields Snapshots, either given explicitly by `snapshotIDs` or filtered from the list of all snapshots. -func FindFilteredSnapshots(ctx context.Context, be restic.Lister, loader restic.LoadJSONUnpackeder, hosts []string, tags []restic.TagList, paths []string, snapshotIDs []string) <-chan *restic.Snapshot { +func FindFilteredSnapshots(ctx context.Context, be restic.Lister, loader restic.LoaderUnpacked, hosts []string, tags []restic.TagList, paths []string, snapshotIDs []string) <-chan *restic.Snapshot { out := make(chan *restic.Snapshot) go func() { defer close(out) diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index fe99e3704..0ed66db5d 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -863,7 +863,7 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps } sn.Tree = &rootTreeID - id, err := arch.Repo.SaveJSONUnpacked(ctx, restic.SnapshotFile, sn) + id, err := restic.SaveSnapshot(ctx, arch.Repo, sn) if err != nil { return nil, restic.ID{}, err } diff --git a/internal/checker/checker_test.go b/internal/checker/checker_test.go index 3ab177c7d..975415a87 100644 --- a/internal/checker/checker_test.go +++ b/internal/checker/checker_test.go @@ -519,7 +519,7 @@ func TestCheckerBlobTypeConfusion(t *testing.T) { snapshot.Tree = &rootID - snapID, err := repo.SaveJSONUnpacked(ctx, restic.SnapshotFile, snapshot) + snapID, err := restic.SaveSnapshot(ctx, repo, snapshot) test.OK(t, err) t.Logf("saved snapshot %v", snapID.Str()) @@ -600,8 +600,7 @@ func benchmarkSnapshotScaling(t *testing.B, newSnapshots int) { t.Fatal(err) } - var sn2 restic.Snapshot - err = repo.LoadJSONUnpacked(context.TODO(), restic.SnapshotFile, snID, &sn2) + sn2, err := restic.LoadSnapshot(context.TODO(), repo, snID) if err != nil { t.Fatal(err) } @@ -615,7 +614,7 @@ func benchmarkSnapshotScaling(t *testing.B, newSnapshots int) { } sn.Tree = treeID - _, err = repo.SaveJSONUnpacked(context.TODO(), restic.SnapshotFile, sn) + _, err = restic.SaveSnapshot(context.TODO(), repo, sn) if err != nil { t.Fatal(err) } diff --git a/internal/migrations/upgrade_repo_v2.go b/internal/migrations/upgrade_repo_v2.go index acef8e0fa..b29fbcdcc 100644 --- a/internal/migrations/upgrade_repo_v2.go +++ b/internal/migrations/upgrade_repo_v2.go @@ -68,7 +68,7 @@ func (*UpgradeRepoV2) upgrade(ctx context.Context, repo restic.Repository) error cfg := repo.Config() cfg.Version = 2 - _, err := repo.SaveJSONUnpacked(ctx, restic.ConfigFile, cfg) + err := restic.SaveConfig(ctx, repo, cfg) if err != nil { return fmt.Errorf("save new config file failed: %w", err) } diff --git a/internal/repository/index_parallel.go b/internal/repository/index_parallel.go index 779387385..dcf33113e 100644 --- a/internal/repository/index_parallel.go +++ b/internal/repository/index_parallel.go @@ -52,7 +52,7 @@ func ForAllIndexes(ctx context.Context, repo restic.Repository, var idx *Index oldFormat := false - buf, err = repo.LoadUnpacked(ctx, buf[:0], restic.IndexFile, fi.ID) + buf, err = repo.LoadUnpacked(ctx, restic.IndexFile, fi.ID, buf[:0]) if err == nil { idx, oldFormat, err = DecodeIndex(buf, fi.ID) } diff --git a/internal/repository/repository.go b/internal/repository/repository.go index 863bb9d7b..babc173db 100644 --- a/internal/repository/repository.go +++ b/internal/repository/repository.go @@ -4,7 +4,6 @@ import ( "bufio" "bytes" "context" - "encoding/json" "fmt" "io" "os" @@ -153,7 +152,7 @@ func (r *Repository) PrefixLength(ctx context.Context, t restic.FileType) (int, // LoadUnpacked loads and decrypts the file with the given type and ID, using // the supplied buffer (which must be empty). If the buffer is nil, a new // buffer will be allocated and returned. -func (r *Repository) LoadUnpacked(ctx context.Context, buf []byte, t restic.FileType, id restic.ID) ([]byte, error) { +func (r *Repository) LoadUnpacked(ctx context.Context, t restic.FileType, id restic.ID, buf []byte) ([]byte, error) { if len(buf) != 0 { panic("buf is not empty") } @@ -315,17 +314,6 @@ func (r *Repository) LoadBlob(ctx context.Context, t restic.BlobType, id restic. return nil, errors.Errorf("loading blob %v from %v packs failed", id.Str(), len(blobs)) } -// LoadJSONUnpacked decrypts the data and afterwards calls json.Unmarshal on -// the item. -func (r *Repository) LoadJSONUnpacked(ctx context.Context, t restic.FileType, id restic.ID, item interface{}) (err error) { - buf, err := r.LoadUnpacked(ctx, nil, t, id) - if err != nil { - return err - } - - return json.Unmarshal(buf, item) -} - // LookupBlobSize returns the size of blob id. func (r *Repository) LookupBlobSize(id restic.ID, tpe restic.BlobType) (uint, bool) { return r.idx.LookupSize(restic.BlobHandle{ID: id, Type: tpe}) @@ -419,18 +407,6 @@ func (r *Repository) saveAndEncrypt(ctx context.Context, t restic.BlobType, data return pm.SaveBlob(ctx, t, id, ciphertext, uncompressedLength) } -// SaveJSONUnpacked serialises item as JSON and encrypts and saves it in the -// backend as type t, without a pack. It returns the storage hash. -func (r *Repository) SaveJSONUnpacked(ctx context.Context, t restic.FileType, item interface{}) (restic.ID, error) { - debug.Log("save new blob %v", t) - plaintext, err := json.Marshal(item) - if err != nil { - return restic.ID{}, errors.Wrap(err, "json.Marshal") - } - - return r.SaveUnpacked(ctx, t, plaintext) -} - func (r *Repository) compressUnpacked(p []byte) ([]byte, error) { // compression is only available starting from version 2 if r.cfg.Version < 2 { @@ -762,8 +738,7 @@ func (r *Repository) init(ctx context.Context, password string, cfg restic.Confi r.key = key.master r.keyName = key.Name() r.setConfig(cfg) - _, err = r.SaveJSONUnpacked(ctx, restic.ConfigFile, cfg) - return err + return restic.SaveConfig(ctx, r, cfg) } // Key returns the current master key. diff --git a/internal/repository/repository_test.go b/internal/repository/repository_test.go index 97e7e55e7..c5a3e3b0a 100644 --- a/internal/repository/repository_test.go +++ b/internal/repository/repository_test.go @@ -238,7 +238,7 @@ func benchmarkLoadUnpacked(b *testing.B, version uint) { b.SetBytes(int64(length)) for i := 0; i < b.N; i++ { - data, err := repo.LoadUnpacked(context.TODO(), nil, restic.PackFile, storageID) + data, err := repo.LoadUnpacked(context.TODO(), restic.PackFile, storageID, nil) rtest.OK(b, err) // See comment in BenchmarkLoadBlob. @@ -255,44 +255,6 @@ func benchmarkLoadUnpacked(b *testing.B, version uint) { } } -func TestLoadJSONUnpacked(t *testing.T) { - repository.TestAllVersions(t, testLoadJSONUnpacked) -} - -func testLoadJSONUnpacked(t *testing.T, version uint) { - repo, cleanup := repository.TestRepositoryWithVersion(t, version) - defer cleanup() - - if rtest.BenchArchiveDirectory == "" { - t.Skip("benchdir not set, skipping") - } - - // archive a snapshot - sn := restic.Snapshot{} - sn.Hostname = "foobar" - sn.Username = "test!" - - id, err := repo.SaveJSONUnpacked(context.TODO(), restic.SnapshotFile, &sn) - rtest.OK(t, err) - - var sn2 restic.Snapshot - - // restore - err = repo.LoadJSONUnpacked(context.TODO(), restic.SnapshotFile, id, &sn2) - rtest.OK(t, err) - - rtest.Equals(t, sn.Hostname, sn2.Hostname) - rtest.Equals(t, sn.Username, sn2.Username) - - var cf restic.Config - - // load and check Config - err = repo.LoadJSONUnpacked(context.TODO(), restic.ConfigFile, id, &cf) - rtest.OK(t, err) - - rtest.Equals(t, cf.ChunkerPolynomial, repository.TestChunkerPol) -} - var repoFixture = filepath.Join("testdata", "test-repo.tar.gz") func TestRepositoryLoadIndex(t *testing.T) { @@ -305,7 +267,7 @@ func TestRepositoryLoadIndex(t *testing.T) { // loadIndex loads the index id from backend and returns it. func loadIndex(ctx context.Context, repo restic.Repository, id restic.ID) (*repository.Index, error) { - buf, err := repo.LoadUnpacked(ctx, nil, restic.IndexFile, id) + buf, err := repo.LoadUnpacked(ctx, restic.IndexFile, id, nil) if err != nil { return nil, err } diff --git a/internal/restic/config.go b/internal/restic/config.go index ae4be0aa3..67ee190bc 100644 --- a/internal/restic/config.go +++ b/internal/restic/config.go @@ -76,12 +76,12 @@ func TestDisableCheckPolynomial(t testing.TB) { } // LoadConfig returns loads, checks and returns the config for a repository. -func LoadConfig(ctx context.Context, r JSONUnpackedLoader) (Config, error) { +func LoadConfig(ctx context.Context, r LoaderUnpacked) (Config, error) { var ( cfg Config ) - err := r.LoadJSONUnpacked(ctx, ConfigFile, ID{}, &cfg) + err := LoadJSONUnpacked(ctx, r, ConfigFile, ID{}, &cfg) if err != nil { return Config{}, err } @@ -98,3 +98,8 @@ func LoadConfig(ctx context.Context, r JSONUnpackedLoader) (Config, error) { return cfg, nil } + +func SaveConfig(ctx context.Context, r SaverUnpacked, cfg Config) error { + _, err := SaveJSONUnpacked(ctx, r, ConfigFile, cfg) + return err +} diff --git a/internal/restic/config_test.go b/internal/restic/config_test.go index fd8e4aeed..662a2e69e 100644 --- a/internal/restic/config_test.go +++ b/internal/restic/config_test.go @@ -8,47 +8,56 @@ import ( rtest "github.com/restic/restic/internal/test" ) -type saver func(restic.FileType, interface{}) (restic.ID, error) - -func (s saver) SaveJSONUnpacked(t restic.FileType, arg interface{}) (restic.ID, error) { - return s(t, arg) +type saver struct { + fn func(restic.FileType, []byte) (restic.ID, error) } -type loader func(context.Context, restic.FileType, restic.ID, interface{}) error +func (s saver) SaveUnpacked(ctx context.Context, t restic.FileType, buf []byte) (restic.ID, error) { + return s.fn(t, buf) +} -func (l loader) LoadJSONUnpacked(ctx context.Context, t restic.FileType, id restic.ID, arg interface{}) error { - return l(ctx, t, id, arg) +func (s saver) Connections() uint { + return 2 +} + +type loader struct { + fn func(restic.FileType, restic.ID, []byte) ([]byte, error) +} + +func (l loader) LoadUnpacked(ctx context.Context, t restic.FileType, id restic.ID, buf []byte) (data []byte, err error) { + return l.fn(t, id, buf) +} + +func (l loader) Connections() uint { + return 2 } func TestConfig(t *testing.T) { - resultConfig := restic.Config{} - save := func(tpe restic.FileType, arg interface{}) (restic.ID, error) { + var resultBuf []byte + save := func(tpe restic.FileType, buf []byte) (restic.ID, error) { rtest.Assert(t, tpe == restic.ConfigFile, "wrong backend type: got %v, wanted %v", tpe, restic.ConfigFile) - cfg := arg.(restic.Config) - resultConfig = cfg + resultBuf = buf return restic.ID{}, nil } cfg1, err := restic.CreateConfig(restic.MaxRepoVersion) rtest.OK(t, err) - _, err = saver(save).SaveJSONUnpacked(restic.ConfigFile, cfg1) + err = restic.SaveConfig(context.TODO(), saver{save}, cfg1) rtest.OK(t, err) - load := func(ctx context.Context, tpe restic.FileType, id restic.ID, arg interface{}) error { + load := func(tpe restic.FileType, id restic.ID, in []byte) ([]byte, error) { rtest.Assert(t, tpe == restic.ConfigFile, "wrong backend type: got %v, wanted %v", tpe, restic.ConfigFile) - cfg := arg.(*restic.Config) - *cfg = resultConfig - return nil + return resultBuf, nil } - cfg2, err := restic.LoadConfig(context.TODO(), loader(load)) + cfg2, err := restic.LoadConfig(context.TODO(), loader{load}) rtest.OK(t, err) rtest.Assert(t, cfg1 == cfg2, diff --git a/internal/restic/json.go b/internal/restic/json.go new file mode 100644 index 000000000..6ad4b5f39 --- /dev/null +++ b/internal/restic/json.go @@ -0,0 +1,32 @@ +package restic + +import ( + "context" + "encoding/json" + + "github.com/restic/restic/internal/debug" + "github.com/restic/restic/internal/errors" +) + +// LoadJSONUnpacked decrypts the data and afterwards calls json.Unmarshal on +// the item. +func LoadJSONUnpacked(ctx context.Context, repo LoaderUnpacked, t FileType, id ID, item interface{}) (err error) { + buf, err := repo.LoadUnpacked(ctx, t, id, nil) + if err != nil { + return err + } + + return json.Unmarshal(buf, item) +} + +// SaveJSONUnpacked serialises item as JSON and encrypts and saves it in the +// backend as type t, without a pack. It returns the storage hash. +func SaveJSONUnpacked(ctx context.Context, repo SaverUnpacked, t FileType, item interface{}) (ID, error) { + debug.Log("save new blob %v", t) + plaintext, err := json.Marshal(item) + if err != nil { + return ID{}, errors.Wrap(err, "json.Marshal") + } + + return repo.SaveUnpacked(ctx, t, plaintext) +} diff --git a/internal/restic/lock.go b/internal/restic/lock.go index 8411d5ad5..3f233483f 100644 --- a/internal/restic/lock.go +++ b/internal/restic/lock.go @@ -158,7 +158,7 @@ func (l *Lock) checkForOtherLocks(ctx context.Context) error { // createLock acquires the lock by creating a file in the repository. func (l *Lock) createLock(ctx context.Context) (ID, error) { - id, err := l.repo.SaveJSONUnpacked(ctx, LockFile, l) + id, err := SaveJSONUnpacked(ctx, l.repo, LockFile, l) if err != nil { return ID{}, err } @@ -255,7 +255,7 @@ func init() { // LoadLock loads and unserializes a lock from a repository. func LoadLock(ctx context.Context, repo Repository, id ID) (*Lock, error) { lock := &Lock{} - if err := repo.LoadJSONUnpacked(ctx, LockFile, id, lock); err != nil { + if err := LoadJSONUnpacked(ctx, repo, LockFile, id, lock); err != nil { return nil, err } lock.lockID = &id diff --git a/internal/restic/lock_test.go b/internal/restic/lock_test.go index 09dce3c78..b92eace70 100644 --- a/internal/restic/lock_test.go +++ b/internal/restic/lock_test.go @@ -99,7 +99,7 @@ func createFakeLock(repo restic.Repository, t time.Time, pid int) (restic.ID, er } newLock := &restic.Lock{Time: t, PID: pid, Hostname: hostname} - return repo.SaveJSONUnpacked(context.TODO(), restic.LockFile, &newLock) + return restic.SaveJSONUnpacked(context.TODO(), repo, restic.LockFile, &newLock) } func removeLock(repo restic.Repository, id restic.ID) error { diff --git a/internal/restic/repository.go b/internal/restic/repository.go index 996f11260..2bf12503f 100644 --- a/internal/restic/repository.go +++ b/internal/restic/repository.go @@ -37,24 +37,20 @@ type Repository interface { // the the pack header. ListPack(context.Context, ID, int64) ([]Blob, uint32, error) + LoadBlob(context.Context, BlobType, ID, []byte) ([]byte, error) + SaveBlob(context.Context, BlobType, []byte, ID, bool) (ID, bool, int, error) + // StartPackUploader start goroutines to upload new pack files. The errgroup // is used to immediately notify about an upload error. Flush() will also return // that error. StartPackUploader(ctx context.Context, wg *errgroup.Group) Flush(context.Context) error - SaveJSONUnpacked(context.Context, FileType, interface{}) (ID, error) - - LoadJSONUnpacked(ctx context.Context, t FileType, id ID, dest interface{}) error - // LoadUnpacked loads and decrypts the file with the given type and ID, // using the supplied buffer (which must be empty). If the buffer is nil, a // new buffer will be allocated and returned. - LoadUnpacked(ctx context.Context, buf []byte, t FileType, id ID) (data []byte, err error) + LoadUnpacked(ctx context.Context, t FileType, id ID, buf []byte) (data []byte, err error) SaveUnpacked(context.Context, FileType, []byte) (ID, error) - - LoadBlob(context.Context, BlobType, ID, []byte) ([]byte, error) - SaveBlob(context.Context, BlobType, []byte, ID, bool) (ID, bool, int, error) } // Lister allows listing files in a backend. @@ -62,16 +58,11 @@ type Lister interface { List(context.Context, FileType, func(FileInfo) error) error } -// LoadJSONUnpackeder allows loading a JSON file not stored in a pack file -type LoadJSONUnpackeder interface { - // Connections returns the maximum number of concurrent backend operations - Connections() uint - LoadJSONUnpacked(ctx context.Context, t FileType, id ID, dest interface{}) error -} - // LoaderUnpacked allows loading a blob not stored in a pack file type LoaderUnpacked interface { - LoadUnpacked(ctx context.Context, buf []byte, t FileType, id ID) (data []byte, err error) + // Connections returns the maximum number of concurrent backend operations + Connections() uint + LoadUnpacked(ctx context.Context, t FileType, id ID, buf []byte) (data []byte, err error) } // SaverUnpacked allows saving a blob not stored in a pack file diff --git a/internal/restic/snapshot.go b/internal/restic/snapshot.go index 1652d85d8..b12548459 100644 --- a/internal/restic/snapshot.go +++ b/internal/restic/snapshot.go @@ -59,9 +59,9 @@ func NewSnapshot(paths []string, tags []string, hostname string, time time.Time) } // LoadSnapshot loads the snapshot with the id and returns it. -func LoadSnapshot(ctx context.Context, loader LoadJSONUnpackeder, id ID) (*Snapshot, error) { +func LoadSnapshot(ctx context.Context, loader LoaderUnpacked, id ID) (*Snapshot, error) { sn := &Snapshot{id: &id} - err := loader.LoadJSONUnpacked(ctx, SnapshotFile, id, sn) + err := LoadJSONUnpacked(ctx, loader, SnapshotFile, id, sn) if err != nil { return nil, err } @@ -69,12 +69,17 @@ func LoadSnapshot(ctx context.Context, loader LoadJSONUnpackeder, id ID) (*Snaps return sn, nil } +// SaveSnapshot saves the snapshot sn and returns its ID. +func SaveSnapshot(ctx context.Context, repo SaverUnpacked, sn *Snapshot) (ID, error) { + return SaveJSONUnpacked(ctx, repo, SnapshotFile, sn) +} + // ForAllSnapshots reads all snapshots in parallel and calls the // given function. It is guaranteed that the function is not run concurrently. // If the called function returns an error, this function is cancelled and // also returns this error. // If a snapshot ID is in excludeIDs, it will be ignored. -func ForAllSnapshots(ctx context.Context, be Lister, loader LoadJSONUnpackeder, excludeIDs IDSet, fn func(ID, *Snapshot, error) error) error { +func ForAllSnapshots(ctx context.Context, be Lister, loader LoaderUnpacked, excludeIDs IDSet, fn func(ID, *Snapshot, error) error) error { var m sync.Mutex // track spawned goroutines using wg, create a new context which is diff --git a/internal/restic/snapshot_find.go b/internal/restic/snapshot_find.go index 74275b552..49f9d62bf 100644 --- a/internal/restic/snapshot_find.go +++ b/internal/restic/snapshot_find.go @@ -14,7 +14,7 @@ import ( var ErrNoSnapshotFound = errors.New("no snapshot found") // FindLatestSnapshot finds latest snapshot with optional target/directory, tags, hostname, and timestamp filters. -func FindLatestSnapshot(ctx context.Context, be Lister, loader LoadJSONUnpackeder, targets []string, +func FindLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, targets []string, tagLists []TagList, hostnames []string, timeStampLimit *time.Time) (ID, error) { var err error @@ -92,7 +92,7 @@ func FindSnapshot(ctx context.Context, be Lister, s string) (ID, error) { // FindFilteredSnapshots yields Snapshots filtered from the list of all // snapshots. -func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoadJSONUnpackeder, hosts []string, tags []TagList, paths []string) (Snapshots, error) { +func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string) (Snapshots, error) { results := make(Snapshots, 0, 20) err := ForAllSnapshots(ctx, be, loader, nil, func(id ID, sn *Snapshot, err error) error { diff --git a/internal/restic/snapshot_test.go b/internal/restic/snapshot_test.go index efcc00960..96325debf 100644 --- a/internal/restic/snapshot_test.go +++ b/internal/restic/snapshot_test.go @@ -1,9 +1,11 @@ package restic_test import ( + "context" "testing" "time" + "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" ) @@ -24,3 +26,27 @@ func TestTagList(t *testing.T) { r := sn.HasTags(tags) rtest.Assert(t, r, "Failed to match untagged snapshot") } + +func TestLoadJSONUnpacked(t *testing.T) { + repository.TestAllVersions(t, testLoadJSONUnpacked) +} + +func testLoadJSONUnpacked(t *testing.T, version uint) { + repo, cleanup := repository.TestRepositoryWithVersion(t, version) + defer cleanup() + + // archive a snapshot + sn := restic.Snapshot{} + sn.Hostname = "foobar" + sn.Username = "test!" + + id, err := restic.SaveSnapshot(context.TODO(), repo, &sn) + rtest.OK(t, err) + + // restore + sn2, err := restic.LoadSnapshot(context.TODO(), repo, id) + rtest.OK(t, err) + + rtest.Equals(t, sn.Hostname, sn2.Hostname) + rtest.Equals(t, sn.Username, sn2.Username) +} diff --git a/internal/restic/testing.go b/internal/restic/testing.go index cc5e7d890..79bc0813a 100644 --- a/internal/restic/testing.go +++ b/internal/restic/testing.go @@ -181,7 +181,7 @@ func TestCreateSnapshot(t testing.TB, repo Repository, at time.Time, depth int, t.Fatal(err) } - id, err := repo.SaveJSONUnpacked(context.TODO(), SnapshotFile, snapshot) + id, err := SaveSnapshot(context.TODO(), repo, snapshot) if err != nil { t.Fatal(err) } diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index 441f28b41..2eea1a6fd 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -137,7 +137,7 @@ func saveSnapshot(t testing.TB, repo restic.Repository, snapshot Snapshot) (*res } sn.Tree = &treeID - id, err := repo.SaveJSONUnpacked(ctx, restic.SnapshotFile, sn) + id, err := restic.SaveSnapshot(ctx, repo, sn) if err != nil { t.Fatal(err) }