Store a source the way a variant is stored, one row and one file per source URL (closes #308)

A source is now one source_content row and one file named by its
SourceKey, the SHA-256 of its host, path and query, made as the variant
key is. A store writes the file, then upserts the row, and removes the
file when the row cannot be written; an eviction deletes the row, then
the file, with no lock. source_metadata, the JSON files under
cache/metadata/, MetadataStorage, the content lock, the eviction
transaction and its test hook are gone, as is the reconciliation pass
that dropped a source row whose file is missing. Two source URLs
serving the same bytes store them twice.

Model: opus-5-5

internal/imgcache/cache.go

@@ -2,10 +2,7 @@ package imgcache
 
 import (
 	"context"
-	"crypto/sha256"
 	"database/sql"
-	"encoding/hex"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -26,9 +23,6 @@ var (
 	ErrNegativeCache = errors.New("negative cache hit")
 )
 
-// HTTP status code for successful fetch.
-const httpStatusOK = 200
-
 // metaCacheSize is how many variants' content types metaCache holds. A
 // variant not among them is served as before, reading its .meta file.
 const metaCacheSize = 10000
@@ -69,12 +63,11 @@ type CacheConfig struct {
 
 // Cache implements the caching layer for the image proxy.
 type Cache struct {
-	db          *sql.DB
-	srcContent  *ContentStorage  // source images by content hash
-	variants    *VariantStorage  // processed variants by cache key
-	srcMetadata *MetadataStorage // source metadata by host/path
-	config      CacheConfig
-	log         *slog.Logger
+	db         *sql.DB
+	srcContent *ContentStorage // source images by source key
+	variants   *VariantStorage // processed variants by cache key
+	config     CacheConfig
+	log        *slog.Logger
 
 	// disabled means the disk cache is turned off entirely: lookups
 	// always miss, stores are no-ops, and no eviction runs.
@@ -114,19 +107,6 @@ type Cache struct {
 	// It never stands in for the variant file, which is always opened.
 	metaCache *lru.Cache[imagerequest.VariantKey, string]
 
-	// contentLocks serializes StoreSource and evictSourceBlob per
-	// content hash, closing the race window between an eviction's row
-	// deletion and its file unlink against a concurrent store of
-	// identical content.
-	contentLocks *contentLock
-
-	// evictSourceBlobTestHook, when set, is invoked by evictSourceBlob
-	// after its row-deletion transaction commits and before the
-	// content file is unlinked. It exists solely so tests can
-	// deterministically pause inside that window to exercise
-	// concurrent stores against it; production code leaves it nil.
-	evictSourceBlobTestHook func(ContentHash)
-
 	// reconciliationPageSize is the most rows one read of a content table
 	// returns in the reconciliation pass and in Stats. newCache sets it to
 	// defaultReconciliationPageSize; tests set it smaller.
@@ -167,7 +147,6 @@ func newCache(
 		evictionPressure: make(chan struct{}, 1),
 		evictionDone:     make(chan struct{}),
 		metaCache:        metaCache,
-		contentLocks:     newContentLock(),
 
 		pendingCountsAdded: make(chan struct{}, 1),
 		pendingCountsDone:  make(chan struct{}),
@@ -193,16 +172,8 @@ func newCache(
 		return nil, fmt.Errorf("failed to create variant storage: %w", err)
 	}
 
-	srcMetadata, err := NewMetadataStorage(
-		filepath.Join(config.StateDir, "cache", "metadata"),
-	)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create source metadata storage: %w", err)
-	}
-
 	c.srcContent = srcContent
 	c.variants = variants
-	c.srcMetadata = srcMetadata
 
 	if config.UseDefaultMaxBytes {
 		limit, err := c.computeDefaultMaxBytes(context.Background(), probe)
@@ -283,88 +254,59 @@ func (c *Cache) GetVariant(
 	return reader, size, contentType, nil
 }
 
-// StoreSource stores fetched source content and metadata. On a
-// disabled cache it is a no-op returning an empty hash.
+// StoreSource stores fetched source content: it writes the file named by
+// the source's key, then the source's row. When the row cannot be written
+// the file is removed and the store fails, as a file with no row is never
+// served or evicted; if that removal fails too, the next reconciliation
+// pass removes the file. On a disabled cache it is a no-op returning an
+// empty key.
 func (c *Cache) StoreSource(
 	ctx context.Context,
 	req *imagerequest.Request,
 	content io.Reader,
 	result *httpfetcher.FetchResult,
-) (ContentHash, error) {
+) (imagerequest.SourceKey, error) {
 	if c.disabled {
 		return "", nil
 	}
 
-	// Hash the content ourselves (rather than via srcContent.Store,
-	// which would hash internally) so the content hash is known before
-	// any file or database work happens: that lets the entire store be
-	// serialized, per hash, against a concurrent eviction of the same
-	// content below.
 	data, err := io.ReadAll(content)
 	if err != nil {
 		return "", fmt.Errorf("failed to read source content: %w", err)
 	}
 
-	sum := sha256.Sum256(data)
-	contentHash := ContentHash(hex.EncodeToString(sum[:]))
+	key := req.Source.Key()
 
-	// Hold the content hash's lock for the whole store operation. A
-	// concurrent eviction of this exact hash (the real SHA-256 dedup
-	// case: a different source path whose bytes hash identically)
-	// deletes the accounting rows and unlinks the file inside the same
-	// lock, so the two can never interleave: either this store
-	// completes first (and a subsequent eviction removes it together
-	// with its rows and file, correctly), or eviction completes first
-	// (and this store finds the file already gone and recreates it
-	// fresh) — never a fresh row left pointing at a file eviction is
-	// mid-unlink on.
-	unlock := c.contentLocks.Lock(string(contentHash))
-	defer unlock()
-
-	size, err := c.srcContent.StoreHashed(contentHash, data)
+	size, err := c.srcContent.Store(key, data)
 	if err != nil {
 		return "", fmt.Errorf("failed to store source content: %w", err)
 	}
 
-	// Store in database
-	pathHash := HashPath(req.Source.Path + "?" + req.Source.Query)
-
-	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)
-		VALUES (?, ?, ?)
-		ON CONFLICT(content_hash) DO NOTHING
-	`, contentHash, result.ContentType, size)
+		INSERT INTO source_content
+			(source_key, source_host, source_path, source_query,
+			 content_type, size_bytes)
+		VALUES (?, ?, ?, ?, ?, ?)
+		ON CONFLICT(source_key) DO UPDATE SET
+			content_type = excluded.content_type,
+			size_bytes = excluded.size_bytes,
+			fetched_at = CURRENT_TIMESTAMP,
+			last_accessed_at = CURRENT_TIMESTAMP
+	`, string(key), req.Source.Host, req.Source.Path, req.Source.Query,
+		result.ContentType, size)
 	if err != nil {
+		deleteErr := c.srcContent.Delete(key)
+		if deleteErr != nil {
+			c.log.Warn("failed to remove source file whose row was not written",
+				"source_key", key, "error", deleteErr)
+		}
+
 		return "", fmt.Errorf("failed to insert source content: %w", err)
 	}
 
-	_, err = c.db.ExecContext(ctx, `
-		INSERT INTO source_metadata
-			(source_host, source_path, source_query, path_hash,
-			 content_hash, status_code, content_type, response_headers)
-		VALUES (?, ?, ?, ?, ?, ?, ?, ?)
-		ON CONFLICT(source_host, source_path, source_query) DO UPDATE SET
-			content_hash = excluded.content_hash,
-			status_code = excluded.status_code,
-			content_type = excluded.content_type,
-			response_headers = excluded.response_headers,
-			fetched_at = CURRENT_TIMESTAMP
-	`, req.Source.Host, req.Source.Path, req.Source.Query, pathHash,
-		contentHash, httpStatusOK, result.ContentType, string(headersJSON))
-	if err != nil {
-		return "", fmt.Errorf("failed to insert source metadata: %w", err)
-	}
-
-	c.writeMetadataSidecar(req, pathHash, contentHash, result)
-
 	c.notifyWritePressure()
 
-	return contentHash, nil
+	return key, nil
 }
 
 // StoreVariant stores a processed variant by its cache key and records
@@ -405,22 +347,24 @@ func (c *Cache) StoreVariant(
 }
 
 // LookupSource checks if we have cached source content for a request.
-// 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.
+// Returns the source key and content type if found, or empty values
+// if not; a row whose file is missing is not found. Hits touch the
+// source's LRU timestamp; a disabled cache always reports no cached
+// source.
 func (c *Cache) LookupSource(
 	ctx context.Context, req *imagerequest.Request,
-) (ContentHash, string, error) {
+) (imagerequest.SourceKey, string, error) {
 	if c.disabled {
 		return "", "", nil
 	}
 
-	var hashStr, contentType string
+	key := req.Source.Key()
+
+	var contentType string
 
 	err := c.db.QueryRowContext(ctx, `
-		SELECT content_hash, content_type FROM source_metadata
-		WHERE source_host = ? AND source_path = ? AND source_query = ?
-	`, req.Source.Host, req.Source.Path, req.Source.Query).Scan(&hashStr, &contentType)
+		SELECT content_type FROM source_content WHERE source_key = ?
+	`, string(key)).Scan(&contentType)
 
 	if errors.Is(err, sql.ErrNoRows) {
 		return "", "", nil
@@ -430,16 +374,14 @@ func (c *Cache) LookupSource(
 		return "", "", fmt.Errorf("failed to lookup source: %w", err)
 	}
 
-	contentHash := ContentHash(hashStr)
-
 	// Verify the content file exists
-	if !c.srcContent.Exists(contentHash) {
+	if !c.srcContent.Exists(key) {
 		return "", "", nil
 	}
 
-	c.touchSourceContent(ctx, contentHash)
+	c.touchSourceContent(ctx, key)
 
-	return contentHash, contentType, nil
+	return key, contentType, nil
 }
 
 // StoreNegative stores a negative cache entry for a failed fetch.
@@ -466,33 +408,16 @@ func (c *Cache) StoreNegative(
 	return nil
 }
 
-// GetSourceMetadataID returns the source metadata ID for a request.
-func (c *Cache) GetSourceMetadataID(
-	ctx context.Context, req *imagerequest.Request,
-) (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.Source.Host, req.Source.Path, req.Source.Query).Scan(&id)
-	if err != nil {
-		return 0, fmt.Errorf("failed to get source metadata ID: %w", err)
-	}
-
-	return id, nil
-}
-
-// GetSourceContent returns a reader for cached source content by its hash,
+// GetSourceContent returns a reader for cached source content by its key,
 // and the content's size in bytes.
 func (c *Cache) GetSourceContent(
-	contentHash ContentHash,
+	key imagerequest.SourceKey,
 ) (io.ReadCloser, int64, error) {
 	if c.disabled {
 		return nil, 0, ErrNotFound
 	}
 
-	return c.srcContent.LoadWithSize(contentHash)
+	return c.srcContent.LoadWithSize(key)
 }
 
 // CleanExpired removes the negative cache rows past their expiry, one
@@ -687,36 +612,6 @@ func (c *Cache) loadVariantWithMeta(
 	return reader, size, contentType, nil
 }
 
-// writeMetadataSidecar writes the JSON metadata sidecar of a stored source.
-// A failure is logged and is otherwise non-fatal; the metadata is in the
-// database.
-func (c *Cache) writeMetadataSidecar(
-	req *imagerequest.Request,
-	pathHash PathHash,
-	contentHash ContentHash,
-	result *httpfetcher.FetchResult,
-) {
-	meta := &SourceMetadata{
-		Host:            req.Source.Host,
-		Path:            req.Source.Path,
-		Query:           req.Source.Query,
-		ContentHash:     string(contentHash),
-		StatusCode:      result.StatusCode,
-		ContentType:     result.ContentType,
-		ContentLength:   result.ContentLength,
-		ResponseHeaders: result.Headers,
-		FetchedAt:       time.Now().UTC().Unix(),
-		FetchDurationMs: result.FetchDurationMs,
-		RemoteAddr:      result.RemoteAddr,
-	}
-
-	err := c.srcMetadata.Store(req.Source.Host, pathHash, meta)
-	if err != nil {
-		c.log.Warn("failed to write metadata sidecar",
-			"host", req.Source.Host, "path_hash", pathHash, "error", err)
-	}
-}
-
 // 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 imagerequest.VariantKey) {
@@ -730,16 +625,16 @@ func (c *Cache) touchVariant(ctx context.Context, cacheKey imagerequest.VariantK
 	}
 }
 
-// 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) {
+// touchSourceContent updates the LRU timestamp of a cached source,
+// best-effort: a failed touch only makes the source look colder.
+func (c *Cache) touchSourceContent(ctx context.Context, key imagerequest.SourceKey) {
 	_, err := c.db.ExecContext(ctx, `
 		UPDATE source_content SET last_accessed_at = CURRENT_TIMESTAMP
-		WHERE content_hash = ?
-	`, string(contentHash))
+		WHERE source_key = ?
+	`, string(key))
 	if err != nil {
 		c.log.Debug("failed to touch source content LRU timestamp",
-			"content_hash", contentHash, "error", err)
+			"source_key", key, "error", err)
 	}
 }