refactor library service to use REST repository pattern and remove CRUD operations

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan 2025-06-06 18:47:21 -04:00
commit d2a2c43ef5
11 changed files with 476 additions and 406 deletions

View file

@ -10,6 +10,7 @@ import (
"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/model"
"github.com/navidrome/navidrome/server"
@ -28,7 +29,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, nil)
nativeRouter := New(ds, nil, nil, nil, core.NewMockLibraryService())
router = server.JWTVerifier(nativeRouter)
// Create test users

View file

@ -5,7 +5,6 @@ import (
"encoding/json"
"errors"
"net/http"
"strconv"
"github.com/go-chi/chi/v5"
"github.com/navidrome/navidrome/core"
@ -13,20 +12,6 @@ import (
"github.com/navidrome/navidrome/model"
)
// Library management endpoints (admin only)
func (n *Router) addLibraryRoute(r chi.Router) {
r.Route("/library", func(r chi.Router) {
r.Get("/", getLibraries(n.libs))
r.Post("/", createLibrary(n.libs))
r.Route("/{id}", func(r chi.Router) {
r.Use(parseIDMiddleware)
r.Get("/", getLibrary(n.libs))
r.Put("/", updateLibrary(n.libs))
r.Delete("/", deleteLibrary(n.libs))
})
})
}
// User-library association endpoints (admin only)
func (n *Router) addUserLibraryRoute(r chi.Router) {
r.Route("/user/{id}/library", func(r chi.Router) {
@ -36,20 +21,6 @@ func (n *Router) addUserLibraryRoute(r chi.Router) {
})
}
// Middleware to parse library ID from URL
func parseIDMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
idStr := chi.URLParam(r, "id")
id, err := strconv.Atoi(idStr)
if err != nil {
http.Error(w, "Invalid library ID", http.StatusBadRequest)
return
}
ctx := context.WithValue(r.Context(), "libraryID", id)
next.ServeHTTP(w, r.WithContext(ctx))
})
}
// Middleware to parse user ID from URL
func parseUserIDMiddleware(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@ -63,126 +34,6 @@ func parseUserIDMiddleware(next http.Handler) http.Handler {
})
}
// Library CRUD handlers
func getLibraries(service core.Library) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
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)
return
}
w.Header().Set("Content-Type", "application/json")
if err := json.NewEncoder(w).Encode(libraries); err != nil {
log.Error(r.Context(), "Error encoding libraries response", err)
}
}
}
func getLibrary(service core.Library) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
libraryID := r.Context().Value("libraryID").(int)
lib, err := service.Get(r.Context(), libraryID)
if err != nil {
if errors.Is(err, model.ErrNotFound) {
http.Error(w, "Library not found", http.StatusNotFound)
return
}
log.Error(r.Context(), "Error getting library", "libraryID", libraryID, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
w.Header().Set("Content-Type", "application/json")
if err := json.NewEncoder(w).Encode(lib); err != nil {
log.Error(r.Context(), "Error encoding library response", err)
}
}
}
func createLibrary(service core.Library) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
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
}
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(lib); err != nil {
log.Error(r.Context(), "Error encoding library response", err)
}
}
}
func updateLibrary(service core.Library) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
libraryID := r.Context().Value("libraryID").(int)
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
lib.ID = libraryID
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(lib); err != nil {
log.Error(r.Context(), "Error encoding library response", err)
}
}
}
func deleteLibrary(service core.Library) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
libraryID := r.Context().Value("libraryID").(int)
if err := service.Delete(r.Context(), libraryID); err != nil {
if errors.Is(err, model.ErrNotFound) {
http.Error(w, "Library not found", http.StatusNotFound)
return
}
log.Error(r.Context(), "Error deleting library", "libraryID", libraryID, err)
http.Error(w, "Failed to delete library", http.StatusInternalServerError)
return
}
w.WriteHeader(http.StatusNoContent)
}
}
// User-library association handlers
func getUserLibraries(service core.Library) http.HandlerFunc {

View file

@ -29,7 +29,7 @@ var _ = Describe("Library API", func() {
DeferCleanup(configtest.SetupConfig())
ds = &tests.MockDataStore{}
auth.Init(ds)
nativeRouter := New(ds, nil, nil, nil, &mockLibraryService{})
nativeRouter := New(ds, nil, nil, nil, core.NewMockLibraryService())
router = server.JWTVerifier(nativeRouter)
// Create test users
@ -126,7 +126,7 @@ var _ = Describe("Library API", func() {
router.ServeHTTP(w, req)
Expect(w.Code).To(Equal(http.StatusBadRequest))
Expect(w.Code).To(Equal(http.StatusNotFound))
})
})
@ -142,13 +142,7 @@ var _ = Describe("Library API", func() {
router.ServeHTTP(w, req)
Expect(w.Code).To(Equal(http.StatusCreated))
var created model.Library
err := json.Unmarshal(w.Body.Bytes(), &created)
Expect(err).ToNot(HaveOccurred())
Expect(created.Name).To(Equal("New Library"))
Expect(created.Path).To(Equal("/music/new"))
Expect(w.Code).To(Equal(http.StatusOK))
})
It("validates required fields", func() {
@ -226,7 +220,7 @@ var _ = Describe("Library API", func() {
router.ServeHTTP(w, req)
Expect(w.Code).To(Equal(http.StatusNoContent))
Expect(w.Code).To(Equal(http.StatusOK))
})
It("returns 404 for non-existent library", func() {
@ -441,91 +435,3 @@ 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
}

View file

@ -70,8 +70,8 @@ func (n *Router) routes() http.Handler {
r.With(adminOnlyMiddleware).Group(func(r chi.Router) {
n.addInspectRoute(r)
n.addConfigRoute(r)
n.addLibraryRoute(r)
n.addUserLibraryRoute(r)
n.RX(r, "/library", n.libs.NewRepository, true)
})
})

View file

@ -11,6 +11,7 @@ import (
"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/model"
"github.com/navidrome/navidrome/server"
@ -94,7 +95,7 @@ var _ = Describe("Song Endpoints", func() {
mfRepo.SetData(testSongs)
// Create the native API router and wrap it with the JWTVerifier middleware
nativeRouter := New(ds, nil, nil, nil, nil)
nativeRouter := New(ds, nil, nil, nil, core.NewMockLibraryService())
router = server.JWTVerifier(nativeRouter)
w = httptest.NewRecorder()
})