From aee645faf8fb27bd86e930e40e4e6c63e556be15 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 03:34:02 +0000 Subject: [PATCH] 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. https://git.eeqj.de/sneak/pixa/issues/252 Model: opus-5-5 --- .../partial_cache_files_internal_test.go | 159 ++++++++++++++++-- 1 file changed, 144 insertions(+), 15 deletions(-) diff --git a/internal/imgcache/partial_cache_files_internal_test.go b/internal/imgcache/partial_cache_files_internal_test.go index c3fa5e1..283ac13 100644 --- a/internal/imgcache/partial_cache_files_internal_test.go +++ b/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 -// it: the first must fetch the source again and cache it whole, so that the -// second is made from the cache without a third fetch. +// file, as a crash or a restored snapshot could, and runs the reconciliation +// pass, which must remove it with a warning naming it. It then asks for two +// 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 -// two new sizes of it: the first must fetch the source again rather than -// fail to decode the partial file, and the second is made from the cache. +// file to half its size, as a crash or a restored snapshot could, and runs +// the reconciliation pass, which must remove it with a warning naming it. +// 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 -// size bytes, then checks that Get converts the image again instead of -// serving the file, and that the next Get serves the whole variant from -// the cache. +// assertConvertedAgain caches a variant of the photo, cuts its file to size +// bytes and runs the reconciliation pass, which must remove it with a +// warning naming it. It then checks that Get converts the image again +// 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 -// converted again rather than served empty. +// variant file, as a crash or a restored snapshot could, and checks that it +// 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 -// that it is converted again rather than served cut short. +// variant file to ten bytes, as a crash or a restored snapshot could, and +// 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.