Remove the unused cache lifetime setting (closes #69)
check / check (push) Waiting to run

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 checks that a source and a transformed image cached two
days ago are still served from the cache without a new upstream fetch.

Existing tests lose only the CacheTTL line of their cache config; no
test's checks change.

Model: opus-5-5
This commit is contained in:
2026-10-13 02:13:26 +00:00
parent 162a2a33ef
commit db49042f65
14 changed files with 63 additions and 18 deletions
+5
View File
@@ -30,6 +30,11 @@ P2: security: per-origin rate limiting
# Completed Steps # 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 - 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, `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 is how many `GET` and `HEAD` requests one client may send to `/v1/image/` and
-1
View File
@@ -106,7 +106,6 @@ func (s *Handlers) WaitForProcessing(ctx context.Context) int {
func newCacheConfig(cfg *config.Config, log *slog.Logger) imgcache.CacheConfig { func newCacheConfig(cfg *config.Config, log *slog.Logger) imgcache.CacheConfig {
return imgcache.CacheConfig{ return imgcache.CacheConfig{
StateDir: cfg.StateDir, StateDir: cfg.StateDir,
CacheTTL: imgcache.DefaultCacheTTL,
NegativeTTL: imgcache.DefaultNegativeTTL, NegativeTTL: imgcache.DefaultNegativeTTL,
MaxBytes: cfg.CacheMaxBytes, MaxBytes: cfg.CacheMaxBytes,
UseDefaultMaxBytes: !cfg.CacheMaxBytesExplicit, UseDefaultMaxBytes: !cfg.CacheMaxBytesExplicit,
@@ -47,7 +47,6 @@ func setupTestHandler(t *testing.T) *testFixtures {
cache, err := imgcache.NewCache(db, imgcache.CacheConfig{ cache, err := imgcache.NewCache(db, imgcache.CacheConfig{
StateDir: tmpDir, StateDir: tmpDir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -31,7 +31,6 @@ func newSignedHostServer(
cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
StateDir: t.TempDir(), StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -29,7 +29,6 @@ func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
StateDir: t.TempDir(), StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -49,7 +49,6 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher)
cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
StateDir: t.TempDir(), StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
-1
View File
@@ -35,7 +35,6 @@ const metaCacheSize = 10000
// CacheConfig holds cache configuration. // CacheConfig holds cache configuration.
type CacheConfig struct { type CacheConfig struct {
StateDir string StateDir string
CacheTTL time.Duration
NegativeTTL time.Duration NegativeTTL time.Duration
// MaxBytes is the disk cache size limit in bytes that eviction // MaxBytes is the disk cache size limit in bytes that eviction
-2
View File
@@ -102,7 +102,6 @@ func setupTestCache(t *testing.T) *Cache {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: tmpDir, StateDir: tmpDir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -241,7 +240,6 @@ func TestCache_NegativeCacheExpiry(t *testing.T) {
// Very short negative TTL for testing // Very short negative TTL for testing
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: tmpDir, StateDir: tmpDir,
CacheTTL: time.Hour,
NegativeTTL: 1 * time.Millisecond, NegativeTTL: 1 * time.Millisecond,
}) })
if err != nil { if err != nil {
@@ -70,7 +70,6 @@ func newEvictionTestCache(t *testing.T, maxBytes int64) (*Cache, string) {
// backwards compatibility with existing fixtures. // backwards compatibility with existing fixtures.
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: tmpDir, StateDir: tmpDir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
MaxBytes: maxBytes, MaxBytes: maxBytes,
DisableDiskCache: maxBytes == 0, DisableDiskCache: maxBytes == 0,
-1
View File
@@ -4,6 +4,5 @@ import "time"
// CacheConfig defaults. // CacheConfig defaults.
const ( const (
DefaultCacheTTL = 24 * time.Hour
DefaultNegativeTTL = 5 * time.Minute DefaultNegativeTTL = 5 * time.Minute
) )
@@ -15,7 +15,6 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: dir, StateDir: dir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -63,7 +62,6 @@ func TestNegativeCache_Expired(t *testing.T) {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: dir, StateDir: dir,
CacheTTL: time.Hour,
NegativeTTL: 1 * time.Millisecond, // very short TTL NegativeTTL: 1 * time.Millisecond, // very short TTL
}) })
if err != nil { if err != nil {
@@ -430,6 +430,64 @@ func TestService_Get_Caching(t *testing.T) {
} }
} }
// TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo caches a source and
// a variant of it, sets their stored fetch and creation times back two days,
// and 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.
func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
resp, err := svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
if err != nil {
t.Fatalf("Get() first request error = %v", err)
}
_ = resp.Content.Close()
for _, statement := range []string{
`UPDATE source_metadata SET fetched_at = datetime('now', '-2 days')`,
`UPDATE source_content SET fetched_at = datetime('now', '-2 days')`,
`UPDATE variant_content SET created_at = datetime('now', '-2 days')`,
} {
result, err := svc.cache.db.ExecContext(t.Context(), statement)
if err != nil {
t.Fatalf("%s: %v", statement, err)
}
rows, err := result.RowsAffected()
if err != nil || rows != 1 {
t.Fatalf("%s: rows = %d, error = %v, want 1 row", statement, rows, err)
}
}
resp, err = svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
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)
}
}
func TestService_Get_DifferentSizes(t *testing.T) { func TestService_Get_DifferentSizes(t *testing.T) {
t.Parallel() t.Parallel()
-5
View File
@@ -43,7 +43,6 @@ func TestStats_HitRateIsRatio(t *testing.T) {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: dir, StateDir: dir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -91,7 +90,6 @@ func TestStats_ZeroCounts(t *testing.T) {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: dir, StateDir: dir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -120,7 +118,6 @@ func TestStats_LogsFailedCountQueries(t *testing.T) {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: t.TempDir(), StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
Logger: slog.New(slog.NewJSONHandler(&logBuf, nil)), Logger: slog.New(slog.NewJSONHandler(&logBuf, nil)),
}) })
@@ -162,7 +159,6 @@ func TestIncrementStats_LogsFailedUpdates(t *testing.T) {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: t.TempDir(), StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
Logger: slog.New(slog.NewJSONHandler(&logBuf, nil)), Logger: slog.New(slog.NewJSONHandler(&logBuf, nil)),
}) })
@@ -232,7 +228,6 @@ func TestStats_DisabledCacheReportsNoItems(t *testing.T) {
disabled, err := NewCache(enabled.db, CacheConfig{ disabled, err := NewCache(enabled.db, CacheConfig{
StateDir: t.TempDir(), StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
DisableDiskCache: true, DisableDiskCache: true,
}) })
@@ -185,7 +185,6 @@ func SetupTestService(
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: tmpDir, StateDir: tmpDir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {