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
 }
 
@@ -530,6 +523,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.