From 893d0d6325ec8b4edb64d168c771c8256dcfaf80 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 24 Dec 2023 14:36:27 +0100 Subject: [PATCH] rewrite: cleanup new metadata options and fix no parameters check --- cmd/restic/cmd_rewrite.go | 53 ++++++++++++---------- cmd/restic/cmd_rewrite_integration_test.go | 16 +++---- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/cmd/restic/cmd_rewrite.go b/cmd/restic/cmd_rewrite.go index 1a6f39d04..553a9da91 100644 --- a/cmd/restic/cmd_rewrite.go +++ b/cmd/restic/cmd_rewrite.go @@ -52,12 +52,16 @@ type snapshotMetadata struct { Time *time.Time } -type SnapshotMetadataArgs struct { +type snapshotMetadataArgs struct { Hostname string Time string } -func (sma SnapshotMetadataArgs) convert() (*snapshotMetadata, error) { +func (sma snapshotMetadataArgs) empty() bool { + return sma.Hostname == "" && sma.Time == "" +} + +func (sma snapshotMetadataArgs) convert() (*snapshotMetadata, error) { if sma.Time == "" && sma.Hostname == "" { return nil, nil } @@ -78,16 +82,15 @@ func (sma SnapshotMetadataArgs) convert() (*snapshotMetadata, error) { // RewriteOptions collects all options for the rewrite command. type RewriteOptions struct { - Forget bool - DryRun bool - Metadata *SnapshotMetadataArgs + Forget bool + DryRun bool + metadata snapshotMetadataArgs restic.SnapshotFilter excludePatternOptions } var rewriteOptions RewriteOptions -var metadataOptions SnapshotMetadataArgs func init() { cmdRoot.AddCommand(cmdRewrite) @@ -95,15 +98,15 @@ func init() { f := cmdRewrite.Flags() f.BoolVarP(&rewriteOptions.Forget, "forget", "", false, "remove original snapshots after creating new ones") f.BoolVarP(&rewriteOptions.DryRun, "dry-run", "n", false, "do not do anything, just print what would be done") - f.StringVar(&metadataOptions.Hostname, "new-host", "", "rewrite hostname") - f.StringVar(&metadataOptions.Time, "new-time", "", "rewrite time of the backup") - - rewriteOptions.Metadata = &metadataOptions + f.StringVar(&rewriteOptions.metadata.Hostname, "new-host", "", "replace hostname") + f.StringVar(&rewriteOptions.metadata.Time, "new-time", "", "replace time of the backup") initMultiSnapshotFilter(f, &rewriteOptions.SnapshotFilter, true) initExcludePatternOptions(f, &rewriteOptions.excludePatternOptions) } +type rewriteFilterFunc func(ctx context.Context, sn *restic.Snapshot) (restic.ID, error) + func rewriteSnapshot(ctx context.Context, repo *repository.Repository, sn *restic.Snapshot, opts RewriteOptions) (bool, error) { if sn.Tree == nil { return false, errors.Errorf("snapshot %v has nil tree", sn.ID().Str()) @@ -114,7 +117,7 @@ func rewriteSnapshot(ctx context.Context, repo *repository.Repository, sn *resti return false, err } - metadata, err := opts.Metadata.convert() + metadata, err := opts.metadata.convert() if err != nil { return false, err @@ -146,7 +149,8 @@ func rewriteSnapshot(ctx context.Context, repo *repository.Repository, sn *resti }, opts.DryRun, opts.Forget, metadata, "rewrite") } -func filterAndReplaceSnapshot(ctx context.Context, repo restic.Repository, sn *restic.Snapshot, filter func(ctx context.Context, sn *restic.Snapshot) (restic.ID, error), dryRun bool, forget bool, metadata *snapshotMetadata, addTag string) (bool, error) { +func filterAndReplaceSnapshot(ctx context.Context, repo restic.Repository, sn *restic.Snapshot, + filter rewriteFilterFunc, dryRun bool, forget bool, newMetadata *snapshotMetadata, addTag string) (bool, error) { wg, wgCtx := errgroup.WithContext(ctx) repo.StartPackUploader(wgCtx, wg) @@ -180,7 +184,7 @@ func filterAndReplaceSnapshot(ctx context.Context, repo restic.Repository, sn *r return true, nil } - if filteredTree == *sn.Tree && metadata == nil { + if filteredTree == *sn.Tree && newMetadata == nil { debug.Log("Snapshot %v not modified", sn) return false, nil } @@ -193,12 +197,12 @@ func filterAndReplaceSnapshot(ctx context.Context, repo restic.Repository, sn *r Verbosef("would remove old snapshot\n") } - if metadata != nil && metadata.Time != nil { - Verbosef("would set time to %s\n", metadata.Time) + if newMetadata != nil && newMetadata.Time != nil { + Verbosef("would set time to %s\n", newMetadata.Time) } - if metadata != nil && metadata.Hostname != "" { - Verbosef("would set time to %s\n", metadata.Hostname) + if newMetadata != nil && newMetadata.Hostname != "" { + Verbosef("would set time to %s\n", newMetadata.Hostname) } return true, nil @@ -212,15 +216,14 @@ func filterAndReplaceSnapshot(ctx context.Context, repo restic.Repository, sn *r sn.AddTags([]string{addTag}) } - if metadata != nil && metadata.Time != nil { - Verbosef("Setting time to %s\n", *metadata.Time) - sn.Time = *metadata.Time + if newMetadata != nil && newMetadata.Time != nil { + Verbosef("setting time to %s\n", *newMetadata.Time) + sn.Time = *newMetadata.Time } - if metadata != nil && metadata.Hostname != "" { - Verbosef("Setting host to %s\n", metadata.Hostname) - sn.Hostname = metadata.Hostname - + if newMetadata != nil && newMetadata.Hostname != "" { + Verbosef("setting host to %s\n", newMetadata.Hostname) + sn.Hostname = newMetadata.Hostname } // Save the new snapshot. @@ -242,7 +245,7 @@ func filterAndReplaceSnapshot(ctx context.Context, repo restic.Repository, sn *r } func runRewrite(ctx context.Context, opts RewriteOptions, gopts GlobalOptions, args []string) error { - if opts.excludePatternOptions.Empty() && opts.Metadata == nil { + if opts.excludePatternOptions.Empty() && opts.metadata.empty() { return errors.Fatal("Nothing to do: no excludes provided and no new metadata provided") } diff --git a/cmd/restic/cmd_rewrite_integration_test.go b/cmd/restic/cmd_rewrite_integration_test.go index d52679f86..b61ae0bb1 100644 --- a/cmd/restic/cmd_rewrite_integration_test.go +++ b/cmd/restic/cmd_rewrite_integration_test.go @@ -9,13 +9,13 @@ import ( rtest "github.com/restic/restic/internal/test" ) -func testRunRewriteExclude(t testing.TB, gopts GlobalOptions, excludes []string, forget bool, metadata *SnapshotMetadataArgs) { +func testRunRewriteExclude(t testing.TB, gopts GlobalOptions, excludes []string, forget bool, metadata snapshotMetadataArgs) { opts := RewriteOptions{ excludePatternOptions: excludePatternOptions{ Excludes: excludes, }, Forget: forget, - Metadata: metadata, + metadata: metadata, } rtest.OK(t, runRewrite(context.TODO(), opts, gopts, nil)) @@ -39,7 +39,7 @@ func TestRewrite(t *testing.T) { createBasicRewriteRepo(t, env) // exclude some data - testRunRewriteExclude(t, env.gopts, []string{"3"}, false, &SnapshotMetadataArgs{Hostname: "", Time: ""}) + testRunRewriteExclude(t, env.gopts, []string{"3"}, false, snapshotMetadataArgs{Hostname: "", Time: ""}) snapshotIDs := testRunList(t, "snapshots", env.gopts) rtest.Assert(t, len(snapshotIDs) == 2, "expected two snapshots, got %v", snapshotIDs) testRunCheck(t, env.gopts) @@ -51,7 +51,7 @@ func TestRewriteUnchanged(t *testing.T) { snapshotID := createBasicRewriteRepo(t, env) // use an exclude that will not exclude anything - testRunRewriteExclude(t, env.gopts, []string{"3dflkhjgdflhkjetrlkhjgfdlhkj"}, false, &SnapshotMetadataArgs{Hostname: "", Time: ""}) + testRunRewriteExclude(t, env.gopts, []string{"3dflkhjgdflhkjetrlkhjgfdlhkj"}, false, snapshotMetadataArgs{Hostname: "", Time: ""}) newSnapshotIDs := testRunList(t, "snapshots", env.gopts) rtest.Assert(t, len(newSnapshotIDs) == 1, "expected one snapshot, got %v", newSnapshotIDs) rtest.Assert(t, snapshotID == newSnapshotIDs[0], "snapshot id changed unexpectedly") @@ -64,7 +64,7 @@ func TestRewriteReplace(t *testing.T) { snapshotID := createBasicRewriteRepo(t, env) // exclude some data - testRunRewriteExclude(t, env.gopts, []string{"3"}, true, &SnapshotMetadataArgs{Hostname: "", Time: ""}) + testRunRewriteExclude(t, env.gopts, []string{"3"}, true, snapshotMetadataArgs{Hostname: "", Time: ""}) newSnapshotIDs := testRunList(t, "snapshots", env.gopts) rtest.Assert(t, len(newSnapshotIDs) == 1, "expected one snapshot, got %v", newSnapshotIDs) rtest.Assert(t, snapshotID != newSnapshotIDs[0], "snapshot id should have changed") @@ -73,14 +73,14 @@ func TestRewriteReplace(t *testing.T) { testRunCheck(t, env.gopts) } -func testRewriteMetadata(t *testing.T, metadata SnapshotMetadataArgs) { +func testRewriteMetadata(t *testing.T, metadata snapshotMetadataArgs) { env, cleanup := withTestEnvironment(t) env.gopts.backendTestHook = nil defer cleanup() createBasicRewriteRepo(t, env) repo, _ := OpenRepository(context.TODO(), env.gopts) - testRunRewriteExclude(t, env.gopts, []string{}, true, &metadata) + testRunRewriteExclude(t, env.gopts, []string{}, true, metadata) snapshots := FindFilteredSnapshots(context.TODO(), repo, repo, &restic.SnapshotFilter{}, []string{}) @@ -99,7 +99,7 @@ func TestRewriteMetadata(t *testing.T) { newHost := "new host" newTime := "1999-01-01 11:11:11" - for _, metadata := range []SnapshotMetadataArgs{ + for _, metadata := range []snapshotMetadataArgs{ {Hostname: "", Time: newTime}, {Hostname: newHost, Time: ""}, {Hostname: newHost, Time: newTime},