Refactor duplicate code for grouping snapshots

This commit is a followup to the addition of the --group-by flag for the
snapshots command. Adding the grouping code there introduced duplicated
code (the forget command also does grouping). This commit refactors
boths sides to only use shared code.

cmd/restic/cmd_forget.go

@@ -4,10 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"io"
-	"sort"
-	"strings"
 
-	"github.com/restic/restic/internal/errors"
 	"github.com/restic/restic/internal/restic"
 	"github.com/spf13/cobra"
 )
@@ -91,81 +88,40 @@ func runForget(opts ForgetOptions, gopts GlobalOptions, args []string) error {
 		return err
 	}
 
-	// group by hostname and dirs
-	type key struct {
-		Hostname string
-		Paths    []string
-		Tags     []string
-	}
-	snapshotGroups := make(map[string]restic.Snapshots)
-
-	var GroupByTag bool
-	var GroupByHost bool
-	var GroupByPath bool
-	var GroupOptionList []string
-
-	GroupOptionList = strings.Split(opts.GroupBy, ",")
-
-	for _, option := range GroupOptionList {
-		switch option {
-		case "host":
-			GroupByHost = true
-		case "paths":
-			GroupByPath = true
-		case "tags":
-			GroupByTag = true
-		case "":
-		default:
-			return errors.Fatal("unknown grouping option: '" + option + "'")
-		}
-	}
-
 	removeSnapshots := 0
 
 	ctx, cancel := context.WithCancel(gopts.ctx)
 	defer cancel()
