Declare only the tables and columns pixa writes and reads (closes #310)
check / check (push) Canceled after 0s

Removes output_content and request_cache, which nothing writes, and every
column that was written but never read: the host, path, query and
fetched_at of source_content, the content_type and created_at of
variant_content, and the status_code, error_message and fetched_at of
negative_cache.

StoreVariant no longer takes a content type and StoreNegative no longer
takes a status code or message, so extractStatusCode and the status
constants, which only fed status_code, are gone. Reconciliation adopts a
variant file without opening it, as its row no longer records a content
type. Tests that set or checked a removed column follow.

Model: opus-5-5
This commit is contained in:
2026-10-21 16:22:28 +00:00
parent 5f9d624d74
commit a6eba6045e
17 changed files with 78 additions and 226 deletions
+4 -4
View File
@@ -188,10 +188,10 @@ allowlisted source hosts for open access.
name. name.
Each source image and each transformed image has one row in the database, Each source image and each transformed image has one row in the database,
holding its size and when it was last used. Two source URLs that serve the same holding its size and when it was last used; a source image's row also holds its
bytes are stored twice, once under each URL. A source image is stored by writing content type. Two source URLs that serve the same bytes are stored twice, once
its file and then its row; when the row cannot be written, the file is removed under each URL. A source image is stored by writing its file and then its row;
again. when the row cannot be written, the file is removed again.
Source and transformed image files are written under a temporary name, synced to Source and transformed image files are written under a temporary name, synced to
disk, and only then renamed into place, so a crash leaves no file under its own disk, and only then renamed into place, so a crash leaves no file under its own
+8
View File
@@ -30,6 +30,14 @@ P2: security: per-origin rate limiting
# Completed Steps # Completed Steps
- 2026-10-21 the schema file declares only the tables and columns pixa writes
and reads (closes #310): `output_content` and `request_cache` are gone, and so
are the columns nothing read: the host, path, query and `fetched_at` of
`source_content`, the `content_type` and `created_at` of `variant_content`,
and the `status_code`, `error_message` and `fetched_at` of `negative_cache`.
`StoreVariant` no longer takes a content type, `StoreNegative` no longer takes
a status code or message, and reconciliation adopts a variant file without
reading its first bytes.
- 2026-10-21 a cache hit makes no database call (closes #309): `Service.Get` - 2026-10-21 a cache hit makes no database call (closes #309): `Service.Get`
opens the variant file with `Cache.GetVariant` and nothing else, with no opens the variant file with `Cache.GetVariant` and nothing else, with no
existence check first. The content type comes from the file's first bytes; a existence check first. The content type comes from the file's first bytes; a
@@ -131,14 +131,13 @@ func writeLikeOneRequest(ctx context.Context, db *sql.DB, key string) error {
} }
_, err = db.ExecContext(ctx, `INSERT INTO source_content _, err = db.ExecContext(ctx, `INSERT INTO source_content
(source_key, source_host, source_path, content_type, size_bytes) (source_key, content_type, size_bytes) VALUES (?, 'image/png', 1)`, key)
VALUES (?, 'example.com', '/image.png', 'image/png', 1)`, key)
if err != nil { if err != nil {
return fmt.Errorf("storing a source: %w", err) return fmt.Errorf("storing a source: %w", err)
} }
_, err = db.ExecContext(ctx, `INSERT INTO variant_content _, err = db.ExecContext(ctx, `INSERT INTO variant_content
(cache_key, size_bytes, content_type) VALUES (?, 1, 'image/png')`, key) (cache_key, size_bytes) VALUES (?, 1)`, key)
if err != nil { if err != nil {
return fmt.Errorf("recording a transformed image: %w", err) return fmt.Errorf("recording a transformed image: %w", err)
} }
+1 -4
View File
@@ -159,10 +159,7 @@ func TestApplyMigrations_CreatesSchemaAndTables(t *testing.T) {
} }
// Verify that the application tables created by 001.sql exist. // Verify that the application tables created by 001.sql exist.
tables := []string{ tables := []string{"source_content", "negative_cache", "cache_stats"}
"source_content", "output_content",
"request_cache", "negative_cache", "cache_stats",
}
for _, table := range tables { for _, table := range tables {
var count int var count int
+3 -41
View File
@@ -6,12 +6,8 @@
-- is the SHA-256 of the source's host, path and query -- is the SHA-256 of the source's host, path and query
CREATE TABLE IF NOT EXISTS source_content ( CREATE TABLE IF NOT EXISTS source_content (
source_key TEXT PRIMARY KEY, source_key TEXT PRIMARY KEY,
source_host TEXT NOT NULL,
source_path TEXT NOT NULL,
source_query TEXT NOT NULL DEFAULT '',
content_type TEXT NOT NULL, content_type TEXT NOT NULL,
size_bytes INTEGER NOT NULL, size_bytes INTEGER NOT NULL,
fetched_at DATETIME DEFAULT CURRENT_TIMESTAMP,
last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP
); );
CREATE INDEX IF NOT EXISTS idx_source_content_last_accessed CREATE INDEX IF NOT EXISTS idx_source_content_last_accessed
@@ -27,8 +23,6 @@ CREATE INDEX IF NOT EXISTS idx_source_content_last_accessed
CREATE TABLE IF NOT EXISTS variant_content ( CREATE TABLE IF NOT EXISTS variant_content (
cache_key TEXT PRIMARY KEY, cache_key TEXT PRIMARY KEY,
size_bytes INTEGER NOT NULL, size_bytes INTEGER NOT NULL,
content_type TEXT NOT NULL,
created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP
); );
CREATE INDEX IF NOT EXISTS idx_variant_content_last_accessed CREATE INDEX IF NOT EXISTS idx_variant_content_last_accessed
@@ -102,46 +96,14 @@ BEGIN
WHERE id = 1; WHERE id = 1;
END; END;
-- Output/transformed content blobs -- Negative cache: source URLs whose fetch failed with an upstream error
-- Not written: transformed images are stored in cache/variants and -- status, or whose first bytes did not match their Content-Type. pixa does
-- tracked in variant_content above. -- not fetch such a URL again until its row expires.
CREATE TABLE IF NOT EXISTS output_content (
content_hash TEXT PRIMARY KEY,
content_type TEXT NOT NULL,
size_bytes INTEGER NOT NULL,
fetched_at DATETIME DEFAULT CURRENT_TIMESTAMP
);
-- Request cache - maps full request params to output content
CREATE TABLE IF NOT EXISTS request_cache (
id INTEGER PRIMARY KEY AUTOINCREMENT,
cache_key TEXT NOT NULL UNIQUE,
source_metadata_id INTEGER NOT NULL,
output_hash TEXT NOT NULL,
width INTEGER NOT NULL,
height INTEGER NOT NULL,
format TEXT NOT NULL,
quality INTEGER NOT NULL DEFAULT 85,
fit_mode TEXT NOT NULL DEFAULT 'cover',
fetched_at DATETIME DEFAULT CURRENT_TIMESTAMP,
access_count INTEGER NOT NULL DEFAULT 1,
FOREIGN KEY (source_metadata_id) REFERENCES source_metadata(id),
FOREIGN KEY (output_hash) REFERENCES output_content(content_hash)
);
CREATE INDEX IF NOT EXISTS idx_request_cache_key ON request_cache(cache_key);
CREATE INDEX IF NOT EXISTS idx_request_cache_source ON request_cache(source_metadata_id);
CREATE INDEX IF NOT EXISTS idx_request_cache_output ON request_cache(output_hash);
CREATE INDEX IF NOT EXISTS idx_request_cache_fetched ON request_cache(fetched_at);
-- Negative cache for failed fetches (404s, timeouts, etc.)
CREATE TABLE IF NOT EXISTS negative_cache ( CREATE TABLE IF NOT EXISTS negative_cache (
id INTEGER PRIMARY KEY AUTOINCREMENT, id INTEGER PRIMARY KEY AUTOINCREMENT,
source_host TEXT NOT NULL, source_host TEXT NOT NULL,
source_path TEXT NOT NULL, source_path TEXT NOT NULL,
source_query TEXT NOT NULL DEFAULT '', source_query TEXT NOT NULL DEFAULT '',
status_code INTEGER NOT NULL,
error_message TEXT,
fetched_at DATETIME DEFAULT CURRENT_TIMESTAMP,
expires_at DATETIME NOT NULL, expires_at DATETIME NOT NULL,
UNIQUE(source_host, source_path, source_query) UNIQUE(source_host, source_path, source_query)
); );
+11 -23
View File
@@ -231,17 +231,13 @@ func (c *Cache) StoreSource(
} }
_, err = c.db.ExecContext(ctx, ` _, err = c.db.ExecContext(ctx, `
INSERT INTO source_content INSERT INTO source_content (source_key, content_type, size_bytes)
(source_key, source_host, source_path, source_query, VALUES (?, ?, ?)
content_type, size_bytes)
VALUES (?, ?, ?, ?, ?, ?)
ON CONFLICT(source_key) DO UPDATE SET ON CONFLICT(source_key) DO UPDATE SET
content_type = excluded.content_type, content_type = excluded.content_type,
size_bytes = excluded.size_bytes, size_bytes = excluded.size_bytes,
fetched_at = CURRENT_TIMESTAMP,
last_accessed_at = CURRENT_TIMESTAMP last_accessed_at = CURRENT_TIMESTAMP
`, string(key), req.Source.Host, req.Source.Path, req.Source.Query, `, string(key), result.ContentType, size)
result.ContentType, size)
if err != nil { if err != nil {
deleteErr := c.srcContent.Delete(key) deleteErr := c.srcContent.Delete(key)
if deleteErr != nil { if deleteErr != nil {
@@ -262,8 +258,7 @@ func (c *Cache) StoreSource(
// accounting insert is best-effort (the startup reconciliation pass // accounting insert is best-effort (the startup reconciliation pass
// adopts any variant file that misses its accounting row). // adopts any variant file that misses its accounting row).
func (c *Cache) StoreVariant( func (c *Cache) StoreVariant(
ctx context.Context, cacheKey imagerequest.VariantKey, ctx context.Context, cacheKey imagerequest.VariantKey, content io.Reader,
content io.Reader, contentType string,
) error { ) error {
if c.disabled { if c.disabled {
return nil return nil
@@ -275,13 +270,12 @@ func (c *Cache) StoreVariant(
} }
_, err = c.db.ExecContext(ctx, ` _, err = c.db.ExecContext(ctx, `
INSERT INTO variant_content (cache_key, size_bytes, content_type) INSERT INTO variant_content (cache_key, size_bytes)
VALUES (?, ?, ?) VALUES (?, ?)
ON CONFLICT(cache_key) DO UPDATE SET ON CONFLICT(cache_key) DO UPDATE SET
size_bytes = excluded.size_bytes, size_bytes = excluded.size_bytes,
content_type = excluded.content_type,
last_accessed_at = CURRENT_TIMESTAMP last_accessed_at = CURRENT_TIMESTAMP
`, string(cacheKey), size, contentType) `, string(cacheKey), size)
if err != nil { if err != nil {
c.log.Warn("failed to record variant in size accounting", c.log.Warn("failed to record variant in size accounting",
"cache_key", cacheKey, "error", err) "cache_key", cacheKey, "error", err)
@@ -331,22 +325,16 @@ func (c *Cache) LookupSource(
} }
// StoreNegative stores a negative cache entry for a failed fetch. // StoreNegative stores a negative cache entry for a failed fetch.
func (c *Cache) StoreNegative( func (c *Cache) StoreNegative(ctx context.Context, req *imagerequest.Request) error {
ctx context.Context, req *imagerequest.Request, statusCode int, errMsg string,
) error {
expiresAt := time.Now().UTC().Add(c.config.NegativeTTL) expiresAt := time.Now().UTC().Add(c.config.NegativeTTL)
_, err := c.db.ExecContext(ctx, ` _, err := c.db.ExecContext(ctx, `
INSERT INTO negative_cache INSERT INTO negative_cache
(source_host, source_path, source_query, status_code, (source_host, source_path, source_query, expires_at)
error_message, expires_at) VALUES (?, ?, ?, ?)
VALUES (?, ?, ?, ?, ?, ?)
ON CONFLICT(source_host, source_path, source_query) DO UPDATE SET ON CONFLICT(source_host, source_path, source_query) DO UPDATE SET
status_code = excluded.status_code,
error_message = excluded.error_message,
fetched_at = CURRENT_TIMESTAMP,
expires_at = excluded.expires_at expires_at = excluded.expires_at
`, req.Source.Host, req.Source.Path, req.Source.Query, statusCode, errMsg, expiresAt) `, req.Source.Host, req.Source.Path, req.Source.Query, expiresAt)
if err != nil { if err != nil {
return fmt.Errorf("failed to insert negative cache: %w", err) return fmt.Errorf("failed to insert negative cache: %w", err)
} }
+3 -4
View File
@@ -16,9 +16,8 @@ import (
) )
// TestGetVariant_ContentTypeFromFirstBytes stores an image in each output // TestGetVariant_ContentTypeFromFirstBytes stores an image in each output
// format as a variant, recorded with the content type text/plain, and checks // format as a variant and checks that GetVariant serves it whole with the
// that GetVariant serves it whole with the content type of its format, read // content type of its format, read from the file.
// from the file.
func TestGetVariant_ContentTypeFromFirstBytes(t *testing.T) { func TestGetVariant_ContentTypeFromFirstBytes(t *testing.T) {
t.Parallel() t.Parallel()
@@ -48,7 +47,7 @@ func TestGetVariant_ContentTypeFromFirstBytes(t *testing.T) {
key := imagerequest.VariantKey("aabbccdd10" + string(format)) key := imagerequest.VariantKey("aabbccdd10" + string(format))
err = cache.StoreVariant(t.Context(), key, bytes.NewReader(image), "text/plain") err = cache.StoreVariant(t.Context(), key, bytes.NewReader(image))
if err != nil { if err != nil {
t.Fatalf("StoreVariant(%s) error = %v", format, err) t.Fatalf("StoreVariant(%s) error = %v", format, err)
} }
+2 -28
View File
@@ -22,41 +22,15 @@ func setupTestDB(t *testing.T) *sql.DB {
schema := ` schema := `
CREATE TABLE source_content ( CREATE TABLE source_content (
source_key TEXT PRIMARY KEY, source_key TEXT PRIMARY KEY,
source_host TEXT NOT NULL,
source_path TEXT NOT NULL,
source_query TEXT NOT NULL DEFAULT '',
content_type TEXT NOT NULL, content_type TEXT NOT NULL,
size_bytes INTEGER NOT NULL, size_bytes INTEGER NOT NULL,
fetched_at DATETIME DEFAULT CURRENT_TIMESTAMP,
last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP
); );
CREATE TABLE output_content (
content_hash TEXT PRIMARY KEY,
content_type TEXT NOT NULL,
size_bytes INTEGER NOT NULL,
fetched_at DATETIME DEFAULT CURRENT_TIMESTAMP
);
CREATE TABLE request_cache (
id INTEGER PRIMARY KEY AUTOINCREMENT,
cache_key TEXT NOT NULL UNIQUE,
source_metadata_id INTEGER NOT NULL,
output_hash TEXT NOT NULL,
width INTEGER NOT NULL,
height INTEGER NOT NULL,
format TEXT NOT NULL,
quality INTEGER NOT NULL DEFAULT 85,
fit_mode TEXT NOT NULL DEFAULT 'cover',
fetched_at DATETIME DEFAULT CURRENT_TIMESTAMP,
access_count INTEGER NOT NULL DEFAULT 1
);
CREATE TABLE negative_cache ( CREATE TABLE negative_cache (
id INTEGER PRIMARY KEY AUTOINCREMENT, id INTEGER PRIMARY KEY AUTOINCREMENT,
source_host TEXT NOT NULL, source_host TEXT NOT NULL,
source_path TEXT NOT NULL, source_path TEXT NOT NULL,
source_query TEXT NOT NULL DEFAULT '', source_query TEXT NOT NULL DEFAULT '',
status_code INTEGER NOT NULL,
error_message TEXT,
fetched_at DATETIME DEFAULT CURRENT_TIMESTAMP,
expires_at DATETIME NOT NULL, expires_at DATETIME NOT NULL,
UNIQUE(source_host, source_path, source_query) UNIQUE(source_host, source_path, source_query)
); );
@@ -138,8 +112,8 @@ func TestCache_CleanExpired(t *testing.T) {
// Insert expired negative cache entry directly // Insert expired negative cache entry directly
_, err := db.ExecContext(ctx, ` _, err := db.ExecContext(ctx, `
INSERT INTO negative_cache INSERT INTO negative_cache
(source_host, source_path, source_query, status_code, expires_at) (source_host, source_path, source_query, expires_at)
VALUES ('example.com', '/old.jpg', '', 404, datetime('now', '-1 hour')) VALUES ('example.com', '/old.jpg', '', datetime('now', '-1 hour'))
`) `)
if err != nil { if err != nil {
t.Fatalf("failed to insert test data: %v", err) t.Fatalf("failed to insert test data: %v", err)
+2 -3
View File
@@ -39,9 +39,8 @@ func TestComputeDefaultMaxBytesCountsWhatTheCacheHolds(t *testing.T) {
// The cache now holds those 3 GiB, which leaves 1 GiB free. // The cache now holds those 3 GiB, which leaves 1 GiB free.
_, err = cache.db.ExecContext(t.Context(), _, err = cache.db.ExecContext(t.Context(),
`INSERT INTO variant_content (cache_key, size_bytes, content_type) `INSERT INTO variant_content (cache_key, size_bytes) VALUES (?, ?)`,
VALUES (?, ?, ?)`, string(testVariantKeyOne), 3221225472,
string(testVariantKeyOne), 3221225472, testContentTypeWebP,
) )
if err != nil { if err != nil {
t.Fatalf("failed to insert variant accounting row: %v", err) t.Fatalf("failed to insert variant accounting row: %v", err)
+5 -19
View File
@@ -51,11 +51,6 @@ const sqliteTimestampLayout = "2006-01-02 15:04:05"
// writes (".tmp-*" patterns in the storage layer). // writes (".tmp-*" patterns in the storage layer).
const tempFilePrefix = ".tmp-" const tempFilePrefix = ".tmp-"
// fallbackContentType is the content type reconciliation records for a
// variant file it adopts whose first bytes are not those of an output
// format. Such a file is never served.
const fallbackContentType = "application/octet-stream"
// UsageBytes returns the total number of bytes of cache content // UsageBytes returns the total number of bytes of cache content
// tracked in the database (source content blobs plus processed // tracked in the database (source content blobs plus processed
// variants). It reads the total the database keeps up to date as rows // variants). It reads the total the database keeps up to date as rows
@@ -590,25 +585,16 @@ func (c *Cache) reconcileVariantFile(
} }
// adoptVariantFile inserts an accounting row for a variant file that // adoptVariantFile inserts an accounting row for a variant file that
// has none, using the file's size and modification time, and the content // has none, using the file's size and modification time.
// type its first bytes show, as a hit reads it.
func (c *Cache) adoptVariantFile( func (c *Cache) adoptVariantFile(
ctx context.Context, info fs.FileInfo, cacheKey imagerequest.VariantKey, ctx context.Context, info fs.FileInfo, cacheKey imagerequest.VariantKey,
) error { ) error {
modTime := info.ModTime().UTC().Format(sqliteTimestampLayout) modTime := info.ModTime().UTC().Format(sqliteTimestampLayout)
contentType := fallbackContentType
content, _, detectedType, err := c.variants.LoadWithContentType(cacheKey) _, err := c.db.ExecContext(ctx, `
if err == nil { INSERT INTO variant_content (cache_key, size_bytes, last_accessed_at)
_ = content.Close() VALUES (?, ?, ?)
contentType = detectedType `, string(cacheKey), info.Size(), modTime)
}
_, err = c.db.ExecContext(ctx, `
INSERT INTO variant_content
(cache_key, size_bytes, content_type, created_at, last_accessed_at)
VALUES (?, ?, ?, ?, ?)
`, string(cacheKey), info.Size(), contentType, modTime, modTime)
if err != nil { if err != nil {
return fmt.Errorf("failed to adopt variant file into accounting: %w", err) return fmt.Errorf("failed to adopt variant file into accounting: %w", err)
} }
+4 -5
View File
@@ -128,7 +128,7 @@ func storeEvictionTestVariant(
) { ) {
t.Helper() t.Helper()
err := cache.StoreVariant(t.Context(), key, bytes.NewReader(content), "image/webp") err := cache.StoreVariant(t.Context(), key, bytes.NewReader(content))
if err != nil { if err != nil {
t.Fatalf("StoreVariant(%s) failed: %v", key, err) t.Fatalf("StoreVariant(%s) failed: %v", key, err)
} }
@@ -437,7 +437,7 @@ func assertDisabledCacheWritesAreNoOps(
ctx := t.Context() ctx := t.Context()
err := cache.StoreVariant( err := cache.StoreVariant(
ctx, imagerequest.CacheKey(req), bytes.NewReader([]byte("data")), "image/webp", ctx, imagerequest.CacheKey(req), bytes.NewReader([]byte("data")),
) )
if err != nil { if err != nil {
t.Fatalf("StoreVariant on disabled cache must be a no-op, got error: %v", err) t.Fatalf("StoreVariant on disabled cache must be a no-op, got error: %v", err)
@@ -667,9 +667,8 @@ func TestStartEvictionReconcilesAccountingWithDisk(t *testing.T) {
// An accounting row whose file is missing must be dropped. // An accounting row whose file is missing must be dropped.
_, err = cache.db.ExecContext(t.Context(), _, err = cache.db.ExecContext(t.Context(),
`INSERT INTO variant_content (cache_key, size_bytes, content_type) `INSERT INTO variant_content (cache_key, size_bytes) VALUES (?, ?)`,
VALUES (?, ?, ?)`, "deadbeef0001", 700,
"deadbeef0001", 700, "image/webp",
) )
if err != nil { if err != nil {
t.Fatalf("failed to insert stale variant accounting row: %v", err) t.Fatalf("failed to insert stale variant accounting row: %v", err)
@@ -43,7 +43,7 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) {
} }
// Store a negative entry // Store a negative entry
err = cache.StoreNegative(ctx, req, 404, "not found") err = cache.StoreNegative(ctx, req)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -82,7 +82,7 @@ func TestNegativeCache_Expired(t *testing.T) {
} }
// Store a negative entry with very short TTL // Store a negative entry with very short TTL
err = cache.StoreNegative(ctx, req, 500, "server error") err = cache.StoreNegative(ctx, req)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -120,8 +120,7 @@ func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) {
_ = resp.Content.Close() _ = resp.Content.Close()
err = svc.cache.StoreNegative(t.Context(), err = svc.cache.StoreNegative(t.Context(),
photoVariant(fixtures, 85, imagerequest.FitCover), photoVariant(fixtures, 85, imagerequest.FitCover))
httpStatusBadGateway, "upstream answered 503")
if err != nil { if err != nil {
t.Fatalf("StoreNegative() error = %v", err) t.Fatalf("StoreNegative() error = %v", err)
} }
@@ -201,7 +200,7 @@ func TestEvictionLoopRemovesExpiredNegativeCacheRows(t *testing.T) {
Source: imagerequest.Source{Host: testHostExample, Path: "/gone.jpg"}, Source: imagerequest.Source{Host: testHostExample, Path: "/gone.jpg"},
} }
err = cache.StoreNegative(t.Context(), expired, 404, "not found") err = cache.StoreNegative(t.Context(), expired)
if err != nil { if err != nil {
t.Fatalf("StoreNegative failed: %v", err) t.Fatalf("StoreNegative failed: %v", err)
} }
@@ -210,8 +209,8 @@ func TestEvictionLoopRemovesExpiredNegativeCacheRows(t *testing.T) {
_, err = cache.db.ExecContext(t.Context(), ` _, err = cache.db.ExecContext(t.Context(), `
INSERT INTO negative_cache INSERT INTO negative_cache
(source_host, source_path, status_code, expires_at) (source_host, source_path, expires_at)
VALUES (?, ?, 404, datetime('now', '+1 hour')) VALUES (?, ?, datetime('now', '+1 hour'))
`, testHostExample, notExpiredPath) `, testHostExample, notExpiredPath)
if err != nil { if err != nil {
t.Fatalf("failed to insert a row that has not expired: %v", err) t.Fatalf("failed to insert a row that has not expired: %v", err)
@@ -279,8 +278,8 @@ func TestCleanExpiredDeletesInBatches(t *testing.T) {
_, err = cache.db.ExecContext(ctx, ` _, err = cache.db.ExecContext(ctx, `
INSERT INTO negative_cache INSERT INTO negative_cache
(source_host, source_path, status_code, expires_at) (source_host, source_path, expires_at)
VALUES (?, ?, 404, datetime('now', '+1 hour')) VALUES (?, ?, datetime('now', '+1 hour'))
`, testHostExample, notExpiredPath) `, testHostExample, notExpiredPath)
if err != nil { if err != nil {
t.Fatalf("failed to insert a row that has not expired: %v", err) t.Fatalf("failed to insert a row that has not expired: %v", err)
@@ -378,8 +377,8 @@ func insertExpiredNegativeCacheRows(t *testing.T, cache *Cache, count int) {
for i := range count { for i := range count {
_, err := cache.db.ExecContext(t.Context(), ` _, err := cache.db.ExecContext(t.Context(), `
INSERT INTO negative_cache INSERT INTO negative_cache
(source_host, source_path, status_code, expires_at) (source_host, source_path, expires_at)
VALUES (?, ?, 404, datetime('now', '-1 hour')) VALUES (?, ?, datetime('now', '-1 hour'))
`, testHostExample, "/expired/"+strconv.Itoa(i)+".jpg") `, testHostExample, "/expired/"+strconv.Itoa(i)+".jpg")
if err != nil { if err != nil {
t.Fatalf("failed to insert an expired row: %v", err) t.Fatalf("failed to insert an expired row: %v", err)
+9 -30
View File
@@ -458,7 +458,7 @@ func (s *Service) fetchAndProcess(
if err != nil { if err != nil {
// Store negative cache for certain errors // Store negative cache for certain errors
if isNegativeCacheable(err) { if isNegativeCacheable(err) {
s.storeNegative(ctx, req, extractStatusCode(err), err) s.storeNegative(ctx, req)
} }
return nil, fmt.Errorf("upstream fetch failed: %w", err) return nil, fmt.Errorf("upstream fetch failed: %w", err)
@@ -512,7 +512,7 @@ func (s *Service) fetchAndProcess(
s.log.Warn("source refused", "request_id", middleware.GetReqID(ctx), s.log.Warn("source refused", "request_id", middleware.GetReqID(ctx),
"host", req.Source.Host, "path", req.Source.Path, "host", req.Source.Host, "path", req.Source.Path,
"content_type", fetchResult.ContentType, "error", err) "content_type", fetchResult.ContentType, "error", err)
s.storeNegative(ctx, req, httpStatusUnsupportedMediaType, err) s.storeNegative(ctx, req)
return nil, fmt.Errorf("content validation failed: %w", err) return nil, fmt.Errorf("content validation failed: %w", err)
} }
@@ -530,15 +530,13 @@ func (s *Service) fetchAndProcess(
} }
// storeNegative stores a negative cache entry for req's source, whose fetch // storeNegative stores a negative cache entry for req's source, whose fetch
// failed with err, so the source is not fetched again until the entry // failed, so the source is not fetched again until the entry expires. A
// expires. A failure to store the entry is only logged. // failure to store the entry is only logged.
func (s *Service) storeNegative( func (s *Service) storeNegative(ctx context.Context, req *imagerequest.Request) {
ctx context.Context, req *imagerequest.Request, statusCode int, err error, err := s.cache.StoreNegative(ctx, req)
) { if err != nil {
storeErr := s.cache.StoreNegative(ctx, req, statusCode, err.Error())
if storeErr != nil {
s.log.Warn("failed to store negative cache entry", s.log.Warn("failed to store negative cache entry",
"host", req.Source.Host, "path", req.Source.Path, "error", storeErr) "host", req.Source.Host, "path", req.Source.Path, "error", err)
} }
} }
@@ -614,9 +612,7 @@ func (s *Service) processAndStore(
) )
// Store variant to cache // Store variant to cache
err = s.cache.StoreVariant( err = s.cache.StoreVariant(ctx, cacheKey, bytes.NewReader(processedData))
ctx, cacheKey, bytes.NewReader(processedData), processResult.ContentType,
)
if err != nil { if err != nil {
s.log.Warn("failed to store variant", "error", err) s.log.Warn("failed to store variant", "error", err)
// Continue even if caching fails // Continue even if caching fails
@@ -629,28 +625,11 @@ func (s *Service) processAndStore(
}, nil }, nil
} }
// HTTP status codes for error responses.
const (
httpStatusBadGateway = 502
httpStatusInternalError = 500
httpStatusUnsupportedMediaType = 415
)
// isNegativeCacheable returns true if the error should be cached. // isNegativeCacheable returns true if the error should be cached.
func isNegativeCacheable(err error) bool { func isNegativeCacheable(err error) bool {
return errors.Is(err, httpfetcher.ErrUpstreamError) return errors.Is(err, httpfetcher.ErrUpstreamError)
} }
// extractStatusCode extracts HTTP status code from error message.
func extractStatusCode(err error) int {
// Default to 502 Bad Gateway for upstream errors
if errors.Is(err, httpfetcher.ErrUpstreamError) {
return httpStatusBadGateway
}
return httpStatusInternalError
}
// etagHashLength is the number of hash characters to use for ETags. // etagHashLength is the number of hash characters to use for ETags.
const etagHashLength = 16 const etagHashLength = 16
+6 -32
View File
@@ -549,17 +549,15 @@ func TestService_Get_Caching(t *testing.T) {
// TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo caches a source and // TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo caches a source and
// a variant of it and sets back two days every time pixa stored for them. // a variant of it and sets back two days every time pixa stored for them.
// For the source: fetched_at and last_accessed_at in source_content, and the // For the source: last_accessed_at in source_content, and the access and
// access and modification times of its file. For the variant: created_at and // modification times of its file. For the variant: last_accessed_at in
// last_accessed_at in variant_content, and the access and modification times // variant_content, and the access and modification times of its file. It
// of its file. It then checks that the variant is still served from the // then checks that the variant is still served from the cache and that a new
// cache and that a new variant is made from the cached source without // variant is made from the cached source without fetching it again.
// fetching it again.
// //
// Left at the present: the files' status change and birth times, which a // Left at the present: the files' status change and birth times, which a
// program cannot set, and the times of the directories holding the files, // program cannot set, and the times of the directories holding the files,
// which other entries share. The request_cache and output_content tables are // which other entries share.
// never written.
func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) { func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) {
t.Parallel() t.Parallel()
@@ -613,11 +611,6 @@ func backdateSource(
t.Helper() t.Helper()
key := req.Source.Key() key := req.Source.Key()
dbTime := when.UTC().Format(sqliteTimestampFormat)
updateOneRow(t, cache,
`UPDATE source_content SET fetched_at = ? WHERE source_key = ?`,
dbTime, string(key))
setSourceLastAccessed(t, cache, key, when) setSourceLastAccessed(t, cache, key, when)
path := cache.srcContent.keyToPath(key) path := cache.srcContent.keyToPath(key)
@@ -639,9 +632,6 @@ func backdateVariant(
key := imagerequest.CacheKey(req) key := imagerequest.CacheKey(req)
path := cache.variants.keyToPath(key) path := cache.variants.keyToPath(key)
updateOneRow(t, cache,
`UPDATE variant_content SET created_at = ? WHERE cache_key = ?`,
when.UTC().Format(sqliteTimestampFormat), string(key))
setVariantLastAccessed(t, cache, key, when) setVariantLastAccessed(t, cache, key, when)
err := os.Chtimes(path, when, when) err := os.Chtimes(path, when, when)
@@ -650,22 +640,6 @@ func backdateVariant(
} }
} }
// updateOneRow runs the UPDATE statement query and fails the test unless it
// changed exactly one row.
func updateOneRow(t *testing.T, cache *Cache, query string, args ...any) {
t.Helper()
result, err := cache.db.ExecContext(t.Context(), query, args...)
if err != nil {
t.Fatalf("%s: %v", query, err)
}
rows, err := result.RowsAffected()
if err != nil || rows != 1 {
t.Fatalf("%s: rows = %d, error = %v, want 1 row", query, rows, err)
}
}
func TestService_Get_DifferentSizes(t *testing.T) { func TestService_Get_DifferentSizes(t *testing.T) {
t.Parallel() t.Parallel()
@@ -32,10 +32,8 @@ func TestStoreSource_StoresIdenticalBytesOfTwoURLsTwice(t *testing.T) {
} }
for sourcePath, key := range keys { for sourcePath, key := range keys {
rows := countRows(t, cache, ` rows := countRows(t, cache,
SELECT COUNT(*) FROM source_content `SELECT COUNT(*) FROM source_content WHERE source_key = ?`, string(key))
WHERE source_key = ? AND source_host = ? AND source_path = ?
`, string(key), testHostCDN, sourcePath)
if rows != 1 { if rows != 1 {
t.Errorf("%s has %d source_content rows, want 1", sourcePath, rows) t.Errorf("%s has %d source_content rows, want 1", sourcePath, rows)
} }
@@ -24,7 +24,6 @@ const (
testHostExample = "example.com" testHostExample = "example.com"
testPathCat = "/photos/cat.jpg" testPathCat = "/photos/cat.jpg"
testContentTypeJPEG = "image/jpeg" testContentTypeJPEG = "image/jpeg"
testContentTypeWebP = "image/webp"
testHeaderContentType = "Content-Type" testHeaderContentType = "Content-Type"
) )
+6 -14
View File
@@ -113,7 +113,7 @@ func TestImageProxyFlow(t *testing.T) {
} }
checkSourceCached(t, db, stateDir, source) checkSourceCached(t, db, stateDir, source)
checkVariantCached(t, db, stateDir, first.Body.Bytes(), tt.contentType) checkVariantCached(t, db, stateDir, first.Body.Bytes())
}) })
} }
} }
@@ -276,27 +276,19 @@ func checkSourceCached(t *testing.T, db *sql.DB, stateDir string, source []byte)
} }
// checkVariantCached checks that the converted image served is recorded in // checkVariantCached checks that the converted image served is recorded in
// variant_content with contentType, and stored under its cache key in // variant_content, and stored under its cache key in cache/variants.
// cache/variants. func checkVariantCached(t *testing.T, db *sql.DB, stateDir string, served []byte) {
func checkVariantCached(
t *testing.T, db *sql.DB, stateDir string, served []byte, contentType string,
) {
t.Helper() t.Helper()
var cacheKey, storedType string var cacheKey string
err := db.QueryRowContext(t.Context(), err := db.QueryRowContext(t.Context(),
"SELECT cache_key, content_type FROM variant_content", "SELECT cache_key FROM variant_content",
).Scan(&cacheKey, &storedType) ).Scan(&cacheKey)
if err != nil { if err != nil {
t.Fatalf("variant_content row: %v", err) t.Fatalf("variant_content row: %v", err)
} }
if storedType != contentType {
t.Errorf("variant_content content_type = %q, want %q",
storedType, contentType)
}
checkFile(t, filepath.Join(stateDir, "cache", "variants", checkFile(t, filepath.Join(stateDir, "cache", "variants",
cacheKey[0:2], cacheKey[2:4], cacheKey), served) cacheKey[0:2], cacheKey[2:4], cacheKey), served)
} }