perf(jellycompat): compress and batch list image work

2026-06-05 17:47:36 -04:00
parent 08a0704a11
commit de89f0136c
4 changed files with 201 additions and 10 deletions
@@ -235,10 +235,9 @@ func (s *directContentService) BrowseItems(ctx context.Context, session *Session
 					mi = localized
 				}
 			}
-			item := mediaItemToListItem(mi)
-			s.presignListItem(ctx, &item)
-			batch = append(batch, item)
+			batch = append(batch, mediaItemToListItem(mi))
 		}
+		s.presignListItems(ctx, batch)
 		if isPlayedFilter != "" {
 			// Need user data to filter by played status. The handler's
 			// resolveUserStateForContentIDs call covers UserData on the wire
@@ -313,10 +312,9 @@ func (s *directContentService) SearchItems(ctx context.Context, session *Session
 				mi = localized
 			}
 		}
-		item := mediaItemToListItem(mi)
-		s.presignListItem(ctx, &item)
-		listItems = append(listItems, item)
+		listItems = append(listItems, mediaItemToListItem(mi))
 	}
+	s.presignListItems(ctx, listItems)
 	s.enrichListItemsUserData(ctx, session, listItems)

 	return &upstreamBrowseResponse{
@@ -652,6 +650,39 @@ func seasonUserDataFromProgress(progress userstore.WatchProgress) *catalog.Seaso
 // --- Image URL presigning helpers ---

 func (s *directContentService) presignListItem(ctx context.Context, item *upstreamListItem) {
+	if item == nil {
+		return
+	}
+	items := []upstreamListItem{*item}
+	s.presignListItems(ctx, items)
+	*item = items[0]
+}
+
+func (s *directContentService) presignListItems(ctx context.Context, items []upstreamListItem) {
+	if len(items) == 0 {
+		return
+	}
+	for i := range items {
+		ensureListItemImagePaths(&items[i])
+	}
+	if s.detailSvc == nil {
+		return
+	}
+
+	posterURLs := s.detailSvc.PresignImageURLsWithExpiry(ctx, collectListImagePaths(items, func(item upstreamListItem) string { return item.PosterURL }), "poster", compatCardImageSize)
+	backdropURLs := s.detailSvc.PresignImageURLsWithExpiry(ctx, collectListImagePaths(items, func(item upstreamListItem) string { return item.BackdropURL }), "backdrop", compatCardImageSize)
+	logoURLs := s.detailSvc.PresignImageURLsWithExpiry(ctx, collectListImagePaths(items, func(item upstreamListItem) string { return item.LogoURL }), "logo", compatCardImageSize)
+	stillURLs := s.detailSvc.PresignImageURLsWithExpiry(ctx, collectListImagePaths(items, func(item upstreamListItem) string { return item.StillURL }), "still", compatCardImageSize)
+
+	for i := range items {
+		items[i].PosterURL = resolvedListImageURL(posterURLs, items[i].PosterURL)
+		items[i].BackdropURL = resolvedListImageURL(backdropURLs, items[i].BackdropURL)
+		items[i].LogoURL = resolvedListImageURL(logoURLs, items[i].LogoURL)
+		items[i].StillURL = resolvedListImageURL(stillURLs, items[i].StillURL)
+	}
+}
+
+func ensureListItemImagePaths(item *upstreamListItem) {
 	if item.PosterPath == "" {
 		item.PosterPath = item.PosterURL
 	}
@@ -664,10 +695,33 @@ func (s *directContentService) presignListItem(ctx context.Context, item *upstre
 	if item.StillPath == "" {
 		item.StillPath = item.StillURL
 	}
-	item.PosterURL = compatPresignImage(s.detailSvc, ctx, item.PosterURL, "poster", compatCardImageSize)
-	item.BackdropURL = compatPresignImage(s.detailSvc, ctx, item.BackdropURL, "backdrop", compatCardImageSize)
-	item.LogoURL = compatPresignImage(s.detailSvc, ctx, item.LogoURL, "logo", compatCardImageSize)
-	item.StillURL = compatPresignImage(s.detailSvc, ctx, item.StillURL, "still", compatCardImageSize)
+}
+
+func collectListImagePaths(items []upstreamListItem, pick func(upstreamListItem) string) []string {
+	paths := make([]string, 0, len(items))
+	seen := make(map[string]struct{}, len(items))
+	for _, item := range items {
+		path := pick(item)
+		if path == "" {
+			continue
+		}
+		if _, ok := seen[path]; ok {
+			continue
+		}
+		seen[path] = struct{}{}
+		paths = append(paths, path)
+	}
+	return paths
+}
+
+func resolvedListImageURL(resolved map[string]catalog.ResolvedImageURL, path string) string {
+	if path == "" {
+		return ""
+	}
+	if value, ok := resolved[path]; ok {
+		return value.URL
+	}
+	return ""
 }

 func (s *directContentService) presignSeason(ctx context.Context, season *upstreamSeason) {
@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/url"
+	"slices"
 	"testing"
 	"time"

@@ -57,6 +58,95 @@ func TestItemEtagIncludesPremiereDate(t *testing.T) {
 	}
 }

+type countingCompatImageResolver struct {
+	singleCalls int
+	batchCalls  int
+	variants    []string
+	paths       []string
+}
+
+func (r *countingCompatImageResolver) ResolveImageURL(_ context.Context, path string, variant string) string {
+	r.singleCalls++
+	return "single:" + variant + ":" + path
+}
+
+func (r *countingCompatImageResolver) ResolveImageURLs(_ context.Context, paths []string, variant string) map[string]string {
+	resolved := r.ResolveImageURLsWithExpiry(context.Background(), paths, variant)
+	urls := make(map[string]string, len(resolved))
+	for path, value := range resolved {
+		urls[path] = value.URL
+	}
+	return urls
+}
+
+func (r *countingCompatImageResolver) ResolveImageURLWithExpiry(_ context.Context, path string, variant string) catalog.ResolvedImageURL {
+	r.singleCalls++
+	return catalog.ResolvedImageURL{URL: "single:" + variant + ":" + path}
+}
+
+func (r *countingCompatImageResolver) ResolveImageURLsWithExpiry(_ context.Context, paths []string, variant string) map[string]catalog.ResolvedImageURL {
+	r.batchCalls++
+	r.variants = append(r.variants, variant)
+	r.paths = append(r.paths, paths...)
+	resolved := make(map[string]catalog.ResolvedImageURL, len(paths))
+	for _, path := range paths {
+		resolved[path] = catalog.ResolvedImageURL{URL: "batch:" + variant + ":" + path}
+	}
+	return resolved
+}
+
+func TestPresignListItemsBatchResolvesImages(t *testing.T) {
+	resolver := &countingCompatImageResolver{}
+	detailSvc := &catalog.DetailService{}
+	detailSvc.SetImageResolver(resolver)
+	svc := &directContentService{detailSvc: detailSvc}
+
+	items := []upstreamListItem{
+		{
+			ContentID:   "movie-1",
+			PosterURL:   "plug://poster-1",
+			BackdropURL: "plug://backdrop-1",
+			LogoURL:     "plug://logo-1",
+		},
+		{
+			ContentID: "movie-2",
+			PosterURL: "plug://poster-2",
+			StillURL:  "plug://still-2",
+		},
+	}
+
+	svc.presignListItems(context.Background(), items)
+
+	if resolver.singleCalls != 0 {
+		t.Fatalf("single image resolver calls = %d, want 0", resolver.singleCalls)
+	}
+	if resolver.batchCalls != 4 {
+		t.Fatalf("batch image resolver calls = %d, want 4", resolver.batchCalls)
+	}
+	for _, variant := range resolver.variants {
+		if variant != "card" {
+			t.Fatalf("batch variant = %q, want card", variant)
+		}
+	}
+	for _, path := range []string{"plug://poster-1", "plug://poster-2", "plug://backdrop-1", "plug://logo-1", "plug://still-2"} {
+		if !slices.Contains(resolver.paths, path) {
+			t.Fatalf("batch paths %v missing %q", resolver.paths, path)
+		}
+	}
+	if items[0].PosterPath != "plug://poster-1" {
+		t.Fatalf("PosterPath = %q, want original path", items[0].PosterPath)
+	}
+	if got := items[0].PosterURL; got != "batch:card:plug://poster-1" {
+		t.Fatalf("PosterURL = %q", got)
+	}
+	if got := items[0].BackdropURL; got != "batch:card:plug://backdrop-1" {
+		t.Fatalf("BackdropURL = %q", got)
+	}
+	if got := items[1].StillURL; got != "batch:card:plug://still-2" {
+		t.Fatalf("StillURL = %q", got)
+	}
+}
+
 // progressCountingStoreProvider is a test double that records calls to
 // ForUser and ListProgressByMediaItems. Used to assert that BrowseItems
 // does not duplicate the handler-level user-data fetch.
@@ -39,6 +39,7 @@ func NewRouter(deps Dependencies) chi.Router {
 		MaxAge:           86400,
 	}))
 	r.Use(normalizeCompatPathMiddleware)
