Repository: kopia/kopia
Pull request #2621 (2022-12-03 18:24:50)

refactor(repository): moved cache protection to separate package

codecov[bot]: # [Codecov](https://codecov.io/gh/kopia/kopia/pull/2621?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) Report
Base: **74.93**% // Head: **74.94**% // Increases project coverage by **`+0.01%`** :tada:
> Coverage data is based on head [(`11e6231`)](https://codecov.io/gh/kopia/kopia/pull/2621?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) compared to base [(`c44c60d`)](https://codecov.io/gh/kopia/kopia/commit/c44c60d3763e671e00750004f7d5f9dd02669e13?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).
> Patch coverage: 100.00% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #2621      +/-   ##
==========================================
+ Coverage   74.93%   74.94%   +0.01%     
==========================================
  Files         453      453              
  Lines       35541    35541              
==========================================
+ Hits        26631    26637       +6     
+ Misses       7023     7022       -1     
+ Partials     1887     1882       -5     
```


| [Impacted Files](https://codecov.io/gh/kopia/kopia/pull/2621?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia) | Coverage Δ | |
|---|---|---|
| [internal/cacheprot/storage\_protection.go](https://codecov.io/gh/kopia/kopia/pull/2621/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-aW50ZXJuYWwvY2FjaGVwcm90L3N0b3JhZ2VfcHJvdGVjdGlvbi5nbw==) | `100.00% <ø> (ø)` | |
| [internal/cache/content\_cache.go](https://codecov.io/gh/kopia/kopia/pull/2621/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-aW50ZXJuYWwvY2FjaGUvY29udGVudF9jYWNoZS5nbw==) | `92.80% <100.00%> (ø)` | |
| [internal/cache/persistent\_lru\_cache.go](https://codecov.io/gh/kopia/kopia/pull/2621/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-aW50ZXJuYWwvY2FjaGUvcGVyc2lzdGVudF9scnVfY2FjaGUuZ28=) | `96.06% <100.00%> (ø)` | |
| [repo/content/committed\_read\_manager.go](https://codecov.io/gh/kopia/kopia/pull/2621/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9jb250ZW50L2NvbW1pdHRlZF9yZWFkX21hbmFnZXIuZ28=) | `71.50% <100.00%> (ø)` | |
| [repo/open.go](https://codecov.io/gh/kopia/kopia/pull/2621/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-cmVwby9vcGVuLmdv) | `79.05% <100.00%> (ø)` | |
| [internal/server/source\_manager.go](https://codecov.io/gh/kopia/kopia/pull/2621/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-aW50ZXJuYWwvc2VydmVyL3NvdXJjZV9tYW5hZ2VyLmdv) | `82.95% <0.00%> (+0.76%)` | :arrow_up: |
| [internal/dirutil/mksubdirall.go](https://codecov.io/gh/kopia/kopia/pull/2621/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia#diff-aW50ZXJuYWwvZGlydXRpbC9ta3N1YmRpcmFsbC5nbw==) | `100.00% <0.00%> (+7.89%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/kopia/kopia/pull/2621?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=kopia).


Patch:
diff --git a/internal/cache/content_cache.go b/internal/cache/content_cache.go
index 74b5a5f0..c3d936eb 100644
--- a/internal/cache/content_cache.go
+++ b/internal/cache/content_cache.go
@@ -5,6 +5,7 @@
 
 	"github.com/pkg/errors"
 
+	"github.com/kopia/kopia/internal/cacheprot"
 	"github.com/kopia/kopia/internal/gather"
 	"github.com/kopia/kopia/internal/impossible"
 	"github.com/kopia/kopia/internal/metrics"
@@ -189,7 +190,7 @@ func NewContentCache(ctx context.Context, st blob.Storage, opt Options, mr *metr
 		}
 	}
 
-	pc, err := NewPersistentCache(ctx, opt.CacheSubDir, cacheStorage, ChecksumProtection(opt.HMACSecret), opt.Sweep, mr)
+	pc, err := NewPersistentCache(ctx, opt.CacheSubDir, cacheStorage, cacheprot.ChecksumProtection(opt.HMACSecret), opt.Sweep, mr)
 	if err != nil {
 		return nil, errors.Wrap(err, "unable to create base cache")
 	}
