Merge pull request #5601 from MichaelEischer/snapshots-fix-groupby-with-latest

snapshots: correctly handle --latest in combination with --group-by

changelog/unreleased/issue-5586

@@ -0,0 +1,7 @@
+Bugfix: correctly handle `snapshots --group-by` in combination with `--latest`
+
+For the `snapshots` command, the `--latest` option did not correctly handle the
+case where an non-default value was passed to `--group-by`. This has been fixed.
+
+https://github.com/restic/restic/issues/5586
+https://github.com/restic/restic/pull/5601

cmd/restic/cmd_snapshots.go

@@ -97,9 +97,9 @@ func runSnapshots(ctx context.Context, opts SnapshotOptions, gopts global.Option
 		if opts.Last {
 			// This branch should be removed in the same time
 			// that --last.
-			list = FilterLatestSnapshots(list, 1)
+			list = filterLatestSnapshotsInGroup(list, 1)
 		} else if opts.Latest > 0 {
-			list = FilterLatestSnapshots(list, opts.Latest)
+			list = filterLatestSnapshotsInGroup(list, opts.Latest)
 		}
 		sort.Sort(sort.Reverse(list))
 		snapshotGroups[k] = list
@@ -133,41 +133,16 @@ func runSnapshots(ctx context.Context, opts SnapshotOptions, gopts global.Option
 	return nil
 }
 
-// filterLastSnapshotsKey is used by FilterLastSnapshots.
-type filterLastSnapshotsKey struct {
-	Hostname    string
-	JoinedPaths string
-}
-
-// newFilterLastSnapshotsKey initializes a filterLastSnapshotsKey from a Snapshot
-func newFilterLastSnapshotsKey(sn *data.Snapshot) filterLastSnapshotsKey {
-	// Shallow slice copy
-	var paths = make([]string, len(sn.Paths))
-	copy(paths, sn.Paths)
-	sort.Strings(paths)
-	return filterLastSnapshotsKey{sn.Hostname, strings.Join(paths, "|")}
-}
-
-// FilterLatestSnapshots filters a list of snapshots to only return
-// the limit last entries for each hostname and path. If the snapshot
-// contains multiple paths, they will be joined and treated as one
-// item.
-func FilterLatestSnapshots(list data.Snapshots, limit int) data.Snapshots {
+// filterLatestSnapshotsInGroup filters a list of snapshots to only return
+// the `limit` last entries. It is assumed that the snapshot list only contains
+// one group of snapshots.
+func filterLatestSnapshotsInGroup(list data.Snapshots, limit int) data.Snapshots {
 	// Sort the snapshots so that the newer ones are listed first
 	sort.SliceStable(list, func(i, j int) bool {
 		return list[i].Time.After(list[j].Time)
 	})
 
-	var results data.Snapshots
-	seen := make(map[filterLastSnapshotsKey]int)
-	for _, sn := range list {
-		key := newFilterLastSnapshotsKey(sn)
-		if seen[key] < limit {
-			seen[key]++
-			results = append(results, sn)
-		}
-	}
-	return results
+	return list[:min(limit, len(list))]
 }
 
 // PrintSnapshots prints a text table of the snapshots in list to stdout.

cmd/restic/cmd_snapshots_integration_test.go

@@ -3,8 +3,11 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"path/filepath"
 	"testing"
+	"time"
 
+	"github.com/restic/restic/internal/data"
 	"github.com/restic/restic/internal/global"
 	"github.com/restic/restic/internal/restic"
 	rtest "github.com/restic/restic/internal/test"
@@ -31,3 +34,34 @@ func testRunSnapshots(t testing.TB, gopts global.Options) (newest *Snapshot, sna
 	}
 	return
 }
+
+func TestSnapshotsGroupByAndLatest(t *testing.T) {
+	env, cleanup := withTestEnvironment(t)
+	defer cleanup()
+
+	testSetupBackupData(t, env)
+	// two backups on the same host but with different paths
+	opts := BackupOptions{Host: "testhost", TimeStamp: time.Now().Format(time.DateTime)}
+	testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata"}, opts, env.gopts)
+	// Use later timestamp for second backup
+	opts.TimeStamp = time.Now().Add(time.Second).Format(time.DateTime)
+	snapshotsIDs := loadSnapshotMap(t, env.gopts)
+	testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata/0"}, opts, env.gopts)
+	_, secondSnapshotID := lastSnapshot(snapshotsIDs, loadSnapshotMap(t, env.gopts))
+
+	buf, err := withCaptureStdout(t, env.gopts, func(ctx context.Context, gopts global.Options) error {
+		gopts.JSON = true
+		// only group by host but not path
+		opts := SnapshotOptions{GroupBy: data.SnapshotGroupByOptions{Host: true}, Latest: 1}
+		return runSnapshots(ctx, opts, gopts, []string{}, gopts.Term)
+	})
+	rtest.OK(t, err)
+	snapshots := []SnapshotGroup{}
+	rtest.OK(t, json.Unmarshal(buf.Bytes(), &snapshots))
+	rtest.Assert(t, len(snapshots) == 1, "expected only one snapshot group, got %d", len(snapshots))
+	rtest.Assert(t, snapshots[0].GroupKey.Hostname == "testhost", "expected group_key.hostname to be set to testhost, got %s", snapshots[0].GroupKey.Hostname)
+	rtest.Assert(t, snapshots[0].GroupKey.Paths == nil, "expected group_key.paths to be set to nil, got %s", snapshots[0].GroupKey.Paths)
+	rtest.Assert(t, snapshots[0].GroupKey.Tags == nil, "expected group_key.tags to be set to nil, got %s", snapshots[0].GroupKey.Tags)
+	rtest.Assert(t, len(snapshots[0].Snapshots) == 1, "expected only one latest snapshot, got %d", len(snapshots[0].Snapshots))
+	rtest.Equals(t, snapshots[0].Snapshots[0].ID.String(), secondSnapshotID, "unexpected snapshot ID")
+}