From be293906bcc3d83af3afdb331489f17f239b9a30 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 8 Jan 2026 16:55:20 -0800 Subject: [PATCH] Add type-safe hash types for cache storage Define ContentHash, VariantKey, and PathHash types to replace raw strings, providing compile-time type safety for storage operations. Update storage layer to use typed parameters, refactor cache to use variant storage keyed by VariantKey, and implement source content reuse on cache misses. --- internal/handlers/handlers.go | 8 +- internal/handlers/handlers_test.go | 8 +- internal/imgcache/cache.go | 247 +++++++++-------------------- internal/imgcache/cache_test.go | 176 ++++++++------------ internal/imgcache/module.go | 5 +- internal/imgcache/service.go | 137 +++++++++++----- internal/imgcache/storage.go | 232 +++++++++++++++++++++++---- internal/imgcache/storage_test.go | 36 +++-- internal/imgcache/testutil_test.go | 8 +- 9 files changed, 476 insertions(+), 381 deletions(-) diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index 17efd44..d80ee34 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -61,11 +61,9 @@ func New(lc fx.Lifecycle, params Params) (*Handlers, error) { func (s *Handlers) initImageService() error { // Create the cache cache, err := imgcache.NewCache(s.db.DB(), imgcache.CacheConfig{ - StateDir: s.config.StateDir, - CacheTTL: imgcache.DefaultCacheTTL, - NegativeTTL: imgcache.DefaultNegativeTTL, - HotCacheSize: imgcache.DefaultHotCacheSize, - HotCacheEnabled: true, + StateDir: s.config.StateDir, + CacheTTL: imgcache.DefaultCacheTTL, + NegativeTTL: imgcache.DefaultNegativeTTL, }) if err != nil { return err diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index 6c04c19..550dff3 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -44,11 +44,9 @@ func setupTestHandler(t *testing.T) *testFixtures { db := setupTestDB(t) cache, err := imgcache.NewCache(db, imgcache.CacheConfig{ - StateDir: tmpDir, - CacheTTL: time.Hour, - NegativeTTL: 5 * time.Minute, - HotCacheSize: 100, - HotCacheEnabled: true, + StateDir: tmpDir, + CacheTTL: time.Hour, + NegativeTTL: 5 * time.Minute, }) if err != nil { t.Fatalf("failed to create cache: %v", err) diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index 1302c71..6d5d4dc 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -23,170 +23,89 @@ const httpStatusOK = 200 // CacheConfig holds cache configuration. type CacheConfig struct { - StateDir string - CacheTTL time.Duration - NegativeTTL time.Duration - HotCacheSize int - HotCacheEnabled bool + StateDir string + CacheTTL time.Duration + NegativeTTL time.Duration } -// hotCacheEntry stores all data needed to serve a cache hit without DB access. -type hotCacheEntry struct { - OutputHash string +// variantMeta stores content type for fast cache hits without reading .meta file. +type variantMeta struct { ContentType string - SizeBytes int64 + Size int64 } // Cache implements the caching layer for the image proxy. type Cache struct { - db *sql.DB - srcContent *ContentStorage - dstContent *ContentStorage - srcMetadata *MetadataStorage - config CacheConfig - hotCache map[string]hotCacheEntry // cache_key -> entry - hotCacheMu sync.RWMutex - hotCacheEnabled bool + 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 + + // In-memory cache of variant metadata (content type, size) to avoid reading .meta files + metaCache map[VariantKey]variantMeta + metaCacheMu sync.RWMutex } // NewCache creates a new cache instance. func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) { - srcContent, err := NewContentStorage(filepath.Join(config.StateDir, "cache", "src-content")) + srcContent, err := NewContentStorage(filepath.Join(config.StateDir, "cache", "sources")) if err != nil { return nil, fmt.Errorf("failed to create source content storage: %w", err) } - dstContent, err := NewContentStorage(filepath.Join(config.StateDir, "cache", "dst-content")) + variants, err := NewVariantStorage(filepath.Join(config.StateDir, "cache", "variants")) if err != nil { - return nil, fmt.Errorf("failed to create destination content storage: %w", err) + return nil, fmt.Errorf("failed to create variant storage: %w", err) } - srcMetadata, err := NewMetadataStorage(filepath.Join(config.StateDir, "cache", "src-metadata")) + 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 := &Cache{ - db: db, - srcContent: srcContent, - dstContent: dstContent, - srcMetadata: srcMetadata, - config: config, - hotCacheEnabled: config.HotCacheEnabled, - } - - if config.HotCacheEnabled && config.HotCacheSize > 0 { - c.hotCache = make(map[string]hotCacheEntry, config.HotCacheSize) - } - - return c, nil + return &Cache{ + db: db, + srcContent: srcContent, + variants: variants, + srcMetadata: srcMetadata, + config: config, + metaCache: make(map[VariantKey]variantMeta), + }, nil } // LookupResult contains the result of a cache lookup. type LookupResult struct { Hit bool - OutputHash string + CacheKey VariantKey ContentType string SizeBytes int64 CacheStatus CacheStatus } -// Lookup checks if a processed image exists in the cache. -func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, error) { +// Lookup checks if a processed variant exists on disk (no DB access for hits). +func (c *Cache) Lookup(_ context.Context, req *ImageRequest) (*LookupResult, error) { cacheKey := CacheKey(req) - // Check hot cache first - if c.hotCacheEnabled { - c.hotCacheMu.RLock() - entry, ok := c.hotCache[cacheKey] - c.hotCacheMu.RUnlock() - - if ok && c.dstContent.Exists(entry.OutputHash) { - return &LookupResult{ - Hit: true, - OutputHash: entry.OutputHash, - ContentType: entry.ContentType, - SizeBytes: entry.SizeBytes, - CacheStatus: CacheHit, - }, nil - } + // Check variant storage directly - no DB needed for cache hits + if c.variants.Exists(cacheKey) { + return &LookupResult{ + Hit: true, + CacheKey: cacheKey, + CacheStatus: CacheHit, + }, nil } - // Check negative cache - negCached, err := c.checkNegativeCache(ctx, req) - if err != nil { - return nil, err - } - - if negCached { - return nil, ErrNegativeCache - } - - // Check database - var outputHash, contentType string - var sizeBytes int64 - var fetchedAt time.Time - - err = c.db.QueryRowContext(ctx, ` - SELECT rc.output_hash, oc.content_type, oc.size_bytes, rc.fetched_at - FROM request_cache rc - JOIN output_content oc ON rc.output_hash = oc.content_hash - WHERE rc.cache_key = ? - `, cacheKey).Scan(&outputHash, &contentType, &sizeBytes, &fetchedAt) - - if errors.Is(err, sql.ErrNoRows) { - return &LookupResult{Hit: false, CacheStatus: CacheMiss}, nil - } - - if err != nil { - return nil, fmt.Errorf("failed to query cache: %w", err) - } - - // Check TTL - if c.config.CacheTTL > 0 && time.Since(fetchedAt) > c.config.CacheTTL { - return &LookupResult{Hit: false, CacheStatus: CacheStale}, nil - } - - // Verify file exists on disk - if !c.dstContent.Exists(outputHash) { - return &LookupResult{Hit: false, CacheStatus: CacheMiss}, nil - } - - // Update hot cache - if c.hotCacheEnabled { - c.hotCacheMu.Lock() - c.hotCache[cacheKey] = hotCacheEntry{ - OutputHash: outputHash, - ContentType: contentType, - SizeBytes: sizeBytes, - } - c.hotCacheMu.Unlock() - } - - // Update access count - _, _ = c.db.ExecContext(ctx, ` - UPDATE request_cache - SET access_count = access_count + 1 - WHERE cache_key = ? - `, cacheKey) - return &LookupResult{ - Hit: true, - OutputHash: outputHash, - ContentType: contentType, - SizeBytes: sizeBytes, - CacheStatus: CacheHit, + Hit: false, + CacheKey: cacheKey, + CacheStatus: CacheMiss, }, nil } -// GetOutput returns a reader for cached output content. -func (c *Cache) GetOutput(outputHash string) (io.ReadCloser, error) { - return c.dstContent.Load(outputHash) -} - -// GetOutputWithSize returns a reader and size for cached output content. -func (c *Cache) GetOutputWithSize(outputHash string) (io.ReadCloser, int64, error) { - return c.dstContent.LoadWithSize(outputHash) +// GetVariant returns a reader, size, and content type for a cached variant. +func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, error) { + return c.variants.LoadWithMeta(cacheKey) } // StoreSource stores fetched source content and metadata. @@ -195,7 +114,7 @@ func (c *Cache) StoreSource( req *ImageRequest, content io.Reader, result *FetchResult, -) (contentHash string, err error) { +) (ContentHash, error) { // Store content contentHash, size, err := c.srcContent.Store(content) if err != nil { @@ -237,12 +156,12 @@ func (c *Cache) StoreSource( Host: req.SourceHost, Path: req.SourcePath, Query: req.SourceQuery, - ContentHash: contentHash, + ContentHash: string(contentHash), StatusCode: result.StatusCode, ContentType: result.ContentType, ContentLength: result.ContentLength, ResponseHeaders: result.Headers, - FetchedAt: time.Now().Unix(), + FetchedAt: time.Now().UTC().Unix(), FetchDurationMs: result.FetchDurationMs, RemoteAddr: result.RemoteAddr, } @@ -255,61 +174,43 @@ func (c *Cache) StoreSource( return contentHash, nil } -// StoreOutput stores processed output content and returns the output hash. -func (c *Cache) StoreOutput( - ctx context.Context, - req *ImageRequest, - sourceMetadataID int64, - content io.Reader, - contentType string, -) (string, error) { - // Store content - outputHash, size, err := c.dstContent.Store(content) +// StoreVariant stores a processed variant by its cache key. +func (c *Cache) StoreVariant(cacheKey VariantKey, content io.Reader, contentType string) error { + _, err := c.variants.Store(cacheKey, content, contentType) + return err +} + +// 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. +func (c *Cache) LookupSource(ctx context.Context, req *ImageRequest) (ContentHash, string, error) { + var hashStr, 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.SourceHost, req.SourcePath, req.SourceQuery).Scan(&hashStr, &contentType) + + if errors.Is(err, sql.ErrNoRows) { + return "", "", nil + } + if err != nil { - return "", fmt.Errorf("failed to store output content: %w", err) + return "", "", fmt.Errorf("failed to lookup source: %w", err) } - cacheKey := CacheKey(req) + contentHash := ContentHash(hashStr) - // Store in database - _, err = c.db.ExecContext(ctx, ` - INSERT INTO output_content (content_hash, content_type, size_bytes) - VALUES (?, ?, ?) - ON CONFLICT(content_hash) DO NOTHING - `, outputHash, contentType, size) - if err != nil { - return "", fmt.Errorf("failed to insert output content: %w", err) + // Verify the content file exists + if !c.srcContent.Exists(contentHash) { + return "", "", nil } - _, err = c.db.ExecContext(ctx, ` - INSERT INTO request_cache (cache_key, source_metadata_id, output_hash, width, height, format, quality, fit_mode) - VALUES (?, ?, ?, ?, ?, ?, ?, ?) - ON CONFLICT(cache_key) DO UPDATE SET - output_hash = excluded.output_hash, - fetched_at = CURRENT_TIMESTAMP, - access_count = request_cache.access_count + 1 - `, cacheKey, sourceMetadataID, outputHash, req.Size.Width, req.Size.Height, req.Format, req.Quality, req.FitMode) - if err != nil { - return "", fmt.Errorf("failed to insert request cache: %w", err) - } - - // Update hot cache - if c.hotCacheEnabled { - c.hotCacheMu.Lock() - c.hotCache[cacheKey] = hotCacheEntry{ - OutputHash: outputHash, - ContentType: contentType, - SizeBytes: size, - } - c.hotCacheMu.Unlock() - } - - return outputHash, nil + return contentHash, contentType, nil } // StoreNegative stores a negative cache entry for a failed fetch. func (c *Cache) StoreNegative(ctx context.Context, req *ImageRequest, statusCode int, errMsg string) error { - expiresAt := time.Now().Add(c.config.NegativeTTL) + expiresAt := time.Now().UTC().Add(c.config.NegativeTTL) _, err := c.db.ExecContext(ctx, ` INSERT INTO negative_cache (source_host, source_path, source_query, status_code, error_message, expires_at) @@ -375,7 +276,7 @@ func (c *Cache) GetSourceMetadataID(ctx context.Context, req *ImageRequest) (int } // GetSourceContent returns a reader for cached source content by its hash. -func (c *Cache) GetSourceContent(contentHash string) (io.ReadCloser, error) { +func (c *Cache) GetSourceContent(contentHash ContentHash) (io.ReadCloser, error) { return c.srcContent.Load(contentHash) } diff --git a/internal/imgcache/cache_test.go b/internal/imgcache/cache_test.go index 388ff2e..dda75ef 100644 --- a/internal/imgcache/cache_test.go +++ b/internal/imgcache/cache_test.go @@ -99,11 +99,9 @@ func setupTestCache(t *testing.T) (*Cache, string) { db := setupTestDB(t) cache, err := NewCache(db, CacheConfig{ - StateDir: tmpDir, - CacheTTL: time.Hour, - NegativeTTL: 5 * time.Minute, - HotCacheSize: 100, - HotCacheEnabled: true, + StateDir: tmpDir, + CacheTTL: time.Hour, + NegativeTTL: 5 * time.Minute, }) if err != nil { t.Fatalf("failed to create cache: %v", err) @@ -168,17 +166,12 @@ func TestCache_StoreAndLookup(t *testing.T) { t.Error("StoreSource() returned empty hash") } - // Get source metadata ID - metaID, err := cache.GetSourceMetadataID(ctx, req) - if err != nil { - t.Fatalf("GetSourceMetadataID() error = %v", err) - } - - // Store output content + // Store variant + cacheKey := CacheKey(req) outputContent := []byte("fake webp data") - _, err = cache.StoreOutput(ctx, req, metaID, bytes.NewReader(outputContent), "image/webp") + err = cache.StoreVariant(cacheKey, bytes.NewReader(outputContent), "image/webp") if err != nil { - t.Fatalf("StoreOutput() error = %v", err) + t.Fatalf("StoreVariant() error = %v", err) } // Now lookup should hit @@ -195,8 +188,8 @@ func TestCache_StoreAndLookup(t *testing.T) { t.Errorf("Lookup() status = %v, want %v", result.CacheStatus, CacheHit) } - if result.OutputHash == "" { - t.Error("Lookup() returned empty output hash") + if result.CacheKey == "" { + t.Error("Lookup() returned empty cache key") } } @@ -217,10 +210,14 @@ func TestCache_NegativeCache(t *testing.T) { t.Fatalf("StoreNegative() error = %v", err) } - // Lookup should return negative cache error - _, err = cache.Lookup(ctx, req) - if err != ErrNegativeCache { - t.Errorf("Lookup() error = %v, want ErrNegativeCache", err) + // Lookup should return miss (negative cache is checked at service level) + result, err := cache.Lookup(ctx, req) + if err != nil { + t.Fatalf("Lookup() error = %v", err) + } + + if result.Hit { + t.Error("Lookup() hit = true, want false for negative cache entry") } } @@ -230,11 +227,9 @@ func TestCache_NegativeCacheExpiry(t *testing.T) { // Very short negative TTL for testing cache, err := NewCache(db, CacheConfig{ - StateDir: tmpDir, - CacheTTL: time.Hour, - NegativeTTL: 1 * time.Millisecond, - HotCacheSize: 100, - HotCacheEnabled: true, + StateDir: tmpDir, + CacheTTL: time.Hour, + NegativeTTL: 1 * time.Millisecond, }) if err != nil { t.Fatalf("failed to create cache: %v", err) @@ -258,7 +253,7 @@ func TestCache_NegativeCacheExpiry(t *testing.T) { // Wait for expiry time.Sleep(10 * time.Millisecond) - // Lookup should return miss (not negative cache) after expiry + // Lookup should return miss after expiry result, err := cache.Lookup(ctx, req) if err != nil { t.Fatalf("Lookup() error = %v, want nil after expiry", err) @@ -269,40 +264,28 @@ func TestCache_NegativeCacheExpiry(t *testing.T) { } } -func TestCache_HotCache(t *testing.T) { +func TestCache_VariantLookup(t *testing.T) { cache, _ := setupTestCache(t) ctx := context.Background() req := &ImageRequest{ SourceHost: "cdn.example.com", - SourcePath: "/photos/hot.jpg", + SourcePath: "/photos/variant.jpg", Size: Size{Width: 800, Height: 600}, Format: FormatWebP, Quality: 85, FitMode: FitCover, } - // Store content - sourceContent := []byte("source data") - fetchResult := &FetchResult{ - ContentType: "image/jpeg", - Headers: map[string][]string{}, - } - - _, err := cache.StoreSource(ctx, req, bytes.NewReader(sourceContent), fetchResult) - if err != nil { - t.Fatalf("StoreSource() error = %v", err) - } - - metaID, _ := cache.GetSourceMetadataID(ctx, req) - + // Store variant + cacheKey := CacheKey(req) outputContent := []byte("output data") - _, err = cache.StoreOutput(ctx, req, metaID, bytes.NewReader(outputContent), "image/webp") + err := cache.StoreVariant(cacheKey, bytes.NewReader(outputContent), "image/webp") if err != nil { - t.Fatalf("StoreOutput() error = %v", err) + t.Fatalf("StoreVariant() error = %v", err) } - // First lookup populates hot cache + // First lookup result1, err := cache.Lookup(ctx, req) if err != nil { t.Fatalf("Lookup() first error = %v", err) @@ -312,7 +295,7 @@ func TestCache_HotCache(t *testing.T) { t.Error("Lookup() first hit = false") } - // Second lookup should use hot cache + // Second lookup should also hit (from disk) result2, err := cache.Lookup(ctx, req) if err != nil { t.Fatalf("Lookup() second error = %v", err) @@ -322,66 +305,59 @@ func TestCache_HotCache(t *testing.T) { t.Error("Lookup() second hit = false") } - if result1.OutputHash != result2.OutputHash { - t.Error("Lookup() returned different hashes") + if result1.CacheKey != result2.CacheKey { + t.Error("Lookup() returned different cache keys") } } -func TestCache_HotCache_ReturnsContentType(t *testing.T) { +func TestCache_GetVariant_ReturnsContentType(t *testing.T) { cache, _ := setupTestCache(t) ctx := context.Background() req := &ImageRequest{ SourceHost: "cdn.example.com", - SourcePath: "/photos/hotct.jpg", + SourcePath: "/photos/variantct.jpg", Size: Size{Width: 800, Height: 600}, Format: FormatWebP, Quality: 85, FitMode: FitCover, } - // Store content - sourceContent := []byte("source data") - fetchResult := &FetchResult{ - ContentType: "image/jpeg", - Headers: map[string][]string{}, - } - - _, err := cache.StoreSource(ctx, req, bytes.NewReader(sourceContent), fetchResult) - if err != nil { - t.Fatalf("StoreSource() error = %v", err) - } - - metaID, _ := cache.GetSourceMetadataID(ctx, req) - + // Store variant + cacheKey := CacheKey(req) outputContent := []byte("output webp data") - _, err = cache.StoreOutput(ctx, req, metaID, bytes.NewReader(outputContent), "image/webp") + err := cache.StoreVariant(cacheKey, bytes.NewReader(outputContent), "image/webp") if err != nil { - t.Fatalf("StoreOutput() error = %v", err) + t.Fatalf("StoreVariant() error = %v", err) } - // First lookup populates hot cache - result1, err := cache.Lookup(ctx, req) + // Lookup + result, err := cache.Lookup(ctx, req) if err != nil { - t.Fatalf("Lookup() first error = %v", err) + t.Fatalf("Lookup() error = %v", err) } - if result1.ContentType != "image/webp" { - t.Errorf("Lookup() first ContentType = %q, want %q", result1.ContentType, "image/webp") + if !result.Hit { + t.Fatal("Lookup() hit = false, want true") } - // Second lookup uses hot cache - must still have ContentType - result2, err := cache.Lookup(ctx, req) + // GetVariant should return content type + reader, size, contentType, err := cache.GetVariant(result.CacheKey) if err != nil { - t.Fatalf("Lookup() second error = %v", err) + t.Fatalf("GetVariant() error = %v", err) + } + defer reader.Close() + + if contentType != "image/webp" { + t.Errorf("GetVariant() ContentType = %q, want %q", contentType, "image/webp") } - if result2.ContentType != "image/webp" { - t.Errorf("Lookup() hot cache ContentType = %q, want %q", result2.ContentType, "image/webp") + if size != int64(len(outputContent)) { + t.Errorf("GetVariant() size = %d, want %d", size, len(outputContent)) } } -func TestCache_GetOutput(t *testing.T) { +func TestCache_GetVariant(t *testing.T) { cache, _ := setupTestCache(t) ctx := context.Background() @@ -394,38 +370,24 @@ func TestCache_GetOutput(t *testing.T) { FitMode: FitCover, } - // Store content - sourceContent := []byte("source") - fetchResult := &FetchResult{ContentType: "image/jpeg", Headers: map[string][]string{}} - _, err := cache.StoreSource(ctx, req, bytes.NewReader(sourceContent), fetchResult) - if err != nil { - t.Fatalf("StoreSource() error = %v", err) - } - - metaID, err := cache.GetSourceMetadataID(ctx, req) - if err != nil { - t.Fatalf("GetSourceMetadataID() error = %v", err) - } - + // Store variant + cacheKey := CacheKey(req) outputContent := []byte("the actual output content") - outputHash, err := cache.StoreOutput(ctx, req, metaID, bytes.NewReader(outputContent), "image/webp") + err := cache.StoreVariant(cacheKey, bytes.NewReader(outputContent), "image/webp") if err != nil { - t.Fatalf("StoreOutput() error = %v", err) - } - if outputHash == "" { - t.Fatal("StoreOutput() returned empty hash") + t.Fatalf("StoreVariant() error = %v", err) } - // Lookup to get hash + // Lookup to get cache key result, err := cache.Lookup(ctx, req) if err != nil { t.Fatalf("Lookup() error = %v", err) } - // Get output content - reader, err := cache.GetOutput(result.OutputHash) + // Get variant content + reader, _, _, err := cache.GetVariant(result.CacheKey) if err != nil { - t.Fatalf("GetOutput() error = %v", err) + t.Fatalf("GetVariant() error = %v", err) } defer reader.Close() @@ -433,7 +395,7 @@ func TestCache_GetOutput(t *testing.T) { n, _ := reader.Read(buf) if !bytes.Equal(buf[:n], outputContent) { - t.Errorf("GetOutput() content = %q, want %q", buf[:n], outputContent) + t.Errorf("GetVariant() content = %q, want %q", buf[:n], outputContent) } } @@ -465,9 +427,8 @@ func TestCache_CleanExpired(t *testing.T) { db := setupTestDB(t) cache, _ := NewCache(db, CacheConfig{ - StateDir: tmpDir, - NegativeTTL: 1 * time.Millisecond, - HotCacheEnabled: false, + StateDir: tmpDir, + NegativeTTL: 1 * time.Millisecond, }) ctx := context.Background() @@ -506,8 +467,7 @@ func TestCache_StorageDirectoriesCreated(t *testing.T) { db := setupTestDB(t) _, err := NewCache(db, CacheConfig{ - StateDir: tmpDir, - HotCacheEnabled: false, + StateDir: tmpDir, }) if err != nil { t.Fatalf("NewCache() error = %v", err) @@ -515,9 +475,9 @@ func TestCache_StorageDirectoriesCreated(t *testing.T) { // Verify directories were created dirs := []string{ - "cache/src-content", - "cache/dst-content", - "cache/src-metadata", + "cache/sources", + "cache/variants", + "cache/metadata", } for _, dir := range dirs { diff --git a/internal/imgcache/module.go b/internal/imgcache/module.go index 9e045c7..266e398 100644 --- a/internal/imgcache/module.go +++ b/internal/imgcache/module.go @@ -4,7 +4,6 @@ import "time" // CacheConfig defaults. const ( - DefaultCacheTTL = 24 * time.Hour - DefaultNegativeTTL = 5 * time.Minute - DefaultHotCacheSize = 1000 + DefaultCacheTTL = 24 * time.Hour + DefaultNegativeTTL = 5 * time.Minute ) diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index c1f7ebf..d3a5c15 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -80,39 +80,36 @@ func NewService(cfg *ServiceConfig) (*Service, error) { // Get retrieves a processed image, fetching and processing if necessary. func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) { - // Check cache first + // Check variant cache first (disk only, no DB) result, err := s.cache.Lookup(ctx, req) if err != nil { - if errors.Is(err, ErrNegativeCache) { - return nil, fmt.Errorf("upstream returned error (cached)") - } - s.log.Warn("cache lookup failed", "error", err) } - // Cache hit - serve from cache + // Cache hit - serve directly from disk if result != nil && result.Hit { - s.cache.IncrementStats(ctx, true, 0) - - reader, size, err := s.cache.GetOutputWithSize(result.OutputHash) + reader, size, contentType, err := s.cache.GetVariant(result.CacheKey) if err != nil { - s.log.Error("failed to get cached output", "hash", result.OutputHash, "error", err) - // Fall through to re-fetch + s.log.Error("failed to get cached variant", "key", result.CacheKey, "error", err) + // Fall through to re-process } else { + s.cache.IncrementStats(ctx, true, 0) + return &ImageResponse{ Content: reader, ContentLength: size, - ContentType: result.ContentType, + ContentType: contentType, CacheStatus: CacheHit, - ETag: formatETag(result.OutputHash), + ETag: formatETag(result.CacheKey), }, nil } } - // Cache miss - need to fetch, process, and cache + // Cache miss - check if we have source content cached + cacheKey := CacheKey(req) s.cache.IncrementStats(ctx, false, 0) - response, err := s.fetchAndProcess(ctx, req) + response, err := s.processFromSourceOrFetch(ctx, req, cacheKey) if err != nil { return nil, err } @@ -122,8 +119,62 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e return response, nil } +// processFromSourceOrFetch processes an image, using cached source content if available. +func (s *Service) processFromSourceOrFetch( + ctx context.Context, + req *ImageRequest, + cacheKey VariantKey, +) (*ImageResponse, error) { + // Check if we have cached source content + contentHash, _, err := s.cache.LookupSource(ctx, req) + if err != nil { + s.log.Warn("source lookup failed", "error", err) + } + + var sourceData []byte + var fetchBytes int64 + + if contentHash != "" { + // We have cached source - load it + s.log.Debug("using cached source", "hash", contentHash) + + reader, err := s.cache.GetSourceContent(contentHash) + if err != nil { + s.log.Warn("failed to load cached source, fetching", "error", err) + // Fall through to fetch + } else { + sourceData, err = io.ReadAll(reader) + _ = reader.Close() + + if err != nil { + s.log.Warn("failed to read cached source, fetching", "error", err) + // Fall through to fetch + } + } + } + + // Fetch from upstream if we don't have source data + if sourceData == nil { + resp, err := s.fetchAndProcess(ctx, req, cacheKey) + if err != nil { + return nil, err + } + + return resp, nil + } + + // Process using cached source + fetchBytes = int64(len(sourceData)) + + return s.processAndStore(ctx, req, cacheKey, sourceData, fetchBytes) +} + // fetchAndProcess fetches from upstream, processes, and caches the result. -func (s *Service) fetchAndProcess(ctx context.Context, req *ImageRequest) (*ImageResponse, error) { +func (s *Service) fetchAndProcess( + ctx context.Context, + req *ImageRequest, + cacheKey VariantKey, +) (*ImageResponse, error) { // Fetch from upstream sourceURL := req.SourceURL() @@ -182,6 +233,17 @@ func (s *Service) fetchAndProcess(ctx context.Context, req *ImageRequest) (*Imag // Continue even if caching fails } + return s.processAndStore(ctx, req, cacheKey, sourceData, fetchBytes) +} + +// processAndStore processes an image and stores the result. +func (s *Service) processAndStore( + ctx context.Context, + req *ImageRequest, + cacheKey VariantKey, + sourceData []byte, + fetchBytes int64, +) (*ImageResponse, error) { // Process the image processStart := time.Now() @@ -192,8 +254,16 @@ func (s *Service) fetchAndProcess(ctx context.Context, req *ImageRequest) (*Imag processDuration := time.Since(processStart) + // Read processed content + processedData, err := io.ReadAll(processResult.Content) + _ = processResult.Content.Close() + + if err != nil { + return nil, fmt.Errorf("failed to read processed content: %w", err) + } + // Log conversion details - outputSize := processResult.ContentLength + outputSize := int64(len(processedData)) sizePercent := float64(outputSize) / float64(fetchBytes) * 100.0 //nolint:mnd // percentage calculation s.log.Info("image converted", @@ -211,30 +281,18 @@ func (s *Service) fetchAndProcess(ctx context.Context, req *ImageRequest) (*Imag "fit", req.FitMode, ) - // Store output content to cache - metaID, err := s.cache.GetSourceMetadataID(ctx, req) - if err != nil { - return nil, fmt.Errorf("failed to get source metadata ID: %w", err) - } - - outputHash, err := s.cache.StoreOutput(ctx, req, metaID, processResult.Content, processResult.ContentType) - _ = processResult.Content.Close() - if err != nil { - return nil, fmt.Errorf("failed to store output content: %w", err) - } - - // Serve from the cached file on disk (same path as cache hits) - reader, size, err := s.cache.GetOutputWithSize(outputHash) - if err != nil { - return nil, fmt.Errorf("failed to read cached output: %w", err) + // Store variant to cache + if err := s.cache.StoreVariant(cacheKey, bytes.NewReader(processedData), processResult.ContentType); err != nil { + s.log.Warn("failed to store variant", "error", err) + // Continue even if caching fails } return &ImageResponse{ - Content: reader, - ContentLength: size, + Content: io.NopCloser(bytes.NewReader(processedData)), + ContentLength: outputSize, ContentType: processResult.ContentType, - FetchedBytes: int64(len(sourceData)), - ETag: formatETag(outputHash), + FetchedBytes: fetchBytes, + ETag: formatETag(cacheKey), }, nil } @@ -309,8 +367,9 @@ func extractStatusCode(err error) int { // etagHashLength is the number of hash characters to use for ETags. const etagHashLength = 16 -// formatETag formats a hash as a quoted ETag value. -func formatETag(hash string) string { +// formatETag formats a VariantKey as a quoted ETag value. +func formatETag(key VariantKey) string { + hash := string(key) // Use first 16 characters of hash for a shorter but still unique ETag if len(hash) > etagHashLength { hash = hash[:etagHashLength] diff --git a/internal/imgcache/storage.go b/internal/imgcache/storage.go index d456f22..612fa8e 100644 --- a/internal/imgcache/storage.go +++ b/internal/imgcache/storage.go @@ -9,6 +9,7 @@ import ( "io" "os" "path/filepath" + "time" ) // Storage constants. @@ -24,6 +25,15 @@ var ( ErrNotFound = errors.New("content not found") ) +// ContentHash is a SHA256 hash of file content (hex-encoded). +type ContentHash string + +// VariantKey is a SHA256 hash identifying a specific image variant (hex-encoded). +type VariantKey string + +// PathHash is a SHA256 hash of a URL path (hex-encoded). +type PathHash string + // ContentStorage handles content-addressable file storage. // Files are stored at: /// type ContentStorage struct { @@ -41,7 +51,7 @@ func NewContentStorage(baseDir string) (*ContentStorage, error) { // Store writes content to storage and returns its SHA256 hash. // The content is read fully into memory to compute the hash before writing. -func (s *ContentStorage) Store(r io.Reader) (hash string, size int64, err error) { +func (s *ContentStorage) Store(r io.Reader) (hash ContentHash, size int64, err error) { // Read all content to compute hash data, err := io.ReadAll(r) if err != nil { @@ -50,7 +60,7 @@ func (s *ContentStorage) Store(r io.Reader) (hash string, size int64, err error) // Compute hash h := sha256.Sum256(data) - hash = hex.EncodeToString(h[:]) + hash = ContentHash(hex.EncodeToString(h[:])) size = int64(len(data)) // Build path: /// @@ -99,10 +109,10 @@ func (s *ContentStorage) Store(r io.Reader) (hash string, size int64, err error) } // Load returns a reader for the content with the given hash. -func (s *ContentStorage) Load(hash string) (io.ReadCloser, error) { +func (s *ContentStorage) Load(hash ContentHash) (io.ReadCloser, error) { path := s.hashToPath(hash) - f, err := os.Open(path) //nolint:gosec // content-addressable path from hash + f, err := os.Open(path) //nolint:gosec // path derived from content hash if err != nil { if os.IsNotExist(err) { return nil, ErrNotFound @@ -115,10 +125,10 @@ func (s *ContentStorage) Load(hash string) (io.ReadCloser, error) { } // LoadWithSize returns a reader and file size for the content with the given hash. -func (s *ContentStorage) LoadWithSize(hash string) (io.ReadCloser, int64, error) { +func (s *ContentStorage) LoadWithSize(hash ContentHash) (io.ReadCloser, int64, error) { path := s.hashToPath(hash) - f, err := os.Open(path) //nolint:gosec // content-addressable path from hash + f, err := os.Open(path) //nolint:gosec // path derived from content hash if err != nil { if os.IsNotExist(err) { return nil, 0, ErrNotFound @@ -138,7 +148,7 @@ func (s *ContentStorage) LoadWithSize(hash string) (io.ReadCloser, int64, error) } // Delete removes content with the given hash. -func (s *ContentStorage) Delete(hash string) error { +func (s *ContentStorage) Delete(hash ContentHash) error { path := s.hashToPath(hash) err := os.Remove(path) @@ -150,25 +160,21 @@ func (s *ContentStorage) Delete(hash string) error { } // Exists checks if content with the given hash exists. -func (s *ContentStorage) Exists(hash string) bool { +func (s *ContentStorage) Exists(hash ContentHash) bool { path := s.hashToPath(hash) _, err := os.Stat(path) return err == nil } -// Path returns the file path for a given hash. -func (s *ContentStorage) Path(hash string) string { - return s.hashToPath(hash) -} - // hashToPath converts a hash to a file path: /// -func (s *ContentStorage) hashToPath(hash string) string { - if len(hash) < MinHashLength { - return filepath.Join(s.baseDir, hash) +func (s *ContentStorage) hashToPath(hash ContentHash) string { + h := string(hash) + if len(h) < MinHashLength { + return filepath.Join(s.baseDir, h) } - return filepath.Join(s.baseDir, hash[0:2], hash[2:4], hash) + return filepath.Join(s.baseDir, h[0:2], h[2:4], h) } // MetadataStorage handles JSON metadata file storage. @@ -205,7 +211,7 @@ type SourceMetadata struct { } // Store writes metadata to storage. -func (s *MetadataStorage) Store(host, pathHash string, meta *SourceMetadata) error { +func (s *MetadataStorage) Store(host string, pathHash PathHash, meta *SourceMetadata) error { path := s.metaPath(host, pathHash) // Create directory structure @@ -252,7 +258,7 @@ func (s *MetadataStorage) Store(host, pathHash string, meta *SourceMetadata) err } // Load reads metadata from storage. -func (s *MetadataStorage) Load(host, pathHash string) (*SourceMetadata, error) { +func (s *MetadataStorage) Load(host string, pathHash PathHash) (*SourceMetadata, error) { path := s.metaPath(host, pathHash) data, err := os.ReadFile(path) //nolint:gosec // path derived from host+hash @@ -273,7 +279,7 @@ func (s *MetadataStorage) Load(host, pathHash string) (*SourceMetadata, error) { } // Delete removes metadata for the given host and path hash. -func (s *MetadataStorage) Delete(host, pathHash string) error { +func (s *MetadataStorage) Delete(host string, pathHash PathHash) error { path := s.metaPath(host, pathHash) err := os.Remove(path) @@ -285,7 +291,7 @@ func (s *MetadataStorage) Delete(host, pathHash string) error { } // Exists checks if metadata exists for the given host and path hash. -func (s *MetadataStorage) Exists(host, pathHash string) bool { +func (s *MetadataStorage) Exists(host string, pathHash PathHash) bool { path := s.metaPath(host, pathHash) _, err := os.Stat(path) @@ -293,20 +299,20 @@ func (s *MetadataStorage) Exists(host, pathHash string) bool { } // metaPath returns the file path for metadata: //.json -func (s *MetadataStorage) metaPath(host, pathHash string) string { - return filepath.Join(s.baseDir, host, pathHash+".json") +func (s *MetadataStorage) metaPath(host string, pathHash PathHash) string { + return filepath.Join(s.baseDir, host, string(pathHash)+".json") } // HashPath computes the SHA256 hash of a path string. -func HashPath(path string) string { +func HashPath(path string) PathHash { h := sha256.Sum256([]byte(path)) - return hex.EncodeToString(h[:]) + return PathHash(hex.EncodeToString(h[:])) } -// CacheKey generates a unique cache key for a request. +// CacheKey generates a unique key for a request variant. // Format: sha256(host:path:query:width:height:format:quality:fit_mode) -func CacheKey(req *ImageRequest) string { +func CacheKey(req *ImageRequest) VariantKey { data := fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%s", req.SourceHost, req.SourcePath, @@ -319,5 +325,175 @@ func CacheKey(req *ImageRequest) string { ) h := sha256.Sum256([]byte(data)) - return hex.EncodeToString(h[:]) + return VariantKey(hex.EncodeToString(h[:])) +} + +// VariantStorage handles key-based file storage for processed image variants. +// Files are stored at: /// +// Metadata is stored at: ///.meta +// Unlike ContentStorage, the key is provided by the caller (not computed from content). +type VariantStorage struct { + baseDir string +} + +// VariantMeta contains metadata about a cached variant. +type VariantMeta struct { + ContentType string `json:"content_type"` + Size int64 `json:"size"` + CreatedAt int64 `json:"created_at"` +} + +// NewVariantStorage creates a new variant storage at the given base directory. +func NewVariantStorage(baseDir string) (*VariantStorage, error) { + if err := os.MkdirAll(baseDir, StorageDirPerm); err != nil { + return nil, fmt.Errorf("failed to create variant storage directory: %w", err) + } + + return &VariantStorage{baseDir: baseDir}, nil +} + +// Store writes content and metadata to storage at the given key. +func (s *VariantStorage) Store(key VariantKey, r io.Reader, contentType string) (size int64, err error) { + data, err := io.ReadAll(r) + if err != nil { + return 0, fmt.Errorf("failed to read content: %w", err) + } + + size = int64(len(data)) + path := s.keyToPath(key) + metaPath := path + ".meta" + + // Create directory structure + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, StorageDirPerm); err != nil { + return 0, fmt.Errorf("failed to create directory: %w", err) + } + + // Write content to temp file first, then rename for atomicity + tmpFile, err := os.CreateTemp(dir, ".tmp-*") + if err != nil { + return 0, fmt.Errorf("failed to create temp file: %w", err) + } + tmpPath := tmpFile.Name() + + defer func() { + if err != nil { + _ = os.Remove(tmpPath) + } + }() + + if _, err := tmpFile.Write(data); err != nil { + _ = tmpFile.Close() + + return 0, fmt.Errorf("failed to write content: %w", err) + } + + if err := tmpFile.Close(); err != nil { + return 0, fmt.Errorf("failed to close temp file: %w", err) + } + + // Atomic rename content + if err := os.Rename(tmpPath, path); err != nil { + return 0, fmt.Errorf("failed to rename temp file: %w", err) + } + + // Write metadata + meta := VariantMeta{ + ContentType: contentType, + Size: size, + CreatedAt: time.Now().UTC().Unix(), + } + + metaData, err := json.Marshal(meta) + if err != nil { + return 0, fmt.Errorf("failed to marshal metadata: %w", err) + } + + if err := os.WriteFile(metaPath, metaData, 0640); err != nil { + // Non-fatal, content is stored + _ = err + } + + return size, nil +} + +// Load returns a reader for the content at the given key. +func (s *VariantStorage) Load(key VariantKey) (io.ReadCloser, error) { + path := s.keyToPath(key) + + f, err := os.Open(path) //nolint:gosec // path derived from cache key + if err != nil { + if os.IsNotExist(err) { + return nil, ErrNotFound + } + + return nil, fmt.Errorf("failed to open content: %w", err) + } + + return f, nil +} + +// LoadWithMeta returns a reader, size, and content type for the content at the given key. +func (s *VariantStorage) LoadWithMeta(key VariantKey) (io.ReadCloser, int64, string, error) { + path := s.keyToPath(key) + metaPath := path + ".meta" + + f, err := os.Open(path) //nolint:gosec // path derived from cache key + if err != nil { + if os.IsNotExist(err) { + return nil, 0, "", ErrNotFound + } + + return nil, 0, "", fmt.Errorf("failed to open content: %w", err) + } + + stat, err := f.Stat() + if err != nil { + _ = f.Close() + + return nil, 0, "", fmt.Errorf("failed to stat content: %w", err) + } + + // Load metadata for content type + contentType := "application/octet-stream" // fallback + + metaData, err := os.ReadFile(metaPath) //nolint:gosec // path derived from cache key + if err == nil { + var meta VariantMeta + if json.Unmarshal(metaData, &meta) == nil && meta.ContentType != "" { + contentType = meta.ContentType + } + } + + return f, stat.Size(), contentType, nil +} + +// Exists checks if content exists at the given key. +func (s *VariantStorage) Exists(key VariantKey) bool { + path := s.keyToPath(key) + _, err := os.Stat(path) + + return err == nil +} + +// Delete removes content at the given key. +func (s *VariantStorage) Delete(key VariantKey) error { + path := s.keyToPath(key) + + err := os.Remove(path) + if err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to delete content: %w", err) + } + + return nil +} + +// keyToPath converts a key to a file path: /// +func (s *VariantStorage) keyToPath(key VariantKey) string { + k := string(key) + if len(k) < MinHashLength { + return filepath.Join(s.baseDir, k) + } + + return filepath.Join(s.baseDir, k[0:2], k[2:4], k) } diff --git a/internal/imgcache/storage_test.go b/internal/imgcache/storage_test.go index 3dd4057..15433a3 100644 --- a/internal/imgcache/storage_test.go +++ b/internal/imgcache/storage_test.go @@ -30,7 +30,8 @@ func TestContentStorage_StoreAndLoad(t *testing.T) { } // Verify file exists at expected path - expectedPath := filepath.Join(tmpDir, hash[0:2], hash[2:4], hash) + hashStr := string(hash) + expectedPath := filepath.Join(tmpDir, hashStr[0:2], hashStr[2:4], hashStr) if _, err := os.Stat(expectedPath); err != nil { t.Errorf("File not at expected path %s: %v", expectedPath, err) } @@ -83,7 +84,7 @@ func TestContentStorage_LoadNotFound(t *testing.T) { t.Fatalf("NewContentStorage() error = %v", err) } - _, err = storage.Load("nonexistent") + _, err = storage.Load(ContentHash("nonexistent")) if err != ErrNotFound { t.Errorf("Load() error = %v, want ErrNotFound", err) } @@ -123,24 +124,29 @@ func TestContentStorage_DeleteNonexistent(t *testing.T) { } // Should not error - if err := storage.Delete("nonexistent"); err != nil { + if err := storage.Delete(ContentHash("nonexistent")); err != nil { t.Errorf("Delete() error = %v, want nil", err) } } -func TestContentStorage_Path(t *testing.T) { +func TestContentStorage_HashToPath(t *testing.T) { tmpDir := t.TempDir() storage, err := NewContentStorage(tmpDir) if err != nil { t.Fatalf("NewContentStorage() error = %v", err) } - hash := "abcdef0123456789" - path := storage.Path(hash) - expected := filepath.Join(tmpDir, "ab", "cd", hash) + // Test by storing and verifying the resulting path structure + content := []byte("test content for path verification") + hash, _, err := storage.Store(bytes.NewReader(content)) + if err != nil { + t.Fatalf("Store() error = %v", err) + } - if path != expected { - t.Errorf("Path() = %q, want %q", path, expected) + hashStr := string(hash) + expectedPath := filepath.Join(tmpDir, hashStr[0:2], hashStr[2:4], hashStr) + if _, err := os.Stat(expectedPath); err != nil { + t.Errorf("File not at expected path %s: %v", expectedPath, err) } } @@ -169,7 +175,7 @@ func TestMetadataStorage_StoreAndLoad(t *testing.T) { } // Verify file exists at expected path - expectedPath := filepath.Join(tmpDir, "cdn.example.com", pathHash+".json") + expectedPath := filepath.Join(tmpDir, "cdn.example.com", string(pathHash)+".json") if _, err := os.Stat(expectedPath); err != nil { t.Errorf("File not at expected path %s: %v", expectedPath, err) } @@ -208,7 +214,7 @@ func TestMetadataStorage_LoadNotFound(t *testing.T) { t.Fatalf("NewMetadataStorage() error = %v", err) } - _, err = storage.Load("example.com", "nonexistent") + _, err = storage.Load("example.com", PathHash("nonexistent")) if err != ErrNotFound { t.Errorf("Load() error = %v, want ErrNotFound", err) } @@ -264,8 +270,8 @@ func TestHashPath(t *testing.T) { } // Hash should be 64 hex chars (256 bits) - if len(hash1) != 64 { - t.Errorf("HashPath() length = %d, want 64", len(hash1)) + if len(string(hash1)) != 64 { + t.Errorf("HashPath() length = %d, want 64", len(string(hash1))) } } @@ -299,8 +305,8 @@ func TestCacheKey(t *testing.T) { } // Key should be 64 hex chars - if len(key1) != 64 { - t.Errorf("CacheKey() length = %d, want 64", len(key1)) + if len(string(key1)) != 64 { + t.Errorf("CacheKey() length = %d, want 64", len(string(key1))) } // Different size should produce different key diff --git a/internal/imgcache/testutil_test.go b/internal/imgcache/testutil_test.go index 8434e5a..b07e98b 100644 --- a/internal/imgcache/testutil_test.go +++ b/internal/imgcache/testutil_test.go @@ -161,11 +161,9 @@ func SetupTestService(t *testing.T, opts ...TestServiceOption) (*Service, *TestF db := setupServiceTestDB(t) cache, err := NewCache(db, CacheConfig{ - StateDir: tmpDir, - CacheTTL: time.Hour, - NegativeTTL: 5 * time.Minute, - HotCacheSize: 100, - HotCacheEnabled: true, + StateDir: tmpDir, + CacheTTL: time.Hour, + NegativeTTL: 5 * time.Minute, }) if err != nil { t.Fatalf("failed to create cache: %v", err)