Remove the unused cache lifetime setting (closes #69) #239

Merged
clawbot merged 1 commits from issue-69-remove-cache-ttl into next 2026-10-13 07:58:56 +02:00
14 changed files with 173 additions and 18 deletions
+5
View File
@@ -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
-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 {
return imgcache.CacheConfig{
StateDir: cfg.StateDir,
CacheTTL: imgcache.DefaultCacheTTL,
NegativeTTL: imgcache.DefaultNegativeTTL,
MaxBytes: cfg.CacheMaxBytes,
UseDefaultMaxBytes: !cfg.CacheMaxBytesExplicit,
@@ -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 {
@@ -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 {
@@ -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 {
@@ -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 {
-1
View File
@@ -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
-2
View File
@@ -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 {
@@ -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,
-1
View File
@@ -4,6 +4,5 @@ import "time"
// CacheConfig defaults.
const (
DefaultCacheTTL = 24 * time.Hour
DefaultNegativeTTL = 5 * time.Minute
)
@@ -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 {
+168
View File
@@ -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()
-5
View File
@@ -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,
})
@@ -185,7 +185,6 @@ func SetupTestService(
cache, err := NewCache(db, CacheConfig{
StateDir: tmpDir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute,
})
if err != nil {