remote/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-10-28 22:27:10 +01:00
Code Issues Projects Releases Packages Wiki Activity

feat(subsonic): update GetIndexes and GetArtists method to support library ID filtering

Browse source 
Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan 2025-06-26 19:36:30 -04:00
commit a3281ca027
8 changed files with 280 additions and 23 deletions
Download patch file Download diff file Expand all files Collapse all files

2
model/artist.go
View file

@ -78,7 +78,7 @@ type ArtistRepository interface {
UpdateExternalInfo(a *Artist) error
Get(id string) (*Artist, error)
GetAll(options ...QueryOptions) (Artists, error)
GetIndex(includeMissing bool, roles ...Role) (ArtistIndexes, error)
GetIndex(includeMissing bool, libraryId int, roles ...Role) (ArtistIndexes, error)
// The following methods are used exclusively by the scanner:
RefreshPlayCounts() (int64, error)

33
persistence/artist_repository.go
View file

@ -146,7 +146,7 @@ func roleFilter(_ string, role any) Sqlizer {
// artistLibraryFilter handles filtering artists by library_id through the library_artist junction table
func artistLibraryFilter(_ string, value any) Sqlizer {
// Use a subquery with Eq to properly handle array parameters
// Use a subquery to find artists that belong to the specified library
subquery := Select("1").From("library_artist la").Where(And{
Expr("la.artist_id = artist.id"),
Eq{"la.library_id": value},
@ -225,7 +225,7 @@ func (r *artistRepository) getIndexKey(a model.Artist) string {
}
// TODO Cache the index (recalculate when there are changes to the DB)
func (r *artistRepository) GetIndex(includeMissing bool, roles ...model.Role) (model.ArtistIndexes, error) {
func (r *artistRepository) GetIndex(includeMissing bool, libraryId int, roles ...model.Role) (model.ArtistIndexes, error) {
options := model.QueryOptions{Sort: "name"}
if len(roles) > 0 {
roleFilters := slice.Map(roles, func(r model.Role) Sqlizer {
@ -240,10 +240,37 @@ func (r *artistRepository) GetIndex(includeMissing bool, roles ...model.Role) (m
options.Filters = And{options.Filters, Eq{"artist.missing": false}}
}
}
artists, err := r.GetAll(options)
// 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)
}
if options.Filters == nil {
options.Filters = libFilter
} else {
options.Filters = And{options.Filters, libFilter}
}
artists, err = r.GetAll(options)
if err != nil {
return nil, err
}
var result model.ArtistIndexes
for k, v := range slice.Group(artists, r.getIndexKey) {
result = append(result, model.ArtistIndex{ID: k, Artists: v})

58
persistence/artist_repository_test.go
View file

@ -3,6 +3,7 @@ package persistence
import (
"context"
"encoding/json"
"strings"
"github.com/Masterminds/squirrel"
"github.com/navidrome/navidrome/conf"
@ -95,7 +96,7 @@ var _ = Describe("ArtistRepository", func() {
er := repo.Put(&artistBeatles)
Expect(er).To(BeNil())
idx, err := repo.GetIndex(false)
idx, err := repo.GetIndex(false, 0)
Expect(err).ToNot(HaveOccurred())
Expect(idx).To(HaveLen(2))
Expect(idx[0].ID).To(Equal("F"))
@ -113,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)
idx, err := repo.GetIndex(false, 0)
Expect(err).ToNot(HaveOccurred())
Expect(idx).To(HaveLen(2))
Expect(idx[0].ID).To(Equal("B"))
@ -135,7 +136,7 @@ var _ = Describe("ArtistRepository", func() {
er := repo.Put(&artistBeatles)
Expect(er).To(BeNil())
idx, err := repo.GetIndex(false)
idx, err := repo.GetIndex(false, 0)
Expect(err).ToNot(HaveOccurred())
Expect(idx).To(HaveLen(2))
Expect(idx[0].ID).To(Equal("B"))
@ -152,7 +153,7 @@ var _ = Describe("ArtistRepository", func() {
})
It("returns the index when SortArtistName is empty", func() {
idx, err := repo.GetIndex(false)
idx, err := repo.GetIndex(false, 0)
Expect(err).ToNot(HaveOccurred())
Expect(idx).To(HaveLen(2))
Expect(idx[0].ID).To(Equal("B"))
@ -189,7 +190,7 @@ var _ = Describe("ArtistRepository", func() {
})
It("returns only artists with the specified role", func() {
idx, err := repo.GetIndex(false, model.RoleComposer)
idx, err := repo.GetIndex(false, 0, model.RoleComposer)
Expect(err).ToNot(HaveOccurred())
Expect(idx).To(HaveLen(1))
Expect(idx[0].ID).To(Equal("B"))
@ -198,7 +199,7 @@ var _ = Describe("ArtistRepository", func() {
})
It("returns artists with any of the specified roles", func() {
idx, err := repo.GetIndex(false, model.RoleComposer, model.RoleProducer)
idx, err := repo.GetIndex(false, 0, model.RoleComposer, model.RoleProducer)
Expect(err).ToNot(HaveOccurred())
Expect(idx).To(HaveLen(2))
@ -219,7 +220,7 @@ var _ = Describe("ArtistRepository", func() {
})
It("returns empty index when no artists have the specified role", func() {
idx, err := repo.GetIndex(false, model.RoleDirector)
idx, err := repo.GetIndex(false, 0, model.RoleDirector)
Expect(err).ToNot(HaveOccurred())
Expect(idx).To(HaveLen(0))
})
@ -306,6 +307,45 @@ var _ = Describe("ArtistRepository", func() {
raw = repo.(*artistRepository)
_, err := raw.executeSQL(squirrel.Update(raw.tableName).Set("missing", true).Where(squirrel.Eq{"id": missing.ID}))
Expect(err).ToNot(HaveOccurred())
// Add missing artist to library 1 so it can be found by library filtering
lr := NewLibraryRepository(request.WithUser(log.NewContext(context.TODO()), adminUser), GetDBXBuilder())
err = lr.AddArtist(1, missing.ID)
Expect(err).ToNot(HaveOccurred())
// Ensure the test user exists and has library access
ur := NewUserRepository(request.WithUser(log.NewContext(context.TODO()), adminUser), GetDBXBuilder())
currentUser, ok := request.UserFrom(repo.(*artistRepository).ctx)
if ok {
// Create the user if it doesn't exist with default values if missing
testUser := model.User{
ID: currentUser.ID,
UserName: currentUser.UserName,
Name: currentUser.Name,
Email: currentUser.Email,
IsAdmin: currentUser.IsAdmin,
}
// Provide defaults for missing fields
if testUser.UserName == "" {
testUser.UserName = testUser.ID
}
if testUser.Name == "" {
testUser.Name = testUser.ID
}
if testUser.Email == "" {
testUser.Email = testUser.ID + "@test.com"
}
// Try to put the user (will fail silently if already exists)
_ = ur.Put(&testUser)
// Add library association
err = ur.AddUserLibrary(currentUser.ID, 1)
// Ignore error if user-library association already exists
if err != nil && !strings.Contains(err.Error(), "UNIQUE constraint failed") && !strings.Contains(err.Error(), "duplicate key") {
Expect(err).ToNot(HaveOccurred())
}
}
}
removeMissing := func() {
@ -337,7 +377,7 @@ var _ = Describe("ArtistRepository", func() {
})
It("does not return missing artist in GetIndex", func() {
idx, err := repo.GetIndex(false)
idx, err := repo.GetIndex(false, 0)
Expect(err).ToNot(HaveOccurred())
// Only 2 artists should be present
total := 0
@ -371,7 +411,7 @@ var _ = Describe("ArtistRepository", func() {
})
It("returns missing artist in GetIndex when included", func() {
idx, err := repo.GetIndex(true)
idx, err := repo.GetIndex(true, 0)
Expect(err).ToNot(HaveOccurred())
total := 0
for _, ix := range idx {

17
persistence/persistence_suite_test.go
View file

@ -141,6 +141,14 @@ var _ = BeforeSuite(func() {
}
}
// Associate users with library 1 (default test library)
for i := range testUsers {
err := ur.AddUserLibrary(testUsers[i].ID, 1)
if err != nil {
panic(err)
}
}
//gr := NewGenreRepository(ctx, conn)
//for i := range testGenres {
// g := testGenres[i]
@ -168,6 +176,15 @@ var _ = BeforeSuite(func() {
}
}
// Associate artists with library 1 (default test library)
lr := NewLibraryRepository(ctx, conn)
for i := range testArtists {
err := lr.AddArtist(1, testArtists[i].ID)
if err != nil {
panic(err)
}
}
mr := NewMediaFileRepository(ctx, conn)
for i := range testSongs {
err := mr.Put(&testSongs[i])

25
server/subsonic/browsing.go
View file

@ -7,6 +7,7 @@ import (
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/server/public"
@ -34,22 +35,31 @@ func (api *Router) GetMusicFolders(r *http.Request) (*responses.Subsonic, error)
func (api *Router) getArtist(r *http.Request, libId int, ifModifiedSince time.Time) (model.ArtistIndexes, int64, error) {
ctx := r.Context()
lib, err := api.ds.Library(ctx).Get(libId)
lastScanStr, err := api.ds.Property(ctx).DefaultGet(consts.LastScanStartTimeKey, "")
if err != nil {
log.Error(ctx, "Error retrieving Library", "id", libId, err)
log.Error(ctx, "Error retrieving last scan start time", err)
return nil, 0, err
}
lastScan := time.Now()
if lastScanStr != "" {
lastScan, err = time.Parse(time.RFC3339, lastScanStr)
}
var indexes model.ArtistIndexes
if lib.LastScanAt.After(ifModifiedSince) {
indexes, err = api.ds.Artist(ctx).GetIndex(false, model.RoleAlbumArtist)
if lastScan.After(ifModifiedSince) {
indexes, err = api.ds.Artist(ctx).GetIndex(false, libId, 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)
return nil, 0, newError(responses.ErrorDataNotFound, "Library not found or empty")
}
}
return indexes, lib.LastScanAt.UnixMilli(), err
return indexes, lastScan.UnixMilli(), err
}
func (api *Router) getArtistIndex(r *http.Request, libId int, ifModifiedSince time.Time) (*responses.Indexes, error) {
@ -92,7 +102,7 @@ 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", 1)
musicFolderId := p.IntOr("musicFolderId", 0) // 0 means all accessible libraries
ifModifiedSince := p.TimeOr("ifModifiedSince", time.Time{})
res, err := api.getArtistIndex(r, musicFolderId, ifModifiedSince)
@ -107,7 +117,8 @@ 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", 1)
musicFolderId := p.IntOr("musicFolderId", 0) // 0 means all accessible libraries
res, err := api.getArtistIndexID3(r, musicFolderId, time.Time{})
if err != nil {
return nil, err

132
server/subsonic/browsing_test.go
View file

@ -4,6 +4,7 @@ import (
"context"
"net/http/httptest"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/tests"
@ -18,6 +19,7 @@ var _ = Describe("Browsing", func() {
BeforeEach(func() {
ds = &tests.MockDataStore{}
auth.Init(ds)
api = &Router{ds: ds}
ctx = context.Background()
})
@ -52,4 +54,134 @@ var _ = Describe("Browsing", func() {
Expect(response.MusicFolders.Folders[2].Name).To(Equal("Test Library 3"))
})
})
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 request with musicFolderId=2 (not accessible)
r := httptest.NewRequest("GET", "/rest/getIndexes?musicFolderId=2", nil)
r = r.WithContext(ctx)
// Call endpoint
response, err := api.GetIndexes(r)
// Should return error due to lack of access
Expect(err).To(HaveOccurred())
Expect(response).To(BeNil())
})
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)
// Setup minimal mock library data for working tests
mockLibRepo := ds.Library(ctx).(*tests.MockLibraryRepo)
mockLibRepo.SetData(model.Libraries{
{ID: 2, Name: "Test Library 2", Path: "/music/library2"},
{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{
{ID: "1", Name: "Test Artist 1"},
{ID: "2", Name: "Test Artist 2"},
})
// Create request without musicFolderId
r := httptest.NewRequest("GET", "/rest/getIndexes", nil)
r = r.WithContext(ctx)
// Call endpoint
response, err := api.GetIndexes(r)
// Should succeed and use first accessible library (2)
Expect(err).ToNot(HaveOccurred())
Expect(response).ToNot(BeNil())
Expect(response.Indexes).ToNot(BeNil())
})
})
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 request with musicFolderId=3 (not accessible)
r := httptest.NewRequest("GET", "/rest/getArtists?musicFolderId=3", nil)
r = r.WithContext(ctx)
// Call endpoint
response, err := api.GetArtists(r)
// Should return error due to lack of access
Expect(err).To(HaveOccurred())
Expect(response).To(BeNil())
})
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)
// Setup minimal mock library data for working tests
mockLibRepo := ds.Library(ctx).(*tests.MockLibraryRepo)
mockLibRepo.SetData(model.Libraries{
{ID: 1, Name: "Test Library 1", Path: "/music/library1"},
{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{
{ID: "1", Name: "Test Artist 1"},
{ID: "2", Name: "Test Artist 2"},
})
// Create request without musicFolderId
r := httptest.NewRequest("GET", "/rest/getArtists", nil)
r = r.WithContext(ctx)
// Call endpoint
response, err := api.GetArtists(r)
// Should succeed and use first accessible library (1)
Expect(err).ToNot(HaveOccurred())
Expect(response).ToNot(BeNil())
Expect(response.Artist).ToNot(BeNil())
})
})
})

3
server/subsonic/helpers.go
View file

@ -479,8 +479,5 @@ func buildLyricsList(mf *model.MediaFile, lyricsList model.LyricList) *responses
// Admin users get all libraries (auto-assigned), regular users get their assigned libraries
func getUserAccessibleLibraries(ctx context.Context, ds model.DataStore) ([]model.Library, error) {
user := getUser(ctx)
// All users (admin and regular) use the same method since admin users
// are automatically assigned to all libraries via user_library table
return ds.User(ctx).GetUserLibraries(user.ID)
}

33
tests/mock_artist_repo.go
View file

@ -108,4 +108,37 @@ 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) {
if m.Err {
return nil, errors.New("mock repo error")
}
artists, err := m.GetAll()
if err != nil {
return nil, err
}
// For mock purposes, if no artists available, return empty result
if len(artists) == 0 {
return model.ArtistIndexes{}, nil
}
// Simple index grouping by first letter (simplified implementation for mocks)
indexMap := make(map[string]model.Artists)
for _, artist := range artists {
key := "#"
if len(artist.Name) > 0 {
key = string(artist.Name[0])
}
indexMap[key] = append(indexMap[key], artist)
}
var result model.ArtistIndexes
for k, artists := range indexMap {
result = append(result, model.ArtistIndex{ID: k, Artists: artists})
}
return result, nil
}
var _ model.ArtistRepository = (*MockArtistRepo)(nil)