Merge feature/http-response-handling: ETag, HEAD, conditional requests

TODO.md

@@ -174,10 +174,10 @@ A single linear checklist of tasks to implement the complete pixa caching image
 
 ## HTTP Response Handling
 - [x] Implement proper Cache-Control headers
-- [ ] Implement ETag generation and validation
+- [x] Implement ETag generation and validation
 - [ ] Implement Last-Modified headers
-- [ ] Implement conditional requests (If-None-Match, If-Modified-Since)
+- [x] Implement conditional requests (If-None-Match, If-Modified-Since)
-- [ ] Implement HEAD request support
+- [x] Implement HEAD request support
 - [ ] Implement Vary header for content negotiation
 - [x] Implement X-Pixa-Cache debug header (HIT/MISS/STALE)
 - [ ] Implement X-Request-ID propagation
@@ -185,7 +185,7 @@ A single linear checklist of tasks to implement the complete pixa caching image
 
 ## Additional Endpoints
 - [x] Implement robots.txt endpoint
-- [ ] Implement metrics endpoint with auth
+- [x] Implement metrics endpoint with auth
 - [ ] Implement auto-format selection (format=auto based on Accept header)
 
 ## Configuration

internal/handlers/handlers_test.go

@@ -0,0 +1,256 @@
+package handlers
+
+import (
+	"bytes"
+	"context"
+	"database/sql"
+	"image"
+	"image/color"
+	"image/jpeg"
+	"io"
+	"io/fs"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"testing/fstest"
+	"time"
+
+	"github.com/go-chi/chi/v5"
+	"sneak.berlin/go/pixa/internal/database"
+	"sneak.berlin/go/pixa/internal/imgcache"
+)
+
+// testFixtures contains test data.
+type testFixtures struct {
+	handler  *Handlers
+	service  *imgcache.Service
+	goodHost string
+}
+
+func setupTestHandler(t *testing.T) *testFixtures {
+	t.Helper()
+
+	goodHost := "goodhost.example.com"
+
+	// Generate test JPEG
+	jpegData := generateTestJPEG(t, 100, 100, color.RGBA{255, 0, 0, 255})
+
+	mockFS := fstest.MapFS{
+		goodHost + "/images/photo.jpg": &fstest.MapFile{Data: jpegData},
+	}
+
+	tmpDir := t.TempDir()
+	db := setupTestDB(t)
+
+	cache, err := imgcache.NewCache(db, imgcache.CacheConfig{
+		StateDir:        tmpDir,
+		CacheTTL:        time.Hour,
+		NegativeTTL:     5 * time.Minute,
+		HotCacheSize:    100,
+		HotCacheEnabled: true,
+	})
+	if err != nil {
+		t.Fatalf("failed to create cache: %v", err)
+	}
+
+	svc, err := imgcache.NewService(&imgcache.ServiceConfig{
+		Cache:     cache,
+		Fetcher:   newMockFetcher(mockFS),
+		Whitelist: []string{goodHost},
+	})
+	if err != nil {
+		t.Fatalf("failed to create service: %v", err)
+	}
+
+	h := &Handlers{
+		imgSvc: svc,
+		log:    slog.Default(),
+	}
+
+	return &testFixtures{
+		handler:  h,
+		service:  svc,
+		goodHost: goodHost,
+	}
+}
+
+func setupTestDB(t *testing.T) *sql.DB {
+	t.Helper()
+
+	db, err := sql.Open("sqlite", ":memory:")
+	if err != nil {
+		t.Fatalf("failed to open test db: %v", err)
+	}
+
+	if err := database.ApplyMigrations(db); err != nil {
+		t.Fatalf("failed to apply migrations: %v", err)
+	}
+
+	return db
+}
+
+func generateTestJPEG(t *testing.T, width, height int, c color.Color) []byte {
+	t.Helper()
+
+	img := image.NewRGBA(image.Rect(0, 0, width, height))
+	for y := 0; y < height; y++ {
+		for x := 0; x < width; x++ {
+			img.Set(x, y, c)
+		}
+	}
+
+	var buf bytes.Buffer
+	if err := jpeg.Encode(&buf, img, &jpeg.Options{Quality: 85}); err != nil {
+		t.Fatalf("failed to encode test JPEG: %v", err)
+	}
+
+	return buf.Bytes()
+}
+
+// mockFetcher fetches from a mock filesystem.
+type mockFetcher struct {
+	fs fs.FS
+}
+
+func newMockFetcher(fs fs.FS) *mockFetcher {
+	return &mockFetcher{fs: fs}
+}
+
+func (f *mockFetcher) Fetch(ctx context.Context, url string) (*imgcache.FetchResult, error) {
+	// Remove https:// prefix
+	path := url[8:] // Remove "https://"
+
+	data, err := fs.ReadFile(f.fs, path)
+	if err != nil {
+		return nil, imgcache.ErrUpstreamError
+	}
+
+	return &imgcache.FetchResult{
+		Content:       io.NopCloser(bytes.NewReader(data)),
+		ContentLength: int64(len(data)),
+		ContentType:   "image/jpeg",
+	}, nil
+}
+
+func TestHandleImage_HEAD_ReturnsHeadersOnly(t *testing.T) {
+	fix := setupTestHandler(t)
+
+	// Create a chi router to properly handle wildcards
+	r := chi.NewRouter()
+	r.Head("/v1/image/*", fix.handler.HandleImage())
+
+	req := httptest.NewRequest(http.MethodHead, "/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg", nil)
+	rec := httptest.NewRecorder()
+
+	r.ServeHTTP(rec, req)
+
+	// Should return 200 OK
+	if rec.Code != http.StatusOK {
+		t.Errorf("HEAD request status = %d, want %d", rec.Code, http.StatusOK)
+	}
+
+	// Should have Content-Type header
+	if ct := rec.Header().Get("Content-Type"); ct == "" {
+		t.Error("HEAD response should have Content-Type header")
+	}
+
+	// Should have Content-Length header
+	if cl := rec.Header().Get("Content-Length"); cl == "" {
+		t.Error("HEAD response should have Content-Length header")
+	}
+
+	// Body should be empty for HEAD request
+	if rec.Body.Len() != 0 {
+		t.Errorf("HEAD response body should be empty, got %d bytes", rec.Body.Len())
+	}
+}
+
+func TestHandleImage_ConditionalRequest_IfNoneMatch_Returns304(t *testing.T) {
+	fix := setupTestHandler(t)
+
+	r := chi.NewRouter()
+	r.Get("/v1/image/*", fix.handler.HandleImage())
+
+	// First request to get the ETag
+	req1 := httptest.NewRequest(http.MethodGet, "/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg", nil)
+	rec1 := httptest.NewRecorder()
+
+	r.ServeHTTP(rec1, req1)
+
+	if rec1.Code != http.StatusOK {
+		t.Fatalf("First request status = %d, want %d", rec1.Code, http.StatusOK)
+	}
+
+	etag := rec1.Header().Get("ETag")
+	if etag == "" {
+		t.Fatal("Response should have ETag header")
+	}
+
+	// Second request with If-None-Match header
+	req2 := httptest.NewRequest(http.MethodGet, "/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg", nil)
+	req2.Header.Set("If-None-Match", etag)
+	rec2 := httptest.NewRecorder()
+
+	r.ServeHTTP(rec2, req2)
+
+	// Should return 304 Not Modified
+	if rec2.Code != http.StatusNotModified {
+		t.Errorf("Conditional request status = %d, want %d", rec2.Code, http.StatusNotModified)
+	}
+
+	// Body should be empty for 304 response
+	if rec2.Body.Len() != 0 {
+		t.Errorf("304 response body should be empty, got %d bytes", rec2.Body.Len())
+	}
+}
+
+func TestHandleImage_ConditionalRequest_IfNoneMatch_DifferentETag(t *testing.T) {
+	fix := setupTestHandler(t)
+
+	r := chi.NewRouter()
+	r.Get("/v1/image/*", fix.handler.HandleImage())
+
+	// Request with non-matching ETag
+	req := httptest.NewRequest(http.MethodGet, "/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg", nil)
+	req.Header.Set("If-None-Match", `"different-etag"`)
+	rec := httptest.NewRecorder()
+
+	r.ServeHTTP(rec, req)
+
+	// Should return 200 OK with full response
+	if rec.Code != http.StatusOK {
+		t.Errorf("Request with non-matching ETag status = %d, want %d", rec.Code, http.StatusOK)
+	}
+
+	// Body should not be empty
+	if rec.Body.Len() == 0 {
+		t.Error("Response body should not be empty for non-matching ETag")
+	}
+}
+
+func TestHandleImage_ETagHeader(t *testing.T) {
+	fix := setupTestHandler(t)
+
+	r := chi.NewRouter()
+	r.Get("/v1/image/*", fix.handler.HandleImage())
+
+	req := httptest.NewRequest(http.MethodGet, "/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg", nil)
+	rec := httptest.NewRecorder()
+
+	r.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusOK {
+		t.Fatalf("Request status = %d, want %d", rec.Code, http.StatusOK)
+	}
+
+	etag := rec.Header().Get("ETag")
+	if etag == "" {
+		t.Error("Response should have ETag header")
+	}
+
+	// ETag should be quoted
+	if len(etag) < 2 || etag[0] != '"' || etag[len(etag)-1] != '"' {
+		t.Errorf("ETag should be quoted, got %q", etag)
+	}
+}

