remote/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-11-13 13:48:39 +01:00
Code Issues Projects Releases Packages Wiki Activity

refactor(library): implement library management service and update API routes

Browse source 
Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan 2025-06-06 16:16:39 -04:00
commit c1c7cfa49f
10 changed files with 637 additions and 168 deletions
Download patch file Download diff file Expand all files Collapse all files

2
server/nativeapi/config_test.go
View file

@ -28,7 +28,7 @@ var _ = Describe("Config API", func() {
conf.Server.DevUIShowConfig = true // Enable config endpoint for tests
ds = &tests.MockDataStore{}
auth.Init(ds)
nativeRouter := New(ds, nil, nil, nil)
nativeRouter := New(ds, nil, nil, nil, nil)
router = server.JWTVerifier(nativeRouter)
// Create test users

174
server/nativeapi/library.go
View file

@ -3,12 +3,12 @@ package nativeapi
import (
"context"
"encoding/json"
"fmt"
"errors"
"net/http"
"strconv"
"github.com/Masterminds/squirrel"
"github.com/go-chi/chi/v5"
"github.com/navidrome/navidrome/core"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
)
@ -16,13 +16,13 @@ import (
// Library management endpoints (admin only)
func (n *Router) addLibraryRoute(r chi.Router) {
r.Route("/library", func(r chi.Router) {
r.Get("/", getLibraries(n.ds))
r.Post("/", createLibrary(n.ds))
r.Get("/", getLibraries(n.libs))
r.Post("/", createLibrary(n.libs))
r.Route("/{id}", func(r chi.Router) {
r.Use(parseIDMiddleware)
r.Get("/", getLibrary(n.ds))
r.Put("/", updateLibrary(n.ds))
r.Delete("/", deleteLibrary(n.ds))
r.Get("/", getLibrary(n.libs))
r.Put("/", updateLibrary(n.libs))
r.Delete("/", deleteLibrary(n.libs))
})
})
}
@ -31,8 +31,8 @@ func (n *Router) addLibraryRoute(r chi.Router) {
func (n *Router) addUserLibraryRoute(r chi.Router) {
r.Route("/user/{id}/library", func(r chi.Router) {
r.Use(parseUserIDMiddleware)
r.Get("/", getUserLibraries(n.ds))
r.Put("/", setUserLibraries(n.ds))
r.Get("/", getUserLibraries(n.libs))
r.Put("/", setUserLibraries(n.libs))
})
}
@ -65,9 +65,9 @@ func parseUserIDMiddleware(next http.Handler) http.Handler {
// Library CRUD handlers
func getLibraries(ds model.DataStore) http.HandlerFunc {
func getLibraries(service core.Library) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
libraries, err := ds.Library(r.Context()).GetAll()
libraries, err := service.GetAll(r.Context())
if err != nil {
log.Error(r.Context(), "Error getting libraries", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
@ -81,13 +81,13 @@ func getLibraries(ds model.DataStore) http.HandlerFunc {
}
}
func getLibrary(ds model.DataStore) http.HandlerFunc {
func getLibrary(service core.Library) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
libraryID := r.Context().Value("libraryID").(int)
library, err := ds.Library(r.Context()).Get(libraryID)
lib, err := service.Get(r.Context(), libraryID)
if err != nil {
if err == model.ErrNotFound {
if errors.Is(err, model.ErrNotFound) {
http.Error(w, "Library not found", http.StatusNotFound)
return
}
@ -97,92 +97,80 @@ func getLibrary(ds model.DataStore) http.HandlerFunc {
}
w.Header().Set("Content-Type", "application/json")
if err := json.NewEncoder(w).Encode(library); err != nil {
if err := json.NewEncoder(w).Encode(lib); err != nil {
log.Error(r.Context(), "Error encoding library response", err)
}
}
}
func createLibrary(ds model.DataStore) http.HandlerFunc {
func createLibrary(service core.Library) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
var library model.Library
if err := json.NewDecoder(r.Body).Decode(&library); err != nil {
var lib model.Library
if err := json.NewDecoder(r.Body).Decode(&lib); err != nil {
log.Error(r.Context(), "Error decoding library request", err)
http.Error(w, "Invalid request body", http.StatusBadRequest)
return
}
// Validate required fields
if library.Name == "" {
http.Error(w, "Library name is required", http.StatusBadRequest)
return
}
if library.Path == "" {
http.Error(w, "Library path is required", http.StatusBadRequest)
return
}
if err := ds.Library(r.Context()).Put(&library); err != nil {
if err := service.Create(r.Context(), &lib); err != nil {
log.Error(r.Context(), "Error creating library", err)
if errors.Is(err, model.ErrValidation) {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
http.Error(w, "Failed to create library", http.StatusInternalServerError)
return
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusCreated)
if err := json.NewEncoder(w).Encode(library); err != nil {
if err := json.NewEncoder(w).Encode(lib); err != nil {
log.Error(r.Context(), "Error encoding library response", err)
}
}
}
func updateLibrary(ds model.DataStore) http.HandlerFunc {
func updateLibrary(service core.Library) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
libraryID := r.Context().Value("libraryID").(int)
var library model.Library
if err := json.NewDecoder(r.Body).Decode(&library); err != nil {
var lib model.Library
if err := json.NewDecoder(r.Body).Decode(&lib); err != nil {
log.Error(r.Context(), "Error decoding library request", err)
http.Error(w, "Invalid request body", http.StatusBadRequest)
return
}
// Ensure the ID matches the URL parameter
library.ID = libraryID
lib.ID = libraryID
// Validate required fields
if library.Name == "" {
http.Error(w, "Library name is required", http.StatusBadRequest)
return
}
if library.Path == "" {
http.Error(w, "Library path is required", http.StatusBadRequest)
return
}
if err := ds.Library(r.Context()).Put(&library); err != nil {
if err == model.ErrNotFound {
if err := service.Update(r.Context(), &lib); err != nil {
if errors.Is(err, model.ErrNotFound) {
http.Error(w, "Library not found", http.StatusNotFound)
return
}
if errors.Is(err, model.ErrValidation) {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
log.Error(r.Context(), "Error updating library", "libraryID", libraryID, err)
http.Error(w, "Failed to update library", http.StatusInternalServerError)
return
}
w.Header().Set("Content-Type", "application/json")
if err := json.NewEncoder(w).Encode(library); err != nil {
if err := json.NewEncoder(w).Encode(lib); err != nil {
log.Error(r.Context(), "Error encoding library response", err)
}
}
}
func deleteLibrary(ds model.DataStore) http.HandlerFunc {
func deleteLibrary(service core.Library) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
libraryID := r.Context().Value("libraryID").(int)
if err := ds.Library(r.Context()).Delete(libraryID); err != nil {
if err == model.ErrNotFound {
if err := service.Delete(r.Context(), libraryID); err != nil {
if errors.Is(err, model.ErrNotFound) {
http.Error(w, "Library not found", http.StatusNotFound)
return
}
@ -197,23 +185,16 @@ func deleteLibrary(ds model.DataStore) http.HandlerFunc {
// User-library association handlers
func getUserLibraries(ds model.DataStore) http.HandlerFunc {
func getUserLibraries(service core.Library) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
userID := r.Context().Value("userID").(string)
// Verify user exists
if _, err := ds.User(r.Context()).Get(userID); err != nil {
if err == model.ErrNotFound {
libraries, err := service.GetUserLibraries(r.Context(), userID)
if err != nil {
if errors.Is(err, model.ErrNotFound) {
http.Error(w, "User not found", http.StatusNotFound)
return
}
log.Error(r.Context(), "Error getting user", "userID", userID, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
libraries, err := ds.User(r.Context()).GetUserLibraries(userID)
if err != nil {
log.Error(r.Context(), "Error getting user libraries", "userID", userID, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
@ -226,22 +207,10 @@ func getUserLibraries(ds model.DataStore) http.HandlerFunc {
}
}
func setUserLibraries(ds model.DataStore) http.HandlerFunc {
func setUserLibraries(service core.Library) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
userID := r.Context().Value("userID").(string)
// Verify user exists
user, err := ds.User(r.Context()).Get(userID)
if err != nil {
if err == model.ErrNotFound {
http.Error(w, "User not found", http.StatusNotFound)
return
}
log.Error(r.Context(), "Error getting user", "userID", userID, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
var request struct {
LibraryIDs []int `json:"libraryIds"`
}
@ -251,63 +220,22 @@ func setUserLibraries(ds model.DataStore) http.HandlerFunc {
return
}
// Admin users get all libraries automatically - don't allow manual assignment
if user.IsAdmin {
http.Error(w, "Cannot manually assign libraries to admin users", http.StatusBadRequest)
return
}
// Validate library IDs exist
if len(request.LibraryIDs) > 0 {
// Use CountAll with IN filter to efficiently check if all library IDs exist
count, err := ds.Library(r.Context()).CountAll(model.QueryOptions{
Filters: squirrel.Eq{"id": request.LibraryIDs},
})
if err != nil {
log.Error(r.Context(), "Error counting libraries", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
if err := service.SetUserLibraries(r.Context(), userID, request.LibraryIDs); err != nil {
log.Error(r.Context(), "Error setting user libraries", "userID", userID, err)
if errors.Is(err, model.ErrNotFound) {
http.Error(w, "User not found", http.StatusNotFound)
return
}
if int(count) != len(request.LibraryIDs) {
// Some library IDs don't exist - find which ones for better error message
existingLibraries, err := ds.Library(r.Context()).GetAll(model.QueryOptions{
Filters: squirrel.Eq{"id": request.LibraryIDs},
})
if err != nil {
log.Error(r.Context(), "Error getting all libraries", err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
libraryMap := make(map[int]bool)
for _, lib := range existingLibraries {
libraryMap[lib.ID] = true
}
for _, libID := range request.LibraryIDs {
if !libraryMap[libID] {
http.Error(w, fmt.Sprintf("Library ID %d does not exist", libID), http.StatusBadRequest)
return
}
}
if errors.Is(err, model.ErrValidation) {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
}
// Regular users must have at least one library
if len(request.LibraryIDs) == 0 {
http.Error(w, "At least one library must be assigned to non-admin users", http.StatusBadRequest)
return
}
if err := ds.User(r.Context()).SetUserLibraries(userID, request.LibraryIDs); err != nil {
log.Error(r.Context(), "Error setting user libraries", "userID", userID, err)
http.Error(w, "Failed to set user libraries", http.StatusInternalServerError)
return
}
// Return updated user libraries
libraries, err := ds.User(r.Context()).GetUserLibraries(userID)
libraries, err := service.GetUserLibraries(r.Context(), userID)
if err != nil {
log.Error(r.Context(), "Error getting updated user libraries", "userID", userID, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)

101
server/nativeapi/library_test.go
View file

@ -10,6 +10,7 @@ import (
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/server"
@ -28,7 +29,7 @@ var _ = Describe("Library API", func() {
DeferCleanup(configtest.SetupConfig())
ds = &tests.MockDataStore{}
auth.Init(ds)
nativeRouter := New(ds, nil, nil, nil)
nativeRouter := New(ds, nil, nil, nil, &mockLibraryService{})
router = server.JWTVerifier(nativeRouter)
// Create test users
@ -162,7 +163,7 @@ var _ = Describe("Library API", func() {
router.ServeHTTP(w, req)
Expect(w.Code).To(Equal(http.StatusBadRequest))
Expect(w.Body.String()).To(ContainSubstring("Library name is required"))
Expect(w.Body.String()).To(ContainSubstring("library name is required"))
})
It("validates path field", func() {
@ -177,7 +178,7 @@ var _ = Describe("Library API", func() {
router.ServeHTTP(w, req)
Expect(w.Code).To(Equal(http.StatusBadRequest))
Expect(w.Body.String()).To(ContainSubstring("Library path is required"))
Expect(w.Body.String()).To(ContainSubstring("library path is required"))
})
})
@ -352,7 +353,7 @@ var _ = Describe("Library API", func() {
router.ServeHTTP(w, req)
Expect(w.Code).To(Equal(http.StatusBadRequest))
Expect(w.Body.String()).To(ContainSubstring("Library ID 999 does not exist"))
Expect(w.Body.String()).To(ContainSubstring("library ID 999 does not exist"))
})
It("requires at least one library for regular users", func() {
@ -366,7 +367,7 @@ var _ = Describe("Library API", func() {
router.ServeHTTP(w, req)
Expect(w.Code).To(Equal(http.StatusBadRequest))
Expect(w.Body.String()).To(ContainSubstring("At least one library must be assigned"))
Expect(w.Body.String()).To(ContainSubstring("at least one library must be assigned"))
})
It("prevents manual assignment to admin users", func() {
@ -380,7 +381,7 @@ var _ = Describe("Library API", func() {
router.ServeHTTP(w, req)
Expect(w.Code).To(Equal(http.StatusBadRequest))
Expect(w.Body.String()).To(ContainSubstring("Cannot manually assign libraries to admin users"))
Expect(w.Body.String()).To(ContainSubstring("cannot manually assign libraries to admin users"))
})
})
})
@ -440,3 +441,91 @@ func Split(s, sep string) []string {
result = append(result, s[start:])
return result
}
// Mock library service for testing
type mockLibraryService struct {
core.Library
}
func (m *mockLibraryService) GetAll(ctx context.Context) (model.Libraries, error) {
return model.Libraries{
{ID: 1, Name: "Test Library 1", Path: "/music/library1"},
{ID: 2, Name: "Test Library 2", Path: "/music/library2"},
}, nil
}
func (m *mockLibraryService) Get(ctx context.Context, id int) (*model.Library, error) {
if id == 999 {
return nil, model.ErrNotFound
}
return &model.Library{ID: 1, Name: "Test Library 1", Path: "/music/library1"}, nil
}
func (m *mockLibraryService) Create(ctx context.Context, library *model.Library) error {
if library.Name == "" {
return fmt.Errorf("%w: library name is required", model.ErrValidation)
}
if library.Path == "" {
return fmt.Errorf("%w: library path is required", model.ErrValidation)
}
return nil
}
func (m *mockLibraryService) Update(ctx context.Context, library *model.Library) error {
if library.ID == 999 {
return model.ErrNotFound
}
if library.Name == "" {
return fmt.Errorf("%w: library name is required", model.ErrValidation)
}
if library.Path == "" {
return fmt.Errorf("%w: library path is required", model.ErrValidation)
}
return nil
}
func (m *mockLibraryService) Delete(ctx context.Context, id int) error {
if id == 999 {
return model.ErrNotFound
}
return nil
}
func (m *mockLibraryService) GetUserLibraries(ctx context.Context, userID string) (model.Libraries, error) {
if userID == "non-existent" {
return nil, model.ErrNotFound
}
// If setting multiple libraries, return all requested ones
if userID == "user-1" {
return model.Libraries{
{ID: 1, Name: "Test Library 1", Path: "/music/library1"},
{ID: 2, Name: "Test Library 2", Path: "/music/library2"},
}, nil
}
return model.Libraries{
{ID: 1, Name: "Test Library 1", Path: "/music/library1"},
}, nil
}
func (m *mockLibraryService) SetUserLibraries(ctx context.Context, userID string, libraryIDs []int) error {
if userID == "non-existent" {
return model.ErrNotFound
}
if userID == "admin-1" {
return fmt.Errorf("%w: cannot manually assign libraries to admin users", model.ErrValidation)
}
if len(libraryIDs) == 0 {
return fmt.Errorf("%w: at least one library must be assigned to non-admin users", model.ErrValidation)
}
for _, id := range libraryIDs {
if id == 999 {
return fmt.Errorf("%w: library ID 999 does not exist", model.ErrValidation)
}
}
return nil
}
func (m *mockLibraryService) ValidateLibraryAccess(ctx context.Context, userID string, libraryID int) error {
// For testing purposes, allow admin access and check basic user access
return nil
}

5
server/nativeapi/native_api.go
View file

@ -26,10 +26,11 @@ type Router struct {
share core.Share
playlists core.Playlists
insights metrics.Insights
libs core.Library
}
func New(ds model.DataStore, share core.Share, playlists core.Playlists, insights metrics.Insights) *Router {
r := &Router{ds: ds, share: share, playlists: playlists, insights: insights}
func New(ds model.DataStore, share core.Share, playlists core.Playlists, insights metrics.Insights, libraryService core.Library) *Router {
r := &Router{ds: ds, share: share, playlists: playlists, insights: insights, libs: libraryService}
r.Handler = r.routes()
return r
}

36
server/nativeapi/native_api_song_test.go
View file

@ -2,20 +2,16 @@ package nativeapi
import (
"bytes"
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"net/url"
"time"
"github.com/deluan/rest"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/core/metrics"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/server"
"github.com/navidrome/navidrome/tests"
@ -23,31 +19,6 @@ import (
. "github.com/onsi/gomega"
)
// Simple mock implementations for missing types
type mockShare struct {
core.Share
}
func (m *mockShare) NewRepository(ctx context.Context) rest.Repository {
return &tests.MockShareRepo{}
}
type mockPlaylists struct {
core.Playlists
}
func (m *mockPlaylists) ImportFile(ctx context.Context, folder *model.Folder, filename string) (*model.Playlist, error) {
return &model.Playlist{}, nil
}
type mockInsights struct {
metrics.Insights
}
func (m *mockInsights) LastRun(ctx context.Context) (time.Time, bool) {
return time.Now(), true
}
var _ = Describe("Song Endpoints", func() {
var (
router http.Handler
@ -122,13 +93,8 @@ var _ = Describe("Song Endpoints", func() {
}
mfRepo.SetData(testSongs)
// Setup router with mocked dependencies
mockShareImpl := &mockShare{}
mockPlaylistsImpl := &mockPlaylists{}
mockInsightsImpl := &mockInsights{}
// Create the native API router and wrap it with the JWTVerifier middleware
nativeRouter := New(ds, mockShareImpl, mockPlaylistsImpl, mockInsightsImpl)
nativeRouter := New(ds, nil, nil, nil, nil)
router = server.JWTVerifier(nativeRouter)
w = httptest.NewRecorder()
})