feat: implement multi-library support for GetRandomSongs, GetSongsByGenre, GetStarred, and GetStarred2

- Added multi-library filtering to GetRandomSongs endpoint using musicFolderId parameter - Added multi-library filtering to GetSongsByGenre endpoint using musicFolderId parameter - Enhanced GetStarred and GetStarred2 to filter artists, albums, and songs by library - Added Options field to MockMediaFileRepo and MockArtistRepo for test compatibility - Added comprehensive Ginkgo/Gomega tests for all new multi-library functionality - All tests verify proper SQL filter generation and library access validation - Supports single/multiple musicFolderId values with fallback to all accessible libraries
2026-10-27 21:57:17 +01:00 · 2025-06-27 11:48:07 -04:00 · 2025-06-27 11:48:07 -04:00 · a93fa53ae7
commit a93fa53ae7
parent b9e7b7cad7
4 changed files with 367 additions and 13 deletions
--- a/server/subsonic/album_lists.go
+++ b/server/subsonic/album_lists.go
@ -115,18 +115,32 @@ func (api *Router) GetAlbumList2(w http.ResponseWriter, r *http.Request) (*respo

 func (api *Router) GetStarred(r *http.Request) (*responses.Subsonic, error) {
 	ctx := r.Context()
-	artists, err := api.ds.Artist(ctx).GetAll(filter.ArtistsByStarred())
+
+	// Get optional library IDs from musicFolderId parameter
+	musicFolderIds, _ := selectedMusicFolderIds(r, false)
+
+	// Get starred artists with library filtering
+	artistOpts := filter.ArtistsByStarred()
+	artistOpts = filter.ApplyLibraryFilter(artistOpts, musicFolderIds)
+	artists, err := api.ds.Artist(ctx).GetAll(artistOpts)
 	if err != nil {
 		log.Error(r, "Error retrieving starred artists", err)
 		return nil, err
 	}
-	options := filter.ByStarred()
-	albums, err := api.ds.Album(ctx).GetAll(options)
+
+	// Get starred albums with library filtering
+	albumOpts := filter.ByStarred()
+	albumOpts = filter.ApplyLibraryFilter(albumOpts, musicFolderIds)
+	albums, err := api.ds.Album(ctx).GetAll(albumOpts)
 	if err != nil {
 		log.Error(r, "Error retrieving starred albums", err)
 		return nil, err
 	}
-	mediaFiles, err := api.ds.MediaFile(ctx).GetAll(options)
+
+	// Get starred media files with library filtering
+	mediaFileOpts := filter.ByStarred()
+	mediaFileOpts = filter.ApplyLibraryFilter(mediaFileOpts, musicFolderIds)
+	mediaFiles, err := api.ds.MediaFile(ctx).GetAll(mediaFileOpts)
 	if err != nil {
 		log.Error(r, "Error retrieving starred mediaFiles", err)
 		return nil, err
@ -142,18 +156,32 @@ func (api *Router) GetStarred(r *http.Request) (*responses.Subsonic, error) {

 func (api *Router) GetStarred2(r *http.Request) (*responses.Subsonic, error) {
 	ctx := r.Context()
-	artists, err := api.ds.Artist(ctx).GetAll(filter.ArtistsByStarred())
+
+	// Get optional library IDs from musicFolderId parameter
+	musicFolderIds, _ := selectedMusicFolderIds(r, false)
+
+	// Get starred artists with library filtering
+	artistOpts := filter.ArtistsByStarred()
+	artistOpts = filter.ApplyLibraryFilter(artistOpts, musicFolderIds)
+	artists, err := api.ds.Artist(ctx).GetAll(artistOpts)
 	if err != nil {
 		log.Error(r, "Error retrieving starred artists", err)
 		return nil, err
 	}
-	options := filter.ByStarred()
-	albums, err := api.ds.Album(ctx).GetAll(options)
+
+	// Get starred albums with library filtering
+	albumOpts := filter.ByStarred()
+	albumOpts = filter.ApplyLibraryFilter(albumOpts, musicFolderIds)
+	albums, err := api.ds.Album(ctx).GetAll(albumOpts)
 	if err != nil {
 		log.Error(r, "Error retrieving starred albums", err)
 		return nil, err
 	}
-	mediaFiles, err := api.ds.MediaFile(ctx).GetAll(options)
+
+	// Get starred media files with library filtering
+	mediaFileOpts := filter.ByStarred()
+	mediaFileOpts = filter.ApplyLibraryFilter(mediaFileOpts, musicFolderIds)
+	mediaFiles, err := api.ds.MediaFile(ctx).GetAll(mediaFileOpts)
 	if err != nil {
 		log.Error(r, "Error retrieving starred mediaFiles", err)
 		return nil, err
@ -197,7 +225,12 @@ func (api *Router) GetRandomSongs(r *http.Request) (*responses.Subsonic, error)
 	fromYear := p.IntOr("fromYear", 0)
 	toYear := p.IntOr("toYear", 0)

-	songs, err := api.getSongs(r.Context(), 0, size, filter.SongsByRandom(genre, fromYear, toYear))
+	// Get optional library IDs from musicFolderId parameter
+	musicFolderIds, _ := selectedMusicFolderIds(r, false)
+	opts := filter.SongsByRandom(genre, fromYear, toYear)
+	opts = filter.ApplyLibraryFilter(opts, musicFolderIds)
+
+	songs, err := api.getSongs(r.Context(), 0, size, opts)
 	if err != nil {
 		log.Error(r, "Error retrieving random songs", err)
 		return nil, err
@ -215,8 +248,13 @@ func (api *Router) GetSongsByGenre(r *http.Request) (*responses.Subsonic, error)
 	offset := p.IntOr("offset", 0)
 	genre, _ := p.String("genre")

+	// Get optional library IDs from musicFolderId parameter
+	musicFolderIds, _ := selectedMusicFolderIds(r, false)
+	opts := filter.ByGenre(genre)
+	opts = filter.ApplyLibraryFilter(opts, musicFolderIds)
+
 	ctx := r.Context()
-	songs, err := api.getSongs(ctx, offset, count, filter.ByGenre(genre))
+	songs, err := api.getSongs(ctx, offset, count, opts)
 	if err != nil {
 		log.Error(r, "Error retrieving random songs", err)
 		return nil, err
--- a/server/subsonic/album_lists_test.go
+++ b/server/subsonic/album_lists_test.go
@ -230,4 +230,312 @@ var _ = Describe("Album Lists", func() {
 			})
 		})
 	})
+
+	Describe("GetRandomSongs", func() {
+		var mockMediaFileRepo *tests.MockMediaFileRepo
+
+		BeforeEach(func() {
+			mockMediaFileRepo = ds.MediaFile(ctx).(*tests.MockMediaFileRepo)
+		})
+
+		It("should return random songs", func() {
+			mockMediaFileRepo.SetData(model.MediaFiles{
+				{ID: "1", Title: "Song 1"},
+				{ID: "2", Title: "Song 2"},
+			})
+			r := newGetRequest("size=2")
+
+			resp, err := router.GetRandomSongs(r)
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(resp.RandomSongs.Songs).To(HaveLen(2))
+		})
+
+		Context("with musicFolderId parameter", func() {
+			var user model.User
+			var ctx context.Context
+
+			BeforeEach(func() {
+				user = model.User{
+					ID: "test-user",
+					Libraries: []model.Library{
+						{ID: 1, Name: "Library 1"},
+						{ID: 2, Name: "Library 2"},
+						{ID: 3, Name: "Library 3"},
+					},
+				}
+				ctx = request.WithUser(context.Background(), user)
+			})
+
+			It("should filter songs by specific library when musicFolderId is provided", func() {
+				mockMediaFileRepo.SetData(model.MediaFiles{
+					{ID: "1", Title: "Song 1"},
+					{ID: "2", Title: "Song 2"},
+				})
+				r := newGetRequest("size=2", "musicFolderId=1")
+				r = r.WithContext(ctx)
+
+				resp, err := router.GetRandomSongs(r)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(resp.RandomSongs.Songs).To(HaveLen(2))
+				// Verify that library filter was applied
+				query, args, _ := mockMediaFileRepo.Options.Filters.ToSql()
+				Expect(query).To(ContainSubstring("library_id IN (?)"))
+				Expect(args).To(ContainElement(1))
+			})
+
+			It("should filter songs by multiple libraries when multiple musicFolderId are provided", func() {
+				mockMediaFileRepo.SetData(model.MediaFiles{
+					{ID: "1", Title: "Song 1"},
+					{ID: "2", Title: "Song 2"},
+				})
+				r := newGetRequest("size=2", "musicFolderId=1", "musicFolderId=2")
+				r = r.WithContext(ctx)
+
+				resp, err := router.GetRandomSongs(r)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(resp.RandomSongs.Songs).To(HaveLen(2))
+				// Verify that library filter was applied
+				query, args, _ := mockMediaFileRepo.Options.Filters.ToSql()
+				Expect(query).To(ContainSubstring("library_id IN (?,?)"))
+				Expect(args).To(ContainElements(1, 2))
+			})
+
+			It("should return all accessible songs when no musicFolderId is provided", func() {
+				mockMediaFileRepo.SetData(model.MediaFiles{
+					{ID: "1", Title: "Song 1"},
+					{ID: "2", Title: "Song 2"},
+				})
+				r := newGetRequest("size=2")
+				r = r.WithContext(ctx)
+
+				resp, err := router.GetRandomSongs(r)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(resp.RandomSongs.Songs).To(HaveLen(2))
+				// Verify that library filter was applied
+				query, args, _ := mockMediaFileRepo.Options.Filters.ToSql()
+				Expect(query).To(ContainSubstring("library_id IN (?,?,?)"))
+				Expect(args).To(ContainElements(1, 2, 3))
+			})
+		})
+	})
+
+	Describe("GetSongsByGenre", func() {
+		var mockMediaFileRepo *tests.MockMediaFileRepo
+
+		BeforeEach(func() {
+			mockMediaFileRepo = ds.MediaFile(ctx).(*tests.MockMediaFileRepo)
+		})
+
+		It("should return songs by genre", func() {
+			mockMediaFileRepo.SetData(model.MediaFiles{
+				{ID: "1", Title: "Song 1"},
+				{ID: "2", Title: "Song 2"},
+			})
+			r := newGetRequest("count=2", "genre=rock")
+
+			resp, err := router.GetSongsByGenre(r)
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(resp.SongsByGenre.Songs).To(HaveLen(2))
+		})
+
+		Context("with musicFolderId parameter", func() {
+			var user model.User
+			var ctx context.Context
+
+			BeforeEach(func() {
+				user = model.User{
+					ID: "test-user",
+					Libraries: []model.Library{
+						{ID: 1, Name: "Library 1"},
+						{ID: 2, Name: "Library 2"},
+						{ID: 3, Name: "Library 3"},
+					},
+				}
+				ctx = request.WithUser(context.Background(), user)
+			})
+
+			It("should filter songs by specific library when musicFolderId is provided", func() {
+				mockMediaFileRepo.SetData(model.MediaFiles{
+					{ID: "1", Title: "Song 1"},
+					{ID: "2", Title: "Song 2"},
+				})
+				r := newGetRequest("count=2", "genre=rock", "musicFolderId=1")
+				r = r.WithContext(ctx)
+
+				resp, err := router.GetSongsByGenre(r)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(resp.SongsByGenre.Songs).To(HaveLen(2))
+				// Verify that library filter was applied
+				query, args, _ := mockMediaFileRepo.Options.Filters.ToSql()
+				Expect(query).To(ContainSubstring("library_id IN (?)"))
+				Expect(args).To(ContainElement(1))
+			})
+
+			It("should filter songs by multiple libraries when multiple musicFolderId are provided", func() {
+				mockMediaFileRepo.SetData(model.MediaFiles{
+					{ID: "1", Title: "Song 1"},
+					{ID: "2", Title: "Song 2"},
+				})
+				r := newGetRequest("count=2", "genre=rock", "musicFolderId=1", "musicFolderId=2")
+				r = r.WithContext(ctx)
+
+				resp, err := router.GetSongsByGenre(r)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(resp.SongsByGenre.Songs).To(HaveLen(2))
+				// Verify that library filter was applied
+				query, args, _ := mockMediaFileRepo.Options.Filters.ToSql()
+				Expect(query).To(ContainSubstring("library_id IN (?,?)"))
+				Expect(args).To(ContainElements(1, 2))
+			})
+
+			It("should return all accessible songs when no musicFolderId is provided", func() {
+				mockMediaFileRepo.SetData(model.MediaFiles{
+					{ID: "1", Title: "Song 1"},
+					{ID: "2", Title: "Song 2"},
+				})
+				r := newGetRequest("count=2", "genre=rock")
+				r = r.WithContext(ctx)
+
+				resp, err := router.GetSongsByGenre(r)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(resp.SongsByGenre.Songs).To(HaveLen(2))
+				// Verify that library filter was applied
+				query, args, _ := mockMediaFileRepo.Options.Filters.ToSql()
+				Expect(query).To(ContainSubstring("library_id IN (?,?,?)"))
+				Expect(args).To(ContainElements(1, 2, 3))
+			})
+		})
+	})
+
+	Describe("GetStarred", func() {
+		var mockArtistRepo *tests.MockArtistRepo
+		var mockAlbumRepo *tests.MockAlbumRepo
+		var mockMediaFileRepo *tests.MockMediaFileRepo
+
+		BeforeEach(func() {
+			mockArtistRepo = ds.Artist(ctx).(*tests.MockArtistRepo)
+			mockAlbumRepo = ds.Album(ctx).(*tests.MockAlbumRepo)
+			mockMediaFileRepo = ds.MediaFile(ctx).(*tests.MockMediaFileRepo)
+		})
+
+		It("should return starred items", func() {
+			mockArtistRepo.SetData(model.Artists{{ID: "1", Name: "Artist 1"}})
+			mockAlbumRepo.SetData(model.Albums{{ID: "1", Name: "Album 1"}})
+			mockMediaFileRepo.SetData(model.MediaFiles{{ID: "1", Title: "Song 1"}})
+			r := newGetRequest()
+
+			resp, err := router.GetStarred(r)
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(resp.Starred.Artist).To(HaveLen(1))
+			Expect(resp.Starred.Album).To(HaveLen(1))
+			Expect(resp.Starred.Song).To(HaveLen(1))
+		})
+
+		Context("with musicFolderId parameter", func() {
+			var user model.User
+			var ctx context.Context
+
+			BeforeEach(func() {
+				user = model.User{
+					ID: "test-user",
+					Libraries: []model.Library{
+						{ID: 1, Name: "Library 1"},
+						{ID: 2, Name: "Library 2"},
+						{ID: 3, Name: "Library 3"},
+					},
+				}
+				ctx = request.WithUser(context.Background(), user)
+			})
+
+			It("should filter starred items by specific library when musicFolderId is provided", func() {
+				mockArtistRepo.SetData(model.Artists{{ID: "1", Name: "Artist 1"}})
+				mockAlbumRepo.SetData(model.Albums{{ID: "1", Name: "Album 1"}})
+				mockMediaFileRepo.SetData(model.MediaFiles{{ID: "1", Title: "Song 1"}})
+				r := newGetRequest("musicFolderId=1")
+				r = r.WithContext(ctx)
+
+				resp, err := router.GetStarred(r)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(resp.Starred.Artist).To(HaveLen(1))
+				Expect(resp.Starred.Album).To(HaveLen(1))
+				Expect(resp.Starred.Song).To(HaveLen(1))
+				// Verify that library filter was applied to all types
+				artistQuery, artistArgs, _ := mockArtistRepo.Options.Filters.ToSql()
+				Expect(artistQuery).To(ContainSubstring("library_id IN (?)"))
+				Expect(artistArgs).To(ContainElement(1))
+			})
+		})
+	})
+
+	Describe("GetStarred2", func() {
+		var mockArtistRepo *tests.MockArtistRepo
+		var mockAlbumRepo *tests.MockAlbumRepo
+		var mockMediaFileRepo *tests.MockMediaFileRepo
+
+		BeforeEach(func() {
+			mockArtistRepo = ds.Artist(ctx).(*tests.MockArtistRepo)
+			mockAlbumRepo = ds.Album(ctx).(*tests.MockAlbumRepo)
+			mockMediaFileRepo = ds.MediaFile(ctx).(*tests.MockMediaFileRepo)
+		})
+
+		It("should return starred items in ID3 format", func() {
+			mockArtistRepo.SetData(model.Artists{{ID: "1", Name: "Artist 1"}})
+			mockAlbumRepo.SetData(model.Albums{{ID: "1", Name: "Album 1"}})
+			mockMediaFileRepo.SetData(model.MediaFiles{{ID: "1", Title: "Song 1"}})
+			r := newGetRequest()
+
+			resp, err := router.GetStarred2(r)
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(resp.Starred2.Artist).To(HaveLen(1))
+			Expect(resp.Starred2.Album).To(HaveLen(1))
+			Expect(resp.Starred2.Song).To(HaveLen(1))
+		})
+
+		Context("with musicFolderId parameter", func() {
+			var user model.User
+			var ctx context.Context
+
+			BeforeEach(func() {
+				user = model.User{
+					ID: "test-user",
+					Libraries: []model.Library{
+						{ID: 1, Name: "Library 1"},
+						{ID: 2, Name: "Library 2"},
+						{ID: 3, Name: "Library 3"},
+					},
+				}
+				ctx = request.WithUser(context.Background(), user)
+			})
+
+			It("should filter starred items by specific library when musicFolderId is provided", func() {
+				mockArtistRepo.SetData(model.Artists{{ID: "1", Name: "Artist 1"}})
+				mockAlbumRepo.SetData(model.Albums{{ID: "1", Name: "Album 1"}})
+				mockMediaFileRepo.SetData(model.MediaFiles{{ID: "1", Title: "Song 1"}})
+				r := newGetRequest("musicFolderId=1")
+				r = r.WithContext(ctx)
+
+				resp, err := router.GetStarred2(r)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(resp.Starred2.Artist).To(HaveLen(1))
+				Expect(resp.Starred2.Album).To(HaveLen(1))
+				Expect(resp.Starred2.Song).To(HaveLen(1))
+				// Verify that library filter was applied to all types
+				artistQuery, artistArgs, _ := mockArtistRepo.Options.Filters.ToSql()
+				Expect(artistQuery).To(ContainSubstring("library_id IN (?)"))
+				Expect(artistArgs).To(ContainElement(1))
+			})
+		})
+	})
 })
--- a/tests/mock_artist_repo.go
+++ b/tests/mock_artist_repo.go
@ -16,8 +16,9 @@ func CreateMockArtistRepo() *MockArtistRepo {

 type MockArtistRepo struct {
 	model.ArtistRepository
-	Data map[string]*model.Artist
-	Err  bool
+	Data    map[string]*model.Artist
+	Err     bool
+	Options model.QueryOptions
 }

 func (m *MockArtistRepo) SetError(err bool) {
@ -73,6 +74,9 @@ func (m *MockArtistRepo) IncPlayCount(id string, timestamp time.Time) error {
 }

 func (m *MockArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, error) {
+	if len(options) > 0 {
+		m.Options = options[0]
+	}
 	if m.Err {
 		return nil, errors.New("mock repo error")
 	}
--- a/tests/mock_mediafile_repo.go
+++ b/tests/mock_mediafile_repo.go
@ -27,6 +27,7 @@ type MockMediaFileRepo struct {
 	CountAllValue         int64
 	CountAllOptions       model.QueryOptions
 	DeleteAllMissingValue int64
+	Options               model.QueryOptions
 }

 func (m *MockMediaFileRepo) SetError(err bool) {
@ -72,7 +73,10 @@ func (m *MockMediaFileRepo) GetWithParticipants(id string) (*model.MediaFile, er
 	return nil, model.ErrNotFound
 }

-func (m *MockMediaFileRepo) GetAll(...model.QueryOptions) (model.MediaFiles, error) {
+func (m *MockMediaFileRepo) GetAll(qo ...model.QueryOptions) (model.MediaFiles, error) {
+	if len(qo) > 0 {
+		m.Options = qo[0]
+	}
 	if m.Err {
 		return nil, errors.New("error")
 	}