+	r.Use(middleware.Compress(5, "application/json"))
 	if debugPath := os.Getenv("JELLYCOMPAT_DEBUG_LOG"); debugPath != "" {
 		rotator := &lumberjack.Logger{
 			Filename:   debugPath,
@@ -0,0 +1,46 @@
+package jellycompat
+
+import (
+	"compress/gzip"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/Silo-Server/silo-server/internal/config"
+)
+
+func TestRouterCompressesJSONResponses(t *testing.T) {
+	cfg, err := config.LoadFromDB(map[string]string{})
+	if err != nil {
+		t.Fatalf("LoadFromDB: %v", err)
+	}
+	router := NewRouter(Dependencies{Config: cfg})
+
+	req := httptest.NewRequest(http.MethodGet, "/System/Info/Public", nil)
+	req.Header.Set("Accept-Encoding", "gzip")
+	rec := httptest.NewRecorder()
+
+	router.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusOK {
+		t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
+	}
+	if got := rec.Header().Get("Content-Encoding"); got != "gzip" {
+		t.Fatalf("Content-Encoding = %q, want gzip", got)
+	}
+
+	reader, err := gzip.NewReader(rec.Body)
+	if err != nil {
+		t.Fatalf("gzip reader: %v", err)
+	}
+	defer reader.Close()
+	body, err := io.ReadAll(reader)
+	if err != nil {
+		t.Fatalf("read compressed body: %v", err)
+	}
+	if !strings.Contains(string(body), `"ProductName":"Jellyfin Server"`) {
+		t.Fatalf("unexpected response body %q", string(body))
+	}
+}