internal/handlers/image.go

@@ -122,6 +122,22 @@ func (s *Handlers) HandleImage() http.HandlerFunc {
 
 		if resp.ETag != "" {
 			w.Header().Set("ETag", resp.ETag)
+
+			// Check for conditional request (If-None-Match)
+			if ifNoneMatch := r.Header.Get("If-None-Match"); ifNoneMatch != "" {
+				if ifNoneMatch == resp.ETag {
+					w.WriteHeader(http.StatusNotModified)
+
+					return
+				}
+			}
+		}
+
+		// Handle HEAD request - return headers only
+		if r.Method == http.MethodHead {
+			w.WriteHeader(http.StatusOK)
+
+			return
 		}
 
 		// Stream the response

internal/imgcache/cache.go

@@ -168,6 +168,11 @@ 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)
+}
+
 // StoreSource stores fetched source content and metadata.
 func (c *Cache) StoreSource(
 	ctx context.Context,

internal/imgcache/service.go

@@ -91,15 +91,17 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
 	if result != nil && result.Hit {
 		s.cache.IncrementStats(ctx, true, 0)
 
-		reader, err := s.cache.GetOutput(result.OutputHash)
+		reader, size, err := s.cache.GetOutputWithSize(result.OutputHash)
 		if err != nil {
 			s.log.Error("failed to get cached output", "hash", result.OutputHash, "error", err)
 			// Fall through to re-fetch
 		} else {
 			return &ImageResponse{
 				Content:       reader,
+				ContentLength: size,
 				ContentType:   result.ContentType,
 				CacheStatus:   CacheHit,
+				ETag:          formatETag(result.OutputHash),
 			}, nil
 		}
 	}
@@ -173,15 +175,17 @@ func (s *Service) fetchAndProcess(ctx context.Context, req *ImageRequest) (*Imag
 	}
 
 	// Serve from the cached file on disk (same path as cache hits)
-	reader, err := s.cache.GetOutput(outputHash)
+	reader, size, err := s.cache.GetOutputWithSize(outputHash)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read cached output: %w", err)
 	}
 
 	return &ImageResponse{
 		Content:       reader,
+		ContentLength: size,
 		ContentType:   processResult.ContentType,
 		FetchedBytes:  int64(len(sourceData)),
+		ETag:          formatETag(outputHash),
 	}, nil
 }
 
