diff --git a/core/library.go b/core/library.go
index 0ef31e720..0c9512751 100644
--- a/core/library.go
+++ b/core/library.go
@@ -4,12 +4,15 @@ import (
"context"
"errors"
"fmt"
+ "io/fs"
"os"
"path/filepath"
"strconv"
+ "strings"
"github.com/Masterminds/squirrel"
"github.com/deluan/rest"
+ "github.com/navidrome/navidrome/core/storage"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
@@ -158,7 +161,7 @@ func (r *libraryRepositoryWrapper) Update(id string, entity interface{}, cols ..
func (r *libraryRepositoryWrapper) Delete(id string) error {
libID, err := strconv.Atoi(id)
if err != nil {
- return rest.ValidationError{Errors: map[string]string{
+ return &rest.ValidationError{Errors: map[string]string{
"id": "invalid library ID format",
}}
}
@@ -181,41 +184,107 @@ func (r *libraryRepositoryWrapper) mapError(err error) error {
}
func (r *libraryRepositoryWrapper) validateLibrary(library *model.Library) error {
- errors := make(map[string]string)
+ validationErrors := make(map[string]string)
if library.Name == "" {
- errors["name"] = "library name is required"
- }
-
- if library.Path == "" {
- errors["path"] = "library path is required"
+ validationErrors["name"] = "library name is required"
} else {
- // Validate path format
- if !filepath.IsAbs(library.Path) {
- errors["path"] = "library path must be absolute"
- } else {
- // Clean the path to normalize it
- cleanPath := filepath.Clean(library.Path)
- library.Path = cleanPath
-
- // Check if path exists and is accessible
- info, err := os.Stat(library.Path)
- if err != nil {
- if os.IsNotExist(err) {
- errors["path"] = "library path does not exist"
- } else if os.IsPermission(err) {
- errors["path"] = "library path is not accessible"
- } else {
- errors["path"] = fmt.Sprintf("error accessing library path: %v", err)
+ // Check for name uniqueness
+ if err := r.validateLibraryNameUnique(library); err != nil {
+ var validationErr *rest.ValidationError
+ if errors.As(err, &validationErr) {
+ for k, v := range validationErr.Errors {
+ validationErrors[k] = v
}
- } else if !info.IsDir() {
- errors["path"] = "library path must be a directory"
+ } else {
+ // For non-validation errors (like database errors), return them immediately
+ return err
}
}
}
- if len(errors) > 0 {
- return rest.ValidationError{Errors: errors}
+ if library.Path == "" {
+ validationErrors["path"] = "library path is required"
+ } else {
+ if err := r.validateLibraryPath(library); err != nil {
+ validationErrors["path"] = err.Error()
+ }
+ }
+
+ if len(validationErrors) > 0 {
+ return &rest.ValidationError{Errors: validationErrors}
+ }
+
+ return nil
+}
+
+func (r *libraryRepositoryWrapper) validateLibraryNameUnique(library *model.Library) error {
+ // Get all libraries to check for name conflicts
+ allLibraries, err := r.LibraryRepository.GetAll()
+ if err != nil {
+ return err
+ }
+
+ // Check if any other library has the same name
+ for _, existingLib := range allLibraries {
+ if existingLib.Name == library.Name && existingLib.ID != library.ID {
+ return &rest.ValidationError{Errors: map[string]string{"name": "ra.validation.unique"}}
+ }
+ }
+
+ return nil
+}
+
+func (r *libraryRepositoryWrapper) validateLibraryPath(library *model.Library) error {
+ // Validate path format
+ if !filepath.IsAbs(library.Path) {
+ return fmt.Errorf("library path must be absolute")
+ }
+
+ // Clean the path to normalize it
+ cleanPath := filepath.Clean(library.Path)
+ library.Path = cleanPath
+
+ // Check if path exists and is accessible using storage abstraction
+ fileStore, err := storage.For(library.Path)
+ if err != nil {
+ return fmt.Errorf("invalid storage scheme: %w", err)
+ }
+
+ fsys, err := fileStore.FS()
+ if err != nil {
+ // Parse the error to provide user-friendly messages
+ errStr := err.Error()
+ if os.IsNotExist(err) ||
+ strings.Contains(errStr, "no such file or directory") ||
+ strings.Contains(errStr, "The system cannot find the path specified.") {
+ return fmt.Errorf("library path does not exist")
+ } else if os.IsPermission(err) {
+ return fmt.Errorf("library path is not accessible")
+ } else {
+ return fmt.Errorf("error accessing library storage: %w", err)
+ }
+ }
+
+ // Check if root directory exists
+ info, err := fs.Stat(fsys, ".")
+ if err != nil {
+ // Parse the error message to check for "not a directory"
+ errStr := err.Error()
+ if strings.Contains(errStr, "not a directory") ||
+ strings.Contains(errStr, "The directory name is invalid.") {
+ return fmt.Errorf("library path must be a directory")
+ } else if os.IsNotExist(err) {
+ return fmt.Errorf("library path does not exist")
+ } else if os.IsPermission(err) {
+ return fmt.Errorf("library path is not accessible")
+ } else {
+ return fmt.Errorf("error accessing library path: %w", err)
+ }
+ }
+
+ if !info.IsDir() {
+ return fmt.Errorf("library path must be a directory")
}
return nil
diff --git a/core/library_test.go b/core/library_test.go
index d956047c9..07cf5024e 100644
--- a/core/library_test.go
+++ b/core/library_test.go
@@ -2,12 +2,15 @@ package core_test
import (
"context"
+ "errors"
"os"
"path/filepath"
"github.com/deluan/rest"
+ _ "github.com/navidrome/navidrome/adapters/taglib" // Register taglib extractor
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/core"
+ _ "github.com/navidrome/navidrome/core/storage/local" // Register local storage
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/tests"
@@ -15,6 +18,7 @@ import (
. "github.com/onsi/gomega"
)
+// These tests require the local storage adapter and the taglib extractor to be registered.
var _ = Describe("Library Service", func() {
var service core.Library
var ds *tests.MockDataStore
@@ -81,6 +85,22 @@ var _ = Describe("Library Service", func() {
Expect(err.Error()).To(ContainSubstring("library path is required"))
})
+ It("fails when library name already exists", func() {
+ // First, create a library
+ existingLibrary := &model.Library{ID: 1, Name: "Existing Library", Path: tempDir}
+ libraryRepo.SetData(model.Libraries{*existingLibrary})
+
+ // Try to create another library with the same name
+ newLibrary := &model.Library{ID: 2, Name: "Existing Library", Path: tempDir + "/other"}
+
+ _, err := repo.Save(newLibrary)
+
+ Expect(err).To(HaveOccurred())
+ var validationErr *rest.ValidationError
+ Expect(errors.As(err, &validationErr)).To(BeTrue())
+ Expect(validationErr.Errors["name"]).To(Equal("ra.validation.unique"))
+ })
+
It("cleans and normalizes the path", func() {
unnormalizedPath := tempDir + "//../" + filepath.Base(tempDir)
library := &model.Library{ID: 1, Name: "Test Library", Path: unnormalizedPath}
@@ -150,9 +170,9 @@ var _ = Describe("Library Service", func() {
_, err := repo.Save(library)
Expect(err).To(HaveOccurred())
- var validationErr rest.ValidationError
- Expect(err).To(BeAssignableToTypeOf(validationErr))
- Expect(err.(rest.ValidationError).Errors["path"]).To(Equal("library path must be absolute"))
+ var validationErr *rest.ValidationError
+ Expect(errors.As(err, &validationErr)).To(BeTrue())
+ Expect(validationErr.Errors["path"]).To(Equal("library path must be absolute"))
})
It("fails when path does not exist", func() {
@@ -162,14 +182,14 @@ var _ = Describe("Library Service", func() {
_, err := repo.Save(library)
Expect(err).To(HaveOccurred())
- var validationErr rest.ValidationError
- Expect(err).To(BeAssignableToTypeOf(validationErr))
- Expect(err.(rest.ValidationError).Errors["path"]).To(Equal("library path does not exist"))
+ var validationErr *rest.ValidationError
+ Expect(errors.As(err, &validationErr)).To(BeTrue())
+ Expect(validationErr.Errors["path"]).To(Equal("library path does not exist"))
})
It("fails when path is a file instead of directory", func() {
testFile := filepath.Join(tempDir, "testfile.txt")
- err := os.WriteFile(testFile, []byte("test"), 0644)
+ err := os.WriteFile(testFile, []byte("test"), 0600)
Expect(err).NotTo(HaveOccurred())
library := &model.Library{Name: "Test", Path: testFile}
@@ -177,9 +197,9 @@ var _ = Describe("Library Service", func() {
_, err = repo.Save(library)
Expect(err).To(HaveOccurred())
- var validationErr rest.ValidationError
- Expect(err).To(BeAssignableToTypeOf(validationErr))
- Expect(err.(rest.ValidationError).Errors["path"]).To(Equal("library path must be a directory"))
+ var validationErr *rest.ValidationError
+ Expect(errors.As(err, &validationErr)).To(BeTrue())
+ Expect(validationErr.Errors["path"]).To(Equal("library path must be a directory"))
})
It("fails when path is not accessible due to permissions", func() {
@@ -194,12 +214,12 @@ var _ = Describe("Library Service", func() {
_, err := repo.Save(library)
Expect(err).To(HaveOccurred())
- var validationErr rest.ValidationError
- Expect(err).To(BeAssignableToTypeOf(validationErr))
- Expect(err.(rest.ValidationError).Errors).To(HaveKey("name"))
- Expect(err.(rest.ValidationError).Errors).To(HaveKey("path"))
- Expect(err.(rest.ValidationError).Errors["name"]).To(Equal("library name is required"))
- Expect(err.(rest.ValidationError).Errors["path"]).To(Equal("library path must be absolute"))
+ var validationErr *rest.ValidationError
+ Expect(errors.As(err, &validationErr)).To(BeTrue())
+ Expect(validationErr.Errors).To(HaveKey("name"))
+ Expect(validationErr.Errors).To(HaveKey("path"))
+ Expect(validationErr.Errors["name"]).To(Equal("library name is required"))
+ Expect(validationErr.Errors["path"]).To(Equal("library path must be absolute"))
})
})
@@ -216,9 +236,41 @@ var _ = Describe("Library Service", func() {
err := repo.Update("1", library)
Expect(err).To(HaveOccurred())
- var validationErr rest.ValidationError
- Expect(err).To(BeAssignableToTypeOf(validationErr))
- Expect(err.(rest.ValidationError).Errors["path"]).To(Equal("library path must be absolute"))
+ var validationErr *rest.ValidationError
+ Expect(errors.As(err, &validationErr)).To(BeTrue())
+ Expect(validationErr.Errors["path"]).To(Equal("library path must be absolute"))
+ })
+
+ It("fails when updated name conflicts with existing library", func() {
+ // Set up two libraries
+ libraryRepo.SetData(model.Libraries{
+ {ID: 1, Name: "Library One", Path: tempDir},
+ {ID: 2, Name: "Library Two", Path: tempDir + "/other"},
+ })
+
+ // Try to update library 2 to have the same name as library 1
+ library := &model.Library{ID: 2, Name: "Library One", Path: tempDir + "/other"}
+
+ err := repo.Update("2", library)
+
+ Expect(err).To(HaveOccurred())
+ var validationErr *rest.ValidationError
+ Expect(errors.As(err, &validationErr)).To(BeTrue())
+ Expect(validationErr.Errors["name"]).To(Equal("ra.validation.unique"))
+ })
+
+ It("allows updating library with same name (no change)", func() {
+ // Set up a library
+ libraryRepo.SetData(model.Libraries{
+ {ID: 1, Name: "Test Library", Path: tempDir},
+ })
+
+ // Update the library keeping the same name (should be allowed)
+ library := &model.Library{ID: 1, Name: "Test Library", Path: tempDir}
+
+ err := repo.Update("1", library)
+
+ Expect(err).NotTo(HaveOccurred())
})
It("fails when updated path does not exist", func() {
@@ -228,14 +280,14 @@ var _ = Describe("Library Service", func() {
err := repo.Update("1", library)
Expect(err).To(HaveOccurred())
- var validationErr rest.ValidationError
- Expect(err).To(BeAssignableToTypeOf(validationErr))
- Expect(err.(rest.ValidationError).Errors["path"]).To(Equal("library path does not exist"))
+ var validationErr *rest.ValidationError
+ Expect(errors.As(err, &validationErr)).To(BeTrue())
+ Expect(validationErr.Errors["path"]).To(Equal("library path does not exist"))
})
It("fails when updated path is a file instead of directory", func() {
testFile := filepath.Join(tempDir, "updatefile.txt")
- err := os.WriteFile(testFile, []byte("test"), 0644)
+ err := os.WriteFile(testFile, []byte("test"), 0600)
Expect(err).NotTo(HaveOccurred())
library := &model.Library{ID: 1, Name: "Test", Path: testFile}
@@ -243,23 +295,30 @@ var _ = Describe("Library Service", func() {
err = repo.Update("1", library)
Expect(err).To(HaveOccurred())
- var validationErr rest.ValidationError
- Expect(err).To(BeAssignableToTypeOf(validationErr))
- Expect(err.(rest.ValidationError).Errors["path"]).To(Equal("library path must be a directory"))
+ var validationErr *rest.ValidationError
+ Expect(errors.As(err, &validationErr)).To(BeTrue())
+ Expect(validationErr.Errors["path"]).To(Equal("library path must be a directory"))
})
It("handles multiple validation errors on update", func() {
- library := &model.Library{ID: 1, Name: "", Path: "relative/path"}
+ // Set up two libraries to test name conflict
+ libraryRepo.SetData(model.Libraries{
+ {ID: 1, Name: "Library One", Path: tempDir},
+ {ID: 2, Name: "Library Two", Path: tempDir + "/other"},
+ })
- err := repo.Update("1", library)
+ // Try to update with conflicting name and invalid path
+ library := &model.Library{ID: 2, Name: "Library One", Path: "relative/path"}
+
+ err := repo.Update("2", library)
Expect(err).To(HaveOccurred())
- var validationErr rest.ValidationError
- Expect(err).To(BeAssignableToTypeOf(validationErr))
- Expect(err.(rest.ValidationError).Errors).To(HaveKey("name"))
- Expect(err.(rest.ValidationError).Errors).To(HaveKey("path"))
- Expect(err.(rest.ValidationError).Errors["name"]).To(Equal("library name is required"))
- Expect(err.(rest.ValidationError).Errors["path"]).To(Equal("library path must be absolute"))
+ var validationErr *rest.ValidationError
+ Expect(errors.As(err, &validationErr)).To(BeTrue())
+ Expect(validationErr.Errors).To(HaveKey("name"))
+ Expect(validationErr.Errors).To(HaveKey("path"))
+ Expect(validationErr.Errors["name"]).To(Equal("ra.validation.unique"))
+ Expect(validationErr.Errors["path"]).To(Equal("library path must be absolute"))
})
})
})
diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go
index 4d35f767c..e7e354f21 100644
--- a/scanner/scanner_test.go
+++ b/scanner/scanner_test.go
@@ -58,12 +58,14 @@ var _ = Describe("Scanner", Ordered, func() {
})
BeforeEach(func() {
+ DeferCleanup(configtest.SetupConfig())
+ conf.Server.MusicFolder = "fake:///music" // Set to match test library path
+ conf.Server.DevExternalScanner = false
+
db.Init(ctx)
DeferCleanup(func() {
Expect(tests.ClearDB()).To(Succeed())
})
- DeferCleanup(configtest.SetupConfig())
- conf.Server.DevExternalScanner = false
ds = &tests.MockDataStore{RealDS: persistence.New(db.Db())}
mfRepo = &mockMediaFileRepo{
diff --git a/ui/src/library/LibraryEdit.jsx b/ui/src/library/LibraryEdit.jsx
index 4774809a3..bdc39bb18 100644
--- a/ui/src/library/LibraryEdit.jsx
+++ b/ui/src/library/LibraryEdit.jsx
@@ -1,4 +1,4 @@
-import React from 'react'
+import React, { useCallback } from 'react'
import {
Edit,
SimpleForm,
@@ -10,6 +10,9 @@ import {
DateField,
FunctionField,
useTranslate,
+ useMutation,
+ useNotify,
+ useRedirect,
} from 'react-admin'
import { makeStyles } from '@material-ui/core/styles'
import { Divider, Typography } from '@material-ui/core'
@@ -83,11 +86,38 @@ const formatBytes = (bytes, decimals = 2) => {
const LibraryEdit = (props) => {
const classes = useStyles()
const translate = useTranslate()
+ const [mutate] = useMutation()
+ const notify = useNotify()
+ const redirect = useRedirect()
const isFirstLibrary = props.id === '1'
+ const save = useCallback(
+ async (values) => {
+ try {
+ await mutate(
+ {
+ type: 'update',
+ resource: 'library',
+ payload: { id: props.id, data: values },
+ },
+ { returnPromise: true },
+ )
+ notify('resources.library.notifications.updated', 'info', {
+ smart_count: 1,
+ })
+ redirect('/library')
+ } catch (error) {
+ if (error.body && error.body.errors) {
+ return error.body.errors
+ }
+ }
+ },
+ [mutate, notify, redirect, props.id],
+ )
+
return (
} undoable={false} {...props}>
- }>
+ } save={save}>
@@ -175,3 +205,4 @@ const LibraryEdit = (props) => {
}
export default LibraryEdit
+
\ No newline at end of file
diff --git a/ui/src/library/index.js b/ui/src/library/index.js
index b12c770dc..868fb2017 100644
--- a/ui/src/library/index.js
+++ b/ui/src/library/index.js
@@ -1,7 +1,7 @@
import LibraryList from './LibraryList'
import LibraryEdit from './LibraryEdit'
import LibraryCreate from './LibraryCreate'
-
+
export default {
list: LibraryList,
edit: LibraryEdit,