Test damaged cached files at startup and the .meta write (refs #252)

The tests of an empty or cut cached source or variant now run the
reconciliation pass, as pixa does at startup, before the next request,
and check that it logs a warning naming the file. New tests: the pass
removes an empty variant file without an accounting row instead of
adopting it, and VariantStorage.Store puts a new .meta file in place of
the old one rather than writing into it.

#252

Model: opus-5-5

internal/imgcache/partial_cache_files_internal_test.go

@@ -5,8 +5,10 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"io"
+	"log/slog"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
@@ -67,10 +69,45 @@ func cachedSourcePath(t *testing.T, svc *Service, req *ImageRequest) string {
 	return svc.cache.srcContent.hashToPath(contentHash)
 }
 
+// reconcileAsAtStartup runs the cache's reconciliation pass, as pixa does
+// when it starts, and returns what the pass logged.
+func reconcileAsAtStartup(t *testing.T, cache *Cache) string {
+	t.Helper()
+
+	var logBuf bytes.Buffer
+
+	cache.log = slog.New(slog.NewJSONHandler(&logBuf, nil))
+
+	err := cache.reconcileAccounting(t.Context())
+	if err != nil {
+		t.Fatalf("reconcileAccounting() error = %v", err)
+	}
+
+	t.Logf("reconciliation log: %s", logBuf.String())
+
+	return logBuf.String()
+}
+
+// assertWarningNames checks that logs hold a warning naming the file at
+// path.
+func assertWarningNames(t *testing.T, logs, path string) {
+	t.Helper()
+
+	for line := range strings.Lines(logs) {
+		if strings.Contains(line, `"level":"WARN"`) &&
+			strings.Contains(line, `"path":"`+path+`"`) {
+			return
+		}
+	}
+
+	t.Errorf("reconciliation logged no warning naming %s", path)
+}
+
 // TestService_Get_FetchesAgainForEmptyCachedSource empties the cached source
-// file, as a crash during its write could, then asks for two new sizes of
+// file, as a crash or a restored snapshot could, and runs the reconciliation
-// it: the first must fetch the source again and cache it whole, so that the
+// pass, which must remove it with a warning naming it. It then asks for two
-// second is made from the cache without a third fetch.
+// new sizes of the source: the first must fetch it again and cache it whole,
+// so that the second is made from the cache without a third fetch.
 func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
 	t.Parallel()
 
@@ -82,6 +119,7 @@ func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
 	wholeSize := fileSize(t, path)
 
 	truncateFile(t, path, 0)
+	assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path)
 
 	getImage(t, svc, photoVariant(fixtures, 40, FitCover))
 	getImage(t, svc, photoVariant(fixtures, 30, FitCover))
@@ -101,9 +139,11 @@ func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
 }
 
 // TestService_Get_FetchesAgainForPartialCachedSource cuts the cached source
-// file to half its size, as a crash during its write could, then asks for
+// file to half its size, as a crash or a restored snapshot could, and runs
-// two new sizes of it: the first must fetch the source again rather than
+// the reconciliation pass, which must remove it with a warning naming it.
-// fail to decode the partial file, and the second is made from the cache.
+// It then asks for two new sizes of the source: the first must fetch it
+// again rather than fail to decode the partial file, and the second is
+// made from the cache.
 func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) {
 	t.Parallel()
 
@@ -115,6 +155,7 @@ func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) {
 	wholeSize := fileSize(t, path)
 
 	truncateFile(t, path, wholeSize/2)
+	assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path)
 
 	getImage(t, svc, photoVariant(fixtures, 40, FitCover))
 	getImage(t, svc, photoVariant(fixtures, 30, FitCover))
@@ -132,10 +173,11 @@ func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) {
 	}
 }
 
