2 Commits
Author SHA1 Message Date
clawbot f586c695c1 Make the cache stats count what is cached, fetched and transcoded (closes #56)
check / check (push) Successful in 3m7s
Stats read request_cache and output_content, which nothing writes, so
TotalItems and TotalSizeBytes were always 0. They now count
source_content plus variant_content, the size through UsageBytes; a
failed query is still logged at warn. Get counts a miss after the work,
passing the bytes fetched from upstream (0 for a cached source; still
counted when the fetched source then fails), so upstream_fetch_count and
upstream_fetch_bytes move. transform_count is incremented after each
successful image processor call. request_cache and output_content stay
in the schema; dropping them is a separate decision.

Model: opus-5-5
2026-09-28 23:54:28 +00:00
clawbot e22ed368e4 Test that the cache stats counters and totals move (closes #56)
check / check (push) Failing after 2m24s
Failing tests, committed ahead of the fix. Stats totals are checked
after storing a source image and two processed variants. A walk through
Service.Get (a miss that fetches, a hit, a miss that reuses the cached
source, a source failing the magic byte check, a source not found)
checks every cache_stats counter after each step. The warn-log test for
the Stats queries now drops source_content and variant_content, the
tables Stats will read.

Model: opus-5-5
2026-09-28 23:47:16 +00:00
11 changed files with 184 additions and 222 deletions
-15
View File
@@ -111,21 +111,6 @@ with a private address can choose the address it is counted by through its own
its own address is trusted too. Setting `trusted_proxies` to the proxy's own its own address is trusted too. Setting `trusted_proxies` to the proxy's own
address closes this. address closes this.
### Image Metadata
pixa decodes and re-encodes every image it serves, and removes all metadata from
the output: EXIF (GPS position, camera make, model and serial number, capture
time, embedded thumbnail), XMP, IPTC and the ICC colour profile. This cannot be
turned off.
- The `orig` format means the source's own format, not the source's bytes: an
`orig` image is re-encoded and stripped like any other.
- An image with an EXIF orientation is turned upright first, so it displays the
same without the tag; a requested size applies to the upright image.
- An image with an ICC profile is converted to sRGB first, since clients show an
image with no profile as sRGB. Colours outside sRGB, such as the most
saturated ones in a Display P3 photo, are clipped.
### Source Hosts ### Source Hosts
Source hosts may be allowlisted in the configuration. Non-allowlisted Source hosts may be allowlisted in the configuration. Non-allowlisted
+10 -6
View File
@@ -30,12 +30,15 @@ exhaustion
# Completed Steps # Completed Steps
- 2026-09-28 strip metadata from processed images (closes #82): every output is - 2026-09-28 cache stats report real numbers (closes #56): `Cache.Stats`
exported with govips' `StripMetadata`, so it carries no EXIF, XMP, IPTC or ICC counts the cached source images and processed variants (`source_content`
profile; the image is first turned upright with `AutoRotate` (before sizes are plus `variant_content`) and takes their size from `Cache.UsageBytes`,
worked out) and, when it has an ICC profile, converted to sRGB; the `orig` instead of reading `request_cache` and `output_content`, which nothing
format is re-encoded and stripped like any other, as pixa never serves the writes; those two tables are left in the schema. A miss is counted after
source bytes; there is no setting to keep metadata; documented in `README.md`. it is served or fails, with the bytes it fetched from upstream, so
`upstream_fetch_count` and `upstream_fetch_bytes` move, including for a
fetched source that then fails the magic byte check; `transform_count`
counts each image the image processor transcodes.
- 2026-09-28 rate limit the login form (closes #66): `POST /` is limited to 5 - 2026-09-28 rate limit the login form (closes #66): `POST /` is limited to 5
attempts per minute per client address, and an attempt over the limit is attempts per minute per client address, and an attempt over the limit is
refused with 429 and a `Retry-After` header; the address is the one refused with 429 and a `Retry-After` header; the address is the one
@@ -257,6 +260,7 @@ exhaustion
# Future Steps # Future Steps
- P1: strip EXIF and other metadata from processed images (privacy)
- P2: security - P2: security
- referer blacklist - referer blacklist
- per-IP rate limiting on the image routes - per-IP rate limiting on the image routes
-22
View File
@@ -161,13 +161,6 @@ func (p *ImageProcessor) Process(
} }
defer img.Close() defer img.Close()
// Turn the image upright now: encode strips the EXIF orientation tag,
// and sizes below must be worked out on the upright image.
err = img.AutoRotate()
if err != nil {
return nil, fmt.Errorf("failed to auto-rotate: %w", err)
}
// Get original dimensions // Get original dimensions
origWidth := img.Width() origWidth := img.Width()
origHeight := img.Height() origHeight := img.Height()
@@ -411,21 +404,6 @@ func (p *ImageProcessor) encode(
return nil, fmt.Errorf("%w: %s", ErrUnsupportedOutputFormat, format) return nil, fmt.Errorf("%w: %s", ErrUnsupportedOutputFormat, format)
} }
// Stripping drops the ICC profile as well, and clients show an image
// with no profile as sRGB, so convert to sRGB first. "srgb" names
// libvips' built-in profile; govips' own sRGB path variable is set on
// first use but read without a lock, so concurrent requests race on it.
if img.HasICCProfile() {
err := img.TransformICCProfileWithFallback("srgb", "srgb")
if err != nil {
return nil, fmt.Errorf("failed to convert to sRGB: %w", err)
}
}
// Drop EXIF, XMP, IPTC and the ICC profile. govips ignores this for
// GIF, which carries none of them.
params.StripMetadata = true
output, _, err := img.Export(&params) output, _, err := img.Export(&params)
if err != nil { if err != nil {
return nil, err return nil, err
@@ -9,9 +9,7 @@ import (
"image/jpeg" "image/jpeg"
"image/png" "image/png"
"io" "io"
"math"
"os" "os"
"slices"
"testing" "testing"
"github.com/davidbyttow/govips/v2/vips" "github.com/davidbyttow/govips/v2/vips"
@@ -563,152 +561,3 @@ func TestImageProcessor_EncodeAVIF(t *testing.T) {
encodeAndCheck(t, FormatAVIF, 85, mimeAVIF) encodeAndCheck(t, FormatAVIF, 85, mimeAVIF)
} }
// processAndDecode runs input through Process and decodes the output with
// vips, so a test can inspect the image a client would receive.
func processAndDecode(t *testing.T, input []byte, req *Request) *vips.ImageRef {
t.Helper()
result, err := New(Params{}).Process(
context.Background(), bytes.NewReader(input), req,
)
if err != nil {
t.Fatalf("Process() error = %v", err)
}
defer func() { _ = result.Content.Close() }()
data, err := io.ReadAll(result.Content)
if err != nil {
t.Fatalf("failed to read result: %v", err)
}
output, err := vips.NewImageFromBuffer(data)
if err != nil {
t.Fatalf("failed to decode output: %v", err)
}
t.Cleanup(output.Close)
return output
}
func TestImageProcessor_StripsEXIF(t *testing.T) {
t.Parallel()
// gps-exif.jpg carries GPS coordinates, a camera make, model and serial
// number, and a capture time.
input, err := os.ReadFile("testdata/gps-exif.jpg")
if err != nil {
t.Fatalf("failed to read test JPEG: %v", err)
}
fixture, err := vips.NewImageFromBuffer(input)
if err != nil {
t.Fatalf("failed to decode test JPEG: %v", err)
}
t.Cleanup(fixture.Close)
if !slices.Contains(fixture.GetFields(), "exif-ifd3-GPSLatitude") {
t.Fatal("testdata/gps-exif.jpg has no GPS latitude")
}
formats := []Format{
FormatJPEG, FormatPNG, FormatWebP, FormatAVIF, FormatGIF, FormatOriginal,
}
for _, format := range formats {
t.Run(string(format), func(t *testing.T) {
t.Parallel()
output := processAndDecode(t, input, &Request{Format: format})
if output.HasExif() {
t.Errorf("output has EXIF: %v", output.GetExif())
}
})
}
}
func TestImageProcessor_AppliesEXIFOrientation(t *testing.T) {
t.Parallel()
// orientation-6.jpg is stored 16x8, red on the left and blue on the
// right, with EXIF orientation 6 (turn 90 degrees clockwise to view).
// Upright it is 8x16, red on top and blue below.
input, err := os.ReadFile("testdata/orientation-6.jpg")
if err != nil {
t.Fatalf("failed to read test JPEG: %v", err)
}
tests := []struct {
name string
size Size
wantW int
wantH int
}{
{name: "original size", size: Size{}, wantW: 8, wantH: 16},
{name: "width only", size: Size{Width: 4}, wantW: 4, wantH: 8},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
output := processAndDecode(t, input, &Request{
Size: tt.size,
Format: FormatPNG,
})
if output.Width() != tt.wantW || output.Height() != tt.wantH {
t.Fatalf("output is %dx%d, want %dx%d",
output.Width(), output.Height(), tt.wantW, tt.wantH)
}
top, err := output.GetPoint(tt.wantW/2, 0)
if err != nil {
t.Fatalf("GetPoint() error = %v", err)
}
bottom, err := output.GetPoint(tt.wantW/2, tt.wantH-1)
if err != nil {
t.Fatalf("GetPoint() error = %v", err)
}
if top[0] <= top[2] || bottom[2] <= bottom[0] {
t.Errorf("top pixel = %v, bottom pixel = %v, want red above blue",
top, bottom)
}
})
}
}
func TestImageProcessor_ConvertsWideGamutToSRGB(t *testing.T) {
t.Parallel()
// display-p3.jpg is a flat 8x8 image with the Display P3 profile
// embedded, filled with Display P3 (234, 51, 35), which is sRGB red.
input, err := os.ReadFile("testdata/display-p3.jpg")
if err != nil {
t.Fatalf("failed to read test JPEG: %v", err)
}
output := processAndDecode(t, input, &Request{Format: FormatPNG})
if output.HasICCProfile() {
t.Error("output has an ICC profile")
}
pixel, err := output.GetPoint(4, 4)
if err != nil {
t.Fatalf("GetPoint() error = %v", err)
}
want := []float64{255, 0, 0}
for i := range want {
if math.Abs(pixel[i]-want[i]) > 5 {
t.Fatalf("pixel = %v, want within 5 of %v", pixel, want)
}
}
}
Binary file not shown.

Before

Width:  |  Height:  |  Size: 1.3 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 1.0 KiB

Binary file not shown.

Before

Width:  |  Height:  |  Size: 811 B

+19 -7
View File
@@ -419,17 +419,16 @@ func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
return nil, fmt.Errorf("failed to get cache stats: %w", err) return nil, fmt.Errorf("failed to get cache stats: %w", err)
} }
// Get actual item count and total size from content tables // Count and size the cached source images and processed variants
err = c.db.QueryRowContext(ctx, err = c.db.QueryRowContext(ctx, `
`SELECT COUNT(*) FROM request_cache`, SELECT (SELECT COUNT(*) FROM source_content)
).Scan(&stats.TotalItems) + (SELECT COUNT(*) FROM variant_content)
`).Scan(&stats.TotalItems)
if err != nil { if err != nil {
c.log.Warn("failed to count cache items for stats", "error", err) c.log.Warn("failed to count cache items for stats", "error", err)
} }
err = c.db.QueryRowContext(ctx, stats.TotalSizeBytes, err = c.UsageBytes(ctx)
`SELECT COALESCE(SUM(size_bytes), 0) FROM output_content`,
).Scan(&stats.TotalSizeBytes)
if err != nil { if err != nil {
c.log.Warn("failed to sum cache size for stats", "error", err) c.log.Warn("failed to sum cache size for stats", "error", err)
} }
@@ -481,6 +480,19 @@ func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64)
} }
} }
// IncrementTransformCount counts one image transcoded by the image processor.
func (c *Cache) IncrementTransformCount(ctx context.Context) {
_, err := c.db.ExecContext(ctx, `
UPDATE cache_stats
SET transform_count = transform_count + 1,
last_updated_at = CURRENT_TIMESTAMP
WHERE id = 1
`)
if err != nil {
c.log.Warn("failed to count transform", "error", err)
}
}
// writeMetadataSidecar writes the JSON metadata sidecar of a stored source. // writeMetadataSidecar writes the JSON metadata sidecar of a stored source.
// A failure is logged and is otherwise non-fatal; the metadata is in the // A failure is logged and is otherwise non-fatal; the metadata is in the
// database. // database.
+4 -2
View File
@@ -162,9 +162,11 @@ type ImageCache interface {
// CacheStats contains cache statistics // CacheStats contains cache statistics
type CacheStats struct { type CacheStats struct {
// TotalItems is the number of cached items // TotalItems is the number of cached source images plus processed
// variants
TotalItems int64 TotalItems int64
// TotalSizeBytes is the total size of cached content // TotalSizeBytes is the total size of cached source images and
// processed variants
TotalSizeBytes int64 TotalSizeBytes int64
// HitCount is the number of cache hits // HitCount is the number of cache hits
HitCount int64 HitCount int64
+24 -18
View File
@@ -155,12 +155,14 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
} }
} }
// Cache miss - check if we have source content cached // Cache miss - process the cached source or fetch it, then count the
// miss with the bytes it fetched from upstream, also when it failed
cacheKey := CacheKey(req) cacheKey := CacheKey(req)
s.cache.IncrementStats(ctx, false, 0) response, fetchedBytes, err := s.processFromSourceOrFetch(ctx, req, cacheKey)
s.cache.IncrementStats(ctx, false, fetchedBytes)
response, err := s.processFromSourceOrFetch(ctx, req, cacheKey)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -268,12 +270,13 @@ func (s *Service) loadCachedSource(contentHash ContentHash) []byte {
} }
// processFromSourceOrFetch processes an image, using cached source content // processFromSourceOrFetch processes an image, using cached source content
// if available. // if available. It also returns the number of bytes fetched from upstream,
// as fetchAndProcess does, or 0 when the cached source was used.
func (s *Service) processFromSourceOrFetch( func (s *Service) processFromSourceOrFetch(
ctx context.Context, ctx context.Context,
req *ImageRequest, req *ImageRequest,
cacheKey VariantKey, cacheKey VariantKey,
) (*ImageResponse, error) { ) (*ImageResponse, int64, error) {
// Check if we have cached source content // Check if we have cached source content
contentHash, _, err := s.cache.LookupSource(ctx, req) contentHash, _, err := s.cache.LookupSource(ctx, req)
if err != nil { if err != nil {
@@ -292,26 +295,25 @@ func (s *Service) processFromSourceOrFetch(
// Fetch from upstream if we don't have source data or it's empty // Fetch from upstream if we don't have source data or it's empty
if len(sourceData) == 0 { if len(sourceData) == 0 {
resp, err := s.fetchAndProcess(ctx, req, cacheKey) return s.fetchAndProcess(ctx, req, cacheKey)
if err != nil {
return nil, err
} }
return resp, nil // Process using cached source; nothing was fetched from upstream
}
// Process using cached source
fetchBytes = int64(len(sourceData)) fetchBytes = int64(len(sourceData))
return s.processAndStore(ctx, req, cacheKey, sourceData, fetchBytes) resp, err := s.processAndStore(ctx, req, cacheKey, sourceData, fetchBytes)
return resp, 0, err
} }
// fetchAndProcess fetches from upstream, processes, and caches the result. // fetchAndProcess fetches from upstream, processes, and caches the result.
// It also returns the number of bytes fetched from upstream once the
// response has been read, including when a later step fails.
func (s *Service) fetchAndProcess( func (s *Service) fetchAndProcess(
ctx context.Context, ctx context.Context,
req *ImageRequest, req *ImageRequest,
cacheKey VariantKey, cacheKey VariantKey,
) (*ImageResponse, error) { ) (*ImageResponse, int64, error) {
// Fetch from upstream // Fetch from upstream
sourceURL := req.SourceURL() sourceURL := req.SourceURL()
@@ -330,7 +332,7 @@ func (s *Service) fetchAndProcess(
} }
} }
return nil, fmt.Errorf("upstream fetch failed: %w", err) return nil, 0, fmt.Errorf("upstream fetch failed: %w", err)
} }
defer func() { _ = fetchResult.Content.Close() }() defer func() { _ = fetchResult.Content.Close() }()
@@ -338,7 +340,7 @@ func (s *Service) fetchAndProcess(
// Read and validate the source content // Read and validate the source content
sourceData, err := io.ReadAll(fetchResult.Content) sourceData, err := io.ReadAll(fetchResult.Content)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to read upstream response: %w", err) return nil, 0, fmt.Errorf("failed to read upstream response: %w", err)
} }
// Calculate download bitrate // Calculate download bitrate
@@ -368,7 +370,7 @@ func (s *Service) fetchAndProcess(
// Validate magic bytes match content type // Validate magic bytes match content type
err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType) err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType)
if err != nil { if err != nil {
return nil, fmt.Errorf("content validation failed: %w", err) return nil, fetchBytes, fmt.Errorf("content validation failed: %w", err)
} }
// Store source content // Store source content
@@ -378,7 +380,9 @@ func (s *Service) fetchAndProcess(
// Continue even if caching fails // Continue even if caching fails
} }
return s.processAndStore(ctx, req, cacheKey, sourceData, fetchBytes) resp, err := s.processAndStore(ctx, req, cacheKey, sourceData, fetchBytes)
return resp, fetchBytes, err
} }
// processAndStore processes an image and stores the result. // processAndStore processes an image and stores the result.
@@ -406,6 +410,8 @@ func (s *Service) processAndStore(
processDuration := time.Since(processStart) processDuration := time.Since(processStart)
s.cache.IncrementTransformCount(ctx)
// Read processed content // Read processed content
processedData, err := io.ReadAll(processResult.Content) processedData, err := io.ReadAll(processResult.Content)
_ = processResult.Content.Close() _ = processResult.Content.Close()
+127 -1
View File
@@ -4,6 +4,7 @@ import (
"bytes" "bytes"
"context" "context"
"database/sql" "database/sql"
"io/fs"
"log/slog" "log/slog"
"math" "math"
"strings" "strings"
@@ -125,7 +126,7 @@ func TestStats_LogsFailedCountQueries(t *testing.T) {
} }
_, err = db.ExecContext(t.Context(), _, err = db.ExecContext(t.Context(),
`DROP TABLE request_cache; DROP TABLE output_content`) `DROP TABLE source_content; DROP TABLE variant_content`)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -183,3 +184,128 @@ func TestIncrementStats_LogsFailedUpdates(t *testing.T) {
} }
} }
} }
// TestStats_TotalsCountSourcesAndVariants verifies that TotalItems and
// TotalSizeBytes cover the stored source images and processed variants.
func TestStats_TotalsCountSourcesAndVariants(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
storeEvictionTestSource(t, cache, testHostCDN, testPathCat,
bytes.Repeat([]byte{0xAA}, 1000))
storeEvictionTestVariant(t, cache, testVariantKeyOne,
bytes.Repeat([]byte{0xAB}, 500))
storeEvictionTestVariant(t, cache, testVariantKeyTwo,
bytes.Repeat([]byte{0xAC}, 250))
stats, err := cache.Stats(t.Context())
if err != nil {
t.Fatalf("Stats() error = %v", err)
}
if stats.TotalItems != 3 {
t.Errorf("TotalItems = %d, want 3 (1 source, 2 variants)", stats.TotalItems)
}
if stats.TotalSizeBytes != 1750 {
t.Errorf("TotalSizeBytes = %d, want 1750 (1000+500+250)",
stats.TotalSizeBytes)
}
}
// cacheStatsCounters holds the counters of the cache_stats row, in column
// order.
type cacheStatsCounters struct {
hitCount int64
missCount int64
upstreamFetchCount int64
upstreamFetchBytes int64
transformCount int64
}
// readCacheStatsCounters reads the counters of the cache_stats row.
func readCacheStatsCounters(t *testing.T, cache *Cache) cacheStatsCounters {
t.Helper()
var got cacheStatsCounters
err := cache.db.QueryRowContext(t.Context(), `
SELECT hit_count, miss_count, upstream_fetch_count,
upstream_fetch_bytes, transform_count
FROM cache_stats WHERE id = 1
`).Scan(&got.hitCount, &got.missCount, &got.upstreamFetchCount,
&got.upstreamFetchBytes, &got.transformCount)
if err != nil {
t.Fatalf("failed to read cache_stats: %v", err)
}
return got
}
// TestService_Get_CountsStats walks Get through a miss that fetches the
// source, a hit, a miss that reuses the cached source, and two misses whose
// source cannot be used, checking every cache_stats counter after each.
func TestService_Get_CountsStats(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
// NewTestFS builds the same files the test service's fetcher serves.
testFS, _ := NewTestFS(t)
photo, err := fs.ReadFile(testFS, fixtures.GoodHostJPEG)
if err != nil {
t.Fatal(err)
}
fake, err := fs.ReadFile(testFS, fixtures.InvalidFile)
if err != nil {
t.Fatal(err)
}
photoBytes, fakeBytes := int64(len(photo)), int64(len(fake))
// want is hits, misses, upstream fetches, upstream bytes, transforms.
steps := []struct {
name string
path string
size int
wantErr bool
want cacheStatsCounters
}{
{"miss that fetches the source", testPathPhoto, 50, false,
cacheStatsCounters{0, 1, 1, photoBytes, 1}},
{"hit", testPathPhoto, 50, false,
cacheStatsCounters{1, 1, 1, photoBytes, 1}},
{"miss that reuses the cached source", testPathPhoto, 25, false,
cacheStatsCounters{1, 2, 1, photoBytes, 2}},
{"miss whose source fails the magic byte check", "/images/fake.jpg", 50, true,
cacheStatsCounters{1, 3, 2, photoBytes + fakeBytes, 2}},
{"miss whose source is not found", "/images/nonexistent.jpg", 50, true,
cacheStatsCounters{1, 4, 2, photoBytes + fakeBytes, 2}},
}
for _, step := range steps {
resp, err := svc.Get(t.Context(), &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: step.path,
Size: Size{Width: step.size, Height: step.size},
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
})
if (err != nil) != step.wantErr {
t.Fatalf("%s: Get() error = %v, want error %t", step.name, err, step.wantErr)
}
if err == nil {
_ = resp.Content.Close()
}
got := readCacheStatsCounters(t, svc.cache)
if got != step.want {
t.Fatalf("after the %s: counters = %+v, want %+v", step.name, got, step.want)
}
}
}