1 Commits
Author SHA1 Message Date
sneak 03b11e9703 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, content_type
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.

StoreSource no longer takes the fetch result and LookupSource returns only
the source key. 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:32:49 +00:00
14 changed files with 45 additions and 71 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; a source image's row also holds its holding its size and when it was last used. Two source URLs that serve the same
content type. Two source URLs that serve the same bytes are stored twice, once bytes are stored twice, once under each URL. A source image is stored by writing
under each URL. A source image is stored by writing its file and then its row; its file and then its row; when the row cannot be written, the file is removed
when the row cannot be written, the file is removed again. 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
+7 -6
View File
@@ -32,12 +32,13 @@ 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 and `fetched_at` of are the columns nothing read: the host, path, query, `content_type` and
`source_content`, the `content_type` and `created_at` of `variant_content`, `fetched_at` of `source_content`, the `content_type` and `created_at` of
and the `status_code`, `error_message` and `fetched_at` of `negative_cache`. `variant_content`, and the `status_code`, `error_message` and `fetched_at` of
`StoreVariant` no longer takes a content type, `StoreNegative` no longer takes `negative_cache`. `StoreSource` no longer takes the fetch result,
a status code or message, and reconciliation adopts a variant file without `LookupSource` returns only the source key, `StoreVariant` no longer takes a
reading its first bytes. 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,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, content_type, size_bytes) VALUES (?, 'image/png', 1)`, key) (source_key, size_bytes) VALUES (?, 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,7 +6,6 @@
-- 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
); );
+15 -18
View File
@@ -12,7 +12,6 @@ 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"
) )
@@ -212,7 +211,6 @@ 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
@@ -231,13 +229,12 @@ func (c *Cache) StoreSource(
} }
_, err = c.db.ExecContext(ctx, ` _, err = c.db.ExecContext(ctx, `
INSERT INTO source_content (source_key, content_type, size_bytes) INSERT INTO source_content (source_key, 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), result.ContentType, size) `, string(key), size)
if err != nil { if err != nil {
deleteErr := c.srcContent.Delete(key) deleteErr := c.srcContent.Delete(key)
if deleteErr != nil { if deleteErr != nil {
@@ -287,41 +284,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 and content type if found, or empty values // Returns the source key if found, or an empty key if not; a row whose
// if not; a row whose file is missing is not found. A source found has // file is missing is not found. A source found has its access time
// its access time handed to the background writer without waiting (see // 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, string, error) { ) (imagerequest.SourceKey, error) {
if c.disabled { if c.disabled {
return "", "", nil return "", nil
} }
key := req.Source.Key() key := req.Source.Key()
var contentType string var foundKey string
err := c.db.QueryRowContext(ctx, ` err := c.db.QueryRowContext(ctx, `
SELECT content_type FROM source_content WHERE source_key = ? SELECT source_key FROM source_content WHERE source_key = ?
`, string(key)).Scan(&contentType) `, string(key)).Scan(&foundKey)
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, contentType, nil return key, 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,7 +22,6 @@ 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
); );
+5 -23
View File
@@ -16,7 +16,6 @@ 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"
) )
@@ -102,17 +101,8 @@ 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), result, context.Background(), req, bytes.NewReader(content),
) )
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)
@@ -443,14 +433,7 @@ 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)
} }
result := &httpfetcher.FetchResult{ key, err := cache.StoreSource(ctx, req, bytes.NewReader([]byte("data")))
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)
} }
@@ -469,14 +452,13 @@ func assertDisabledCacheReadsAlwaysMiss(
ctx := t.Context() ctx := t.Context()
key, srcType, err := cache.LookupSource(ctx, req) key, 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 != "" || srcType != "" { if key != "" {
t.Errorf("LookupSource on disabled cache = (%q, %q), want empty", t.Errorf("LookupSource on disabled cache = %q, want empty", key)
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), fetchResult) _, err = s.cache.StoreSource(ctx, req, bytes.NewReader(sourceData))
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,7 +7,6 @@ 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"
) )
@@ -85,10 +84,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, contentType, err := cache.LookupSource(t.Context(), &imagerequest.Request{ kept, 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, %q, error %v", kept, contentType, err) t.Logf("LookupSource() of the kept source = %q, error %v", kept, 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)
@@ -128,10 +127,8 @@ 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")), result) bytes.NewReader([]byte("source bytes")))
t.Logf("StoreSource() = %q, error = %v", key, err) t.Logf("StoreSource() = %q, error = %v", key, err)
if err == nil { if err == nil {
+4 -5
View File
@@ -20,11 +20,10 @@ 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.