restic: Change FindSnapshot functions to return the snapshot

internal/archiver/archiver.go

@@ -676,24 +676,17 @@ type SnapshotOptions struct {
 	Hostname       string
 	Excludes       []string
 	Time           time.Time
-	ParentSnapshot restic.ID
+	ParentSnapshot *restic.Snapshot
 }
 
 // loadParentTree loads a tree referenced by snapshot id. If id is null, nil is returned.
-func (arch *Archiver) loadParentTree(ctx context.Context, snapshotID restic.ID) *restic.Tree {
-	if snapshotID.IsNull() {
-		return nil
-	}
-
-	debug.Log("load parent snapshot %v", snapshotID)
-	sn, err := restic.LoadSnapshot(ctx, arch.Repo, snapshotID)
-	if err != nil {
-		debug.Log("unable to load snapshot %v: %v", snapshotID, err)
+func (arch *Archiver) loadParentTree(ctx context.Context, sn *restic.Snapshot) *restic.Tree {
+	if sn == nil {
 		return nil
 	}
 
 	if sn.Tree == nil {
-		debug.Log("snapshot %v has empty tree %v", snapshotID)
+		debug.Log("snapshot %v has empty tree %v", *sn.ID())
 		return nil
 	}
 
@@ -801,9 +794,8 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps
 	}
 
 	sn.Excludes = opts.Excludes
-	if !opts.ParentSnapshot.IsNull() {
-		id := opts.ParentSnapshot
-		sn.Parent = &id
+	if opts.ParentSnapshot != nil {
+		sn.Parent = opts.ParentSnapshot.ID()
 	}
 	sn.Tree = &rootTreeID
 

internal/archiver/archiver_test.go

@@ -1662,7 +1662,7 @@ func TestArchiverParent(t *testing.T) {
 			back := restictest.Chdir(t, tempdir)
 			defer back()
 
-			_, firstSnapshotID, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()})
+			firstSnapshot, firstSnapshotID, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1690,7 +1690,7 @@ func TestArchiverParent(t *testing.T) {
 
 			opts := SnapshotOptions{
 				Time:           time.Now(),
-				ParentSnapshot: firstSnapshotID,
+				ParentSnapshot: firstSnapshot,
 			}
 			_, secondSnapshotID, err := arch.Snapshot(ctx, []string{"."}, opts)
 			if err != nil {
@@ -2063,7 +2063,7 @@ func TestArchiverAbortEarlyOnError(t *testing.T) {
 	}
 }
 
-func snapshot(t testing.TB, repo restic.Repository, fs fs.FS, parent restic.ID, filename string) (restic.ID, *restic.Node) {
+func snapshot(t testing.TB, repo restic.Repository, fs fs.FS, parent *restic.Snapshot, filename string) (*restic.Snapshot, *restic.Node) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
@@ -2073,7 +2073,7 @@ func snapshot(t testing.TB, repo restic.Repository, fs fs.FS, parent restic.ID,
 		Time:           time.Now(),
 		ParentSnapshot: parent,
 	}
-	snapshot, snapshotID, err := arch.Snapshot(ctx, []string{filename}, sopts)
+	snapshot, _, err := arch.Snapshot(ctx, []string{filename}, sopts)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -2088,7 +2088,7 @@ func snapshot(t testing.TB, repo restic.Repository, fs fs.FS, parent restic.ID,
 		t.Fatalf("unable to find node for testfile in snapshot")
 	}
 
-	return snapshotID, node
+	return snapshot, node
 }
 
 // StatFS allows overwriting what is returned by the Lstat function.
@@ -2170,7 +2170,7 @@ func TestMetadataChanged(t *testing.T) {
 		},
 	}
 
-	snapshotID, node2 := snapshot(t, repo, fs, restic.ID{}, "testfile")
+	sn, node2 := snapshot(t, repo, fs, nil, "testfile")
 
 	// set some values so we can then compare the nodes
 	want.Content = node2.Content
@@ -2201,7 +2201,7 @@ func TestMetadataChanged(t *testing.T) {
 	want.Group = ""
 
 	// make another snapshot
-	_, node3 := snapshot(t, repo, fs, snapshotID, "testfile")
+	_, node3 := snapshot(t, repo, fs, sn, "testfile")
 	// Override username and group to empty string - in case underlying system has user with UID 51234
 	// See https://github.com/restic/restic/issues/2372
 	node3.User = ""

internal/archiver/testing.go

@@ -26,7 +26,11 @@ func TestSnapshot(t testing.TB, repo restic.Repository, path string, parent *res
 		Tags:     []string{"test"},
 	}
 	if parent != nil {
-		opts.ParentSnapshot = *parent
+		sn, err := restic.LoadSnapshot(context.TODO(), arch.Repo, *parent)
+		if err != nil {
+			t.Fatal(err)
+		}
+		opts.ParentSnapshot = sn
 	}
 	sn, _, err := arch.Snapshot(context.TODO(), []string{path}, opts)
 	if err != nil {

internal/checker/checker_test.go

@@ -592,11 +592,7 @@ func benchmarkSnapshotScaling(t *testing.B, newSnapshots int) {
 	chkr, repo, cleanup := loadBenchRepository(t)
 	defer cleanup()
 
-	snID, err := restic.FindSnapshot(context.TODO(), repo.Backend(), "51d249d2")
-	if err != nil {
-		t.Fatal(err)
-	}
-
+	snID := restic.TestParseID("51d249d28815200d59e4be7b3f21a157b864dc343353df9d8e498220c2499b02")
 	sn2, err := restic.LoadSnapshot(context.TODO(), repo, snID)
 	if err != nil {
 		t.Fatal(err)

internal/restic/snapshot_find.go

@@ -13,8 +13,8 @@ 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 LoaderUnpacked,
-	hosts []string, tags []TagList, paths []string, timeStampLimit *time.Time) (ID, error) {
+func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string,
+	tags []TagList, paths []string, timeStampLimit *time.Time) (*Snapshot, error) {
 
 	var err error
 	absTargets := make([]string, 0, len(paths))
@@ -22,17 +22,13 @@ func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked,
 		if !filepath.IsAbs(target) {
 			target, err = filepath.Abs(target)
 			if err != nil {
-				return ID{}, errors.Wrap(err, "Abs")
+				return nil, errors.Wrap(err, "Abs")
 			}
 		}
 		absTargets = append(absTargets, filepath.Clean(target))
 	}
 
-	var (
-		latest   time.Time
-		latestID ID
-		found    bool
-	)
+	var latest *Snapshot
 
 	err = ForAllSnapshots(ctx, be, loader, nil, func(id ID, snapshot *Snapshot, err error) error {
 		if err != nil {
@@ -43,7 +39,7 @@ func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked,
 			return nil
 		}
 
-		if snapshot.Time.Before(latest) {
+		if latest != nil && snapshot.Time.Before(latest.Time) {
 			return nil
 		}
 
@@ -59,50 +55,47 @@ func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked,
 			return nil
 		}
 
-		latest = snapshot.Time
-		latestID = id
-		found = true
+		latest = snapshot
 		return nil
 	})
 
 	if err != nil {
-		return ID{}, err
+		return nil, err
 	}
 
-	if !found {
-		return ID{}, ErrNoSnapshotFound
+	if latest == nil {
+		return nil, ErrNoSnapshotFound
 	}
 
-	return latestID, nil
+	return latest, nil
 }
 
 // FindSnapshot takes a string and tries to find a snapshot whose ID matches
 // the string as closely as possible.
-func FindSnapshot(ctx context.Context, be Lister, s string) (ID, error) {
-
+func FindSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, s string) (*Snapshot, error) {
 	// find snapshot id with prefix
 	name, err := Find(ctx, be, SnapshotFile, s)
 	if err != nil {
-		return ID{}, err
+		return nil, err
 	}
 
-	return ParseID(name)
+	id, err := ParseID(name)
+	if err != nil {
+		return nil, err
+	}
+	return LoadSnapshot(ctx, loader, id)
 }
 
-func FindFilteredSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string, timeStampLimit *time.Time, snapshotID string) (ID, error) {
+// FindFilteredSnapshot returns either the latests from a filtered list of all snapshots or a snapshot specified by `snapshotID`.
+func FindFilteredSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string, timeStampLimit *time.Time, snapshotID string) (*Snapshot, error) {
 	if snapshotID == "latest" {
-		id, err := findLatestSnapshot(ctx, be, loader, hosts, tags, paths, timeStampLimit)
+		sn, err := findLatestSnapshot(ctx, be, loader, hosts, tags, paths, timeStampLimit)
 		if err == ErrNoSnapshotFound {
 			err = fmt.Errorf("snapshot filter (Paths:%v Tags:%v Hosts:%v): %w", paths, tags, hosts, err)
 		}
-		return id, err
-	} else {
-		id, err := FindSnapshot(ctx, be, snapshotID)
-		if err != nil {
-			return ID{}, err
-		}
-		return id, err
+		return sn, err
 	}
+	return FindSnapshot(ctx, be, loader, snapshotID)
 }
 
 type SnapshotFindCb func(string, *Snapshot, error) error
@@ -116,7 +109,7 @@ func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoaderUnpacked
 		ids := NewIDSet()
 		// Process all snapshot IDs given as arguments.
 		for _, s := range snapshotIDs {
-			var id ID
+			var sn *Snapshot
 			if s == "latest" {
 				if usedFilter {
 					continue
@@ -124,23 +117,24 @@ func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoaderUnpacked
 
 				usedFilter = true
 
-				id, err = findLatestSnapshot(ctx, be, loader, hosts, tags, paths, nil)
+				sn, err = findLatestSnapshot(ctx, be, loader, hosts, tags, paths, nil)
 				if err == ErrNoSnapshotFound {
 					err = errors.Errorf("no snapshot matched given filter (Paths:%v Tags:%v Hosts:%v)", paths, tags, hosts)
 				}
+				if sn != nil {
+					ids.Insert(*sn.ID())
+				}
 			} else {
-				id, err = FindSnapshot(ctx, be, s)
+				sn, err = FindSnapshot(ctx, be, loader, s)
+				if err == nil {
+					if ids.Has(*sn.ID()) {
+						continue
+					} else {
+						ids.Insert(*sn.ID())
+						s = sn.ID().String()
+					}
+				}
 			}
-
-			var sn *Snapshot
-			if ids.Has(id) {
-				continue
-			} else if !id.IsNull() {
-				ids.Insert(id)
-				sn, err = LoadSnapshot(ctx, loader, id)
-				s = id.String()
-			}
-
 			err = fn(s, sn, err)
 			if err != nil {
 				return err

internal/restic/snapshot_find_test.go

@@ -16,13 +16,13 @@ func TestFindLatestSnapshot(t *testing.T) {
 	restic.TestCreateSnapshot(t, repo, parseTimeUTC("2017-07-07 07:07:07"), 1, 0)
 	latestSnapshot := restic.TestCreateSnapshot(t, repo, parseTimeUTC("2019-09-09 09:09:09"), 1, 0)
 
-	id, err := restic.FindFilteredSnapshot(context.TODO(), repo.Backend(), repo, []string{"foo"}, []restic.TagList{}, []string{}, nil, "latest")
+	sn, err := restic.FindFilteredSnapshot(context.TODO(), repo.Backend(), repo, []string{"foo"}, []restic.TagList{}, []string{}, nil, "latest")
 	if err != nil {
 		t.Fatalf("FindLatestSnapshot returned error: %v", err)
 	}
 
-	if id != *latestSnapshot.ID() {
-		t.Errorf("FindLatestSnapshot returned wrong snapshot ID: %v", id)
+	if *sn.ID() != *latestSnapshot.ID() {
+		t.Errorf("FindLatestSnapshot returned wrong snapshot ID: %v", *sn.ID())
 	}
 }
 
@@ -36,12 +36,12 @@ func TestFindLatestSnapshotWithMaxTimestamp(t *testing.T) {
 
 	maxTimestamp := parseTimeUTC("2018-08-08 08:08:08")
 
-	id, err := restic.FindFilteredSnapshot(context.TODO(), repo.Backend(), repo, []string{"foo"}, []restic.TagList{}, []string{}, &maxTimestamp, "latest")
+	sn, err := restic.FindFilteredSnapshot(context.TODO(), repo.Backend(), repo, []string{"foo"}, []restic.TagList{}, []string{}, &maxTimestamp, "latest")
 	if err != nil {
 		t.Fatalf("FindLatestSnapshot returned error: %v", err)
 	}
 
-	if id != *desiredSnapshot.ID() {
-		t.Errorf("FindLatestSnapshot returned wrong snapshot ID: %v", id)
+	if *sn.ID() != *desiredSnapshot.ID() {
+		t.Errorf("FindLatestSnapshot returned wrong snapshot ID: %v", *sn.ID())
 	}
 }

internal/restorer/restorer.go

@@ -27,22 +27,16 @@ type Restorer struct {
 var restorerAbortOnAllErrors = func(location string, err error) error { return err }
 
 // NewRestorer creates a restorer preloaded with the content from the snapshot id.
-func NewRestorer(ctx context.Context, repo restic.Repository, id restic.ID, sparse bool) (*Restorer, error) {
+func NewRestorer(ctx context.Context, repo restic.Repository, sn *restic.Snapshot, sparse bool) *Restorer {
 	r := &Restorer{
 		repo:         repo,
 		sparse:       sparse,
 		Error:        restorerAbortOnAllErrors,
 		SelectFilter: func(string, string, *restic.Node) (bool, bool) { return true, true },
+		sn:           sn,
 	}
 
-	var err error
-
-	r.sn, err = restic.LoadSnapshot(ctx, repo, id)
-	if err != nil {
-		return nil, err
-	}
-
-	return r, nil
+	return r
 }
 
 type treeVisitor struct {

internal/restorer/restorer_test.go

@@ -323,13 +323,10 @@ func TestRestorer(t *testing.T) {
 		t.Run("", func(t *testing.T) {
 			repo, cleanup := repository.TestRepository(t)
 			defer cleanup()
-			_, id := saveSnapshot(t, repo, test.Snapshot)
+			sn, id := saveSnapshot(t, repo, test.Snapshot)
 			t.Logf("snapshot saved as %v", id.Str())
 
-			res, err := NewRestorer(context.TODO(), repo, id, false)
-			if err != nil {
-				t.Fatal(err)
-			}
+			res := NewRestorer(context.TODO(), repo, sn, false)
 
 			tempdir, cleanup := rtest.TempDir(t)
 			defer cleanup()
@@ -366,7 +363,7 @@ func TestRestorer(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 
-			err = res.RestoreTo(ctx, tempdir)
+			err := res.RestoreTo(ctx, tempdir)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -446,13 +443,10 @@ func TestRestorerRelative(t *testing.T) {
 			repo, cleanup := repository.TestRepository(t)
 			defer cleanup()
 
-			_, id := saveSnapshot(t, repo, test.Snapshot)
+			sn, id := saveSnapshot(t, repo, test.Snapshot)
 			t.Logf("snapshot saved as %v", id.Str())
 
-			res, err := NewRestorer(context.TODO(), repo, id, false)
-			if err != nil {
-				t.Fatal(err)
-			}
+			res := NewRestorer(context.TODO(), repo, sn, false)
 
 			tempdir, cleanup := rtest.TempDir(t)
 			defer cleanup()
@@ -470,7 +464,7 @@ func TestRestorerRelative(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.Background())
 			defer cancel()
 
-			err = res.RestoreTo(ctx, "restore")
+			err := res.RestoreTo(ctx, "restore")
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -682,12 +676,9 @@ func TestRestorerTraverseTree(t *testing.T) {
 		t.Run("", func(t *testing.T) {
 			repo, cleanup := repository.TestRepository(t)
 			defer cleanup()
-			sn, id := saveSnapshot(t, repo, test.Snapshot)
+			sn, _ := saveSnapshot(t, repo, test.Snapshot)
 
-			res, err := NewRestorer(context.TODO(), repo, id, false)
-			if err != nil {
-				t.Fatal(err)
-			}
+			res := NewRestorer(context.TODO(), repo, sn, false)
 
 			res.SelectFilter = test.Select
 
@@ -700,7 +691,7 @@ func TestRestorerTraverseTree(t *testing.T) {
 			// make sure we're creating a new subdir of the tempdir
 			target := filepath.Join(tempdir, "target")
 
-			_, err = res.traverseTree(ctx, target, string(filepath.Separator), *sn.Tree, test.Visitor(t))
+			_, err := res.traverseTree(ctx, target, string(filepath.Separator), *sn.Tree, test.Visitor(t))
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -735,7 +726,7 @@ func TestRestorerConsistentTimestampsAndPermissions(t *testing.T) {
 	repo, cleanup := repository.TestRepository(t)
 	defer cleanup()
 
-	_, id := saveSnapshot(t, repo, Snapshot{
+	sn, _ := saveSnapshot(t, repo, Snapshot{
 		Nodes: map[string]Node{
 			"dir": Dir{
 				Mode:    normalizeFileMode(0750 | os.ModeDir),
@@ -766,8 +757,7 @@ func TestRestorerConsistentTimestampsAndPermissions(t *testing.T) {
 		},
 	})
 
-	res, err := NewRestorer(context.TODO(), repo, id, false)
-	rtest.OK(t, err)
+	res := NewRestorer(context.TODO(), repo, sn, false)
 
 	res.SelectFilter = func(item string, dstpath string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) {
 		switch filepath.ToSlash(item) {
@@ -792,7 +782,7 @@ func TestRestorerConsistentTimestampsAndPermissions(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	err = res.RestoreTo(ctx, tempdir)
+	err := res.RestoreTo(ctx, tempdir)
 	rtest.OK(t, err)
 
 	var testPatterns = []struct {
@@ -824,10 +814,9 @@ func TestVerifyCancel(t *testing.T) {
 	repo, cleanup := repository.TestRepository(t)
 	defer cleanup()
 
-	_, id := saveSnapshot(t, repo, snapshot)
+	sn, _ := saveSnapshot(t, repo, snapshot)
 
-	res, err := NewRestorer(context.TODO(), repo, id, false)
-	rtest.OK(t, err)
+	res := NewRestorer(context.TODO(), repo, sn, false)
 
 	tempdir, cleanup := rtest.TempDir(t)
 	defer cleanup()
@@ -836,7 +825,7 @@ func TestVerifyCancel(t *testing.T) {
 	defer cancel()
 
 	rtest.OK(t, res.RestoreTo(ctx, tempdir))
-	err = ioutil.WriteFile(filepath.Join(tempdir, "foo"), []byte("bar"), 0644)
+	err := ioutil.WriteFile(filepath.Join(tempdir, "foo"), []byte("bar"), 0644)
 	rtest.OK(t, err)
 
 	var errs []error
@@ -868,12 +857,11 @@ func TestRestorerSparseFiles(t *testing.T) {
 	rtest.OK(t, err)
 
 	arch := archiver.New(repo, target, archiver.Options{})
-	_, id, err := arch.Snapshot(context.Background(), []string{"/zeros"},
+	sn, _, err := arch.Snapshot(context.Background(), []string{"/zeros"},
 		archiver.SnapshotOptions{})
 	rtest.OK(t, err)
 
-	res, err := NewRestorer(context.TODO(), repo, id, true)
-	rtest.OK(t, err)
+	res := NewRestorer(context.TODO(), repo, sn, true)
 
 	tempdir, cleanup := rtest.TempDir(t)
 	defer cleanup()

internal/restorer/restorer_unix_test.go

@@ -19,7 +19,7 @@ func TestRestorerRestoreEmptyHardlinkedFileds(t *testing.T) {
 	repo, cleanup := repository.TestRepository(t)
 	defer cleanup()
 
-	_, id := saveSnapshot(t, repo, Snapshot{
+	sn, _ := saveSnapshot(t, repo, Snapshot{
 		Nodes: map[string]Node{
 			"dirtest": Dir{
 				Nodes: map[string]Node{
@@ -30,8 +30,7 @@ func TestRestorerRestoreEmptyHardlinkedFileds(t *testing.T) {
 		},
 	})
 
-	res, err := NewRestorer(context.TODO(), repo, id, false)
-	rtest.OK(t, err)
+	res := NewRestorer(context.TODO(), repo, sn, false)
 
 	res.SelectFilter = func(item string, dstpath string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) {
 		return true, true
@@ -43,7 +42,7 @@ func TestRestorerRestoreEmptyHardlinkedFileds(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	err = res.RestoreTo(ctx, tempdir)
+	err := res.RestoreTo(ctx, tempdir)
 	rtest.OK(t, err)
 
 	f1, err := os.Stat(filepath.Join(tempdir, "dirtest/file1"))