From 89d7055382b21979b01c622192e65dcd1d3a91e3 Mon Sep 17 00:00:00 2001
From: clawbot <35+clawbot@noreply.example.org>
Date: Tue, 13 Oct 2026 01:31:00 +0000
Subject: [PATCH] Remove the unused cache lifetime setting (closes #69)

CacheConfig.CacheTTL was set to DefaultCacheTTL (24 hours) and never
read, so cached images never expired. Remove the field, the constant
and the line that set it. Cached source and transformed images stay
until eviction removes them to keep the cache within cache_max_bytes.

A new test caches a source and a transformed image, sets back two days
fetched_at in source_metadata and source_content, created_at in
variant_content, the fetched_at in the source's metadata file, the
created_at in the .meta file, and the access and modification times of
those two files and of the two image files, and checks that both are
still served from the cache without a new upstream fetch.

Existing tests lose only the CacheTTL line of their cache config.

Model: opus-5-5
---
 TODO.md                                       |   5 +
 internal/handlers/handlers.go                 |   1 -
 internal/handlers/handlers_internal_test.go   |   1 -
 .../image_cache_control_internal_test.go      |   1 -
 .../handlers/image_errors_internal_test.go    |   1 -
 .../referer_blocklist_internal_test.go        |   1 -
 internal/imgcache/cache.go                    |   1 -
 internal/imgcache/cache_internal_test.go      |   2 -
 internal/imgcache/eviction_internal_test.go   |   1 -
 internal/imgcache/module.go                   |   1 -
 .../imgcache/negative_cache_internal_test.go  |   2 -
 internal/imgcache/service_internal_test.go    | 166 ++++++++++++++++++
 internal/imgcache/stats_internal_test.go      |   5 -
 internal/imgcache/testutil_internal_test.go   |   1 -
 14 files changed, 171 insertions(+), 18 deletions(-)

diff --git a/TODO.md b/TODO.md
index 66067f4..0f36ccf 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-12 per-client limit on the image routes (closes #236): the new setting
   `image_requests_per_minute` (`PIXA_IMAGE_REQUESTS_PER_MINUTE`), default 600,
   is how many `GET` and `HEAD` requests one client may send to `/v1/image/` and
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..b5563e2 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,170 @@ 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 and 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 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: last_accessed_at in source_content and variant_content,
+// which is when eviction last saw the entry used, not when it was cached; 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)
+
+	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))
+
+	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 {