Split internal/imgcache into the image proxy and the cache (closes #317)

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

Model: opus-5-5

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
-// format as a variant and checks that GetVariant serves it whole with the
+// TestVariant_ContentTypeFromFirstBytes stores an image in each output
+// 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)
-		t.Logf("%s: GetVariant() error = %v", tt.name, err)
+		reader, _, contentType, err := cache.Variant(key)
+		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