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

@@ -193,7 +193,10 @@ func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, e
 	}
 
 	contentType, known := c.metaCache.Get(cacheKey)
-	if known {
+	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
@@ -205,16 +208,6 @@ func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, e
 	return reader, size, contentType, nil
 }
 
-	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
-}
-
 // StoreSource stores fetched source content and metadata. On a
 // disabled cache it is a no-op returning an empty hash.
 func (c *Cache) StoreSource(
@@ -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.

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.