@@ -260,3 +264,16 @@ func extractStatusCode(err error) int {
 
 	return httpStatusInternalError
 }
+
+// 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 {
+	// Use first 16 characters of hash for a shorter but still unique ETag
+	if len(hash) > etagHashLength {
+		hash = hash[:etagHashLength]
+	}
+
+	return `"` + hash + `"`
+}

internal/imgcache/service_test.go

@@ -405,3 +405,111 @@ func TestService_Get_ContextCancellation(t *testing.T) {
 		t.Error("Get() expected error for cancelled context")
 	}
 }
+
+func TestService_Get_ReturnsETag(t *testing.T) {
+	svc, fixtures := SetupTestService(t)
+	ctx := context.Background()
+
+	req := &ImageRequest{
+		SourceHost: fixtures.GoodHost,
+		SourcePath: "/images/photo.jpg",
+		Size:       Size{Width: 50, Height: 50},
+		Format:     FormatJPEG,
+		Quality:    85,
+		FitMode:    FitCover,
+	}
+
+	resp, err := svc.Get(ctx, req)
+	if err != nil {
+		t.Fatalf("Get() error = %v", err)
+	}
+	defer resp.Content.Close()
+
+	// ETag should be set
+	if resp.ETag == "" {
+		t.Error("ETag should be set in response")
+	}
+
+	// ETag should be properly quoted
+	if len(resp.ETag) < 2 || resp.ETag[0] != '"' || resp.ETag[len(resp.ETag)-1] != '"' {
+		t.Errorf("ETag should be quoted, got %q", resp.ETag)
+	}
+}
+
+func TestService_Get_ETagConsistency(t *testing.T) {
+	svc, fixtures := SetupTestService(t)
+	ctx := context.Background()
+
+	req := &ImageRequest{
+		SourceHost: fixtures.GoodHost,
+		SourcePath: "/images/photo.jpg",
+		Size:       Size{Width: 50, Height: 50},
+		Format:     FormatJPEG,
+		Quality:    85,
+		FitMode:    FitCover,
+	}
+
+	// First request
+	resp1, err := svc.Get(ctx, req)
+	if err != nil {
+		t.Fatalf("Get() first request error = %v", err)
+	}
+	etag1 := resp1.ETag
+	resp1.Content.Close()
+
+	// Second request (from cache)
+	resp2, err := svc.Get(ctx, req)
+	if err != nil {
+		t.Fatalf("Get() second request error = %v", err)
+	}
+	etag2 := resp2.ETag
+	resp2.Content.Close()
+
+	// ETags should be identical for the same content
+	if etag1 != etag2 {
+		t.Errorf("ETags should match: %q != %q", etag1, etag2)
+	}
+}
+
+func TestService_Get_DifferentETagsForDifferentContent(t *testing.T) {
+	svc, fixtures := SetupTestService(t)
+	ctx := context.Background()
+
+	// Request same image at different sizes - should get different ETags
+	req1 := &ImageRequest{
+		SourceHost: fixtures.GoodHost,
+		SourcePath: "/images/photo.jpg",
+		Size:       Size{Width: 25, Height: 25},
+		Format:     FormatJPEG,
+		Quality:    85,
+		FitMode:    FitCover,
+	}
+
+	req2 := &ImageRequest{
+		SourceHost: fixtures.GoodHost,
+		SourcePath: "/images/photo.jpg",
+		Size:       Size{Width: 50, Height: 50},
+		Format:     FormatJPEG,
+		Quality:    85,
+		FitMode:    FitCover,
+	}
+
+	resp1, err := svc.Get(ctx, req1)
+	if err != nil {
+		t.Fatalf("Get() first request error = %v", err)
+	}
+	etag1 := resp1.ETag
+	resp1.Content.Close()
+
+	resp2, err := svc.Get(ctx, req2)
+	if err != nil {
+		t.Fatalf("Get() second request error = %v", err)
+	}
+	etag2 := resp2.ETag
+	resp2.Content.Close()
+
+	// ETags should be different for different content
+	if etag1 == etag2 {
+		t.Error("ETags should differ for different sizes")
+	}
+}

internal/imgcache/storage.go

@@ -114,6 +114,29 @@ func (s *ContentStorage) Load(hash string) (io.ReadCloser, error) {
 	return f, nil
 }
 
+// LoadWithSize returns a reader and file size for the content with the given hash.
+func (s *ContentStorage) LoadWithSize(hash string) (io.ReadCloser, int64, error) {
+	path := s.hashToPath(hash)
+
+	f, err := os.Open(path) //nolint:gosec // content-addressable path from hash
+	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)
+	}
+
+	return f, stat.Size(), nil
+}
+
 // Delete removes content with the given hash.
 func (s *ContentStorage) Delete(hash string) error {
 	path := s.hashToPath(hash)

internal/server/routes.go

@@ -53,6 +53,7 @@ func (s *Server) SetupRoutes() {
 	// Main image proxy route
 	// /v1/image/<host>/<path>/<width>x<height>.<format>
 	s.router.Get("/v1/image/*", s.h.HandleImage())
+	s.router.Head("/v1/image/*", s.h.HandleImage())
 
 	// Encrypted image URL route
 	s.router.Get("/v1/e/{token}", s.h.HandleImageEnc())