sneak/pixa
1
0
Fork 0
Code Issues 7 Pull Requests 3 Actions 18 Packages Projects Releases Wiki Activity

Split internal/imgcache into the image proxy and the cache (closes #317)
check / check (push) Waiting to run
Details

Browse Source 
The new package internal/imageproxy holds Proxy, which was imgcache.Service,
with its Config, ImageResponse, CacheStatus and errors. imageproxy.New makes
and registers the counters of upstream fetches and transforms.
internal/imgcache holds the cache only, with its hit and miss counters, and
imgcache.CacheConfig is imgcache.Config. The cache reads and writes each kind
it keeps with one pair of methods: Variant and PutVariant, Source and
PutSource, Failure and PutFailure. Source replaces LookupSource followed by
GetSourceContent. imgcache.go and module.go are gone. The proxy's tests moved
with it. The cache's tests check that the reconciliation pass removes an empty
or cut file and its row. The proxy's tests remove them by hand, then check
that Get fetches or converts again.

Model: opus-5-5
This commit is contained in:
clawbot
2026-10-26 18:51:01 +00:00
committed by sneak
parent 87cb1ab5f6
commit ef70bb6fa0
38 changed files with 1473 additions and 1113 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/imgcache/negative_cache_internal_test.go
+13 -90
View File
@@ -17,7 +17,7 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) {
db := setupTestDB(t)
dir := t.TempDir()
cache, err := NewCache(db, CacheConfig{
cache, err := NewCache(db, Config{
StateDir: dir,
NegativeTTL: 5 * time.Minute,
MaxBytes: 1 << 30,
@@ -35,7 +35,7 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) {
}
// Initially should not be in negative cache
hit, err := cache.checkNegativeCache(ctx, req)
hit, err := cache.Failure(ctx, req.Source)
if err != nil {
t.Fatal(err)
}
@@ -45,13 +45,13 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) {
}
// Store a negative entry
err = cache.StoreNegative(ctx, req)
err = cache.PutFailure(ctx, req.Source)
if err != nil {
t.Fatal(err)
}
// Now should be in negative cache
hit, err = cache.checkNegativeCache(ctx, req)
hit, err = cache.Failure(ctx, req.Source)
if err != nil {
t.Fatal(err)
}
@@ -67,7 +67,7 @@ func TestNegativeCache_Expired(t *testing.T) {
db := setupTestDB(t)
dir := t.TempDir()
cache, err := NewCache(db, CacheConfig{
cache, err := NewCache(db, Config{
StateDir: dir,
NegativeTTL: 1 * time.Millisecond, // very short TTL
MaxBytes: 1 << 30,
@@ -85,7 +85,7 @@ func TestNegativeCache_Expired(t *testing.T) {
}
// Store a negative entry with very short TTL
err = cache.StoreNegative(ctx, req)
err = cache.PutFailure(ctx, req.Source)
if err != nil {
t.Fatal(err)
}
@@ -94,7 +94,7 @@ func TestNegativeCache_Expired(t *testing.T) {
time.Sleep(10 * time.Millisecond)
// Should no longer be in negative cache
hit, err := cache.checkNegativeCache(ctx, req)
hit, err := cache.Failure(ctx, req.Source)
if err != nil {
t.Fatal(err)
}
@@ -104,71 +104,6 @@ func TestNegativeCache_Expired(t *testing.T) {
}
}
// TestService_Get_ServesCachedVariantOfNegativeCachedURL caches a variant of
// the test photo, then keeps a failed fetch of the photo's URL in the negative
// cache, as a failed fetch for another variant does once the source has been
// evicted. The cached variant is still served, and a variant not in the cache
// is made from the cached source. Once the source is evicted, a variant not in
// the cache needs a fetch and is refused.
func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
resp, err := svc.Get(t.Context(), photoVariant(fixtures, 85, imagerequest.FitCover))
if err != nil {
t.Fatalf("Get() first request error = %v", err)
}
_ = resp.Content.Close()
err = svc.cache.StoreNegative(t.Context(),
photoVariant(fixtures, 85, imagerequest.FitCover))
if err != nil {
t.Fatalf("StoreNegative() error = %v", err)
}
resp, err = svc.Get(t.Context(), photoVariant(fixtures, 85, imagerequest.FitCover))
if err != nil {
t.Fatalf("Get() of the cached variant error = %v", err)
}
_ = resp.Content.Close()
if resp.CacheStatus != CacheHit {
t.Errorf("cached variant CacheStatus = %q, want %q",
resp.CacheStatus, CacheHit)
}
resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, imagerequest.FitCover))
t.Logf("Get() of a variant made from the cached source: error = %v", err)
if err != nil {
t.Fatalf("Get() of a variant made from the cached source error = %v", err)
}
_ = resp.Content.Close()
sourceKey, err := svc.cache.LookupSource(t.Context(),
photoVariant(fixtures, 85, imagerequest.FitCover))
if err != nil || sourceKey == "" {
t.Fatalf("LookupSource() = %q, %v, want the cached source", sourceKey, err)
}
err = svc.cache.evictSourceBlob(t.Context(), sourceKey)
if err != nil {
t.Fatalf("evictSourceBlob() error = %v", err)
}
_, err = svc.Get(t.Context(), photoVariant(fixtures, 60, imagerequest.FitCover))
t.Logf("Get() of a variant whose source was evicted: error = %v", err)
if !errors.Is(err, ErrNegativeCached) {
t.Errorf("Get() of a variant whose source was evicted error = %v, want %v",
err, ErrNegativeCached)
}
}
// TestEvictionLoopRemovesExpiredNegativeCacheRows stores a failed fetch
// that expires a second later and starts the eviction loop with a short
// interval: a later tick must remove its row although its URL is never
@@ -190,7 +125,7 @@ func TestEvictionLoopRemovesExpiredNegativeCacheRows(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
cache, err := NewCache(evictionTestDB(t), CacheConfig{
cache, err := NewCache(evictionTestDB(t), Config{
StateDir: t.TempDir(),
NegativeTTL: time.Second,
MaxBytes: tt.maxBytes,
@@ -203,9 +138,9 @@ func TestEvictionLoopRemovesExpiredNegativeCacheRows(t *testing.T) {
Source: imagerequest.Source{Host: testHostExample, Path: "/gone.jpg"},
}
err = cache.StoreNegative(t.Context(), expired)
err = cache.PutFailure(t.Context(), expired.Source)
if err != nil {
t.Fatalf("StoreNegative failed: %v", err)
t.Fatalf("PutFailure failed: %v", err)
}
const notExpiredPath = "/later.jpg"
@@ -269,7 +204,7 @@ func TestCleanExpiredDeletesInBatches(t *testing.T) {
t.Parallel()
cache, err := NewCache(evictionTestDB(t),
CacheConfig{StateDir: t.TempDir(), MaxBytes: 1 << 30},
Config{StateDir: t.TempDir(), MaxBytes: 1 << 30},
prometheus.NewRegistry())
if err != nil {
t.Fatalf("failed to create cache: %v", err)
@@ -325,7 +260,7 @@ func TestCleanExpiredRemovesMoreThanOneBatch(t *testing.T) {
t.Parallel()
cache, err := NewCache(evictionTestDB(t),
CacheConfig{StateDir: t.TempDir(), MaxBytes: 1 << 30},
Config{StateDir: t.TempDir(), MaxBytes: 1 << 30},
prometheus.NewRegistry())
if err != nil {
t.Fatalf("failed to create cache: %v", err)
@@ -357,7 +292,7 @@ func TestCleanExpiredStopsOnceContextEnds(t *testing.T) {
t.Parallel()
cache, err := NewCache(evictionTestDB(t),
CacheConfig{StateDir: t.TempDir(), MaxBytes: 1 << 30},
Config{StateDir: t.TempDir(), MaxBytes: 1 << 30},
prometheus.NewRegistry())
if err != nil {
t.Fatalf("failed to create cache: %v", err)
@@ -410,15 +345,3 @@ func negativeCacheTableRowCount(tb testing.TB, cache *Cache) int {
return count
}
func TestService_Get_ReturnsErrorForNegativeCachedURL(t *testing.T) {
t.Parallel()
// This test verifies that Service.Get() checks the negative cache
// We can't easily test the full pipeline without vips, but we can
// verify the error type
err := ErrNegativeCached
if !errors.Is(err, ErrNegativeCached) {
t.Error("ErrNegativeCached should be identifiable with errors.Is")
}
}