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, so
TestReconciliationReadsAPageAtATime now counts 15 rows read, not 20.
Two source URLs serving the same bytes store them twice. The tests of
the removed sharing and lock go with them; the two eviction
cancellation tests no longer use the hook.

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)
 	}
 }