From fc87c2117d04af51736423bc2b1a92afe3ce24f8 Mon Sep 17 00:00:00 2001
From: clawbot <35+clawbot@noreply.example.org>
Date: Tue, 29 Sep 2026 07:22:21 +0000
Subject: [PATCH] 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
---
 internal/imgcache/cache.go                   | 35 ++++++++++++--------
 internal/imgcache/metacache_internal_test.go | 31 +++++++++++++++++
 2 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go
index ef4cacf..02a8edd 100644
--- a/internal/imgcache/cache.go
+++ b/internal/imgcache/cache.go
@@ -193,25 +193,18 @@ func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, e
 	}
 
 	contentType, known := c.metaCache.Get(cacheKey)
-	if known {
-		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
+	if !known {
+		return c.loadVariantWithMeta(cacheKey)
 	}
 
-	reader, size, contentType, err := c.variants.LoadWithMeta(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
 	}
 
-	c.metaCache.Add(cacheKey, contentType)
-
 	return reader, size, contentType, nil
 }
 
@@ -527,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.
diff --git a/internal/imgcache/metacache_internal_test.go b/internal/imgcache/metacache_internal_test.go
index 0a2d368..c327249 100644
--- a/internal/imgcache/metacache_internal_test.go
+++ b/internal/imgcache/metacache_internal_test.go
@@ -114,6 +114,37 @@ func TestSecondHitDoesNotReadMetaFile(t *testing.T) {
 	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.