Test that a read during a store keeps the stored content type (closes #70)

A GetVariant that begins before StoreVariant finishes can find the
variant file but not yet its .meta file, and so reads
application/octet-stream. When it adds that to memory after the store
added the real type, the wrong type is served to every later hit. The
part of GetVariant that runs after its check of memory moves, unchanged,
into loadVariantWithMeta, so the test can run it after a store. The
test fails now.

Model: opus-5-5

README.md

@@ -86,7 +86,10 @@ prevent abuse, and allowlisted source hosts for open access.
 
 Multiple source paths may reference the same content blob; the
 database tracks references rather than using filesystem refcounting.
-In-process caching of request-to-output mappings targets 1-5k r/s.
+Toward a target of 1-5k r/s, pixa keeps in memory the content types of
+the 10,000 transformed images most recently cached or served, so a
+cache hit on one of them reads only the image file from disk and not
+the metadata file stored beside it.
 
 ### Routes
 

TODO.md

@@ -30,6 +30,15 @@ exhaustion
 
 # Completed Steps
 
+- 2026-09-29 variant content types kept in memory (closes #70):
+  `Cache.metaCache` holds the content types of up to 10,000 variants in an LRU
+  (`github.com/hashicorp/golang-lru/v2`), filled by `StoreVariant` and by
+  `GetVariant` after it reads a `.meta` file; for a variant it holds, `Lookup`
+  skips the check of the disk and `GetVariant` skips the `.meta` read, still
+  opening the variant file and taking the size from it; eviction removes the
+  entry before deleting the files, and `GetVariant` removes it when the file
+  will not open; the cap is a constant, not a setting; the unused `variantMeta`
+  type is gone; `README.md` describes it.
 - 2026-09-29 Dockerfiles install through `script/bootstrap` (closes #95): the
   `Dockerfile` lint and build stages and `Dockerfile.lint` copy `script/`,
   `go.mod` and `go.sum`, then run `script/bootstrap` in place of their own

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/go-chi/httprate v0.16.0
 	github.com/gorilla/csrf v1.7.3
 	github.com/gorilla/securecookie v1.1.2
+	github.com/hashicorp/golang-lru/v2 v2.0.7
 	github.com/prometheus/client_golang v1.23.2
 	github.com/slok/go-http-metrics v0.13.0
 	github.com/spf13/cobra v1.10.2

go.sum

@@ -228,6 +228,8 @@ github.com/hashicorp/go-version v1.2.1/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09
 github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
 github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc=
 github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4=
+github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k=
+github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM=
 github.com/hashicorp/hcl v1.0.1-vault-7 h1:ag5OxFVy3QYTFTJODRzTKVZ6xvdfLLCA1cy/Y6xGI0I=
 github.com/hashicorp/hcl v1.0.1-vault-7/go.mod h1:XYhtn6ijBSAj6n4YqAaf7RBPS4I06AItNorpy+MoQNM=
 github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64=

internal/imgcache/cache.go

@@ -14,6 +14,7 @@ import (
 	"sync"
 	"time"
 
+	lru "github.com/hashicorp/golang-lru/v2"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 )
 
@@ -26,6 +27,10 @@ var (
 // HTTP status code for successful fetch.
 const httpStatusOK = 200
 
+// metaCacheSize is how many variants' content types metaCache holds. A
+// variant not among them is served as before, reading its .meta file.
+const metaCacheSize = 10000
+
 // CacheConfig holds cache configuration.
 type CacheConfig struct {
 	StateDir    string
@@ -49,12 +54,6 @@ type CacheConfig struct {
 	Logger *slog.Logger
 }
 
-// variantMeta stores content type for fast cache hits without reading .meta file.
-type variantMeta struct {
-	ContentType string
-	Size        int64
-}
-
 // Cache implements the caching layer for the image proxy.
 type Cache struct {
 	db          *sql.DB
@@ -76,9 +75,10 @@ type Cache struct {
 	evictionStarted  bool
 	evictionStopOnce sync.Once
 
-	// In-memory cache of variant metadata (content type, size) to avoid
-	// reading .meta files
-	metaCache map[VariantKey]variantMeta
+	// metaCache holds the content types of the variants most recently
+	// stored or served, so a hit does not read the variant's .meta file.
+	// It never stands in for the variant file, which is always opened.
+	metaCache *lru.Cache[VariantKey, string]
 
 	// contentLocks serializes StoreSource and evictSourceBlob per
 	// content hash, closing the race window between an eviction's row
@@ -101,6 +101,11 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
 		log = slog.Default()
 	}
 
+	metaCache, err := lru.New[VariantKey, string](metaCacheSize)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create variant content type cache: %w", err)
+	}
+
 	c := &Cache{
 		db:               db,
 		config:           config,
@@ -109,7 +114,7 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
 		evictionPressure: make(chan struct{}, 1),
 		evictionStop:     make(chan struct{}),
 		evictionDone:     make(chan struct{}),
-		metaCache:        make(map[VariantKey]variantMeta),
+		metaCache:        metaCache,
 		contentLocks:     newContentLock(),
 	}
 
@@ -154,13 +159,15 @@ type LookupResult struct {
 	CacheStatus CacheStatus
 }
 
-// Lookup checks if a processed variant exists on disk. Hits touch the
-// variant's LRU timestamp; a disabled cache always misses.
+// Lookup checks if a processed variant exists on disk: a variant held
+// in metaCache counts as present without a check of the disk. Hits
+// touch the variant's LRU timestamp; a disabled cache always misses.
 func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, error) {
 	cacheKey := CacheKey(req)
 
-	// Check variant storage directly - no DB needed for cache hits
-	if !c.disabled && c.variants.Exists(cacheKey) {
+	// Check memory, then variant storage - no DB needed for cache hits
+	if !c.disabled &&
+		(c.metaCache.Contains(cacheKey) || c.variants.Exists(cacheKey)) {
 		c.touchVariant(ctx, cacheKey)
 
 		return &LookupResult{
@@ -177,13 +184,28 @@ func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, e
 	}, nil
 }
 
-// GetVariant returns a reader, size, and content type for a cached variant.
+// GetVariant returns a reader, size, and content type for a cached
+// variant. The content type comes from metaCache, or else from the
+// variant's .meta file and is then kept in metaCache.
 func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, error) {
 	if c.disabled {
 		return nil, 0, "", ErrNotFound
 	}
 
-	return c.variants.LoadWithMeta(cacheKey)
+	contentType, known := c.metaCache.Get(cacheKey)
+	if !known {
+		return c.loadVariantWithMeta(cacheKey)
+	}
+
+	reader, size, err := c.variants.LoadWithSize(cacheKey)
+	if err != nil {
+		// The file is gone, e.g. deleted outside pixa
+		c.metaCache.Remove(cacheKey)
+
+		return nil, 0, "", err
+	}
+
+	return reader, size, contentType, nil
 }
 
 // StoreSource stores fetched source content and metadata. On a
@@ -286,6 +308,8 @@ func (c *Cache) StoreVariant(
 		return err
 	}
 
+	c.metaCache.Add(cacheKey, contentType)
+
 	_, err = c.db.ExecContext(ctx, `
 		INSERT INTO variant_content (cache_key, size_bytes, content_type)
 		VALUES (?, ?, ?)
@@ -496,6 +520,22 @@ func (c *Cache) IncrementTransformCount(ctx context.Context) {
 	}
 }
 
+// loadVariantWithMeta is GetVariant for a variant metaCache does not
+// hold: it reads the content type from the variant's .meta file and
+// keeps it in metaCache.
+func (c *Cache) loadVariantWithMeta(
+	cacheKey VariantKey,
+) (io.ReadCloser, int64, string, error) {
+	reader, size, contentType, err := c.variants.LoadWithMeta(cacheKey)
+	if err != nil {
+		return nil, 0, "", err
+	}
+
+	c.metaCache.Add(cacheKey, contentType)
+
+	return reader, size, contentType, nil
+}
+
 // writeMetadataSidecar writes the JSON metadata sidecar of a stored source.
 // A failure is logged and is otherwise non-fatal; the metadata is in the
 // database.

internal/imgcache/eviction.go

@@ -271,7 +271,9 @@ func (c *Cache) sourceCandidates(ctx context.Context) ([]evictionCandidate, erro
 
 // evictVariant removes one variant: accounting row first, then the
 // content and .meta files, so the database never references a deleted
-// file.
+// file. The metaCache entry goes before the files; a GetVariant that
+// read them just before may put it back, and the next GetVariant then
+// fails to open the file and removes it again.
 func (c *Cache) evictVariant(ctx context.Context, cacheKey VariantKey) error {
 	_, err := c.db.ExecContext(ctx,
 		`DELETE FROM variant_content WHERE cache_key = ?`, string(cacheKey))
@@ -279,6 +281,8 @@ func (c *Cache) evictVariant(ctx context.Context, cacheKey VariantKey) error {
 		return fmt.Errorf("failed to delete variant accounting row: %w", err)
 	}
 
+	c.metaCache.Remove(cacheKey)
+
 	err = c.variants.DeleteWithMeta(cacheKey)
 	if err != nil {
 		return err

internal/imgcache/metacache_internal_test.go

@@ -0,0 +1,253 @@
+package imgcache
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"sync"
+	"testing"
+	"time"
+)
+
+// webpRequest returns a request for a 100x100 WebP variant of path.
+func webpRequest(path string) *ImageRequest {
+	return &ImageRequest{
+		SourceHost: testHostCDN,
+		SourcePath: path,
+		Size:       Size{Width: 100, Height: 100},
+		Format:     FormatWebP,
+		Quality:    85,
+		FitMode:    FitCover,
+	}
+}
+
+// assertVariantServed checks that GetVariant serves key with the given
+// content and the image/webp content type storeEvictionTestVariant stores.
+func assertVariantServed(t *testing.T, cache *Cache, key VariantKey, content []byte) {
+	t.Helper()
+
+	reader, size, contentType, err := cache.GetVariant(key)
+	if err != nil {
+		t.Fatalf("GetVariant(%s) error = %v", key, err)
+	}
+
+	defer func() { _ = reader.Close() }()
+
+	got, err := io.ReadAll(reader)
+	if err != nil {
+		t.Fatalf("reading variant %s: %v", key, err)
+	}
+
+	if !bytes.Equal(got, content) {
+		t.Errorf("GetVariant(%s) content = %q, want %q", key, got, content)
+	}
+
+	if size != int64(len(content)) {
+		t.Errorf("GetVariant(%s) size = %d, want %d", key, size, len(content))
+	}
+
+	if contentType != testContentTypeWebP {
+		t.Errorf("GetVariant(%s) content type = %q, want %q",
+			key, contentType, testContentTypeWebP)
+	}
+}
+
+// assertVariantNotFound checks that GetVariant refuses key with
+// ErrNotFound.
+func assertVariantNotFound(t *testing.T, cache *Cache, key VariantKey) {
+	t.Helper()
+
+	reader, _, _, err := cache.GetVariant(key)
+	if err == nil {
+		_ = reader.Close()
+	}
+
+	if !errors.Is(err, ErrNotFound) {
+		t.Errorf("GetVariant(%s) error = %v, want ErrNotFound", key, err)
+	}
+}
+
+// assertLookupMisses checks that Lookup reports request as a miss.
+func assertLookupMisses(t *testing.T, cache *Cache, request *ImageRequest) {
+	t.Helper()
+
+	lookup, err := cache.Lookup(t.Context(), request)
+	if err != nil {
+		t.Fatalf("Lookup(%s) error = %v", request.SourcePath, err)
+	}
+
+	if lookup.Hit {
+		t.Errorf("Lookup(%s) is a hit, want a miss", request.SourcePath)
+	}
+}
+
+// TestSecondHitDoesNotReadMetaFile checks that once a variant has been
+// stored or read, a hit takes its content type from memory: with the
+// .meta file deleted, GetVariant must still return the stored content
+// type rather than the application/octet-stream it uses without one.
+func TestSecondHitDoesNotReadMetaFile(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<20)
+	content := []byte("webp variant bytes")
+
+	storeEvictionTestVariant(t, cache, testVariantKeyOne, content)
+
+	// A second Cache on the same state directory starts with nothing in
+	// memory, as pixad does after a restart, so its first read uses the
+	// .meta file.
+	restarted, err := NewCache(cache.db, cache.config)
+	if err != nil {
+		t.Fatalf("NewCache() error = %v", err)
+	}
+
+	assertVariantServed(t, restarted, testVariantKeyOne, content)
+
+	err = os.Remove(cache.variants.keyToPath(testVariantKeyOne) + ".meta")
+	if err != nil {
+		t.Fatalf("removing .meta file: %v", err)
+	}
+
+	assertVariantServed(t, cache, testVariantKeyOne, content)
+	assertVariantServed(t, restarted, testVariantKeyOne, content)
+}
+
+// TestReadDuringStoreKeepsStoredContentType checks that a GetVariant
+// which began before StoreVariant finished cannot replace the content
+// type the store kept in memory. Such a read can find the variant file
+// but not yet its .meta file, and so gets application/octet-stream. The
+// test deletes the .meta file after the store, then runs the part of
+// GetVariant that comes after its check of memory.
+func TestReadDuringStoreKeepsStoredContentType(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<20)
+	content := []byte("webp variant bytes")
+
+	storeEvictionTestVariant(t, cache, testVariantKeyOne, content)
+
+	err := os.Remove(cache.variants.keyToPath(testVariantKeyOne) + ".meta")
+	if err != nil {
+		t.Fatalf("removing .meta file: %v", err)
+	}
+
+	reader, _, contentType, err := cache.loadVariantWithMeta(testVariantKeyOne)
+	if err != nil {
+		t.Fatalf("loadVariantWithMeta(%s) error = %v", testVariantKeyOne, err)
+	}
+
+	_ = reader.Close()
+
+	t.Logf("the read without a .meta file got content type %q", contentType)
+
+	assertVariantServed(t, cache, testVariantKeyOne, content)
+}
+
+// TestEvictedVariantIsNotServed checks that a variant the evictor
+// removed is a miss and cannot be read, although it had been stored
+// and served before.
+func TestEvictedVariantIsNotServed(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1500)
+
+	oldRequest := webpRequest("/old.jpg")
+	newRequest := webpRequest("/new.jpg")
+	oldKey := CacheKey(oldRequest)
+	newKey := CacheKey(newRequest)
+	oldContent := bytes.Repeat([]byte{0x01}, 1000)
+	newContent := bytes.Repeat([]byte{0x02}, 1000)
+
+	storeEvictionTestVariant(t, cache, oldKey, oldContent)
+	storeEvictionTestVariant(t, cache, newKey, newContent)
+	assertVariantServed(t, cache, oldKey, oldContent)
+	setVariantLastAccessed(t, cache, oldKey, time.Now().Add(-time.Hour))
+
+	err := cache.EvictToLimit(t.Context())
+	if err != nil {
+		t.Fatalf("EvictToLimit() error = %v", err)
+	}
+
+	assertLookupMisses(t, cache, oldRequest)
+	assertVariantNotFound(t, cache, oldKey)
+	assertVariantServed(t, cache, newKey, newContent)
+}
+
+// TestVariantDeletedFromDiskIsNotServed checks that a variant whose
+// file was deleted by something other than the evictor cannot be read,
+// and is a miss afterwards.
+func TestVariantDeletedFromDiskIsNotServed(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<20)
+
+	request := webpRequest("/deleted.jpg")
+	key := CacheKey(request)
+	content := []byte("webp variant bytes")
+
+	storeEvictionTestVariant(t, cache, key, content)
+	assertVariantServed(t, cache, key, content)
+
+	err := os.Remove(cache.variants.keyToPath(key))
+	if err != nil {
+		t.Fatalf("removing variant file: %v", err)
+	}
+
+	assertVariantNotFound(t, cache, key)
+	assertLookupMisses(t, cache, request)
+}
+
+// TestConcurrentVariantStoreReadAndEvict stores, reads and evicts
+// variants from several goroutines at once, for the race detector.
+func TestConcurrentVariantStoreReadAndEvict(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<20)
+	ctx := t.Context()
+
+	var wg sync.WaitGroup
+
+	for goroutine := range 8 {
+		wg.Go(func() {
+			key := VariantKey(fmt.Sprintf("aabbccdd01%02d", goroutine))
+			content := []byte(key)
+
+			for range 20 {
+				err := cache.StoreVariant(
+					ctx, key, bytes.NewReader(content), testContentTypeWebP)
+				if err != nil {
+					t.Errorf("StoreVariant(%s) error = %v", key, err)
+
+					return
+				}
+
+				reader, _, contentType, err := cache.GetVariant(key)
+				if err != nil {
+					t.Errorf("GetVariant(%s) error = %v", key, err)
+
+					return
+				}
+
+				_ = reader.Close()
+
+				if contentType != testContentTypeWebP {
+					t.Errorf("GetVariant(%s) content type = %q, want %q",
+						key, contentType, testContentTypeWebP)
+				}
+
+				err = cache.evictVariant(ctx, key)
+				if err != nil {
+					t.Errorf("evictVariant(%s) error = %v", key, err)
+
+					return
+				}
+
+				assertVariantNotFound(t, cache, key)
+			}
+		})
+	}
+
+	wg.Wait()
+}

internal/imgcache/storage.go

@@ -506,32 +506,43 @@ func (s *VariantStorage) Load(key VariantKey) (io.ReadCloser, error) {
 	return f, nil
 }
 
-// LoadWithMeta returns a reader, size, and content type for the content at
-// the given key.
-func (s *VariantStorage) LoadWithMeta(
-	key VariantKey,
-) (io.ReadCloser, int64, string, error) {
+// LoadWithSize returns a reader and file size for the content at the
+// given key.
+func (s *VariantStorage) LoadWithSize(key VariantKey) (io.ReadCloser, int64, error) {
 	path := s.keyToPath(key)
-	metaPath := path + ".meta"
 
 	f, err := os.Open(path) //nolint:gosec // path derived from cache key
 	if err != nil {
 		if os.IsNotExist(err) {
-			return nil, 0, "", ErrNotFound
+			return nil, 0, ErrNotFound
 		}
 
-		return nil, 0, "", fmt.Errorf("failed to open content: %w", err)
+		return nil, 0, fmt.Errorf("failed to open content: %w", err)
 	}
 
 	stat, err := f.Stat()
 	if err != nil {
 		_ = f.Close()
 
-		return nil, 0, "", fmt.Errorf("failed to stat content: %w", err)
+		return nil, 0, fmt.Errorf("failed to stat content: %w", err)
+	}
+
+	return f, stat.Size(), nil
+}
+
+// LoadWithMeta returns a reader, size, and content type for the content at
+// the given key.
+func (s *VariantStorage) LoadWithMeta(
+	key VariantKey,
+) (io.ReadCloser, int64, string, error) {
+	f, size, err := s.LoadWithSize(key)
+	if err != nil {
+		return nil, 0, "", err
 	}
 
 	// Load metadata for content type
 	contentType := "application/octet-stream" // fallback
+	metaPath := s.keyToPath(key) + ".meta"
 
 	metaData, err := os.ReadFile(metaPath) //nolint:gosec // path derived from cache key
 	if err == nil {
@@ -541,7 +552,7 @@ func (s *VariantStorage) LoadWithMeta(
 		}
 	}
 
-	return f, stat.Size(), contentType, nil
+	return f, size, contentType, nil
 }
 
 // Exists checks if content exists at the given key.

internal/imgcache/testutil_internal_test.go

@@ -24,6 +24,7 @@ const (
 	testHostExample       = "example.com"
 	testPathCat           = "/photos/cat.jpg"
 	testContentTypeJPEG   = "image/jpeg"
+	testContentTypeWebP   = "image/webp"
 	testHeaderContentType = "Content-Type"
 )