diff --git a/model/artist.go b/model/artist.go index f6a92a313..309ee800f 100644 --- a/model/artist.go +++ b/model/artist.go @@ -78,7 +78,7 @@ type ArtistRepository interface { UpdateExternalInfo(a *Artist) error Get(id string) (*Artist, error) GetAll(options ...QueryOptions) (Artists, error) - GetIndex(includeMissing bool, libraryId int, roles ...Role) (ArtistIndexes, error) + GetIndex(includeMissing bool, libraryIds []int, roles ...Role) (ArtistIndexes, error) // The following methods are used exclusively by the scanner: RefreshPlayCounts() (int64, error) diff --git a/persistence/artist_repository.go b/persistence/artist_repository.go index 45e696dc2..5a3aff157 100644 --- a/persistence/artist_repository.go +++ b/persistence/artist_repository.go @@ -224,8 +224,15 @@ func (r *artistRepository) getIndexKey(a model.Artist) string { return "#" } -// TODO Cache the index (recalculate when there are changes to the DB) -func (r *artistRepository) GetIndex(includeMissing bool, libraryId int, roles ...model.Role) (model.ArtistIndexes, error) { +// GetIndex returns a list of artists grouped by the first letter of their name, or by the index group if configured. +// It can filter by roles and libraries, and optionally include artists that are missing (i.e., have no albums). +// TODO Cache the index (recalculate at scan time) +func (r *artistRepository) GetIndex(includeMissing bool, libraryIds []int, roles ...model.Role) (model.ArtistIndexes, error) { + // Validate library IDs. If no library IDs are provided, return an empty index. + if len(libraryIds) == 0 { + return nil, nil + } + options := model.QueryOptions{Sort: "name"} if len(roles) > 0 { roleFilters := slice.Map(roles, func(r model.Role) Sqlizer { @@ -241,32 +248,14 @@ func (r *artistRepository) GetIndex(includeMissing bool, libraryId int, roles .. } } - // Get artists based on library filtering - var artists model.Artists - var err error - - var libFilter Sqlizer - - if libraryId > 0 { - libFilter = artistLibraryFilter("library_id", libraryId) - } else { - // All accessible libraries - use regular GetAll for backward compatibility - userRepo := NewUserRepository(r.ctx, r.db) - libs, err := userRepo.GetUserLibraries(userId(r.ctx)) - if err != nil { - return nil, fmt.Errorf("getting user libraries: %w", err) - } - libIds := slice.Map(libs, func(lib model.Library) int { return lib.ID }) - libFilter = artistLibraryFilter("library_id", libIds) - } - + libFilter := artistLibraryFilter("library_id", libraryIds) if options.Filters == nil { options.Filters = libFilter } else { options.Filters = And{options.Filters, libFilter} } - artists, err = r.GetAll(options) + artists, err := r.GetAll(options) if err != nil { return nil, err } diff --git a/persistence/artist_repository_test.go b/persistence/artist_repository_test.go index c9062a950..57fbc34e1 100644 --- a/persistence/artist_repository_test.go +++ b/persistence/artist_repository_test.go @@ -96,7 +96,7 @@ var _ = Describe("ArtistRepository", func() { er := repo.Put(&artistBeatles) Expect(er).To(BeNil()) - idx, err := repo.GetIndex(false, 0) + idx, err := repo.GetIndex(false, []int{1}) Expect(err).ToNot(HaveOccurred()) Expect(idx).To(HaveLen(2)) Expect(idx[0].ID).To(Equal("F")) @@ -114,7 +114,7 @@ var _ = Describe("ArtistRepository", func() { // BFR Empty SortArtistName is not saved in the DB anymore XIt("returns the index when PreferSortTags is true and SortArtistName is empty", func() { - idx, err := repo.GetIndex(false, 0) + idx, err := repo.GetIndex(false, []int{1}) Expect(err).ToNot(HaveOccurred()) Expect(idx).To(HaveLen(2)) Expect(idx[0].ID).To(Equal("B")) @@ -136,7 +136,7 @@ var _ = Describe("ArtistRepository", func() { er := repo.Put(&artistBeatles) Expect(er).To(BeNil()) - idx, err := repo.GetIndex(false, 0) + idx, err := repo.GetIndex(false, []int{1}) Expect(err).ToNot(HaveOccurred()) Expect(idx).To(HaveLen(2)) Expect(idx[0].ID).To(Equal("B")) @@ -153,7 +153,7 @@ var _ = Describe("ArtistRepository", func() { }) It("returns the index when SortArtistName is empty", func() { - idx, err := repo.GetIndex(false, 0) + idx, err := repo.GetIndex(false, []int{1}) Expect(err).ToNot(HaveOccurred()) Expect(idx).To(HaveLen(2)) Expect(idx[0].ID).To(Equal("B")) @@ -190,7 +190,7 @@ var _ = Describe("ArtistRepository", func() { }) It("returns only artists with the specified role", func() { - idx, err := repo.GetIndex(false, 0, model.RoleComposer) + idx, err := repo.GetIndex(false, []int{1}, model.RoleComposer) Expect(err).ToNot(HaveOccurred()) Expect(idx).To(HaveLen(1)) Expect(idx[0].ID).To(Equal("B")) @@ -199,7 +199,7 @@ var _ = Describe("ArtistRepository", func() { }) It("returns artists with any of the specified roles", func() { - idx, err := repo.GetIndex(false, 0, model.RoleComposer, model.RoleProducer) + idx, err := repo.GetIndex(false, []int{1}, model.RoleComposer, model.RoleProducer) Expect(err).ToNot(HaveOccurred()) Expect(idx).To(HaveLen(2)) @@ -220,7 +220,28 @@ var _ = Describe("ArtistRepository", func() { }) It("returns empty index when no artists have the specified role", func() { - idx, err := repo.GetIndex(false, 0, model.RoleDirector) + idx, err := repo.GetIndex(false, []int{1}, model.RoleDirector) + Expect(err).ToNot(HaveOccurred()) + Expect(idx).To(HaveLen(0)) + }) + }) + + When("validating library IDs", func() { + It("returns nil when no library IDs are provided", func() { + idx, err := repo.GetIndex(false, []int{}) + Expect(err).ToNot(HaveOccurred()) + Expect(idx).To(BeNil()) + }) + + It("returns artists from multiple libraries when multiple library IDs are provided", func() { + // This test would require setting up multiple libraries + // For now, test with the existing library ID 1 only + idx, err := repo.GetIndex(false, []int{1}) + Expect(err).ToNot(HaveOccurred()) + Expect(idx).To(HaveLen(2)) + + // Test with a non-existent library ID should return empty result + idx, err = repo.GetIndex(false, []int{999}) Expect(err).ToNot(HaveOccurred()) Expect(idx).To(HaveLen(0)) }) @@ -377,7 +398,7 @@ var _ = Describe("ArtistRepository", func() { }) It("does not return missing artist in GetIndex", func() { - idx, err := repo.GetIndex(false, 0) + idx, err := repo.GetIndex(false, []int{1}) Expect(err).ToNot(HaveOccurred()) // Only 2 artists should be present total := 0 @@ -411,7 +432,7 @@ var _ = Describe("ArtistRepository", func() { }) It("returns missing artist in GetIndex when included", func() { - idx, err := repo.GetIndex(true, 0) + idx, err := repo.GetIndex(true, []int{1}) Expect(err).ToNot(HaveOccurred()) total := 0 for _, ix := range idx { diff --git a/server/subsonic/browsing.go b/server/subsonic/browsing.go index 9bd4966a9..c8584543d 100644 --- a/server/subsonic/browsing.go +++ b/server/subsonic/browsing.go @@ -30,7 +30,7 @@ func (api *Router) GetMusicFolders(r *http.Request) (*responses.Subsonic, error) return response, nil } -func (api *Router) getArtist(r *http.Request, libId int, ifModifiedSince time.Time) (model.ArtistIndexes, int64, error) { +func (api *Router) getArtist(r *http.Request, libIds []int, ifModifiedSince time.Time) (model.ArtistIndexes, int64, error) { ctx := r.Context() lastScanStr, err := api.ds.Property(ctx).DefaultGet(consts.LastScanStartTimeKey, "") @@ -45,13 +45,13 @@ func (api *Router) getArtist(r *http.Request, libId int, ifModifiedSince time.Ti var indexes model.ArtistIndexes if lastScan.After(ifModifiedSince) { - indexes, err = api.ds.Artist(ctx).GetIndex(false, libId, model.RoleAlbumArtist) + indexes, err = api.ds.Artist(ctx).GetIndex(false, libIds, model.RoleAlbumArtist) if err != nil { log.Error(ctx, "Error retrieving Indexes", err) return nil, 0, err } if len(indexes) == 0 { - log.Debug(ctx, "No artists found in library", "libId", libId) + log.Debug(ctx, "No artists found in library", "libId", libIds) return nil, 0, newError(responses.ErrorDataNotFound, "Library not found or empty") } } @@ -59,8 +59,8 @@ func (api *Router) getArtist(r *http.Request, libId int, ifModifiedSince time.Ti return indexes, lastScan.UnixMilli(), err } -func (api *Router) getArtistIndex(r *http.Request, libId int, ifModifiedSince time.Time) (*responses.Indexes, error) { - indexes, modified, err := api.getArtist(r, libId, ifModifiedSince) +func (api *Router) getArtistIndex(r *http.Request, libIds []int, ifModifiedSince time.Time) (*responses.Indexes, error) { + indexes, modified, err := api.getArtist(r, libIds, ifModifiedSince) if err != nil { return nil, err } @@ -78,8 +78,8 @@ func (api *Router) getArtistIndex(r *http.Request, libId int, ifModifiedSince ti return res, nil } -func (api *Router) getArtistIndexID3(r *http.Request, libId int, ifModifiedSince time.Time) (*responses.Artists, error) { - indexes, modified, err := api.getArtist(r, libId, ifModifiedSince) +func (api *Router) getArtistIndexID3(r *http.Request, libIds []int, ifModifiedSince time.Time) (*responses.Artists, error) { + indexes, modified, err := api.getArtist(r, libIds, ifModifiedSince) if err != nil { return nil, err } @@ -99,10 +99,10 @@ func (api *Router) getArtistIndexID3(r *http.Request, libId int, ifModifiedSince func (api *Router) GetIndexes(r *http.Request) (*responses.Subsonic, error) { p := req.Params(r) - musicFolderId := p.IntOr("musicFolderId", 0) // 0 means all accessible libraries + musicFolderIds, _ := selectedMusicFolderIds(r, false) ifModifiedSince := p.TimeOr("ifModifiedSince", time.Time{}) - res, err := api.getArtistIndex(r, musicFolderId, ifModifiedSince) + res, err := api.getArtistIndex(r, musicFolderIds, ifModifiedSince) if err != nil { return nil, err } @@ -113,10 +113,9 @@ func (api *Router) GetIndexes(r *http.Request) (*responses.Subsonic, error) { } func (api *Router) GetArtists(r *http.Request) (*responses.Subsonic, error) { - p := req.Params(r) - musicFolderId := p.IntOr("musicFolderId", 0) // 0 means all accessible libraries + musicFolderIds, _ := selectedMusicFolderIds(r, false) - res, err := api.getArtistIndexID3(r, musicFolderId, time.Time{}) + res, err := api.getArtistIndexID3(r, musicFolderIds, time.Time{}) if err != nil { return nil, err } diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index 2a139063d..0e387fafc 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -482,6 +482,9 @@ func getUserAccessibleLibraries(ctx context.Context) []model.Library { return user.Libraries } +// selectedMusicFolderIds retrieves the music folder IDs from the request parameters. +// If no IDs are provided, it returns all libraries the user has access to (based on the user found in the context). +// If the parameter is required and not present, it returns an error. func selectedMusicFolderIds(r *http.Request, required bool) ([]int, error) { p := req.Params(r) musicFolderIds, err := p.Ints("musicFolderId") diff --git a/tests/mock_artist_repo.go b/tests/mock_artist_repo.go index 9387c227a..760d089af 100644 --- a/tests/mock_artist_repo.go +++ b/tests/mock_artist_repo.go @@ -108,7 +108,7 @@ func (m *MockArtistRepo) RefreshPlayCounts() (int64, error) { return int64(len(m.Data)), nil } -func (m *MockArtistRepo) GetIndex(includeMissing bool, libraryId int, roles ...model.Role) (model.ArtistIndexes, error) { +func (m *MockArtistRepo) GetIndex(includeMissing bool, libraryIds []int, roles ...model.Role) (model.ArtistIndexes, error) { if m.Err { return nil, errors.New("mock repo error") }