replace usages of backend.Remove() with repository.RemoveUnpacked()

RemoveUnpacked will eventually block removal of all filetypes other than
snapshots. However, getting there requires a major refactor to provide
some components with privileged access.

cmd/restic/cmd_rewrite.go

@@ -8,7 +8,6 @@ import (
 	"github.com/spf13/cobra"
 	"golang.org/x/sync/errgroup"
 
-	"github.com/restic/restic/internal/backend"
 	"github.com/restic/restic/internal/debug"
 	"github.com/restic/restic/internal/errors"
 	"github.com/restic/restic/internal/repository"
@@ -181,8 +180,7 @@ func filterAndReplaceSnapshot(ctx context.Context, repo restic.Repository, sn *r
 		if dryRun {
 			Verbosef("would delete empty snapshot\n")
 		} else {
-			h := backend.Handle{Type: restic.SnapshotFile, Name: sn.ID().String()}
+			if err = repo.RemoveUnpacked(ctx, restic.SnapshotFile, *sn.ID()); err != nil {
-			if err = repo.Backend().Remove(ctx, h); err != nil {
 				return false, err
 			}
 			debug.Log("removed empty snapshot %v", sn.ID())
@@ -241,8 +239,7 @@ func filterAndReplaceSnapshot(ctx context.Context, repo restic.Repository, sn *r
 	Verbosef("saved new snapshot %v\n", id.Str())
 
 	if forget {
-		h := backend.Handle{Type: restic.SnapshotFile, Name: sn.ID().String()}
+		if err = repo.RemoveUnpacked(ctx, restic.SnapshotFile, *sn.ID()); err != nil {
-		if err = repo.Backend().Remove(ctx, h); err != nil {
 			return false, err
 		}
 		debug.Log("removed old snapshot %v", sn.ID())

cmd/restic/cmd_tag.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/spf13/cobra"
 
-	"github.com/restic/restic/internal/backend"
 	"github.com/restic/restic/internal/debug"
 	"github.com/restic/restic/internal/errors"
 	"github.com/restic/restic/internal/repository"
@@ -86,8 +85,7 @@ func changeTags(ctx context.Context, repo *repository.Repository, sn *restic.Sna
 		debug.Log("new snapshot saved as %v", id)
 
 		// Remove the old snapshot.
-		h := backend.Handle{Type: restic.SnapshotFile, Name: sn.ID().String()}
+		if err = repo.RemoveUnpacked(ctx, restic.SnapshotFile, *sn.ID()); err != nil {
-		if err = repo.Backend().Remove(ctx, h); err != nil {
 			return false, err
 		}
 

cmd/restic/integration_helpers_test.go

@@ -267,7 +267,7 @@ func removePacks(gopts GlobalOptions, t testing.TB, remove restic.IDSet) {
 	defer unlock()
 
 	for id := range remove {
-		rtest.OK(t, r.Backend().Remove(ctx, backend.Handle{Type: restic.PackFile, Name: id.String()}))
+		rtest.OK(t, r.RemoveUnpacked(ctx, restic.PackFile, id))
 	}
 }
 
@@ -291,7 +291,7 @@ func removePacksExcept(gopts GlobalOptions, t testing.TB, keep restic.IDSet, rem
 		if treePacks.Has(id) != removeTreePacks || keep.Has(id) {
 			return nil
 		}
-		return r.Backend().Remove(ctx, backend.Handle{Type: restic.PackFile, Name: id.String()})
+		return r.RemoveUnpacked(ctx, restic.PackFile, id)
 	}))
 }
 

internal/checker/checker_test.go

@@ -94,11 +94,8 @@ func TestMissingPack(t *testing.T) {
 	repo, cleanup := repository.TestFromFixture(t, checkerTestData)
 	defer cleanup()
 
-	packHandle := backend.Handle{
+	packID := restic.TestParseID("657f7fb64f6a854fff6fe9279998ee09034901eded4e6db9bcee0e59745bbce6")
-		Type: restic.PackFile,
+	test.OK(t, repo.RemoveUnpacked(context.TODO(), restic.PackFile, packID))
-		Name: "657f7fb64f6a854fff6fe9279998ee09034901eded4e6db9bcee0e59745bbce6",
-	}
-	test.OK(t, repo.Backend().Remove(context.TODO(), packHandle))
 
 	chkr := checker.New(repo, false)
 	hints, errs := chkr.LoadIndex(context.TODO(), nil)
@@ -113,7 +110,7 @@ func TestMissingPack(t *testing.T) {
 		"expected exactly one error, got %v", len(errs))
 
 	if err, ok := errs[0].(*checker.PackError); ok {
-		test.Equals(t, packHandle.Name, err.ID.String())
+		test.Equals(t, packID, err.ID)
 	} else {
 		t.Errorf("expected error returned by checker.Packs() to be PackError, got %v", err)
 	}
@@ -125,11 +122,8 @@ func TestUnreferencedPack(t *testing.T) {
 
 	// index 3f1a only references pack 60e0
 	packID := "60e0438dcb978ec6860cc1f8c43da648170ee9129af8f650f876bad19f8f788e"
-	indexHandle := backend.Handle{
+	indexID := restic.TestParseID("3f1abfcb79c6f7d0a3be517d2c83c8562fba64ef2c8e9a3544b4edaf8b5e3b44")
-		Type: restic.IndexFile,
+	test.OK(t, repo.RemoveUnpacked(context.TODO(), restic.IndexFile, indexID))
-		Name: "3f1abfcb79c6f7d0a3be517d2c83c8562fba64ef2c8e9a3544b4edaf8b5e3b44",
-	}
-	test.OK(t, repo.Backend().Remove(context.TODO(), indexHandle))
 
 	chkr := checker.New(repo, false)
 	hints, errs := chkr.LoadIndex(context.TODO(), nil)
@@ -154,11 +148,8 @@ func TestUnreferencedBlobs(t *testing.T) {
 	repo, cleanup := repository.TestFromFixture(t, checkerTestData)
 	defer cleanup()
 
-	snapshotHandle := backend.Handle{
+	snapshotID := restic.TestParseID("51d249d28815200d59e4be7b3f21a157b864dc343353df9d8e498220c2499b02")
-		Type: restic.SnapshotFile,
+	test.OK(t, repo.RemoveUnpacked(context.TODO(), restic.SnapshotFile, snapshotID))
-		Name: "51d249d28815200d59e4be7b3f21a157b864dc343353df9d8e498220c2499b02",
-	}
-	test.OK(t, repo.Backend().Remove(context.TODO(), snapshotHandle))
 
 	unusedBlobsBySnapshot := restic.BlobHandles{
 		restic.TestParseHandle("58c748bbe2929fdf30c73262bd8313fe828f8925b05d1d4a87fe109082acb849", restic.DataBlob),

internal/repository/repack_test.go

@@ -167,7 +167,7 @@ func repack(t *testing.T, repo restic.Repository, packs restic.IDSet, blobs rest
 	}
 
 	for id := range repackedBlobs {
-		err = repo.Backend().Remove(context.TODO(), backend.Handle{Type: restic.PackFile, Name: id.String()})
+		err = repo.RemoveUnpacked(context.TODO(), restic.PackFile, id)
 		if err != nil {
 			t.Fatal(err)
 		}

internal/repository/repository.go

@@ -520,6 +520,11 @@ func (r *Repository) verifyUnpacked(buf []byte, t restic.FileType, expected []by
 	return nil
 }
 
+func (r *Repository) RemoveUnpacked(ctx context.Context, t restic.FileType, id restic.ID) error {
+	// TODO prevent everything except removing snapshots for non-repository code
+	return r.be.Remove(ctx, backend.Handle{Type: t, Name: id.String()})
+}
+
 // Flush saves all remaining packs and the index
 func (r *Repository) Flush(ctx context.Context) error {
 	if err := r.flushPacks(ctx); err != nil {

internal/restic/lock.go

@@ -12,7 +12,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/restic/restic/internal/backend"
 	"github.com/restic/restic/internal/errors"
 
 	"github.com/restic/restic/internal/debug"
@@ -226,7 +225,7 @@ func (l *Lock) Unlock() error {
 		return nil
 	}
 
-	return l.repo.Backend().Remove(context.TODO(), backend.Handle{Type: LockFile, Name: l.lockID.String()})
+	return l.repo.RemoveUnpacked(context.TODO(), LockFile, *l.lockID)
 }
 
 var StaleLockTimeout = 30 * time.Minute
@@ -286,7 +285,7 @@ func (l *Lock) Refresh(ctx context.Context) error {
 	oldLockID := l.lockID
 	l.lockID = &id
 
-	return l.repo.Backend().Remove(context.TODO(), backend.Handle{Type: LockFile, Name: oldLockID.String()})
+	return l.repo.RemoveUnpacked(context.TODO(), LockFile, *oldLockID)
 }
 
 // RefreshStaleLock is an extended variant of Refresh that can also refresh stale lock files.
@@ -315,13 +314,13 @@ func (l *Lock) RefreshStaleLock(ctx context.Context) error {
 	exists, err = l.checkExistence(ctx)
 	if err != nil {
 		// cleanup replacement lock
-		_ = l.repo.Backend().Remove(context.TODO(), backend.Handle{Type: LockFile, Name: id.String()})
+		_ = l.repo.RemoveUnpacked(context.TODO(), LockFile, id)
 		return err
 	}
 
 	if !exists {
 		// cleanup replacement lock
-		_ = l.repo.Backend().Remove(context.TODO(), backend.Handle{Type: LockFile, Name: id.String()})
+		_ = l.repo.RemoveUnpacked(context.TODO(), LockFile, id)
 		return ErrRemovedLock
 	}
 
@@ -332,7 +331,7 @@ func (l *Lock) RefreshStaleLock(ctx context.Context) error {
 	oldLockID := l.lockID
 	l.lockID = &id
 
-	return l.repo.Backend().Remove(context.TODO(), backend.Handle{Type: LockFile, Name: oldLockID.String()})
+	return l.repo.RemoveUnpacked(context.TODO(), LockFile, *oldLockID)
 }
 
 func (l *Lock) checkExistence(ctx context.Context) (bool, error) {
@@ -400,7 +399,7 @@ func RemoveStaleLocks(ctx context.Context, repo Repository) (uint, error) {
 		}
 
 		if lock.Stale() {
-			err = repo.Backend().Remove(ctx, backend.Handle{Type: LockFile, Name: id.String()})
+			err = repo.RemoveUnpacked(ctx, LockFile, id)
 			if err == nil {
 				processed++
 			}
@@ -416,7 +415,7 @@ func RemoveStaleLocks(ctx context.Context, repo Repository) (uint, error) {
 func RemoveAllLocks(ctx context.Context, repo Repository) (uint, error) {
 	var processed uint32
 	err := ParallelList(ctx, repo, LockFile, repo.Connections(), func(ctx context.Context, id ID, _ int64) error {
-		err := repo.Backend().Remove(ctx, backend.Handle{Type: LockFile, Name: id.String()})
+		err := repo.RemoveUnpacked(ctx, LockFile, id)
 		if err == nil {
 			atomic.AddUint32(&processed, 1)
 		}

internal/restic/lock_test.go

@@ -131,8 +131,7 @@ func createFakeLock(repo restic.SaverUnpacked, t time.Time, pid int) (restic.ID,
 }
 
 func removeLock(repo restic.Repository, id restic.ID) error {
-	h := backend.Handle{Type: restic.LockFile, Name: id.String()}
+	return repo.RemoveUnpacked(context.TODO(), restic.LockFile, id)
-	return repo.Backend().Remove(context.TODO(), h)
 }
 
 var staleLockTests = []struct {
@@ -318,7 +317,7 @@ func TestLockRefreshStaleMissing(t *testing.T) {
 	lockID := checkSingleLock(t, repo)
 
 	// refresh must fail if lock was removed
-	rtest.OK(t, repo.Backend().Remove(context.TODO(), backend.Handle{Type: restic.LockFile, Name: lockID.String()}))
+	rtest.OK(t, repo.RemoveUnpacked(context.TODO(), restic.LockFile, lockID))
 	time.Sleep(time.Millisecond)
 	err = lock.RefreshStaleLock(context.TODO())
 	rtest.Assert(t, err == restic.ErrRemovedLock, "unexpected error, expected %v, got %v", restic.ErrRemovedLock, err)

internal/restic/parallel.go

@@ -3,7 +3,6 @@ package restic
 import (
 	"context"
 
-	"github.com/restic/restic/internal/backend"
 	"github.com/restic/restic/internal/debug"
 	"github.com/restic/restic/internal/ui/progress"
 	"golang.org/x/sync/errgroup"
@@ -77,8 +76,7 @@ func ParallelRemove(ctx context.Context, repo Repository, fileList IDSet, fileTy
 	for i := 0; i < int(workerCount); i++ {
 		wg.Go(func() error {
 			for id := range fileChan {
-				h := backend.Handle{Type: fileType, Name: id.String()}
+				err := repo.RemoveUnpacked(ctx, fileType, id)
-				err := repo.Backend().Remove(ctx, h)
 				if report != nil {
 					err = report(id, err)
 				}

internal/restic/repository.go

@@ -57,6 +57,8 @@ type Repository interface {
 	// LoadUnpacked loads and decrypts the file with the given type and ID.
 	LoadUnpacked(ctx context.Context, t FileType, id ID) (data []byte, err error)
 	SaveUnpacked(context.Context, FileType, []byte) (ID, error)
+	// RemoveUnpacked removes a file from the repository. This will eventually be restricted to deleting only snapshots.
+	RemoveUnpacked(ctx context.Context, t FileType, id ID) error
 
 	// LoadRaw reads all data stored in the backend for the file with id and filetype t.
 	// If the backend returns data that does not match the id, then the buffer is returned