From e9390352a77e8eaf59fc7797d109317208a631dd Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Thu, 9 May 2024 13:07:21 +0200
Subject: [PATCH] cache: code cleanups

---
 internal/cache/backend.go   | 15 +++++----------
 internal/cache/file.go      | 16 ++++++++++------
 internal/cache/file_test.go |  8 ++++----
 3 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/internal/cache/backend.go b/internal/cache/backend.go
index 5cbdb5444..7a7e5cd9f 100644
--- a/internal/cache/backend.go
+++ b/internal/cache/backend.go
@@ -79,10 +79,9 @@ func (b *Backend) Save(ctx context.Context, h backend.Handle, rd backend.RewindR
 		return err
 	}
 
-	err = b.Cache.Save(h, rd)
+	err = b.Cache.save(h, rd)
 	if err != nil {
 		debug.Log("unable to save %v to cache: %v", h, err)
-		_ = b.Cache.remove(h)
 		return err
 	}
 
@@ -120,7 +119,7 @@ func (b *Backend) cacheFile(ctx context.Context, h backend.Handle) error {
 	if !b.Cache.Has(h) {
 		// nope, it's still not in the cache, pull it from the repo and save it
 		err := b.Backend.Load(ctx, h, 0, 0, func(rd io.Reader) error {
-			return b.Cache.Save(h, rd)
+			return b.Cache.save(h, rd)
 		})
 		if err != nil {
 			// try to remove from the cache, ignore errors
@@ -198,13 +197,9 @@ func (b *Backend) Stat(ctx context.Context, h backend.Handle) (backend.FileInfo,
 	debug.Log("cache Stat(%v)", h)
 
 	fi, err := b.Backend.Stat(ctx, h)
-	if err != nil {
-		if b.Backend.IsNotExist(err) {
-			// try to remove from the cache, ignore errors
-			_ = b.Cache.remove(h)
-		}
-
-		return fi, err
+	if err != nil && b.Backend.IsNotExist(err) {
+		// try to remove from the cache, ignore errors
+		_ = b.Cache.remove(h)
 	}
 
 	return fi, err
diff --git a/internal/cache/file.go b/internal/cache/file.go
index 59444a788..b2e9ec618 100644
--- a/internal/cache/file.go
+++ b/internal/cache/file.go
@@ -32,7 +32,7 @@ func (c *Cache) canBeCached(t backend.FileType) bool {
 	return ok
 }
 
-// Load returns a reader that yields the contents of the file with the
+// load returns a reader that yields the contents of the file with the
 // given handle. rd must be closed after use. If an error is returned, the
 // ReadCloser is nil.
 func (c *Cache) load(h backend.Handle, length int, offset int64) (io.ReadCloser, error) {
@@ -78,8 +78,8 @@ func (c *Cache) load(h backend.Handle, length int, offset int64) (io.ReadCloser,
 	return util.LimitReadCloser(f, int64(length)), nil
 }
 
-// Save saves a file in the cache.
-func (c *Cache) Save(h backend.Handle, rd io.Reader) error {
+// save saves a file in the cache.
+func (c *Cache) save(h backend.Handle, rd io.Reader) error {
 	debug.Log("Save to cache: %v", h)
 	if rd == nil {
 		return errors.New("Save() called with nil reader")
@@ -139,13 +139,17 @@ func (c *Cache) Save(h backend.Handle, rd io.Reader) error {
 	return errors.WithStack(err)
 }
 
-// Remove deletes a file. When the file is not cache, no error is returned.
+// remove deletes a file. When the file is not cached, no error is returned.
 func (c *Cache) remove(h backend.Handle) error {
-	if !c.Has(h) {
+	if !c.canBeCached(h.Type) {
 		return nil
 	}
 
-	return fs.Remove(c.filename(h))
+	err := fs.Remove(c.filename(h))
+	if errors.Is(err, os.ErrNotExist) {
+		err = nil
+	}
+	return err
 }
 
 // Clear removes all files of type t from the cache that are not contained in
diff --git a/internal/cache/file_test.go b/internal/cache/file_test.go
index 7935f9806..2ce238888 100644
--- a/internal/cache/file_test.go
+++ b/internal/cache/file_test.go
@@ -30,7 +30,7 @@ func generateRandomFiles(t testing.TB, tpe backend.FileType, c *Cache) restic.ID
 			t.Errorf("index %v present before save", id)
 		}
 
-		err := c.Save(h, bytes.NewReader(buf))
+		err := c.save(h, bytes.NewReader(buf))
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -151,7 +151,7 @@ func TestFileLoad(t *testing.T) {
 		Type: restic.PackFile,
 		Name: id.String(),
 	}
-	if err := c.Save(h, bytes.NewReader(data)); err != nil {
+	if err := c.save(h, bytes.NewReader(data)); err != nil {
 		t.Fatalf("Save() returned error: %v", err)
 	}
 
@@ -237,7 +237,7 @@ func TestFileSaveConcurrent(t *testing.T) {
 	}
 
 	for i := 0; i < nproc/2; i++ {
-		g.Go(func() error { return c.Save(h, bytes.NewReader(data)) })
+		g.Go(func() error { return c.save(h, bytes.NewReader(data)) })
 
 		// Can't use load because only the main goroutine may call t.Fatal.
 		g.Go(func() error {
@@ -280,7 +280,7 @@ func TestFileSaveAfterDamage(t *testing.T) {
 		Type: restic.PackFile,
 		Name: id.String(),
 	}
-	if err := c.Save(h, bytes.NewReader(data)); err == nil {
+	if err := c.save(h, bytes.NewReader(data)); err == nil {
 		t.Fatal("Missing error when saving to deleted cache directory")
 	}
 }