diff --git a/internal/cache/persistent_lru_cache.go b/internal/cache/persistent_lru_cache.go
index 6b539fab..09d0f8af 100644
--- a/internal/cache/persistent_lru_cache.go
+++ b/internal/cache/persistent_lru_cache.go
@@ -11,6 +11,7 @@
 	lru "github.com/hashicorp/golang-lru"
 	"github.com/pkg/errors"
 
+	"github.com/kopia/kopia/internal/cacheprot"
 	"github.com/kopia/kopia/internal/clock"
 	"github.com/kopia/kopia/internal/ctxutil"
 	"github.com/kopia/kopia/internal/gather"
@@ -42,7 +43,7 @@ type PersistentCache struct {
 	anyChange atomic.Bool
 
 	cacheStorage      Storage
-	storageProtection StorageProtection
+	storageProtection cacheprot.StorageProtection
 	sweep             SweepSettings
 
 	description string
@@ -137,7 +138,7 @@ func (c *PersistentCache) GetPartial(ctx context.Context, key string, offset, le
 		prot := c.storageProtection
 		if length >= 0 {
 			// only full items have protection.
-			prot = nullStorageProtection{}
+			prot = cacheprot.NoProtection()
 		}
 
 		if err := prot.Verify(key, tmp.Bytes(), output); err == nil {
@@ -333,7 +334,7 @@ func (s SweepSettings) applyDefaults() SweepSettings {
 }
 
 // NewPersistentCache creates the persistent cache in the provided storage.
-func NewPersistentCache(ctx context.Context, description string, cacheStorage Storage, storageProtection StorageProtection, sweep SweepSettings, mr *metrics.Registry) (*PersistentCache, error) {
+func NewPersistentCache(ctx context.Context, description string, cacheStorage Storage, storageProtection cacheprot.StorageProtection, sweep SweepSettings, mr *metrics.Registry) (*PersistentCache, error) {
 	if cacheStorage == nil {
 		return nil, nil
 	}
@@ -341,7 +342,7 @@ func NewPersistentCache(ctx context.Context, description string, cacheStorage St
 	sweep = sweep.applyDefaults()
 
 	if storageProtection == nil {
-		storageProtection = NoProtection()
+		storageProtection = cacheprot.NoProtection()
 	}
 
 	c := &PersistentCache{
diff --git a/internal/cache/storage_protection.go b/internal/cacheprot/storage_protection.go
similarity index 97%
rename from internal/cache/storage_protection.go
rename to internal/cacheprot/storage_protection.go
index 947a45c7..77658215 100644
--- a/internal/cache/storage_protection.go
+++ b/internal/cacheprot/storage_protection.go
@@ -1,4 +1,5 @@
-package cache
+// Package cacheprot provides utilities for protection of cache entries.
+package cacheprot
 
 import (
 	"crypto/sha256"
diff --git a/repo/content/committed_read_manager.go b/repo/content/committed_read_manager.go
index 9800bf82..1aa99fa7 100644
--- a/repo/content/committed_read_manager.go
+++ b/repo/content/committed_read_manager.go
@@ -11,6 +11,7 @@
 	"go.uber.org/zap"
 
 	"github.com/kopia/kopia/internal/cache"
+	"github.com/kopia/kopia/internal/cacheprot"
 	"github.com/kopia/kopia/internal/clock"
 	"github.com/kopia/kopia/internal/epoch"
 	"github.com/kopia/kopia/internal/gather"
@@ -427,7 +428,7 @@ func (sm *SharedManager) setupReadManagerCaches(ctx context.Context, caching *Ca
 		return errors.Wrap(err, "unable to initialize index blob cache storage")
 	}
 
-	indexBlobCache, err := cache.NewPersistentCache(ctx, "index-blobs", indexBlobStorage, cache.ChecksumProtection(caching.HMACSecret), cache.SweepSettings{
+	indexBlobCache, err := cache.NewPersistentCache(ctx, "index-blobs", indexBlobStorage, cacheprot.ChecksumProtection(caching.HMACSecret), cache.SweepSettings{
 		MaxSizeBytes: metadataCacheSize,
 		MinSweepAge:  caching.MinMetadataSweepAge.DurationOrDefault(DefaultMetadataCacheSweepAge),
 	}, mr)
diff --git a/repo/open.go b/repo/open.go
index d44e73cf..206c5f57 100644
--- a/repo/open.go
+++ b/repo/open.go
@@ -13,6 +13,7 @@
 	"golang.org/x/crypto/scrypt"
 
 	"github.com/kopia/kopia/internal/cache"
+	"github.com/kopia/kopia/internal/cacheprot"
 	"github.com/kopia/kopia/internal/epoch"
 	"github.com/kopia/kopia/internal/feature"
 	"github.com/kopia/kopia/internal/metrics"
@@ -143,7 +144,7 @@ func getContentCacheOrNil(ctx context.Context, opt *content.CachingOptions, pass
 		return nil, errors.Wrap(err, "unable to derive cache encryption key from password")
 	}
 
-	prot, err := cache.AuthenticatedEncryptionProtection(cacheEncryptionKey)
+	prot, err := cacheprot.AuthenticatedEncryptionProtection(cacheEncryptionKey)
 	if err != nil {
 		return nil, errors.Wrap(err, "unable to initialize protection")
 	}


Test patch:
diff --git a/internal/cache/persistent_lru_cache_test.go b/internal/cache/persistent_lru_cache_test.go
index 24dec137..76ee7ee5 100644
--- a/internal/cache/persistent_lru_cache_test.go
+++ b/internal/cache/persistent_lru_cache_test.go
@@ -11,6 +11,7 @@
 
 	"github.com/kopia/kopia/internal/blobtesting"
 	"github.com/kopia/kopia/internal/cache"
+	"github.com/kopia/kopia/internal/cacheprot"
 	"github.com/kopia/kopia/internal/fault"
 	"github.com/kopia/kopia/internal/gather"
 	"github.com/kopia/kopia/internal/testlogging"
@@ -25,7 +26,7 @@ func TestPersistentLRUCache(t *testing.T) {
 
 	cs := blobtesting.NewMapStorage(blobtesting.DataMap{}, nil, nil).(cache.Storage)
 
-	pc, err := cache.NewPersistentCache(ctx, "testing", cs, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{
+	pc, err := cache.NewPersistentCache(ctx, "testing", cs, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{
 		MaxSizeBytes:   maxSizeBytes,
 		TouchThreshold: cache.DefaultTouchThreshold,
 		SweepFrequency: cache.DefaultSweepFrequency,
@@ -67,7 +68,7 @@ func TestPersistentLRUCache(t *testing.T) {
 	verifyBlobExists(ctx, t, cs, "key3")
 	verifyBlobExists(ctx, t, cs, "key4")
 
-	pc, err = cache.NewPersistentCache(ctx, "testing", cs, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{
+	pc, err = cache.NewPersistentCache(ctx, "testing", cs, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{
 		MaxSizeBytes:   maxSizeBytes,
 		TouchThreshold: cache.DefaultTouchThreshold,
 		SweepFrequency: cache.DefaultSweepFrequency,
@@ -81,7 +82,7 @@ func TestPersistentLRUCache(t *testing.T) {
 
 	// create another persistent cache based on the same storage but wrong protection key.
 	// all reads from cache will be invalid, which means GetOrLoad will fetch them from the source.
-	pc2, err := cache.NewPersistentCache(ctx, "testing", cs, cache.ChecksumProtection([]byte{3, 2, 1}), cache.SweepSettings{
+	pc2, err := cache.NewPersistentCache(ctx, "testing", cs, cacheprot.ChecksumProtection([]byte{3, 2, 1}), cache.SweepSettings{
 		MaxSizeBytes:   maxSizeBytes,
 		TouchThreshold: cache.DefaultTouchThreshold,
 		SweepFrequency: cache.DefaultSweepFrequency,
@@ -153,7 +154,7 @@ func TestPersistentLRUCache_GetDeletesInvalidBlob(t *testing.T) {
 	fs := blobtesting.NewFaultyStorage(st)
 	fc := faultyCache{fs}
 
-	pc, err := cache.NewPersistentCache(ctx, "test", fc, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{}, nil)
+	pc, err := cache.NewPersistentCache(ctx, "test", fc, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{}, nil)
 	require.NoError(t, err)
 
 	pc.Put(ctx, "key", gather.FromSlice([]byte{1, 2, 3}))
@@ -183,7 +184,7 @@ func TestPersistentLRUCache_PutIgnoresStorageFailure(t *testing.T) {
 	fs := blobtesting.NewFaultyStorage(st)
 	fc := faultyCache{fs}
 
-	pc, err := cache.NewPersistentCache(ctx, "test", fc, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{}, nil)
+	pc, err := cache.NewPersistentCache(ctx, "test", fc, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{}, nil)
 	require.NoError(t, err)
 
 	fs.AddFault(blobtesting.MethodPutBlob).ErrorInstead(someError)
@@ -209,7 +210,7 @@ func TestPersistentLRUCache_SweepMinSweepAge(t *testing.T) {
 	fs := blobtesting.NewFaultyStorage(st)
 	fc := faultyCache{fs}
 
-	pc, err := cache.NewPersistentCache(ctx, "test", fc, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{
+	pc, err := cache.NewPersistentCache(ctx, "test", fc, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{
 		SweepFrequency: 100 * time.Millisecond,
 		MaxSizeBytes:   1000,
 		MinSweepAge:    10 * time.Second,
@@ -238,7 +239,7 @@ func TestPersistentLRUCache_SweepIgnoresErrors(t *testing.T) {
 	fs := blobtesting.NewFaultyStorage(st)
 	fc := faultyCache{fs}
 
-	pc, err := cache.NewPersistentCache(ctx, "test", fc, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{
+	pc, err := cache.NewPersistentCache(ctx, "test", fc, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{
 		SweepFrequency: 100 * time.Millisecond,
 		MaxSizeBytes:   1000,
 	}, nil)
@@ -271,7 +272,7 @@ func TestPersistentLRUCache_Sweep1(t *testing.T) {
 	fs := blobtesting.NewFaultyStorage(st)
 	fc := faultyCache{fs}
 
-	pc, err := cache.NewPersistentCache(ctx, "test", fc, cache.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{
+	pc, err := cache.NewPersistentCache(ctx, "test", fc, cacheprot.ChecksumProtection([]byte{1, 2, 3}), cache.SweepSettings{
 		SweepFrequency: 10 * time.Millisecond,
 		MaxSizeBytes:   1,
 		MinSweepAge:    0 * time.Second,
diff --git a/internal/cache/storage_protection_test.go b/internal/cacheprot/storage_protection_test.go
similarity index 76%
rename from internal/cache/storage_protection_test.go
rename to internal/cacheprot/storage_protection_test.go
index 4d57fce0..31d5ee11 100644
--- a/internal/cache/storage_protection_test.go
+++ b/internal/cacheprot/storage_protection_test.go
@@ -1,4 +1,4 @@
-package cache_test
+package cacheprot_test
 
 import (
 	"bytes"
@@ -6,27 +6,27 @@
 
 	"github.com/stretchr/testify/require"
 
-	"github.com/kopia/kopia/internal/cache"
+	"github.com/kopia/kopia/internal/cacheprot"
 	"github.com/kopia/kopia/internal/gather"
 )
 
 func TestNoStorageProection(t *testing.T) {
-	testStorageProtection(t, cache.NoProtection(), false)
+	testStorageProtection(t, cacheprot.NoProtection(), false)
 }
 
 func TestHMACStorageProtection(t *testing.T) {
-	testStorageProtection(t, cache.ChecksumProtection([]byte{1, 2, 3, 4}), true)
+	testStorageProtection(t, cacheprot.ChecksumProtection([]byte{1, 2, 3, 4}), true)
 }
 
 func TestEncryptionStorageProtection(t *testing.T) {
-	e, err := cache.AuthenticatedEncryptionProtection([]byte{1})
+	e, err := cacheprot.AuthenticatedEncryptionProtection([]byte{1})
 	require.NoError(t, err)
 
 	testStorageProtection(t, e, true)
 }
 
 //nolint:thelper
-func testStorageProtection(t *testing.T, sp cache.StorageProtection, protectsFromBitFlips bool) {
+func testStorageProtection(t *testing.T, sp cacheprot.StorageProtection, protectsFromBitFlips bool) {
 	payload := []byte{0, 1, 2, 3, 4}
 
 	var protected gather.WriteBuffer