+
+	var snapshots restic.Snapshots
+
 	for sn := range FindFilteredSnapshots(ctx, repo, opts.Host, opts.Tags, opts.Paths, args) {
+		snapshots = append(snapshots, sn)
+	}
+
 	if len(args) > 0 {
 		// When explicit snapshots args are given, remove them immediately.
+		for _, sn := range snapshots {
 			if !opts.DryRun {
 				h := restic.Handle{Type: restic.SnapshotFile, Name: sn.ID().String()}
 				if err = repo.Backend().Remove(gopts.ctx, h); err != nil {
 					return err
 				}
+				if !gopts.JSON {
 					Verbosef("removed snapshot %v\n", sn.ID().Str())
+				}
 				removeSnapshots++
 			} else {
+				if !gopts.JSON {
 					Verbosef("would have removed snapshot %v\n", sn.ID().Str())
 				}
+			}
+		}
 	} else {
-			// Determining grouping-keys
+		snapshotGroups, _, err := restic.GroupSnapshots(snapshots, opts.GroupBy)
-			var tags []string
-			var hostname string
-			var paths []string
-
-			if GroupByTag {
-				tags = sn.Tags
-				sort.StringSlice(tags).Sort()
-			}
-			if GroupByHost {
-				hostname = sn.Hostname
-			}
-			if GroupByPath {
-				paths = sn.Paths
-			}
-
-			sort.StringSlice(sn.Paths).Sort()
-			var k []byte
-			var err error
-
-			k, err = json.Marshal(key{Tags: tags, Hostname: hostname, Paths: paths})
-
 		if err != nil {
 			return err
 		}
-			snapshotGroups[string(k)] = append(snapshotGroups[string(k)], sn)
-		}
-	}
 
 		policy := restic.ExpirePolicy{
 			Last:    opts.Last,
@@ -179,8 +135,10 @@ func runForget(opts ForgetOptions, gopts GlobalOptions, args []string) error {
 		}
 
 		if policy.Empty() && len(args) == 0 {
+			if !gopts.JSON {
 				Verbosef("no policy was specified, no snapshots will be removed\n")
 			}
+		}
 
 		if !policy.Empty() {
 			if !gopts.JSON {
@@ -190,35 +148,22 @@ func runForget(opts ForgetOptions, gopts GlobalOptions, args []string) error {
 			var jsonGroups []*ForgetGroup
 
 			for k, snapshotGroup := range snapshotGroups {
-			var key key
+				if gopts.Verbose >= 1 && !gopts.JSON {
+					err = PrintSnapshotGroupHeader(gopts.stdout, k)
+					if err != nil {
+						return err
+					}
+				}
+
+				var key restic.SnapshotGroupKey
 				if json.Unmarshal([]byte(k), &key) != nil {
 					return err
 				}
 
 				var fg ForgetGroup
-			// Info
-			if !gopts.JSON {
-				Verbosef("snapshots")
-			}
-			var infoStrings []string
-			if GroupByTag {
-				infoStrings = append(infoStrings, "tags ["+strings.Join(key.Tags, ", ")+"]")
 				fg.Tags = key.Tags
-			}
-			if GroupByHost {
-				infoStrings = append(infoStrings, "host ["+key.Hostname+"]")
 				fg.Host = key.Hostname
-			}
-			if GroupByPath {
-				infoStrings = append(infoStrings, "paths ["+strings.Join(key.Paths, ", ")+"]")
 				fg.Paths = key.Paths
-			}
-			if infoStrings != nil && !gopts.JSON {
-				Verbosef(" for (" + strings.Join(infoStrings, ", ") + ")")
-			}
-			if !gopts.JSON {
-				Verbosef(":\n\n")
-			}
 
 				keep, remove, reasons := restic.ApplyPolicy(snapshotGroup, policy)
 
@@ -260,9 +205,12 @@ func runForget(opts ForgetOptions, gopts GlobalOptions, args []string) error {
 				}
 			}
 		}
+	}
 
 	if removeSnapshots > 0 && opts.Prune {
+		if !gopts.JSON {
 			Verbosef("%d snapshots have been removed, running prune\n", removeSnapshots)
+		}
 		if !opts.DryRun {
 			return pruneRepository(gopts, repo)
 		}

cmd/restic/cmd_snapshots.go

@@ -8,7 +8,6 @@ import (
 	"sort"
 	"strings"
 
-	"github.com/restic/restic/internal/errors"
 	"github.com/restic/restic/internal/restic"
 	"github.com/restic/restic/internal/ui/table"
 	"github.com/spf13/cobra"
@@ -50,12 +49,6 @@ func init() {
 	f.StringVarP(&snapshotOptions.GroupBy, "group-by", "g", "", "string for grouping snapshots by host,paths,tags")
 }
 
-type groupKey struct {
-	Hostname string   `json:"hostname"`
-	Paths    []string `json:"paths"`
-	Tags     []string `json:"tags"`
-}
-
 func runSnapshots(opts SnapshotOptions, gopts GlobalOptions, args []string) error {
 	repo, err := OpenRepository(gopts)
 	if err != nil {
@@ -77,7 +70,7 @@ func runSnapshots(opts SnapshotOptions, gopts GlobalOptions, args []string) erro
 	for sn := range FindFilteredSnapshots(ctx, repo, opts.Host, opts.Tags, opts.Paths, args) {
 		snapshots = append(snapshots, sn)
 	}
-	snapshotGroups, grouped, err := GroupSnapshots(snapshots, opts.GroupBy)
+	snapshotGroups, grouped, err := restic.GroupSnapshots(snapshots, opts.GroupBy)
 	if err != nil {
 		return err
 	}
@@ -148,65 +141,6 @@ func FilterLastSnapshots(list restic.Snapshots) restic.Snapshots {
 	return results
 }
 
-// GroupSnapshots takes a list of snapshots and a grouping criteria and creates
-// a group list of snapshots.
-func GroupSnapshots(snapshots restic.Snapshots, options string) (map[string]restic.Snapshots, bool, error) {
-	// group by hostname and dirs
-	snapshotGroups := make(map[string]restic.Snapshots)
-
-	var GroupByTag bool
-	var GroupByHost bool
-	var GroupByPath bool
-	var GroupOptionList []string
-
-	GroupOptionList = strings.Split(options, ",")
-
-	for _, option := range GroupOptionList {
-		switch option {
-		case "host":
-			GroupByHost = true
-		case "paths":
-			GroupByPath = true
-		case "tags":
-			GroupByTag = true
-		case "":
-		default:
-			return nil, false, errors.Fatal("unknown grouping option: '" + option + "'")
-		}
-	}
-
-	for _, sn := range snapshots {
-		// Determining grouping-keys
-		var tags []string
-		var hostname string
-		var paths []string
-
-		if GroupByTag {
-			tags = sn.Tags
-			sort.StringSlice(tags).Sort()
-		}
-		if GroupByHost {
-			hostname = sn.Hostname
-		}
-		if GroupByPath {
-			paths = sn.Paths
-		}
-
-		sort.StringSlice(sn.Paths).Sort()
-		var k []byte
-		var err error
-
-		k, err = json.Marshal(groupKey{Tags: tags, Hostname: hostname, Paths: paths})
-
-		if err != nil {
-			return nil, false, err
-		}
-		snapshotGroups[string(k)] = append(snapshotGroups[string(k)], sn)
-	}
-
-	return snapshotGroups, GroupByTag || GroupByHost || GroupByPath, nil
-}
-
 // PrintSnapshots prints a text table of the snapshots in list to stdout.
 func PrintSnapshots(stdout io.Writer, list restic.Snapshots, reasons []restic.KeepReason, compact bool) {
 	// keep the reasons a snasphot is being kept in a map, so that it doesn't
@@ -311,7 +245,7 @@ func PrintSnapshots(stdout io.Writer, list restic.Snapshots, reasons []restic.Ke
 // following snapshots belong to.
 // Prints nothing, if we did not group at all.
 func PrintSnapshotGroupHeader(stdout io.Writer, groupKeyJSON string) error {
-	var key groupKey
+	var key restic.SnapshotGroupKey
 	var err error
 
 	err = json.Unmarshal([]byte(groupKeyJSON), &key)
@@ -353,7 +287,7 @@ type Snapshot struct {
 
 // SnapshotGroup helps to print SnaphotGroups as JSON with their GroupReasons included.
 type SnapshotGroup struct {
-	GroupKey  groupKey   `json:"group_key"`
+	GroupKey  restic.SnapshotGroupKey `json:"group_key"`
 	Snapshots []Snapshot              `json:"snapshots"`
 }
 
@@ -363,7 +297,7 @@ func printSnapshotGroupJSON(stdout io.Writer, snGroups map[string]restic.Snapsho
 		var snapshotGroups []SnapshotGroup
 
 		for k, list := range snGroups {
-			var key groupKey
+			var key restic.SnapshotGroupKey
 			var err error
 			var snapshots []Snapshot
 

internal/restic/snapshot_group.go

@@ -0,0 +1,76 @@
+package restic
+
+import (
+	"encoding/json"
+	"sort"
+	"strings"
+
+	"github.com/restic/restic/internal/errors"
+)
+
+// SnapshotGroupKey is the structure for identifying groups in a grouped
+// snapshot list. This is used by GroupSnapshots()
+type SnapshotGroupKey struct {
+	Hostname string   `json:"hostname"`
+	Paths    []string `json:"paths"`
+	Tags     []string `json:"tags"`
+}
+
+// GroupSnapshots takes a list of snapshots and a grouping criteria and creates
+// a group list of snapshots.
+func GroupSnapshots(snapshots Snapshots, options string) (map[string]Snapshots, bool, error) {
+	// group by hostname and dirs
+	snapshotGroups := make(map[string]Snapshots)
+
+	var GroupByTag bool
+	var GroupByHost bool
+	var GroupByPath bool
+	var GroupOptionList []string
+
+	GroupOptionList = strings.Split(options, ",")
+
+	for _, option := range GroupOptionList {
+		switch option {
+		case "host":
+			GroupByHost = true
+		case "paths":
+			GroupByPath = true
+		case "tags":
+			GroupByTag = true
+		case "":
+		default:
+			return nil, false, errors.Fatal("unknown grouping option: '" + option + "'")
+		}
+	}
+
+	for _, sn := range snapshots {
+		// Determining grouping-keys
+		var tags []string
+		var hostname string
+		var paths []string
+
+		if GroupByTag {
+			tags = sn.Tags
+			sort.StringSlice(tags).Sort()
+		}
+		if GroupByHost {
+			hostname = sn.Hostname
+		}
+		if GroupByPath {
+			paths = sn.Paths
+		}
+
+		sort.StringSlice(sn.Paths).Sort()
+		var k []byte
+		var err error
+
+		k, err = json.Marshal(SnapshotGroupKey{Tags: tags, Hostname: hostname, Paths: paths})
+
+		if err != nil {
+			return nil, false, err
+		}
+		snapshotGroups[string(k)] = append(snapshotGroups[string(k)], sn)
+	}
+
+	return snapshotGroups, GroupByTag || GroupByHost || GroupByPath, nil
+}