feat: enhance selectedMusicFolderIds function to handle valid music folder IDs and improve fallback logic

Signed-off-by: Deluan <deluan@navidrome.org>

server/subsonic/helpers.go

@@ -482,18 +482,20 @@ func getUserAccessibleLibraries(ctx context.Context) []model.Library {
 	return user.Libraries
 }
 
-// selectedMusicFolderIds returns the music folder IDs from the request parameters.
-// If no IDs are provided, it returns all libraries accessible to the user.
-func selectedMusicFolderIds(r *http.Request) ([]int, error) {
-	user := getUser(r.Context())
+func selectedMusicFolderIds(r *http.Request, required bool) ([]int, error) {
 	p := req.Params(r)
 	musicFolderIds, err := p.Ints("musicFolderId")
-	if !errors.Is(err, req.ErrMissingParam) {
+
+	// If the parameter is not present, it returns an error if it is required.
+	if errors.Is(err, req.ErrMissingParam) && required {
 		return nil, err
 	}
 
 	if len(musicFolderIds) > 0 {
 		return musicFolderIds, nil
 	}
-	return slice.Map(user.Libraries, func(lib model.Library) int { return lib.ID }), nil
+
+	// If no musicFolderId is provided, return all libraries the user has access to.
+	libraries := getUserAccessibleLibraries(r.Context())
+	return slice.Map(libraries, func(lib model.Library) int { return lib.ID }), nil
 }

server/subsonic/helpers_test.go

@@ -1,10 +1,15 @@
 package subsonic
 
 import (
+	"context"
+	"net/http/httptest"
+
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/conf/configtest"
 	"github.com/navidrome/navidrome/model"
+	"github.com/navidrome/navidrome/model/request"
 	"github.com/navidrome/navidrome/server/subsonic/responses"
+	"github.com/navidrome/navidrome/utils/req"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
@@ -163,4 +168,98 @@ var _ = Describe("helpers", func() {
 			Expect(result).To(Equal(int32(4)))
 		})
 	})
+
+	Describe("selectedMusicFolderIds", 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)
+		})
+
+		Context("when musicFolderId parameter is provided", func() {
+			It("should return the specified musicFolderId values", func() {
+				r := httptest.NewRequest("GET", "/test?musicFolderId=1&musicFolderId=3", nil)
+				r = r.WithContext(ctx)
+
+				ids, err := selectedMusicFolderIds(r, false)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ids).To(Equal([]int{1, 3}))
+			})
+
+			It("should ignore invalid musicFolderId parameter values", func() {
+				r := httptest.NewRequest("GET", "/test?musicFolderId=invalid&musicFolderId=2", nil)
+				r = r.WithContext(ctx)
+
+				ids, err := selectedMusicFolderIds(r, false)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ids).To(Equal([]int{2})) // Only valid ID is returned
+			})
+		})
+
+		Context("when musicFolderId parameter is not provided", func() {
+			Context("and required is false", func() {
+				It("should return all user's library IDs", func() {
+					r := httptest.NewRequest("GET", "/test", nil)
+					r = r.WithContext(ctx)
+
+					ids, err := selectedMusicFolderIds(r, false)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(ids).To(Equal([]int{1, 2, 3}))
+				})
+
+				It("should return empty slice when user has no libraries", func() {
+					userWithoutLibs := model.User{ID: "no-libs-user", Libraries: []model.Library{}}
+					ctxWithoutLibs := request.WithUser(context.Background(), userWithoutLibs)
+					r := httptest.NewRequest("GET", "/test", nil)
+					r = r.WithContext(ctxWithoutLibs)
+
+					ids, err := selectedMusicFolderIds(r, false)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(ids).To(Equal([]int{}))
+				})
+			})
+
+			Context("and required is true", func() {
+				It("should return ErrMissingParam error", func() {
+					r := httptest.NewRequest("GET", "/test", nil)
+					r = r.WithContext(ctx)
+
+					ids, err := selectedMusicFolderIds(r, true)
+					Expect(err).To(MatchError(req.ErrMissingParam))
+					Expect(ids).To(BeNil())
+				})
+			})
+		})
+
+		Context("when musicFolderId parameter is empty", func() {
+			It("should return all user's library IDs even when empty parameter is provided", func() {
+				r := httptest.NewRequest("GET", "/test?musicFolderId=", nil)
+				r = r.WithContext(ctx)
+
+				ids, err := selectedMusicFolderIds(r, false)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ids).To(Equal([]int{1, 2, 3}))
+			})
+		})
+
+		Context("when all musicFolderId parameters are invalid", func() {
+			It("should return all user libraries when all musicFolderId parameters are invalid", func() {
+				r := httptest.NewRequest("GET", "/test?musicFolderId=invalid&musicFolderId=notanumber", nil)
+				r = r.WithContext(ctx)
+
+				ids, err := selectedMusicFolderIds(r, false)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(ids).To(Equal([]int{1, 2, 3})) // Falls back to all user libraries
+			})
+		})
+	})
 })