style: mechanical lint conformance for #55's code under the canonical config
No behavior changes. Covers the purely mechanical findings the canonical
v2.12.2 config raises on the cache-size/eviction work:
- nolintlint: deleted 7 dead //nolint:gosec directives (cachesize.go x2,
config.go, eviction.go x2, storage.go x2). gosec never raises G115/G703
on those lines under the pinned toolchain, exactly the defect class
that failed round 2 of this PR. The 6 live gosec suppressions are
untouched.
- funcorder: moved writeIfAbsent after Exists (storage.go) and
touchVariant/touchSourceContent after IncrementStats (cache.go).
- lll: wrapped over-length signatures, calls and messages at 88 columns.
- paralleltest: t.Parallel() on the new eviction, contentlock and
cache_max_bytes tests and their subtests. configFromYAML uses only
t.TempDir, so the config cases are parallel-safe.
- noctx: test helper DB calls now use ExecContext/QueryContext/
QueryRowContext with t.Context().
- goconst: extracted testHeaderContentType into the shared test constant
block and testVariantKeyOne into the eviction tests; reused the
existing testContentTypeJPEG and keyCacheMaxBytes constants.
- modernize: interface{} to any, atomic.Int32 for the contentlock
counters, min/max in ComputeDefaultCacheMaxBytes.
- intrange: integer range loops in the contentlock tests.
- wsl_v5: whitespace before the contentlock rendezvous statements.
- sloglint: slog.DiscardHandler in the config test logger.
- cyclop/funlen: split TestZeroMaxBytesDisablesDiskCache into four
assertion helpers and extracted the concurrent store goroutine from
TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent. Every
assertion is preserved verbatim; only their grouping changed.
This commit is contained in:
@@ -2,7 +2,6 @@ package config
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"io"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
@@ -13,7 +12,7 @@ import (
|
||||
// discardLogger returns a logger that swallows all output, for tests
|
||||
// that exercise code paths which log.
|
||||
func discardLogger() *slog.Logger {
|
||||
return slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
return slog.New(slog.DiscardHandler)
|
||||
}
|
||||
|
||||
// TestCacheMaxBytesExplicitValueUsedWithoutFloor verifies that an
|
||||
@@ -21,6 +20,8 @@ func discardLogger() *slog.Logger {
|
||||
// given: the 500 MiB floor applies only to the computed default, never
|
||||
// to explicit values.
|
||||
func TestCacheMaxBytesExplicitValueUsedWithoutFloor(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
yamlContent := "signing_key: " + validTestSigningKey + "\ncache_max_bytes: 1024\n"
|
||||
|
||||
c, err := configFromYAML(t, yamlContent)
|
||||
@@ -38,6 +39,8 @@ func TestCacheMaxBytesExplicitValueUsedWithoutFloor(t *testing.T) {
|
||||
// explicit zero is a valid value (it disables the disk cache), not an
|
||||
// error.
|
||||
func TestCacheMaxBytesZeroIsValidAndDisablesCache(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
yamlContent := "signing_key: " + validTestSigningKey + "\ncache_max_bytes: 0\n"
|
||||
|
||||
c, err := configFromYAML(t, yamlContent)
|
||||
@@ -53,7 +56,10 @@ func TestCacheMaxBytesZeroIsValidAndDisablesCache(t *testing.T) {
|
||||
// TestCacheMaxBytesLargeExplicitValueParses verifies that values above
|
||||
// 32-bit range parse correctly (the field is an int64 byte count).
|
||||
func TestCacheMaxBytesLargeExplicitValueParses(t *testing.T) {
|
||||
yamlContent := "signing_key: " + validTestSigningKey + "\ncache_max_bytes: 10737418240\n"
|
||||
t.Parallel()
|
||||
|
||||
yamlContent := "signing_key: " + validTestSigningKey +
|
||||
"\ncache_max_bytes: 10737418240\n"
|
||||
|
||||
c, err := configFromYAML(t, yamlContent)
|
||||
if err != nil {
|
||||
@@ -70,6 +76,8 @@ func TestCacheMaxBytesLargeExplicitValueParses(t *testing.T) {
|
||||
// offending value, per the no-silent-fallback rule: defaults apply
|
||||
// only to omitted keys.
|
||||
func TestCacheMaxBytesInvalidValuesAbortStartup(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
signingKeyLine := "signing_key: " + validTestSigningKey + "\n"
|
||||
|
||||
cases := []struct {
|
||||
@@ -81,45 +89,48 @@ func TestCacheMaxBytesInvalidValuesAbortStartup(t *testing.T) {
|
||||
{
|
||||
name: "negative",
|
||||
yaml: signingKeyLine + "cache_max_bytes: -1024\n",
|
||||
wantErrSubstrings: []string{"cache_max_bytes", "-1024"},
|
||||
wantErrSubstrings: []string{keyCacheMaxBytes, "-1024"},
|
||||
},
|
||||
{
|
||||
name: "float",
|
||||
yaml: signingKeyLine + "cache_max_bytes: 3.5\n",
|
||||
wantErrSubstrings: []string{"cache_max_bytes", "3.5"},
|
||||
wantErrSubstrings: []string{keyCacheMaxBytes, "3.5"},
|
||||
},
|
||||
{
|
||||
name: "non-numeric string",
|
||||
yaml: signingKeyLine + "cache_max_bytes: banana\n",
|
||||
wantErrSubstrings: []string{"cache_max_bytes", "banana"},
|
||||
wantErrSubstrings: []string{keyCacheMaxBytes, "banana"},
|
||||
},
|
||||
{
|
||||
name: "explicit null",
|
||||
yaml: signingKeyLine + "cache_max_bytes: null\n",
|
||||
wantErrSubstrings: []string{"cache_max_bytes", "null"},
|
||||
wantErrSubstrings: []string{keyCacheMaxBytes, "null"},
|
||||
},
|
||||
{
|
||||
name: "bare key no value",
|
||||
yaml: signingKeyLine + "cache_max_bytes:\n",
|
||||
wantErrSubstrings: []string{"cache_max_bytes", "null"},
|
||||
wantErrSubstrings: []string{keyCacheMaxBytes, "null"},
|
||||
},
|
||||
{
|
||||
name: "boolean",
|
||||
yaml: signingKeyLine + "cache_max_bytes: true\n",
|
||||
wantErrSubstrings: []string{"cache_max_bytes", "true"},
|
||||
wantErrSubstrings: []string{keyCacheMaxBytes, "true"},
|
||||
},
|
||||
{
|
||||
name: "list",
|
||||
yaml: signingKeyLine + "cache_max_bytes:\n - 1\n",
|
||||
wantErrSubstrings: []string{"cache_max_bytes"},
|
||||
wantErrSubstrings: []string{keyCacheMaxBytes},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
c, err := configFromYAML(t, tc.yaml)
|
||||
if err == nil {
|
||||
t.Fatalf("config with %s cache_max_bytes must abort startup, got config: %+v",
|
||||
t.Fatalf(
|
||||
"config with %s cache_max_bytes must abort startup, got config: %+v",
|
||||
tc.name, c)
|
||||
}
|
||||
|
||||
@@ -138,6 +149,8 @@ func TestCacheMaxBytesInvalidValuesAbortStartup(t *testing.T) {
|
||||
// computed default is 75% of the probed free space when that exceeds
|
||||
// the floor.
|
||||
func TestComputeDefaultCacheMaxBytesUses75PercentOfFreeSpace(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// 4 GiB free -> 3 GiB default.
|
||||
probe := func(string) (uint64, error) { return 4294967296, nil }
|
||||
|
||||
@@ -147,7 +160,8 @@ func TestComputeDefaultCacheMaxBytesUses75PercentOfFreeSpace(t *testing.T) {
|
||||
}
|
||||
|
||||
if got != 3221225472 {
|
||||
t.Errorf("ComputeDefaultCacheMaxBytes = %d, want 3221225472 (75%% of 4 GiB)", got)
|
||||
t.Errorf("ComputeDefaultCacheMaxBytes = %d, want 3221225472 (75%% of 4 GiB)",
|
||||
got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -155,6 +169,8 @@ func TestComputeDefaultCacheMaxBytesUses75PercentOfFreeSpace(t *testing.T) {
|
||||
// verifies that when 75% of free space is below 500 MiB, the computed
|
||||
// default is floored at DefaultCacheMaxBytesFloor.
|
||||
func TestComputeDefaultCacheMaxBytesAppliesFloorToComputedDefault(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
freeBytes uint64
|
||||
@@ -166,6 +182,8 @@ func TestComputeDefaultCacheMaxBytesAppliesFloorToComputedDefault(t *testing.T)
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
probe := func(string) (uint64, error) { return tc.freeBytes, nil }
|
||||
|
||||
got, err := ComputeDefaultCacheMaxBytes(t.TempDir(), probe)
|
||||
@@ -185,6 +203,8 @@ func TestComputeDefaultCacheMaxBytesAppliesFloorToComputedDefault(t *testing.T)
|
||||
// failing free-space probe produces an error naming the config key,
|
||||
// instead of a silently wrong default.
|
||||
func TestComputeDefaultCacheMaxBytesPropagatesProbeError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
probe := func(string) (uint64, error) { return 0, errors.New("statfs failed") }
|
||||
|
||||
_, err := ComputeDefaultCacheMaxBytes(t.TempDir(), probe)
|
||||
@@ -194,7 +214,7 @@ func TestComputeDefaultCacheMaxBytesPropagatesProbeError(t *testing.T) {
|
||||
|
||||
t.Logf("got expected error: %v", err)
|
||||
|
||||
if !strings.Contains(err.Error(), "cache_max_bytes") {
|
||||
if !strings.Contains(err.Error(), keyCacheMaxBytes) {
|
||||
t.Errorf("error %q does not name the config key cache_max_bytes", err.Error())
|
||||
}
|
||||
}
|
||||
@@ -205,6 +225,8 @@ func TestComputeDefaultCacheMaxBytesPropagatesProbeError(t *testing.T) {
|
||||
// first so statfs measures the right filesystem), and that the result
|
||||
// lands on the Config.
|
||||
func TestResolveCacheMaxBytesComputesDefaultWhenOmitted(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
c, err := configFromYAML(t, "signing_key: "+validTestSigningKey+"\n")
|
||||
if err != nil {
|
||||
t.Fatalf("minimal config should be valid, got error: %v", err)
|
||||
@@ -227,11 +249,13 @@ func TestResolveCacheMaxBytesComputesDefaultWhenOmitted(t *testing.T) {
|
||||
}
|
||||
|
||||
if c.CacheMaxBytes != 3221225472 {
|
||||
t.Errorf("CacheMaxBytes = %d, want computed default 3221225472", c.CacheMaxBytes)
|
||||
t.Errorf("CacheMaxBytes = %d, want computed default 3221225472",
|
||||
c.CacheMaxBytes)
|
||||
}
|
||||
|
||||
if probedPath != wantCacheDir {
|
||||
t.Errorf("free space probed at %q, want cache directory %q", probedPath, wantCacheDir)
|
||||
t.Errorf("free space probed at %q, want cache directory %q",
|
||||
probedPath, wantCacheDir)
|
||||
}
|
||||
|
||||
info, err := os.Stat(wantCacheDir)
|
||||
@@ -245,6 +269,8 @@ func TestResolveCacheMaxBytesComputesDefaultWhenOmitted(t *testing.T) {
|
||||
// an explicitly configured value survives resolution untouched and
|
||||
// that the free-space probe is never consulted for it.
|
||||
func TestResolveCacheMaxBytesDoesNotOverrideExplicitValue(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
yamlContent := "signing_key: " + validTestSigningKey + "\ncache_max_bytes: 1024\n"
|
||||
|
||||
c, err := configFromYAML(t, yamlContent)
|
||||
|
||||
@@ -44,7 +44,7 @@ func defaultFreeSpaceProbe(path string) (uint64, error) {
|
||||
return 0, fmt.Errorf("statfs reported negative block size %d for %q", stat.Bsize, path)
|
||||
}
|
||||
|
||||
blockSize := uint64(stat.Bsize) //nolint:gosec // G115: negative Bsize rejected above
|
||||
blockSize := uint64(stat.Bsize)
|
||||
|
||||
return stat.Bavail * blockSize, nil
|
||||
}
|
||||
@@ -52,7 +52,9 @@ func defaultFreeSpaceProbe(path string) (uint64, error) {
|
||||
// ComputeDefaultCacheMaxBytes returns the default cache size limit for
|
||||
// the filesystem containing cacheDir: 75% of the free bytes reported
|
||||
// by probe, with a floor of DefaultCacheMaxBytesFloor.
|
||||
func ComputeDefaultCacheMaxBytes(cacheDir string, probe FreeSpaceProbeFunc) (int64, error) {
|
||||
func ComputeDefaultCacheMaxBytes(
|
||||
cacheDir string, probe FreeSpaceProbeFunc,
|
||||
) (int64, error) {
|
||||
freeBytes, err := probe(cacheDir)
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("config key %q: cannot determine free space for %q: %w",
|
||||
@@ -60,15 +62,10 @@ func ComputeDefaultCacheMaxBytes(cacheDir string, probe FreeSpaceProbeFunc) (int
|
||||
}
|
||||
|
||||
computed := freeBytes / freeSpaceFractionDenominator * freeSpaceFractionNumerator
|
||||
if computed > math.MaxInt64 {
|
||||
computed = math.MaxInt64
|
||||
}
|
||||
computed = min(computed, math.MaxInt64)
|
||||
|
||||
limit := int64(computed) //nolint:gosec // G115: clamped to MaxInt64 above
|
||||
|
||||
if limit < DefaultCacheMaxBytesFloor {
|
||||
limit = DefaultCacheMaxBytesFloor
|
||||
}
|
||||
limit := int64(computed)
|
||||
limit = max(limit, DefaultCacheMaxBytesFloor)
|
||||
|
||||
return limit, nil
|
||||
}
|
||||
@@ -79,7 +76,9 @@ func ComputeDefaultCacheMaxBytes(cacheDir string, probe FreeSpaceProbeFunc) (int
|
||||
// on free space in <state_dir>/cache/. The cache directory is created
|
||||
// first so statfs measures the filesystem that will actually hold the
|
||||
// cache. The effective limit is logged either way.
|
||||
func (c *Config) resolveCacheMaxBytes(log *slog.Logger, probe FreeSpaceProbeFunc) error {
|
||||
func (c *Config) resolveCacheMaxBytes(
|
||||
log *slog.Logger, probe FreeSpaceProbeFunc,
|
||||
) error {
|
||||
if !c.cacheMaxBytesExplicit {
|
||||
cacheDir := filepath.Join(c.StateDir, "cache")
|
||||
|
||||
|
||||
@@ -622,7 +622,7 @@ func getInt64(sc *smartconfig.Config, key string, defaultVal int64) (int64, erro
|
||||
key, val)
|
||||
}
|
||||
|
||||
return int64(val), nil //nolint:gosec // G115: bounds checked above
|
||||
return int64(val), nil
|
||||
case float64:
|
||||
if val != math.Trunc(val) {
|
||||
return 0, fmt.Errorf("config key %q: value %v is not an integer", key, val)
|
||||
|
||||
@@ -177,32 +177,6 @@ func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, e
|
||||
}, nil
|
||||
}
|
||||
|
||||
// touchVariant updates the LRU timestamp of a variant, best-effort:
|
||||
// a failed touch only makes the entry look colder to eviction.
|
||||
func (c *Cache) touchVariant(ctx context.Context, cacheKey VariantKey) {
|
||||
_, err := c.db.ExecContext(ctx, `
|
||||
UPDATE variant_content SET last_accessed_at = CURRENT_TIMESTAMP
|
||||
WHERE cache_key = ?
|
||||
`, string(cacheKey))
|
||||
if err != nil {
|
||||
c.log.Debug("failed to touch variant LRU timestamp",
|
||||
"cache_key", cacheKey, "error", err)
|
||||
}
|
||||
}
|
||||
|
||||
// touchSourceContent updates the LRU timestamp of a source content
|
||||
// blob, best-effort: a failed touch only makes the blob look colder.
|
||||
func (c *Cache) touchSourceContent(ctx context.Context, contentHash ContentHash) {
|
||||
_, err := c.db.ExecContext(ctx, `
|
||||
UPDATE source_content SET last_accessed_at = CURRENT_TIMESTAMP
|
||||
WHERE content_hash = ?
|
||||
`, string(contentHash))
|
||||
if err != nil {
|
||||
c.log.Debug("failed to touch source content LRU timestamp",
|
||||
"content_hash", contentHash, "error", err)
|
||||
}
|
||||
}
|
||||
|
||||
// GetVariant returns a reader, size, and content type for a cached variant.
|
||||
func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, error) {
|
||||
if c.disabled {
|
||||
@@ -506,6 +480,32 @@ func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64)
|
||||
}
|
||||
}
|
||||
|
||||
// touchVariant updates the LRU timestamp of a variant, best-effort:
|
||||
// a failed touch only makes the entry look colder to eviction.
|
||||
func (c *Cache) touchVariant(ctx context.Context, cacheKey VariantKey) {
|
||||
_, err := c.db.ExecContext(ctx, `
|
||||
UPDATE variant_content SET last_accessed_at = CURRENT_TIMESTAMP
|
||||
WHERE cache_key = ?
|
||||
`, string(cacheKey))
|
||||
if err != nil {
|
||||
c.log.Debug("failed to touch variant LRU timestamp",
|
||||
"cache_key", cacheKey, "error", err)
|
||||
}
|
||||
}
|
||||
|
||||
// touchSourceContent updates the LRU timestamp of a source content
|
||||
// blob, best-effort: a failed touch only makes the blob look colder.
|
||||
func (c *Cache) touchSourceContent(ctx context.Context, contentHash ContentHash) {
|
||||
_, err := c.db.ExecContext(ctx, `
|
||||
UPDATE source_content SET last_accessed_at = CURRENT_TIMESTAMP
|
||||
WHERE content_hash = ?
|
||||
`, string(contentHash))
|
||||
if err != nil {
|
||||
c.log.Debug("failed to touch source content LRU timestamp",
|
||||
"content_hash", contentHash, "error", err)
|
||||
}
|
||||
}
|
||||
|
||||
// checkNegativeCache checks if a request is in the negative cache.
|
||||
func (c *Cache) checkNegativeCache(
|
||||
ctx context.Context, req *ImageRequest,
|
||||
|
||||
@@ -160,7 +160,7 @@ func TestCache_StoreAndLookup(t *testing.T) {
|
||||
sourceContent := []byte("fake jpeg data")
|
||||
fetchResult := &httpfetcher.FetchResult{
|
||||
ContentType: testContentTypeJPEG,
|
||||
Headers: map[string][]string{"Content-Type": {testContentTypeJPEG}},
|
||||
Headers: map[string][]string{testHeaderContentType: {testContentTypeJPEG}},
|
||||
}
|
||||
|
||||
contentHash, err := cache.StoreSource(
|
||||
|
||||
@@ -10,10 +10,12 @@ import (
|
||||
// TestContentLockExcludesSameKey verifies that two goroutines locking
|
||||
// the same key never run their critical sections concurrently.
|
||||
func TestContentLockExcludesSameKey(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
lock := newContentLock()
|
||||
|
||||
var (
|
||||
active int32
|
||||
active atomic.Int32
|
||||
maxSeen int32
|
||||
wg sync.WaitGroup
|
||||
)
|
||||
@@ -22,14 +24,14 @@ func TestContentLockExcludesSameKey(t *testing.T) {
|
||||
|
||||
wg.Add(goroutines)
|
||||
|
||||
for i := 0; i < goroutines; i++ {
|
||||
for range goroutines {
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
|
||||
unlock := lock.Lock("same-key")
|
||||
defer unlock()
|
||||
|
||||
n := atomic.AddInt32(&active, 1)
|
||||
n := active.Add(1)
|
||||
|
||||
for {
|
||||
seen := atomic.LoadInt32(&maxSeen)
|
||||
@@ -40,7 +42,7 @@ func TestContentLockExcludesSameKey(t *testing.T) {
|
||||
|
||||
time.Sleep(time.Millisecond)
|
||||
|
||||
atomic.AddInt32(&active, -1)
|
||||
active.Add(-1)
|
||||
}()
|
||||
}
|
||||
|
||||
@@ -57,13 +59,15 @@ func TestContentLockExcludesSameKey(t *testing.T) {
|
||||
// them reaching the rendezvous point before any is allowed to
|
||||
// proceed.
|
||||
func TestContentLockAllowsDifferentKeys(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
lock := newContentLock()
|
||||
|
||||
const goroutines = 20
|
||||
|
||||
var (
|
||||
wg sync.WaitGroup
|
||||
inside int32
|
||||
inside atomic.Int32
|
||||
reached = make(chan struct{}, goroutines)
|
||||
)
|
||||
|
||||
@@ -71,7 +75,7 @@ func TestContentLockAllowsDifferentKeys(t *testing.T) {
|
||||
|
||||
release := make(chan struct{})
|
||||
|
||||
for i := 0; i < goroutines; i++ {
|
||||
for i := range goroutines {
|
||||
key := string(rune('a' + i))
|
||||
|
||||
go func() {
|
||||
@@ -80,8 +84,10 @@ func TestContentLockAllowsDifferentKeys(t *testing.T) {
|
||||
unlock := lock.Lock(key)
|
||||
defer unlock()
|
||||
|
||||
atomic.AddInt32(&inside, 1)
|
||||
inside.Add(1)
|
||||
|
||||
reached <- struct{}{}
|
||||
|
||||
<-release
|
||||
}()
|
||||
}
|
||||
@@ -90,7 +96,7 @@ func TestContentLockAllowsDifferentKeys(t *testing.T) {
|
||||
// own key's lock) without needing any other to release first. If
|
||||
// keys were incorrectly serialized onto one underlying lock, only
|
||||
// one would get here and this would time out.
|
||||
for i := 0; i < goroutines; i++ {
|
||||
for i := range goroutines {
|
||||
select {
|
||||
case <-reached:
|
||||
case <-time.After(2 * time.Second):
|
||||
@@ -99,8 +105,9 @@ func TestContentLockAllowsDifferentKeys(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
if n := atomic.LoadInt32(&inside); n != goroutines {
|
||||
t.Errorf("goroutines inside their critical section = %d, want %d", n, goroutines)
|
||||
if n := inside.Load(); n != goroutines {
|
||||
t.Errorf("goroutines inside their critical section = %d, want %d",
|
||||
n, goroutines)
|
||||
}
|
||||
|
||||
close(release)
|
||||
@@ -111,6 +118,8 @@ func TestContentLockAllowsDifferentKeys(t *testing.T) {
|
||||
// entries map does not grow without bound: once no goroutine holds or
|
||||
// awaits a key, its entry is removed.
|
||||
func TestContentLockRemovesEntryAfterUnlock(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
lock := newContentLock()
|
||||
|
||||
unlock := lock.Lock("k")
|
||||
|
||||
@@ -516,7 +516,9 @@ func (c *Cache) reconcileAccounting(ctx context.Context) error {
|
||||
// reconcileVariantFiles walks the variant storage directory, adopting
|
||||
// files without accounting rows and sweeping stale temp files.
|
||||
func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
|
||||
return filepath.WalkDir(c.variants.baseDir, func(path string, entry fs.DirEntry, err error) error {
|
||||
return filepath.WalkDir(
|
||||
c.variants.baseDir,
|
||||
func(path string, entry fs.DirEntry, err error) error {
|
||||
if err != nil || entry.IsDir() {
|
||||
return err
|
||||
}
|
||||
@@ -534,7 +536,8 @@ func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
|
||||
}
|
||||
|
||||
return c.adoptVariantFile(ctx, path, entry, VariantKey(name))
|
||||
})
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
// adoptVariantFile inserts an accounting row for a variant file that
|
||||
@@ -581,7 +584,8 @@ func (c *Cache) adoptVariantFile(
|
||||
// variantContentTypeFromSidecar reads the content type from a variant
|
||||
// .meta sidecar, falling back to application/octet-stream.
|
||||
func (c *Cache) variantContentTypeFromSidecar(variantPath string) string {
|
||||
metaData, err := os.ReadFile(variantPath + variantMetaSuffix) //nolint:gosec // path from cache walk
|
||||
//nolint:gosec // path from cache walk
|
||||
metaData, err := os.ReadFile(variantPath + variantMetaSuffix)
|
||||
if err != nil {
|
||||
return fallbackContentType
|
||||
}
|
||||
@@ -650,7 +654,9 @@ func (c *Cache) allVariantKeys(ctx context.Context) ([]VariantKey, error) {
|
||||
// lookups always go through source_metadata) and sweeping stale temp
|
||||
// files.
|
||||
func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
|
||||
return filepath.WalkDir(c.srcContent.baseDir, func(path string, entry fs.DirEntry, err error) error {
|
||||
return filepath.WalkDir(
|
||||
c.srcContent.baseDir,
|
||||
func(path string, entry fs.DirEntry, err error) error {
|
||||
if err != nil || entry.IsDir() {
|
||||
return err
|
||||
}
|
||||
@@ -664,7 +670,8 @@ func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
|
||||
}
|
||||
|
||||
return c.removeUntrackedSourceFile(ctx, path, ContentHash(name))
|
||||
})
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
// removeUntrackedSourceFile deletes a source blob file that has no
|
||||
@@ -691,7 +698,6 @@ func (c *Cache) removeUntrackedSourceFile(
|
||||
return fmt.Errorf("failed to delete metadata rows for untracked blob: %w", err)
|
||||
}
|
||||
|
||||
//nolint:gosec // G703: path comes from walking our own cache directory
|
||||
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
|
||||
return fmt.Errorf("failed to remove untracked source file: %w", err)
|
||||
}
|
||||
@@ -765,7 +771,6 @@ func (c *Cache) sweepStaleTempFile(path string, entry fs.DirEntry) {
|
||||
return
|
||||
}
|
||||
|
||||
//nolint:gosec // G703: path comes from walking our own cache directory
|
||||
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
|
||||
c.log.Warn("failed to remove stale temp file", "path", path, "error", err)
|
||||
|
||||
|
||||
@@ -20,6 +20,10 @@ import (
|
||||
// implementation writes.
|
||||
const sqliteTimestampFormat = "2006-01-02 15:04:05"
|
||||
|
||||
// testVariantKeyOne is the variant cache key reused across the eviction
|
||||
// tests as the first stored variant.
|
||||
const testVariantKeyOne VariantKey = "aabbccdd0001"
|
||||
|
||||
// evictionTestDB creates an in-memory SQLite database with the real
|
||||
// production schema, limited to a single connection so the background
|
||||
// eviction goroutine shares the same in-memory database as the test.
|
||||
@@ -85,12 +89,16 @@ func storeEvictionTestSource(
|
||||
|
||||
result := &httpfetcher.FetchResult{
|
||||
StatusCode: 200,
|
||||
ContentType: "image/jpeg",
|
||||
ContentType: testContentTypeJPEG,
|
||||
ContentLength: int64(len(content)),
|
||||
Headers: map[string][]string{"Content-Type": {"image/jpeg"}},
|
||||
Headers: map[string][]string{
|
||||
testHeaderContentType: {testContentTypeJPEG},
|
||||
},
|
||||
}
|
||||
|
||||
hash, err := cache.StoreSource(context.Background(), req, bytes.NewReader(content), result)
|
||||
hash, err := cache.StoreSource(
|
||||
context.Background(), req, bytes.NewReader(content), result,
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("StoreSource(%s%s) failed: %v", host, path, err)
|
||||
}
|
||||
@@ -100,20 +108,25 @@ func storeEvictionTestSource(
|
||||
|
||||
// storeEvictionTestVariant stores content as a processed variant under
|
||||
// the given cache key.
|
||||
func storeEvictionTestVariant(t *testing.T, cache *Cache, key VariantKey, content []byte) {
|
||||
func storeEvictionTestVariant(
|
||||
t *testing.T, cache *Cache, key VariantKey, content []byte,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
if err := cache.StoreVariant(key, bytes.NewReader(content), "image/webp"); err != nil {
|
||||
err := cache.StoreVariant(key, bytes.NewReader(content), "image/webp")
|
||||
if err != nil {
|
||||
t.Fatalf("StoreVariant(%s) failed: %v", key, err)
|
||||
}
|
||||
}
|
||||
|
||||
// setVariantLastAccessed backdates the last access time of a tracked
|
||||
// variant, to make LRU ordering deterministic in tests.
|
||||
func setVariantLastAccessed(t *testing.T, cache *Cache, key VariantKey, when time.Time) {
|
||||
func setVariantLastAccessed(
|
||||
t *testing.T, cache *Cache, key VariantKey, when time.Time,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
res, err := cache.db.Exec(
|
||||
res, err := cache.db.ExecContext(t.Context(),
|
||||
`UPDATE variant_content SET last_accessed_at = ? WHERE cache_key = ?`,
|
||||
when.UTC().Format(sqliteTimestampFormat), string(key),
|
||||
)
|
||||
@@ -134,10 +147,12 @@ func setVariantLastAccessed(t *testing.T, cache *Cache, key VariantKey, when tim
|
||||
|
||||
// setSourceLastAccessed backdates the last access time of a tracked
|
||||
// source content blob.
|
||||
func setSourceLastAccessed(t *testing.T, cache *Cache, hash ContentHash, when time.Time) {
|
||||
func setSourceLastAccessed(
|
||||
t *testing.T, cache *Cache, hash ContentHash, when time.Time,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
res, err := cache.db.Exec(
|
||||
res, err := cache.db.ExecContext(t.Context(),
|
||||
`UPDATE source_content SET last_accessed_at = ? WHERE content_hash = ?`,
|
||||
when.UTC().Format(sqliteTimestampFormat), string(hash),
|
||||
)
|
||||
@@ -156,11 +171,13 @@ func setSourceLastAccessed(t *testing.T, cache *Cache, hash ContentHash, when ti
|
||||
}
|
||||
|
||||
// countRows returns the number of rows the given query yields.
|
||||
func countRows(t *testing.T, cache *Cache, query string, args ...interface{}) int {
|
||||
func countRows(t *testing.T, cache *Cache, query string, args ...any) int {
|
||||
t.Helper()
|
||||
|
||||
var n int
|
||||
if err := cache.db.QueryRow(query, args...).Scan(&n); err != nil {
|
||||
|
||||
err := cache.db.QueryRowContext(t.Context(), query, args...).Scan(&n)
|
||||
if err != nil {
|
||||
t.Fatalf("count query %q failed: %v", query, err)
|
||||
}
|
||||
|
||||
@@ -173,7 +190,7 @@ func countRows(t *testing.T, cache *Cache, query string, args ...interface{}) in
|
||||
func assertNoDanglingReferences(t *testing.T, cache *Cache) {
|
||||
t.Helper()
|
||||
|
||||
rows, err := cache.db.Query(
|
||||
rows, err := cache.db.QueryContext(t.Context(),
|
||||
`SELECT content_hash FROM source_metadata
|
||||
WHERE content_hash IS NOT NULL AND content_hash != ''`,
|
||||
)
|
||||
@@ -185,20 +202,25 @@ func assertNoDanglingReferences(t *testing.T, cache *Cache) {
|
||||
|
||||
for rows.Next() {
|
||||
var hash string
|
||||
if err := rows.Scan(&hash); err != nil {
|
||||
|
||||
err := rows.Scan(&hash)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to scan content_hash: %v", err)
|
||||
}
|
||||
|
||||
if !cache.srcContent.Exists(ContentHash(hash)) {
|
||||
t.Errorf("source_metadata references content %s but the file is missing", hash)
|
||||
t.Errorf("source_metadata references content %s but the file is missing",
|
||||
hash)
|
||||
}
|
||||
}
|
||||
|
||||
if err := rows.Err(); err != nil {
|
||||
err = rows.Err()
|
||||
if err != nil {
|
||||
t.Fatalf("source_metadata iteration failed: %v", err)
|
||||
}
|
||||
|
||||
variantRows, err := cache.db.Query(`SELECT cache_key FROM variant_content`)
|
||||
variantRows, err := cache.db.QueryContext(t.Context(),
|
||||
`SELECT cache_key FROM variant_content`)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to query variant_content: %v", err)
|
||||
}
|
||||
@@ -207,7 +229,9 @@ func assertNoDanglingReferences(t *testing.T, cache *Cache) {
|
||||
|
||||
for variantRows.Next() {
|
||||
var key string
|
||||
if err := variantRows.Scan(&key); err != nil {
|
||||
|
||||
err := variantRows.Scan(&key)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to scan cache_key: %v", err)
|
||||
}
|
||||
|
||||
@@ -216,14 +240,17 @@ func assertNoDanglingReferences(t *testing.T, cache *Cache) {
|
||||
}
|
||||
}
|
||||
|
||||
if err := variantRows.Err(); err != nil {
|
||||
err = variantRows.Err()
|
||||
if err != nil {
|
||||
t.Fatalf("variant_content iteration failed: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// waitForUsageAtOrBelow polls UsageBytes until it reaches limit or the
|
||||
// timeout expires, returning the last observed usage.
|
||||
func waitForUsageAtOrBelow(t *testing.T, cache *Cache, limit int64, timeout time.Duration) int64 {
|
||||
func waitForUsageAtOrBelow(
|
||||
t *testing.T, cache *Cache, limit int64, timeout time.Duration,
|
||||
) int64 {
|
||||
t.Helper()
|
||||
|
||||
deadline := time.Now().Add(timeout)
|
||||
@@ -249,14 +276,18 @@ func waitForUsageAtOrBelow(t *testing.T, cache *Cache, limit int64, timeout time
|
||||
}
|
||||
|
||||
func TestUsageBytesAccountsSourceAndVariantBytes(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cache, _ := newEvictionTestCache(t, 1<<30)
|
||||
|
||||
storeEvictionTestSource(t, cache, "src.example.com", "/a.jpg",
|
||||
bytes.Repeat([]byte{0xAA}, 1000))
|
||||
storeEvictionTestSource(t, cache, "src.example.com", "/b.jpg",
|
||||
bytes.Repeat([]byte{0xAB}, 2000))
|
||||
storeEvictionTestVariant(t, cache, "aabbccdd0001", bytes.Repeat([]byte{0xAC}, 500))
|
||||
storeEvictionTestVariant(t, cache, "aabbccdd0002", bytes.Repeat([]byte{0xAD}, 250))
|
||||
storeEvictionTestVariant(t, cache, testVariantKeyOne,
|
||||
bytes.Repeat([]byte{0xAC}, 500))
|
||||
storeEvictionTestVariant(t, cache, "aabbccdd0002",
|
||||
bytes.Repeat([]byte{0xAD}, 250))
|
||||
|
||||
usage, err := cache.UsageBytes(context.Background())
|
||||
if err != nil {
|
||||
@@ -269,15 +300,20 @@ func TestUsageBytesAccountsSourceAndVariantBytes(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestUsageBytesCountsMultiReferencedBlobOnce(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cache, _ := newEvictionTestCache(t, 1<<30)
|
||||
|
||||
content := bytes.Repeat([]byte{0xCC}, 1200)
|
||||
|
||||
hashOne := storeEvictionTestSource(t, cache, "src.example.com", "/one.jpg", content)
|
||||
hashTwo := storeEvictionTestSource(t, cache, "src.example.com", "/two.jpg", content)
|
||||
hashOne := storeEvictionTestSource(t, cache,
|
||||
"src.example.com", "/one.jpg", content)
|
||||
hashTwo := storeEvictionTestSource(t, cache,
|
||||
"src.example.com", "/two.jpg", content)
|
||||
|
||||
if hashOne != hashTwo {
|
||||
t.Fatalf("identical content produced different hashes: %s vs %s", hashOne, hashTwo)
|
||||
t.Fatalf("identical content produced different hashes: %s vs %s",
|
||||
hashOne, hashTwo)
|
||||
}
|
||||
|
||||
usage, err := cache.UsageBytes(context.Background())
|
||||
@@ -291,12 +327,16 @@ func TestUsageBytesCountsMultiReferencedBlobOnce(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestEvictToLimitEvictsLeastRecentlyUsedFirst(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const limit = 3000
|
||||
|
||||
cache, _ := newEvictionTestCache(t, limit)
|
||||
|
||||
now := time.Now()
|
||||
keys := []VariantKey{"aabbccdd0001", "aabbccdd0002", "aabbccdd0003", "aabbccdd0004"}
|
||||
keys := []VariantKey{
|
||||
testVariantKeyOne, "aabbccdd0002", "aabbccdd0003", "aabbccdd0004",
|
||||
}
|
||||
fills := []byte{0x01, 0x02, 0x03, 0x04}
|
||||
ages := []time.Duration{4 * time.Hour, 3 * time.Hour, 2 * time.Hour, 1 * time.Hour}
|
||||
|
||||
@@ -338,6 +378,8 @@ func TestEvictToLimitEvictsLeastRecentlyUsedFirst(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestEvictionRemovesMultiReferencedBlobTogetherWithAllReferences(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const limit = 1000
|
||||
|
||||
cache, _ := newEvictionTestCache(t, limit)
|
||||
@@ -346,10 +388,14 @@ func TestEvictionRemovesMultiReferencedBlobTogetherWithAllReferences(t *testing.
|
||||
|
||||
// One 800-byte blob referenced by two source paths.
|
||||
sharedContent := bytes.Repeat([]byte{0xDD}, 800)
|
||||
sharedHash := storeEvictionTestSource(t, cache, "src.example.com", "/a.jpg", sharedContent)
|
||||
sharedHash := storeEvictionTestSource(t, cache,
|
||||
"src.example.com", "/a.jpg", sharedContent)
|
||||
|
||||
if h := storeEvictionTestSource(t, cache, "src.example.com", "/b.jpg", sharedContent); h != sharedHash {
|
||||
t.Fatalf("identical content produced different hashes: %s vs %s", h, sharedHash)
|
||||
h := storeEvictionTestSource(t, cache,
|
||||
"src.example.com", "/b.jpg", sharedContent)
|
||||
if h != sharedHash {
|
||||
t.Fatalf("identical content produced different hashes: %s vs %s",
|
||||
h, sharedHash)
|
||||
}
|
||||
|
||||
// A newer 600-byte blob referenced by one source path.
|
||||
@@ -408,23 +454,28 @@ func TestEvictionRemovesMultiReferencedBlobTogetherWithAllReferences(t *testing.
|
||||
if n := countRows(t, cache,
|
||||
`SELECT COUNT(*) FROM source_metadata WHERE content_hash = ?`, string(recentHash),
|
||||
); n != 1 {
|
||||
t.Errorf("recently used blob %s has %d source_metadata rows, want 1", recentHash, n)
|
||||
t.Errorf("recently used blob %s has %d source_metadata rows, want 1",
|
||||
recentHash, n)
|
||||
}
|
||||
|
||||
assertNoDanglingReferences(t, cache)
|
||||
}
|
||||
|
||||
func TestEvictionKeepsEverythingWhenUnderLimit(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cache, _ := newEvictionTestCache(t, 1<<30)
|
||||
|
||||
content := bytes.Repeat([]byte{0xDF}, 800)
|
||||
hash := storeEvictionTestSource(t, cache, "src.example.com", "/a.jpg", content)
|
||||
|
||||
if h := storeEvictionTestSource(t, cache, "src.example.com", "/b.jpg", content); h != hash {
|
||||
h := storeEvictionTestSource(t, cache, "src.example.com", "/b.jpg", content)
|
||||
if h != hash {
|
||||
t.Fatalf("identical content produced different hashes: %s vs %s", h, hash)
|
||||
}
|
||||
|
||||
storeEvictionTestVariant(t, cache, "aabbccdd0001", bytes.Repeat([]byte{0xE0}, 500))
|
||||
storeEvictionTestVariant(t, cache, testVariantKeyOne,
|
||||
bytes.Repeat([]byte{0xE0}, 500))
|
||||
|
||||
if err := cache.EvictToLimit(context.Background()); err != nil {
|
||||
t.Fatalf("EvictToLimit failed: %v", err)
|
||||
@@ -440,7 +491,7 @@ func TestEvictionKeepsEverythingWhenUnderLimit(t *testing.T) {
|
||||
t.Errorf("blob %s has %d source_metadata rows, want 2", hash, n)
|
||||
}
|
||||
|
||||
if !cache.variants.Exists("aabbccdd0001") {
|
||||
if !cache.variants.Exists(testVariantKeyOne) {
|
||||
t.Error("variant must not be evicted while usage is under the limit")
|
||||
}
|
||||
|
||||
@@ -448,8 +499,9 @@ func TestEvictionKeepsEverythingWhenUnderLimit(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestZeroMaxBytesDisablesDiskCache(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cache, tmpDir := newEvictionTestCache(t, 0)
|
||||
ctx := context.Background()
|
||||
|
||||
req := &ImageRequest{
|
||||
SourceHost: "src.example.com",
|
||||
@@ -459,14 +511,31 @@ func TestZeroMaxBytesDisablesDiskCache(t *testing.T) {
|
||||
FitMode: FitCover,
|
||||
}
|
||||
|
||||
// Writes are no-ops that report success.
|
||||
if err := cache.StoreVariant(CacheKey(req), bytes.NewReader([]byte("data")), "image/webp"); err != nil {
|
||||
assertDisabledCacheWritesAreNoOps(t, cache, req)
|
||||
assertDisabledCacheReadsAlwaysMiss(t, cache, req)
|
||||
assertDisabledCacheTracksNothing(t, cache)
|
||||
assertDisabledCacheWroteNothingToDisk(t, tmpDir)
|
||||
}
|
||||
|
||||
// assertDisabledCacheWritesAreNoOps verifies that stores against a
|
||||
// disabled cache report success without recording anything.
|
||||
func assertDisabledCacheWritesAreNoOps(
|
||||
t *testing.T, cache *Cache, req *ImageRequest,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
ctx := t.Context()
|
||||
|
||||
err := cache.StoreVariant(
|
||||
CacheKey(req), bytes.NewReader([]byte("data")), "image/webp",
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("StoreVariant on disabled cache must be a no-op, got error: %v", err)
|
||||
}
|
||||
|
||||
result := &httpfetcher.FetchResult{
|
||||
StatusCode: 200,
|
||||
ContentType: "image/jpeg",
|
||||
ContentType: testContentTypeJPEG,
|
||||
ContentLength: 4,
|
||||
Headers: map[string][]string{},
|
||||
}
|
||||
@@ -479,8 +548,17 @@ func TestZeroMaxBytesDisablesDiskCache(t *testing.T) {
|
||||
if hash != "" {
|
||||
t.Errorf("StoreSource on disabled cache returned hash %q, want empty", hash)
|
||||
}
|
||||
}
|
||||
|
||||
// assertDisabledCacheReadsAlwaysMiss verifies that lookups against a
|
||||
// disabled cache never report a hit.
|
||||
func assertDisabledCacheReadsAlwaysMiss(
|
||||
t *testing.T, cache *Cache, req *ImageRequest,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
ctx := t.Context()
|
||||
|
||||
// Reads always miss.
|
||||
lookup, err := cache.Lookup(ctx, req)
|
||||
if err != nil {
|
||||
t.Fatalf("Lookup on disabled cache failed: %v", err)
|
||||
@@ -496,11 +574,17 @@ func TestZeroMaxBytesDisablesDiskCache(t *testing.T) {
|
||||
}
|
||||
|
||||
if srcHash != "" || srcType != "" {
|
||||
t.Errorf("LookupSource on disabled cache = (%q, %q), want empty", srcHash, srcType)
|
||||
t.Errorf("LookupSource on disabled cache = (%q, %q), want empty",
|
||||
srcHash, srcType)
|
||||
}
|
||||
}
|
||||
|
||||
// Nothing is tracked and nothing is written to disk.
|
||||
usage, err := cache.UsageBytes(ctx)
|
||||
// assertDisabledCacheTracksNothing verifies that a disabled cache
|
||||
// records no usage and writes no accounting rows.
|
||||
func assertDisabledCacheTracksNothing(t *testing.T, cache *Cache) {
|
||||
t.Helper()
|
||||
|
||||
usage, err := cache.UsageBytes(t.Context())
|
||||
if err != nil {
|
||||
t.Fatalf("UsageBytes failed: %v", err)
|
||||
}
|
||||
@@ -516,14 +600,23 @@ func TestZeroMaxBytesDisablesDiskCache(t *testing.T) {
|
||||
if n := countRows(t, cache, `SELECT COUNT(*) FROM source_metadata`); n != 0 {
|
||||
t.Errorf("disabled cache wrote %d source_metadata rows, want 0", n)
|
||||
}
|
||||
}
|
||||
|
||||
if _, err := os.Stat(filepath.Join(tmpDir, "cache")); !os.IsNotExist(err) {
|
||||
t.Errorf("disabled cache must not create the cache directory tree (stat err=%v)", err)
|
||||
// assertDisabledCacheWroteNothingToDisk verifies that a disabled cache
|
||||
// creates neither the cache directory tree nor any file under stateDir.
|
||||
func assertDisabledCacheWroteNothingToDisk(t *testing.T, stateDir string) {
|
||||
t.Helper()
|
||||
|
||||
_, err := os.Stat(filepath.Join(stateDir, "cache"))
|
||||
if !os.IsNotExist(err) {
|
||||
t.Errorf("disabled cache must not create the cache directory tree "+
|
||||
"(stat err=%v)", err)
|
||||
}
|
||||
|
||||
var foundFiles []string
|
||||
|
||||
walkErr := filepath.WalkDir(tmpDir, func(path string, d fs.DirEntry, err error) error {
|
||||
walkErr := filepath.WalkDir(stateDir,
|
||||
func(path string, d fs.DirEntry, err error) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
@@ -544,6 +637,8 @@ func TestZeroMaxBytesDisablesDiskCache(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestEvictionRunsUnderWritePressure(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const limit = 1500
|
||||
|
||||
cache, _ := newEvictionTestCache(t, limit)
|
||||
@@ -553,11 +648,12 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) {
|
||||
cache.StartEviction(time.Hour)
|
||||
defer cache.StopEviction()
|
||||
|
||||
keys := []VariantKey{"aabbccdd0001", "aabbccdd0002", "aabbccdd0003"}
|
||||
keys := []VariantKey{testVariantKeyOne, "aabbccdd0002", "aabbccdd0003"}
|
||||
fills := []byte{0x11, 0x12, 0x13}
|
||||
|
||||
for i, key := range keys {
|
||||
storeEvictionTestVariant(t, cache, key, bytes.Repeat([]byte{fills[i]}, 1000))
|
||||
storeEvictionTestVariant(t, cache, key,
|
||||
bytes.Repeat([]byte{fills[i]}, 1000))
|
||||
}
|
||||
|
||||
usage := waitForUsageAtOrBelow(t, cache, limit, 5*time.Second)
|
||||
@@ -570,6 +666,8 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const limit = 1500
|
||||
|
||||
cache, _ := newEvictionTestCache(t, limit)
|
||||
@@ -581,21 +679,23 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
|
||||
cache.StartEviction(100 * time.Millisecond)
|
||||
defer cache.StopEviction()
|
||||
|
||||
keys := []VariantKey{"aabbccdd0001", "aabbccdd0002", "aabbccdd0003"}
|
||||
keys := []VariantKey{testVariantKeyOne, "aabbccdd0002", "aabbccdd0003"}
|
||||
fills := []byte{0x21, 0x22, 0x23}
|
||||
|
||||
for i, key := range keys {
|
||||
content := bytes.Repeat([]byte{fills[i]}, 1000)
|
||||
|
||||
if _, err := cache.variants.Store(key, bytes.NewReader(content), "image/webp"); err != nil {
|
||||
_, err := cache.variants.Store(key, bytes.NewReader(content), "image/webp")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to store variant file: %v", err)
|
||||
}
|
||||
|
||||
if _, err := cache.db.Exec(
|
||||
_, err = cache.db.ExecContext(t.Context(),
|
||||
`INSERT INTO variant_content (cache_key, size_bytes, content_type)
|
||||
VALUES (?, ?, ?)`,
|
||||
string(key), len(content), "image/webp",
|
||||
); err != nil {
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to insert variant accounting row: %v", err)
|
||||
}
|
||||
}
|
||||
@@ -610,21 +710,28 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestStartEvictionReconcilesAccountingWithDisk(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cache, _ := newEvictionTestCache(t, 1<<30)
|
||||
|
||||
// An untracked variant file on disk (e.g. written before this
|
||||
// feature existed) must be adopted into the accounting.
|
||||
untracked := bytes.Repeat([]byte{0x31}, 1000)
|
||||
if _, err := cache.variants.Store("aabbccdd0001", bytes.NewReader(untracked), "image/webp"); err != nil {
|
||||
|
||||
_, err := cache.variants.Store(
|
||||
testVariantKeyOne, bytes.NewReader(untracked), "image/webp",
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to store untracked variant file: %v", err)
|
||||
}
|
||||
|
||||
// An accounting row whose file is missing must be dropped.
|
||||
if _, err := cache.db.Exec(
|
||||
_, err = cache.db.ExecContext(t.Context(),
|
||||
`INSERT INTO variant_content (cache_key, size_bytes, content_type)
|
||||
VALUES (?, ?, ?)`,
|
||||
"deadbeef0001", 700, "image/webp",
|
||||
); err != nil {
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to insert stale variant accounting row: %v", err)
|
||||
}
|
||||
|
||||
@@ -656,7 +763,8 @@ func TestStartEvictionReconcilesAccountingWithDisk(t *testing.T) {
|
||||
}
|
||||
|
||||
if n := countRows(t, cache,
|
||||
`SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`, "aabbccdd0001",
|
||||
`SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`,
|
||||
string(testVariantKeyOne),
|
||||
); n != 1 {
|
||||
t.Errorf("untracked variant file was not adopted into accounting (rows=%d)", n)
|
||||
}
|
||||
@@ -678,6 +786,8 @@ func TestStartEvictionReconcilesAccountingWithDisk(t *testing.T) {
|
||||
// staying invisible to UsageBytes/EvictToLimit until the next process
|
||||
// restart.
|
||||
func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cache, _ := newEvictionTestCache(t, 1<<30)
|
||||
|
||||
const interval = 100 * time.Millisecond
|
||||
@@ -696,7 +806,11 @@ func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
|
||||
// accounting insert) entirely, exactly as would happen if that
|
||||
// insert had failed and only the file write had succeeded.
|
||||
untracked := bytes.Repeat([]byte{0x41}, 900)
|
||||
if _, err := cache.variants.Store("aabbccdd0099", bytes.NewReader(untracked), "image/webp"); err != nil {
|
||||
|
||||
_, err := cache.variants.Store(
|
||||
"aabbccdd0099", bytes.NewReader(untracked), "image/webp",
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to store untracked variant file: %v", err)
|
||||
}
|
||||
|
||||
@@ -747,7 +861,8 @@ func TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent(t *testing.T)
|
||||
|
||||
content := bytes.Repeat([]byte{0x55}, 400)
|
||||
|
||||
hash := storeEvictionTestSource(t, cache, "race.example.com", "/first.jpg", content)
|
||||
hash := storeEvictionTestSource(t, cache,
|
||||
"race.example.com", "/first.jpg", content)
|
||||
|
||||
proceed := make(chan struct{})
|
||||
storeAttempted := make(chan struct{})
|
||||
@@ -772,26 +887,7 @@ func TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent(t *testing.T)
|
||||
// content file: exactly the window the review flagged.
|
||||
<-storeAttempted
|
||||
|
||||
storeDone := make(chan error, 1)
|
||||
|
||||
go func() {
|
||||
req := &ImageRequest{
|
||||
SourceHost: "race.example.com",
|
||||
SourcePath: "/dup.jpg",
|
||||
Format: FormatJPEG,
|
||||
Quality: 85,
|
||||
FitMode: FitCover,
|
||||
}
|
||||
result := &httpfetcher.FetchResult{
|
||||
StatusCode: 200,
|
||||
ContentType: "image/jpeg",
|
||||
ContentLength: int64(len(content)),
|
||||
Headers: map[string][]string{"Content-Type": {"image/jpeg"}},
|
||||
}
|
||||
|
||||
_, err := cache.StoreSource(ctx, req, bytes.NewReader(content), result)
|
||||
storeDone <- err
|
||||
}()
|
||||
storeDone := storeIdenticalContentConcurrently(ctx, cache, content)
|
||||
|
||||
// The concurrent store must not be able to complete while eviction
|
||||
// still holds the content hash (i.e. before the file is unlinked):
|
||||
@@ -832,6 +928,39 @@ func TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent(t *testing.T)
|
||||
}
|
||||
|
||||
if !cache.srcContent.Exists(dupHash) {
|
||||
t.Errorf("source_content/source_metadata references %s but its file is missing", dupHash)
|
||||
t.Errorf("source_content/source_metadata references %s but its file is missing",
|
||||
dupHash)
|
||||
}
|
||||
}
|
||||
|
||||
// storeIdenticalContentConcurrently starts a StoreSource for a second
|
||||
// source path whose body hashes to the same content hash, returning the
|
||||
// channel its error is delivered on.
|
||||
func storeIdenticalContentConcurrently(
|
||||
ctx context.Context, cache *Cache, content []byte,
|
||||
) chan error {
|
||||
storeDone := make(chan error, 1)
|
||||
|
||||
go func() {
|
||||
req := &ImageRequest{
|
||||
SourceHost: "race.example.com",
|
||||
SourcePath: "/dup.jpg",
|
||||
Format: FormatJPEG,
|
||||
Quality: 85,
|
||||
FitMode: FitCover,
|
||||
}
|
||||
result := &httpfetcher.FetchResult{
|
||||
StatusCode: 200,
|
||||
ContentType: testContentTypeJPEG,
|
||||
ContentLength: int64(len(content)),
|
||||
Headers: map[string][]string{
|
||||
testHeaderContentType: {testContentTypeJPEG},
|
||||
},
|
||||
}
|
||||
|
||||
_, err := cache.StoreSource(ctx, req, bytes.NewReader(content), result)
|
||||
storeDone <- err
|
||||
}()
|
||||
|
||||
return storeDone
|
||||
}
|
||||
|
||||
@@ -79,7 +79,9 @@ func (s *ContentStorage) Store(r io.Reader) (ContentHash, int64, error) {
|
||||
// across the whole store operation) use this instead of Store. Like
|
||||
// Store, it is idempotent: content already on disk at that path is
|
||||
// left untouched.
|
||||
func (s *ContentStorage) StoreHashed(hash ContentHash, data []byte) (size int64, err error) {
|
||||
func (s *ContentStorage) StoreHashed(
|
||||
hash ContentHash, data []byte,
|
||||
) (size int64, err error) {
|
||||
if err := s.writeIfAbsent(hash, data); err != nil {
|
||||
return 0, err
|
||||
}
|
||||
@@ -87,57 +89,6 @@ func (s *ContentStorage) StoreHashed(hash ContentHash, data []byte) (size int64,
|
||||
return int64(len(data)), nil
|
||||
}
|
||||
|
||||
// writeIfAbsent writes data to the path derived from hash, unless
|
||||
// content already exists there, via a temp-file-plus-rename so
|
||||
// concurrent readers never observe a partial file.
|
||||
func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) (err error) {
|
||||
// Build path: <basedir>/<ab>/<cd>/<hash>
|
||||
path := s.hashToPath(hash)
|
||||
|
||||
// Check if already exists
|
||||
if _, statErr := os.Stat(path); statErr == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Create directory structure
|
||||
dir := filepath.Dir(path)
|
||||
if err := os.MkdirAll(dir, StorageDirPerm); err != nil {
|
||||
return fmt.Errorf("failed to create directory: %w", err)
|
||||
}
|
||||
|
||||
// Write to temp file first, then rename for atomicity
|
||||
tmpFile, err := os.CreateTemp(dir, ".tmp-*")
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create temp file: %w", err)
|
||||
}
|
||||
|
||||
tmpPath := tmpFile.Name()
|
||||
|
||||
defer func() {
|
||||
if err != nil {
|
||||
_ = os.Remove(tmpPath)
|
||||
}
|
||||
}()
|
||||
|
||||
if _, err := tmpFile.Write(data); err != nil {
|
||||
_ = tmpFile.Close()
|
||||
|
||||
return fmt.Errorf("failed to write content: %w", err)
|
||||
}
|
||||
|
||||
if err := tmpFile.Close(); err != nil {
|
||||
return fmt.Errorf("failed to close temp file: %w", err)
|
||||
}
|
||||
|
||||
// Atomic rename
|
||||
//nolint:gosec // G703: paths from internal SHA256 hashes
|
||||
if err := os.Rename(filepath.Clean(tmpPath), filepath.Clean(path)); err != nil {
|
||||
return fmt.Errorf("failed to rename temp file: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// Load returns a reader for the content with the given hash.
|
||||
func (s *ContentStorage) Load(hash ContentHash) (io.ReadCloser, error) {
|
||||
path := s.hashToPath(hash)
|
||||
@@ -197,6 +148,56 @@ func (s *ContentStorage) Exists(hash ContentHash) bool {
|
||||
return err == nil
|
||||
}
|
||||
|
||||
// writeIfAbsent writes data to the path derived from hash, unless
|
||||
// content already exists there, via a temp-file-plus-rename so
|
||||
// concurrent readers never observe a partial file.
|
||||
func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) (err error) {
|
||||
// Build path: <basedir>/<ab>/<cd>/<hash>
|
||||
path := s.hashToPath(hash)
|
||||
|
||||
// Check if already exists
|
||||
if _, statErr := os.Stat(path); statErr == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Create directory structure
|
||||
dir := filepath.Dir(path)
|
||||
if err := os.MkdirAll(dir, StorageDirPerm); err != nil {
|
||||
return fmt.Errorf("failed to create directory: %w", err)
|
||||
}
|
||||
|
||||
// Write to temp file first, then rename for atomicity
|
||||
tmpFile, err := os.CreateTemp(dir, ".tmp-*")
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to create temp file: %w", err)
|
||||
}
|
||||
|
||||
tmpPath := tmpFile.Name()
|
||||
|
||||
defer func() {
|
||||
if err != nil {
|
||||
_ = os.Remove(tmpPath)
|
||||
}
|
||||
}()
|
||||
|
||||
if _, err := tmpFile.Write(data); err != nil {
|
||||
_ = tmpFile.Close()
|
||||
|
||||
return fmt.Errorf("failed to write content: %w", err)
|
||||
}
|
||||
|
||||
if err := tmpFile.Close(); err != nil {
|
||||
return fmt.Errorf("failed to close temp file: %w", err)
|
||||
}
|
||||
|
||||
// Atomic rename
|
||||
if err := os.Rename(filepath.Clean(tmpPath), filepath.Clean(path)); err != nil {
|
||||
return fmt.Errorf("failed to rename temp file: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// hashToPath converts a hash to a file path: <basedir>/<ab>/<cd>/<hash>
|
||||
func (s *ContentStorage) hashToPath(hash ContentHash) string {
|
||||
h := string(hash)
|
||||
@@ -552,7 +553,6 @@ func (s *VariantStorage) DeleteWithMeta(key VariantKey) error {
|
||||
|
||||
metaPath := s.keyToPath(key) + ".meta"
|
||||
|
||||
//nolint:gosec // G703: path derived from cache key
|
||||
err := os.Remove(metaPath)
|
||||
if err != nil && !os.IsNotExist(err) {
|
||||
return fmt.Errorf("failed to delete variant metadata: %w", err)
|
||||
|
||||
@@ -24,6 +24,7 @@ const (
|
||||
testHostExample = "example.com"
|
||||
testPathCat = "/photos/cat.jpg"
|
||||
testContentTypeJPEG = "image/jpeg"
|
||||
testHeaderContentType = "Content-Type"
|
||||
)
|
||||
|
||||
// TestFixtures contains paths to test files in the mock filesystem.
|
||||
|
||||
Reference in New Issue
Block a user