mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-28 22:27:10 +01:00
fix(jellyfin): artist search 500 (library filter leaked into FTS query)
getArtists/listArtists reused the browse-path filters (notMissing +
ApplyArtistLibraryFilter) for the search path, but artist Search expects a
sole Eq{library_id} filter it can consume as a scope — artists have no
library_id column, so the compound/join filter leaked into the FTS query
and 500'd. Every Finamp artist search failed (the filter is applied even
unscoped, since admins resolve to all library ids). Build search filters
separately. Adds e2e search coverage.
This commit is contained in:
parent
b10ec9e9c9
commit
a2b8d8c0d0
2 changed files with 61 additions and 8 deletions
43
server/jellyfin/e2e/search_test.go
Normal file
43
server/jellyfin/e2e/search_test.go
Normal file
|
|
@ -0,0 +1,43 @@
|
|||
package e2e
|
||||
|
||||
import (
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
// Search with a ParentId library scope is how Finamp drives its search screen. Artists are the
|
||||
// tricky case: they have no library_id column, so the repo's Search does its own scope handling.
|
||||
var _ = Describe("Search", func() {
|
||||
BeforeEach(func() { setupTestDB() })
|
||||
|
||||
lib1 := func() string { return enc("1") } // Library id 1 encodes to "31"
|
||||
|
||||
Describe("artists", func() {
|
||||
It("searches all album artists", func() {
|
||||
q := queryResult(get("/Artists/AlbumArtists?SearchTerm=Beatles"))
|
||||
Expect(names(q.Items)).To(ConsistOf("The Beatles"))
|
||||
})
|
||||
|
||||
It("searches album artists scoped to a library (ParentId)", func() {
|
||||
q := queryResult(get("/Artists/AlbumArtists?ParentId=" + lib1() + "&SearchTerm=Beatles&Recursive=true&SortBy=SortName"))
|
||||
Expect(names(q.Items)).To(ConsistOf("The Beatles"))
|
||||
})
|
||||
|
||||
It("returns an empty result for a non-matching term", func() {
|
||||
q := queryResult(get("/Artists?ParentId=" + lib1() + "&SearchTerm=nonexistentxyz"))
|
||||
Expect(q.Items).To(BeEmpty())
|
||||
})
|
||||
})
|
||||
|
||||
Describe("albums and songs", func() {
|
||||
It("searches albums scoped to a library", func() {
|
||||
q := queryResult(get("/Items?IncludeItemTypes=MusicAlbum&Recursive=true&ParentId=" + lib1() + "&SearchTerm=Abbey"))
|
||||
Expect(names(q.Items)).To(ContainElement("Abbey Road"))
|
||||
})
|
||||
|
||||
It("searches songs scoped to a library", func() {
|
||||
q := queryResult(get("/Items?IncludeItemTypes=Audio&Recursive=true&ParentId=" + lib1() + "&SearchTerm=Stairway"))
|
||||
Expect(names(q.Items)).To(ContainElement("Stairway To Heaven"))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -197,20 +197,30 @@ func (api *Router) listSongs(ctx context.Context, opts model.QueryOptions, paren
|
|||
|
||||
func (api *Router) listArtists(ctx context.Context, opts model.QueryOptions, scopeIDs []int, search string, fav bool) (dto.QueryResult, error) {
|
||||
repo := api.ds.Artist(ctx)
|
||||
|
||||
// Artist Search does its own library scoping: it consumes a sole Eq{"library_id": ...} filter
|
||||
// (artists have no library_id column, so it can't be a real WHERE clause) and realizes it as a
|
||||
// search scope. The join-based ApplyArtistLibraryFilter, or any compound filter, would leak
|
||||
// library_artist.library_id into the FTS query and 500. So the search and browse paths build
|
||||
// their filters differently. See persistence/artist_repository.go's searchScope.
|
||||
if search != "" {
|
||||
if len(scopeIDs) > 0 {
|
||||
opts.Filters = squirrel.Eq{"library_id": scopeIDs}
|
||||
}
|
||||
artists, err := repo.Search(search, opts)
|
||||
if err != nil {
|
||||
return dto.QueryResult{}, err
|
||||
}
|
||||
return result(slice.Map(artists, dto.ArtistToBaseItem), len(artists), opts.Offset), nil
|
||||
}
|
||||
|
||||
if fav {
|
||||
opts.Filters = filter.ArtistsByStarred().Filters
|
||||
} else {
|
||||
opts.Filters = notMissing
|
||||
}
|
||||
opts = filter.ApplyArtistLibraryFilter(opts, scopeIDs)
|
||||
|
||||
var artists model.Artists
|
||||
var err error
|
||||
if search != "" {
|
||||
artists, err = repo.Search(search, opts)
|
||||
} else {
|
||||
artists, err = repo.GetAll(opts)
|
||||
}
|
||||
artists, err := repo.GetAll(opts)
|
||||
if err != nil {
|
||||
return dto.QueryResult{}, err
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue