sneak/pixa
1
0
Fork 0
Code Issues 51 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

chore: update golangci-lint to v2.12.2 with canonical config (#54)
All checks were successful
check / check (push) Successful in 4s
Details

Browse Source 
Canonical v2-schema `.golangci.yml`, golangci-lint pins bumped to v2.12.2 in `Dockerfile` and `script/bootstrap`, and the tree brought to `0 issues.` under it.

Three behaviour deltas: `Cache.StoreVariant` takes a context (cancelled requests skip the accounting row, recovered by reconciliation); `MetadataStorage.Store` no longer leaks `.tmp-*.json` on Write/Close/Rename failure (dead-defer bug fix); the `signing_key` too-short error text gained a `value too short:` prefix.

Eviction-loop context cancellation deferred to #102.
This commit was merged in pull request #54.
This commit is contained in:
clawbot
2026-08-10 16:12:22 +02:00
parent 63fbc98e63
commit 2d805125ee
61 changed files with 3550 additions and 2472 deletions
Download Patch File Download Diff File Expand all files Collapse all files

184
internal/imgcache/cache.go
View File

@@ -76,7 +76,8 @@ type Cache struct {
evictionStarted bool
evictionStopOnce sync.Once
// In-memory cache of variant metadata (content type, size) to avoid reading .meta files
// In-memory cache of variant metadata (content type, size) to avoid
// reading .meta files
metaCache map[VariantKey]variantMeta
// contentLocks serializes StoreSource and evictSourceBlob per
@@ -116,17 +117,23 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
return c, nil
}
srcContent, err := NewContentStorage(filepath.Join(config.StateDir, "cache", "sources"))
srcContent, err := NewContentStorage(
filepath.Join(config.StateDir, "cache", "sources"),
)
if err != nil {
return nil, fmt.Errorf("failed to create source content storage: %w", err)
}
variants, err := NewVariantStorage(filepath.Join(config.StateDir, "cache", "variants"))
variants, err := NewVariantStorage(
filepath.Join(config.StateDir, "cache", "variants"),
)
if err != nil {
return nil, fmt.Errorf("failed to create variant storage: %w", err)
}
srcMetadata, err := NewMetadataStorage(filepath.Join(config.StateDir, "cache", "metadata"))
srcMetadata, err := NewMetadataStorage(
filepath.Join(config.StateDir, "cache", "metadata"),
)
if err != nil {
return nil, fmt.Errorf("failed to create source metadata storage: %w", err)
}
@@ -170,32 +177,6 @@ func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, e
}, nil
}
// touchVariant updates the LRU timestamp of a variant, best-effort:
// a failed touch only makes the entry look colder to eviction.
func (c *Cache) touchVariant(ctx context.Context, cacheKey VariantKey) {
_, err := c.db.ExecContext(ctx, `
UPDATE variant_content SET last_accessed_at = CURRENT_TIMESTAMP
WHERE cache_key = ?
`, string(cacheKey))
if err != nil {
c.log.Debug("failed to touch variant LRU timestamp",
"cache_key", cacheKey, "error", err)
}
}
// touchSourceContent updates the LRU timestamp of a source content
// blob, best-effort: a failed touch only makes the blob look colder.
func (c *Cache) touchSourceContent(ctx context.Context, contentHash ContentHash) {
_, err := c.db.ExecContext(ctx, `
UPDATE source_content SET last_accessed_at = CURRENT_TIMESTAMP
WHERE content_hash = ?
`, string(contentHash))
if err != nil {
c.log.Debug("failed to touch source content LRU timestamp",
"content_hash", contentHash, "error", err)
}
}
// GetVariant returns a reader, size, and content type for a cached variant.
func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, error) {
if c.disabled {
@@ -250,7 +231,11 @@ func (c *Cache) StoreSource(
// Store in database
pathHash := HashPath(req.SourcePath + "?" + req.SourceQuery)
headersJSON, _ := json.Marshal(result.Headers)
headersJSON, err := json.Marshal(result.Headers)
if err != nil {
return "", fmt.Errorf("failed to marshal response headers: %w", err)
}
_, err = c.db.ExecContext(ctx, `
INSERT INTO source_content (content_hash, content_type, size_bytes)
@@ -293,10 +278,8 @@ func (c *Cache) StoreSource(
RemoteAddr: result.RemoteAddr,
}
if err := c.srcMetadata.Store(req.SourceHost, pathHash, meta); err != nil {
// Non-fatal, we have it in the database
_ = err
}
// A failure here is non-fatal; the metadata is in the database.
_ = c.srcMetadata.Store(req.SourceHost, pathHash, meta)
c.notifyWritePressure()
@@ -307,7 +290,9 @@ func (c *Cache) StoreSource(
// it in the size accounting. On a disabled cache it is a no-op. The
// accounting insert is best-effort (the startup reconciliation pass
// adopts any variant file that misses its accounting row).
func (c *Cache) StoreVariant(cacheKey VariantKey, content io.Reader, contentType string) error {
func (c *Cache) StoreVariant(
ctx context.Context, cacheKey VariantKey, content io.Reader, contentType string,
) error {
if c.disabled {
return nil
}
@@ -317,7 +302,7 @@ func (c *Cache) StoreVariant(cacheKey VariantKey, content io.Reader, contentType
return err
}
_, err = c.db.Exec(`
_, err = c.db.ExecContext(ctx, `
INSERT INTO variant_content (cache_key, size_bytes, content_type)
VALUES (?, ?, ?)
ON CONFLICT(cache_key) DO UPDATE SET
@@ -339,7 +324,9 @@ func (c *Cache) StoreVariant(cacheKey VariantKey, content io.Reader, contentType
// Returns the content hash and content type if found, or empty values
// if not. Hits touch the blob's LRU timestamp; a disabled cache always
// reports no cached source.
func (c *Cache) LookupSource(ctx context.Context, req *ImageRequest) (ContentHash, string, error) {
func (c *Cache) LookupSource(
ctx context.Context, req *ImageRequest,
) (ContentHash, string, error) {
if c.disabled {
return "", "", nil
}
@@ -372,11 +359,15 @@ func (c *Cache) LookupSource(ctx context.Context, req *ImageRequest) (ContentHas
}
// StoreNegative stores a negative cache entry for a failed fetch.
func (c *Cache) StoreNegative(ctx context.Context, req *ImageRequest, statusCode int, errMsg string) error {
func (c *Cache) StoreNegative(
ctx context.Context, req *ImageRequest, statusCode int, errMsg string,
) error {
expiresAt := time.Now().UTC().Add(c.config.NegativeTTL)
_, err := c.db.ExecContext(ctx, `
INSERT INTO negative_cache (source_host, source_path, source_query, status_code, error_message, expires_at)
INSERT INTO negative_cache
(source_host, source_path, source_query, status_code,
error_message, expires_at)
VALUES (?, ?, ?, ?, ?, ?)
ON CONFLICT(source_host, source_path, source_query) DO UPDATE SET
status_code = excluded.status_code,
@@ -391,46 +382,16 @@ func (c *Cache) StoreNegative(ctx context.Context, req *ImageRequest, statusCode
return nil
}
// checkNegativeCache checks if a request is in the negative cache.
func (c *Cache) checkNegativeCache(ctx context.Context, req *ImageRequest) (bool, error) {
var expiresAt time.Time
err := c.db.QueryRowContext(ctx, `
SELECT expires_at FROM negative_cache
WHERE source_host = ? AND source_path = ? AND source_query = ?
`, req.SourceHost, req.SourcePath, req.SourceQuery).Scan(&expiresAt)
if errors.Is(err, sql.ErrNoRows) {
return false, nil
}
if err != nil {
return false, fmt.Errorf("failed to check negative cache: %w", err)
}
// Check if expired
if time.Now().After(expiresAt) {
// Clean up expired entry
_, _ = c.db.ExecContext(ctx, `
DELETE FROM negative_cache
WHERE source_host = ? AND source_path = ? AND source_query = ?
`, req.SourceHost, req.SourcePath, req.SourceQuery)
return false, nil
}
return true, nil
}
// GetSourceMetadataID returns the source metadata ID for a request.
func (c *Cache) GetSourceMetadataID(ctx context.Context, req *ImageRequest) (int64, error) {
func (c *Cache) GetSourceMetadataID(
ctx context.Context, req *ImageRequest,
) (int64, error) {
var id int64
err := c.db.QueryRowContext(ctx, `
SELECT id FROM source_metadata
WHERE source_host = ? AND source_path = ? AND source_query = ?
`, req.SourceHost, req.SourcePath, req.SourceQuery).Scan(&id)
if err != nil {
return 0, fmt.Errorf("failed to get source metadata ID: %w", err)
}
@@ -475,8 +436,12 @@ func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
}
// Get actual item count and total size from content tables
_ = c.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM request_cache`).Scan(&stats.TotalItems)
_ = c.db.QueryRowContext(ctx, `SELECT COALESCE(SUM(size_bytes), 0) FROM output_content`).Scan(&stats.TotalSizeBytes)
_ = c.db.QueryRowContext(ctx,
`SELECT COUNT(*) FROM request_cache`,
).Scan(&stats.TotalItems)
_ = c.db.QueryRowContext(ctx,
`SELECT COALESCE(SUM(size_bytes), 0) FROM output_content`,
).Scan(&stats.TotalSizeBytes)
// Compute hit rate as a ratio
if stats.HitCount+stats.MissCount > 0 {
@@ -490,11 +455,17 @@ func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64) {
if hit {
_, _ = c.db.ExecContext(ctx, `
UPDATE cache_stats SET hit_count = hit_count + 1, last_updated_at = CURRENT_TIMESTAMP WHERE id = 1
UPDATE cache_stats
SET hit_count = hit_count + 1,
last_updated_at = CURRENT_TIMESTAMP
WHERE id = 1
`)
} else {
_, _ = c.db.ExecContext(ctx, `
UPDATE cache_stats SET miss_count = miss_count + 1, last_updated_at = CURRENT_TIMESTAMP WHERE id = 1
UPDATE cache_stats
SET miss_count = miss_count + 1,
last_updated_at = CURRENT_TIMESTAMP
WHERE id = 1
`)
}
@@ -508,3 +479,62 @@ func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64)
`, fetchBytes)
}
}
// touchVariant updates the LRU timestamp of a variant, best-effort:
// a failed touch only makes the entry look colder to eviction.
func (c *Cache) touchVariant(ctx context.Context, cacheKey VariantKey) {
_, err := c.db.ExecContext(ctx, `
UPDATE variant_content SET last_accessed_at = CURRENT_TIMESTAMP
WHERE cache_key = ?
`, string(cacheKey))
if err != nil {
c.log.Debug("failed to touch variant LRU timestamp",
"cache_key", cacheKey, "error", err)
}
}
// touchSourceContent updates the LRU timestamp of a source content
// blob, best-effort: a failed touch only makes the blob look colder.
func (c *Cache) touchSourceContent(ctx context.Context, contentHash ContentHash) {
_, err := c.db.ExecContext(ctx, `
UPDATE source_content SET last_accessed_at = CURRENT_TIMESTAMP
WHERE content_hash = ?
`, string(contentHash))
if err != nil {
c.log.Debug("failed to touch source content LRU timestamp",
"content_hash", contentHash, "error", err)
}
}
// checkNegativeCache checks if a request is in the negative cache.
func (c *Cache) checkNegativeCache(
ctx context.Context, req *ImageRequest,
) (bool, error) {
var expiresAt time.Time
err := c.db.QueryRowContext(ctx, `
SELECT expires_at FROM negative_cache
WHERE source_host = ? AND source_path = ? AND source_query = ?
`, req.SourceHost, req.SourcePath, req.SourceQuery).Scan(&expiresAt)
if errors.Is(err, sql.ErrNoRows) {
return false, nil
}
if err != nil {
return false, fmt.Errorf("failed to check negative cache: %w", err)
}
// Check if expired
if time.Now().After(expiresAt) {
// Clean up expired entry
_, _ = c.db.ExecContext(ctx, `
DELETE FROM negative_cache
WHERE source_host = ? AND source_path = ? AND source_query = ?
`, req.SourceHost, req.SourcePath, req.SourceQuery)
return false, nil
}
return true, nil
}

108
internal/imgcache/cache_test.go → internal/imgcache/cache_internal_test.go
View File

@@ -86,14 +86,15 @@ func setupTestDB(t *testing.T) *sql.DB {
INSERT INTO cache_stats (id) VALUES (1);
`
if _, err := db.Exec(schema); err != nil {
_, err = db.ExecContext(t.Context(), schema)
if err != nil {
t.Fatalf("failed to create schema: %v", err)
}
return db
}
func setupTestCache(t *testing.T) (*Cache, string) {
func setupTestCache(t *testing.T) *Cache {
t.Helper()
tmpDir := t.TempDir()
@@ -108,16 +109,18 @@ func setupTestCache(t *testing.T) (*Cache, string) {
t.Fatalf("failed to create cache: %v", err)
}
return cache, tmpDir
return cache
}
func TestCache_LookupMiss(t *testing.T) {
cache, _ := setupTestCache(t)
t.Parallel()
cache := setupTestCache(t)
ctx := context.Background()
req := &ImageRequest{
SourceHost: "cdn.example.com",
SourcePath: "/photos/cat.jpg",
SourceHost: testHostCDN,
SourcePath: testPathCat,
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
Quality: 85,
@@ -139,12 +142,14 @@ func TestCache_LookupMiss(t *testing.T) {
}
func TestCache_StoreAndLookup(t *testing.T) {
cache, _ := setupTestCache(t)
t.Parallel()
cache := setupTestCache(t)
ctx := context.Background()
req := &ImageRequest{
SourceHost: "cdn.example.com",
SourcePath: "/photos/cat.jpg",
SourceHost: testHostCDN,
SourcePath: testPathCat,
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
Quality: 85,
@@ -154,11 +159,12 @@ func TestCache_StoreAndLookup(t *testing.T) {
// Store source content
sourceContent := []byte("fake jpeg data")
fetchResult := &httpfetcher.FetchResult{
ContentType: "image/jpeg",
Headers: map[string][]string{"Content-Type": {"image/jpeg"}},
ContentType: testContentTypeJPEG,
Headers: map[string][]string{testHeaderContentType: {testContentTypeJPEG}},
}
contentHash, err := cache.StoreSource(ctx, req, bytes.NewReader(sourceContent), fetchResult)
contentHash, err := cache.StoreSource(
ctx, req, bytes.NewReader(sourceContent), fetchResult)
if err != nil {
t.Fatalf("StoreSource() error = %v", err)
}
@@ -170,7 +176,9 @@ func TestCache_StoreAndLookup(t *testing.T) {
// Store variant
cacheKey := CacheKey(req)
outputContent := []byte("fake webp data")
err = cache.StoreVariant(cacheKey, bytes.NewReader(outputContent), "image/webp")
err = cache.StoreVariant(
t.Context(), cacheKey, bytes.NewReader(outputContent), "image/webp")
if err != nil {
t.Fatalf("StoreVariant() error = %v", err)
}
@@ -195,11 +203,13 @@ func TestCache_StoreAndLookup(t *testing.T) {
}
func TestCache_NegativeCache(t *testing.T) {
cache, _ := setupTestCache(t)
t.Parallel()
cache := setupTestCache(t)
ctx := context.Background()
req := &ImageRequest{
SourceHost: "cdn.example.com",
SourceHost: testHostCDN,
SourcePath: "/photos/notfound.jpg",
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
@@ -223,6 +233,8 @@ func TestCache_NegativeCache(t *testing.T) {
}
func TestCache_NegativeCacheExpiry(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
db := setupTestDB(t)
@@ -239,7 +251,7 @@ func TestCache_NegativeCacheExpiry(t *testing.T) {
ctx := context.Background()
req := &ImageRequest{
SourceHost: "cdn.example.com",
SourceHost: testHostCDN,
SourcePath: "/photos/expired.jpg",
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
@@ -266,11 +278,13 @@ func TestCache_NegativeCacheExpiry(t *testing.T) {
}
func TestCache_VariantLookup(t *testing.T) {
cache, _ := setupTestCache(t)
t.Parallel()
cache := setupTestCache(t)
ctx := context.Background()
req := &ImageRequest{
SourceHost: "cdn.example.com",
SourceHost: testHostCDN,
SourcePath: "/photos/variant.jpg",
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
@@ -281,7 +295,9 @@ func TestCache_VariantLookup(t *testing.T) {
// Store variant
cacheKey := CacheKey(req)
outputContent := []byte("output data")
err := cache.StoreVariant(cacheKey, bytes.NewReader(outputContent), "image/webp")
err := cache.StoreVariant(
t.Context(), cacheKey, bytes.NewReader(outputContent), "image/webp")
if err != nil {
t.Fatalf("StoreVariant() error = %v", err)
}
@@ -312,11 +328,13 @@ func TestCache_VariantLookup(t *testing.T) {
}
func TestCache_GetVariant_ReturnsContentType(t *testing.T) {
cache, _ := setupTestCache(t)
t.Parallel()
cache := setupTestCache(t)
ctx := context.Background()
req := &ImageRequest{
SourceHost: "cdn.example.com",
SourceHost: testHostCDN,
SourcePath: "/photos/variantct.jpg",
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
@@ -327,7 +345,9 @@ func TestCache_GetVariant_ReturnsContentType(t *testing.T) {
// Store variant
cacheKey := CacheKey(req)
outputContent := []byte("output webp data")
err := cache.StoreVariant(cacheKey, bytes.NewReader(outputContent), "image/webp")
err := cache.StoreVariant(
t.Context(), cacheKey, bytes.NewReader(outputContent), "image/webp")
if err != nil {
t.Fatalf("StoreVariant() error = %v", err)
}
@@ -347,7 +367,8 @@ func TestCache_GetVariant_ReturnsContentType(t *testing.T) {
if err != nil {
t.Fatalf("GetVariant() error = %v", err)
}
defer reader.Close()
defer func() { _ = reader.Close() }()
if contentType != "image/webp" {
t.Errorf("GetVariant() ContentType = %q, want %q", contentType, "image/webp")
@@ -359,11 +380,13 @@ func TestCache_GetVariant_ReturnsContentType(t *testing.T) {
}
func TestCache_GetVariant(t *testing.T) {
cache, _ := setupTestCache(t)
t.Parallel()
cache := setupTestCache(t)
ctx := context.Background()
req := &ImageRequest{
SourceHost: "cdn.example.com",
SourceHost: testHostCDN,
SourcePath: "/photos/output.jpg",
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
@@ -374,7 +397,9 @@ func TestCache_GetVariant(t *testing.T) {
// Store variant
cacheKey := CacheKey(req)
outputContent := []byte("the actual output content")
err := cache.StoreVariant(cacheKey, bytes.NewReader(outputContent), "image/webp")
err := cache.StoreVariant(
t.Context(), cacheKey, bytes.NewReader(outputContent), "image/webp")
if err != nil {
t.Fatalf("StoreVariant() error = %v", err)
}
@@ -390,7 +415,8 @@ func TestCache_GetVariant(t *testing.T) {
if err != nil {
t.Fatalf("GetVariant() error = %v", err)
}
defer reader.Close()
defer func() { _ = reader.Close() }()
buf := make([]byte, 100)
n, _ := reader.Read(buf)
@@ -401,7 +427,9 @@ func TestCache_GetVariant(t *testing.T) {
}
func TestCache_Stats(t *testing.T) {
cache, _ := setupTestCache(t)
t.Parallel()
cache := setupTestCache(t)
ctx := context.Background()
// Increment some stats
@@ -424,6 +452,8 @@ func TestCache_Stats(t *testing.T) {
}
func TestCache_CleanExpired(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
db := setupTestDB(t)
@@ -436,7 +466,8 @@ func TestCache_CleanExpired(t *testing.T) {
// Insert expired negative cache entry directly
_, err := db.ExecContext(ctx, `
INSERT INTO negative_cache (source_host, source_path, source_query, status_code, expires_at)
INSERT INTO negative_cache
(source_host, source_path, source_query, status_code, expires_at)
VALUES ('example.com', '/old.jpg', '', 404, datetime('now', '-1 hour'))
`)
if err != nil {
@@ -445,7 +476,12 @@ func TestCache_CleanExpired(t *testing.T) {
// Verify it exists
var count int
db.QueryRowContext(ctx, `SELECT COUNT(*) FROM negative_cache`).Scan(&count)
err = db.QueryRowContext(ctx, `SELECT COUNT(*) FROM negative_cache`).Scan(&count)
if err != nil {
t.Fatalf("failed to count negative cache entries: %v", err)
}
if count != 1 {
t.Fatalf("expected 1 negative cache entry, got %d", count)
}
@@ -457,13 +493,19 @@ func TestCache_CleanExpired(t *testing.T) {
}
// Verify it's gone
db.QueryRowContext(ctx, `SELECT COUNT(*) FROM negative_cache`).Scan(&count)
err = db.QueryRowContext(ctx, `SELECT COUNT(*) FROM negative_cache`).Scan(&count)
if err != nil {
t.Fatalf("failed to count negative cache entries: %v", err)
}
if count != 0 {
t.Errorf("expected 0 negative cache entries after clean, got %d", count)
}
}
func TestCache_StorageDirectoriesCreated(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
db := setupTestDB(t)
@@ -483,7 +525,9 @@ func TestCache_StorageDirectoriesCreated(t *testing.T) {
for _, dir := range dirs {
path := tmpDir + "/" + dir
if _, err := os.Stat(path); os.IsNotExist(err) {
_, err := os.Stat(path)
if os.IsNotExist(err) {
t.Errorf("directory %s was not created", dir)
}
}

29
internal/imgcache/contentlock_test.go → internal/imgcache/contentlock_internal_test.go
View File

@@ -10,10 +10,12 @@ import (
// TestContentLockExcludesSameKey verifies that two goroutines locking
// the same key never run their critical sections concurrently.
func TestContentLockExcludesSameKey(t *testing.T) {
t.Parallel()
lock := newContentLock()
var (
active int32
active atomic.Int32
maxSeen int32
wg sync.WaitGroup
)
@@ -22,14 +24,14 @@ func TestContentLockExcludesSameKey(t *testing.T) {
wg.Add(goroutines)
for i := 0; i < goroutines; i++ {
for range goroutines {
go func() {
defer wg.Done()
unlock := lock.Lock("same-key")
defer unlock()
n := atomic.AddInt32(&active, 1)
n := active.Add(1)
for {
seen := atomic.LoadInt32(&maxSeen)
@@ -40,7 +42,7 @@ func TestContentLockExcludesSameKey(t *testing.T) {
time.Sleep(time.Millisecond)
atomic.AddInt32(&active, -1)
active.Add(-1)
}()
}
@@ -57,13 +59,15 @@ func TestContentLockExcludesSameKey(t *testing.T) {
// them reaching the rendezvous point before any is allowed to
// proceed.
func TestContentLockAllowsDifferentKeys(t *testing.T) {
t.Parallel()
lock := newContentLock()
const goroutines = 20
var (
wg sync.WaitGroup
inside int32
inside atomic.Int32
reached = make(chan struct{}, goroutines)
)
@@ -71,7 +75,7 @@ func TestContentLockAllowsDifferentKeys(t *testing.T) {
release := make(chan struct{})
for i := 0; i < goroutines; i++ {
for i := range goroutines {
key := string(rune('a' + i))
go func() {
@@ -80,8 +84,10 @@ func TestContentLockAllowsDifferentKeys(t *testing.T) {
unlock := lock.Lock(key)
defer unlock()
atomic.AddInt32(&inside, 1)
inside.Add(1)
reached <- struct{}{}
<-release
}()
}
@@ -90,7 +96,7 @@ func TestContentLockAllowsDifferentKeys(t *testing.T) {
// own key's lock) without needing any other to release first. If
// keys were incorrectly serialized onto one underlying lock, only
// one would get here and this would time out.
for i := 0; i < goroutines; i++ {
for i := range goroutines {
select {
case <-reached:
case <-time.After(2 * time.Second):
@@ -99,8 +105,9 @@ func TestContentLockAllowsDifferentKeys(t *testing.T) {
}
}
if n := atomic.LoadInt32(&inside); n != goroutines {
t.Errorf("goroutines inside their critical section = %d, want %d", n, goroutines)
if n := inside.Load(); n != goroutines {
t.Errorf("goroutines inside their critical section = %d, want %d",
n, goroutines)
}
close(release)
@@ -111,6 +118,8 @@ func TestContentLockAllowsDifferentKeys(t *testing.T) {
// entries map does not grow without bound: once no goroutine holds or
// awaits a key, its entry is removed.
func TestContentLockRemovesEntryAfterUnlock(t *testing.T) {
t.Parallel()
lock := newContentLock()
unlock := lock.Lock("k")

4
internal/imgcache/divzero_test.go → internal/imgcache/divzero_internal_test.go
View File

@@ -7,6 +7,8 @@ import (
)
func TestSizePercentSafeWithZeroFetchBytes(t *testing.T) {
t.Parallel()
// Simulate the calculation from processAndStore
fetchBytes := int64(0)
outputSize := int64(100)
@@ -29,6 +31,8 @@ func TestSizePercentSafeWithZeroFetchBytes(t *testing.T) {
}
func TestSizePercentNormalCase(t *testing.T) {
t.Parallel()
fetchBytes := int64(1000)
outputSize := int64(500)

205
internal/imgcache/eviction.go
View File

@@ -2,6 +2,7 @@ package imgcache
import (
"context"
"database/sql"
"encoding/json"
"fmt"
"io/fs"
@@ -130,7 +131,8 @@ func (c *Cache) evictBatch(ctx context.Context, excessBytes int64) (int64, error
break
}
if err := c.evictCandidate(ctx, candidate); err != nil {
err := c.evictCandidate(ctx, candidate)
if err != nil {
c.log.Warn("failed to evict cache entry",
"cache_key", candidate.cacheKey,
"content_hash", candidate.contentHash,
@@ -209,7 +211,9 @@ func (c *Cache) variantCandidates(ctx context.Context) ([]evictionCandidate, err
candidate := evictionCandidate{isVariant: true}
var key string
if err := rows.Scan(&key, &candidate.sizeBytes, &candidate.lastAccessedAt); err != nil {
err := rows.Scan(&key, &candidate.sizeBytes, &candidate.lastAccessedAt)
if err != nil {
return nil, fmt.Errorf("failed to scan variant candidate: %w", err)
}
@@ -217,7 +221,8 @@ func (c *Cache) variantCandidates(ctx context.Context) ([]evictionCandidate, err
candidates = append(candidates, candidate)
}
if err := rows.Err(); err != nil {
err = rows.Err()
if err != nil {
return nil, fmt.Errorf("variant candidate iteration failed: %w", err)
}
@@ -246,7 +251,9 @@ func (c *Cache) sourceCandidates(ctx context.Context) ([]evictionCandidate, erro
var candidate evictionCandidate
var hash string
if err := rows.Scan(&hash, &candidate.sizeBytes, &candidate.lastAccessedAt); err != nil {
err := rows.Scan(&hash, &candidate.sizeBytes, &candidate.lastAccessedAt)
if err != nil {
return nil, fmt.Errorf("failed to scan source candidate: %w", err)
}
@@ -254,7 +261,8 @@ func (c *Cache) sourceCandidates(ctx context.Context) ([]evictionCandidate, erro
candidates = append(candidates, candidate)
}
if err := rows.Err(); err != nil {
err = rows.Err()
if err != nil {
return nil, fmt.Errorf("source candidate iteration failed: %w", err)
}
@@ -271,7 +279,8 @@ func (c *Cache) evictVariant(ctx context.Context, cacheKey VariantKey) error {
return fmt.Errorf("failed to delete variant accounting row: %w", err)
}
if err := c.variants.DeleteWithMeta(cacheKey); err != nil {
err = c.variants.DeleteWithMeta(cacheKey)
if err != nil {
return err
}
@@ -315,17 +324,20 @@ func (c *Cache) evictSourceBlob(ctx context.Context, contentHash ContentHash) er
defer func() { _ = tx.Rollback() }()
if _, err := tx.ExecContext(ctx,
`DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash)); err != nil {
_, err = tx.ExecContext(ctx,
`DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash))
if err != nil {
return fmt.Errorf("failed to delete source metadata rows: %w", err)
}
if _, err := tx.ExecContext(ctx,
`DELETE FROM source_content WHERE content_hash = ?`, string(contentHash)); err != nil {
_, err = tx.ExecContext(ctx,
`DELETE FROM source_content WHERE content_hash = ?`, string(contentHash))
if err != nil {
return fmt.Errorf("failed to delete source content row: %w", err)
}
if err := tx.Commit(); err != nil {
err = tx.Commit()
if err != nil {
return fmt.Errorf("failed to commit eviction transaction: %w", err)
}
@@ -335,13 +347,15 @@ func (c *Cache) evictSourceBlob(ctx context.Context, contentHash ContentHash) er
// Only after the rows are gone may the files be removed.
for _, reference := range references {
if err := c.srcMetadata.Delete(reference.host, reference.pathHash); err != nil {
err := c.srcMetadata.Delete(reference.host, reference.pathHash)
if err != nil {
c.log.Warn("failed to delete metadata sidecar",
"host", reference.host, "path_hash", reference.pathHash, "error", err)
}
}
if err := c.srcContent.Delete(contentHash); err != nil {
err = c.srcContent.Delete(contentHash)
if err != nil {
return err
}
@@ -368,7 +382,9 @@ func (c *Cache) sourceReferences(
var reference sourceReference
var pathHash string
if err := rows.Scan(&reference.host, &pathHash); err != nil {
err := rows.Scan(&reference.host, &pathHash)
if err != nil {
return nil, fmt.Errorf("failed to scan source reference: %w", err)
}
@@ -376,7 +392,8 @@ func (c *Cache) sourceReferences(
references = append(references, reference)
}
if err := rows.Err(); err != nil {
err = rows.Err()
if err != nil {
return nil, fmt.Errorf("source reference iteration failed: %w", err)
}
@@ -465,7 +482,8 @@ func (c *Cache) evictionLoop(interval time.Duration) {
// runEvictionPass runs one eviction pass, logging failures instead of
// propagating them (the loop must keep running).
func (c *Cache) runEvictionPass(ctx context.Context) {
if err := c.EvictToLimit(ctx); err != nil {
err := c.EvictToLimit(ctx)
if err != nil {
c.log.Warn("cache eviction pass failed", "error", err)
}
}
@@ -473,7 +491,8 @@ func (c *Cache) runEvictionPass(ctx context.Context) {
// runReconciliationPass runs one reconciliation pass, logging failures
// instead of propagating them (the loop must keep running).
func (c *Cache) runReconciliationPass(ctx context.Context) {
if err := c.reconcileAccounting(ctx); err != nil {
err := c.reconcileAccounting(ctx)
if err != nil {
c.log.Warn("cache accounting reconciliation failed", "error", err)
}
}
@@ -494,19 +513,23 @@ func (c *Cache) reconcileAccounting(ctx context.Context) error {
return nil
}
if err := c.reconcileVariantFiles(ctx); err != nil {
err := c.reconcileVariantFiles(ctx)
if err != nil {
return err
}
if err := c.reconcileVariantRows(ctx); err != nil {
err = c.reconcileVariantRows(ctx)
if err != nil {
return err
}
if err := c.reconcileSourceFiles(ctx); err != nil {
err = c.reconcileSourceFiles(ctx)
if err != nil {
return err
}
if err := c.reconcileSourceRows(ctx); err != nil {
err = c.reconcileSourceRows(ctx)
if err != nil {
return err
}
@@ -516,25 +539,28 @@ func (c *Cache) reconcileAccounting(ctx context.Context) error {
// reconcileVariantFiles walks the variant storage directory, adopting
// files without accounting rows and sweeping stale temp files.
func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
return filepath.WalkDir(c.variants.baseDir, func(path string, entry fs.DirEntry, err error) error {
if err != nil || entry.IsDir() {
return err
}
return filepath.WalkDir(
c.variants.baseDir,
func(path string, entry fs.DirEntry, err error) error {
if err != nil || entry.IsDir() {
return err
}
name := entry.Name()
name := entry.Name()
if strings.HasPrefix(name, tempFilePrefix) {
c.sweepStaleTempFile(path, entry)
if strings.HasPrefix(name, tempFilePrefix) {
c.sweepStaleTempFile(path, entry)
return nil
}
return nil
}
if strings.HasSuffix(name, variantMetaSuffix) {
return nil
}
if strings.HasSuffix(name, variantMetaSuffix) {
return nil
}
return c.adoptVariantFile(ctx, path, entry, VariantKey(name))
})
return c.adoptVariantFile(ctx, path, entry, VariantKey(name))
},
)
}
// adoptVariantFile inserts an accounting row for a variant file that
@@ -581,7 +607,8 @@ func (c *Cache) adoptVariantFile(
// variantContentTypeFromSidecar reads the content type from a variant
// .meta sidecar, falling back to application/octet-stream.
func (c *Cache) variantContentTypeFromSidecar(variantPath string) string {
metaData, err := os.ReadFile(variantPath + variantMetaSuffix) //nolint:gosec // path from cache walk
//nolint:gosec // path from cache walk
metaData, err := os.ReadFile(variantPath + variantMetaSuffix)
if err != nil {
return fallbackContentType
}
@@ -607,8 +634,9 @@ func (c *Cache) reconcileVariantRows(ctx context.Context) error {
continue
}
if _, err := c.db.ExecContext(ctx,
`DELETE FROM variant_content WHERE cache_key = ?`, string(key)); err != nil {
_, err := c.db.ExecContext(ctx,
`DELETE FROM variant_content WHERE cache_key = ?`, string(key))
if err != nil {
return fmt.Errorf("failed to drop stale variant accounting row: %w", err)
}
@@ -620,29 +648,43 @@ func (c *Cache) reconcileVariantRows(ctx context.Context) error {
// allVariantKeys returns every tracked variant cache key.
func (c *Cache) allVariantKeys(ctx context.Context) ([]VariantKey, error) {
rows, err := c.db.QueryContext(ctx, `SELECT cache_key FROM variant_content`)
return queryStringColumn[VariantKey](ctx, c.db,
`SELECT cache_key FROM variant_content`, "variant keys", "variant key")
}
// queryStringColumn runs a single-column query and returns the column
// values as T. plural names the set for the query and scan failure
// messages; singular names one row for the scan and iteration failure
// messages.
func queryStringColumn[T ~string](
ctx context.Context, db *sql.DB, query, plural, singular string,
) ([]T, error) {
rows, err := db.QueryContext(ctx, query)
if err != nil {
return nil, fmt.Errorf("failed to query variant keys: %w", err)
return nil, fmt.Errorf("failed to query %s: %w", plural, err)
}
defer func() { _ = rows.Close() }()
var keys []VariantKey
var values []T
for rows.Next() {
var key string
if err := rows.Scan(&key); err != nil {
return nil, fmt.Errorf("failed to scan variant key: %w", err)
var value string
err := rows.Scan(&value)
if err != nil {
return nil, fmt.Errorf("failed to scan %s: %w", singular, err)
}
keys = append(keys, VariantKey(key))
values = append(values, T(value))
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("variant key iteration failed: %w", err)
err = rows.Err()
if err != nil {
return nil, fmt.Errorf("%s iteration failed: %w", singular, err)
}
return keys, nil
return values, nil
}
// reconcileSourceFiles walks the source content directory, removing
@@ -650,21 +692,24 @@ func (c *Cache) allVariantKeys(ctx context.Context) ([]VariantKey, error) {
// lookups always go through source_metadata) and sweeping stale temp
// files.
func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
return filepath.WalkDir(c.srcContent.baseDir, func(path string, entry fs.DirEntry, err error) error {
if err != nil || entry.IsDir() {
return err
}
return filepath.WalkDir(
c.srcContent.baseDir,
func(path string, entry fs.DirEntry, err error) error {
if err != nil || entry.IsDir() {
return err
}
name := entry.Name()
name := entry.Name()
if strings.HasPrefix(name, tempFilePrefix) {
c.sweepStaleTempFile(path, entry)
if strings.HasPrefix(name, tempFilePrefix) {
c.sweepStaleTempFile(path, entry)
return nil
}
return nil
}
return c.removeUntrackedSourceFile(ctx, path, ContentHash(name))
})
return c.removeUntrackedSourceFile(ctx, path, ContentHash(name))
},
)
}
// removeUntrackedSourceFile deletes a source blob file that has no
@@ -686,13 +731,14 @@ func (c *Cache) removeUntrackedSourceFile(
return nil
}
if _, err := c.db.ExecContext(ctx,
`DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash)); err != nil {
_, err = c.db.ExecContext(ctx,
`DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash))
if err != nil {
return fmt.Errorf("failed to delete metadata rows for untracked blob: %w", err)
}
//nolint:gosec // G703: path comes from walking our own cache directory
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
err = os.Remove(path)
if err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to remove untracked source file: %w", err)
}
@@ -716,7 +762,8 @@ func (c *Cache) reconcileSourceRows(ctx context.Context) error {
// The blob file is already gone; evictSourceBlob removes the
// rows and sidecars and tolerates the missing file.
if err := c.evictSourceBlob(ctx, hash); err != nil {
err := c.evictSourceBlob(ctx, hash)
if err != nil {
return err
}
@@ -728,29 +775,9 @@ func (c *Cache) reconcileSourceRows(ctx context.Context) error {
// allSourceContentHashes returns every tracked source content hash.
func (c *Cache) allSourceContentHashes(ctx context.Context) ([]ContentHash, error) {
rows, err := c.db.QueryContext(ctx, `SELECT content_hash FROM source_content`)
if err != nil {
return nil, fmt.Errorf("failed to query source content hashes: %w", err)
}
defer func() { _ = rows.Close() }()
var hashes []ContentHash
for rows.Next() {
var hash string
if err := rows.Scan(&hash); err != nil {
return nil, fmt.Errorf("failed to scan content hash: %w", err)
}
hashes = append(hashes, ContentHash(hash))
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("content hash iteration failed: %w", err)
}
return hashes, nil
return queryStringColumn[ContentHash](ctx, c.db,
`SELECT content_hash FROM source_content`,
"source content hashes", "content hash")
}
// sweepStaleTempFile removes a temp file left behind by a crashed
@@ -765,8 +792,8 @@ func (c *Cache) sweepStaleTempFile(path string, entry fs.DirEntry) {
return
}
//nolint:gosec // G703: path comes from walking our own cache directory
if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
err = os.Remove(path)
if err != nil && !os.IsNotExist(err) {
c.log.Warn("failed to remove stale temp file", "path", path, "error", err)
return

328
internal/imgcache/eviction_test.go → internal/imgcache/eviction_internal_test.go
View File

@@ -20,6 +20,16 @@ import (
// implementation writes.
const sqliteTimestampFormat = "2006-01-02 15:04:05"
// testVariantKeyOne through testVariantKeyFour are the variant cache
// keys reused across the eviction tests, in the order the tests store
// them.
const (
testVariantKeyOne VariantKey = "aabbccdd0001"
testVariantKeyTwo VariantKey = "aabbccdd0002"
testVariantKeyThree VariantKey = "aabbccdd0003"
testVariantKeyFour VariantKey = "aabbccdd0004"
)
// evictionTestDB creates an in-memory SQLite database with the real
// production schema, limited to a single connection so the background
// eviction goroutine shares the same in-memory database as the test.
@@ -33,7 +43,8 @@ func evictionTestDB(t *testing.T) *sql.DB {
db.SetMaxOpenConns(1)
if err := database.ApplyMigrations(context.Background(), db, nil); err != nil {
err = database.ApplyMigrations(context.Background(), db, nil)
if err != nil {
t.Fatalf("failed to apply migrations: %v", err)
}
@@ -85,12 +96,16 @@ func storeEvictionTestSource(
result := &httpfetcher.FetchResult{
StatusCode: 200,
ContentType: "image/jpeg",
ContentType: testContentTypeJPEG,
ContentLength: int64(len(content)),
Headers: map[string][]string{"Content-Type": {"image/jpeg"}},
Headers: map[string][]string{
testHeaderContentType: {testContentTypeJPEG},
},
}
hash, err := cache.StoreSource(context.Background(), req, bytes.NewReader(content), result)
hash, err := cache.StoreSource(
context.Background(), req, bytes.NewReader(content), result,
)
if err != nil {
t.Fatalf("StoreSource(%s%s) failed: %v", host, path, err)
}
@@ -100,20 +115,25 @@ func storeEvictionTestSource(
// storeEvictionTestVariant stores content as a processed variant under
// the given cache key.
func storeEvictionTestVariant(t *testing.T, cache *Cache, key VariantKey, content []byte) {
func storeEvictionTestVariant(
t *testing.T, cache *Cache, key VariantKey, content []byte,
) {
t.Helper()
if err := cache.StoreVariant(key, bytes.NewReader(content), "image/webp"); err != nil {
err := cache.StoreVariant(t.Context(), key, bytes.NewReader(content), "image/webp")
if err != nil {
t.Fatalf("StoreVariant(%s) failed: %v", key, err)
}
}
// setVariantLastAccessed backdates the last access time of a tracked
// variant, to make LRU ordering deterministic in tests.
func setVariantLastAccessed(t *testing.T, cache *Cache, key VariantKey, when time.Time) {
func setVariantLastAccessed(
t *testing.T, cache *Cache, key VariantKey, when time.Time,
) {
t.Helper()
res, err := cache.db.Exec(
res, err := cache.db.ExecContext(t.Context(),
`UPDATE variant_content SET last_accessed_at = ? WHERE cache_key = ?`,
when.UTC().Format(sqliteTimestampFormat), string(key),
)
@@ -134,10 +154,12 @@ func setVariantLastAccessed(t *testing.T, cache *Cache, key VariantKey, when tim
// setSourceLastAccessed backdates the last access time of a tracked
// source content blob.
func setSourceLastAccessed(t *testing.T, cache *Cache, hash ContentHash, when time.Time) {
func setSourceLastAccessed(
t *testing.T, cache *Cache, hash ContentHash, when time.Time,
) {
t.Helper()
res, err := cache.db.Exec(
res, err := cache.db.ExecContext(t.Context(),
`UPDATE source_content SET last_accessed_at = ? WHERE content_hash = ?`,
when.UTC().Format(sqliteTimestampFormat), string(hash),
)
@@ -156,11 +178,13 @@ func setSourceLastAccessed(t *testing.T, cache *Cache, hash ContentHash, when ti
}
// countRows returns the number of rows the given query yields.
func countRows(t *testing.T, cache *Cache, query string, args ...interface{}) int {
func countRows(t *testing.T, cache *Cache, query string, args ...any) int {
t.Helper()
var n int
if err := cache.db.QueryRow(query, args...).Scan(&n); err != nil {
err := cache.db.QueryRowContext(t.Context(), query, args...).Scan(&n)
if err != nil {
t.Fatalf("count query %q failed: %v", query, err)
}
@@ -173,7 +197,7 @@ func countRows(t *testing.T, cache *Cache, query string, args ...interface{}) in
func assertNoDanglingReferences(t *testing.T, cache *Cache) {
t.Helper()
rows, err := cache.db.Query(
rows, err := cache.db.QueryContext(t.Context(),
`SELECT content_hash FROM source_metadata
WHERE content_hash IS NOT NULL AND content_hash != ''`,
)
@@ -185,20 +209,25 @@ func assertNoDanglingReferences(t *testing.T, cache *Cache) {
for rows.Next() {
var hash string
if err := rows.Scan(&hash); err != nil {
err := rows.Scan(&hash)
if err != nil {
t.Fatalf("failed to scan content_hash: %v", err)
}
if !cache.srcContent.Exists(ContentHash(hash)) {
t.Errorf("source_metadata references content %s but the file is missing", hash)
t.Errorf("source_metadata references content %s but the file is missing",
hash)
}
}
if err := rows.Err(); err != nil {
err = rows.Err()
if err != nil {
t.Fatalf("source_metadata iteration failed: %v", err)
}
variantRows, err := cache.db.Query(`SELECT cache_key FROM variant_content`)
variantRows, err := cache.db.QueryContext(t.Context(),
`SELECT cache_key FROM variant_content`)
if err != nil {
t.Fatalf("failed to query variant_content: %v", err)
}
@@ -207,7 +236,9 @@ func assertNoDanglingReferences(t *testing.T, cache *Cache) {
for variantRows.Next() {
var key string
if err := variantRows.Scan(&key); err != nil {
err := variantRows.Scan(&key)
if err != nil {
t.Fatalf("failed to scan cache_key: %v", err)
}
@@ -216,14 +247,17 @@ func assertNoDanglingReferences(t *testing.T, cache *Cache) {
}
}
if err := variantRows.Err(); err != nil {
err = variantRows.Err()
if err != nil {
t.Fatalf("variant_content iteration failed: %v", err)
}
}
// waitForUsageAtOrBelow polls UsageBytes until it reaches limit or the
// timeout expires, returning the last observed usage.
func waitForUsageAtOrBelow(t *testing.T, cache *Cache, limit int64, timeout time.Duration) int64 {
func waitForUsageAtOrBelow(
t *testing.T, cache *Cache, limit int64, timeout time.Duration,
) int64 {
t.Helper()
deadline := time.Now().Add(timeout)
@@ -249,14 +283,18 @@ func waitForUsageAtOrBelow(t *testing.T, cache *Cache, limit int64, timeout time
}
func TestUsageBytesAccountsSourceAndVariantBytes(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
storeEvictionTestSource(t, cache, "src.example.com", "/a.jpg",
bytes.Repeat([]byte{0xAA}, 1000))
storeEvictionTestSource(t, cache, "src.example.com", "/b.jpg",
bytes.Repeat([]byte{0xAB}, 2000))
storeEvictionTestVariant(t, cache, "aabbccdd0001", bytes.Repeat([]byte{0xAC}, 500))
storeEvictionTestVariant(t, cache, "aabbccdd0002", bytes.Repeat([]byte{0xAD}, 250))
storeEvictionTestVariant(t, cache, testVariantKeyOne,
bytes.Repeat([]byte{0xAC}, 500))
storeEvictionTestVariant(t, cache, testVariantKeyTwo,
bytes.Repeat([]byte{0xAD}, 250))
usage, err := cache.UsageBytes(context.Background())
if err != nil {
@@ -269,15 +307,20 @@ func TestUsageBytesAccountsSourceAndVariantBytes(t *testing.T) {
}
func TestUsageBytesCountsMultiReferencedBlobOnce(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
content := bytes.Repeat([]byte{0xCC}, 1200)
hashOne := storeEvictionTestSource(t, cache, "src.example.com", "/one.jpg", content)
hashTwo := storeEvictionTestSource(t, cache, "src.example.com", "/two.jpg", content)
hashOne := storeEvictionTestSource(t, cache,
"src.example.com", "/one.jpg", content)
hashTwo := storeEvictionTestSource(t, cache,
"src.example.com", "/two.jpg", content)
if hashOne != hashTwo {
t.Fatalf("identical content produced different hashes: %s vs %s", hashOne, hashTwo)
t.Fatalf("identical content produced different hashes: %s vs %s",
hashOne, hashTwo)
}
usage, err := cache.UsageBytes(context.Background())
@@ -291,12 +334,17 @@ func TestUsageBytesCountsMultiReferencedBlobOnce(t *testing.T) {
}
func TestEvictToLimitEvictsLeastRecentlyUsedFirst(t *testing.T) {
t.Parallel()
const limit = 3000
cache, _ := newEvictionTestCache(t, limit)
now := time.Now()
keys := []VariantKey{"aabbccdd0001", "aabbccdd0002", "aabbccdd0003", "aabbccdd0004"}
keys := []VariantKey{
testVariantKeyOne, testVariantKeyTwo,
testVariantKeyThree, testVariantKeyFour,
}
fills := []byte{0x01, 0x02, 0x03, 0x04}
ages := []time.Duration{4 * time.Hour, 3 * time.Hour, 2 * time.Hour, 1 * time.Hour}
@@ -305,7 +353,8 @@ func TestEvictToLimitEvictsLeastRecentlyUsedFirst(t *testing.T) {
setVariantLastAccessed(t, cache, key, now.Add(-ages[i]))
}
if err := cache.EvictToLimit(context.Background()); err != nil {
err := cache.EvictToLimit(context.Background())
if err != nil {
t.Fatalf("EvictToLimit failed: %v", err)
}
@@ -338,6 +387,8 @@ func TestEvictToLimitEvictsLeastRecentlyUsedFirst(t *testing.T) {
}
func TestEvictionRemovesMultiReferencedBlobTogetherWithAllReferences(t *testing.T) {
t.Parallel()
const limit = 1000
cache, _ := newEvictionTestCache(t, limit)
@@ -346,10 +397,14 @@ func TestEvictionRemovesMultiReferencedBlobTogetherWithAllReferences(t *testing.
// One 800-byte blob referenced by two source paths.
sharedContent := bytes.Repeat([]byte{0xDD}, 800)
sharedHash := storeEvictionTestSource(t, cache, "src.example.com", "/a.jpg", sharedContent)
sharedHash := storeEvictionTestSource(t, cache,
"src.example.com", "/a.jpg", sharedContent)
if h := storeEvictionTestSource(t, cache, "src.example.com", "/b.jpg", sharedContent); h != sharedHash {
t.Fatalf("identical content produced different hashes: %s vs %s", h, sharedHash)
h := storeEvictionTestSource(t, cache,
"src.example.com", "/b.jpg", sharedContent)
if h != sharedHash {
t.Fatalf("identical content produced different hashes: %s vs %s",
h, sharedHash)
}
// A newer 600-byte blob referenced by one source path.
@@ -359,7 +414,8 @@ func TestEvictionRemovesMultiReferencedBlobTogetherWithAllReferences(t *testing.
setSourceLastAccessed(t, cache, sharedHash, now.Add(-2*time.Hour))
setSourceLastAccessed(t, cache, recentHash, now.Add(-time.Minute))
if err := cache.EvictToLimit(context.Background()); err != nil {
err := cache.EvictToLimit(context.Background())
if err != nil {
t.Fatalf("EvictToLimit failed: %v", err)
}
@@ -408,25 +464,31 @@ func TestEvictionRemovesMultiReferencedBlobTogetherWithAllReferences(t *testing.
if n := countRows(t, cache,
`SELECT COUNT(*) FROM source_metadata WHERE content_hash = ?`, string(recentHash),
); n != 1 {
t.Errorf("recently used blob %s has %d source_metadata rows, want 1", recentHash, n)
t.Errorf("recently used blob %s has %d source_metadata rows, want 1",
recentHash, n)
}
assertNoDanglingReferences(t, cache)
}
func TestEvictionKeepsEverythingWhenUnderLimit(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
content := bytes.Repeat([]byte{0xDF}, 800)
hash := storeEvictionTestSource(t, cache, "src.example.com", "/a.jpg", content)
if h := storeEvictionTestSource(t, cache, "src.example.com", "/b.jpg", content); h != hash {
h := storeEvictionTestSource(t, cache, "src.example.com", "/b.jpg", content)
if h != hash {
t.Fatalf("identical content produced different hashes: %s vs %s", h, hash)
}
storeEvictionTestVariant(t, cache, "aabbccdd0001", bytes.Repeat([]byte{0xE0}, 500))
storeEvictionTestVariant(t, cache, testVariantKeyOne,
bytes.Repeat([]byte{0xE0}, 500))
if err := cache.EvictToLimit(context.Background()); err != nil {
err := cache.EvictToLimit(context.Background())
if err != nil {
t.Fatalf("EvictToLimit failed: %v", err)
}
@@ -440,7 +502,7 @@ func TestEvictionKeepsEverythingWhenUnderLimit(t *testing.T) {
t.Errorf("blob %s has %d source_metadata rows, want 2", hash, n)
}
if !cache.variants.Exists("aabbccdd0001") {
if !cache.variants.Exists(testVariantKeyOne) {
t.Error("variant must not be evicted while usage is under the limit")
}
@@ -448,8 +510,9 @@ func TestEvictionKeepsEverythingWhenUnderLimit(t *testing.T) {
}
func TestZeroMaxBytesDisablesDiskCache(t *testing.T) {
t.Parallel()
cache, tmpDir := newEvictionTestCache(t, 0)
ctx := context.Background()
req := &ImageRequest{
SourceHost: "src.example.com",
@@ -459,14 +522,31 @@ func TestZeroMaxBytesDisablesDiskCache(t *testing.T) {
FitMode: FitCover,
}
// Writes are no-ops that report success.
if err := cache.StoreVariant(CacheKey(req), bytes.NewReader([]byte("data")), "image/webp"); err != nil {
assertDisabledCacheWritesAreNoOps(t, cache, req)
assertDisabledCacheReadsAlwaysMiss(t, cache, req)
assertDisabledCacheTracksNothing(t, cache)
assertDisabledCacheWroteNothingToDisk(t, tmpDir)
}
// assertDisabledCacheWritesAreNoOps verifies that stores against a
// disabled cache report success without recording anything.
func assertDisabledCacheWritesAreNoOps(
t *testing.T, cache *Cache, req *ImageRequest,
) {
t.Helper()
ctx := t.Context()
err := cache.StoreVariant(
ctx, CacheKey(req), bytes.NewReader([]byte("data")), "image/webp",
)
if err != nil {
t.Fatalf("StoreVariant on disabled cache must be a no-op, got error: %v", err)
}
result := &httpfetcher.FetchResult{
StatusCode: 200,
ContentType: "image/jpeg",
ContentType: testContentTypeJPEG,
ContentLength: 4,
Headers: map[string][]string{},
}
@@ -479,8 +559,17 @@ func TestZeroMaxBytesDisablesDiskCache(t *testing.T) {
if hash != "" {
t.Errorf("StoreSource on disabled cache returned hash %q, want empty", hash)
}
}
// assertDisabledCacheReadsAlwaysMiss verifies that lookups against a
// disabled cache never report a hit.
func assertDisabledCacheReadsAlwaysMiss(
t *testing.T, cache *Cache, req *ImageRequest,
) {
t.Helper()
ctx := t.Context()
// Reads always miss.
lookup, err := cache.Lookup(ctx, req)
if err != nil {
t.Fatalf("Lookup on disabled cache failed: %v", err)
@@ -496,11 +585,17 @@ func TestZeroMaxBytesDisablesDiskCache(t *testing.T) {
}
if srcHash != "" || srcType != "" {
t.Errorf("LookupSource on disabled cache = (%q, %q), want empty", srcHash, srcType)
t.Errorf("LookupSource on disabled cache = (%q, %q), want empty",
srcHash, srcType)
}
}
// Nothing is tracked and nothing is written to disk.
usage, err := cache.UsageBytes(ctx)
// assertDisabledCacheTracksNothing verifies that a disabled cache
// records no usage and writes no accounting rows.
func assertDisabledCacheTracksNothing(t *testing.T, cache *Cache) {
t.Helper()
usage, err := cache.UsageBytes(t.Context())
if err != nil {
t.Fatalf("UsageBytes failed: %v", err)
}
@@ -516,24 +611,33 @@ func TestZeroMaxBytesDisablesDiskCache(t *testing.T) {
if n := countRows(t, cache, `SELECT COUNT(*) FROM source_metadata`); n != 0 {
t.Errorf("disabled cache wrote %d source_metadata rows, want 0", n)
}
}
if _, err := os.Stat(filepath.Join(tmpDir, "cache")); !os.IsNotExist(err) {
t.Errorf("disabled cache must not create the cache directory tree (stat err=%v)", err)
// assertDisabledCacheWroteNothingToDisk verifies that a disabled cache
// creates neither the cache directory tree nor any file under stateDir.
func assertDisabledCacheWroteNothingToDisk(t *testing.T, stateDir string) {
t.Helper()
_, err := os.Stat(filepath.Join(stateDir, "cache"))
if !os.IsNotExist(err) {
t.Errorf("disabled cache must not create the cache directory tree "+
"(stat err=%v)", err)
}
var foundFiles []string
walkErr := filepath.WalkDir(tmpDir, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
walkErr := filepath.WalkDir(stateDir,
func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if !d.IsDir() {
foundFiles = append(foundFiles, path)
}
if !d.IsDir() {
foundFiles = append(foundFiles, path)
}
return nil
})
return nil
})
if walkErr != nil {
t.Fatalf("failed to walk state dir: %v", walkErr)
}
@@ -544,6 +648,8 @@ func TestZeroMaxBytesDisablesDiskCache(t *testing.T) {
}
func TestEvictionRunsUnderWritePressure(t *testing.T) {
t.Parallel()
const limit = 1500
cache, _ := newEvictionTestCache(t, limit)
@@ -553,11 +659,14 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) {
cache.StartEviction(time.Hour)
defer cache.StopEviction()
keys := []VariantKey{"aabbccdd0001", "aabbccdd0002", "aabbccdd0003"}
keys := []VariantKey{
testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree,
}
fills := []byte{0x11, 0x12, 0x13}
for i, key := range keys {
storeEvictionTestVariant(t, cache, key, bytes.Repeat([]byte{fills[i]}, 1000))
storeEvictionTestVariant(t, cache, key,
bytes.Repeat([]byte{fills[i]}, 1000))
}
usage := waitForUsageAtOrBelow(t, cache, limit, 5*time.Second)
@@ -570,6 +679,8 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) {
}
func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
t.Parallel()
const limit = 1500
cache, _ := newEvictionTestCache(t, limit)
@@ -581,21 +692,25 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
cache.StartEviction(100 * time.Millisecond)
defer cache.StopEviction()
keys := []VariantKey{"aabbccdd0001", "aabbccdd0002", "aabbccdd0003"}
keys := []VariantKey{
testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree,
}
fills := []byte{0x21, 0x22, 0x23}
for i, key := range keys {
content := bytes.Repeat([]byte{fills[i]}, 1000)
if _, err := cache.variants.Store(key, bytes.NewReader(content), "image/webp"); err != nil {
_, err := cache.variants.Store(key, bytes.NewReader(content), "image/webp")
if err != nil {
t.Fatalf("failed to store variant file: %v", err)
}
if _, err := cache.db.Exec(
_, err = cache.db.ExecContext(t.Context(),
`INSERT INTO variant_content (cache_key, size_bytes, content_type)
VALUES (?, ?, ?)`,
string(key), len(content), "image/webp",
); err != nil {
)
if err != nil {
t.Fatalf("failed to insert variant accounting row: %v", err)
}
}
@@ -610,21 +725,28 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
}
func TestStartEvictionReconcilesAccountingWithDisk(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
// An untracked variant file on disk (e.g. written before this
// feature existed) must be adopted into the accounting.
untracked := bytes.Repeat([]byte{0x31}, 1000)
if _, err := cache.variants.Store("aabbccdd0001", bytes.NewReader(untracked), "image/webp"); err != nil {
_, err := cache.variants.Store(
testVariantKeyOne, bytes.NewReader(untracked), "image/webp",
)
if err != nil {
t.Fatalf("failed to store untracked variant file: %v", err)
}
// An accounting row whose file is missing must be dropped.
if _, err := cache.db.Exec(
_, err = cache.db.ExecContext(t.Context(),
`INSERT INTO variant_content (cache_key, size_bytes, content_type)
VALUES (?, ?, ?)`,
"deadbeef0001", 700, "image/webp",
); err != nil {
)
if err != nil {
t.Fatalf("failed to insert stale variant accounting row: %v", err)
}
@@ -656,7 +778,8 @@ func TestStartEvictionReconcilesAccountingWithDisk(t *testing.T) {
}
if n := countRows(t, cache,
`SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`, "aabbccdd0001",
`SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`,
string(testVariantKeyOne),
); n != 1 {
t.Errorf("untracked variant file was not adopted into accounting (rows=%d)", n)
}
@@ -678,6 +801,8 @@ func TestStartEvictionReconcilesAccountingWithDisk(t *testing.T) {
// staying invisible to UsageBytes/EvictToLimit until the next process
// restart.
func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
const interval = 100 * time.Millisecond
@@ -696,7 +821,11 @@ func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
// accounting insert) entirely, exactly as would happen if that
// insert had failed and only the file write had succeeded.
untracked := bytes.Repeat([]byte{0x41}, 900)
if _, err := cache.variants.Store("aabbccdd0099", bytes.NewReader(untracked), "image/webp"); err != nil {
_, err := cache.variants.Store(
"aabbccdd0099", bytes.NewReader(untracked), "image/webp",
)
if err != nil {
t.Fatalf("failed to store untracked variant file: %v", err)
}
@@ -742,12 +871,15 @@ func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
// mid-unlink, and must not lose its own store once eviction has fully
// released the content hash.
func TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
ctx := context.Background()
content := bytes.Repeat([]byte{0x55}, 400)
hash := storeEvictionTestSource(t, cache, "race.example.com", "/first.jpg", content)
hash := storeEvictionTestSource(t, cache,
"race.example.com", "/first.jpg", content)
proceed := make(chan struct{})
storeAttempted := make(chan struct{})
@@ -772,26 +904,7 @@ func TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent(t *testing.T)
// content file: exactly the window the review flagged.
<-storeAttempted
storeDone := make(chan error, 1)
go func() {
req := &ImageRequest{
SourceHost: "race.example.com",
SourcePath: "/dup.jpg",
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
}
result := &httpfetcher.FetchResult{
StatusCode: 200,
ContentType: "image/jpeg",
ContentLength: int64(len(content)),
Headers: map[string][]string{"Content-Type": {"image/jpeg"}},
}
_, err := cache.StoreSource(ctx, req, bytes.NewReader(content), result)
storeDone <- err
}()
storeDone := storeIdenticalContentConcurrently(ctx, cache, content)
// The concurrent store must not be able to complete while eviction
// still holds the content hash (i.e. before the file is unlinked):
@@ -808,11 +921,13 @@ func TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent(t *testing.T)
close(proceed)
if err := <-evictDone; err != nil {
err := <-evictDone
if err != nil {
t.Fatalf("evictSourceBlob failed: %v", err)
}
if err := <-storeDone; err != nil {
err = <-storeDone
if err != nil {
t.Fatalf("StoreSource failed: %v", err)
}
@@ -832,6 +947,39 @@ func TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent(t *testing.T)
}
if !cache.srcContent.Exists(dupHash) {
t.Errorf("source_content/source_metadata references %s but its file is missing", dupHash)
t.Errorf("source_content/source_metadata references %s but its file is missing",
dupHash)
}
}
// storeIdenticalContentConcurrently starts a StoreSource for a second
// source path whose body hashes to the same content hash, returning the
// channel its error is delivered on.
func storeIdenticalContentConcurrently(
ctx context.Context, cache *Cache, content []byte,
) chan error {
storeDone := make(chan error, 1)
go func() {
req := &ImageRequest{
SourceHost: "race.example.com",
SourcePath: "/dup.jpg",
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
}
result := &httpfetcher.FetchResult{
StatusCode: 200,
ContentType: testContentTypeJPEG,
ContentLength: int64(len(content)),
Headers: map[string][]string{
testHeaderContentType: {testContentTypeJPEG},
},
}
_, err := cache.StoreSource(ctx, req, bytes.NewReader(content), result)
storeDone <- err
}()
return storeDone
}

1
internal/imgcache/imgcache.go
View File

@@ -90,6 +90,7 @@ func (r *ImageRequest) SourceURL() string {
if r.AllowHTTP {
scheme = "http"
}
url := scheme + "://" + r.SourceHost + r.SourcePath
if r.SourceQuery != "" {
url += "?" + r.SourceQuery

13
internal/imgcache/negative_cache_test.go → internal/imgcache/negative_cache_internal_test.go
View File

@@ -8,6 +8,8 @@ import (
)
func TestNegativeCache_StoreAndCheck(t *testing.T) {
t.Parallel()
db := setupTestDB(t)
dir := t.TempDir()
@@ -22,7 +24,7 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) {
ctx := context.Background()
req := &ImageRequest{
SourceHost: "example.com",
SourceHost: testHostExample,
SourcePath: "/missing.jpg",
}
@@ -31,6 +33,7 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if hit {
t.Error("expected no negative cache hit initially")
}
@@ -46,12 +49,15 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if !hit {
t.Error("expected negative cache hit after storing")
}
}
func TestNegativeCache_Expired(t *testing.T) {
t.Parallel()
db := setupTestDB(t)
dir := t.TempDir()
@@ -66,7 +72,7 @@ func TestNegativeCache_Expired(t *testing.T) {
ctx := context.Background()
req := &ImageRequest{
SourceHost: "example.com",
SourceHost: testHostExample,
SourcePath: "/expired.jpg",
}
@@ -84,12 +90,15 @@ func TestNegativeCache_Expired(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if hit {
t.Error("expected expired negative cache entry to be a miss")
}
}
func TestService_Get_ReturnsErrorForNegativeCachedURL(t *testing.T) {
t.Parallel()
// This test verifies that Service.Get() checks the negative cache
// We can't easily test the full pipeline without vips, but we can
// verify the error type

152
internal/imgcache/service.go
View File

@@ -18,7 +18,8 @@ import (
"sneak.berlin/go/pixa/internal/signature"
)
// Service implements the ImageCache interface, orchestrating cache, fetcher, and processor.
// Service implements the ImageCache interface, orchestrating cache,
// fetcher, and processor.
type Service struct {
cache *Cache
fetcher httpfetcher.Fetcher
@@ -46,14 +47,21 @@ type ServiceConfig struct {
Logger *slog.Logger
}
// Static errors for service construction and unimplemented operations.
var (
errCacheRequired = errors.New("cache is required")
errSigningKeyRequired = errors.New("signing key is required")
errPurgeNotImplemented = errors.New("purge not implemented")
)
// NewService creates a new image service.
func NewService(cfg *ServiceConfig) (*Service, error) {
if cfg.Cache == nil {
return nil, errors.New("cache is required")
return nil, errCacheRequired
}
if cfg.SigningKey == "" {
return nil, errors.New("signing key is required")
return nil, errSigningKeyRequired
}
// Resolve fetcher config for defaults
@@ -83,11 +91,14 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
}
maxResponseSize := fetcherCfg.MaxResponseSize
processor := imageprocessor.New(
imageprocessor.Params{MaxInputBytes: maxResponseSize},
)
return &Service{
cache: cfg.Cache,
fetcher: fetcher,
processor: imageprocessor.New(imageprocessor.Params{MaxInputBytes: maxResponseSize}),
processor: processor,
signer: signer,
allowlist: allowlist.New(cfg.Allowlist),
log: log,
@@ -109,6 +120,7 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
if err != nil {
s.log.Warn("negative cache check failed", "error", err)
}
if negHit {
s.log.Debug("negative cache hit",
"host", req.SourceHost,
@@ -145,6 +157,7 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
// Cache miss - check if we have source content cached
cacheKey := CacheKey(req)
s.cache.IncrementStats(ctx, false, 0)
response, err := s.processFromSourceOrFetch(ctx, req, cacheKey)
@@ -157,6 +170,57 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
return response, nil
}
// Warm pre-fetches and caches an image without returning it.
func (s *Service) Warm(ctx context.Context, req *ImageRequest) error {
_, err := s.Get(ctx, req)
return err
}
// Purge removes a cached image. Purging is not implemented yet.
func (s *Service) Purge(_ context.Context, _ *ImageRequest) error {
return errPurgeNotImplemented
}
// Stats returns cache statistics.
func (s *Service) Stats(ctx context.Context) (*CacheStats, error) {
return s.cache.Stats(ctx)
}
// ValidateRequest validates the request signature if required.
func (s *Service) ValidateRequest(req *ImageRequest) error {
// Check if host is allowed (no signature required)
sourceURL := req.SourceURL()
parsedURL, err := url.Parse(sourceURL)
if err != nil {
return fmt.Errorf("invalid source URL: %w", err)
}
if s.allowlist.IsAllowed(parsedURL) {
return nil
}
// Signature required for non-allowed hosts
return s.signer.Verify(signatureRequest(req))
}
// GenerateSignedURL generates a signed URL for the given request.
func (s *Service) GenerateSignedURL(
baseURL string,
req *ImageRequest,
ttl time.Duration,
) (string, error) {
sigReq := signatureRequest(req)
path, sig, exp := s.signer.GenerateSignedURL(sigReq, ttl)
// Propagate the generated signature and expiration back onto the request.
req.Expires = sigReq.Expires
req.Signature = sigReq.Signature
return fmt.Sprintf("%s%s?sig=%s&exp=%d", baseURL, path, sig, exp), nil
}
// loadCachedSource attempts to load source content from cache, returning nil
// if the cached data is unavailable or exceeds maxResponseSize.
func (s *Service) loadCachedSource(contentHash ContentHash) []byte {
@@ -191,7 +255,8 @@ func (s *Service) loadCachedSource(contentHash ContentHash) []byte {
return data
}
// processFromSourceOrFetch processes an image, using cached source content if available.
// processFromSourceOrFetch processes an image, using cached source content
// if available.
func (s *Service) processFromSourceOrFetch(
ctx context.Context,
req *ImageRequest,
@@ -203,8 +268,10 @@ func (s *Service) processFromSourceOrFetch(
s.log.Warn("source lookup failed", "error", err)
}
var sourceData []byte
var fetchBytes int64
var (
sourceData []byte
fetchBytes int64
)
if contentHash != "" {
s.log.Debug("using cached source", "hash", contentHash)
@@ -248,6 +315,7 @@ func (s *Service) fetchAndProcess(
return nil, fmt.Errorf("upstream fetch failed: %w", err)
}
defer func() { _ = fetchResult.Content.Close() }()
// Read and validate the source content
@@ -258,6 +326,7 @@ func (s *Service) fetchAndProcess(
// Calculate download bitrate
fetchBytes := int64(len(sourceData))
var downloadRate string
if fetchResult.FetchDurationMs > 0 {
@@ -280,7 +349,8 @@ func (s *Service) fetchAndProcess(
)
// Validate magic bytes match content type
if err := magic.ValidateMagicBytes(sourceData, fetchResult.ContentType); err != nil {
err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType)
if err != nil {
return nil, fmt.Errorf("content validation failed: %w", err)
}
@@ -332,7 +402,8 @@ func (s *Service) processAndStore(
var sizePercent float64
if fetchBytes > 0 {
sizePercent = float64(outputSize) / float64(fetchBytes) * 100.0 //nolint:mnd // percentage calculation
//nolint:mnd // percentage calculation
sizePercent = float64(outputSize) / float64(fetchBytes) * 100.0
}
s.log.Info("image converted",
@@ -342,8 +413,10 @@ func (s *Service) processAndStore(
"dst_format", req.Format,
"src_bytes", fetchBytes,
"dst_bytes", outputSize,
"src_dimensions", fmt.Sprintf("%dx%d", processResult.InputWidth, processResult.InputHeight),
"dst_dimensions", fmt.Sprintf("%dx%d", processResult.Width, processResult.Height),
"src_dimensions", fmt.Sprintf("%dx%d",
processResult.InputWidth, processResult.InputHeight),
"dst_dimensions", fmt.Sprintf("%dx%d",
processResult.Width, processResult.Height),
"size_ratio", fmt.Sprintf("%.1f%%", sizePercent),
"convert_ms", processDuration.Milliseconds(),
"quality", req.Quality,
@@ -351,7 +424,10 @@ func (s *Service) processAndStore(
)
// Store variant to cache
if err := s.cache.StoreVariant(cacheKey, bytes.NewReader(processedData), processResult.ContentType); err != nil {
err = s.cache.StoreVariant(
ctx, cacheKey, bytes.NewReader(processedData), processResult.ContentType,
)
if err != nil {
s.log.Warn("failed to store variant", "error", err)
// Continue even if caching fails
}
@@ -365,58 +441,6 @@ func (s *Service) processAndStore(
}, nil
}
// Warm pre-fetches and caches an image without returning it.
func (s *Service) Warm(ctx context.Context, req *ImageRequest) error {
_, err := s.Get(ctx, req)
return err
}
// Purge removes a cached image.
func (s *Service) Purge(_ context.Context, _ *ImageRequest) error {
// TODO: Implement purge
return errors.New("purge not implemented")
}
// Stats returns cache statistics.
func (s *Service) Stats(ctx context.Context) (*CacheStats, error) {
return s.cache.Stats(ctx)
}
// ValidateRequest validates the request signature if required.
func (s *Service) ValidateRequest(req *ImageRequest) error {
// Check if host is allowed (no signature required)
sourceURL := req.SourceURL()
parsedURL, err := url.Parse(sourceURL)
if err != nil {
return fmt.Errorf("invalid source URL: %w", err)
}
if s.allowlist.IsAllowed(parsedURL) {
return nil
}
// Signature required for non-allowed hosts
return s.signer.Verify(signatureRequest(req))
}
// GenerateSignedURL generates a signed URL for the given request.
func (s *Service) GenerateSignedURL(
baseURL string,
req *ImageRequest,
ttl time.Duration,
) (string, error) {
sigReq := signatureRequest(req)
path, sig, exp := s.signer.GenerateSignedURL(sigReq, ttl)
// Propagate the generated signature and expiration back onto the request.
req.Expires = sigReq.Expires
req.Signature = sigReq.Signature
return fmt.Sprintf("%s%s?sig=%s&exp=%d", baseURL, path, sig, exp), nil
}
// signatureRequest projects an ImageRequest onto the standalone
// signature.Request type used by the signature package. This keeps the
// import edge one-way: imgcache depends on signature, never the reverse.

142
internal/imgcache/service_test.go → internal/imgcache/service_internal_test.go
View File

@@ -10,13 +10,22 @@ import (
"sneak.berlin/go/pixa/internal/signature"
)
// Test data literals used repeatedly in this file (goconst).
const (
testPathPhoto = "/images/photo.jpg"
testPathUpload = "/uploads/image.jpg"
testSigningKey = "test-signing-key-12345"
)
func TestService_Get_AllowlistedHost(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
ctx := context.Background()
req := &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: "/images/photo.jpg",
SourcePath: testPathPhoto,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
@@ -27,7 +36,8 @@ func TestService_Get_AllowlistedHost(t *testing.T) {
if err != nil {
t.Fatalf("Get() error = %v", err)
}
defer resp.Content.Close()
defer func() { _ = resp.Content.Close() }()
// Verify we got content
data, err := io.ReadAll(resp.Content)
@@ -39,17 +49,19 @@ func TestService_Get_AllowlistedHost(t *testing.T) {
t.Error("expected non-empty response")
}
if resp.ContentType != "image/jpeg" {
t.Errorf("ContentType = %q, want %q", resp.ContentType, "image/jpeg")
if resp.ContentType != testContentTypeJPEG {
t.Errorf("ContentType = %q, want %q", resp.ContentType, testContentTypeJPEG)
}
}
func TestService_Get_NonAllowlistedHost_NoSignature(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t, WithSigningKey("test-key"))
req := &ImageRequest{
SourceHost: fixtures.OtherHost,
SourcePath: "/uploads/image.jpg",
SourcePath: testPathUpload,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
@@ -64,13 +76,15 @@ func TestService_Get_NonAllowlistedHost_NoSignature(t *testing.T) {
}
func TestService_Get_NonAllowlistedHost_ValidSignature(t *testing.T) {
signingKey := "test-signing-key-12345"
t.Parallel()
signingKey := testSigningKey
svc, fixtures := SetupTestService(t, WithSigningKey(signingKey))
ctx := context.Background()
req := &ImageRequest{
SourceHost: fixtures.OtherHost,
SourcePath: "/uploads/image.jpg",
SourcePath: testPathUpload,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
@@ -93,7 +107,8 @@ func TestService_Get_NonAllowlistedHost_ValidSignature(t *testing.T) {
if err != nil {
t.Fatalf("Get() error = %v", err)
}
defer resp.Content.Close()
defer func() { _ = resp.Content.Close() }()
data, err := io.ReadAll(resp.Content)
if err != nil {
@@ -106,12 +121,14 @@ func TestService_Get_NonAllowlistedHost_ValidSignature(t *testing.T) {
}
func TestService_Get_NonAllowlistedHost_ExpiredSignature(t *testing.T) {
signingKey := "test-signing-key-12345"
t.Parallel()
signingKey := testSigningKey
svc, fixtures := SetupTestService(t, WithSigningKey(signingKey))
req := &ImageRequest{
SourceHost: fixtures.OtherHost,
SourcePath: "/uploads/image.jpg",
SourcePath: testPathUpload,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
@@ -131,12 +148,14 @@ func TestService_Get_NonAllowlistedHost_ExpiredSignature(t *testing.T) {
}
func TestService_Get_NonAllowlistedHost_InvalidSignature(t *testing.T) {
signingKey := "test-signing-key-12345"
t.Parallel()
signingKey := testSigningKey
svc, fixtures := SetupTestService(t, WithSigningKey(signingKey))
req := &ImageRequest{
SourceHost: fixtures.OtherHost,
SourcePath: "/uploads/image.jpg",
SourcePath: testPathUpload,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
@@ -159,6 +178,8 @@ func TestService_Get_NonAllowlistedHost_InvalidSignature(t *testing.T) {
// signature for one host must not verify for a different host, even
// if they share a domain suffix.
func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
t.Parallel()
signingKey := "test-signing-key-must-be-32-chars"
svc, _ := SetupTestService(t,
WithSigningKey(signingKey),
@@ -169,8 +190,8 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
// Sign a request for "cdn.example.com"
signedReq := &ImageRequest{
SourceHost: "cdn.example.com",
SourcePath: "/photos/cat.jpg",
SourceHost: testHostCDN,
SourcePath: testPathCat,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
@@ -181,6 +202,8 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
// The original request should pass validation
t.Run("exact host passes", func(t *testing.T) {
t.Parallel()
err := svc.ValidateRequest(signedReq)
if err != nil {
t.Errorf("ValidateRequest() exact host failed: %v", err)
@@ -192,7 +215,7 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
name string
host string
}{
{"parent domain", "example.com"},
{"parent domain", testHostExample},
{"sibling subdomain", "images.example.com"},
{"deeper subdomain", "a.cdn.example.com"},
{"evil suffix domain", "cdn.example.com.evil.com"},
@@ -201,6 +224,8 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name+" rejected", func(t *testing.T) {
t.Parallel()
req := &ImageRequest{
SourceHost: tt.host,
SourcePath: signedReq.SourcePath,
@@ -215,7 +240,8 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
err := svc.ValidateRequest(req)
if err == nil {
t.Errorf("ValidateRequest() should reject signature for host %q (signed for %q)",
t.Errorf(
"ValidateRequest() should reject signature for host %q (signed for %q)",
tt.host, signedReq.SourceHost)
}
})
@@ -223,6 +249,8 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
}
func TestService_Get_InvalidFile(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
ctx := context.Background()
@@ -243,6 +271,8 @@ func TestService_Get_InvalidFile(t *testing.T) {
}
func TestService_Get_NotFound(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
ctx := context.Background()
@@ -262,6 +292,8 @@ func TestService_Get_NotFound(t *testing.T) {
}
func TestService_Get_FormatConversion(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
ctx := context.Background()
@@ -273,7 +305,7 @@ func TestService_Get_FormatConversion(t *testing.T) {
}{
{
name: "JPEG to PNG",
sourcePath: "/images/photo.jpg",
sourcePath: testPathPhoto,
outFormat: FormatPNG,
wantMIME: "image/png",
},
@@ -281,7 +313,7 @@ func TestService_Get_FormatConversion(t *testing.T) {
name: "PNG to JPEG",
sourcePath: "/images/logo.png",
outFormat: FormatJPEG,
wantMIME: "image/jpeg",
wantMIME: testContentTypeJPEG,
},
{
name: "GIF to PNG",
@@ -293,6 +325,8 @@ func TestService_Get_FormatConversion(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
req := &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: tt.sourcePath,
@@ -306,7 +340,8 @@ func TestService_Get_FormatConversion(t *testing.T) {
if err != nil {
t.Fatalf("Get() error = %v", err)
}
defer resp.Content.Close()
defer func() { _ = resp.Content.Close() }()
if resp.ContentType != tt.wantMIME {
t.Errorf("ContentType = %q, want %q", resp.ContentType, tt.wantMIME)
@@ -341,12 +376,14 @@ func TestService_Get_FormatConversion(t *testing.T) {
}
func TestService_Get_Caching(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
ctx := context.Background()
req := &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: "/images/photo.jpg",
SourcePath: testPathPhoto,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
@@ -367,7 +404,8 @@ func TestService_Get_Caching(t *testing.T) {
if err != nil {
t.Fatalf("failed to read first response: %v", err)
}
resp1.Content.Close()
_ = resp1.Content.Close()
// Second request - should be a cache hit
resp2, err := svc.Get(ctx, req)
@@ -383,7 +421,8 @@ func TestService_Get_Caching(t *testing.T) {
if err != nil {
t.Fatalf("failed to read second response: %v", err)
}
resp2.Content.Close()
_ = resp2.Content.Close()
// Content should be identical
if len(data1) != len(data2) {
@@ -392,6 +431,8 @@ func TestService_Get_Caching(t *testing.T) {
}
func TestService_Get_DifferentSizes(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
ctx := context.Background()
@@ -402,12 +443,12 @@ func TestService_Get_DifferentSizes(t *testing.T) {
{Width: 75, Height: 75},
}
var responses [][]byte
responses := make([][]byte, 0, len(sizes))
for _, size := range sizes {
req := &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: "/images/photo.jpg",
SourcePath: testPathPhoto,
Size: size,
Format: FormatJPEG,
Quality: 85,
@@ -423,27 +464,31 @@ func TestService_Get_DifferentSizes(t *testing.T) {
if err != nil {
t.Fatalf("failed to read response: %v", err)
}
resp.Content.Close()
_ = resp.Content.Close()
responses = append(responses, data)
}
// All responses should be different sizes (different cache entries)
for i := 0; i < len(responses)-1; i++ {
for i := range len(responses) - 1 {
if len(responses[i]) == len(responses[i+1]) {
// Not necessarily an error, but worth noting
t.Logf("responses %d and %d have same size: %d bytes", i, i+1, len(responses[i]))
t.Logf("responses %d and %d have same size: %d bytes",
i, i+1, len(responses[i]))
}
}
}
func TestService_ValidateRequest_NoSigningKey(t *testing.T) {
t.Parallel()
// Service with no signing key - all non-allowlisted requests should fail
svc, fixtures := SetupTestService(t, WithNoAllowlist())
req := &ImageRequest{
SourceHost: fixtures.OtherHost,
SourcePath: "/uploads/image.jpg",
SourcePath: testPathUpload,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
@@ -452,11 +497,15 @@ func TestService_ValidateRequest_NoSigningKey(t *testing.T) {
err := svc.ValidateRequest(req)
if err == nil {
t.Error("ValidateRequest() expected error when no signing key and host not allowlisted")
t.Error(
"ValidateRequest() expected error when no signing key and host not allowlisted",
)
}
}
func TestService_Get_ContextCancellation(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
ctx, cancel := context.WithCancel(context.Background())
@@ -464,7 +513,7 @@ func TestService_Get_ContextCancellation(t *testing.T) {
req := &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: "/images/photo.jpg",
SourcePath: testPathPhoto,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
@@ -478,12 +527,14 @@ func TestService_Get_ContextCancellation(t *testing.T) {
}
func TestService_Get_ReturnsETag(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
ctx := context.Background()
req := &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: "/images/photo.jpg",
SourcePath: testPathPhoto,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
@@ -494,7 +545,8 @@ func TestService_Get_ReturnsETag(t *testing.T) {
if err != nil {
t.Fatalf("Get() error = %v", err)
}
defer resp.Content.Close()
defer func() { _ = resp.Content.Close() }()
// ETag should be set
if resp.ETag == "" {
@@ -508,12 +560,14 @@ func TestService_Get_ReturnsETag(t *testing.T) {
}
func TestService_Get_ETagConsistency(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
ctx := context.Background()
req := &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: "/images/photo.jpg",
SourcePath: testPathPhoto,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
@@ -525,16 +579,20 @@ func TestService_Get_ETagConsistency(t *testing.T) {
if err != nil {
t.Fatalf("Get() first request error = %v", err)
}
etag1 := resp1.ETag
resp1.Content.Close()
_ = resp1.Content.Close()
// Second request (from cache)
resp2, err := svc.Get(ctx, req)
if err != nil {
t.Fatalf("Get() second request error = %v", err)
}
etag2 := resp2.ETag
resp2.Content.Close()
_ = resp2.Content.Close()
// ETags should be identical for the same content
if etag1 != etag2 {
@@ -543,13 +601,15 @@ func TestService_Get_ETagConsistency(t *testing.T) {
}
func TestService_Get_DifferentETagsForDifferentContent(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
ctx := context.Background()
// Request same image at different sizes - should get different ETags
req1 := &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: "/images/photo.jpg",
SourcePath: testPathPhoto,
Size: Size{Width: 25, Height: 25},
Format: FormatJPEG,
Quality: 85,
@@ -558,7 +618,7 @@ func TestService_Get_DifferentETagsForDifferentContent(t *testing.T) {
req2 := &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: "/images/photo.jpg",
SourcePath: testPathPhoto,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
@@ -569,15 +629,19 @@ func TestService_Get_DifferentETagsForDifferentContent(t *testing.T) {
if err != nil {
t.Fatalf("Get() first request error = %v", err)
}
etag1 := resp1.ETag
resp1.Content.Close()
_ = resp1.Content.Close()
resp2, err := svc.Get(ctx, req2)
if err != nil {
t.Fatalf("Get() second request error = %v", err)
}
etag2 := resp2.ETag
resp2.Content.Close()
_ = resp2.Content.Close()
// ETags should be different for different content
if etag1 == etag2 {

16
internal/imgcache/sourceurl_test.go → internal/imgcache/sourceurl_internal_test.go
View File

@@ -3,13 +3,16 @@ package imgcache
import "testing"
func TestImageRequest_SourceURL_DefaultHTTPS(t *testing.T) {
t.Parallel()
req := &ImageRequest{
SourceHost: "cdn.example.com",
SourcePath: "/photos/cat.jpg",
SourceHost: testHostCDN,
SourcePath: testPathCat,
SourceQuery: "v=2",
}
got := req.SourceURL()
want := "https://cdn.example.com/photos/cat.jpg?v=2"
if got != want {
t.Errorf("SourceURL() = %q, want %q", got, want)
@@ -17,13 +20,16 @@ func TestImageRequest_SourceURL_DefaultHTTPS(t *testing.T) {
}
func TestImageRequest_SourceURL_AllowHTTP(t *testing.T) {
t.Parallel()
req := &ImageRequest{
SourceHost: "localhost:8080",
SourcePath: "/photos/cat.jpg",
SourcePath: testPathCat,
AllowHTTP: true,
}
got := req.SourceURL()
want := "http://localhost:8080/photos/cat.jpg"
if got != want {
t.Errorf("SourceURL() = %q, want %q", got, want)
@@ -31,8 +37,10 @@ func TestImageRequest_SourceURL_AllowHTTP(t *testing.T) {
}
func TestImageRequest_SourceURL_AllowHTTPFalse(t *testing.T) {
t.Parallel()
req := &ImageRequest{
SourceHost: "cdn.example.com",
SourceHost: testHostCDN,
SourcePath: "/img.jpg",
AllowHTTP: false,
}

21
internal/imgcache/stats_test.go → internal/imgcache/stats_internal_test.go
View File

@@ -12,18 +12,25 @@ import (
func setupStatsTestDB(t *testing.T) *sql.DB {
t.Helper()
db, err := sql.Open("sqlite", ":memory:")
if err != nil {
t.Fatal(err)
}
if err := database.ApplyMigrations(context.Background(), db, nil); err != nil {
err = database.ApplyMigrations(context.Background(), db, nil)
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { db.Close() })
t.Cleanup(func() { _ = db.Close() })
return db
}
func TestStats_HitRateIsRatio(t *testing.T) {
t.Parallel()
db := setupStatsTestDB(t)
dir := t.TempDir()
@@ -40,7 +47,9 @@ func TestStats_HitRateIsRatio(t *testing.T) {
// Set some hit/miss counts and a transform_count
_, err = db.ExecContext(ctx, `
UPDATE cache_stats SET hit_count = 75, miss_count = 25, transform_count = 9999 WHERE id = 1
UPDATE cache_stats
SET hit_count = 75, miss_count = 25, transform_count = 9999
WHERE id = 1
`)
if err != nil {
t.Fatal(err)
@@ -54,6 +63,7 @@ func TestStats_HitRateIsRatio(t *testing.T) {
if stats.HitCount != 75 {
t.Errorf("HitCount = %d, want 75", stats.HitCount)
}
if stats.MissCount != 25 {
t.Errorf("MissCount = %d, want 25", stats.MissCount)
}
@@ -61,11 +71,14 @@ func TestStats_HitRateIsRatio(t *testing.T) {
// HitRate should be 0.75, NOT 9999 (transform_count)
expectedRate := 0.75
if math.Abs(stats.HitRate-expectedRate) > 0.001 {
t.Errorf("HitRate = %f, want %f (was it scanning transform_count?)", stats.HitRate, expectedRate)
t.Errorf("HitRate = %f, want %f (was it scanning transform_count?)",
stats.HitRate, expectedRate)
}
}
func TestStats_ZeroCounts(t *testing.T) {
t.Parallel()
db := setupStatsTestDB(t)
dir := t.TempDir()

229
internal/imgcache/storage.go
View File

@@ -44,7 +44,8 @@ type ContentStorage struct {
// NewContentStorage creates a new content storage at the given base directory.
func NewContentStorage(baseDir string) (*ContentStorage, error) {
if err := os.MkdirAll(baseDir, StorageDirPerm); err != nil {
err := os.MkdirAll(baseDir, StorageDirPerm)
if err != nil {
return nil, fmt.Errorf("failed to create storage directory: %w", err)
}
@@ -53,7 +54,7 @@ func NewContentStorage(baseDir string) (*ContentStorage, error) {
// Store writes content to storage and returns its SHA256 hash.
// The content is read fully into memory to compute the hash before writing.
func (s *ContentStorage) Store(r io.Reader) (hash ContentHash, size int64, err error) {
func (s *ContentStorage) Store(r io.Reader) (ContentHash, int64, error) {
// Read all content to compute hash
data, err := io.ReadAll(r)
if err != nil {
@@ -62,10 +63,11 @@ func (s *ContentStorage) Store(r io.Reader) (hash ContentHash, size int64, err e
// Compute hash
h := sha256.Sum256(data)
hash = ContentHash(hex.EncodeToString(h[:]))
size = int64(len(data))
hash := ContentHash(hex.EncodeToString(h[:]))
size := int64(len(data))
if err := s.writeIfAbsent(hash, data); err != nil {
err = s.writeIfAbsent(hash, data)
if err != nil {
return "", 0, err
}
@@ -78,64 +80,17 @@ func (s *ContentStorage) Store(r io.Reader) (hash ContentHash, size int64, err e
// across the whole store operation) use this instead of Store. Like
// Store, it is idempotent: content already on disk at that path is
// left untouched.
func (s *ContentStorage) StoreHashed(hash ContentHash, data []byte) (size int64, err error) {
if err := s.writeIfAbsent(hash, data); err != nil {
func (s *ContentStorage) StoreHashed(
hash ContentHash, data []byte,
) (int64, error) {
err := s.writeIfAbsent(hash, data)
if err != nil {
return 0, err
}
return int64(len(data)), nil
}
// writeIfAbsent writes data to the path derived from hash, unless
// content already exists there, via a temp-file-plus-rename so
// concurrent readers never observe a partial file.
func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) (err error) {
// Build path: <basedir>/<ab>/<cd>/<hash>
path := s.hashToPath(hash)
// Check if already exists
if _, statErr := os.Stat(path); statErr == nil {
return nil
}
// Create directory structure
dir := filepath.Dir(path)
if err := os.MkdirAll(dir, StorageDirPerm); err != nil {
return fmt.Errorf("failed to create directory: %w", err)
}
// Write to temp file first, then rename for atomicity
tmpFile, err := os.CreateTemp(dir, ".tmp-*")
if err != nil {
return fmt.Errorf("failed to create temp file: %w", err)
}
tmpPath := tmpFile.Name()
defer func() {
if err != nil {
_ = os.Remove(tmpPath)
}
}()
if _, err := tmpFile.Write(data); err != nil {
_ = tmpFile.Close()
return fmt.Errorf("failed to write content: %w", err)
}
if err := tmpFile.Close(); err != nil {
return fmt.Errorf("failed to close temp file: %w", err)
}
// Atomic rename
//nolint:gosec // G703: paths from internal SHA256 hashes
if err := os.Rename(filepath.Clean(tmpPath), filepath.Clean(path)); err != nil {
return fmt.Errorf("failed to rename temp file: %w", err)
}
return nil
}
// Load returns a reader for the content with the given hash.
func (s *ContentStorage) Load(hash ContentHash) (io.ReadCloser, error) {
path := s.hashToPath(hash)
@@ -195,6 +150,67 @@ func (s *ContentStorage) Exists(hash ContentHash) bool {
return err == nil
}
// writeIfAbsent writes data to the path derived from hash, unless
// content already exists there, via a temp-file-plus-rename so
// concurrent readers never observe a partial file.
func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
// Build path: <basedir>/<ab>/<cd>/<hash>
path := s.hashToPath(hash)
// Check if already exists
_, statErr := os.Stat(path)
if statErr == nil {
return nil
}
// Create directory structure
dir := filepath.Dir(path)
err := os.MkdirAll(dir, StorageDirPerm)
if err != nil {
return fmt.Errorf("failed to create directory: %w", err)
}
// Write to temp file first, then rename for atomicity
tmpFile, err := os.CreateTemp(dir, ".tmp-*")
if err != nil {
return fmt.Errorf("failed to create temp file: %w", err)
}
tmpPath := tmpFile.Name()
// Each failure path below unlinks the temp file explicitly. This
// replaces a deferred cleanup that read a named result, which the
// canonical config does not permit; the set of paths that remove
// tmpPath, and the order relative to Close, is unchanged. This
// mirrors how MetadataStorage.Store and VariantStorage.Store below
// already express the same cleanup.
_, err = tmpFile.Write(data)
if err != nil {
_ = tmpFile.Close()
_ = os.Remove(tmpPath)
return fmt.Errorf("failed to write content: %w", err)
}
err = tmpFile.Close()
if err != nil {
_ = os.Remove(tmpPath)
return fmt.Errorf("failed to close temp file: %w", err)
}
// Atomic rename
err = os.Rename(filepath.Clean(tmpPath), filepath.Clean(path))
if err != nil {
_ = os.Remove(tmpPath)
return fmt.Errorf("failed to rename temp file: %w", err)
}
return nil
}
// hashToPath converts a hash to a file path: <basedir>/<ab>/<cd>/<hash>
func (s *ContentStorage) hashToPath(hash ContentHash) string {
h := string(hash)
@@ -213,7 +229,8 @@ type MetadataStorage struct {
// NewMetadataStorage creates a new metadata storage at the given base directory.
func NewMetadataStorage(baseDir string) (*MetadataStorage, error) {
if err := os.MkdirAll(baseDir, StorageDirPerm); err != nil {
err := os.MkdirAll(baseDir, StorageDirPerm)
if err != nil {
return nil, fmt.Errorf("failed to create metadata directory: %w", err)
}
@@ -221,6 +238,8 @@ func NewMetadataStorage(baseDir string) (*MetadataStorage, error) {
}
// SourceMetadata represents cached metadata about a source URL.
//
//nolint:tagliatelle // stored metadata format uses snake_case
type SourceMetadata struct {
Host string `json:"host"`
Path string `json:"path"`
@@ -239,12 +258,16 @@ type SourceMetadata struct {
}
// Store writes metadata to storage.
func (s *MetadataStorage) Store(host string, pathHash PathHash, meta *SourceMetadata) error {
func (s *MetadataStorage) Store(
host string, pathHash PathHash, meta *SourceMetadata,
) error {
path := s.metaPath(host, pathHash)
// Create directory structure
dir := filepath.Dir(path)
if err := os.MkdirAll(dir, StorageDirPerm); err != nil {
err := os.MkdirAll(dir, StorageDirPerm)
if err != nil {
return fmt.Errorf("failed to create directory: %w", err)
}
@@ -259,27 +282,29 @@ func (s *MetadataStorage) Store(host string, pathHash PathHash, meta *SourceMeta
if err != nil {
return fmt.Errorf("failed to create temp file: %w", err)
}
tmpPath := tmpFile.Name()
defer func() {
if err != nil {
_ = os.Remove(tmpPath)
}
}()
if _, err := tmpFile.Write(data); err != nil {
_, err = tmpFile.Write(data)
if err != nil {
_ = tmpFile.Close()
_ = os.Remove(tmpPath)
return fmt.Errorf("failed to write metadata: %w", err)
}
if err := tmpFile.Close(); err != nil {
err = tmpFile.Close()
if err != nil {
_ = os.Remove(tmpPath)
return fmt.Errorf("failed to close temp file: %w", err)
}
// Atomic rename
//nolint:gosec // G703: paths from internal SHA256 hashes
if err := os.Rename(filepath.Clean(tmpPath), filepath.Clean(path)); err != nil {
err = os.Rename(filepath.Clean(tmpPath), filepath.Clean(path))
if err != nil {
_ = os.Remove(tmpPath)
return fmt.Errorf("failed to rename temp file: %w", err)
}
@@ -287,7 +312,9 @@ func (s *MetadataStorage) Store(host string, pathHash PathHash, meta *SourceMeta
}
// Load reads metadata from storage.
func (s *MetadataStorage) Load(host string, pathHash PathHash) (*SourceMetadata, error) {
func (s *MetadataStorage) Load(
host string, pathHash PathHash,
) (*SourceMetadata, error) {
path := s.metaPath(host, pathHash)
data, err := os.ReadFile(path) //nolint:gosec // path derived from host+hash
@@ -300,7 +327,9 @@ func (s *MetadataStorage) Load(host string, pathHash PathHash) (*SourceMetadata,
}
var meta SourceMetadata
if err := json.Unmarshal(data, &meta); err != nil {
err = json.Unmarshal(data, &meta)
if err != nil {
return nil, fmt.Errorf("failed to unmarshal metadata: %w", err)
}
@@ -366,6 +395,8 @@ type VariantStorage struct {
}
// VariantMeta contains metadata about a cached variant.
//
//nolint:tagliatelle // stored metadata format uses snake_case
type VariantMeta struct {
ContentType string `json:"content_type"`
Size int64 `json:"size"`
@@ -374,7 +405,8 @@ type VariantMeta struct {
// NewVariantStorage creates a new variant storage at the given base directory.
func NewVariantStorage(baseDir string) (*VariantStorage, error) {
if err := os.MkdirAll(baseDir, StorageDirPerm); err != nil {
err := os.MkdirAll(baseDir, StorageDirPerm)
if err != nil {
return nil, fmt.Errorf("failed to create variant storage directory: %w", err)
}
@@ -382,19 +414,23 @@ func NewVariantStorage(baseDir string) (*VariantStorage, error) {
}
// Store writes content and metadata to storage at the given key.
func (s *VariantStorage) Store(key VariantKey, r io.Reader, contentType string) (size int64, err error) {
func (s *VariantStorage) Store(
key VariantKey, r io.Reader, contentType string,
) (int64, error) {
data, err := io.ReadAll(r)
if err != nil {
return 0, fmt.Errorf("failed to read content: %w", err)
}
size = int64(len(data))
size := int64(len(data))
path := s.keyToPath(key)
metaPath := path + ".meta"
// Create directory structure
dir := filepath.Dir(path)
if err := os.MkdirAll(dir, StorageDirPerm); err != nil {
err = os.MkdirAll(dir, StorageDirPerm)
if err != nil {
return 0, fmt.Errorf("failed to create directory: %w", err)
}
@@ -403,27 +439,29 @@ func (s *VariantStorage) Store(key VariantKey, r io.Reader, contentType string)
if err != nil {
return 0, fmt.Errorf("failed to create temp file: %w", err)
}
tmpPath := tmpFile.Name()
defer func() {
if err != nil {
_ = os.Remove(tmpPath)
}
}()
if _, err := tmpFile.Write(data); err != nil {
_, err = tmpFile.Write(data)
if err != nil {
_ = tmpFile.Close()
_ = os.Remove(tmpPath)
return 0, fmt.Errorf("failed to write content: %w", err)
}
if err := tmpFile.Close(); err != nil {
err = tmpFile.Close()
if err != nil {
_ = os.Remove(tmpPath)
return 0, fmt.Errorf("failed to close temp file: %w", err)
}
// Atomic rename content
//nolint:gosec // G703: paths from internal SHA256 hashes
if err := os.Rename(filepath.Clean(tmpPath), filepath.Clean(path)); err != nil {
err = os.Rename(filepath.Clean(tmpPath), filepath.Clean(path))
if err != nil {
_ = os.Remove(tmpPath)
return 0, fmt.Errorf("failed to rename temp file: %w", err)
}
@@ -439,10 +477,8 @@ func (s *VariantStorage) Store(key VariantKey, r io.Reader, contentType string)
return 0, fmt.Errorf("failed to marshal metadata: %w", err)
}
if err := os.WriteFile(metaPath, metaData, StorageFilePerm); err != nil {
// Non-fatal, content is stored
_ = err
}
// Metadata write failure is non-fatal; content is already stored.
_ = os.WriteFile(metaPath, metaData, StorageFilePerm)
return size, nil
}
@@ -463,8 +499,11 @@ func (s *VariantStorage) Load(key VariantKey) (io.ReadCloser, error) {
return f, nil
}
// LoadWithMeta returns a reader, size, and content type for the content at the given key.
func (s *VariantStorage) LoadWithMeta(key VariantKey) (io.ReadCloser, int64, string, error) {
// LoadWithMeta returns a reader, size, and content type for the content at
// the given key.
func (s *VariantStorage) LoadWithMeta(
key VariantKey,
) (io.ReadCloser, int64, string, error) {
path := s.keyToPath(key)
metaPath := path + ".meta"
@@ -521,14 +560,14 @@ func (s *VariantStorage) Delete(key VariantKey) error {
// DeleteWithMeta removes the content at the given key together with
// its .meta sidecar file. A missing file is not an error.
func (s *VariantStorage) DeleteWithMeta(key VariantKey) error {
if err := s.Delete(key); err != nil {
err := s.Delete(key)
if err != nil {
return err
}
metaPath := s.keyToPath(key) + ".meta"
//nolint:gosec // G703: path derived from cache key
err := os.Remove(metaPath)
err = os.Remove(metaPath)
if err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to delete variant metadata: %w", err)
}

113
internal/imgcache/storage_test.go → internal/imgcache/storage_internal_test.go
View File

@@ -2,6 +2,7 @@ package imgcache
import (
"bytes"
"errors"
"io"
"os"
"path/filepath"
@@ -9,13 +10,17 @@ import (
)
func TestContentStorage_StoreAndLoad(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
storage, err := NewContentStorage(tmpDir)
if err != nil {
t.Fatalf("NewContentStorage() error = %v", err)
}
content := []byte("hello world")
hash, size, err := storage.Store(bytes.NewReader(content))
if err != nil {
t.Fatalf("Store() error = %v", err)
@@ -31,8 +36,11 @@ func TestContentStorage_StoreAndLoad(t *testing.T) {
// Verify file exists at expected path
hashStr := string(hash)
expectedPath := filepath.Join(tmpDir, hashStr[0:2], hashStr[2:4], hashStr)
if _, err := os.Stat(expectedPath); err != nil {
_, err = os.Stat(expectedPath)
if err != nil {
t.Errorf("File not at expected path %s: %v", expectedPath, err)
}
@@ -41,7 +49,8 @@ func TestContentStorage_StoreAndLoad(t *testing.T) {
if err != nil {
t.Fatalf("Load() error = %v", err)
}
defer r.Close()
defer func() { _ = r.Close() }()
loaded, err := io.ReadAll(r)
if err != nil {
@@ -54,7 +63,10 @@ func TestContentStorage_StoreAndLoad(t *testing.T) {
}
func TestContentStorage_StoreIdempotent(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
storage, err := NewContentStorage(tmpDir)
if err != nil {
t.Fatalf("NewContentStorage() error = %v", err)
@@ -78,26 +90,33 @@ func TestContentStorage_StoreIdempotent(t *testing.T) {
}
func TestContentStorage_LoadNotFound(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
storage, err := NewContentStorage(tmpDir)
if err != nil {
t.Fatalf("NewContentStorage() error = %v", err)
}
_, err = storage.Load(ContentHash("nonexistent"))
if err != ErrNotFound {
if !errors.Is(err, ErrNotFound) {
t.Errorf("Load() error = %v, want ErrNotFound", err)
}
}
func TestContentStorage_Delete(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
storage, err := NewContentStorage(tmpDir)
if err != nil {
t.Fatalf("NewContentStorage() error = %v", err)
}
content := []byte("to be deleted")
hash, _, err := storage.Store(bytes.NewReader(content))
if err != nil {
t.Fatalf("Store() error = %v", err)
@@ -107,7 +126,8 @@ func TestContentStorage_Delete(t *testing.T) {
t.Error("Exists() = false, want true")
}
if err := storage.Delete(hash); err != nil {
err = storage.Delete(hash)
if err != nil {
t.Fatalf("Delete() error = %v", err)
}
@@ -117,20 +137,27 @@ func TestContentStorage_Delete(t *testing.T) {
}
func TestContentStorage_DeleteNonexistent(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
storage, err := NewContentStorage(tmpDir)
if err != nil {
t.Fatalf("NewContentStorage() error = %v", err)
}
// Should not error
if err := storage.Delete(ContentHash("nonexistent")); err != nil {
err = storage.Delete(ContentHash("nonexistent"))
if err != nil {
t.Errorf("Delete() error = %v, want nil", err)
}
}
func TestContentStorage_HashToPath(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
storage, err := NewContentStorage(tmpDir)
if err != nil {
t.Fatalf("NewContentStorage() error = %v", err)
@@ -138,50 +165,59 @@ func TestContentStorage_HashToPath(t *testing.T) {
// Test by storing and verifying the resulting path structure
content := []byte("test content for path verification")
hash, _, err := storage.Store(bytes.NewReader(content))
if err != nil {
t.Fatalf("Store() error = %v", err)
}
hashStr := string(hash)
expectedPath := filepath.Join(tmpDir, hashStr[0:2], hashStr[2:4], hashStr)
if _, err := os.Stat(expectedPath); err != nil {
_, err = os.Stat(expectedPath)
if err != nil {
t.Errorf("File not at expected path %s: %v", expectedPath, err)
}
}
func TestMetadataStorage_StoreAndLoad(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
storage, err := NewMetadataStorage(tmpDir)
if err != nil {
t.Fatalf("NewMetadataStorage() error = %v", err)
}
meta := &SourceMetadata{
Host: "cdn.example.com",
Path: "/photos/cat.jpg",
Host: testHostCDN,
Path: testPathCat,
ContentHash: "abc123",
StatusCode: 200,
ContentType: "image/jpeg",
ContentType: testContentTypeJPEG,
FetchedAt: 1704067200,
ETag: `"etag123"`,
}
pathHash := HashPath("/photos/cat.jpg")
pathHash := HashPath(testPathCat)
err = storage.Store("cdn.example.com", pathHash, meta)
err = storage.Store(testHostCDN, pathHash, meta)
if err != nil {
t.Fatalf("Store() error = %v", err)
}
// Verify file exists at expected path
expectedPath := filepath.Join(tmpDir, "cdn.example.com", string(pathHash)+".json")
if _, err := os.Stat(expectedPath); err != nil {
expectedPath := filepath.Join(tmpDir, testHostCDN, string(pathHash)+".json")
_, err = os.Stat(expectedPath)
if err != nil {
t.Errorf("File not at expected path %s: %v", expectedPath, err)
}
// Load and verify
loaded, err := storage.Load("cdn.example.com", pathHash)
loaded, err := storage.Load(testHostCDN, pathHash)
if err != nil {
t.Fatalf("Load() error = %v", err)
}
@@ -208,55 +244,64 @@ func TestMetadataStorage_StoreAndLoad(t *testing.T) {
}
func TestMetadataStorage_LoadNotFound(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
storage, err := NewMetadataStorage(tmpDir)
if err != nil {
t.Fatalf("NewMetadataStorage() error = %v", err)
}
_, err = storage.Load("example.com", PathHash("nonexistent"))
if err != ErrNotFound {
_, err = storage.Load(testHostExample, PathHash("nonexistent"))
if !errors.Is(err, ErrNotFound) {
t.Errorf("Load() error = %v, want ErrNotFound", err)
}
}
func TestMetadataStorage_Delete(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()
storage, err := NewMetadataStorage(tmpDir)
if err != nil {
t.Fatalf("NewMetadataStorage() error = %v", err)
}
meta := &SourceMetadata{
Host: "example.com",
Host: testHostExample,
Path: "/test.jpg",
StatusCode: 200,
}
pathHash := HashPath("/test.jpg")
err = storage.Store("example.com", pathHash, meta)
err = storage.Store(testHostExample, pathHash, meta)
if err != nil {
t.Fatalf("Store() error = %v", err)
}
if !storage.Exists("example.com", pathHash) {
if !storage.Exists(testHostExample, pathHash) {
t.Error("Exists() = false, want true")
}
if err := storage.Delete("example.com", pathHash); err != nil {
err = storage.Delete(testHostExample, pathHash)
if err != nil {
t.Fatalf("Delete() error = %v", err)
}
if storage.Exists("example.com", pathHash) {
if storage.Exists(testHostExample, pathHash) {
t.Error("Exists() = true after delete, want false")
}
}
func TestHashPath(t *testing.T) {
t.Parallel()
// Same input should produce same hash
hash1 := HashPath("/photos/cat.jpg")
hash2 := HashPath("/photos/cat.jpg")
hash1 := HashPath(testPathCat)
hash2 := HashPath(testPathCat)
if hash1 != hash2 {
t.Errorf("HashPath() not deterministic: %s vs %s", hash1, hash2)
@@ -276,9 +321,11 @@ func TestHashPath(t *testing.T) {
}
func TestCacheKey(t *testing.T) {
t.Parallel()
req1 := &ImageRequest{
SourceHost: "cdn.example.com",
SourcePath: "/photos/cat.jpg",
SourceHost: testHostCDN,
SourcePath: testPathCat,
SourceQuery: "",
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
@@ -287,8 +334,8 @@ func TestCacheKey(t *testing.T) {
}
req2 := &ImageRequest{
SourceHost: "cdn.example.com",
SourcePath: "/photos/cat.jpg",
SourceHost: testHostCDN,
SourcePath: testPathCat,
SourceQuery: "",
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
@@ -311,8 +358,8 @@ func TestCacheKey(t *testing.T) {
// Different size should produce different key
req3 := &ImageRequest{
SourceHost: "cdn.example.com",
SourcePath: "/photos/cat.jpg",
SourceHost: testHostCDN,
SourcePath: testPathCat,
SourceQuery: "",
Size: Size{Width: 400, Height: 300}, // Different size
Format: FormatWebP,
@@ -327,8 +374,8 @@ func TestCacheKey(t *testing.T) {
// Different format should produce different key
req4 := &ImageRequest{
SourceHost: "cdn.example.com",
SourcePath: "/photos/cat.jpg",
SourceHost: testHostCDN,
SourcePath: testPathCat,
SourceQuery: "",
Size: Size{Width: 800, Height: 600},
Format: FormatPNG, // Different format
@@ -343,8 +390,8 @@ func TestCacheKey(t *testing.T) {
// Different quality should produce different key
req5 := &ImageRequest{
SourceHost: "cdn.example.com",
SourcePath: "/photos/cat.jpg",
SourceHost: testHostCDN,
SourcePath: testPathCat,
SourceQuery: "",
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,

45
internal/imgcache/testutil_test.go → internal/imgcache/testutil_internal_test.go
View File

@@ -18,6 +18,15 @@ import (
"sneak.berlin/go/pixa/internal/httpfetcher"
)
// Shared test data literals, extracted as constants for goconst.
const (
testHostCDN = "cdn.example.com"
testHostExample = "example.com"
testPathCat = "/photos/cat.jpg"
testContentTypeJPEG = "image/jpeg"
testHeaderContentType = "Content-Type"
)
// TestFixtures contains paths to test files in the mock filesystem.
type TestFixtures struct {
// Valid image files
@@ -89,14 +98,16 @@ func generateTestJPEG(t *testing.T, width, height int, c color.Color) []byte {
t.Helper()
img := image.NewRGBA(image.Rect(0, 0, width, height))
for y := 0; y < height; y++ {
for x := 0; x < width; x++ {
for y := range height {
for x := range width {
img.Set(x, y, c)
}
}
var buf bytes.Buffer
if err := jpeg.Encode(&buf, img, &jpeg.Options{Quality: 85}); err != nil {
err := jpeg.Encode(&buf, img, &jpeg.Options{Quality: 85})
if err != nil {
t.Fatalf("failed to encode test JPEG: %v", err)
}
@@ -108,14 +119,16 @@ func generateTestPNG(t *testing.T, width, height int, c color.Color) []byte {
t.Helper()
img := image.NewRGBA(image.Rect(0, 0, width, height))
for y := 0; y < height; y++ {
for x := 0; x < width; x++ {
for y := range height {
for x := range width {
img.Set(x, y, c)
}
}
var buf bytes.Buffer
if err := png.Encode(&buf, img); err != nil {
err := png.Encode(&buf, img)
if err != nil {
t.Fatalf("failed to encode test PNG: %v", err)
}
@@ -126,15 +139,20 @@ func generateTestPNG(t *testing.T, width, height int, c color.Color) []byte {
func generateTestGIF(t *testing.T, width, height int, c color.Color) []byte {
t.Helper()
img := image.NewPaletted(image.Rect(0, 0, width, height), []color.Color{c, color.White})
for y := 0; y < height; y++ {
for x := 0; x < width; x++ {
img := image.NewPaletted(
image.Rect(0, 0, width, height),
[]color.Color{c, color.White},
)
for y := range height {
for x := range width {
img.SetColorIndex(x, y, 0)
}
}
var buf bytes.Buffer
if err := gif.Encode(&buf, img, nil); err != nil {
err := gif.Encode(&buf, img, nil)
if err != nil {
t.Fatalf("failed to encode test GIF: %v", err)
}
@@ -142,7 +160,9 @@ func generateTestGIF(t *testing.T, width, height int, c color.Color) []byte {
}
// SetupTestService creates a Service with mock fetcher for testing.
func SetupTestService(t *testing.T, opts ...TestServiceOption) (*Service, *TestFixtures) {
func SetupTestService(
t *testing.T, opts ...TestServiceOption,
) (*Service, *TestFixtures) {
t.Helper()
mockFS, fixtures := NewTestFS(t)
@@ -195,7 +215,8 @@ func setupServiceTestDB(t *testing.T) *sql.DB {
}
// Use the real production schema via migrations
if err := database.ApplyMigrations(context.Background(), db, nil); err != nil {
err = database.ApplyMigrations(context.Background(), db, nil)
if err != nil {
t.Fatalf("failed to apply migrations: %v", err)
}

16
internal/imgcache/urlparser.go
View File

@@ -40,7 +40,8 @@ type ParsedURL struct {
Format ImageFormat
}
// ParseImagePath parses the path captured by chi's wildcard: <host>/<path>/<size>.<format>
// ParseImagePath parses the path captured by chi's wildcard:
// <host>/<path>/<size>.<format>
// This is the primary entry point when using chi routing.
// Examples:
// - cdn.example.com/photos/cat.jpg/800x600.webp
@@ -76,7 +77,8 @@ func ParseImageURL(urlPath string) (*ParsedURL, error) {
// parseImageComponents parses <host>/<path>/<size>.<format> structure.
func parseImageComponents(remainder string) (*ParsedURL, error) {
// Check for path traversal before any other processing
if err := checkPathTraversal(remainder); err != nil {
err := checkPathTraversal(remainder)
if err != nil {
return nil, err
}
@@ -102,6 +104,7 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
// Split host from path
// The first segment is the host, everything after is the path
firstSlash := strings.Index(hostAndPath, "/")
var host, path, query string
if firstSlash == -1 {
@@ -181,8 +184,7 @@ func checkPathTraversal(path string) error {
// Also check for ".." as a path segment in the original path
// This catches cases where the path hasn't been normalized
segments := strings.Split(path, "/")
for _, seg := range segments {
for seg := range strings.SplitSeq(path, "/") {
// URL decode the segment
decodedSeg, _ := url.PathUnescape(seg)
decodedSeg = strings.ReplaceAll(decodedSeg, "\\", "/")
@@ -202,8 +204,10 @@ func parseSizeFormat(s string) (Size, ImageFormat, error) {
return Size{}, "", ErrInvalidSize
}
var size Size
var formatStr string
var (
size Size
formatStr string
)
if matches[4] == "orig" {
// "orig.format" pattern

198
internal/imgcache/urlparser_test.go → internal/imgcache/urlparser_internal_test.go
View File

@@ -1,93 +1,124 @@
package imgcache
import (
"errors"
"testing"
)
// assertParsedURL compares all fields of a parsed URL against the
// expected value.
func assertParsedURL(t *testing.T, got, want *ParsedURL) {
t.Helper()
if got.Host != want.Host {
t.Errorf("Host = %q, want %q", got.Host, want.Host)
}
if got.Path != want.Path {
t.Errorf("Path = %q, want %q", got.Path, want.Path)
}
if got.Query != want.Query {
t.Errorf("Query = %q, want %q", got.Query, want.Query)
}
if got.Size != want.Size {
t.Errorf("Size = %v, want %v", got.Size, want.Size)
}
if got.Format != want.Format {
t.Errorf("Format = %q, want %q", got.Format, want.Format)
}
}
func TestParseImageURL(t *testing.T) {
t.Parallel()
tests := []struct {
name string
input string
want *ParsedURL
wantErr error
name string
input string
want *ParsedURL
}{
{
name: "basic path with size",
input: "/v1/image/cdn.example.com/photos/cat.jpg/800x600.webp",
want: &ParsedURL{
Host: "cdn.example.com",
Path: "/photos/cat.jpg",
Query: "",
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
Host: testHostCDN, Path: testPathCat,
Size: Size{Width: 800, Height: 600}, Format: FormatWebP,
},
},
{
name: "original size with 0x0",
input: "/v1/image/cdn.example.com/photos/cat.jpg/0x0.jpeg",
want: &ParsedURL{
Host: "cdn.example.com",
Path: "/photos/cat.jpg",
Query: "",
Size: Size{Width: 0, Height: 0},
Format: FormatJPEG,
Host: testHostCDN, Path: testPathCat,
Size: Size{Width: 0, Height: 0}, Format: FormatJPEG,
},
},
{
name: "original size with orig keyword",
input: "/v1/image/cdn.example.com/photos/cat.jpg/orig.png",
want: &ParsedURL{
Host: "cdn.example.com",
Path: "/photos/cat.jpg",
Query: "",
Size: Size{Width: 0, Height: 0},
Format: FormatPNG,
Host: testHostCDN, Path: testPathCat,
Size: Size{Width: 0, Height: 0}, Format: FormatPNG,
},
},
{
name: "path with query string",
input: "/v1/image/cdn.example.com/photos/cat.jpg?arg1=val1&arg2=val2/800x600.webp",
want: &ParsedURL{
Host: "cdn.example.com",
Path: "/photos/cat.jpg",
Query: "arg1=val1&arg2=val2",
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
Host: testHostCDN, Path: testPathCat, Query: "arg1=val1&arg2=val2",
Size: Size{Width: 800, Height: 600}, Format: FormatWebP,
},
},
{
name: "deep nested path",
input: "/v1/image/cdn.example.com/a/b/c/d/image.jpg/1920x1080.avif",
want: &ParsedURL{
Host: "cdn.example.com",
Path: "/a/b/c/d/image.jpg",
Query: "",
Size: Size{Width: 1920, Height: 1080},
Format: FormatAVIF,
Host: testHostCDN, Path: "/a/b/c/d/image.jpg",
Size: Size{Width: 1920, Height: 1080}, Format: FormatAVIF,
},
},
{
name: "jpg alias for jpeg",
input: "/v1/image/example.com/img.png/100x100.jpg",
want: &ParsedURL{
Host: "example.com",
Path: "/img.png",
Query: "",
Size: Size{Width: 100, Height: 100},
Format: FormatJPEG,
Host: testHostExample, Path: "/img.png",
Size: Size{Width: 100, Height: 100}, Format: FormatJPEG,
},
},
{
name: "gif format",
input: "/v1/image/example.com/animated.gif/200x200.gif",
want: &ParsedURL{
Host: "example.com",
Path: "/animated.gif",
Query: "",
Size: Size{Width: 200, Height: 200},
Format: FormatGIF,
Host: testHostExample, Path: "/animated.gif",
Size: Size{Width: 200, Height: 200}, Format: FormatGIF,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got, err := ParseImageURL(tt.input)
if err != nil {
t.Fatalf("ParseImageURL() unexpected error = %v", err)
}
assertParsedURL(t, got, tt.want)
})
}
}
func TestParseImageURL_Errors(t *testing.T) {
t.Parallel()
tests := []struct {
name string
input string
wantErr error
}{
{
name: "missing prefix",
input: "/image/cdn.example.com/photo.jpg/800x600.webp",
@@ -122,47 +153,23 @@ func TestParseImageURL(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := ParseImageURL(tt.input)
t.Parallel()
if tt.wantErr != nil {
if err == nil {
t.Errorf("ParseImageURL() error = nil, wantErr %v", tt.wantErr)
return
}
if !errorIs(err, tt.wantErr) {
t.Errorf("ParseImageURL() error = %v, wantErr %v", err, tt.wantErr)
}
return
_, err := ParseImageURL(tt.input)
if err == nil {
t.Fatalf("ParseImageURL() error = nil, wantErr %v", tt.wantErr)
}
if err != nil {
t.Errorf("ParseImageURL() unexpected error = %v", err)
return
}
if got.Host != tt.want.Host {
t.Errorf("Host = %q, want %q", got.Host, tt.want.Host)
}
if got.Path != tt.want.Path {
t.Errorf("Path = %q, want %q", got.Path, tt.want.Path)
}
if got.Query != tt.want.Query {
t.Errorf("Query = %q, want %q", got.Query, tt.want.Query)
}
if got.Size != tt.want.Size {
t.Errorf("Size = %v, want %v", got.Size, tt.want.Size)
}
if got.Format != tt.want.Format {
t.Errorf("Format = %q, want %q", got.Format, tt.want.Format)
if !errorIs(err, tt.wantErr) {
t.Errorf("ParseImageURL() error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}
func TestParseImagePath(t *testing.T) {
t.Parallel()
// ParseImagePath is for chi wildcard capture (no /v1/image/ prefix)
tests := []struct {
name string
@@ -174,8 +181,8 @@ func TestParseImagePath(t *testing.T) {
name: "chi wildcard capture",
input: "cdn.example.com/photos/cat.jpg/800x600.webp",
want: &ParsedURL{
Host: "cdn.example.com",
Path: "/photos/cat.jpg",
Host: testHostCDN,
Path: testPathCat,
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
},
@@ -184,8 +191,8 @@ func TestParseImagePath(t *testing.T) {
name: "with leading slash from chi",
input: "/cdn.example.com/photos/cat.jpg/800x600.webp",
want: &ParsedURL{
Host: "cdn.example.com",
Path: "/photos/cat.jpg",
Host: testHostCDN,
Path: testPathCat,
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
},
@@ -194,35 +201,30 @@ func TestParseImagePath(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got, err := ParseImagePath(tt.input)
if (err != nil) != tt.wantErr {
t.Errorf("ParseImagePath() error = %v, wantErr %v", err, tt.wantErr)
return
}
if err != nil {
return
}
if got.Host != tt.want.Host {
t.Errorf("Host = %q, want %q", got.Host, tt.want.Host)
}
if got.Path != tt.want.Path {
t.Errorf("Path = %q, want %q", got.Path, tt.want.Path)
}
if got.Size != tt.want.Size {
t.Errorf("Size = %v, want %v", got.Size, tt.want.Size)
}
if got.Format != tt.want.Format {
t.Errorf("Format = %q, want %q", got.Format, tt.want.Format)
}
assertParsedURL(t, got, tt.want)
})
}
}
func TestParsedURL_ToImageRequest(t *testing.T) {
t.Parallel()
parsed := &ParsedURL{
Host: "cdn.example.com",
Path: "/photos/cat.jpg",
Host: testHostCDN,
Path: testPathCat,
Query: "version=2",
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
@@ -233,21 +235,27 @@ func TestParsedURL_ToImageRequest(t *testing.T) {
if req.SourceHost != parsed.Host {
t.Errorf("SourceHost = %q, want %q", req.SourceHost, parsed.Host)
}
if req.SourcePath != parsed.Path {
t.Errorf("SourcePath = %q, want %q", req.SourcePath, parsed.Path)
}
if req.SourceQuery != parsed.Query {
t.Errorf("SourceQuery = %q, want %q", req.SourceQuery, parsed.Query)
}
if req.Size != parsed.Size {
t.Errorf("Size = %v, want %v", req.Size, parsed.Size)
}
if req.Format != parsed.Format {
t.Errorf("Format = %q, want %q", req.Format, parsed.Format)
}
}
func TestParseImageURL_PathTraversal(t *testing.T) {
t.Parallel()
// All path traversal attempts should be rejected
tests := []struct {
name string
@@ -293,12 +301,14 @@ func TestParseImageURL_PathTraversal(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
_, err := ParseImageURL(tt.input)
if err == nil {
t.Error("ParseImageURL() should reject path traversal attempts")
}
if err != ErrPathTraversal {
if !errors.Is(err, ErrPathTraversal) {
t.Errorf("ParseImageURL() error = %v, want ErrPathTraversal", err)
}
})
@@ -306,6 +316,8 @@ func TestParseImageURL_PathTraversal(t *testing.T) {
}
func TestParseImagePath_PathTraversal(t *testing.T) {
t.Parallel()
// Test path traversal via ParseImagePath (chi wildcard)
tests := []struct {
name string
@@ -323,12 +335,14 @@ func TestParseImagePath_PathTraversal(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
_, err := ParseImagePath(tt.input)
if err == nil {
t.Error("ParseImagePath() should reject path traversal attempts")
}
if err != ErrPathTraversal {
if !errors.Is(err, ErrPathTraversal) {
t.Errorf("ParseImagePath() error = %v, want ErrPathTraversal", err)
}
})
@@ -337,7 +351,7 @@ func TestParseImagePath_PathTraversal(t *testing.T) {
// errorIs checks if err matches target (handles wrapped errors).
func errorIs(err, target error) bool {
if err == target {
if errors.Is(err, target) {
return true
}
// Check if error message contains target message for wrapped errors