TODO.md

@@ -30,6 +30,37 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
+- 2026-10-26 `internal/imgcache` holds the cache only (closes #317). The new
+  package `internal/imageproxy` fetches, transforms, caches and serves images.
+  It imports `imgcache`, and `imgcache` does not import it.
+    - `imgcache.Service` is `imageproxy.Proxy`, built by `imageproxy.New` from
+      an `imageproxy.Config`.
+    - `ImageResponse`, `CacheStatus`, `ErrNegativeCached` and
+      `ErrProcessingTimedOut` moved with it, under their names.
+    - `imgcache.CacheConfig` is `imgcache.Config`.
+    - The cache has one read and one write for each kind it keeps. Each takes
+      the place of an earlier method:
+        - `Variant`, of `GetVariant`
+        - `PutVariant`, of `StoreVariant`
+        - `Source`, of `LookupSource` followed by `GetSourceContent`
+        - `PutSource`, of `StoreSource`
+        - `Failure`, of `checkNegativeCache`
+        - `PutFailure`, of `StoreNegative`
+    - `Variant` and `Source` each open the file and hand its access time to the
+      background writer. A cache hit still makes no database call.
+    - `imgcache.go` and `module.go` are gone.
+    - `imageproxy.New` makes the counters of upstream fetches and transforms,
+      and registers them on the registry it is given. The cache keeps only the
+      hit and miss counters.
+    - A cached source whose row cannot be read is logged as "failed to load
+      cached source, fetching". It was logged as "source lookup failed".
+    - The proxy's tests moved to `internal/imageproxy`. They reach the cache
+      through its database, its state directory and its metrics registry.
+    - The tests of an empty or cut cached file are split in two:
+        - `internal/imgcache` checks that the reconciliation pass removes the
+          file and its row, with a warning naming the file.
+        - `internal/imageproxy` removes the file and its row as the pass does,
+          and checks that `Get` fetches or converts again.
 - 2026-10-26 both image routes serve an image with one function, `serveImage`,
   and answer its errors with one, `respondImageError` (closes #316).
   `HandleImage` parses the path and verifies the signature, and `HandleImageEnc`

internal/handlers/cache_hit_internal_test.go

@@ -23,6 +23,7 @@ import (
 	"sneak.berlin/go/pixa/internal/database"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imageproxy"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/imgcache"
 	"sneak.berlin/go/pixa/internal/seal"
@@ -122,7 +123,7 @@ func newCountingDatabaseServer(
 		tb.Fatalf("encoding the photo: %v", err)
 	}
 
-	cache, err := imgcache.NewCache(db, imgcache.CacheConfig{
+	cache, err := imgcache.NewCache(db, imgcache.Config{
 		StateDir:    tb.TempDir(),
 		NegativeTTL: 5 * time.Minute,
 		MaxBytes:    1 << 30,
@@ -131,7 +132,7 @@ func newCountingDatabaseServer(
 		tb.Fatalf("imgcache.NewCache() error = %v", err)
 	}
 
-	svc, err := imgcache.NewService(&imgcache.ServiceConfig{
+	svc, err := imageproxy.New(&imageproxy.Config{
 		Cache: cache,
 		Fetcher: newMockFetcher(fstest.MapFS{
 			countingHost + photoPath: &fstest.MapFile{Data: photo.Bytes()},
@@ -142,9 +143,9 @@ func newCountingDatabaseServer(
 		}),
 		MaxResponseSize: 50 << 20,
 		Logger:          slog.New(slog.DiscardHandler),
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
-		tb.Fatalf("imgcache.NewService() error = %v", err)
+		tb.Fatalf("imageproxy.New() error = %v", err)
 	}
 
 	keys, err := seal.DeriveKeys(testSigningKey)
@@ -228,9 +229,9 @@ func TestImageRoutes_CacheHitsMakeNoDatabaseCall(t *testing.T) {
 			rec := serveImage(t, srv, target)
 
 			cacheStatus := rec.Header().Get("X-Pixa-Cache")
-			if cacheStatus != string(imgcache.CacheHit) {
+			if cacheStatus != string(imageproxy.CacheHit) {
 				t.Errorf("GET %s: X-Pixa-Cache = %q, want %s",
-					target, cacheStatus, imgcache.CacheHit)
+					target, cacheStatus, imageproxy.CacheHit)
 			}
 		}
 
@@ -260,9 +261,9 @@ func BenchmarkHandleImage_CacheHits(b *testing.B) {
 			rec := serveImage(b, srv, target)
 
 			cacheStatus := rec.Header().Get("X-Pixa-Cache")
-			if cacheStatus != string(imgcache.CacheHit) {
+			if cacheStatus != string(imageproxy.CacheHit) {
 				b.Errorf("GET %s: X-Pixa-Cache = %q, want %s",
-					target, cacheStatus, imgcache.CacheHit)
+					target, cacheStatus, imageproxy.CacheHit)
 
 				return
 			}

internal/handlers/first_bytes_internal_test.go

@@ -17,6 +17,7 @@ import (
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imageproxy"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/imgcache"
 	"sneak.berlin/go/pixa/internal/seal"
@@ -38,7 +39,7 @@ func newRoutesFetchingThrough(
 ) (*Handlers, http.Handler) {
 	t.Helper()
 
-	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
+	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.Config{
 		StateDir:    t.TempDir(),
 		NegativeTTL: 5 * time.Minute,
 		MaxBytes:    1 << 30,
@@ -47,7 +48,7 @@ func newRoutesFetchingThrough(
 		t.Fatalf("imgcache.NewCache() error = %v", err)
 	}
 
-	svc, err := imgcache.NewService(&imgcache.ServiceConfig{
+	svc, err := imageproxy.New(&imageproxy.Config{
 		Cache:   cache,
 		Fetcher: fetcher,
 		Processor: imageprocessor.New(imageprocessor.Params{
@@ -56,9 +57,9 @@ func newRoutesFetchingThrough(
 		}),
 		MaxResponseSize: 50 << 20,
 		Logger:          log,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
-		t.Fatalf("imgcache.NewService() error = %v", err)
+		t.Fatalf("imageproxy.New() error = %v", err)
 	}
 
 	keys, err := seal.DeriveKeys(testSigningKey)

internal/handlers/handlers.go

@@ -13,7 +13,7 @@ import (
 	"sneak.berlin/go/pixa/internal/config"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/healthcheck"
-	"sneak.berlin/go/pixa/internal/imgcache"
+	"sneak.berlin/go/pixa/internal/imageproxy"
 	"sneak.berlin/go/pixa/internal/logger"
 	"sneak.berlin/go/pixa/internal/seal"
 	"sneak.berlin/go/pixa/internal/session"
@@ -29,7 +29,7 @@ type Params struct {
 	Config      *config.Config
 	Keys        seal.Keys
 
-	ImageService          *imgcache.Service
+	ImageService          *imageproxy.Proxy
 	SessionManager        *session.Manager
 	EncryptedURLGenerator *encurl.Generator
 }
@@ -39,7 +39,7 @@ type Handlers struct {
 	log         *slog.Logger
 	hc          *healthcheck.Healthcheck
 	config      *config.Config
-	imgSvc      *imgcache.Service
+	imgSvc      *imageproxy.Proxy
 	sessMgr     *session.Manager
 	encGen      *encurl.Generator
 	csrfProtect func(http.Handler) http.Handler

internal/handlers/handlers_internal_test.go

@@ -23,6 +23,7 @@ import (
 	"sneak.berlin/go/pixa/internal/database"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imageproxy"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/imgcache"
 	"sneak.berlin/go/pixa/internal/signature"
@@ -49,7 +50,7 @@ func setupTestHandler(t *testing.T) *testFixtures {
 	tmpDir := t.TempDir()
 	db := setupTestDB(t)
 
-	cache, err := imgcache.NewCache(db, imgcache.CacheConfig{
+	cache, err := imgcache.NewCache(db, imgcache.Config{
 		StateDir:    tmpDir,
 		NegativeTTL: 5 * time.Minute,
 		MaxBytes:    1 << 30,
@@ -58,7 +59,7 @@ func setupTestHandler(t *testing.T) *testFixtures {
 		t.Fatalf("failed to create cache: %v", err)
 	}
 
-	svc, err := imgcache.NewService(&imgcache.ServiceConfig{
+	svc, err := imageproxy.New(&imageproxy.Config{
 		Cache:   cache,
 		Fetcher: newMockFetcher(mockFS),
 		Processor: imageprocessor.New(imageprocessor.Params{
@@ -66,9 +67,9 @@ func setupTestHandler(t *testing.T) *testFixtures {
 			MaxConcurrentProcessing: 2,
 		}),
 		MaxResponseSize: 50 << 20,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
-		t.Fatalf("failed to create service: %v", err)
+		t.Fatalf("failed to create proxy: %v", err)
 	}
 
 	h := &Handlers{

internal/handlers/image.go

@@ -14,8 +14,8 @@ import (
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imageproxy"
 	"sneak.berlin/go/pixa/internal/imagerequest"
-	"sneak.berlin/go/pixa/internal/imgcache"
 	"sneak.berlin/go/pixa/internal/magic"
 )
 
@@ -253,7 +253,7 @@ func (s *Handlers) respondImageError(
 		status, message = http.StatusGatewayTimeout, "upstream timeout"
 	case errors.Is(err, httpfetcher.ErrTooManyConnections),
 		errors.Is(err, imageprocessor.ErrTooManyImages),
-		errors.Is(err, imgcache.ErrProcessingTimedOut):
+		errors.Is(err, imageproxy.ErrProcessingTimedOut):
 		status = http.StatusServiceUnavailable
 		message = "server busy, try again later"
 	case errors.Is(err, httpfetcher.ErrInvalidContentType),

internal/handlers/image_cache_control_internal_test.go

@@ -17,6 +17,7 @@ import (
 	"sneak.berlin/go/pixa/internal/allowlist"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imageproxy"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/imgcache"
 	"sneak.berlin/go/pixa/internal/seal"
@@ -35,7 +36,7 @@ func newSignedHostServer(
 ) (*Handlers, http.Handler) {
 	t.Helper()
 
-	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
+	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.Config{
 		StateDir:    t.TempDir(),
 		NegativeTTL: 5 * time.Minute,
 		MaxBytes:    1 << 30,
@@ -46,7 +47,7 @@ func newSignedHostServer(
 
 	jpegData := generateTestJPEG(t, 100, 100, color.RGBA{255, 0, 0, 255})
 
-	svc, err := imgcache.NewService(&imgcache.ServiceConfig{
+	svc, err := imageproxy.New(&imageproxy.Config{
 		Cache: cache,
 		Fetcher: newMockFetcher(fstest.MapFS{
 			signedHost + photoPath: &fstest.MapFile{Data: jpegData},
@@ -57,9 +58,9 @@ func newSignedHostServer(
 		}),
 		MaxResponseSize: 50 << 20,
 		Logger:          log,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
-		t.Fatalf("imgcache.NewService() error = %v", err)
+		t.Fatalf("imageproxy.New() error = %v", err)
 	}
 
 	keys, err := seal.DeriveKeys(testSigningKey)

internal/handlers/image_errors_internal_test.go

@@ -22,6 +22,7 @@ import (
 	"sneak.berlin/go/pixa/internal/allowlist"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imageproxy"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/imgcache"
 	"sneak.berlin/go/pixa/internal/signature"
@@ -36,7 +37,7 @@ const allowlistedHost = "allowed.example.com"
 func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
 	t.Helper()
 
-	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
+	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.Config{
 		StateDir:    t.TempDir(),
 		NegativeTTL: 5 * time.Minute,
 		MaxBytes:    1 << 30,
@@ -45,7 +46,7 @@ func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
 		t.Fatalf("failed to create cache: %v", err)
 	}
 
-	svc, err := imgcache.NewService(&imgcache.ServiceConfig{
+	svc, err := imageproxy.New(&imageproxy.Config{
 		Cache:   cache,
 		Fetcher: fetcher,
 		Processor: imageprocessor.New(imageprocessor.Params{
@@ -53,9 +54,9 @@ func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
 			MaxConcurrentProcessing: 2,
 		}),
 		MaxResponseSize: 50 << 20,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
-		t.Fatalf("failed to create service: %v", err)
+		t.Fatalf("failed to create proxy: %v", err)
 	}
 
 	h := &Handlers{

internal/handlers/referer_blocklist_internal_test.go

@@ -15,6 +15,7 @@ import (
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imageproxy"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/imgcache"
 	"sneak.berlin/go/pixa/internal/seal"
@@ -52,7 +53,7 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher)
 		Fetcher: newPhotoFetcher(t, allowlistedHost, signedHost),
 	}
 
-	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
+	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.Config{
 		StateDir:    t.TempDir(),
 		NegativeTTL: 5 * time.Minute,
 		MaxBytes:    1 << 30,
@@ -61,7 +62,7 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher)
 		t.Fatalf("imgcache.NewCache() error = %v", err)
 	}
 
-	svc, err := imgcache.NewService(&imgcache.ServiceConfig{
+	svc, err := imageproxy.New(&imageproxy.Config{
 		Cache:   cache,
 		Fetcher: fetcher,
 		Processor: imageprocessor.New(imageprocessor.Params{
@@ -69,9 +70,9 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher)
 			MaxConcurrentProcessing: 2,
 		}),
 		MaxResponseSize: 50 << 20,
-	})
+	}, prometheus.NewRegistry())
 	if err != nil {
-		t.Fatalf("imgcache.NewService() error = %v", err)
+		t.Fatalf("imageproxy.New() error = %v", err)
 	}
 
 	keys, err := seal.DeriveKeys(testSigningKey)

internal/handlers/server_busy_internal_test.go

@@ -11,8 +11,8 @@ import (
 
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imageproxy"
 	"sneak.berlin/go/pixa/internal/imagerequest"
-	"sneak.berlin/go/pixa/internal/imgcache"
 )
 
 // TestServerBusyAnswers503 checks that respondImageError, which answers the
@@ -58,7 +58,7 @@ func TestProcessingTimedOutAnswers503(t *testing.T) {
 
 	h := &Handlers{log: slog.New(slog.DiscardHandler)}
 	err := fmt.Errorf("%w: failed to encode: %w",
-		imgcache.ErrProcessingTimedOut, context.DeadlineExceeded)
+		imageproxy.ErrProcessingTimedOut, context.DeadlineExceeded)
 
 	rec := httptest.NewRecorder()
 	h.respondImageError(rec, &imagerequest.Request{}, err)

internal/imageproxy/cache_hit_internal_test.go

@@ -0,0 +1,100 @@
+package imageproxy
+
+import (
+	"bytes"
+	"os"
+	"testing"
+	"time"
+
+	"sneak.berlin/go/pixa/internal/imagerequest"
+	"sneak.berlin/go/pixa/internal/imgcache"
+)
+
+// TestProxy_Get_MakesAgainVariantOfNoOutputFormat replaces a cached
+// variant's file with text, and checks that the next request is a miss
+// answered with the variant made again, and the one after a hit.
+func TestProxy_Get_MakesAgainVariantOfNoOutputFormat(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := setupTestProxy(t)
+	req := photoVariant(fixtures, 85, imagerequest.FitCover)
+
+	want, _ := getImage(t, svc, req)
+
+	path := variantFilePath(svc, imagerequest.CacheKey(req))
+
+	err := os.WriteFile(path, []byte("this is text, not an image"),
+		imgcache.StorageFilePerm)
+	if err != nil {
+		t.Fatalf("replacing the variant file: %v", err)
+	}
+
+	got, status := getImage(t, svc, req)
+	t.Logf("after the replacement: %d bytes, %s", len(got), status)
+
+	if status != CacheMiss || !bytes.Equal(got, want) {
+		t.Errorf("after the replacement: Get() = %d bytes, %s; want the %d bytes "+
+			"made again, %s", len(got), status, len(want), CacheMiss)
+	}
+
+	got, status = getImage(t, svc, req)
+
+	if status != CacheHit || !bytes.Equal(got, want) {
+		t.Errorf("next request: Get() = %d bytes, %s; want the %d bytes, %s",
+			len(got), status, len(want), CacheHit)
+	}
+}
+
+// TestProxy_Get_HitWaitsForNoDatabaseWrite holds the database's one
+// connection while a cached variant is requested. The hit must be answered
+// while the connection is still held, and its access time must reach the
+// database once it is free.
+func TestProxy_Get_HitWaitsForNoDatabaseWrite(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := setupTestProxy(t)
+	req := photoVariant(fixtures, 85, imagerequest.FitCover)
+	key := imagerequest.CacheKey(req)
+
+	getImage(t, svc, req)
+
+	// Each connection to an in-memory SQLite database opens a new, empty one,
+	// so the goroutine that writes the access times must share the
+	// connection that holds the schema.
+	svc.db.SetMaxOpenConns(1)
+	svc.cache.StartPendingAccessTimeWrites()
+
+	defer func() { _ = svc.cache.StopPendingAccessTimeWrites(t.Context()) }()
+
+	anHourAgo := time.Now().Add(-time.Hour)
+	setVariantLastAccessed(t, svc.db, key, anHourAgo)
+
+	releaseDatabase := holdDatabase(t, svc.db)
+	defer releaseDatabase()
+
+	results := startGet(t.Context(), svc, req)
+
+	select {
+	case got := <-results:
+		if got.err != nil {
+			t.Fatalf("Get() of the cached variant error = %v", got.err)
+		}
+	case <-time.After(pendingAccessTimesWait):
+		t.Fatal("the cache hit waited for the database")
+	}
+
+	releaseDatabase()
+
+	deadline := time.Now().Add(pendingAccessTimesWait)
+
+	for countRows(t, svc.db, `
+		SELECT COUNT(*) FROM variant_content
+		WHERE cache_key = ? AND last_accessed_at > ?
+	`, string(key), anHourAgo.UTC().Format(sqliteTimestampFormat)) == 0 {
+		if time.Now().After(deadline) {
+			t.Fatal("the hit's access time was not written")
+		}
+
+		time.Sleep(10 * time.Millisecond)
+	}
+}

internal/imageproxy/concurrent_misses_internal_test.go

@@ -1,4 +1,4 @@
-package imgcache
+package imageproxy
 
 import (
 	"bytes"
@@ -56,15 +56,15 @@ func (f *heldFetcher) releaseFetches() {
 	f.releaseOnce.Do(func() { close(f.release) })
 }
 
-// setupHeldFetchService returns a test service whose fetches go through a
+// setupHeldFetchProxy returns a test proxy whose fetches go through a
 // heldFetcher. Its database is limited to one connection: each connection to
 // an in-memory SQLite database opens a new, empty one, so requests running at
 // once must share the connection that holds the schema.
-func setupHeldFetchService(t *testing.T) (*Service, *TestFixtures, *heldFetcher) {
+func setupHeldFetchProxy(t *testing.T) (*testProxy, *TestFixtures, *heldFetcher) {
 	t.Helper()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
-	svc.cache.db.SetMaxOpenConns(1)
+	svc.db.SetMaxOpenConns(1)
 
 	fetcher := &heldFetcher{
 		upstream: svc.fetcher,
@@ -107,7 +107,7 @@ type getResult struct {
 // startGet calls Get in a goroutine of its own and delivers what it returned
 // on the channel.
 func startGet(
-	ctx context.Context, svc *Service, req *imagerequest.Request,
+	ctx context.Context, svc *testProxy, req *imagerequest.Request,
 ) <-chan getResult {
 	results := make(chan getResult, 1)
 
@@ -139,14 +139,14 @@ func jpegSize(data []byte) (int, int) {
 	return config.Width, config.Height
 }
 
-// TestService_Get_ConcurrentMissesShareOneFetch starts several requests for
+// TestProxy_Get_ConcurrentMissesShareOneFetch starts several requests for
 // one uncached variant while the first one's fetch is held. Between them they
 // must fetch the source once and transcode it once, every one must be answered
 // with the same 50x25 JPEG, and each must count one miss.
-func TestService_Get_ConcurrentMissesShareOneFetch(t *testing.T) {
+func TestProxy_Get_ConcurrentMissesShareOneFetch(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures, fetcher := setupHeldFetchService(t)
+	svc, fixtures, fetcher := setupHeldFetchProxy(t)
 
 	const requests = 8
 
@@ -183,7 +183,7 @@ func TestService_Get_ConcurrentMissesShareOneFetch(t *testing.T) {
 		t.Errorf("%d requests made %d upstream fetches, want 1", requests, fetches)
 	}
 
-	// NewTestFS builds the same files the test service's fetcher serves.
+	// NewTestFS builds the same files the test proxy's fetcher serves.
 	testFS, _ := NewTestFS(t)
 
 	photo, err := fs.ReadFile(testFS, fixtures.GoodHostJPEG)
@@ -193,18 +193,18 @@ func TestService_Get_ConcurrentMissesShareOneFetch(t *testing.T) {
 
 	want := counterValues{0, requests, 1, int64(len(photo)), 1}
 
-	if got := readCounters(svc.cache); got != want {
+	if got := readCounters(t, svc); got != want {
 		t.Errorf("counters = %+v, want %+v", got, want)
 	}
 }
 
-// TestService_Get_ConcurrentVariantsStayApart requests three variants of the
+// TestProxy_Get_ConcurrentVariantsStayApart requests three variants of the
 // test photo at once that differ only in quality or fit. Each must be made by
 // a fetch and a transcode of its own, and each answer must be its own variant.
-func TestService_Get_ConcurrentVariantsStayApart(t *testing.T) {
+func TestProxy_Get_ConcurrentVariantsStayApart(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures, fetcher := setupHeldFetchService(t)
+	svc, fixtures, fetcher := setupHeldFetchProxy(t)
 
 	cover := startGet(t.Context(), svc, photoVariant(fixtures, 85, imagerequest.FitCover))
 	lowQuality := startGet(t.Context(), svc,
@@ -253,14 +253,14 @@ func TestService_Get_ConcurrentVariantsStayApart(t *testing.T) {
 	}
 }
 
-// TestService_Get_WaiterStopsWhenItsContextEnds has a second request for a
+// TestProxy_Get_WaiterStopsWhenItsContextEnds has a second request for a
 // variant join the first one's held fetch, then ends the second request's
 // context. The second request must return at once with the context's error,
 // while the fetch is still held, and the first must still be answered.
-func TestService_Get_WaiterStopsWhenItsContextEnds(t *testing.T) {
+func TestProxy_Get_WaiterStopsWhenItsContextEnds(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures, fetcher := setupHeldFetchService(t)
+	svc, fixtures, fetcher := setupHeldFetchProxy(t)
 
 	first := startGet(t.Context(), svc, photoVariant(fixtures, 85, imagerequest.FitCover))
 
@@ -298,15 +298,15 @@ func TestService_Get_WaiterStopsWhenItsContextEnds(t *testing.T) {
 	}
 }
 
-// TestService_Get_FirstRequestLeavingKeepsTheWork ends the context of the
+// TestProxy_Get_FirstRequestLeavingKeepsTheWork ends the context of the
 // request whose fetch is held, after a second request has joined it. The fetch
 // and transcode must go on and answer the second request. The first request
 // waits for its own work, as every request did before misses were shared, and
 // is answered too.
-func TestService_Get_FirstRequestLeavingKeepsTheWork(t *testing.T) {
+func TestProxy_Get_FirstRequestLeavingKeepsTheWork(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures, fetcher := setupHeldFetchService(t)
+	svc, fixtures, fetcher := setupHeldFetchProxy(t)
 
 	firstCtx, cancelFirst := context.WithCancel(t.Context())
 	first := startGet(firstCtx, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
@@ -337,11 +337,11 @@ func TestService_Get_FirstRequestLeavingKeepsTheWork(t *testing.T) {
 	}
 }
 
-// TestService_Get_ConcurrentMissesShareAFailure has several requests for an
+// TestProxy_Get_ConcurrentMissesShareAFailure has several requests for an
 // image that cannot be served arrive while its fetch is held: the one fetch
 // answers all of them with its error. The request after them is answered from
 // the negative cache, where the failure is kept.
-func TestService_Get_ConcurrentMissesShareAFailure(t *testing.T) {
+func TestProxy_Get_ConcurrentMissesShareAFailure(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
@@ -362,7 +362,7 @@ func TestService_Get_ConcurrentMissesShareAFailure(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			svc, fixtures, fetcher := setupHeldFetchService(t)
+			svc, fixtures, fetcher := setupHeldFetchProxy(t)
 
 			request := func() *imagerequest.Request {
 				req := photoVariant(fixtures, 85, imagerequest.FitCover)
@@ -398,12 +398,12 @@ func TestService_Get_ConcurrentMissesShareAFailure(t *testing.T) {
 	}
 }
 
-// TestService_Get_EndedRequestFetchesNothing checks that a request whose
+// TestProxy_Get_EndedRequestFetchesNothing checks that a request whose
 // context has already ended when it misses the cache starts no fetch.
-func TestService_Get_EndedRequestFetchesNothing(t *testing.T) {
+func TestProxy_Get_EndedRequestFetchesNothing(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures, fetcher := setupHeldFetchService(t)
+	svc, fixtures, fetcher := setupHeldFetchProxy(t)
 
 	ctx, cancel := context.WithCancel(t.Context())
 	cancel()
@@ -418,14 +418,14 @@ func TestService_Get_EndedRequestFetchesNothing(t *testing.T) {
 	}
 }
 
-// TestService_Get_ReturnsByItsDeadline gives a request a deadline and holds
+// TestProxy_Get_ReturnsByItsDeadline gives a request a deadline and holds
 // its fetch until the fetch's context ends, as the fetcher does while it waits
 // for a free connection to the host. The request must return by its deadline
 // with the deadline's error.
-func TestService_Get_ReturnsByItsDeadline(t *testing.T) {
+func TestProxy_Get_ReturnsByItsDeadline(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures, _ := setupHeldFetchService(t)
+	svc, fixtures, _ := setupHeldFetchProxy(t)
 
 	const timeout = 200 * time.Millisecond
 
@@ -444,16 +444,16 @@ func TestService_Get_ReturnsByItsDeadline(t *testing.T) {
 	}
 }
 
-// TestService_Get_WaiterGetsProcessingTimedOut has a request with no deadline
+// TestProxy_Get_WaiterGetsProcessingTimedOut has a request with no deadline
 // join the held fetch of a first request with a short one. With the only
 // processing slot held, the first request's transcode waits for it until the
 // first request's deadline ends, and stops then. The waiting request, whose
 // own context has not ended, must get ErrProcessingTimedOut, which the image
 // routes answer 503 as when pixa is busy.
-func TestService_Get_WaiterGetsProcessingTimedOut(t *testing.T) {
+func TestProxy_Get_WaiterGetsProcessingTimedOut(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures, fetcher := setupHeldFetchService(t)
+	svc, fixtures, fetcher := setupHeldFetchProxy(t)
 	svc.processor = imageprocessor.New(
 		imageprocessor.Params{MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1},
 	)
@@ -494,13 +494,13 @@ func (panickingFetcher) Fetch(
 	panic("upstream fetcher panicked")
 }
 
-// TestService_Get_PanicBecomesAnError checks that a panic while a variant is
+// TestProxy_Get_PanicBecomesAnError checks that a panic while a variant is
 // being made reaches its request as an error naming the panic, instead of
 // being raised again.
-func TestService_Get_PanicBecomesAnError(t *testing.T) {
+func TestProxy_Get_PanicBecomesAnError(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	svc.fetcher = panickingFetcher{}
 
 	var err error
@@ -520,13 +520,13 @@ func TestService_Get_PanicBecomesAnError(t *testing.T) {
 	}
 }
 
-// TestService_Get_PanicIsReportedToSentry checks that a panic while a variant
+// TestProxy_Get_PanicIsReportedToSentry checks that a panic while a variant
 // is being made is reported through the Sentry hub on the request's context,
 // where the Sentry middleware puts one when sentry_dsn is set.
-func TestService_Get_PanicIsReportedToSentry(t *testing.T) {
+func TestProxy_Get_PanicIsReportedToSentry(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	svc.fetcher = panickingFetcher{}
 
 	transport := &sentry.MockTransport{}

internal/imageproxy/counters_internal_test.go

@@ -0,0 +1,77 @@
+package imageproxy
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/testutil"
+	"sneak.berlin/go/pixa/internal/imgcache"
+)
+
+// TestNew_RegistersItsCountersOnItsRegistry checks that New registers the
+// proxy's counters on the registry it is given, under the names README.md
+// gives them. The cache registers its own counters on another registry.
+func TestNew_RegistersItsCountersOnItsRegistry(t *testing.T) {
+	t.Parallel()
+
+	cache, err := imgcache.NewCache(setupTestDB(t),
+		imgcache.Config{StateDir: t.TempDir(), MaxBytes: 1 << 30},
+		prometheus.NewRegistry())
+	if err != nil {
+		t.Fatalf("imgcache.NewCache() error = %v", err)
+	}
+
+	registry := prometheus.NewRegistry()
+
+	_, err = New(&Config{Cache: cache}, registry)
+	if err != nil {
+		t.Fatalf("New() error = %v", err)
+	}
+
+	names := []string{
+		"pixa_upstream_fetches_total",
+		"pixa_upstream_fetch_bytes_total",
+		"pixa_transforms_total",
+	}
+
+	count, err := testutil.GatherAndCount(registry, names...)
+	if err != nil {
+		t.Fatalf("testutil.GatherAndCount() error = %v", err)
+	}
+
+	if count != len(names) {
+		t.Errorf("%d of the counters %v are on the registry, want %d",
+			count, names, len(names))
+	}
+}
+
+// TestNew_FailsWhenItsCountersCannotBeRegistered makes a second proxy on
+// the registry of a first. Its counters have the same names, so the
+// registry refuses them, and New must return that error.
+func TestNew_FailsWhenItsCountersCannotBeRegistered(t *testing.T) {
+	t.Parallel()
+
+	cache, err := imgcache.NewCache(setupTestDB(t),
+		imgcache.Config{StateDir: t.TempDir(), MaxBytes: 1 << 30},
+		prometheus.NewRegistry())
+	if err != nil {
+		t.Fatalf("imgcache.NewCache() error = %v", err)
+	}
+
+	registry := prometheus.NewRegistry()
+
+	_, err = New(&Config{Cache: cache}, registry)
+	if err != nil {
+		t.Fatalf("New() of the first proxy error = %v", err)
+	}
+
+	_, err = New(&Config{Cache: cache}, registry)
+	t.Logf("New() of the second proxy error = %v", err)
+
+	var alreadyRegistered prometheus.AlreadyRegisteredError
+	if !errors.As(err, &alreadyRegistered) {
+		t.Errorf("New() of the second proxy error = %v, want %T",
+			err, alreadyRegistered)
+	}
+}

internal/imageproxy/divzero_internal_test.go

@@ -1,4 +1,4 @@
-package imgcache
+package imageproxy
 
 import (
 	"fmt"

internal/imageproxy/imageproxy.go

@@ -1,4 +1,7 @@
-package imgcache
+// Package imageproxy fetches, transforms, caches and serves images. Its
+// Proxy serves an image from the cache when it can, and otherwise makes it
+// from the cached or fetched source image.
+package imageproxy
 
 import (
 	"bytes"
@@ -13,16 +16,18 @@ import (
 	"github.com/dustin/go-humanize"
 	"github.com/getsentry/sentry-go"
 	"github.com/go-chi/chi/v5/middleware"
+	"github.com/prometheus/client_golang/prometheus"
 	"golang.org/x/sync/singleflight"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/imagerequest"
+	"sneak.berlin/go/pixa/internal/imgcache"
 	"sneak.berlin/go/pixa/internal/magic"
 )
 
-// Service orchestrates the cache, fetcher, and processor.
+// Proxy orchestrates the cache, fetcher, and processor.
-type Service struct {
+type Proxy struct {
-	cache           *Cache
+	cache           *imgcache.Cache
 	fetcher         httpfetcher.Fetcher
 	processor       *imageprocessor.ImageProcessor
 	log             *slog.Logger
@@ -30,12 +35,18 @@ type Service struct {
 	// variantsInProgress lets the requests that miss the same variant at the
 	// same time share one fetch and one transcode.
 	variantsInProgress singleflight.Group
+
+	// The counts of upstream fetches and transforms, made and registered by
+	// New. They are kept in memory, so adding to one waits for nothing.
+	upstreamFetches    prometheus.Counter
+	upstreamFetchBytes prometheus.Counter
+	transforms         prometheus.Counter
 }
 
-// ServiceConfig holds configuration for the image service.
+// Config holds the proxy's configuration.
-type ServiceConfig struct {
+type Config struct {
 	// Cache is the cache instance
-	Cache *Cache
+	Cache *imgcache.Cache
 	// Fetcher fetches source images from upstream hosts
 	Fetcher httpfetcher.Fetcher
 	// Processor converts and resizes the images
@@ -47,11 +58,16 @@ type ServiceConfig struct {
 	Logger *slog.Logger
 }
 
-// errCacheRequired is returned when the service is built without a cache.
+// errCacheRequired is returned when the proxy is built without a cache.
 var errCacheRequired = errors.New("cache is required")
 
-// NewService creates a new image service.
+// New creates a new image proxy and registers its counters on
-func NewService(cfg *ServiceConfig) (*Service, error) {
+// metricsRegistry. pixad passes Prometheus' default registry, the one
+// /metrics serves. It fails when metricsRegistry already holds counters of
+// the same names.
+func New(
+	cfg *Config, metricsRegistry prometheus.Registerer,
+) (*Proxy, error) {
 	if cfg.Cache == nil {
 		return nil, errCacheRequired
 	}
@@ -68,13 +84,36 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 		return nil, err
 	}
 
-	return &Service{
+	p := &Proxy{
 		cache:           cfg.Cache,
 		fetcher:         cfg.Fetcher,
 		processor:       cfg.Processor,
 		log:             log,
 		maxResponseSize: cfg.MaxResponseSize,
-	}, nil
+		upstreamFetches: prometheus.NewCounter(prometheus.CounterOpts{
+			Name: "pixa_upstream_fetches_total",
+			Help: "Fetches of a source image from upstream that read any bytes.",
+		}),
+		upstreamFetchBytes: prometheus.NewCounter(prometheus.CounterOpts{
+			Name: "pixa_upstream_fetch_bytes_total",
+			Help: "Bytes read from upstream by fetches of source images.",
+		}),
+		transforms: prometheus.NewCounter(prometheus.CounterOpts{
+			Name: "pixa_transforms_total",
+			Help: "Images the image processor transformed.",
+		}),
+	}
+
+	for _, counter := range []prometheus.Counter{
+		p.upstreamFetches, p.upstreamFetchBytes, p.transforms,
+	} {
+		err = metricsRegistry.Register(counter)
+		if err != nil {
+			return nil, fmt.Errorf("failed to register the proxy's counters: %w", err)
+		}
+	}
+
+	return p, nil
 }
 
 // ErrNegativeCached is returned when a URL is in the negative cache (recently failed).
@@ -86,25 +125,49 @@ var ErrNegativeCached = errors.New("request is in negative cache (recently faile
 var ErrProcessingTimedOut = errors.New(
 	"image processing stopped at the deadline of the request doing it")
 
+// ImageResponse represents a processed image ready to serve
+type ImageResponse struct {
+	// Content is the image data reader
+	Content io.ReadCloser
+	// ContentLength is the size in bytes (-1 if unknown)
+	ContentLength int64
+	// ContentType is the MIME type of the response
+	ContentType string
+	// ETag is the entity tag for caching
+	ETag string
+	// CacheStatus indicates HIT or MISS
+	CacheStatus CacheStatus
+	// FetchedBytes is the number of bytes fetched from upstream (0 if cache hit)
+	FetchedBytes int64
+}
+
+// CacheStatus indicates whether the response was served from cache.
+type CacheStatus string
+
+// Cache status values for response headers.
+const (
+	CacheHit  CacheStatus = "HIT"
+	CacheMiss CacheStatus = "MISS"
+)
+
 // Get retrieves a processed image, fetching and processing if necessary.
 // While its source URL is in the negative cache, a cached variant is still
 // served and a variant not in the cache is still made from a cached source;
 // only a request that needs the source fetched is refused.
 //
-// A cache hit opens the variant file and makes no database call: its access
+// A cache hit opens the variant file and makes no database call: the cache
-// time is handed to the goroutine that StartPendingAccessTimeWrites starts,
+// hands its access time to the goroutine that writes access times, without
-// without waiting for it, and its hit is counted in memory. A variant file
+// waiting for it, and its hit is counted in memory. A variant file that
-// that cannot be served, such as one whose first bytes are not those of an
+// cannot be served, such as one whose first bytes are not those of an output
-// output format, is a miss, and the variant is made again.
+// format, is a miss, and the variant is made again.
-func (s *Service) Get(
+func (p *Proxy) Get(
 	ctx context.Context, req *imagerequest.Request,
 ) (*ImageResponse, error) {
 	cacheKey := imagerequest.CacheKey(req)
 
-	reader, size, contentType, err := s.cache.GetVariant(cacheKey)
+	reader, size, contentType, err := p.cache.Variant(cacheKey)
 	if err == nil {
-		s.cache.touchVariant(cacheKey)
+		p.cache.IncrementStats(true)
-		s.cache.IncrementStats(true, 0)
 
 		return &ImageResponse{
 			Content:       reader,
@@ -115,17 +178,17 @@ func (s *Service) Get(
 		}, nil
 	}
 
-	if !errors.Is(err, ErrNotFound) {
+	if !errors.Is(err, imgcache.ErrNotFound) {
-		s.log.Warn("cached variant not served, making it again",
+		p.log.Warn("cached variant not served, making it again",
 			"cache_key", cacheKey, "error", err)
 	}
 
 	// Cache miss - get the variant, processed once for all the requests that
 	// miss it at the same time, then count this request's miss, also when it
 	// failed or the request context has ended meanwhile
-	response, err := s.processOrWait(ctx, req)
+	response, err := p.processOrWait(ctx, req)
 
-	s.cache.IncrementStats(false, 0)
+	p.cache.IncrementStats(false)
 
 	if err != nil {
 		return nil, err
@@ -138,8 +201,8 @@ func (s *Service) Get(
 
 // WaitForProcessing waits until no image is being processed, or until ctx
 // ends, and returns how many images were still being processed then.
-func (s *Service) WaitForProcessing(ctx context.Context) int {
+func (p *Proxy) WaitForProcessing(ctx context.Context) int {
-	return s.processor.WaitForProcessing(ctx)
+	return p.processor.WaitForProcessing(ctx)
 }
 
 // errPanicked is returned when processing a variant panicked.
@@ -151,7 +214,7 @@ var errPanicked = errors.New("panic while processing image")
 // and take no upstream connection or processing slot. The processing ignores
 // the first request's cancellation, so the others are still served if that
 // client goes away, but keeps its deadline.
-func (s *Service) processOrWait(
+func (p *Proxy) processOrWait(
 	ctx context.Context, req *imagerequest.Request,
 ) (*ImageResponse, error) {
 	// A request that has already ended starts no processing
@@ -165,7 +228,7 @@ func (s *Service) processOrWait(
 	// only when no other request is processing the variant
 	processing := make(chan struct{})
 
-	results := s.variantsInProgress.DoChan(string(cacheKey),
+	results := p.variantsInProgress.DoChan(string(cacheKey),
 		func() (_ any, err error) {
 			close(processing)
 
@@ -176,7 +239,7 @@ func (s *Service) processOrWait(
 			defer func() {
 				recovered := recover()
 				if recovered != nil {
-					s.log.Error("panic while processing image",
+					p.log.Error("panic while processing image",
 						"host", req.Source.Host, "path", req.Source.Path,
 						"panic", recovered, "stack", string(debug.Stack()))
 
@@ -191,7 +254,7 @@ func (s *Service) processOrWait(
 			processingCtx, cancel := withoutCancelKeepingDeadline(ctx)
 			defer cancel()
 
-			return s.processFromSourceOrFetch(processingCtx, req, cacheKey)
+			return p.processFromSourceOrFetch(processingCtx, req, cacheKey)
 		})
 
 	var result singleflight.Result
@@ -240,25 +303,31 @@ func withoutCancelKeepingDeadline(
 	return context.WithDeadline(context.WithoutCancel(ctx), deadline)
 }
 
-// loadCachedSource opens source content from cache, without reading it, and
+// loadCachedSource opens src's cached source, without reading it, and
-// returns it with its size; nil if the cached data is unavailable, empty or
+// returns it with its size; nil if it is not in the cache, cannot be read,
-// exceeds maxResponseSize.
+// is empty or exceeds maxResponseSize.
-func (s *Service) loadCachedSource(
+func (p *Proxy) loadCachedSource(
-	sourceKey imagerequest.SourceKey,
+	ctx context.Context, src imagerequest.Source,
 ) (io.ReadCloser, int64) {
-	reader, size, err := s.cache.GetSourceContent(sourceKey)
+	reader, size, err := p.cache.Source(ctx, src)
+	if errors.Is(err, imgcache.ErrNotFound) {
+		return nil, 0
+	}
+
 	if err != nil {
-		s.log.Warn("failed to load cached source, fetching", "error", err)
+		p.log.Warn("failed to load cached source, fetching", "error", err)
 
 		return nil, 0
 	}
 
-	if size > s.maxResponseSize {
+	p.log.Debug("using cached source", "source_key", src.Key())
+
+	if size > p.maxResponseSize {
 		_ = reader.Close()
 
-		s.log.Warn("cached source exceeds max response size, discarding",
+		p.log.Warn("cached source exceeds max response size, discarding",
-			"source_key", sourceKey,
+			"source_key", src.Key(),
-			"max_bytes", s.maxResponseSize,
+			"max_bytes", p.maxResponseSize,
 		)
 
 		return nil, 0
@@ -275,30 +344,16 @@ func (s *Service) loadCachedSource(
 
 // processFromSourceOrFetch processes an image, using cached source content
 // if available.
-func (s *Service) processFromSourceOrFetch(
+func (p *Proxy) processFromSourceOrFetch(
 	ctx context.Context,
 	req *imagerequest.Request,
 	cacheKey imagerequest.VariantKey,
 ) (*processedVariant, error) {
-	// Check if we have cached source content
+	source, sourceSize := p.loadCachedSource(ctx, req.Source)
-	sourceKey, err := s.cache.LookupSource(ctx, req)
-	if err != nil {
-		s.log.Warn("source lookup failed", "error", err)
-	}
-
-	var (
-		source     io.ReadCloser
-		sourceSize int64
-	)
-
-	if sourceKey != "" {
-		s.log.Debug("using cached source", "source_key", sourceKey)
-		source, sourceSize = s.loadCachedSource(sourceKey)
-	}
 
 	// Fetch from upstream if we don't have source data or it's empty
 	if source == nil {
-		return s.fetchAndProcess(ctx, req, cacheKey)
+		return p.fetchAndProcess(ctx, req, cacheKey)
 	}
 
 	defer func() { _ = source.Close() }()
@@ -306,26 +361,26 @@ func (s *Service) processFromSourceOrFetch(
 	// Process using cached source; nothing was fetched from upstream. The
 	// image processor reads the source only once it has a processing slot,
 	// so a request waiting for one holds none of it in memory.
-	return s.processAndStore(ctx, req, cacheKey, source, sourceSize)
+	return p.processAndStore(ctx, req, cacheKey, source, sourceSize)
 }
 
 // fetchAndProcess fetches from upstream, processes, and caches the result.
 // It counts the fetch with the bytes read from upstream, including when
 // reading the response or a later step fails. It fetches nothing while the
 // source URL is in the negative cache.
-func (s *Service) fetchAndProcess(
+func (p *Proxy) fetchAndProcess(
 	ctx context.Context,
 	req *imagerequest.Request,
 	cacheKey imagerequest.VariantKey,
 ) (*processedVariant, error) {
 	// Check negative cache - skip fetching for recently-failed URLs
-	negHit, err := s.cache.checkNegativeCache(ctx, req)
+	negHit, err := p.cache.Failure(ctx, req.Source)
 	if err != nil {
-		s.log.Warn("negative cache check failed", "error", err)
+		p.log.Warn("negative cache check failed", "error", err)
 	}
 
 	if negHit {
-		s.log.Debug("negative cache hit",
+		p.log.Debug("negative cache hit",
 			"host", req.Source.Host,
 			"path", req.Source.Path,
 		)
@@ -334,14 +389,14 @@ func (s *Service) fetchAndProcess(
 	}
 
 	// Fetch from upstream
-	s.log.Debug("fetching from upstream", "host", req.Source.Host,
+	p.log.Debug("fetching from upstream", "host", req.Source.Host,
 		"path", req.Source.Path, "query", req.Source.Query)
 
-	fetchResult, err := s.fetcher.Fetch(ctx, req.Source)
+	fetchResult, err := p.fetcher.Fetch(ctx, req.Source)
 	if err != nil {
 		// Store negative cache for certain errors
 		if isNegativeCacheable(err) {
-			s.storeNegative(ctx, req)
+			p.storeNegative(ctx, req)
 		}
 
 		return nil, fmt.Errorf("upstream fetch failed: %w", err)
@@ -358,7 +413,7 @@ func (s *Service) fetchAndProcess(
 	fetchBytes := int64(len(sourceData))
 
 	// Counted also when the request context has ended meanwhile
-	s.cache.IncrementUpstreamFetch(fetchBytes)
+	p.incrementUpstreamFetch(fetchBytes)
 
 	if err != nil {
 		return nil, fmt.Errorf("failed to read upstream response: %w", err)
@@ -374,7 +429,7 @@ func (s *Service) fetchAndProcess(
 	}
 
 	// Log upstream fetch details
-	s.log.Info("upstream fetched",
+	p.log.Info("upstream fetched",
 		"request_id", middleware.GetReqID(ctx),
 		"host", req.Source.Host,
 		"path", req.Source.Path,
@@ -392,33 +447,44 @@ func (s *Service) fetchAndProcess(
 	// error status does.
 	err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType)
 	if err != nil {
-		s.log.Warn("source refused", "request_id", middleware.GetReqID(ctx),
+		p.log.Warn("source refused", "request_id", middleware.GetReqID(ctx),
 			"host", req.Source.Host, "path", req.Source.Path,
 			"content_type", fetchResult.ContentType, "error", err)
-		s.storeNegative(ctx, req)
+		p.storeNegative(ctx, req)
 
 		return nil, fmt.Errorf("content validation failed: %w", err)
 	}
 
 	// Store source content
-	_, err = s.cache.StoreSource(ctx, req, bytes.NewReader(sourceData))
+	_, err = p.cache.PutSource(ctx, req.Source, bytes.NewReader(sourceData))
 	if err != nil {
-		s.log.Warn("failed to store source content", "error", err)
+		p.log.Warn("failed to store source content", "error", err)
 		// Continue even if caching fails
 	}
 
-	return s.processAndStore(
+	return p.processAndStore(
 		ctx, req, cacheKey, bytes.NewReader(sourceData), fetchBytes,
 	)
 }
 
+// incrementUpstreamFetch counts one upstream fetch that read fetchBytes bytes.
+// A fetch that read no bytes is not counted.
+func (p *Proxy) incrementUpstreamFetch(fetchBytes int64) {
+	if fetchBytes <= 0 {
+		return
+	}
+
+	p.upstreamFetches.Inc()
+	p.upstreamFetchBytes.Add(float64(fetchBytes))
+}
+
 // storeNegative stores a negative cache entry for req's source, whose fetch
 // failed, so the source is not fetched again until the entry expires. A
 // failure to store the entry is only logged.
-func (s *Service) storeNegative(ctx context.Context, req *imagerequest.Request) {
+func (p *Proxy) storeNegative(ctx context.Context, req *imagerequest.Request) {
-	err := s.cache.StoreNegative(ctx, req)
+	err := p.cache.PutFailure(ctx, req.Source)
 	if err != nil {
-		s.log.Warn("failed to store negative cache entry",
+		p.log.Warn("failed to store negative cache entry",
 			"host", req.Source.Host, "path", req.Source.Path, "error", err)
 	}
 }
@@ -433,7 +499,7 @@ type processedVariant struct {
 
 // processAndStore processes the image read from source and stores the
 // result.
-func (s *Service) processAndStore(
+func (p *Proxy) processAndStore(
 	ctx context.Context,
 	req *imagerequest.Request,
 	cacheKey imagerequest.VariantKey,
@@ -445,7 +511,7 @@ func (s *Service) processAndStore(
 
 	// ctx has the deadline of the request doing the processing, which can
 	// end while a request waiting for its image has time left
-	processResult, err := s.processor.Process(ctx, source, &req.Transform)
+	processResult, err := p.processor.Process(ctx, source, &req.Transform)
 	if errors.Is(err, context.DeadlineExceeded) {
 		return nil, fmt.Errorf("%w: %w", ErrProcessingTimedOut, err)
 	}
@@ -457,7 +523,7 @@ func (s *Service) processAndStore(
 	processDuration := time.Since(processStart)
 
 	// Counted also when the request context has ended meanwhile
-	s.cache.IncrementTransformCount()
+	p.transforms.Inc()
 
 	// Read processed content
 	processedData, err := io.ReadAll(processResult.Content)
@@ -476,7 +542,7 @@ func (s *Service) processAndStore(
 		sizePercent = float64(outputSize) / float64(fetchBytes) * 100.0
 	}
 
-	s.log.Info("image converted",
+	p.log.Info("image converted",
 		"request_id", middleware.GetReqID(ctx),
 		"host", req.Source.Host,
 		"path", req.Source.Path,
@@ -495,9 +561,9 @@ func (s *Service) processAndStore(
 	)
 
 	// Store variant to cache
-	err = s.cache.StoreVariant(ctx, cacheKey, bytes.NewReader(processedData))
+	err = p.cache.PutVariant(ctx, cacheKey, bytes.NewReader(processedData))
 	if err != nil {
-		s.log.Warn("failed to store variant", "error", err)
+		p.log.Warn("failed to store variant", "error", err)
 		// Continue even if caching fails
 	}
 

internal/imageproxy/imageproxy_internal_test.go

@@ -1,4 +1,4 @@
-package imgcache
+package imageproxy
 
 import (
 	"context"
@@ -17,10 +17,10 @@ const (
 	testPathUpload = "/uploads/image.jpg"
 )
 
-func TestService_Get_GoodHost(t *testing.T) {
+func TestProxy_Get_GoodHost(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	ctx := context.Background()
 
 	req := &imagerequest.Request{
@@ -58,12 +58,12 @@ func TestService_Get_GoodHost(t *testing.T) {
 	}
 }
 
-// TestService_Get_OtherHost checks that the image service serves a second host
+// TestProxy_Get_OtherHost checks that the proxy serves a second host as it
-// as it serves GoodHost.
+// serves GoodHost.
-func TestService_Get_OtherHost(t *testing.T) {
+func TestProxy_Get_OtherHost(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	ctx := context.Background()
 
 	req := &imagerequest.Request{
@@ -97,10 +97,10 @@ func TestService_Get_OtherHost(t *testing.T) {
 	}
 }
 
-func TestService_Get_InvalidFile(t *testing.T) {
+func TestProxy_Get_InvalidFile(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	ctx := context.Background()
 
 	req := photoVariant(fixtures, 85, imagerequest.FitCover)
@@ -114,10 +114,10 @@ func TestService_Get_InvalidFile(t *testing.T) {
 	}
 }
 
-func TestService_Get_NotFound(t *testing.T) {
+func TestProxy_Get_NotFound(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	ctx := context.Background()
 
 	req := photoVariant(fixtures, 85, imagerequest.FitCover)
@@ -130,10 +130,10 @@ func TestService_Get_NotFound(t *testing.T) {
 	}
 }
 
-func TestService_Get_FormatConversion(t *testing.T) {
+func TestProxy_Get_FormatConversion(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	ctx := context.Background()
 
 	tests := []struct {
@@ -213,10 +213,10 @@ func TestService_Get_FormatConversion(t *testing.T) {
 	}
 }
 
-func TestService_Get_Caching(t *testing.T) {
+func TestProxy_Get_Caching(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	ctx := context.Background()
 
 	req := &imagerequest.Request{
@@ -272,7 +272,7 @@ func TestService_Get_Caching(t *testing.T) {
 	}
 }
 
-// TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo caches a source and
+// TestProxy_Get_ServesSourceAndVariantCachedTwoDaysAgo caches a source and
 // a variant of it and sets back two days every time pixa stored for them.
 // For the source: last_accessed_at in source_content, and the access and
 // modification times of its file. For the variant: last_accessed_at in
@@ -283,10 +283,10 @@ func TestService_Get_Caching(t *testing.T) {
 // Left at the present: the files' status change and birth times, which a
 // program cannot set, and the times of the directories holding the files,
 // which other entries share.
-func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) {
+func TestProxy_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	variant := photoVariant(fixtures, 85, imagerequest.FitCover)
 
 	resp, err := svc.Get(t.Context(), variant)
@@ -297,8 +297,8 @@ func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) {
 	_ = resp.Content.Close()
 
 	twoDaysAgo := time.Now().AddDate(0, 0, -2)
-	backdateSource(t, svc.cache, variant, twoDaysAgo)
+	backdateSource(t, svc, variant, twoDaysAgo)
-	backdateVariant(t, svc.cache, variant, twoDaysAgo)
+	backdateVariant(t, svc, variant, twoDaysAgo)
 
 	resp, err = svc.Get(t.Context(), variant)
 	if err != nil {
@@ -320,23 +320,23 @@ func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) {
 
 	_ = resp.Content.Close()
 
-	fetches := readCounters(svc.cache).upstreamFetchCount
+	fetches := readCounters(t, svc).upstreamFetchCount
 	if fetches != 1 {
 		t.Errorf("upstream fetches = %d, want 1", fetches)
 	}
 }
 
 // backdateSource sets every time pixa stored for req's cached source to when,
-// as listed on TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo.
+// as listed on TestProxy_Get_ServesSourceAndVariantCachedTwoDaysAgo.
 func backdateSource(
-	t *testing.T, cache *Cache, req *imagerequest.Request, when time.Time,
+	t *testing.T, svc *testProxy, req *imagerequest.Request, when time.Time,
 ) {
 	t.Helper()
 
 	key := req.Source.Key()
-	setSourceLastAccessed(t, cache, key, when)
+	setSourceLastAccessed(t, svc.db, key, when)
 
-	path := cache.srcContent.keyToPath(key)
+	path := sourceFilePath(svc, key)
 
 	err := os.Chtimes(path, when, when)
 	if err != nil {
@@ -346,16 +346,16 @@ func backdateSource(
 
 // backdateVariant sets every time pixa stored for the cached variant req asks
 // for to when, as listed on
-// TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo.
+// TestProxy_Get_ServesSourceAndVariantCachedTwoDaysAgo.
 func backdateVariant(
-	t *testing.T, cache *Cache, req *imagerequest.Request, when time.Time,
+	t *testing.T, svc *testProxy, req *imagerequest.Request, when time.Time,
 ) {
 	t.Helper()
 
 	key := imagerequest.CacheKey(req)
-	path := cache.variants.keyToPath(key)
+	path := variantFilePath(svc, key)
 
-	setVariantLastAccessed(t, cache, key, when)
+	setVariantLastAccessed(t, svc.db, key, when)
 
 	err := os.Chtimes(path, when, when)
 	if err != nil {
@@ -363,10 +363,10 @@ func backdateVariant(
 	}
 }
 
-func TestService_Get_DifferentSizes(t *testing.T) {
+func TestProxy_Get_DifferentSizes(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	ctx := context.Background()
 
 	// Request same image at different sizes
@@ -417,10 +417,10 @@ func TestService_Get_DifferentSizes(t *testing.T) {
 	}
 }
 
-func TestService_Get_ContextCancellation(t *testing.T) {
+func TestProxy_Get_ContextCancellation(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel() // Cancel immediately
@@ -444,10 +444,10 @@ func TestService_Get_ContextCancellation(t *testing.T) {
 	}
 }
 
-func TestService_Get_ReturnsETag(t *testing.T) {
+func TestProxy_Get_ReturnsETag(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	ctx := context.Background()
 
 	req := &imagerequest.Request{
@@ -481,10 +481,10 @@ func TestService_Get_ReturnsETag(t *testing.T) {
 	}
 }
 
-func TestService_Get_ETagConsistency(t *testing.T) {
+func TestProxy_Get_ETagConsistency(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	ctx := context.Background()
 
 	req := &imagerequest.Request{
@@ -526,10 +526,10 @@ func TestService_Get_ETagConsistency(t *testing.T) {
 	}
 }
 
-func TestService_Get_DifferentETagsForDifferentContent(t *testing.T) {
+func TestProxy_Get_DifferentETagsForDifferentContent(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	ctx := context.Background()
 
 	// Request same image at different sizes - should get different ETags

internal/imageproxy/max_concurrent_processing_internal_test.go

@@ -1,4 +1,4 @@
-package imgcache
+package imageproxy
 
 import (
 	"image/color"
@@ -55,16 +55,16 @@ func holdProcessingSlot(
 	return release
 }
 
-// TestService_Get_WaitsForSlotBeforeReadingCachedSource checks that a
+// TestProxy_Get_WaitsForSlotBeforeReadingCachedSource checks that a
 // request whose source is cached holds none of it while it waits for a
 // processing slot: it reads the cached file only once it has a slot. With
 // the only slot held, a request for a new width of the cached 100x100 photo
 // waits; the cached file is then rewritten as a 100x50 image before the slot
 // is freed, so the request must answer with that image scaled to 40x20.
-func TestService_Get_WaitsForSlotBeforeReadingCachedSource(t *testing.T) {
+func TestProxy_Get_WaitsForSlotBeforeReadingCachedSource(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 	svc.processor = imageprocessor.New(
 		imageprocessor.Params{MaxInputBytes: 50 << 20, MaxConcurrentProcessing: 1},
 	)
@@ -77,13 +77,15 @@ func TestService_Get_WaitsForSlotBeforeReadingCachedSource(t *testing.T) {
 
 	_ = resp.Content.Close()
 
-	sourceKey, err := svc.cache.LookupSource(t.Context(),
+	cachedSource := widthOnlyRequest(fixtures, 50).Source
-		widthOnlyRequest(fixtures, 50))
+
-	if err != nil || sourceKey == "" {
+	reader, _, err := svc.cache.Source(t.Context(), cachedSource)
-		t.Fatalf("LookupSource() = %q, %v; want the cached source",
+	if err != nil {
-			sourceKey, err)
+		t.Fatalf("Source() error = %v; want the cached source", err)
 	}
 
+	_ = reader.Close()
+
 	release := holdProcessingSlot(t, svc.processor)
 
 	var (
@@ -103,7 +105,7 @@ func TestService_Get_WaitsForSlotBeforeReadingCachedSource(t *testing.T) {
 	// before waiting, it would have read it by now.
 	time.Sleep(100 * time.Millisecond)
 
-	err = os.WriteFile(svc.cache.srcContent.keyToPath(sourceKey),
+	err = os.WriteFile(sourceFilePath(svc, cachedSource.Key()),
 		generateTestJPEG(t, 100, 50, color.RGBA{0, 0, 255, 255}), 0o600)
 	if err != nil {
 		t.Fatalf("failed to rewrite the cached source: %v", err)

internal/imageproxy/negative_cache_internal_test.go

@@ -0,0 +1,96 @@
+package imageproxy
+
+import (
+	"errors"
+	"os"
+	"testing"
+
+	"sneak.berlin/go/pixa/internal/imagerequest"
+)
+
+// TestProxy_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 TestProxy_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := setupTestProxy(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.PutFailure(t.Context(),
+		photoVariant(fixtures, 85, imagerequest.FitCover).Source)
+	if err != nil {
+		t.Fatalf("PutFailure() 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()
+
+	source := photoVariant(fixtures, 85, imagerequest.FitCover).Source
+
+	reader, _, err := svc.cache.Source(t.Context(), source)
+	if err != nil {
+		t.Fatalf("Source() error = %v, want the cached source", err)
+	}
+
+	_ = reader.Close()
+
+	// Evicts the source as the cache's eviction does: its row, then its file
+	_, err = svc.db.ExecContext(t.Context(),
+		`DELETE FROM source_content WHERE source_key = ?`, string(source.Key()))
+	if err != nil {
+		t.Fatalf("deleting the source's row: %v", err)
+	}
+
+	err = os.Remove(sourceFilePath(svc, source.Key()))
+	if err != nil {
+		t.Fatalf("removing the source's file: %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)
+	}
+}
+
+func TestProxy_Get_ReturnsErrorForNegativeCachedURL(t *testing.T) {
+	t.Parallel()
+
+	// This test verifies that Proxy.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")
+	}
+}

internal/imageproxy/partial_cache_files_internal_test.go

@@ -0,0 +1,149 @@
+package imageproxy
+
+import (
+	"bytes"
+	"io"
+	"os"
+	"testing"
+
+	"sneak.berlin/go/pixa/internal/imagerequest"
+)
+
+// getImage calls Get for req and returns the image it answered with and
+// its cache status.
+func getImage(
+	t *testing.T, svc *testProxy, req *imagerequest.Request,
+) ([]byte, CacheStatus) {
+	t.Helper()
+
+	resp, err := svc.Get(t.Context(), req)
+	if err != nil {
+		t.Fatalf("Get() of quality %d error = %v", req.Transform.Quality, err)
+	}
+
+	defer func() { _ = resp.Content.Close() }()
+
+	body, err := io.ReadAll(resp.Content)
+	if err != nil {
+		t.Fatalf("reading the image of quality %d: %v", req.Transform.Quality, err)
+	}
+
+	return body, resp.CacheStatus
+}
+
+// fileSize returns the size of the file at path.
+func fileSize(t *testing.T, path string) int64 {
+	t.Helper()
+
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatalf("stat %s: %v", path, err)
+	}
+
+	return info.Size()
+}
+
+// cachedSourcePath returns the path of the cached source file req uses.
+func cachedSourcePath(
+	t *testing.T, svc *testProxy, req *imagerequest.Request,
+) string {
+	t.Helper()
+
+	reader, _, err := svc.cache.Source(t.Context(), req.Source)
+	if err != nil {
+		t.Fatalf("Source() error = %v; want the cached source", err)
+	}
+
+	_ = reader.Close()
+
+	return sourceFilePath(svc, req.Source.Key())
+}
+
+// TestProxy_Get_FetchesAgainForRemovedCachedSource removes the cached
+// source's row and file, as the reconciliation pass does when the file is
+// empty or cut short. internal/imgcache tests that the pass does so. The
+// test then asks for two new sizes of the source. The first must fetch it
+// again and cache it whole, so the second is made from the cache without a
+// third fetch.
+func TestProxy_Get_FetchesAgainForRemovedCachedSource(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := setupTestProxy(t)
+
+	getImage(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
+
+	source := photoVariant(fixtures, 85, imagerequest.FitCover).Source
+	path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
+	wholeSize := fileSize(t, path)
+
+	// Removes the source as the reconciliation pass does: its row, then its file
+	_, err := svc.db.ExecContext(t.Context(),
+		`DELETE FROM source_content WHERE source_key = ?`, string(source.Key()))
+	if err != nil {
+		t.Fatalf("deleting the source's row: %v", err)
+	}
+
+	err = os.Remove(path)
+	if err != nil {
+		t.Fatalf("removing the source's file: %v", err)
+	}
+
+	getImage(t, svc, photoVariant(fixtures, 40, imagerequest.FitCover))
+	getImage(t, svc, photoVariant(fixtures, 30, imagerequest.FitCover))
+
+	fetches := readCounters(t, svc).upstreamFetchCount
+	t.Logf("upstream fetches = %d", fetches)
+
+	if fetches != 2 {
+		t.Errorf("upstream fetches = %d, want 2", fetches)
+	}
+
+	size := fileSize(t, path)
+	if size != wholeSize {
+		t.Errorf("cached source is %d bytes, want %d", size, wholeSize)
+	}
+}
+
+// TestProxy_Get_ConvertsAgainForRemovedCachedVariant caches a variant of
+// the photo and removes its row and file, as the reconciliation pass does
+// when the file is empty or cut short. internal/imgcache tests that the
+// pass does so. The test then checks that Get converts the image again, and
+// that the next Get serves the whole variant from the cache.
+func TestProxy_Get_ConvertsAgainForRemovedCachedVariant(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := setupTestProxy(t)
+	req := photoVariant(fixtures, 85, imagerequest.FitCover)
+
+	want, _ := getImage(t, svc, req)
+
+	key := imagerequest.CacheKey(req)
+
+	// Removes the variant as the reconciliation pass does: its row, then its file
+	_, err := svc.db.ExecContext(t.Context(),
+		`DELETE FROM variant_content WHERE cache_key = ?`, string(key))
+	if err != nil {
+		t.Fatalf("deleting the variant's row: %v", err)
+	}
+
+	err = os.Remove(variantFilePath(svc, key))
+	if err != nil {
+		t.Fatalf("removing the variant's file: %v", err)
+	}
+
+	got, status := getImage(t, svc, req)
+	t.Logf("after the removal: %d bytes, %s", len(got), status)
+
+	if status != CacheMiss || !bytes.Equal(got, want) {
+		t.Errorf("after the removal: Get() = %d bytes, %s; want the %d bytes "+
+			"converted again, %s", len(got), status, len(want), CacheMiss)
+	}
+
+	got, status = getImage(t, svc, req)
+	t.Logf("next request: %d bytes, %s", len(got), status)
+
+	if status != CacheHit || !bytes.Equal(got, want) {
+		t.Errorf("next request: Get() = %d bytes, %s; want the %d bytes, %s",
+			len(got), status, len(want), CacheHit)
+	}
+}

internal/imageproxy/pendingaccesstimes_internal_test.go

@@ -0,0 +1,78 @@
+package imageproxy
+
+import (
+	"context"
+	"database/sql"
+	"errors"
+	"testing"
+	"time"
+
+	"sneak.berlin/go/pixa/internal/imagerequest"
+)
+
+// pendingAccessTimesWait is how long a test waits for the database's
+// connection, for a cache hit to be answered, for an access time to reach
+// the database, or for a request to return past its deadline.
+const pendingAccessTimesWait = 5 * time.Second
+
+// holdDatabase takes the one connection of db, so that every other query
+// waits for it, and returns the func that frees it.
+func holdDatabase(t *testing.T, db *sql.DB) func() {
+	t.Helper()
+
+	ctx, cancel := context.WithTimeout(t.Context(), pendingAccessTimesWait)
+	defer cancel()
+
+	conn, err := db.Conn(ctx)
+	if err != nil {
+		t.Fatalf("failed to take the database connection: %v", err)
+	}
+
+	return func() { _ = conn.Close() }
+}
+
+// TestProxy_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy holds the
+// database's one connection while a request whose fetch is held reaches its
+// deadline. The request must return with the deadline's error while the
+// connection is still held, as it waits for no database write past its
+// deadline.
+func TestProxy_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures, fetcher := setupHeldFetchProxy(t)
+
+	svc.cache.StartPendingAccessTimeWrites()
+	defer func() { _ = svc.cache.StopPendingAccessTimeWrites(t.Context()) }()
+
+	// Room for the request to reach its fetch on a busy host
+	const timeout = 2 * time.Second
+
+	ctx, cancel := context.WithTimeout(t.Context(), timeout)
+	defer cancel()
+
+	deadline, _ := ctx.Deadline()
+
+	results := startGet(ctx, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
+
+	// The request has made its database reads by the time it fetches.
+	select {
+	case <-fetcher.started:
+	case <-time.After(timeout):
+		t.Fatal("request did not reach its fetch by its deadline")
+	}
+
+	releaseDatabase := holdDatabase(t, svc.db)
+	defer releaseDatabase()
+
+	select {
+	case got := <-results:
+		t.Logf("Get() returned %v after its deadline, error = %v",
+			time.Since(deadline), got.err)
+
+		if !errors.Is(got.err, context.DeadlineExceeded) {
+			t.Errorf("Get() error = %v, want %v", got.err, context.DeadlineExceeded)
+		}
+	case <-time.After(time.Until(deadline) + pendingAccessTimesWait):
+		t.Fatal("request did not return while the database was busy")
+	}
+}

internal/imageproxy/stats_internal_test.go

@@ -1,4 +1,4 @@
-package imgcache
+package imageproxy
 
 import (
 	"bytes"
@@ -12,15 +12,16 @@ import (
 	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
-// TestService_Get_CountsStats walks Get through a miss that fetches the
+// TestProxy_Get_CountsStats walks Get through a miss that fetches the
 // source, a hit, a miss that reuses the cached source, and two misses whose
-// source cannot be used, checking every counter of the cache after each.
+// source cannot be used, checking every counter of the proxy and its cache
-func TestService_Get_CountsStats(t *testing.T) {
+// after each.
+func TestProxy_Get_CountsStats(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 
-	// NewTestFS builds the same files the test service's fetcher serves.
+	// NewTestFS builds the same files the test proxy's fetcher serves.
 	testFS, _ := NewTestFS(t)
 
 	photo, err := fs.ReadFile(testFS, fixtures.GoodHostJPEG)
@@ -76,7 +77,7 @@ func TestService_Get_CountsStats(t *testing.T) {
 			_ = resp.Content.Close()
 		}
 
-		got := readCounters(svc.cache)
+		got := readCounters(t, svc)
 		if got != step.want {
 			t.Fatalf("after the %s: counters = %+v, want %+v", step.name, got, step.want)
 		}
@@ -114,10 +115,11 @@ func (u *fakeUpstream) Read(p []byte) (int, error) {
 	return 0, u.err
 }
 
-// TestService_Get_CountsInterruptedMisses checks every counter of the cache
+// TestProxy_Get_CountsInterruptedMisses checks every counter of the proxy
-// after a miss whose request context ends during or after the upstream
+// and its cache after a miss whose request context ends during or after the
-// fetch, and after a miss whose upstream body is over the size limit.
+// upstream fetch, and after a miss whose upstream body is over the size
-func TestService_Get_CountsInterruptedMisses(t *testing.T) {
+// limit.
+func TestProxy_Get_CountsInterruptedMisses(t *testing.T) {
 	t.Parallel()
 
 	photo := generateTestJPEG(t, 100, 100, color.RGBA{255, 0, 0, 255})
@@ -145,7 +147,7 @@ func TestService_Get_CountsInterruptedMisses(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			svc, fixtures := SetupTestService(t)
+			svc, fixtures := setupTestProxy(t)
 
 			ctx, cancel := context.WithCancel(t.Context())
 			defer cancel()
@@ -182,7 +184,7 @@ func TestService_Get_CountsInterruptedMisses(t *testing.T) {
 				_ = resp.Content.Close()
 			}
 
-			got := readCounters(svc.cache)
+			got := readCounters(t, svc)
 			if got != tc.want {
 				t.Errorf("counters = %+v, want %+v", got, tc.want)
 			}
@@ -190,13 +192,13 @@ func TestService_Get_CountsInterruptedMisses(t *testing.T) {
 	}
 }
 
-// TestService_Get_CountsHitAfterRequestEnds checks every counter of the
+// TestProxy_Get_CountsHitAfterRequestEnds checks every counter of the
-// cache after a hit served with a request context that has already ended:
+// proxy and its cache after a hit served with a request context that has
-// only the hit count moves.
+// already ended: only the hit count moves.
-func TestService_Get_CountsHitAfterRequestEnds(t *testing.T) {
+func TestProxy_Get_CountsHitAfterRequestEnds(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	svc, fixtures := setupTestProxy(t)
 
 	req := &imagerequest.Request{
 		Source: imagerequest.Source{
@@ -219,7 +221,7 @@ func TestService_Get_CountsHitAfterRequestEnds(t *testing.T) {
 
 	_ = resp.Content.Close()
 
-	want := readCounters(svc.cache)
+	want := readCounters(t, svc)
 	want.hitCount++
 
 	ctx, cancel := context.WithCancel(t.Context())
@@ -236,7 +238,7 @@ func TestService_Get_CountsHitAfterRequestEnds(t *testing.T) {
 		t.Fatalf("CacheStatus = %v, want %v", resp.CacheStatus, CacheHit)
 	}
 
-	got := readCounters(svc.cache)
+	got := readCounters(t, svc)
 	if got != want {
 		t.Errorf("counters = %+v, want %+v", got, want)
 	}

internal/imageproxy/testutil_internal_test.go

@@ -0,0 +1,355 @@
+package imageproxy
+
+import (
+	"bytes"
+	"context"
+	"database/sql"
+	"image"
+	"image/color"
+	"image/gif"
+	"image/jpeg"
+	"image/png"
+	"io/fs"
+	"path/filepath"
+	"testing"
+	"testing/fstest"
+	"time"
+
+	"github.com/prometheus/client_golang/prometheus"
+	"sneak.berlin/go/pixa/internal/database"
+	"sneak.berlin/go/pixa/internal/httpfetcher"
+	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imagerequest"
+	"sneak.berlin/go/pixa/internal/imgcache"
+)
+
+// Shared test data literals, extracted as constants for goconst.
+const (
+	testContentTypeJPEG = "image/jpeg"
+)
+
+// sqliteTimestampFormat matches the format SQLite's CURRENT_TIMESTAMP
+// produces, so injected timestamps compare correctly against ones the
+// cache writes.
+const sqliteTimestampFormat = "2006-01-02 15:04:05"
+
+// TestFixtures contains paths to test files in the mock filesystem.
+type TestFixtures struct {
+	// Valid image files
+	GoodHostJPEG  string // valid JPEG on GoodHost
+	GoodHostPNG   string // valid PNG on GoodHost
+	GoodHostGIF   string // valid GIF on GoodHost
+	OtherHostJPEG string // valid JPEG on OtherHost
+	OtherHostPNG  string // valid PNG on OtherHost
+
+	// Invalid/edge case files
+	InvalidFile string // file with wrong magic bytes
+	EmptyFile   string // zero-byte file
+	TextFile    string // text file masquerading as image
+
+	// Hostnames
+	GoodHost  string // a host the mock fetcher serves
+	OtherHost string // a second host the mock fetcher serves
+}
+
+// DefaultFixtures returns the standard test fixture paths.
+func DefaultFixtures() *TestFixtures {
+	return &TestFixtures{
+		GoodHostJPEG:  "goodhost.example.com/images/photo.jpg",
+		GoodHostPNG:   "goodhost.example.com/images/logo.png",
+		GoodHostGIF:   "goodhost.example.com/images/animation.gif",
+		OtherHostJPEG: "otherhost.example.com/uploads/image.jpg",
+		OtherHostPNG:  "otherhost.example.com/uploads/icon.png",
+		InvalidFile:   "goodhost.example.com/images/fake.jpg",
+		EmptyFile:     "goodhost.example.com/images/empty.jpg",
+		TextFile:      "goodhost.example.com/images/text.png",
+		GoodHost:      "goodhost.example.com",
+		OtherHost:     "otherhost.example.com",
+	}
+}
+
+// NewTestFS creates a mock filesystem with test images.
+func NewTestFS(t *testing.T) (fs.FS, *TestFixtures) {
+	t.Helper()
+
+	fixtures := DefaultFixtures()
+
+	// Generate test images
+	jpegData := generateTestJPEG(t, 100, 100, color.RGBA{255, 0, 0, 255})
+	pngData := generateTestPNG(t, 100, 100, color.RGBA{0, 255, 0, 255})
+	gifData := generateTestGIF(t, 100, 100, color.RGBA{0, 0, 255, 255})
+
+	// Create the mock filesystem
+	mockFS := fstest.MapFS{
+		// Good host files
+		fixtures.GoodHostJPEG: &fstest.MapFile{Data: jpegData},
+		fixtures.GoodHostPNG:  &fstest.MapFile{Data: pngData},
+		fixtures.GoodHostGIF:  &fstest.MapFile{Data: gifData},
+
+		// Other host files
+		fixtures.OtherHostJPEG: &fstest.MapFile{Data: jpegData},
+		fixtures.OtherHostPNG:  &fstest.MapFile{Data: pngData},
+
+		// Invalid files
+		fixtures.InvalidFile: &fstest.MapFile{Data: []byte("not a real image file content")},
+		fixtures.EmptyFile:   &fstest.MapFile{Data: []byte{}},
+		fixtures.TextFile:    &fstest.MapFile{Data: []byte("This is just text, not a PNG")},
+	}
+
+	return mockFS, fixtures
+}
+
+// generateTestJPEG creates a minimal valid JPEG image.
+func generateTestJPEG(t *testing.T, width, height int, c color.Color) []byte {
+	t.Helper()
+
+	img := image.NewRGBA(image.Rect(0, 0, width, height))
+	for y := range height {
+		for x := range width {
+			img.Set(x, y, c)
+		}
+	}
+
+	var buf bytes.Buffer
+
+	err := jpeg.Encode(&buf, img, &jpeg.Options{Quality: 85})
+	if err != nil {
+		t.Fatalf("failed to encode test JPEG: %v", err)
+	}
+
+	return buf.Bytes()
+}
+
+// generateTestPNG creates a minimal valid PNG image.
+func generateTestPNG(t *testing.T, width, height int, c color.Color) []byte {
+	t.Helper()
+
+	img := image.NewRGBA(image.Rect(0, 0, width, height))
+	for y := range height {
+		for x := range width {
+			img.Set(x, y, c)
+		}
+	}
+
+	var buf bytes.Buffer
+
+	err := png.Encode(&buf, img)
+	if err != nil {
+		t.Fatalf("failed to encode test PNG: %v", err)
+	}
+
+	return buf.Bytes()
+}
+
+// generateTestGIF creates a minimal valid GIF image.
+func generateTestGIF(t *testing.T, width, height int, c color.Color) []byte {
+	t.Helper()
+
+	img := image.NewPaletted(
+		image.Rect(0, 0, width, height),
+		[]color.Color{c, color.White},
+	)
+	for y := range height {
+		for x := range width {
+			img.SetColorIndex(x, y, 0)
+		}
+	}
+
+	var buf bytes.Buffer
+
+	err := gif.Encode(&buf, img, nil)
+	if err != nil {
+		t.Fatalf("failed to encode test GIF: %v", err)
+	}
+
+	return buf.Bytes()
+}
+
+// testProxy is a Proxy with the database and the state directory of its
+// cache, through which a test reads and changes the cache behind the proxy.
+// registry is the metrics registry the proxy and its cache share.
+type testProxy struct {
+	*Proxy
+
+	db       *sql.DB
+	stateDir string
+	registry *prometheus.Registry
+}
+
+// setupTestProxy creates a Proxy with mock fetcher for testing.
+func setupTestProxy(t *testing.T) (*testProxy, *TestFixtures) {
+	t.Helper()
+
+	mockFS, fixtures := NewTestFS(t)
+
+	// Create temp directory for cache
+	tmpDir := t.TempDir()
+
+	// Create in-memory database
+	db := setupTestDB(t)
+
+	registry := prometheus.NewRegistry()
+
+	cache, err := imgcache.NewCache(db, imgcache.Config{
+		StateDir:    tmpDir,
+		NegativeTTL: 5 * time.Minute,
+		MaxBytes:    1 << 30,
+	}, registry)
+	if err != nil {
+		t.Fatalf("failed to create cache: %v", err)
+	}
+
+	proxy, err := New(&Config{
+		Cache:   cache,
+		Fetcher: httpfetcher.NewMock(mockFS),
+		Processor: imageprocessor.New(imageprocessor.Params{
+			MaxInputBytes:           50 << 20,
+			MaxConcurrentProcessing: 2,
+		}),
+		MaxResponseSize: 50 << 20,
+	}, registry)
+	if err != nil {
+		t.Fatalf("failed to create proxy: %v", err)
+	}
+
+	return &testProxy{
+		Proxy:    proxy,
+		db:       db,
+		stateDir: tmpDir,
+		registry: registry,
+	}, fixtures
+}
+
+// counterValues holds the values of the counters of a proxy and its cache.
+type counterValues struct {
+	hitCount           int64
+	missCount          int64
+	upstreamFetchCount int64
+	upstreamFetchBytes int64
+	transformCount     int64
+}
+
+// readCounters reads the counters of svc and its cache from their metrics
+// registry.
+func readCounters(t *testing.T, svc *testProxy) counterValues {
+	t.Helper()
+
+	families, err := svc.registry.Gather()
+	if err != nil {
+		t.Fatalf("failed to gather the counters: %v", err)
+	}
+
+	values := make(map[string]int64, len(families))
+
+	for _, family := range families {
+		values[family.GetName()] = int64(family.GetMetric()[0].GetCounter().GetValue())
+	}
+
+	return counterValues{
+		hitCount:           values["pixa_cache_hits_total"],
+		missCount:          values["pixa_cache_misses_total"],
+		upstreamFetchCount: values["pixa_upstream_fetches_total"],
+		upstreamFetchBytes: values["pixa_upstream_fetch_bytes_total"],
+		transformCount:     values["pixa_transforms_total"],
+	}
+}
+
+// sourceFilePath returns the path of the file of the source key in svc's
+// cache, as README.md gives it: <state_dir>/cache/sources/<ab>/<cd>/<key>.
+func sourceFilePath(svc *testProxy, key imagerequest.SourceKey) string {
+	name := string(key)
+
+	return filepath.Join(svc.stateDir, "cache", "sources", name[0:2], name[2:4], name)
+}
+
+// variantFilePath returns the path of the file of the variant key in svc's
+// cache, as README.md gives it: <state_dir>/cache/variants/<ab>/<cd>/<key>.
+func variantFilePath(svc *testProxy, key imagerequest.VariantKey) string {
+	name := string(key)
+
+	return filepath.Join(svc.stateDir, "cache", "variants", name[0:2], name[2:4], name)
+}
+
+// setVariantLastAccessed backdates the last access time of a tracked
+// variant.
+func setVariantLastAccessed(
+	t *testing.T, db *sql.DB, key imagerequest.VariantKey, when time.Time,
+) {
+	t.Helper()
+
+	res, err := db.ExecContext(t.Context(),
+		`UPDATE variant_content SET last_accessed_at = ? WHERE cache_key = ?`,
+		when.UTC().Format(sqliteTimestampFormat), string(key),
+	)
+	if err != nil {
+		t.Fatalf("failed to set variant last_accessed_at: %v", err)
+	}
+
+	affected, err := res.RowsAffected()
+	if err != nil {
+		t.Fatalf("failed to read affected rows: %v", err)
+	}
+
+	if affected != 1 {
+		t.Fatalf("variant %s has no accounting row (affected=%d); "+
+			"stores must track variants in the database", key, affected)
+	}
+}
+
+// setSourceLastAccessed backdates the last access time of a tracked
+// source content blob.
+func setSourceLastAccessed(
+	t *testing.T, db *sql.DB, key imagerequest.SourceKey, when time.Time,
+) {
+	t.Helper()
+
+	res, err := db.ExecContext(t.Context(),
+		`UPDATE source_content SET last_accessed_at = ? WHERE source_key = ?`,
+		when.UTC().Format(sqliteTimestampFormat), string(key),
+	)
+	if err != nil {
+		t.Fatalf("failed to set source last_accessed_at: %v", err)
+	}
+
+	affected, err := res.RowsAffected()
+	if err != nil {
+		t.Fatalf("failed to read affected rows: %v", err)
+	}
+
+	if affected != 1 {
+		t.Fatalf("source %s has no accounting row (affected=%d)", key, affected)
+	}
+}
+
+// countRows returns the number of rows the given query yields.
+func countRows(t *testing.T, db *sql.DB, query string, args ...any) int {
+	t.Helper()
+
+	var n int
+
+	err := db.QueryRowContext(t.Context(), query, args...).Scan(&n)
+	if err != nil {
+		t.Fatalf("count query %q failed: %v", query, err)
+	}
+
+	return n
+}
+
+// setupTestDB creates an in-memory SQLite database for testing
+// using the production schema.
+func setupTestDB(t *testing.T) *sql.DB {
+	t.Helper()
+
+	db, err := sql.Open("sqlite", ":memory:")
+	if err != nil {
+		t.Fatalf("failed to open test db: %v", err)
+	}
+
+	// Use the real production schema via migrations
+	err = database.ApplyMigrations(context.Background(), db, nil)
+	if err != nil {
+		t.Fatalf("failed to apply migrations: %v", err)
+	}
+
+	return db
+}

internal/imgcache/cache.go

@@ -1,3 +1,10 @@
+// Package imgcache is the image cache. It keeps:
+//   - the source images fetched from upstream
+//   - the variants made from them
+//   - the negative cache: the source URLs whose fetch failed recently
+//
+// Each image is a file with a row in the database. Eviction removes the
+// least recently used images to keep the cache under its size limit.
 package imgcache
 
 import (
@@ -26,8 +33,12 @@ const maxPendingAccessTimes = 10000
 // have expired.
 const cleanExpiredBatchSize = 1000
 
-// CacheConfig holds cache configuration.
+// DefaultNegativeTTL is how long pixad keeps a failed fetch in the negative
-type CacheConfig struct {
+// cache.
+const DefaultNegativeTTL = 5 * time.Minute
+
+// Config holds cache configuration.
+type Config struct {
 	StateDir    string
 	NegativeTTL time.Duration
 
@@ -48,7 +59,7 @@ type Cache struct {
 	db         *sql.DB
 	srcContent *ContentStorage // source images by source key
 	variants   *VariantStorage // processed variants by cache key
-	config     CacheConfig
+	config     Config
 	log        *slog.Logger
 
 	// Eviction machinery. The channels are created in NewCache so
@@ -73,13 +84,10 @@ type Cache struct {
 	pendingAccessTimesDone    chan struct{}
 	pendingAccessTimesCancel  context.CancelFunc
 
-	// The counts of what the cache does, made and registered by
+	// The counts of cache hits and misses, made and registered by
 	// registerCounters (see counters.go)
 	cacheHits   prometheus.Counter
 	cacheMisses prometheus.Counter
-	upstreamFetches    prometheus.Counter
-	upstreamFetchBytes prometheus.Counter
-	transforms         prometheus.Counter
 
 	// reconciliationPageSize is the most rows one read of a content table
 	// returns in the reconciliation pass. NewCache sets it to
@@ -97,7 +105,7 @@ type Cache struct {
 // metricsRegistry. pixad passes Prometheus' default registry, the one
 // /metrics serves. It makes no database call.
 func NewCache(
-	db *sql.DB, config CacheConfig, metricsRegistry prometheus.Registerer,
+	db *sql.DB, config Config, metricsRegistry prometheus.Registerer,
 ) (*Cache, error) {
 	log := config.Logger
 	if log == nil {
@@ -147,75 +155,35 @@ func NewCache(
 	return c, nil
 }
 
-// GetVariant opens a cached variant and returns a reader, its size and its
+// Variant opens a cached variant and returns a reader, its size and its
 // content type, read from the variant file's first bytes. It opens that one
-// file and makes no database call. A variant not in the cache, or any
+// file and makes no database call. A variant it opens has its access time
-// variant of a disabled cache, is ErrNotFound; a variant file whose first
+// handed to the background writer without waiting (see touchVariant). A
-// bytes are not those of an output format is another error.
+// variant not in the cache, or any variant of a disabled cache, is
-func (c *Cache) GetVariant(
+// ErrNotFound; a variant file whose first bytes are not those of an output
+// format is another error.
+func (c *Cache) Variant(
 	cacheKey imagerequest.VariantKey,
 ) (io.ReadCloser, int64, string, error) {
 	if c.config.MaxBytes == 0 {
 		return nil, 0, "", ErrNotFound
 	}
 
-	return c.variants.LoadWithContentType(cacheKey)
+	reader, size, contentType, err := c.variants.LoadWithContentType(cacheKey)
+	if err != nil {
+		return nil, 0, "", err
+	}
+
+	c.touchVariant(cacheKey)
+
+	return reader, size, contentType, nil
 }
 
-// StoreSource stores fetched source content: it writes the file named by
+// PutVariant stores a processed variant by its cache key and records
-// the source's key, then the source's row. When the row cannot be written
-// the file is removed and the store fails, as a file with no row is never
-// served or evicted; if that removal fails too, the next reconciliation
-// pass removes the file. On a disabled cache it is a no-op returning an
-// empty key.
-func (c *Cache) StoreSource(
-	ctx context.Context,
-	req *imagerequest.Request,
-	content io.Reader,
-) (imagerequest.SourceKey, error) {
-	if c.config.MaxBytes == 0 {
-		return "", nil
-	}
-
-	data, err := io.ReadAll(content)
-	if err != nil {
-		return "", fmt.Errorf("failed to read source content: %w", err)
-	}
-
-	key := req.Source.Key()
-
-	size, err := c.srcContent.Store(key, data)
-	if err != nil {
-		return "", fmt.Errorf("failed to store source content: %w", err)
-	}
-
-	_, err = c.db.ExecContext(ctx, `
-		INSERT INTO source_content (source_key, size_bytes)
-		VALUES (?, ?)
-		ON CONFLICT(source_key) DO UPDATE SET
-			size_bytes = excluded.size_bytes,
-			last_accessed_at = CURRENT_TIMESTAMP
-	`, string(key), size)
-	if err != nil {
-		deleteErr := c.srcContent.Delete(key)
-		if deleteErr != nil {
-			c.log.Warn("failed to remove source file whose row was not written",
-				"source_key", key, "error", deleteErr)
-		}
-
-		return "", fmt.Errorf("failed to insert source content: %w", err)
-	}
-
-	c.notifyWritePressure()
-
-	return key, nil
-}
-
-// StoreVariant stores a processed variant by its cache key and records
 // it in the size accounting. On a disabled cache it is a no-op. The
 // accounting insert is best-effort (the startup reconciliation pass
 // adopts any variant file that misses its accounting row).
-func (c *Cache) StoreVariant(
+func (c *Cache) PutVariant(
 	ctx context.Context, cacheKey imagerequest.VariantKey, content io.Reader,
 ) error {
 	if c.config.MaxBytes == 0 {
@@ -244,19 +212,18 @@ func (c *Cache) StoreVariant(
 	return nil
 }
 
-// LookupSource checks if we have cached source content for a request.
+// Source opens the cached source of src and returns a reader and its size
-// Returns the source key if found, or an empty key if not; a row whose
+// in bytes. A source whose row or file is missing is ErrNotFound, as is any
-// file is missing is not found. A source found has its access time
+// source of a disabled cache. A source it opens has its access time handed
-// handed to the background writer without waiting (see
+// to the background writer without waiting (see touchSourceContent).
-// touchSourceContent); a disabled cache always reports no cached source.
+func (c *Cache) Source(
-func (c *Cache) LookupSource(
+	ctx context.Context, src imagerequest.Source,
-	ctx context.Context, req *imagerequest.Request,
+) (io.ReadCloser, int64, error) {
-) (imagerequest.SourceKey, error) {
 	if c.config.MaxBytes == 0 {
-		return "", nil
+		return nil, 0, ErrNotFound
 	}
 
-	key := req.Source.Key()
+	key := src.Key()
 
 	var rowExists bool
 
@@ -264,25 +231,100 @@ func (c *Cache) LookupSource(
 		SELECT EXISTS(SELECT 1 FROM source_content WHERE source_key = ?)
 	`, string(key)).Scan(&rowExists)
 	if err != nil {
-		return "", fmt.Errorf("failed to lookup source: %w", err)
+		return nil, 0, fmt.Errorf("failed to lookup source: %w", err)
 	}
 
 	if !rowExists {
-		return "", nil
+		return nil, 0, ErrNotFound
 	}
 
-	// Verify the content file exists
+	reader, size, err := c.srcContent.LoadWithSize(key)
-	if !c.srcContent.Exists(key) {
+	if err != nil {
-		return "", nil
+		return nil, 0, err
 	}
 
 	c.touchSourceContent(key)
 
+	return reader, size, nil
+}
+
+// PutSource stores fetched source content: it writes the file named by
+// the source's key, then the source's row. When the row cannot be written
+// the file is removed and the store fails, as a file with no row is never
+// served or evicted; if that removal fails too, the next reconciliation
+// pass removes the file. On a disabled cache it is a no-op returning an
+// empty key.
+func (c *Cache) PutSource(
+	ctx context.Context,
+	src imagerequest.Source,
+	content io.Reader,
+) (imagerequest.SourceKey, error) {
+	if c.config.MaxBytes == 0 {
+		return "", nil
+	}
+
+	data, err := io.ReadAll(content)
+	if err != nil {
+		return "", fmt.Errorf("failed to read source content: %w", err)
+	}
+
+	key := src.Key()
+
+	size, err := c.srcContent.Store(key, data)
+	if err != nil {
+		return "", fmt.Errorf("failed to store source content: %w", err)
+	}
+
+	_, err = c.db.ExecContext(ctx, `
+		INSERT INTO source_content (source_key, size_bytes)
+		VALUES (?, ?)
+		ON CONFLICT(source_key) DO UPDATE SET
+			size_bytes = excluded.size_bytes,
+			last_accessed_at = CURRENT_TIMESTAMP
+	`, string(key), size)
+	if err != nil {
+		deleteErr := c.srcContent.Delete(key)
+		if deleteErr != nil {
+			c.log.Warn("failed to remove source file whose row was not written",
+				"source_key", key, "error", deleteErr)
+		}
+
+		return "", fmt.Errorf("failed to insert source content: %w", err)
+	}
+
+	c.notifyWritePressure()
+
 	return key, nil
 }
 
-// StoreNegative stores a negative cache entry for a failed fetch.
+// Failure reports whether src is in the negative cache. It only reads: a
-func (c *Cache) StoreNegative(ctx context.Context, req *imagerequest.Request) error {
+// row past its expiry is not a hit, and the eviction goroutine deletes it
+// with the other expired rows (see CleanExpired).
+func (c *Cache) Failure(
+	ctx context.Context, src imagerequest.Source,
+) (bool, error) {
+	var expiresAt time.Time
+
+	err := c.db.QueryRowContext(ctx, `
+		SELECT expires_at FROM negative_cache
+		WHERE source_host = ? AND source_path = ? AND source_query = ?
+	`, src.Host, src.Path, src.Query).Scan(&expiresAt)
+
+	if errors.Is(err, sql.ErrNoRows) {
+		return false, nil
+	}
+
+	if err != nil {
+		return false, fmt.Errorf("failed to check negative cache: %w", err)
+	}
+
+	expired := time.Now().After(expiresAt)
+
+	return !expired, nil
+}
+
+// PutFailure stores a negative cache entry for src, whose fetch failed.
+func (c *Cache) PutFailure(ctx context.Context, src imagerequest.Source) error {
 	expiresAt := time.Now().UTC().Add(c.config.NegativeTTL)
 
 	_, err := c.db.ExecContext(ctx, `
@@ -291,7 +333,7 @@ func (c *Cache) StoreNegative(ctx context.Context, req *imagerequest.Request) er
 		VALUES (?, ?, ?, ?)
 		ON CONFLICT(source_host, source_path, source_query) DO UPDATE SET
 			expires_at = excluded.expires_at
-	`, req.Source.Host, req.Source.Path, req.Source.Query, expiresAt)
+	`, src.Host, src.Path, src.Query, expiresAt)
 	if err != nil {
 		return fmt.Errorf("failed to insert negative cache: %w", err)
 	}
@@ -299,18 +341,6 @@ func (c *Cache) StoreNegative(ctx context.Context, req *imagerequest.Request) er
 	return nil
 }
 
-// GetSourceContent returns a reader for cached source content by its key,
-// and the content's size in bytes.
-func (c *Cache) GetSourceContent(
-	key imagerequest.SourceKey,
-) (io.ReadCloser, int64, error) {
-	if c.config.MaxBytes == 0 {
-		return nil, 0, ErrNotFound
-	}
-
-	return c.srcContent.LoadWithSize(key)
-}
-
 // CleanExpired removes the negative cache rows past their expiry, one
 // batch per statement, until none is left. Once ctx ends it starts no
 // further batch and returns ctx's error. The eviction goroutine calls it
@@ -334,34 +364,15 @@ func (c *Cache) CleanExpired(ctx context.Context) error {
 	}
 }
 
-// IncrementStats counts a cache hit or miss, and an upstream fetch that read
+// IncrementStats counts a cache hit or miss. It only adds to the cache's
-// fetchBytes bytes, as IncrementUpstreamFetch does. Like the other Increment
+// Prometheus counters in memory (see counters.go), so it never waits for the
-// methods, it only adds to the cache's Prometheus counters in memory (see
+// database or fails.
-// counters.go), so it never waits for the database or fails.
+func (c *Cache) IncrementStats(hit bool) {
-func (c *Cache) IncrementStats(hit bool, fetchBytes int64) {
 	if hit {
 		c.cacheHits.Inc()
 	} else {
 		c.cacheMisses.Inc()
 	}
-
-	c.IncrementUpstreamFetch(fetchBytes)
-}
-
-// IncrementUpstreamFetch counts one upstream fetch that read fetchBytes bytes.
-// A fetch that read no bytes is not counted.
-func (c *Cache) IncrementUpstreamFetch(fetchBytes int64) {
-	if fetchBytes <= 0 {
-		return
-	}
-
-	c.upstreamFetches.Inc()
-	c.upstreamFetchBytes.Add(float64(fetchBytes))
-}
-
-// IncrementTransformCount counts one image transcoded by the image processor.
-func (c *Cache) IncrementTransformCount() {
-	c.transforms.Inc()
 }
 
 // touchVariant hands the access time of a variant served from the cache to
@@ -388,32 +399,6 @@ func (c *Cache) touchSourceContent(key imagerequest.SourceKey) {
 	}
 }
 
-// checkNegativeCache checks if a request is in the negative cache. It only
-// reads: a row past its expiry is not a hit, and the eviction goroutine
-// deletes it with the other expired rows (see CleanExpired).
-func (c *Cache) checkNegativeCache(
-	ctx context.Context, req *imagerequest.Request,
-) (bool, error) {
-	var expiresAt time.Time
-
-	err := c.db.QueryRowContext(ctx, `
-		SELECT expires_at FROM negative_cache
-		WHERE source_host = ? AND source_path = ? AND source_query = ?
-	`, req.Source.Host, req.Source.Path, req.Source.Query).Scan(&expiresAt)
-
-	if errors.Is(err, sql.ErrNoRows) {
-		return false, nil
-	}
-
-	if err != nil {
-		return false, fmt.Errorf("failed to check negative cache: %w", err)
-	}
-
-	expired := time.Now().After(expiresAt)
-
-	return !expired, nil
-}
-
 // cleanExpiredBatch deletes up to cleanExpiredBatchSize negative cache
 // rows past their expiry in one statement, and returns how many it
 // deleted. DELETE ... LIMIT needs a SQLite build option, so the subquery

internal/imgcache/cache_hit_internal_test.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"image/color"
 	"io"
-	"os"
 	"testing"
 	"time"
 
@@ -15,10 +14,10 @@ import (
 	"sneak.berlin/go/pixa/internal/magic"
 )
 
-// TestGetVariant_ContentTypeFromFirstBytes stores an image in each output
+// TestVariant_ContentTypeFromFirstBytes stores an image in each output
-// format as a variant and checks that GetVariant serves it whole with the
+// format as a variant and checks that Variant serves it whole with the
 // content type of its format, read from the file.
-func TestGetVariant_ContentTypeFromFirstBytes(t *testing.T) {
+func TestVariant_ContentTypeFromFirstBytes(t *testing.T) {
 	t.Parallel()
 
 	cache, _ := newEvictionTestCache(t, 1<<30)
@@ -50,14 +49,14 @@ func TestGetVariant_ContentTypeFromFirstBytes(t *testing.T) {
 
 		key := imagerequest.VariantKey("aabbccdd10" + string(format))
 
-		err = cache.StoreVariant(t.Context(), key, bytes.NewReader(image))
+		err = cache.PutVariant(t.Context(), key, bytes.NewReader(image))
 		if err != nil {
-			t.Fatalf("StoreVariant(%s) error = %v", format, err)
+			t.Fatalf("PutVariant(%s) error = %v", format, err)
 		}
 
-		reader, size, contentType, err := cache.GetVariant(key)
+		reader, size, contentType, err := cache.Variant(key)
 		if err != nil {
-			t.Fatalf("GetVariant(%s) error = %v", format, err)
+			t.Fatalf("Variant(%s) error = %v", format, err)
 		}
 
 		served, err := io.ReadAll(reader)
@@ -80,11 +79,11 @@ func TestGetVariant_ContentTypeFromFirstBytes(t *testing.T) {
 	}
 }
 
-// TestGetVariant_RefusesFileOfNoOutputFormat checks that GetVariant refuses
+// TestVariant_RefusesFileOfNoOutputFormat checks that Variant refuses
 // a variant file whose first bytes are not those of an output format, with
 // an error other than ErrNotFound, and that a variant with no file, or any
 // variant of a disabled cache, is ErrNotFound.
-func TestGetVariant_RefusesFileOfNoOutputFormat(t *testing.T) {
+func TestVariant_RefusesFileOfNoOutputFormat(t *testing.T) {
 	t.Parallel()
 
 	cache, _ := newEvictionTestCache(t, 1<<30)
@@ -110,118 +109,30 @@ func TestGetVariant_RefusesFileOfNoOutputFormat(t *testing.T) {
 			t.Fatalf("%s: storing the variant file: %v", tt.name, err)
 		}
 
-		reader, _, contentType, err := cache.GetVariant(key)
+		reader, _, contentType, err := cache.Variant(key)
-		t.Logf("%s: GetVariant() error = %v", tt.name, err)
+		t.Logf("%s: Variant() error = %v", tt.name, err)
 
 		if reader != nil || contentType != "" || !errors.Is(err, tt.wantErr) {
-			t.Errorf("%s: GetVariant() = reader %v, content type %q, error %v; "+
+			t.Errorf("%s: Variant() = reader %v, content type %q, error %v; "+
 				"want no reader and %v", tt.name, reader, contentType, err, tt.wantErr)
 		}
 	}
 
-	reader, _, _, err := cache.GetVariant(testVariantKeyFour)
+	reader, _, _, err := cache.Variant(testVariantKeyFour)
 	if reader != nil || !errors.Is(err, ErrNotFound) {
-		t.Errorf("GetVariant() of a variant with no file = reader %v, error %v; "+
+		t.Errorf("Variant() of a variant with no file = reader %v, error %v; "+
 			"want no reader and %v", reader, err, ErrNotFound)
 	}
 
 	disabled, _ := newEvictionTestCache(t, 0)
 
-	reader, _, _, err = disabled.GetVariant(testVariantKeyOne)
+	reader, _, _, err = disabled.Variant(testVariantKeyOne)
 	if reader != nil || !errors.Is(err, ErrNotFound) {
-		t.Errorf("GetVariant() on a disabled cache = reader %v, error %v; "+
+		t.Errorf("Variant() on a disabled cache = reader %v, error %v; "+
 			"want no reader and %v", reader, err, ErrNotFound)
 	}
 }
 
-// TestService_Get_MakesAgainVariantOfNoOutputFormat replaces a cached
-// variant's file with text, and checks that the next request is a miss
-// answered with the variant made again, and the one after a hit.
-func TestService_Get_MakesAgainVariantOfNoOutputFormat(t *testing.T) {
-	t.Parallel()
-
-	svc, fixtures := SetupTestService(t)
-	req := photoVariant(fixtures, 85, imagerequest.FitCover)
-
-	want, _ := getImage(t, svc, req)
-
-	path := svc.cache.variants.keyToPath(imagerequest.CacheKey(req))
-
-	err := os.WriteFile(path, []byte("this is text, not an image"), StorageFilePerm)
-	if err != nil {
-		t.Fatalf("replacing the variant file: %v", err)
-	}
-
-	got, status := getImage(t, svc, req)
-	t.Logf("after the replacement: %d bytes, %s", len(got), status)
-
-	if status != CacheMiss || !bytes.Equal(got, want) {
-		t.Errorf("after the replacement: Get() = %d bytes, %s; want the %d bytes "+
-			"made again, %s", len(got), status, len(want), CacheMiss)
-	}
-
-	got, status = getImage(t, svc, req)
-
-	if status != CacheHit || !bytes.Equal(got, want) {
-		t.Errorf("next request: Get() = %d bytes, %s; want the %d bytes, %s",
-			len(got), status, len(want), CacheHit)
-	}
-}
-
-// TestService_Get_HitWaitsForNoDatabaseWrite holds the database's one
-// connection while a cached variant is requested. The hit must be answered
-// while the connection is still held, and its access time must reach the
-// database once it is free.
-func TestService_Get_HitWaitsForNoDatabaseWrite(t *testing.T) {
-	t.Parallel()
-
-	svc, fixtures := SetupTestService(t)
-	req := photoVariant(fixtures, 85, imagerequest.FitCover)
-	key := imagerequest.CacheKey(req)
-
-	getImage(t, svc, req)
-
-	// Each connection to an in-memory SQLite database opens a new, empty one,
-	// so the goroutine that writes the access times must share the
-	// connection that holds the schema.
-	svc.cache.db.SetMaxOpenConns(1)
-	svc.cache.StartPendingAccessTimeWrites()
-
-	defer func() { _ = svc.cache.StopPendingAccessTimeWrites(t.Context()) }()
-
-	anHourAgo := time.Now().Add(-time.Hour)
-	setVariantLastAccessed(t, svc.cache, key, anHourAgo)
-
-	releaseDatabase := holdDatabase(t, svc.cache)
-	defer releaseDatabase()
-
-	results := startGet(t.Context(), svc, req)
-
-	select {
-	case got := <-results:
-		if got.err != nil {
-			t.Fatalf("Get() of the cached variant error = %v", got.err)
-		}
-	case <-time.After(pendingAccessTimesWait):
-		t.Fatal("the cache hit waited for the database")
-	}
-
-	releaseDatabase()
-
-	deadline := time.Now().Add(pendingAccessTimesWait)
-
-	for countRows(t, svc.cache, `
-		SELECT COUNT(*) FROM variant_content
-		WHERE cache_key = ? AND last_accessed_at > ?
-	`, string(key), anHourAgo.UTC().Format(sqliteTimestampFormat)) == 0 {
-		if time.Now().After(deadline) {
-			t.Fatal("the hit's access time was not written")
-		}
-
-		time.Sleep(10 * time.Millisecond)
-	}
-}
-
 // TestTouchVariantAndTouchSourceContent_DropAccessTimesPastTheLimit hands one
 // more access time of a variant, and one more of a source, than may wait to
 // be written. touchVariant and touchSourceContent must return each time

internal/imgcache/cache_internal_test.go

@@ -50,7 +50,7 @@ func TestCache_CleanExpired(t *testing.T) {
 	tmpDir := t.TempDir()
 	db := setupTestDB(t)
 
-	cache, _ := NewCache(db, CacheConfig{
+	cache, _ := NewCache(db, Config{
 		StateDir:    tmpDir,
 		NegativeTTL: 1 * time.Millisecond,
 		MaxBytes:    1 << 30,
@@ -103,7 +103,7 @@ func TestCache_StorageDirectoriesCreated(t *testing.T) {
 	tmpDir := t.TempDir()
 	db := setupTestDB(t)
 
-	_, err := NewCache(db, CacheConfig{
+	_, err := NewCache(db, Config{
 		StateDir: tmpDir,
 		MaxBytes: 1 << 30,
 	}, prometheus.NewRegistry())

internal/imgcache/counters.go

@@ -19,22 +19,8 @@ func (c *Cache) registerCounters(metricsRegistry prometheus.Registerer) error {
 		Name: "pixa_cache_misses_total",
 		Help: "Image requests not served from the cache.",
 	})
-	c.upstreamFetches = prometheus.NewCounter(prometheus.CounterOpts{
-		Name: "pixa_upstream_fetches_total",
-		Help: "Fetches of a source image from upstream that read any bytes.",
-	})
-	c.upstreamFetchBytes = prometheus.NewCounter(prometheus.CounterOpts{
-		Name: "pixa_upstream_fetch_bytes_total",
-		Help: "Bytes read from upstream by fetches of source images.",
-	})
-	c.transforms = prometheus.NewCounter(prometheus.CounterOpts{
-		Name: "pixa_transforms_total",
-		Help: "Images the image processor transformed.",
-	})
 
-	for _, counter := range []prometheus.Counter{
+	for _, counter := range []prometheus.Counter{c.cacheHits, c.cacheMisses} {
-		c.cacheHits, c.cacheMisses, c.upstreamFetches, c.upstreamFetchBytes, c.transforms,
-	} {
 		err := metricsRegistry.Register(counter)
 		if err != nil {
 			return fmt.Errorf("failed to register the cache's counters: %w", err)

internal/imgcache/counters_internal_test.go

@@ -8,26 +8,6 @@ import (
 	"github.com/prometheus/client_golang/prometheus/testutil"
 )
 
-// counterValues holds the values of a cache's counters.
-type counterValues struct {
-	hitCount           int64
-	missCount          int64
-	upstreamFetchCount int64
-	upstreamFetchBytes int64
-	transformCount     int64
-}
-
-// readCounters reads cache's counters.
-func readCounters(cache *Cache) counterValues {
-	return counterValues{
-		hitCount:           int64(testutil.ToFloat64(cache.cacheHits)),
-		missCount:          int64(testutil.ToFloat64(cache.cacheMisses)),
-		upstreamFetchCount: int64(testutil.ToFloat64(cache.upstreamFetches)),
-		upstreamFetchBytes: int64(testutil.ToFloat64(cache.upstreamFetchBytes)),
-		transformCount:     int64(testutil.ToFloat64(cache.transforms)),
-	}
-}
-
 // TestNewCache_RegistersItsCountersOnItsRegistry checks that NewCache
 // registers the cache's counters on the registry it is given, under the
 // names README.md gives them.
@@ -37,7 +17,7 @@ func TestNewCache_RegistersItsCountersOnItsRegistry(t *testing.T) {
 	registry := prometheus.NewRegistry()
 
 	_, err := NewCache(evictionTestDB(t),
-		CacheConfig{StateDir: t.TempDir(), MaxBytes: 1 << 30}, registry)
+		Config{StateDir: t.TempDir(), MaxBytes: 1 << 30}, registry)
 	if err != nil {
 		t.Fatalf("NewCache() error = %v", err)
 	}
@@ -45,9 +25,6 @@ func TestNewCache_RegistersItsCountersOnItsRegistry(t *testing.T) {
 	names := []string{
 		"pixa_cache_hits_total",
 		"pixa_cache_misses_total",
-		"pixa_upstream_fetches_total",
-		"pixa_upstream_fetch_bytes_total",
-		"pixa_transforms_total",
 	}
 
 	count, err := testutil.GatherAndCount(registry, names...)
@@ -70,13 +47,13 @@ func TestNewCache_FailsWhenItsCountersCannotBeRegistered(t *testing.T) {
 	registry := prometheus.NewRegistry()
 
 	_, err := NewCache(evictionTestDB(t),
-		CacheConfig{StateDir: t.TempDir(), MaxBytes: 1 << 30}, registry)
+		Config{StateDir: t.TempDir(), MaxBytes: 1 << 30}, registry)
 	if err != nil {
 		t.Fatalf("NewCache() of the first cache error = %v", err)
 	}
 
 	_, err = NewCache(evictionTestDB(t),
-		CacheConfig{StateDir: t.TempDir(), MaxBytes: 1 << 30}, registry)
+		Config{StateDir: t.TempDir(), MaxBytes: 1 << 30}, registry)
 	t.Logf("NewCache() of the second cache error = %v", err)
 
 	var alreadyRegistered prometheus.AlreadyRegisteredError

internal/imgcache/eviction.go

@@ -317,7 +317,7 @@ func (c *Cache) evictVariant(
 // evictSourceBlob removes one source: its row first, then its file, as
 // evictVariant does for a variant. It holds no lock: a store of the same
 // source in between can leave the source's row with no file, which
-// LookupSource treats as a miss.
+// Source treats as a miss.
 func (c *Cache) evictSourceBlob(ctx context.Context, key imagerequest.SourceKey) error {
 	_, err := c.db.ExecContext(ctx,
 		`DELETE FROM source_content WHERE source_key = ?`, string(key))
@@ -479,7 +479,7 @@ func (c *Cache) runCleanExpired(ctx context.Context) {
 // then on the first periodic eviction tick that comes at least ten times
 // as long after the last pass ended as that pass took, off the request
 // hot path: it adopts variant files that predate the accounting table
-// (or whose accounting insert failed, e.g. StoreVariant's best-effort
+// (or whose accounting insert failed, e.g. PutVariant's best-effort
 // insert under transient DB contention), drops variant accounting rows
 // whose files are missing, removes source blob files the database does
 // not know, removes source and variant files that are empty or not the

internal/imgcache/eviction_internal_test.go

@@ -67,7 +67,7 @@ func newEvictionTestCache(t *testing.T, maxBytes int64) (*Cache, string) {
 	db := evictionTestDB(t)
 
 	// maxBytes zero turns the disk cache off, as cache_max_bytes: 0 does.
-	cache, err := NewCache(db, CacheConfig{
+	cache, err := NewCache(db, Config{
 		StateDir:    tmpDir,
 		NegativeTTL: 5 * time.Minute,
 		MaxBytes:    maxBytes,
@@ -98,11 +98,11 @@ func storeEvictionTestSource(
 		},
 	}
 
-	key, err := cache.StoreSource(
+	key, err := cache.PutSource(
-		context.Background(), req, bytes.NewReader(content),
+		context.Background(), req.Source, bytes.NewReader(content),
 	)
 	if err != nil {
-		t.Fatalf("StoreSource(%s%s) failed: %v", host, path, err)
+		t.Fatalf("PutSource(%s%s) failed: %v", host, path, err)
 	}
 
 	return key
@@ -115,9 +115,9 @@ func storeEvictionTestVariant(
 ) {
 	t.Helper()
 
-	err := cache.StoreVariant(t.Context(), key, bytes.NewReader(content))
+	err := cache.PutVariant(t.Context(), key, bytes.NewReader(content))
 	if err != nil {
-		t.Fatalf("StoreVariant(%s) failed: %v", key, err)
+		t.Fatalf("PutVariant(%s) failed: %v", key, err)
 	}
 }
 
@@ -423,20 +423,20 @@ func assertDisabledCacheWritesAreNoOps(
 
 	ctx := t.Context()
 
-	err := cache.StoreVariant(
+	err := cache.PutVariant(
 		ctx, imagerequest.CacheKey(req), bytes.NewReader([]byte("data")),
 	)
 	if err != nil {
-		t.Fatalf("StoreVariant on disabled cache must be a no-op, got error: %v", err)
+		t.Fatalf("PutVariant on disabled cache must be a no-op, got error: %v", err)
 	}
 
-	key, err := cache.StoreSource(ctx, req, bytes.NewReader([]byte("data")))
+	key, err := cache.PutSource(ctx, req.Source, bytes.NewReader([]byte("data")))
 	if err != nil {
-		t.Fatalf("StoreSource on disabled cache must be a no-op, got error: %v", err)
+		t.Fatalf("PutSource on disabled cache must be a no-op, got error: %v", err)
 	}
 
 	if key != "" {
-		t.Errorf("StoreSource on disabled cache returned key %q, want empty", key)
+		t.Errorf("PutSource on disabled cache returned key %q, want empty", key)
 	}
 }
 
@@ -449,13 +449,13 @@ func assertDisabledCacheReadsAlwaysMiss(
 
 	ctx := t.Context()
 
-	key, err := cache.LookupSource(ctx, req)
+	reader, _, err := cache.Source(ctx, req.Source)
-	if err != nil {
+	if !errors.Is(err, ErrNotFound) {
-		t.Fatalf("LookupSource on disabled cache failed: %v", err)
+		t.Fatalf("Source on disabled cache error = %v, want %v", err, ErrNotFound)
 	}
 
-	if key != "" {
+	if reader != nil {
-		t.Errorf("LookupSource on disabled cache = %q, want empty", key)
+		t.Errorf("Source on disabled cache = reader %v, want none", reader)
 	}
 }
 
@@ -544,7 +544,7 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) {
 }
 
 // TestEvictionRunsOnPeriodicSchedule writes three variant files straight
-// to disk, bypassing StoreVariant, so they have no accounting rows and no
+// to disk, bypassing PutVariant, so they have no accounting rows and no
 // write-pressure notification fires. Only a periodic reconciliation pass
 // can then adopt them, and only the eviction pass that follows it can
 // evict them.
@@ -697,7 +697,7 @@ func TestStartEvictionReconcilesAccountingWithDisk(t *testing.T) {
 // TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup proves
 // reconciliation is not a one-shot startup-only pass: it must also run
 // on the periodic ticker, so a variant file that lands on disk with no
-// accounting row well after startup (e.g. because StoreVariant's
+// accounting row well after startup (e.g. because PutVariant's
 // best-effort accounting insert failed under transient contention, or
 // any other cause of an untracked file appearing during steady-state
 // operation) is still adopted into accounting eventually, rather than
@@ -733,7 +733,7 @@ func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
 
 	// Simulate a variant whose accounting insert failed after the
 	// process was already running and serving requests: the content
-	// file is written directly, bypassing StoreVariant's (and thus its
+	// file is written directly, bypassing PutVariant's (and thus its
 	// 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)

internal/imgcache/imgcache.go

@@ -1,29 +0,0 @@
-// Package imgcache provides types for the image caching proxy.
-package imgcache
-
-import "io"
-
-// ImageResponse represents a processed image ready to serve
-type ImageResponse struct {
-	// Content is the image data reader
-	Content io.ReadCloser
-	// ContentLength is the size in bytes (-1 if unknown)
-	ContentLength int64
-	// ContentType is the MIME type of the response
-	ContentType string
-	// ETag is the entity tag for caching
-	ETag string
-	// CacheStatus indicates HIT or MISS
-	CacheStatus CacheStatus
-	// FetchedBytes is the number of bytes fetched from upstream (0 if cache hit)
-	FetchedBytes int64
-}
-
-// CacheStatus indicates whether the response was served from cache.
-type CacheStatus string
-
-// Cache status values for response headers.
-const (
-	CacheHit  CacheStatus = "HIT"
-	CacheMiss CacheStatus = "MISS"
-)

internal/imgcache/module.go

@@ -1,8 +0,0 @@
-package imgcache
-
-import "time"
-
-// CacheConfig defaults.
-const (
-	DefaultNegativeTTL = 5 * time.Minute
-)

internal/imgcache/negative_cache_internal_test.go

@@ -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")
-	}
-}

internal/imgcache/partial_cache_files_internal_test.go

@@ -12,67 +12,6 @@ import (
 	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
-// getImage calls Get for req and returns the image it answered with and
-// its cache status.
-func getImage(
-	t *testing.T, svc *Service, req *imagerequest.Request,
-) ([]byte, CacheStatus) {
-	t.Helper()
-
-	resp, err := svc.Get(t.Context(), req)
-	if err != nil {
-		t.Fatalf("Get() of quality %d error = %v", req.Transform.Quality, err)
-	}
-
-	defer func() { _ = resp.Content.Close() }()
-
-	body, err := io.ReadAll(resp.Content)
-	if err != nil {
-		t.Fatalf("reading the image of quality %d: %v", req.Transform.Quality, err)
-	}
-
-	return body, resp.CacheStatus
-}
-
-// fileSize returns the size of the file at path.
-func fileSize(t *testing.T, path string) int64 {
-	t.Helper()
-
-	info, err := os.Stat(path)
-	if err != nil {
-		t.Fatalf("stat %s: %v", path, err)
-	}
-
-	return info.Size()
-}
-
-// truncateFile cuts the file at path down to size bytes.
-func truncateFile(t *testing.T, path string, size int64) {
-	t.Helper()
-
-	t.Logf("cutting %s from %d to %d bytes",
-		filepath.Base(path), fileSize(t, path), size)
-
-	err := os.Truncate(path, size)
-	if err != nil {
-		t.Fatalf("truncating %s: %v", path, err)
-	}
-}
-
-// cachedSourcePath returns the path of the cached source file req uses.
-func cachedSourcePath(
-	t *testing.T, svc *Service, req *imagerequest.Request,
-) string {
-	t.Helper()
-
-	sourceKey, err := svc.cache.LookupSource(t.Context(), req)
-	if err != nil || sourceKey == "" {
-		t.Fatalf("LookupSource() = %q, %v; want the cached source", sourceKey, err)
-	}
-
-	return svc.cache.srcContent.keyToPath(sourceKey)
-}
-
 // reconcileAsAtStartup runs the cache's reconciliation pass, as pixa does
 // when it starts, and returns what the pass logged.
 func reconcileAsAtStartup(t *testing.T, cache *Cache) string {
@@ -107,127 +46,65 @@ func assertWarningNames(t *testing.T, logs, path string) {
 	t.Errorf("reconciliation logged no warning naming %s", path)
 }
 
-// TestService_Get_FetchesAgainForEmptyCachedSource empties the cached source
+// TestReconciliationRemovesDamagedFiles empties a cached source file and a
-// file, as a crash or a restored snapshot could, and runs the reconciliation
+// cached variant file, or cuts them to half their size, as a crash or a
-// pass, which must remove it with a warning naming it. It then asks for two
+// restored snapshot could. It checks that the reconciliation pass removes
-// new sizes of the source: the first must fetch it again and cache it whole,
+// each file and its row, with a warning naming the file.
-// so that the second is made from the cache without a third fetch.
+func TestReconciliationRemovesDamagedFiles(t *testing.T) {
-func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+	content := []byte("the whole cached image")
 
-	getImage(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
+	for name, size := range map[string]int64{
-
+		"empty":   0,
-	path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
+		"partial": int64(len(content) / 2),
-	wholeSize := fileSize(t, path)
+	} {
-
+		t.Run(name, func(t *testing.T) {
-	truncateFile(t, path, 0)
-	assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path)
-
-	getImage(t, svc, photoVariant(fixtures, 40, imagerequest.FitCover))
-	getImage(t, svc, photoVariant(fixtures, 30, imagerequest.FitCover))
-
-	fetches := readCounters(svc.cache).upstreamFetchCount
-	t.Logf("upstream fetches = %d", fetches)
-
-	if fetches != 2 {
-		t.Errorf("upstream fetches = %d, want 2: the empty cached source "+
-			"was kept and fetched again on every new size", fetches)
-	}
-
-	size := fileSize(t, path)
-	if size != wholeSize {
-		t.Errorf("cached source is %d bytes, want %d", size, wholeSize)
-	}
-}
-
-// TestService_Get_FetchesAgainForPartialCachedSource cuts the cached source
-// file to half its size, as a crash or a restored snapshot could, and runs
-// the reconciliation pass, which must remove it with a warning naming it.
-// It then asks for two new sizes of the source: the first must fetch it
-// again rather than fail to decode the partial file, and the second is
-// made from the cache.
-func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) {
 			t.Parallel()
 
-	svc, fixtures := SetupTestService(t)
+			cache, _ := newEvictionTestCache(t, 1<<30)
 
-	getImage(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
+			sourceKey := storeEvictionTestSource(t, cache, testHostCDN, testPathCat, content)
+			storeEvictionTestVariant(t, cache, testVariantKeyOne, content)
 
-	path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
+			sourcePath := cache.srcContent.keyToPath(sourceKey)
-	wholeSize := fileSize(t, path)
+			variantPath := cache.variants.keyToPath(testVariantKeyOne)
 
-	truncateFile(t, path, wholeSize/2)
+			for _, path := range []string{sourcePath, variantPath} {
-	assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path)
+				err := os.Truncate(path, size)
-
+				if err != nil {
-	getImage(t, svc, photoVariant(fixtures, 40, imagerequest.FitCover))
+					t.Fatalf("cutting %s to %d bytes: %v", path, size, err)
-	getImage(t, svc, photoVariant(fixtures, 30, imagerequest.FitCover))
+				}
-
-	fetches := readCounters(svc.cache).upstreamFetchCount
-	t.Logf("upstream fetches = %d", fetches)
-
-	if fetches != 2 {
-		t.Errorf("upstream fetches = %d, want 2", fetches)
 			}
 
-	size := fileSize(t, path)
+			logs := reconcileAsAtStartup(t, cache)
-	if size != wholeSize {
-		t.Errorf("cached source is %d bytes, want %d", size, wholeSize)
-	}
-}
 
-// assertConvertedAgain caches a variant of the photo, cuts its file to size
+			assertWarningNames(t, logs, sourcePath)
-// bytes and runs the reconciliation pass, which must remove it with a
+			assertWarningNames(t, logs, variantPath)
-// warning naming it. It then checks that Get converts the image again
-// instead of serving the file, and that the next Get serves the whole
-// variant from the cache.
-func assertConvertedAgain(t *testing.T, size int64) {
-	t.Helper()
 
-	svc, fixtures := SetupTestService(t)
+			if cache.srcContent.Exists(sourceKey) {
-	req := photoVariant(fixtures, 85, imagerequest.FitCover)
+				t.Error("the damaged source file is still on disk")
-
-	want, _ := getImage(t, svc, req)
-
-	path := svc.cache.variants.keyToPath(imagerequest.CacheKey(req))
-
-	truncateFile(t, path, size)
-	assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path)
-
-	got, status := getImage(t, svc, req)
-	t.Logf("after the cut: %d bytes, %s", len(got), status)
-
-	if status != CacheMiss || !bytes.Equal(got, want) {
-		t.Errorf("after the cut: Get() = %d bytes, %s; want the %d bytes "+
-			"converted again, %s", len(got), status, len(want), CacheMiss)
 			}
 
-	got, status = getImage(t, svc, req)
+			if cache.variants.Exists(testVariantKeyOne) {
-	t.Logf("next request: %d bytes, %s", len(got), status)
+				t.Error("the damaged variant file is still on disk")
-
-	if status != CacheHit || !bytes.Equal(got, want) {
-		t.Errorf("next request: Get() = %d bytes, %s; want the %d bytes, %s",
-			len(got), status, len(want), CacheHit)
 			}
-}
 
-// TestService_Get_ConvertsAgainForEmptyCachedVariant empties a cached
+			rows := countRows(t, cache,
-// variant file, as a crash or a restored snapshot could, and checks that it
+				`SELECT COUNT(*) FROM source_content WHERE source_key = ?`,
-// is converted again rather than served empty.
+				string(sourceKey))
-func TestService_Get_ConvertsAgainForEmptyCachedVariant(t *testing.T) {
+			if rows != 0 {
-	t.Parallel()
+				t.Errorf("the damaged source file kept %d rows, want 0", rows)
+			}
 
-	assertConvertedAgain(t, 0)
+			rows = countRows(t, cache,
-}
+				`SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`,
-
+				string(testVariantKeyOne))
-// TestService_Get_ConvertsAgainForPartialCachedVariant cuts a cached
+			if rows != 0 {
-// variant file to ten bytes, as a crash or a restored snapshot could, and
+				t.Errorf("the damaged variant file kept %d rows, want 0", rows)
-// checks that it is converted again rather than served cut short.
+			}
-func TestService_Get_ConvertsAgainForPartialCachedVariant(t *testing.T) {
+		})
-	t.Parallel()
+	}
-
-	assertConvertedAgain(t, 10)
 }
 
 // TestReconciliationRemovesEmptyVariantFileWithoutRow checks that the

internal/imgcache/pendingaccesstimes_internal_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"database/sql"
 	"errors"
+	"io"
 	"log/slog"
 	"strings"
 	"testing"
@@ -15,8 +16,7 @@ import (
 
 // pendingAccessTimesWait is how long a test waits for the database's
 // connection, for an access time to reach the database, for a write to start
-// waiting for the connection, for a request to return past its deadline, or
+// waiting for the connection, or for StopPendingAccessTimeWrites to return.
-// for StopPendingAccessTimeWrites to return.
 const pendingAccessTimesWait = 5 * time.Second
 
 // otherWritesWait is how long a test gives other writes, once one write waits
@@ -56,52 +56,6 @@ func waitForConnectionWaits(t *testing.T, db *sql.DB, waits int64) {
 	}
 }
 
-// TestService_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy holds the
-// database's one connection while a request whose fetch is held reaches its
-// deadline. The request must return with the deadline's error while the
-// connection is still held, as it waits for no database write past its
-// deadline.
-func TestService_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy(t *testing.T) {
-	t.Parallel()
-
-	svc, fixtures, fetcher := setupHeldFetchService(t)
-
-	svc.cache.StartPendingAccessTimeWrites()
-	defer func() { _ = svc.cache.StopPendingAccessTimeWrites(t.Context()) }()
-
-	// Room for the request to reach its fetch on a busy host
-	const timeout = 2 * time.Second
-
-	ctx, cancel := context.WithTimeout(t.Context(), timeout)
-	defer cancel()
-
-	deadline, _ := ctx.Deadline()
-
-	results := startGet(ctx, svc, photoVariant(fixtures, 85, imagerequest.FitCover))
-
-	// The request has made its database reads by the time it fetches.
-	select {
-	case <-fetcher.started:
-	case <-time.After(timeout):
-		t.Fatal("request did not reach its fetch by its deadline")
-	}
-
-	releaseDatabase := holdDatabase(t, svc.cache)
-	defer releaseDatabase()
-
-	select {
-	case got := <-results:
-		t.Logf("Get() returned %v after its deadline, error = %v",
-			time.Since(deadline), got.err)
-
-		if !errors.Is(got.err, context.DeadlineExceeded) {
-			t.Errorf("Get() error = %v, want %v", got.err, context.DeadlineExceeded)
-		}
-	case <-time.After(time.Until(deadline) + pendingAccessTimesWait):
-		t.Fatal("request did not return while the database was busy")
-	}
-}
-
 // TestStopPendingAccessTimeWrites_LetsTheWriteUnderWayFinish holds the
 // database's one connection while the goroutine that writes the pending
 // access times waits for it, hands over another access time, then stops that
@@ -242,11 +196,11 @@ func TestStopPendingAccessTimeWrites_ReturnsWhenItsContextEnds(t *testing.T) {
 	}
 }
 
-// TestLookupSource_HandsAccessTimeToTheBackgroundWriter finds a cached source
+// TestSource_HandsAccessTimeToTheBackgroundWriter finds a cached source
-// whose access time is an hour old. LookupSource must not write the access
+// whose access time is an hour old. Source must not write the access
 // time itself but hand it over to be written, and StopPendingAccessTimeWrites
 // must then write it.
-func TestLookupSource_HandsAccessTimeToTheBackgroundWriter(t *testing.T) {
+func TestSource_HandsAccessTimeToTheBackgroundWriter(t *testing.T) {
 	t.Parallel()
 
 	cache, _ := newEvictionTestCache(t, 1<<30)
@@ -261,15 +215,21 @@ func TestLookupSource_HandsAccessTimeToTheBackgroundWriter(t *testing.T) {
 	`
 	anHourAgoInDatabase := anHourAgo.UTC().Format(sqliteTimestampFormat)
 
-	found, err := cache.LookupSource(t.Context(), &imagerequest.Request{
+	reader, _, err := cache.Source(t.Context(),
-		Source: imagerequest.Source{Host: "example.com", Path: "/photo.jpg"},
+		imagerequest.Source{Host: "example.com", Path: "/photo.jpg"})
-	})
+	if err != nil {
-	if err != nil || found != key {
+		t.Fatalf("Source() error = %v; want the source of %q", err, key)
-		t.Fatalf("LookupSource() = %q, %v; want %q", found, err, key)
+	}
+
+	found, err := io.ReadAll(reader)
+	_ = reader.Close()
+
+	if err != nil || string(found) != "source bytes" {
+		t.Fatalf("Source() reads %q, %v; want the source of %q", found, err, key)
 	}
 
 	if countRows(t, cache, touchedQuery, string(key), anHourAgoInDatabase) != 0 {
-		t.Error("LookupSource wrote the source's access time itself")
+		t.Error("Source wrote the source's access time itself")
 	}
 
 	if waiting := len(cache.pendingSourceAccessTimes); waiting != 1 {

internal/imgcache/source_content_internal_test.go

@@ -10,11 +10,11 @@ import (
 	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
-// TestStoreSource_StoresIdenticalBytesOfTwoURLsTwice stores the same bytes
+// TestPutSource_StoresIdenticalBytesOfTwoURLsTwice stores the same bytes
 // for two source URLs and checks that each URL has a source_content row of
 // its own and a file of its own, named by its source key under
 // cache/sources/<ab>/<cd>/.
-func TestStoreSource_StoresIdenticalBytesOfTwoURLsTwice(t *testing.T) {
+func TestPutSource_StoresIdenticalBytesOfTwoURLsTwice(t *testing.T) {
 	t.Parallel()
 
 	cache, stateDir := newEvictionTestCache(t, 1<<30)
@@ -84,18 +84,12 @@ func TestEvictSourceBlob_LeavesSourceWithIdenticalBytesServed(t *testing.T) {
 		t.Errorf("the evicted source still has %d source_content rows", n)
 	}
 
-	kept, err := cache.LookupSource(t.Context(), &imagerequest.Request{
+	reader, _, err := cache.Source(t.Context(),
-		Source: imagerequest.Source{Host: testHostCDN, Path: "/kept.jpg"},
+		imagerequest.Source{Host: testHostCDN, Path: "/kept.jpg"})
-	})
+	t.Logf("Source() of the kept source: error %v", err)
-	t.Logf("LookupSource() of the kept source = %q, error %v", kept, err)
 
-	if err != nil || kept == "" {
-		t.Fatalf("LookupSource() of the kept source = %q, %v; want it cached", kept, err)
-	}
-
-	reader, _, err := cache.GetSourceContent(kept)
 	if err != nil {
-		t.Fatalf("GetSourceContent() of the kept source error = %v", err)
+		t.Fatalf("Source() of the kept source error = %v; want it cached", err)
 	}
 
 	defer func() { _ = reader.Close() }()
@@ -111,10 +105,10 @@ func TestEvictSourceBlob_LeavesSourceWithIdenticalBytesServed(t *testing.T) {
 	}
 }
 
-// TestStoreSource_FailedRowWriteLeavesNoFile drops source_content, so that
+// TestPutSource_FailedRowWriteLeavesNoFile drops source_content, so that
-// StoreSource cannot write the source's row, and checks that the store fails
+// PutSource cannot write the source's row, and checks that the store fails
 // and leaves no file under the source's key.
-func TestStoreSource_FailedRowWriteLeavesNoFile(t *testing.T) {
+func TestPutSource_FailedRowWriteLeavesNoFile(t *testing.T) {
 	t.Parallel()
 
 	cache, _ := newEvictionTestCache(t, 1<<30)
@@ -127,12 +121,12 @@ func TestStoreSource_FailedRowWriteLeavesNoFile(t *testing.T) {
 	req := &imagerequest.Request{
 		Source: imagerequest.Source{Host: testHostCDN, Path: testPathCat},
 	}
-	key, err := cache.StoreSource(t.Context(), req,
+	key, err := cache.PutSource(t.Context(), req.Source,
 		bytes.NewReader([]byte("source bytes")))
-	t.Logf("StoreSource() = %q, error = %v", key, err)
+	t.Logf("PutSource() = %q, error = %v", key, err)
 
 	if err == nil {
-		t.Error("StoreSource() error = nil, want the failed row write")
+		t.Error("PutSource() error = nil, want the failed row write")
 	}
 
 	if cache.srcContent.Exists(req.Source.Key()) {

internal/imgcache/testutil_internal_test.go

@@ -2,22 +2,10 @@ package imgcache
 
 import (
 	"bytes"
-	"context"
-	"database/sql"
 	"image"
 	"image/color"
-	"image/gif"
 	"image/jpeg"
-	"image/png"
-	"io/fs"
 	"testing"
-	"testing/fstest"
-	"time"
-
-	"github.com/prometheus/client_golang/prometheus"
-	"sneak.berlin/go/pixa/internal/database"
-	"sneak.berlin/go/pixa/internal/httpfetcher"
-	"sneak.berlin/go/pixa/internal/imageprocessor"
 )
 
 // Shared test data literals, extracted as constants for goconst.
@@ -25,75 +13,8 @@ const (
 	testHostCDN     = "cdn.example.com"
 	testHostExample = "example.com"
 	testPathCat     = "/photos/cat.jpg"
-	testContentTypeJPEG = "image/jpeg"
 )
 
-// TestFixtures contains paths to test files in the mock filesystem.
-type TestFixtures struct {
-	// Valid image files
-	GoodHostJPEG  string // valid JPEG on GoodHost
-	GoodHostPNG   string // valid PNG on GoodHost
-	GoodHostGIF   string // valid GIF on GoodHost
-	OtherHostJPEG string // valid JPEG on OtherHost
-	OtherHostPNG  string // valid PNG on OtherHost
-
-	// Invalid/edge case files
-	InvalidFile string // file with wrong magic bytes
-	EmptyFile   string // zero-byte file
-	TextFile    string // text file masquerading as image
-
-	// Hostnames
-	GoodHost  string // a host the mock fetcher serves
-	OtherHost string // a second host the mock fetcher serves
-}
-
-// DefaultFixtures returns the standard test fixture paths.
-func DefaultFixtures() *TestFixtures {
-	return &TestFixtures{
-		GoodHostJPEG:  "goodhost.example.com/images/photo.jpg",
-		GoodHostPNG:   "goodhost.example.com/images/logo.png",
-		GoodHostGIF:   "goodhost.example.com/images/animation.gif",
-		OtherHostJPEG: "otherhost.example.com/uploads/image.jpg",
-		OtherHostPNG:  "otherhost.example.com/uploads/icon.png",
-		InvalidFile:   "goodhost.example.com/images/fake.jpg",
-		EmptyFile:     "goodhost.example.com/images/empty.jpg",
-		TextFile:      "goodhost.example.com/images/text.png",
-		GoodHost:      "goodhost.example.com",
-		OtherHost:     "otherhost.example.com",
-	}
-}
-
-// NewTestFS creates a mock filesystem with test images.
-func NewTestFS(t *testing.T) (fs.FS, *TestFixtures) {
-	t.Helper()
-
-	fixtures := DefaultFixtures()
-
-	// Generate test images
-	jpegData := generateTestJPEG(t, 100, 100, color.RGBA{255, 0, 0, 255})
-	pngData := generateTestPNG(t, 100, 100, color.RGBA{0, 255, 0, 255})
-	gifData := generateTestGIF(t, 100, 100, color.RGBA{0, 0, 255, 255})
-
-	// Create the mock filesystem
-	mockFS := fstest.MapFS{
-		// Good host files
-		fixtures.GoodHostJPEG: &fstest.MapFile{Data: jpegData},
-		fixtures.GoodHostPNG:  &fstest.MapFile{Data: pngData},
-		fixtures.GoodHostGIF:  &fstest.MapFile{Data: gifData},
-
-		// Other host files
-		fixtures.OtherHostJPEG: &fstest.MapFile{Data: jpegData},
-		fixtures.OtherHostPNG:  &fstest.MapFile{Data: pngData},
-
-		// Invalid files
-		fixtures.InvalidFile: &fstest.MapFile{Data: []byte("not a real image file content")},
-		fixtures.EmptyFile:   &fstest.MapFile{Data: []byte{}},
-		fixtures.TextFile:    &fstest.MapFile{Data: []byte("This is just text, not a PNG")},
-	}
-
-	return mockFS, fixtures
-}
-
 // generateTestJPEG creates a minimal valid JPEG image.
 func generateTestJPEG(t *testing.T, width, height int, c color.Color) []byte {
 	t.Helper()
@@ -114,104 +35,3 @@ func generateTestJPEG(t *testing.T, width, height int, c color.Color) []byte {
 
 	return buf.Bytes()
 }
-
-// generateTestPNG creates a minimal valid PNG image.
-func generateTestPNG(t *testing.T, width, height int, c color.Color) []byte {
-	t.Helper()
-
-	img := image.NewRGBA(image.Rect(0, 0, width, height))
-	for y := range height {
-		for x := range width {
-			img.Set(x, y, c)
-		}
-	}
-
-	var buf bytes.Buffer
-
-	err := png.Encode(&buf, img)
-	if err != nil {
-		t.Fatalf("failed to encode test PNG: %v", err)
-	}
-
-	return buf.Bytes()
-}
-
-// generateTestGIF creates a minimal valid GIF image.
-func generateTestGIF(t *testing.T, width, height int, c color.Color) []byte {
-	t.Helper()
-
-	img := image.NewPaletted(
-		image.Rect(0, 0, width, height),
-		[]color.Color{c, color.White},
-	)
-	for y := range height {
-		for x := range width {
-			img.SetColorIndex(x, y, 0)
-		}
-	}
-
-	var buf bytes.Buffer
-
-	err := gif.Encode(&buf, img, nil)
-	if err != nil {
-		t.Fatalf("failed to encode test GIF: %v", err)
-	}
-
-	return buf.Bytes()
-}
-
-// SetupTestService creates a Service with mock fetcher for testing.
-func SetupTestService(t *testing.T) (*Service, *TestFixtures) {
-	t.Helper()
-
-	mockFS, fixtures := NewTestFS(t)
-
-	// Create temp directory for cache
-	tmpDir := t.TempDir()
-
-	// Create in-memory database
-	db := setupServiceTestDB(t)
-
-	cache, err := NewCache(db, CacheConfig{
-		StateDir:    tmpDir,
-		NegativeTTL: 5 * time.Minute,
-		MaxBytes:    1 << 30,
-	}, prometheus.NewRegistry())
-	if err != nil {
-		t.Fatalf("failed to create cache: %v", err)
-	}
-
-	svc, err := NewService(&ServiceConfig{
-		Cache:   cache,
-		Fetcher: httpfetcher.NewMock(mockFS),
-		Processor: imageprocessor.New(imageprocessor.Params{
-			MaxInputBytes:           50 << 20,
-			MaxConcurrentProcessing: 2,
-		}),
-		MaxResponseSize: 50 << 20,
-	})
-	if err != nil {
-		t.Fatalf("failed to create service: %v", err)
-	}
-
-	return svc, fixtures
-}
-
-// setupServiceTestDB creates an in-memory SQLite database for testing
-// using the production schema.
-func setupServiceTestDB(t *testing.T) *sql.DB {
-	t.Helper()
-
-	db, err := sql.Open("sqlite", ":memory:")
-	if err != nil {
-		t.Fatalf("failed to open test db: %v", err)
-	}
-
-	// Use the real production schema via migrations
-	err = database.ApplyMigrations(context.Background(), db, nil)
-	if err != nil {
-		t.Fatalf("failed to apply migrations: %v", err)
-	}
-
-	return db
-}

internal/providers/providers.go

@@ -26,6 +26,7 @@ import (
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imageproxy"
 	"sneak.berlin/go/pixa/internal/imgcache"
 	"sneak.berlin/go/pixa/internal/logger"
 	"sneak.berlin/go/pixa/internal/seal"
@@ -34,7 +35,7 @@ import (
 
 // Constructors returns the constructors of the parts this package builds,
 // for fx.Provide. The fx graph must also provide the prometheus.Registerer
-// the cache registers its counters on.
+// the cache and the image service register their counters on.
 func Constructors() []any {
 	return []any{
 		newKeys,
@@ -79,7 +80,7 @@ func newCache(lc fx.Lifecycle, params cacheParams) (*imgcache.Cache, error) {
 		return nil, err
 	}
 
-	cache, err := imgcache.NewCache(db, imgcache.CacheConfig{
+	cache, err := imgcache.NewCache(db, imgcache.Config{
 		StateDir:    cfg.StateDir,
 		NegativeTTL: imgcache.DefaultNegativeTTL,
 		MaxBytes:    maxBytes,
@@ -218,20 +219,21 @@ type imageServiceParams struct {
 	Fetcher         *httpfetcher.HTTPFetcher
 	Processor       *imageprocessor.ImageProcessor
 	Logger          *logger.Logger
+	MetricsRegistry prometheus.Registerer
 }
 
 // newImageService builds the image service from the cache, the fetcher
 // and the processor, with the largest upstream response the config allows.
 func newImageService(
 	_ fx.Lifecycle, params imageServiceParams,
-) (*imgcache.Service, error) {
+) (*imageproxy.Proxy, error) {
-	return imgcache.NewService(&imgcache.ServiceConfig{
+	return imageproxy.New(&imageproxy.Config{
 		Cache:           params.Cache,
 		Fetcher:         params.Fetcher,
 		Processor:       params.Processor,
 		MaxResponseSize: params.Config.UpstreamMaxResponseSize,
 		Logger:          params.Logger.Get(),
-	})
+	}, params.MetricsRegistry)
 }
 
 // sessionManagerParams defines dependencies for newSessionManager.

internal/server/source_url_internal_test.go

@@ -14,8 +14,8 @@ import (
 	"time"
 
 	"sneak.berlin/go/pixa/internal/encurl"
+	"sneak.berlin/go/pixa/internal/imageproxy"
 	"sneak.berlin/go/pixa/internal/imagerequest"
-	"sneak.berlin/go/pixa/internal/imgcache"
 	"sneak.berlin/go/pixa/internal/seal"
 	"sneak.berlin/go/pixa/internal/session"
 	"sneak.berlin/go/pixa/internal/signature"
@@ -227,7 +227,7 @@ func TestSourcePathsHoldingSpaceAndPercent20AreFetchedApart(t *testing.T) {
 
 	for _, imageURL := range imageURLs {
 		cacheStatus := getImage(t, s, imageURL).Header().Get("X-Pixa-Cache")
-		if cacheStatus != string(imgcache.CacheHit) {
+		if cacheStatus != string(imageproxy.CacheHit) {
 			t.Errorf("second GET %s: X-Pixa-Cache = %q, want HIT",
 				imageURL, cacheStatus)
 		}
@@ -256,7 +256,7 @@ func TestSourceQueryWrittenTwoWaysIsOneUpstreamRequest(t *testing.T) {
 
 	second := getImage(t, s,
 		"/v1/image/"+upstreamHost+"/photo.png%3Fname=my%20cat/orig.png")
-	if got := second.Header().Get("X-Pixa-Cache"); got != string(imgcache.CacheHit) {
+	if got := second.Header().Get("X-Pixa-Cache"); got != string(imageproxy.CacheHit) {
 		t.Errorf("second X-Pixa-Cache = %q, want HIT", got)
 	}
 
@@ -365,7 +365,7 @@ func TestGeneratorMakesOneImageURLForTwoSpellingsOfASourceURL(t *testing.T) {
 			getImage(t, s, imageURLs[0])
 
 			second := getImage(t, s, imageURLs[1])
-			if got := second.Header().Get("X-Pixa-Cache"); got != string(imgcache.CacheHit) {
+			if got := second.Header().Get("X-Pixa-Cache"); got != string(imageproxy.CacheHit) {
 				t.Errorf("second X-Pixa-Cache = %q, want HIT", got)
 			}