Remove the unused cache lifetime setting (closes #69) #239
@@ -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-13 SVG sources are refused, and `orig` is never saved as JPEG in place
|
- 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
|
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
|
`image/svg+xml`, which libvips would render. Both image routes answer 415 for
|
||||||
|
|||||||
@@ -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 {
|
||||||
|
|||||||
@@ -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
|
||||||
|
|||||||
@@ -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,
|
||||||
|
|||||||
@@ -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 {
|
||||||
|
|||||||
@@ -2,7 +2,9 @@ package imgcache
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"encoding/json"
|
||||||
"io"
|
"io"
|
||||||
|
"os"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"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) {
|
func TestService_Get_DifferentSizes(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -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 {
|
||||||
|
|||||||
Reference in New Issue
Block a user