diff --git a/TODO.md b/TODO.md index f239092..5a5cf5d 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,11 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-13 the unused cache lifetime is removed (closes #69): + `CacheConfig.CacheTTL`, which pixad set to `DefaultCacheTTL` (24 hours), was + never read, so cached images never expired. The field and the constant are + removed. Cached source and transformed images stay until eviction removes them + to keep the cache within `cache_max_bytes`. - 2026-10-13 SVG sources are refused, and `orig` is never saved as JPEG in place of the source's own format (closes #68): the upstream fetch no longer accepts `image/svg+xml`, which libvips would render. Both image routes answer 415 for diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 6405d08..297ba2d 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -106,7 +106,6 @@ func (s *Handlers) WaitForProcessing(ctx context.Context) int { func newCacheConfig(cfg *config.Config, log *slog.Logger) imgcache.CacheConfig { return imgcache.CacheConfig{ StateDir: cfg.StateDir, - CacheTTL: imgcache.DefaultCacheTTL, NegativeTTL: imgcache.DefaultNegativeTTL, MaxBytes: cfg.CacheMaxBytes, UseDefaultMaxBytes: !cfg.CacheMaxBytesExplicit, diff --git a/internal/handlers/handlers_internal_test.go b/internal/handlers/handlers_internal_test.go index 35c1522..f396a05 100644 --- a/internal/handlers/handlers_internal_test.go +++ b/internal/handlers/handlers_internal_test.go @@ -47,7 +47,6 @@ func setupTestHandler(t *testing.T) *testFixtures { cache, err := imgcache.NewCache(db, imgcache.CacheConfig{ StateDir: tmpDir, - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, }) if err != nil { diff --git a/internal/handlers/image_cache_control_internal_test.go b/internal/handlers/image_cache_control_internal_test.go index 7845df5..f4e65a5 100644 --- a/internal/handlers/image_cache_control_internal_test.go +++ b/internal/handlers/image_cache_control_internal_test.go @@ -31,7 +31,6 @@ func newSignedHostServer( cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ StateDir: t.TempDir(), - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, }) if err != nil { diff --git a/internal/handlers/image_errors_internal_test.go b/internal/handlers/image_errors_internal_test.go index 0bb3fea..fbbcf6f 100644 --- a/internal/handlers/image_errors_internal_test.go +++ b/internal/handlers/image_errors_internal_test.go @@ -29,7 +29,6 @@ func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler { cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ StateDir: t.TempDir(), - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, }) if err != nil { diff --git a/internal/handlers/referer_blocklist_internal_test.go b/internal/handlers/referer_blocklist_internal_test.go index 9235eb1..86f0c32 100644 --- a/internal/handlers/referer_blocklist_internal_test.go +++ b/internal/handlers/referer_blocklist_internal_test.go @@ -49,7 +49,6 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher) cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ StateDir: t.TempDir(), - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, }) if err != nil { diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index ff27456..1f73019 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -35,7 +35,6 @@ const metaCacheSize = 10000 // CacheConfig holds cache configuration. type CacheConfig struct { StateDir string - CacheTTL time.Duration NegativeTTL time.Duration // MaxBytes is the disk cache size limit in bytes that eviction diff --git a/internal/imgcache/cache_internal_test.go b/internal/imgcache/cache_internal_test.go index b4026f5..6659b93 100644 --- a/internal/imgcache/cache_internal_test.go +++ b/internal/imgcache/cache_internal_test.go @@ -102,7 +102,6 @@ func setupTestCache(t *testing.T) *Cache { cache, err := NewCache(db, CacheConfig{ StateDir: tmpDir, - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, }) if err != nil { @@ -241,7 +240,6 @@ func TestCache_NegativeCacheExpiry(t *testing.T) { // Very short negative TTL for testing cache, err := NewCache(db, CacheConfig{ StateDir: tmpDir, - CacheTTL: time.Hour, NegativeTTL: 1 * time.Millisecond, }) if err != nil { diff --git a/internal/imgcache/eviction_internal_test.go b/internal/imgcache/eviction_internal_test.go index af91ac5..7d54df1 100644 --- a/internal/imgcache/eviction_internal_test.go +++ b/internal/imgcache/eviction_internal_test.go @@ -70,7 +70,6 @@ func newEvictionTestCache(t *testing.T, maxBytes int64) (*Cache, string) { // backwards compatibility with existing fixtures. cache, err := NewCache(db, CacheConfig{ StateDir: tmpDir, - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, MaxBytes: maxBytes, DisableDiskCache: maxBytes == 0, diff --git a/internal/imgcache/module.go b/internal/imgcache/module.go index 266e398..81d329a 100644 --- a/internal/imgcache/module.go +++ b/internal/imgcache/module.go @@ -4,6 +4,5 @@ import "time" // CacheConfig defaults. const ( - DefaultCacheTTL = 24 * time.Hour DefaultNegativeTTL = 5 * time.Minute ) diff --git a/internal/imgcache/negative_cache_internal_test.go b/internal/imgcache/negative_cache_internal_test.go index 856f6ae..f2c2115 100644 --- a/internal/imgcache/negative_cache_internal_test.go +++ b/internal/imgcache/negative_cache_internal_test.go @@ -15,7 +15,6 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) { cache, err := NewCache(db, CacheConfig{ StateDir: dir, - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, }) if err != nil { @@ -63,7 +62,6 @@ func TestNegativeCache_Expired(t *testing.T) { cache, err := NewCache(db, CacheConfig{ StateDir: dir, - CacheTTL: time.Hour, NegativeTTL: 1 * time.Millisecond, // very short TTL }) if err != nil { diff --git a/internal/imgcache/service_internal_test.go b/internal/imgcache/service_internal_test.go index 5607853..85ad9dd 100644 --- a/internal/imgcache/service_internal_test.go +++ b/internal/imgcache/service_internal_test.go @@ -2,7 +2,9 @@ package imgcache import ( "context" + "encoding/json" "io" + "os" "testing" "time" @@ -430,6 +432,172 @@ func TestService_Get_Caching(t *testing.T) { } } +// TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo caches a source and +// a variant of it and sets back two days every time pixa stored for them. +// For the source: fetched_at in source_metadata, fetched_at and +// last_accessed_at in source_content, the fetched_at in its metadata file, +// and the access and modification times of its file and of its metadata +// file. For the variant: created_at and last_accessed_at in variant_content, +// the created_at in its .meta file, and the access and modification times of +// its file and of its .meta file. It then checks that the variant is still +// served from the cache and that a new variant is made from the cached source +// without fetching it again. +// +// Left at the present: the files' status change and birth times, which a +// program cannot set, and the times of the directories holding the files, +// which other entries share. The source's stored response headers are empty, +// as the mock fetcher sends none. expires_at, etag and last_modified in +// source_metadata and in the metadata file are never written, nor are the +// request_cache and output_content tables. +func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) { + t.Parallel() + + svc, fixtures := SetupTestService(t) + variant := photoVariant(fixtures, 85, FitCover) + + resp, err := svc.Get(t.Context(), variant) + if err != nil { + t.Fatalf("Get() first request error = %v", err) + } + + _ = resp.Content.Close() + + twoDaysAgo := time.Now().AddDate(0, 0, -2) + backdateSource(t, svc.cache, variant, twoDaysAgo) + backdateVariant(t, svc.cache, variant, twoDaysAgo) + + resp, err = svc.Get(t.Context(), variant) + if err != nil { + t.Fatalf("Get() of the cached variant error = %v", err) + } + + _ = resp.Content.Close() + + if resp.CacheStatus != CacheHit { + t.Errorf("cached variant CacheStatus = %q, want %q", + resp.CacheStatus, CacheHit) + } + + // Another quality is a new variant, made from the cached source + resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, FitCover)) + if err != nil { + t.Fatalf("Get() of a new variant error = %v", err) + } + + _ = resp.Content.Close() + + fetches := readCacheStatsCounters(t, svc.cache).upstreamFetchCount + if fetches != 1 { + t.Errorf("upstream fetches = %d, want 1", fetches) + } +} + +// backdateSource sets every time pixa stored for req's cached source to when, +// as listed on TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo. +func backdateSource( + t *testing.T, cache *Cache, req *ImageRequest, when time.Time, +) { + t.Helper() + + pathHash := HashPath(req.SourcePath + "?" + req.SourceQuery) + + meta, err := cache.srcMetadata.Load(req.SourceHost, pathHash) + if err != nil { + t.Fatalf("reading the source's metadata file: %v", err) + } + + meta.FetchedAt = when.Unix() + + err = cache.srcMetadata.Store(req.SourceHost, pathHash, meta) + if err != nil { + t.Fatalf("writing the source's metadata file: %v", err) + } + + dbTime := when.UTC().Format(sqliteTimestampFormat) + + updateOneRow(t, cache, ` + UPDATE source_metadata SET fetched_at = ? + WHERE source_host = ? AND source_path = ? AND source_query = ? + `, dbTime, req.SourceHost, req.SourcePath, req.SourceQuery) + updateOneRow(t, cache, + `UPDATE source_content SET fetched_at = ? WHERE content_hash = ?`, + dbTime, meta.ContentHash) + setSourceLastAccessed(t, cache, ContentHash(meta.ContentHash), when) + + for _, path := range []string{ + cache.srcContent.hashToPath(ContentHash(meta.ContentHash)), + cache.srcMetadata.metaPath(req.SourceHost, pathHash), + } { + err = os.Chtimes(path, when, when) + if err != nil { + t.Fatalf("setting back the times of %s: %v", path, err) + } + } +} + +// backdateVariant sets every time pixa stored for the cached variant req asks +// for to when, as listed on +// TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo. +func backdateVariant( + t *testing.T, cache *Cache, req *ImageRequest, when time.Time, +) { + t.Helper() + + key := CacheKey(req) + + content, size, contentType, err := cache.variants.LoadWithMeta(key) + if err != nil || contentType == "" { + t.Fatalf("reading the variant's .meta file: content type %q, error %v", + contentType, err) + } + + _ = content.Close() + + meta, err := json.Marshal(VariantMeta{ + ContentType: contentType, + Size: size, + CreatedAt: when.Unix(), + }) + if err != nil { + t.Fatalf("encoding the variant's .meta file: %v", err) + } + + path := cache.variants.keyToPath(key) + + err = os.WriteFile(path+variantMetaSuffix, meta, StorageFilePerm) + if err != nil { + t.Fatalf("writing the variant's .meta file: %v", err) + } + + updateOneRow(t, cache, + `UPDATE variant_content SET created_at = ? WHERE cache_key = ?`, + when.UTC().Format(sqliteTimestampFormat), string(key)) + setVariantLastAccessed(t, cache, key, when) + + for _, file := range []string{path, path + variantMetaSuffix} { + err = os.Chtimes(file, when, when) + if err != nil { + t.Fatalf("setting back the times of %s: %v", file, err) + } + } +} + +// updateOneRow runs the UPDATE statement query and fails the test unless it +// changed exactly one row. +func updateOneRow(t *testing.T, cache *Cache, query string, args ...any) { + t.Helper() + + result, err := cache.db.ExecContext(t.Context(), query, args...) + if err != nil { + t.Fatalf("%s: %v", query, err) + } + + rows, err := result.RowsAffected() + if err != nil || rows != 1 { + t.Fatalf("%s: rows = %d, error = %v, want 1 row", query, rows, err) + } +} + func TestService_Get_DifferentSizes(t *testing.T) { t.Parallel() diff --git a/internal/imgcache/stats_internal_test.go b/internal/imgcache/stats_internal_test.go index 94ee241..2fe9dbb 100644 --- a/internal/imgcache/stats_internal_test.go +++ b/internal/imgcache/stats_internal_test.go @@ -43,7 +43,6 @@ func TestStats_HitRateIsRatio(t *testing.T) { cache, err := NewCache(db, CacheConfig{ StateDir: dir, - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, }) if err != nil { @@ -91,7 +90,6 @@ func TestStats_ZeroCounts(t *testing.T) { cache, err := NewCache(db, CacheConfig{ StateDir: dir, - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, }) if err != nil { @@ -120,7 +118,6 @@ func TestStats_LogsFailedCountQueries(t *testing.T) { cache, err := NewCache(db, CacheConfig{ StateDir: t.TempDir(), - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, Logger: slog.New(slog.NewJSONHandler(&logBuf, nil)), }) @@ -162,7 +159,6 @@ func TestIncrementStats_LogsFailedUpdates(t *testing.T) { cache, err := NewCache(db, CacheConfig{ StateDir: t.TempDir(), - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, Logger: slog.New(slog.NewJSONHandler(&logBuf, nil)), }) @@ -232,7 +228,6 @@ func TestStats_DisabledCacheReportsNoItems(t *testing.T) { disabled, err := NewCache(enabled.db, CacheConfig{ StateDir: t.TempDir(), - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, DisableDiskCache: true, }) diff --git a/internal/imgcache/testutil_internal_test.go b/internal/imgcache/testutil_internal_test.go index b29f750..326537f 100644 --- a/internal/imgcache/testutil_internal_test.go +++ b/internal/imgcache/testutil_internal_test.go @@ -185,7 +185,6 @@ func SetupTestService( cache, err := NewCache(db, CacheConfig{ StateDir: tmpDir, - CacheTTL: time.Hour, NegativeTTL: 5 * time.Minute, }) if err != nil {