-// assertConvertedAgain caches a variant of the photo and cuts its file to
+// assertConvertedAgain caches a variant of the photo, cuts its file to size
-// size bytes, then checks that Get converts the image again instead of
+// bytes and runs the reconciliation pass, which must remove it with a
-// serving the file, and that the next Get serves the whole variant from
+// warning naming it. It then checks that Get converts the image again
-// the cache.
+// instead of serving the file, and that the next Get serves the whole
+// variant from the cache.
 func assertConvertedAgain(t *testing.T, size int64) {
 	t.Helper()
 
@@ -144,7 +186,10 @@ func assertConvertedAgain(t *testing.T, size int64) {
 
 	want, _ := getImage(t, svc, req)
 
-	truncateFile(t, svc.cache.variants.keyToPath(CacheKey(req)), size)
+	path := svc.cache.variants.keyToPath(CacheKey(req))
+
+	truncateFile(t, path, size)
+	assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path)
 
 	got, status := getImage(t, svc, req)
 	t.Logf("after the cut: %d bytes, %s", len(got), status)
@@ -164,8 +209,8 @@ func assertConvertedAgain(t *testing.T, size int64) {
 }
 
 // TestService_Get_ConvertsAgainForEmptyCachedVariant empties a cached
-// variant file, as a crash during its write could, and checks that it is
+// variant file, as a crash or a restored snapshot could, and checks that it
-// converted again rather than served empty.
+// is converted again rather than served empty.
 func TestService_Get_ConvertsAgainForEmptyCachedVariant(t *testing.T) {
 	t.Parallel()
 
@@ -173,14 +218,98 @@ func TestService_Get_ConvertsAgainForEmptyCachedVariant(t *testing.T) {
 }
 
 // TestService_Get_ConvertsAgainForPartialCachedVariant cuts a cached
-// variant file to ten bytes, as a crash during its write could, and checks
+// variant file to ten bytes, as a crash or a restored snapshot could, and
-// that it is converted again rather than served cut short.
+// checks that it is converted again rather than served cut short.
 func TestService_Get_ConvertsAgainForPartialCachedVariant(t *testing.T) {
 	t.Parallel()
 
 	assertConvertedAgain(t, 10)
 }
 
+// TestReconciliationRemovesEmptyVariantFileWithoutRow checks that the
+// reconciliation pass removes an empty variant file that has no
+// accounting row, with a warning naming it, instead of adding a row for
+// it that would have it served empty.
+func TestReconciliationRemovesEmptyVariantFileWithoutRow(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+
+	_, err := cache.variants.Store(testVariantKeyOne, bytes.NewReader(nil),
+		testContentTypeWebP)
+	if err != nil {
+		t.Fatalf("failed to store an empty variant file: %v", err)
+	}
+
+	logs := reconcileAsAtStartup(t, cache)
+
+	assertWarningNames(t, logs, cache.variants.keyToPath(testVariantKeyOne))
+
+	if cache.variants.Exists(testVariantKeyOne) {
+		t.Error("the empty variant file is still on disk")
+	}
+
+	rows := countRows(t, cache,
+		`SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`,
+		string(testVariantKeyOne))
+	if rows != 0 {
+		t.Errorf("the empty variant file got %d accounting rows, want 0", rows)
+	}
+}
+
+// TestVariantStorage_StoreReplacesMetaFile stores a variant over one
+// already stored and checks that Store puts a new .meta file in place of
+// the old one, as it does the image, rather than writing into the old
+// file, which a crash could leave empty. The old .meta file is given a
+// second name before the store, so that it can be told apart from the new
+// one after it.
+func TestVariantStorage_StoreReplacesMetaFile(t *testing.T) {
+	t.Parallel()
+
+	storage, key := storeTestVariant(t)
+	metaPath := storage.keyToPath(key) + variantMetaSuffix
+	oldMetaPath := metaPath + ".old"
+
+	err := os.Link(metaPath, oldMetaPath)
+	if err != nil {
+		t.Fatalf("giving the .meta file a second name: %v", err)
+	}
+
+	_, err = storage.Store(key, bytes.NewReader([]byte("new variant data")),
+		testContentTypeJPEG)
+	if err != nil {
+		t.Fatalf("Store() error = %v", err)
+	}
+
+	oldMeta, err := os.Stat(oldMetaPath)
+	if err != nil {
+		t.Fatalf("stat of the old .meta file: %v", err)
+	}
+
+	newMeta, err := os.Stat(metaPath)
+	if err != nil {
+		t.Fatalf("stat of the new .meta file: %v", err)
+	}
+
+	if os.SameFile(oldMeta, newMeta) {
+		t.Error("Store() wrote into the old .meta file instead of putting " +
+			"a new one in its place")
+	}
+
+	reader, _, contentType, err := storage.LoadWithMeta(key)
+	if err != nil {
+		t.Fatalf("LoadWithMeta() error = %v", err)
+	}
+
+	_ = reader.Close()
+
+	t.Logf("content type after the store: %s", contentType)
+
+	if contentType != testContentTypeJPEG {
+		t.Errorf("content type = %q, want %q", contentType, testContentTypeJPEG)
+	}
+}
+
 // TestContentStorage_StoreHashedReplacesFileOfAnotherSize checks that a
 // store over a file of another size at the content's path, such as a
 // partial file a crash left, replaces it with the content.