refactor: simplify getUserAccessibleLibraries function and update related tests

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan 2025-06-27 09:21:15 -04:00
commit ad0c9ae153
3 changed files with 44 additions and 58 deletions

View file

@ -18,10 +18,7 @@ import (
)
func (api *Router) GetMusicFolders(r *http.Request) (*responses.Subsonic, error) {
libraries, err := getUserAccessibleLibraries(r.Context(), api.ds)
if err != nil {
return nil, err
}
libraries := getUserAccessibleLibraries(r.Context())
folders := make([]responses.MusicFolder, len(libraries))
for i, f := range libraries {

View file

@ -2,6 +2,7 @@ package subsonic
import (
"context"
"fmt"
"net/http/httptest"
"github.com/navidrome/navidrome/core/auth"
@ -12,6 +13,18 @@ import (
. "github.com/onsi/gomega"
)
func contextWithUser(ctx context.Context, userID string, libraryIDs ...int) context.Context {
libraries := make([]model.Library, len(libraryIDs))
for i, id := range libraryIDs {
libraries[i] = model.Library{ID: id, Name: fmt.Sprintf("Test Library %d", id), Path: fmt.Sprintf("/music/library%d", id)}
}
user := model.User{
ID: userID,
Libraries: libraries,
}
return request.WithUser(ctx, user)
}
var _ = Describe("Browsing", func() {
var api *Router
var ctx context.Context
@ -26,16 +39,8 @@ var _ = Describe("Browsing", func() {
Describe("GetMusicFolders", func() {
It("should return all libraries the user has access", func() {
// Create mock user
user := model.User{
ID: "user-id",
}
ctx = request.WithUser(ctx, user)
// Setup mock expectations - admin users get all libraries via GetUserLibraries
mockUserRepo := ds.User(ctx).(*tests.MockedUserRepo)
err := mockUserRepo.SetUserLibraries("user-id", []int{1, 2, 3})
Expect(err).ToNot(HaveOccurred())
// Create mock user with libraries
ctx := contextWithUser(ctx, "user-id", 1, 2, 3)
// Create request
r := httptest.NewRequest("GET", "/rest/getMusicFolders", nil)
@ -57,16 +62,8 @@ var _ = Describe("Browsing", func() {
Describe("GetIndexes", func() {
It("should validate user access to the specified musicFolderId", func() {
// Create mock user
user := model.User{
ID: "user-id",
}
ctx = request.WithUser(ctx, user)
// Setup mock expectations - user only has access to library 1
mockUserRepo := ds.User(ctx).(*tests.MockedUserRepo)
err := mockUserRepo.SetUserLibraries("user-id", []int{1})
Expect(err).ToNot(HaveOccurred())
// Create mock user with access to library 1 only
ctx = contextWithUser(ctx, "user-id", 1)
// Create request with musicFolderId=2 (not accessible)
r := httptest.NewRequest("GET", "/rest/getIndexes?musicFolderId=2", nil)
@ -81,11 +78,8 @@ var _ = Describe("Browsing", func() {
})
It("should default to first accessible library when no musicFolderId specified", func() {
// Create mock user
user := model.User{
ID: "user-id",
}
ctx = request.WithUser(ctx, user)
// Create mock user with access to libraries 2 and 3
ctx = contextWithUser(ctx, "user-id", 2, 3)
// Setup minimal mock library data for working tests
mockLibRepo := ds.Library(ctx).(*tests.MockLibraryRepo)
@ -94,11 +88,6 @@ var _ = Describe("Browsing", func() {
{ID: 3, Name: "Test Library 3", Path: "/music/library3"},
})
// Setup mock expectations
mockUserRepo := ds.User(ctx).(*tests.MockedUserRepo)
err := mockUserRepo.SetUserLibraries("user-id", []int{2, 3})
Expect(err).ToNot(HaveOccurred())
// Setup mock artist data
mockArtistRepo := ds.Artist(ctx).(*tests.MockArtistRepo)
mockArtistRepo.SetData(model.Artists{
@ -122,16 +111,8 @@ var _ = Describe("Browsing", func() {
Describe("GetArtists", func() {
It("should validate user access to the specified musicFolderId", func() {
// Create mock user
user := model.User{
ID: "user-id",
}
ctx = request.WithUser(ctx, user)
// Setup mock expectations - user only has access to library 1
mockUserRepo := ds.User(ctx).(*tests.MockedUserRepo)
err := mockUserRepo.SetUserLibraries("user-id", []int{1})
Expect(err).ToNot(HaveOccurred())
// Create mock user with access to library 1 only
ctx = contextWithUser(ctx, "user-id", 1)
// Create request with musicFolderId=3 (not accessible)
r := httptest.NewRequest("GET", "/rest/getArtists?musicFolderId=3", nil)
@ -146,11 +127,8 @@ var _ = Describe("Browsing", func() {
})
It("should default to first accessible library when no musicFolderId specified", func() {
// Create mock user
user := model.User{
ID: "user-id",
}
ctx = request.WithUser(ctx, user)
// Create mock user with access to libraries 1 and 2
ctx = contextWithUser(ctx, "user-id", 1, 2)
// Setup minimal mock library data for working tests
mockLibRepo := ds.Library(ctx).(*tests.MockLibraryRepo)
@ -159,11 +137,6 @@ var _ = Describe("Browsing", func() {
{ID: 2, Name: "Test Library 2", Path: "/music/library2"},
})
// Setup mock expectations
mockUserRepo := ds.User(ctx).(*tests.MockedUserRepo)
err := mockUserRepo.SetUserLibraries("user-id", []int{1, 2})
Expect(err).ToNot(HaveOccurred())
// Setup mock artist data
mockArtistRepo := ds.Artist(ctx).(*tests.MockArtistRepo)
mockArtistRepo.SetData(model.Artists{

View file

@ -17,6 +17,7 @@ import (
"github.com/navidrome/navidrome/server/public"
"github.com/navidrome/navidrome/server/subsonic/responses"
"github.com/navidrome/navidrome/utils/number"
"github.com/navidrome/navidrome/utils/req"
"github.com/navidrome/navidrome/utils/slice"
)
@ -475,9 +476,24 @@ func buildLyricsList(mf *model.MediaFile, lyricsList model.LyricList) *responses
return res
}
// getUserAccessibleLibraries returns the list of libraries the current user has access to
// Admin users get all libraries (auto-assigned), regular users get their assigned libraries
func getUserAccessibleLibraries(ctx context.Context, ds model.DataStore) ([]model.Library, error) {
// getUserAccessibleLibraries returns the list of libraries the current user has access to.
func getUserAccessibleLibraries(ctx context.Context) []model.Library {
user := getUser(ctx)
return ds.User(ctx).GetUserLibraries(user.ID)
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())
p := req.Params(r)
musicFolderIds, err := p.Ints("musicFolderId")
if !errors.Is(err, req.ErrMissingParam) {
return nil, err
}
if len(musicFolderIds) > 0 {
return musicFolderIds, nil
}
return slice.Map(user.Libraries, func(lib model.Library) int { return lib.ID }), nil
}