CodeRabbit Generated Unit Tests: Add unit tests for PR changes

This commit is contained in:
coderabbitai[bot]
2026-06-27 18:51:29 +00:00
committed by GitHub
parent 4d2a990de1
commit 94a280e89a
3 changed files with 562 additions and 2 deletions
+249
View File
@@ -5,6 +5,7 @@ import (
"fmt"
"net/url"
"slices"
"strings"
"testing"
"time"
@@ -460,6 +461,44 @@ func (s *stubBrowseSource) ListGenres(_ context.Context, _ catalog.BrowseFilters
return nil, nil
}
type recordingCatalogSearchProvider struct {
requests []catalog.CatalogSearchRequest
result *catalog.CatalogSearchResult
}
func (p *recordingCatalogSearchProvider) Search(_ context.Context, req catalog.CatalogSearchRequest) (*catalog.CatalogSearchResult, error) {
p.requests = append(p.requests, req)
if p.result != nil {
return p.result, nil
}
return &catalog.CatalogSearchResult{}, nil
}
type recordingItemAccessSource struct {
searchQueries []string
searchTypes [][]string
searchLimits []int
searchOffsets []int
items []*models.MediaItem
total int
}
func (s *recordingItemAccessSource) EnsureAccessible(context.Context, string, catalog.AccessFilter) error {
return nil
}
func (s *recordingItemAccessSource) Search(_ context.Context, query string, itemTypes []string, limit, offset int, _ catalog.AccessFilter) ([]*models.MediaItem, int, error) {
s.searchQueries = append(s.searchQueries, query)
s.searchTypes = append(s.searchTypes, append([]string(nil), itemTypes...))
s.searchLimits = append(s.searchLimits, limit)
s.searchOffsets = append(s.searchOffsets, offset)
return append([]*models.MediaItem(nil), s.items...), s.total, nil
}
func (s *recordingItemAccessSource) GetByIDs(context.Context, []string) ([]*models.MediaItem, error) {
return nil, nil
}
// newDirectContentServiceForTest builds a directContentService with stubbed
// catalog dependencies. Useful for behavioral tests that don't need real
// Postgres state.
@@ -470,6 +509,216 @@ func newDirectContentServiceForTest(browse browseSource, provider userstore.User
}
}
func TestSearchItemsUsesCatalogSearchProviderWithCompatScope(t *testing.T) {
libraryID := 7
provider := &recordingCatalogSearchProvider{
result: &catalog.CatalogSearchResult{
Items: []*models.MediaItem{{
ContentID: "movie-1",
Type: "movie",
Title: "Dune",
}},
Total: 12,
HasMore: true,
},
}
svc := &directContentService{
searchProvider: provider,
accessFilter: func(context.Context, int, string) catalog.AccessFilter {
return catalog.AccessFilter{
AllowedLibraryIDs: []int{1, 2},
ExcludedMediaTypes: []string{"ebook"},
MaxContentRating: "PG-13",
}
},
}
result, err := svc.SearchItems(context.Background(), &Session{
StreamAppUserID: 22,
ProfileID: "profile-1",
}, SearchItemsOptions{
Query: "dune",
Limit: 5,
Offset: 10,
LibraryID: &libraryID,
SkipTotal: true,
})
if err != nil {
t.Fatalf("SearchItems error: %v", err)
}
if len(provider.requests) != 1 {
t.Fatalf("provider requests = %d, want 1", len(provider.requests))
}
req := provider.requests[0]
if req.Query != "dune" || req.Limit != 5 || req.Offset != 10 || !req.SkipTotal {
t.Fatalf("provider request shape = %#v", req)
}
if want := []string{"movie", "series", "episode"}; !slices.Equal(req.ItemTypes, want) {
t.Fatalf("ItemTypes = %#v, want %#v", req.ItemTypes, want)
}
if req.Access.PresentationLibraryID == nil || *req.Access.PresentationLibraryID != libraryID {
t.Fatalf("PresentationLibraryID = %#v, want %d", req.Access.PresentationLibraryID, libraryID)
}
for _, mediaType := range []string{"ebook", "audiobook", "podcast"} {
if !slices.Contains(req.Access.ExcludedMediaTypes, mediaType) {
t.Fatalf("ExcludedMediaTypes = %#v, missing %q", req.Access.ExcludedMediaTypes, mediaType)
}
}
if result.Total != 12 || !result.HasMore || len(result.Items) != 1 || result.Items[0].Title != "Dune" {
t.Fatalf("result = %#v", result)
}
}
func TestSearchItemsFallsBackToItemRepoWhenProviderMissing(t *testing.T) {
itemRepo := &recordingItemAccessSource{
items: []*models.MediaItem{{
ContentID: "movie-1",
Type: "movie",
Title: "Fallback",
}},
total: 3,
}
svc := &directContentService{itemRepo: itemRepo}
result, err := svc.SearchItems(context.Background(), &Session{}, SearchItemsOptions{
Query: "fallback",
ItemTypes: []string{"MusicAlbum"},
Limit: 2,
Offset: 1,
})
if err != nil {
t.Fatalf("SearchItems error: %v", err)
}
if len(itemRepo.searchQueries) != 1 || itemRepo.searchQueries[0] != "fallback" {
t.Fatalf("search queries = %#v", itemRepo.searchQueries)
}
if want := []string{compatNoMatchType}; !slices.Equal(itemRepo.searchTypes[0], want) {
t.Fatalf("search types = %#v, want %#v", itemRepo.searchTypes[0], want)
}
if result.Total != 3 || !result.HasMore || len(result.Items) != 1 || result.Items[0].Title != "Fallback" {
t.Fatalf("result = %#v", result)
}
}
func TestSearchItemsProviderReturnsNilResult(t *testing.T) {
// explicitlyNilResultProvider returns (nil, nil) — no error, no result.
// This exercises the nil-guard in directContentService.SearchItems
// (the recordingCatalogSearchProvider stub always returns a non-nil result).
provider := &explicitlyNilResultProvider{}
svc := &directContentService{searchProvider: provider}
result, err := svc.SearchItems(context.Background(), &Session{}, SearchItemsOptions{
Query: "ghost",
Limit: 10,
})
if err != nil {
t.Fatalf("SearchItems error: %v", err)
}
if result == nil {
t.Fatal("expected non-nil result")
}
if len(result.Items) != 0 {
t.Fatalf("Items = %d, want 0 when provider returns nil", len(result.Items))
}
if result.Total != 0 {
t.Fatalf("Total = %d, want 0 when provider returns nil", result.Total)
}
if result.HasMore {
t.Fatal("HasMore = true, want false when provider returns nil")
}
}
// explicitlyNilResultProvider is a CatalogSearchProvider that returns (nil, nil)
// on every call, exercising the nil-guard in directContentService.SearchItems.
type explicitlyNilResultProvider struct {
calls int
}
func (p *explicitlyNilResultProvider) Search(_ context.Context, _ catalog.CatalogSearchRequest) (*catalog.CatalogSearchResult, error) {
p.calls++
return nil, nil
}
func TestSearchItemsProviderErrorPropagated(t *testing.T) {
errProvider := &erroringCatalogSearchProvider{err: fmt.Errorf("search backend unavailable")}
svc := &directContentService{searchProvider: errProvider}
_, err := svc.SearchItems(context.Background(), &Session{}, SearchItemsOptions{
Query: "test",
Limit: 5,
})
if err == nil {
t.Fatal("expected error from provider, got nil")
}
if !strings.Contains(err.Error(), "search backend unavailable") {
t.Fatalf("error %q does not contain expected message", err.Error())
}
}
// erroringCatalogSearchProvider always returns an error from Search.
type erroringCatalogSearchProvider struct {
err error
}
func (p *erroringCatalogSearchProvider) Search(_ context.Context, _ catalog.CatalogSearchRequest) (*catalog.CatalogSearchResult, error) {
return nil, p.err
}
func TestSearchItemsFallbackHasMoreFalseAtBoundary(t *testing.T) {
// When offset + len(items) == total, hasMore must be false.
// e.g. offset=2, return 1 item, total=3 → 2+1 == 3 → hasMore=false
itemRepo := &recordingItemAccessSource{
items: []*models.MediaItem{{
ContentID: "movie-3",
Type: "movie",
Title: "Last",
}},
total: 3,
}
svc := &directContentService{itemRepo: itemRepo}
result, err := svc.SearchItems(context.Background(), &Session{}, SearchItemsOptions{
Query: "last",
Limit: 1,
Offset: 2,
})
if err != nil {
t.Fatalf("SearchItems error: %v", err)
}
if result.HasMore {
t.Fatalf("HasMore = true, want false when offset(%d)+len(%d) == total(%d)", 2, 1, 3)
}
if result.Total != 3 {
t.Fatalf("Total = %d, want 3", result.Total)
}
}
func TestSearchItemsFallbackHasMoreTrueWhenMoreRemain(t *testing.T) {
// When offset + len(items) < total, hasMore must be true.
// e.g. offset=0, return 1 item, total=3 → 0+1 < 3 → hasMore=true
itemRepo := &recordingItemAccessSource{
items: []*models.MediaItem{{
ContentID: "movie-1",
Type: "movie",
Title: "First",
}},
total: 3,
}
svc := &directContentService{itemRepo: itemRepo}
result, err := svc.SearchItems(context.Background(), &Session{}, SearchItemsOptions{
Query: "first",
Limit: 1,
Offset: 0,
})
if err != nil {
t.Fatalf("SearchItems error: %v", err)
}
if !result.HasMore {
t.Fatalf("HasMore = false, want true when offset(%d)+len(%d) < total(%d)", 0, 1, 3)
}
}
// TestBrowseItems_DoesNotFetchProgressWhenNoPlayedFilter verifies that
// BrowseItems does NOT call ListProgressByMediaItems on the user store when
// the is_played filter is empty. The handler-level resolveUserStateForContentIDs
+179 -1
View File
@@ -50,7 +50,7 @@ func (s *countingContentService) BrowseItems(context.Context, *Session, url.Valu
panic("unused")
}
func (s *countingContentService) SearchItems(context.Context, *Session, string, []string, int, int, *int) (*upstreamBrowseResponse, error) {
func (s *countingContentService) SearchItems(context.Context, *Session, SearchItemsOptions) (*upstreamBrowseResponse, error) {
panic("unused")
}
@@ -81,6 +81,20 @@ func (s *countingContentService) ListItemFilters(context.Context, *Session, url.
panic("unused")
}
type recordingSearchContentService struct {
countingContentService
options []SearchItemsOptions
result *upstreamBrowseResponse
}
func (s *recordingSearchContentService) SearchItems(_ context.Context, _ *Session, opts SearchItemsOptions) (*upstreamBrowseResponse, error) {
s.options = append(s.options, opts)
if s.result != nil {
return s.result, nil
}
return &upstreamBrowseResponse{Items: []upstreamListItem{}}, nil
}
func TestHandleItems_SeriesParentSeasonFilterReturnsPagedSeasons(t *testing.T) {
codec := NewResourceIDCodec()
seriesContentID := "series-1"
@@ -140,6 +154,170 @@ func TestHandleItems_SeriesParentSeasonFilterReturnsPagedSeasons(t *testing.T) {
}
}
func TestHandleItemsSearchPropagatesEnableTotalRecordCount(t *testing.T) {
for _, tc := range []struct {
name string
querySuffix string
wantSkipTotal bool
}{
{name: "default includes total", wantSkipTotal: false},
{name: "disabled skips total", querySuffix: "&EnableTotalRecordCount=false", wantSkipTotal: true},
} {
t.Run(tc.name, func(t *testing.T) {
codec := NewResourceIDCodec()
contentSvc := &recordingSearchContentService{}
h := &ItemsHandler{
content: contentSvc,
userData: &mockUserDataService{},
codec: codec,
mapper: newMapper(codec, &config.Config{}),
images: NewImageCache(time.Hour, time.Now),
}
req := httptest.NewRequest("GET", "/Users/test/Items?SearchTerm=dune&Limit=5&StartIndex=2"+tc.querySuffix, nil)
req = req.WithContext(context.WithValue(req.Context(), compatSessionKey, &Session{
StreamAppUserID: 1,
ProfileID: "profile-1",
}))
rec := httptest.NewRecorder()
h.HandleItems(rec, req)
if rec.Code != 200 {
t.Fatalf("expected status 200; got %d, body=%s", rec.Code, rec.Body.String())
}
if len(contentSvc.options) != 1 {
t.Fatalf("SearchItems calls = %d, want 1", len(contentSvc.options))
}
opts := contentSvc.options[0]
if opts.Query != "dune" || opts.Limit != 5 || opts.Offset != 2 {
t.Fatalf("SearchItems options = %#v", opts)
}
if opts.SkipTotal != tc.wantSkipTotal {
t.Fatalf("SkipTotal = %v, want %v", opts.SkipTotal, tc.wantSkipTotal)
}
})
}
}
// TestHandleSearchHints_PassesOnlyQueryAndLimit verifies that HandleSearchHints
// calls SearchItems with a SearchItemsOptions that carries only Query and Limit
// (no LibraryID, empty ItemTypes, and SkipTotal=false). The search-hints
// endpoint does not scope by library and never specifies item types — returning
// all video-compatible types — so it must not accidentally suppress the total
// or restrict the result set.
func TestHandleSearchHints_PassesOnlyQueryAndLimit(t *testing.T) {
codec := NewResourceIDCodec()
contentSvc := &recordingSearchContentService{
result: &upstreamBrowseResponse{
Items: []upstreamListItem{
{ContentID: "movie-1", Type: "movie", Title: "Dune"},
},
Total: 1,
},
}
h := &ItemsHandler{
content: contentSvc,
userData: &mockUserDataService{},
codec: codec,
mapper: newMapper(codec, &config.Config{}),
images: NewImageCache(time.Hour, time.Now),
}
req := httptest.NewRequest("GET", "/Search/Hints?SearchTerm=dune&Limit=7", nil)
req = req.WithContext(context.WithValue(req.Context(), compatSessionKey, &Session{
StreamAppUserID: 1,
ProfileID: "profile-1",
}))
rec := httptest.NewRecorder()
h.HandleSearchHints(rec, req)
if rec.Code != 200 {
t.Fatalf("expected status 200; got %d, body=%s", rec.Code, rec.Body.String())
}
if len(contentSvc.options) != 1 {
t.Fatalf("SearchItems calls = %d, want 1", len(contentSvc.options))
}
opts := contentSvc.options[0]
if opts.Query != "dune" {
t.Errorf("Query = %q, want %q", opts.Query, "dune")
}
if opts.Limit != 7 {
t.Errorf("Limit = %d, want 7", opts.Limit)
}
if len(opts.ItemTypes) != 0 {
t.Errorf("ItemTypes = %#v, want empty (HandleSearchHints must not restrict types)", opts.ItemTypes)
}
if opts.LibraryID != nil {
t.Errorf("LibraryID = %v, want nil", opts.LibraryID)
}
if opts.SkipTotal {
t.Error("SkipTotal = true, want false for search hints")
}
if opts.Offset != 0 {
t.Errorf("Offset = %d, want 0", opts.Offset)
}
}
// TestHandleSearchHints_EmptyQueryReturnsEmptyWithoutCallingSearch verifies
// that an empty or whitespace-only SearchTerm returns a 200 empty result
// without invoking the ContentService at all.
func TestHandleSearchHints_EmptyQueryReturnsEmptyWithoutCallingSearch(t *testing.T) {
codec := NewResourceIDCodec()
contentSvc := &recordingSearchContentService{}
h := &ItemsHandler{
content: contentSvc,
userData: &mockUserDataService{},
codec: codec,
mapper: newMapper(codec, &config.Config{}),
images: NewImageCache(time.Hour, time.Now),
}
req := httptest.NewRequest("GET", "/Search/Hints?SearchTerm= ", nil)
req = req.WithContext(context.WithValue(req.Context(), compatSessionKey, &Session{
StreamAppUserID: 1,
ProfileID: "profile-1",
}))
rec := httptest.NewRecorder()
h.HandleSearchHints(rec, req)
if rec.Code != 200 {
t.Fatalf("expected status 200; got %d, body=%s", rec.Code, rec.Body.String())
}
if len(contentSvc.options) != 0 {
t.Errorf("SearchItems called %d times, want 0 for empty query", len(contentSvc.options))
}
}
// TestHandleSearchHints_DefaultLimitIsTwenty verifies that when Limit is
// omitted from the query string, HandleSearchHints uses a default of 20.
func TestHandleSearchHints_DefaultLimitIsTwenty(t *testing.T) {
codec := NewResourceIDCodec()
contentSvc := &recordingSearchContentService{}
h := &ItemsHandler{
content: contentSvc,
userData: &mockUserDataService{},
codec: codec,
mapper: newMapper(codec, &config.Config{}),
images: NewImageCache(time.Hour, time.Now),
}
req := httptest.NewRequest("GET", "/Search/Hints?SearchTerm=inception", nil)
req = req.WithContext(context.WithValue(req.Context(), compatSessionKey, &Session{
StreamAppUserID: 1,
ProfileID: "profile-1",
}))
rec := httptest.NewRecorder()
h.HandleSearchHints(rec, req)
if rec.Code != 200 {
t.Fatalf("expected status 200; got %d, body=%s", rec.Code, rec.Body.String())
}
if len(contentSvc.options) == 1 && contentSvc.options[0].Limit != 20 {
t.Errorf("Limit = %d, want 20 (default)", contentSvc.options[0].Limit)
}
}
// TestHandleItem_Episode_FetchesSeriesDetailForStableParentImageTags verifies
// that episode detail responses fetch parent series image metadata even when
// image URLs are already cached. Cached URLs are not enough to build stable
+134 -1
View File
@@ -1,6 +1,139 @@
package jellycompat
import "testing"
import (
"context"
"net/url"
"slices"
"testing"
)
// recordingPersonsContentService is a ContentService stub that records all
// SearchItems calls made by PersonsHandler. It is only used in
// handlers_persons_test.go and lives here to keep it close to the tests that
// rely on it.
type recordingPersonsContentService struct {
searchOptions []SearchItemsOptions
searchResult *upstreamBrowseResponse
}
func (s *recordingPersonsContentService) SearchItems(_ context.Context, _ *Session, opts SearchItemsOptions) (*upstreamBrowseResponse, error) {
s.searchOptions = append(s.searchOptions, opts)
if s.searchResult != nil {
return s.searchResult, nil
}
return &upstreamBrowseResponse{Items: []upstreamListItem{}}, nil
}
func (s *recordingPersonsContentService) ListUserLibraries(context.Context, *Session) ([]upstreamUserLibrary, error) {
panic("unused")
}
func (s *recordingPersonsContentService) BrowseItems(context.Context, *Session, url.Values) (*upstreamBrowseResponse, error) {
panic("unused")
}
func (s *recordingPersonsContentService) GetItemDetail(context.Context, *Session, string, *int) (*upstreamItemDetail, error) {
panic("unused")
}
func (s *recordingPersonsContentService) ListSeasons(context.Context, *Session, string, *int) ([]upstreamSeason, error) {
panic("unused")
}
func (s *recordingPersonsContentService) GetSeason(context.Context, *Session, string, int, *int) (*upstreamSeason, error) {
panic("unused")
}
func (s *recordingPersonsContentService) ListEpisodes(context.Context, *Session, string, int, *int) ([]upstreamEpisode, error) {
panic("unused")
}
func (s *recordingPersonsContentService) ListEpisodesBySeasonID(context.Context, *Session, string, *int) ([]upstreamEpisode, error) {
panic("unused")
}
func (s *recordingPersonsContentService) ListItemFilters(context.Context, *Session, url.Values) (*upstreamItemFiltersResponse, error) {
panic("unused")
}
// TestShouldSuppressSearchPeople_PassesSkipTotalTrue verifies that
// shouldSuppressSearchPeople always sets SkipTotal:true in the SearchItems call
// it issues to probe whether the query matches media titles. Fetching a total
// count on the hot people-search path is unnecessary overhead.
func TestShouldSuppressSearchPeople_PassesSkipTotalTrue(t *testing.T) {
// "Avatar 2" fails looksLikePersonName (has a digit) so the probe fires.
contentSvc := &recordingPersonsContentService{
searchResult: &upstreamBrowseResponse{
Items: []upstreamListItem{
{ContentID: "movie-1", Title: "Avatar 2"},
},
},
}
h := &PersonsHandler{content: contentSvc}
_ = h.shouldSuppressSearchPeople(context.Background(), &Session{StreamAppUserID: 1, ProfileID: "p1"}, "Avatar 2")
if len(contentSvc.searchOptions) != 1 {
t.Fatalf("SearchItems call count = %d, want 1", len(contentSvc.searchOptions))
}
opts := contentSvc.searchOptions[0]
if !opts.SkipTotal {
t.Errorf("SkipTotal = false, want true (total is unnecessary on the probe path)")
}
if want := []string{"movie", "series"}; !slices.Equal(opts.ItemTypes, want) {
t.Errorf("ItemTypes = %#v, want %#v", opts.ItemTypes, want)
}
if opts.Limit != 5 {
t.Errorf("Limit = %d, want 5", opts.Limit)
}
}
// TestShouldSuppressSearchPeople_SkipsProbeForPersonNameShape verifies that
// when the query looks like a person name, SearchItems is never called —
// the handler short-circuits before reaching the media probe.
func TestShouldSuppressSearchPeople_SkipsProbeForPersonNameShape(t *testing.T) {
contentSvc := &recordingPersonsContentService{}
h := &PersonsHandler{content: contentSvc}
// "Tom Hanks" looks like a person name → no probe.
suppressed := h.shouldSuppressSearchPeople(context.Background(), &Session{StreamAppUserID: 1, ProfileID: "p1"}, "Tom Hanks")
if len(contentSvc.searchOptions) != 0 {
t.Errorf("SearchItems called %d times, want 0 for person-name queries", len(contentSvc.searchOptions))
}
if suppressed {
t.Error("suppressed = true for a person-name query, want false")
}
}
// TestShouldSuppressSearchPeople_ReturnsFalseWhenNoMediaMatch verifies that
// the handler does NOT suppress when the media search returns no results.
func TestShouldSuppressSearchPeople_ReturnsFalseWhenNoMediaMatch(t *testing.T) {
// Empty result → should not suppress.
contentSvc := &recordingPersonsContentService{
searchResult: &upstreamBrowseResponse{Items: []upstreamListItem{}},
}
h := &PersonsHandler{content: contentSvc}
suppressed := h.shouldSuppressSearchPeople(context.Background(), &Session{}, "Avatar 2")
if suppressed {
t.Error("suppressed = true when no media matched, want false")
}
}
// TestShouldSuppressSearchPeople_ReturnsTrueOnExactTitleMatch verifies the
// core suppression logic: an exact title match causes the person search to
// be suppressed so clients receive an empty person list.
func TestShouldSuppressSearchPeople_ReturnsTrueOnExactTitleMatch(t *testing.T) {
contentSvc := &recordingPersonsContentService{
searchResult: &upstreamBrowseResponse{
Items: []upstreamListItem{
{ContentID: "movie-1", Title: "Avatar 2"},
},
},
}
h := &PersonsHandler{content: contentSvc}
suppressed := h.shouldSuppressSearchPeople(context.Background(), &Session{}, "Avatar 2")
if !suppressed {
t.Error("suppressed = false for exact title match, want true")
}
}
func TestLooksLikePersonName_AcceptsRealNames(t *testing.T) {
cases := []string{"Christopher Nolan", "Tom Hanks", "Émilie Dequenne", "O'Brien"}