1 Commits
Author SHA1 Message Date
sneak a6eba6045e 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
2026-10-21 16:22:28 +00:00
14 changed files with 71 additions and 45 deletions

No files matched your search

+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
+6 -7
View File
@@ -32,13 +32,12 @@ P2: security: per-origin rate limiting
- 2026-10-21 the schema file declares only the tables and columns pixa writes - 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 and reads (closes #310): `output_content` and `request_cache` are gone, and so
are the columns nothing read: the host, path, query, `content_type` and are the columns nothing read: the host, path, query and `fetched_at` of
`fetched_at` of `source_content`, the `content_type` and `created_at` of `source_content`, the `content_type` and `created_at` of `variant_content`,
`variant_content`, and the `status_code`, `error_message` and `fetched_at` of and the `status_code`, `error_message` and `fetched_at` of `negative_cache`.
`negative_cache`. `StoreSource` no longer takes the fetch result, `StoreVariant` no longer takes a content type, `StoreNegative` no longer takes
`LookupSource` returns only the source key, `StoreVariant` no longer takes a a status code or message, and reconciliation adopts a variant file without
content type, `StoreNegative` no longer takes a status code or message, and reading its first bytes.
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,7 +131,7 @@ 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, size_bytes) VALUES (?, 1)`, key) (source_key, content_type, size_bytes) VALUES (?, '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)
} }
+1
View File
@@ -6,6 +6,7 @@
-- 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,
content_type TEXT NOT NULL,
size_bytes INTEGER NOT NULL, size_bytes INTEGER NOT NULL,
last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP
); );
+18 -15
View File
@@ -12,6 +12,7 @@ import (
"sync/atomic" "sync/atomic"
"time" "time"
"sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imagerequest"
) )
@@ -211,6 +212,7 @@ func (c *Cache) StoreSource(
ctx context.Context, ctx context.Context,
req *imagerequest.Request, req *imagerequest.Request,
content io.Reader, content io.Reader,
result *httpfetcher.FetchResult,
) (imagerequest.SourceKey, error) { ) (imagerequest.SourceKey, error) {
if c.disabled { if c.disabled {
return "", nil return "", nil
@@ -229,12 +231,13 @@ func (c *Cache) StoreSource(
} }
_, err = c.db.ExecContext(ctx, ` _, err = c.db.ExecContext(ctx, `
INSERT INTO source_content (source_key, size_bytes) INSERT INTO source_content (source_key, content_type, size_bytes)
VALUES (?, ?) VALUES (?, ?, ?)
ON CONFLICT(source_key) DO UPDATE SET ON CONFLICT(source_key) DO UPDATE SET
content_type = excluded.content_type,
size_bytes = excluded.size_bytes, size_bytes = excluded.size_bytes,
last_accessed_at = CURRENT_TIMESTAMP last_accessed_at = CURRENT_TIMESTAMP
`, string(key), size) `, string(key), 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 {
@@ -284,41 +287,41 @@ func (c *Cache) StoreVariant(
} }
// LookupSource checks if we have cached source content for a request. // LookupSource checks if we have cached source content for a request.
// Returns the source key if found, or an empty key if not; a row whose // Returns the source key and content type if found, or empty values
// file is missing is not found. A source found has its access time // if not; a row whose file is missing is not found. A source found has
// handed to the background writer without waiting (see // its access time handed to the background writer without waiting (see
// touchSourceContent); a disabled cache always reports no cached source. // touchSourceContent); a disabled cache always reports no cached source.
func (c *Cache) LookupSource( func (c *Cache) LookupSource(
ctx context.Context, req *imagerequest.Request, ctx context.Context, req *imagerequest.Request,
) (imagerequest.SourceKey, error) { ) (imagerequest.SourceKey, string, error) {
if c.disabled { if c.disabled {
return "", nil return "", "", nil
} }
key := req.Source.Key() key := req.Source.Key()
var foundKey string var contentType string
err := c.db.QueryRowContext(ctx, ` err := c.db.QueryRowContext(ctx, `
SELECT source_key FROM source_content WHERE source_key = ? SELECT content_type FROM source_content WHERE source_key = ?
`, string(key)).Scan(&foundKey) `, string(key)).Scan(&contentType)
if errors.Is(err, sql.ErrNoRows) { if errors.Is(err, sql.ErrNoRows) {
return "", nil return "", "", nil
} }
if err != nil { if err != nil {
return "", fmt.Errorf("failed to lookup source: %w", err) return "", "", fmt.Errorf("failed to lookup source: %w", err)
} }
// Verify the content file exists // Verify the content file exists
if !c.srcContent.Exists(key) { if !c.srcContent.Exists(key) {
return "", nil return "", "", nil
} }
c.touchSourceContent(key) c.touchSourceContent(key)
return key, nil return key, contentType, nil
} }
// StoreNegative stores a negative cache entry for a failed fetch. // StoreNegative stores a negative cache entry for a failed fetch.
+1
View File
@@ -22,6 +22,7 @@ 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,
content_type TEXT NOT NULL,
size_bytes INTEGER NOT NULL, size_bytes INTEGER NOT NULL,
last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP
); );
+23 -5
View File
@@ -16,6 +16,7 @@ import (
"modernc.org/sqlite" "modernc.org/sqlite"
"sneak.berlin/go/pixa/internal/database" "sneak.berlin/go/pixa/internal/database"
"sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imagerequest"
) )
@@ -101,8 +102,17 @@ func storeEvictionTestSource(
}, },
} }
result := &httpfetcher.FetchResult{
StatusCode: 200,
ContentType: testContentTypeJPEG,
ContentLength: int64(len(content)),
Headers: map[string][]string{
testHeaderContentType: {testContentTypeJPEG},
},
}
key, err := cache.StoreSource( key, err := cache.StoreSource(
context.Background(), req, bytes.NewReader(content), context.Background(), req, bytes.NewReader(content), result,
) )
if err != nil { if err != nil {
t.Fatalf("StoreSource(%s%s) failed: %v", host, path, err) t.Fatalf("StoreSource(%s%s) failed: %v", host, path, err)
@@ -433,7 +443,14 @@ func assertDisabledCacheWritesAreNoOps(
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)
} }
key, err := cache.StoreSource(ctx, req, bytes.NewReader([]byte("data"))) result := &httpfetcher.FetchResult{
StatusCode: 200,
ContentType: testContentTypeJPEG,
ContentLength: 4,
Headers: map[string][]string{},
}
key, err := cache.StoreSource(ctx, req, bytes.NewReader([]byte("data")), result)
if err != nil { if err != nil {
t.Fatalf("StoreSource on disabled cache must be a no-op, got error: %v", err) t.Fatalf("StoreSource on disabled cache must be a no-op, got error: %v", err)
} }
@@ -452,13 +469,14 @@ func assertDisabledCacheReadsAlwaysMiss(
ctx := t.Context() ctx := t.Context()
key, err := cache.LookupSource(ctx, req) key, srcType, err := cache.LookupSource(ctx, req)
if err != nil { if err != nil {
t.Fatalf("LookupSource on disabled cache failed: %v", err) t.Fatalf("LookupSource on disabled cache failed: %v", err)
} }
if key != "" { if key != "" || srcType != "" {
t.Errorf("LookupSource on disabled cache = %q, want empty", key) t.Errorf("LookupSource on disabled cache = (%q, %q), want empty",
key, srcType)
} }
} }
@@ -77,7 +77,7 @@ func TestService_Get_WaitsForSlotBeforeReadingCachedSource(t *testing.T) {
_ = resp.Content.Close() _ = resp.Content.Close()
sourceKey, err := svc.cache.LookupSource(t.Context(), sourceKey, _, err := svc.cache.LookupSource(t.Context(),
widthOnlyRequest(fixtures, 50)) widthOnlyRequest(fixtures, 50))
if err != nil || sourceKey == "" { if err != nil || sourceKey == "" {
t.Fatalf("LookupSource() = %q, %v; want the cached source", t.Fatalf("LookupSource() = %q, %v; want the cached source",
@@ -146,7 +146,7 @@ func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) {
_ = resp.Content.Close() _ = resp.Content.Close()
sourceKey, err := svc.cache.LookupSource(t.Context(), sourceKey, _, err := svc.cache.LookupSource(t.Context(),
photoVariant(fixtures, 85, imagerequest.FitCover)) photoVariant(fixtures, 85, imagerequest.FitCover))
if err != nil || sourceKey == "" { if err != nil || sourceKey == "" {
t.Fatalf("LookupSource() = %q, %v, want the cached source", sourceKey, err) t.Fatalf("LookupSource() = %q, %v, want the cached source", sourceKey, err)
@@ -65,7 +65,7 @@ func cachedSourcePath(
) string { ) string {
t.Helper() t.Helper()
sourceKey, err := svc.cache.LookupSource(t.Context(), req) sourceKey, _, err := svc.cache.LookupSource(t.Context(), req)
if err != nil || sourceKey == "" { if err != nil || sourceKey == "" {
t.Fatalf("LookupSource() = %q, %v; want the cached source", sourceKey, err) t.Fatalf("LookupSource() = %q, %v; want the cached source", sourceKey, err)
} }
@@ -364,7 +364,7 @@ func TestLookupSource_HandsAccessTimeToTheBackgroundWriter(t *testing.T) {
` `
anHourAgoInDatabase := anHourAgo.UTC().Format(sqliteTimestampFormat) anHourAgoInDatabase := anHourAgo.UTC().Format(sqliteTimestampFormat)
found, err := cache.LookupSource(t.Context(), &imagerequest.Request{ found, _, err := cache.LookupSource(t.Context(), &imagerequest.Request{
Source: imagerequest.Source{Host: "example.com", Path: "/photo.jpg"}, Source: imagerequest.Source{Host: "example.com", Path: "/photo.jpg"},
}) })
if err != nil || found != key { if err != nil || found != key {
+2 -2
View File
@@ -397,7 +397,7 @@ func (s *Service) processFromSourceOrFetch(
cacheKey imagerequest.VariantKey, cacheKey imagerequest.VariantKey,
) (*processedVariant, error) { ) (*processedVariant, error) {
// Check if we have cached source content // Check if we have cached source content
sourceKey, err := s.cache.LookupSource(ctx, req) sourceKey, _, err := s.cache.LookupSource(ctx, req)
if err != nil { if err != nil {
s.log.Warn("source lookup failed", "error", err) s.log.Warn("source lookup failed", "error", err)
} }
@@ -518,7 +518,7 @@ func (s *Service) fetchAndProcess(
} }
// Store source content // Store source content
_, err = s.cache.StoreSource(ctx, req, bytes.NewReader(sourceData)) _, err = s.cache.StoreSource(ctx, req, bytes.NewReader(sourceData), fetchResult)
if err != nil { if err != nil {
s.log.Warn("failed to store source content", "error", err) s.log.Warn("failed to store source content", "error", err)
// Continue even if caching fails // Continue even if caching fails
@@ -7,6 +7,7 @@ import (
"path/filepath" "path/filepath"
"testing" "testing"
"sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imagerequest" "sneak.berlin/go/pixa/internal/imagerequest"
) )
@@ -84,10 +85,10 @@ func TestEvictSourceBlob_LeavesSourceWithIdenticalBytesServed(t *testing.T) {
t.Errorf("the evicted source still has %d source_content rows", n) t.Errorf("the evicted source still has %d source_content rows", n)
} }
kept, err := cache.LookupSource(t.Context(), &imagerequest.Request{ kept, contentType, err := cache.LookupSource(t.Context(), &imagerequest.Request{
Source: imagerequest.Source{Host: testHostCDN, Path: "/kept.jpg"}, Source: imagerequest.Source{Host: testHostCDN, Path: "/kept.jpg"},
}) })
t.Logf("LookupSource() of the kept source = %q, error %v", kept, err) t.Logf("LookupSource() of the kept source = %q, %q, error %v", kept, contentType, err)
if err != nil || kept == "" { if err != nil || kept == "" {
t.Fatalf("LookupSource() of the kept source = %q, %v; want it cached", kept, err) t.Fatalf("LookupSource() of the kept source = %q, %v; want it cached", kept, err)
@@ -127,8 +128,10 @@ func TestStoreSource_FailedRowWriteLeavesNoFile(t *testing.T) {
req := &imagerequest.Request{ req := &imagerequest.Request{
Source: imagerequest.Source{Host: testHostCDN, Path: testPathCat}, Source: imagerequest.Source{Host: testHostCDN, Path: testPathCat},
} }
result := &httpfetcher.FetchResult{ContentType: testContentTypeJPEG}
key, err := cache.StoreSource(t.Context(), req, key, err := cache.StoreSource(t.Context(), req,
bytes.NewReader([]byte("source bytes"))) bytes.NewReader([]byte("source bytes")), result)
t.Logf("StoreSource() = %q, error = %v", key, err) t.Logf("StoreSource() = %q, error = %v", key, err)
if err == nil { if err == nil {
+5 -4
View File
@@ -20,10 +20,11 @@ import (
// Shared test data literals, extracted as constants for goconst. // Shared test data literals, extracted as constants for goconst.
const ( const (
testHostCDN = "cdn.example.com" testHostCDN = "cdn.example.com"
testHostExample = "example.com" testHostExample = "example.com"
testPathCat = "/photos/cat.jpg" testPathCat = "/photos/cat.jpg"
testContentTypeJPEG = "image/jpeg" testContentTypeJPEG = "image/jpeg"
testHeaderContentType = "Content-Type"
) )
// TestFixtures contains paths to test files in the mock filesystem. // TestFixtures contains paths to test files in the mock filesystem.