diff --git a/adapters/gotaglib/end_to_end_test.go b/adapters/gotaglib/end_to_end_test.go
index e7dd18ac1..0f9a90d94 100644
--- a/adapters/gotaglib/end_to_end_test.go
+++ b/adapters/gotaglib/end_to_end_test.go
@@ -90,7 +90,7 @@ var _ = Describe("Extractor", func() {
info.FileInfo = testFileInfo{FileInfo: fileInfo}
metadata := metadata.New(path, info)
- return new(metadata.ToMediaFile(1, "folderID"))
+ return new(metadata.ToMediaFile(model.Library{ID: 1}, "folderID"))
}
BeforeEach(func() {
diff --git a/cmd/inspect.go b/cmd/inspect.go
index 5e88793cc..05f569f3e 100644
--- a/cmd/inspect.go
+++ b/cmd/inspect.go
@@ -1,13 +1,17 @@
package cmd
import (
+ "context"
"encoding/json"
"fmt"
+ "path/filepath"
"strings"
"github.com/navidrome/navidrome/core"
+ "github.com/navidrome/navidrome/db"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
+ "github.com/navidrome/navidrome/persistence"
"github.com/pelletier/go-toml/v2"
"github.com/spf13/cobra"
"gopkg.in/yaml.v3"
@@ -28,7 +32,7 @@ var inspectCmd = &cobra.Command{
Long: "Show file tags as seen by Navidrome",
Args: cobra.MinimumNArgs(1),
Run: func(cmd *cobra.Command, args []string) {
- runInspector(args)
+ runInspector(cmd.Context(), args)
},
}
@@ -55,18 +59,24 @@ func prettyMarshal(v any) ([]byte, error) {
return []byte(res.String()), nil
}
-func runInspector(args []string) {
+func runInspector(ctx context.Context, args []string) {
marshal := marshalers[format]
if marshal == nil {
log.Fatal("Invalid format", "format", format)
}
+ libs := loadLibraries(ctx)
+ matcher := model.NewLibraryMatcher(libs)
var out []core.InspectOutput
for _, filePath := range args {
if !model.IsAudioFile(filePath) {
log.Warn("Not an audio file", "file", filePath)
continue
}
- output, err := core.Inspect(filePath, 1, "")
+ lib, ok := libraryForFile(matcher, filePath)
+ if !ok && len(libs) > 0 {
+ log.Warn("File is not in any library, using the global PID config", "file", filePath)
+ }
+ output, err := core.Inspect(filePath, lib, "")
if err != nil {
log.Warn("Unable to process file", "file", filePath, "error", err)
continue
@@ -77,3 +87,33 @@ func runInspector(args []string) {
data, _ := marshal(out)
fmt.Println(string(data))
}
+
+// loadLibraries reads the libraries, so each file gets its library's PID config. It never creates a DB.
+func loadLibraries(ctx context.Context) model.Libraries {
+ if dbFile, ok := existingDBFile(); !ok {
+ log.Warn(ctx, "No database found, using the global PID config", "path", dbFile)
+ return nil
+ }
+ defer db.Init(ctx)()
+ libs, err := persistence.New(db.Db()).Library().GetAll(ctx)
+ if err != nil {
+ log.Warn(ctx, "Could not load libraries, using the global PID config", err)
+ return nil
+ }
+ for i := range libs {
+ if absPath, err := filepath.Abs(libs[i].Path); err == nil {
+ libs[i].Path = absPath
+ }
+ }
+ return libs
+}
+
+// libraryForFile falls back to the default library with no overrides, which uses the global PID config.
+func libraryForFile(matcher *model.LibraryMatcher, filePath string) (model.Library, bool) {
+ if absPath, err := filepath.Abs(filePath); err == nil {
+ if lib, ok := matcher.FindLibrary(absPath); ok {
+ return lib, true
+ }
+ }
+ return model.Library{ID: model.DefaultLibraryID}, false
+}
diff --git a/cmd/inspect_test.go b/cmd/inspect_test.go
new file mode 100644
index 000000000..728dc8770
--- /dev/null
+++ b/cmd/inspect_test.go
@@ -0,0 +1,61 @@
+package cmd
+
+import (
+ "os"
+ "path/filepath"
+
+ "github.com/navidrome/navidrome/conf"
+ "github.com/navidrome/navidrome/conf/configtest"
+ "github.com/navidrome/navidrome/model"
+ . "github.com/onsi/ginkgo/v2"
+ . "github.com/onsi/gomega"
+)
+
+var _ = Describe("inspect", func() {
+ Describe("libraryForFile", func() {
+ var matcher *model.LibraryMatcher
+ var root string
+
+ BeforeEach(func() {
+ root = GinkgoT().TempDir()
+ cwd, err := os.Getwd()
+ Expect(err).ToNot(HaveOccurred())
+ matcher = model.NewLibraryMatcher(model.Libraries{
+ {ID: 1, Path: filepath.Join(root, "music")},
+ {ID: 2, Path: filepath.Join(cwd, "loose"), PIDAlbum: "folder"},
+ })
+ })
+
+ It("returns the library that contains an absolute path", func() {
+ lib, ok := libraryForFile(matcher, filepath.Join(root, "music", "album", "track.mp3"))
+ Expect(ok).To(BeTrue())
+ Expect(lib.ID).To(Equal(1))
+ })
+
+ It("resolves a relative path against the working directory", func() {
+ lib, ok := libraryForFile(matcher, filepath.Join("loose", "track.mp3"))
+ Expect(ok).To(BeTrue())
+ Expect(lib.PIDAlbum).To(Equal("folder"))
+ })
+
+ It("falls back to the default library without overrides", func() {
+ lib, ok := libraryForFile(matcher, filepath.Join(root, "elsewhere", "track.mp3"))
+ Expect(ok).To(BeFalse())
+ Expect(lib).To(Equal(model.Library{ID: model.DefaultLibraryID}))
+ })
+ })
+
+ Describe("loadLibraries", func() {
+ BeforeEach(func() {
+ DeferCleanup(configtest.SetupConfig())
+ })
+
+ It("does not create a database when there is none", func() {
+ dbFile := filepath.Join(GinkgoT().TempDir(), "navidrome.db")
+ conf.Server.DbPath = dbFile + "?_journal_mode=WAL"
+
+ Expect(loadLibraries(GinkgoT().Context())).To(BeNil())
+ Expect(dbFile).ToNot(BeAnExistingFile())
+ })
+ })
+})
diff --git a/cmd/root.go b/cmd/root.go
index 089f09472..e39c55365 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -190,16 +190,20 @@ func schedulePeriodicScan(ctx context.Context) func() error {
}
}
-func pidHashChanged(ds model.DataStore) (bool, error) {
- pidAlbum, err := ds.Property().DefaultGet(context.Background(), consts.PIDAlbumKey, "")
+// librariesWithChangedPID returns the names of the libraries whose effective PID config differs from
+// the one used by their last finished scan
+func librariesWithChangedPID(ctx context.Context, ds model.DataStore) ([]string, error) {
+ libs, err := ds.Library().GetAll(ctx)
if err != nil {
- return false, err
+ return nil, err
}
- pidTrack, err := ds.Property().DefaultGet(context.Background(), consts.PIDTrackKey, "")
- if err != nil {
- return false, err
+ var names []string
+ for _, lib := range libs {
+ if lib.PIDChanged() {
+ names = append(names, lib.Name)
+ }
}
- return !strings.EqualFold(pidAlbum, conf.Server.PID.Album) || !strings.EqualFold(pidTrack, conf.Server.PID.Track), nil
+ return names, nil
}
// runInitialScan runs an initial scan of the music library if needed.
@@ -214,12 +218,12 @@ func runInitialScan(ctx context.Context) func() error {
if err != nil {
return err
}
- pidHasChanged, err := pidHashChanged(ds)
+ pidChangedLibs, err := librariesWithChangedPID(ctx, ds)
if err != nil {
return err
}
scanOnStartup := conf.Server.Scanner.Enabled && conf.Server.Scanner.ScanOnStartup
- scanNeeded := scanOnStartup || inProgress || fullScanRequired == "1" || pidHasChanged
+ scanNeeded := scanOnStartup || inProgress || fullScanRequired == "1" || len(pidChangedLibs) > 0
time.Sleep(2 * time.Second) // Wait 2 seconds before the initial scan
if scanNeeded {
s := CreateScanner(ctx)
@@ -227,9 +231,9 @@ func runInitialScan(ctx context.Context) func() error {
case fullScanRequired == "1":
log.Warn(ctx, "Full scan required after migration")
_ = ds.Property().Delete(ctx, consts.FullScanAfterMigrationFlagKey)
- case pidHasChanged:
- log.Warn(ctx, "PID config changed, performing full scan")
- fullScanRequired = "1"
+ case len(pidChangedLibs) > 0:
+ // Includes never-scanned libraries. The scanner rescans in full only the ones that need it
+ log.Warn(ctx, "Libraries with a new or changed PID config, scanning", "libraries", pidChangedLibs)
case inProgress:
log.Warn(ctx, "Resuming interrupted scan")
default:
diff --git a/cmd/root_test.go b/cmd/root_test.go
index af8d44e7e..423cd2a8f 100644
--- a/cmd/root_test.go
+++ b/cmd/root_test.go
@@ -1,6 +1,7 @@
package cmd
import (
+ "errors"
"net/http"
"net/http/httptest"
"path"
@@ -9,6 +10,8 @@ import (
"github.com/go-chi/chi/v5"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
+ "github.com/navidrome/navidrome/model"
+ "github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
@@ -44,3 +47,30 @@ var _ = Describe("profilerHandler", func() {
Entry("with a trailing-slash BasePath", "/music/"),
)
})
+
+var _ = Describe("librariesWithChangedPID", func() {
+ var ds *tests.MockDataStore
+ var libs *tests.MockLibraryRepo
+
+ BeforeEach(func() {
+ DeferCleanup(configtest.SetupConfig())
+ libs = &tests.MockLibraryRepo{}
+ ds = &tests.MockDataStore{MockedLibrary: libs}
+ })
+
+ It("returns only the libraries whose PID config changed", func() {
+ pid := model.Library{}.EffectivePID()
+ libs.SetData(model.Libraries{
+ {ID: 1, Name: "Same", ScannedPIDAlbum: pid.Album, ScannedPIDTrack: pid.Track},
+ {ID: 2, Name: "Changed", PIDAlbum: "folder", ScannedPIDAlbum: pid.Album, ScannedPIDTrack: pid.Track},
+ {ID: 3, Name: "Never scanned"},
+ })
+ Expect(librariesWithChangedPID(GinkgoT().Context(), ds)).To(ConsistOf("Changed", "Never scanned"))
+ })
+
+ It("returns the error from the repository", func() {
+ libs.Err = errors.New("db down")
+ _, err := librariesWithChangedPID(GinkgoT().Context(), ds)
+ Expect(err).To(MatchError("db down"))
+ })
+})
diff --git a/cmd/utils.go b/cmd/utils.go
index 72ec67f90..f35a31fb1 100644
--- a/cmd/utils.go
+++ b/cmd/utils.go
@@ -18,11 +18,16 @@ import (
"github.com/navidrome/navidrome/persistence"
)
-// requireExistingDB aborts the command when the database file (DbPath minus DSN
-// params) does not exist.
-func requireExistingDB() {
+// existingDBFile returns the database file (DbPath minus DSN params), and whether it exists.
+func existingDBFile() (string, bool) {
path, _, _ := strings.Cut(conf.Server.DbPath, "?")
- if _, err := os.Stat(path); os.IsNotExist(err) {
+ _, err := os.Stat(path)
+ return path, err == nil
+}
+
+// requireExistingDB aborts the command when the database file does not exist.
+func requireExistingDB() {
+ if path, ok := existingDBFile(); !ok {
log.Fatal("No existing database", "path", path)
}
}
diff --git a/consts/consts.go b/consts/consts.go
index 9bdac9125..42e9ec42f 100644
--- a/consts/consts.go
+++ b/consts/consts.go
@@ -156,8 +156,6 @@ const (
//DefaultAlbumPID = "album_legacy"
DefaultAlbumPID = "musicbrainz_albumid|albumartistid,album,albumversion,releasedate"
DefaultTrackPID = "musicbrainz_trackid|albumid,discnumber,tracknumber,title"
- PIDAlbumKey = "PIDAlbum"
- PIDTrackKey = "PIDTrack"
)
const (
diff --git a/core/inspect.go b/core/inspect.go
index 01ec33760..c60459b88 100644
--- a/core/inspect.go
+++ b/core/inspect.go
@@ -15,7 +15,7 @@ type InspectOutput struct {
MappedTags *model.MediaFile `json:"mappedTags,omitempty"`
}
-func Inspect(filePath string, libraryId int, folderId string) (*InspectOutput, error) {
+func Inspect(filePath string, lib model.Library, folderId string) (*InspectOutput, error) {
path, file := filepath.Split(filePath)
s, err := storage.For(path)
@@ -39,12 +39,22 @@ func Inspect(filePath string, libraryId int, folderId string) (*InspectOutput, e
return nil, model.ErrNotFound
}
- md := metadata.New(path, tag)
+ md := metadata.New(scannerPath(lib, filePath), tag)
result := &InspectOutput{
File: filePath,
RawTags: tags[file].Tags,
- MappedTags: new(md.ToMediaFile(libraryId, folderId)),
+ MappedTags: new(md.ToMediaFile(lib, folderId)),
}
return result, nil
}
+
+// scannerPath returns the path the scanner uses for the file (relative to its library), so
+// folder-based PIDs match the DB. Files outside the library keep their absolute path.
+func scannerPath(lib model.Library, filePath string) string {
+ absPath, err := filepath.Abs(filePath)
+ if err != nil || lib.Path == "" {
+ return filePath
+ }
+ return model.LibraryRelativePath(lib.Path, absPath)
+}
diff --git a/core/inspect_test.go b/core/inspect_test.go
new file mode 100644
index 000000000..0ac90990c
--- /dev/null
+++ b/core/inspect_test.go
@@ -0,0 +1,45 @@
+package core_test
+
+import (
+ "path/filepath"
+
+ "github.com/navidrome/navidrome/core"
+ "github.com/navidrome/navidrome/model"
+ . "github.com/onsi/ginkgo/v2"
+ . "github.com/onsi/gomega"
+)
+
+var _ = Describe("Inspect", func() {
+ var fixtures string
+
+ BeforeEach(func() {
+ var err error
+ fixtures, err = filepath.Abs(filepath.Join("tests", "fixtures"))
+ Expect(err).ToNot(HaveOccurred())
+ })
+
+ It("maps the file with the library-relative path the scanner uses", func() {
+ lib := model.Library{ID: 2, Path: filepath.Dir(fixtures), PIDAlbum: "folder"}
+ out, err := core.Inspect(filepath.Join(fixtures, "test.mp3"), lib, "")
+ Expect(err).ToNot(HaveOccurred())
+ Expect(out.MappedTags.Path).To(Equal("fixtures/test.mp3"))
+ Expect(out.MappedTags.LibraryID).To(Equal(2))
+ })
+
+ It("gives the same IDs for relative and absolute paths", func() {
+ lib := model.Library{ID: 2, Path: filepath.Dir(fixtures), PIDAlbum: "folder"}
+ abs, err := core.Inspect(filepath.Join(fixtures, "test.mp3"), lib, "")
+ Expect(err).ToNot(HaveOccurred())
+ rel, err := core.Inspect(filepath.Join("tests", "fixtures", "test.mp3"), lib, "")
+ Expect(err).ToNot(HaveOccurred())
+ Expect(rel.MappedTags.AlbumID).To(Equal(abs.MappedTags.AlbumID))
+ Expect(rel.MappedTags.PID).To(Equal(abs.MappedTags.PID))
+ })
+
+ It("keeps the given path for a file outside the library", func() {
+ filePath := filepath.Join(fixtures, "test.mp3")
+ out, err := core.Inspect(filePath, model.Library{ID: model.DefaultLibraryID}, "")
+ Expect(err).ToNot(HaveOccurred())
+ Expect(out.MappedTags.Path).To(Equal(filePath))
+ })
+})
diff --git a/core/library.go b/core/library.go
index 628ee4b7b..f1153da26 100644
--- a/core/library.go
+++ b/core/library.go
@@ -2,10 +2,12 @@ package core
import (
"context"
+ "errors"
"fmt"
"io/fs"
"os"
"path/filepath"
+ "slices"
"strconv"
"strings"
"time"
@@ -15,6 +17,7 @@ import (
"github.com/navidrome/navidrome/core/storage"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
+ "github.com/navidrome/navidrome/model/metadata"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/server/events"
"github.com/navidrome/navidrome/utils/slice"
@@ -200,23 +203,22 @@ func (r *libraryRepositoryWrapper) Update(ctx context.Context, id string, entity
}
pathChanged := originalLib.Path != lib.Path
+ pidChanged := (updatesColumn(cols, "pidAlbum") && originalLib.PIDAlbum != lib.PIDAlbum) ||
+ (updatesColumn(cols, "pidTrack") && originalLib.PIDTrack != lib.PIDTrack)
err = r.LibraryRepository.Put(ctx, lib, cols...)
if err != nil {
return r.mapError(err)
}
- // Restart watcher and trigger scan if path was updated
- if pathChanged {
- if r.watcher != nil {
- if err := r.watcher.Watch(ctx, lib); err != nil {
- log.Warn(ctx, "Failed to restart watcher for updated library", "libraryID", lib.ID, "name", lib.Name, "path", lib.Path, err)
- }
+ if pathChanged && r.watcher != nil {
+ if err := r.watcher.Watch(ctx, lib); err != nil {
+ log.Warn(ctx, "Failed to restart watcher for updated library", "libraryID", lib.ID, "name", lib.Name, "path", lib.Path, err)
}
+ }
- if r.scanner != nil {
- go r.triggerScan(ctx, lib, "updated")
- }
+ if (pathChanged || pidChanged) && r.scanner != nil {
+ go r.triggerScan(ctx, lib, "updated")
}
// Send library refresh event to all clients
@@ -325,6 +327,15 @@ func (r *libraryRepositoryWrapper) validateLibrary(ctx context.Context, library
}
}
+ library.PIDAlbum = strings.TrimSpace(library.PIDAlbum)
+ library.PIDTrack = strings.TrimSpace(library.PIDTrack)
+ if err := metadata.ValidatePIDSpec(library.PIDAlbum, true); err != nil {
+ validationErrors["pidAlbum"] = err.Error()
+ }
+ if err := metadata.ValidatePIDSpec(library.PIDTrack, false); err != nil {
+ validationErrors["pidTrack"] = err.Error()
+ }
+
if len(validationErrors) > 0 {
return &rest.ValidationError{Errors: validationErrors}
}
@@ -332,6 +343,11 @@ func (r *libraryRepositoryWrapper) validateLibrary(ctx context.Context, library
return nil
}
+// updatesColumn reports whether an update with these columns writes col. No columns means all of them.
+func updatesColumn(cols []string, col string) bool {
+ return len(cols) == 0 || slices.Contains(cols, col)
+}
+
func (r *libraryRepositoryWrapper) validateLibraryPath(ctx context.Context, library *model.Library) error {
// Validate path format
if !filepath.IsAbs(library.Path) {
@@ -399,11 +415,27 @@ func (s *libraryService) validateLibraryIDs(ctx context.Context, libraryIDs []in
return nil
}
+var scanWaitInterval = time.Second
+
func (r *libraryRepositoryWrapper) triggerScan(ctx context.Context, lib *model.Library, action string) {
+ // Runs in its own goroutine and outlives the HTTP request
+ ctx = context.WithoutCancel(ctx)
+
+ // A running scan loaded the libraries before this change, and would reject a new request
+ for {
+ status, err := r.scanner.Status(ctx)
+ if err != nil || !status.Scanning {
+ break
+ }
+ time.Sleep(scanWaitInterval)
+ }
+
log.Info(ctx, fmt.Sprintf("Triggering scan for %s library", action), "libraryID", lib.ID, "name", lib.Name, "path", lib.Path)
start := time.Now()
- warnings, err := r.scanner.ScanAll(ctx, false) // Quick scan for new library
- if err != nil {
+ warnings, err := r.scanner.ScanAll(ctx, false) // Quick scan: the scanner rescans libraries with a changed PID config in full
+ if errors.Is(err, model.ErrAlreadyScanning) {
+ log.Debug(ctx, "Scan already running, it covers this change", "libraryID", lib.ID, "name", lib.Name)
+ } else if err != nil {
log.Error(ctx, fmt.Sprintf("Error scanning %s library", action), "libraryID", lib.ID, "name", lib.Name, err)
} else {
log.Info(ctx, fmt.Sprintf("Scan completed for %s library", action), "libraryID", lib.ID, "name", lib.Name, "warnings", len(warnings), "elapsed", time.Since(start))
diff --git a/core/library_test.go b/core/library_test.go
index 5402eac22..e6ebb1974 100644
--- a/core/library_test.go
+++ b/core/library_test.go
@@ -322,6 +322,37 @@ var _ = Describe("Library Service", func() {
})
})
+ Describe("PID validation", func() {
+ pidError := func(err error, field string) string {
+ var validationErr *rest.ValidationError
+ Expect(errors.As(err, &validationErr)).To(BeTrue())
+ return validationErr.Errors[field]
+ }
+
+ It("rejects an unknown attribute in the album PID", func() {
+ _, err := repo.Save(ctx, &model.Library{Name: "Lib", Path: tempDir, PIDAlbum: "albmversion"})
+ Expect(pidError(err, "pidAlbum")).To(ContainSubstring(`unknown attribute "albmversion"`))
+ })
+
+ It("rejects albumid in the album PID", func() {
+ _, err := repo.Save(ctx, &model.Library{Name: "Lib", Path: tempDir, PIDAlbum: "albumid"})
+ Expect(pidError(err, "pidAlbum")).To(ContainSubstring("albumid"))
+ })
+
+ It("rejects an unknown attribute in the track PID", func() {
+ _, err := repo.Save(ctx, &model.Library{Name: "Lib", Path: tempDir, PIDTrack: "nosuchtag"})
+ Expect(pidError(err, "pidTrack")).To(ContainSubstring(`unknown attribute "nosuchtag"`))
+ })
+
+ It("trims spaces", func() {
+ library := &model.Library{Name: "Lib", Path: tempDir, PIDAlbum: " folder ", PIDTrack: " "}
+ _, err := repo.Save(ctx, library)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(library.PIDAlbum).To(Equal("folder"))
+ Expect(library.PIDTrack).To(BeEmpty())
+ })
+ })
+
Describe("Path Validation", func() {
Context("Create operation", func() {
It("fails when path is not absolute", func() {
@@ -679,6 +710,48 @@ var _ = Describe("Library Service", func() {
}, "100ms", "10ms").Should(Equal(0))
})
+ It("triggers scan when updating the library PID config", func() {
+ libraryRepo.SetData(model.Libraries{{ID: 1, Name: "Library", Path: tempDir}})
+
+ library := model.Library{ID: 1, Name: "Library", Path: tempDir, PIDAlbum: "folder"}
+ Expect(repo.Update(ctx, "1", library)).To(Succeed())
+
+ Eventually(func() int {
+ return scanner.GetScanAllCallCount()
+ }, "1s", "10ms").Should(Equal(1))
+ // A quick scan: the scanner itself rescans this library in full
+ Expect(scanner.GetScanAllCalls()[0].FullScan).To(BeFalse())
+ })
+
+ It("does not trigger scan when the PID fields were not sent", func() {
+ libraryRepo.SetData(model.Libraries{{ID: 1, Name: "Library", Path: tempDir, PIDAlbum: "folder"}})
+
+ // The REST layer decodes a missing pidAlbum as "". Only the sent fields count.
+ library := model.Library{ID: 1, Name: "Renamed", Path: tempDir}
+ Expect(repo.Update(ctx, "1", library, "name", "path")).To(Succeed())
+
+ Consistently(func() int {
+ return scanner.GetScanAllCallCount()
+ }, "100ms", "10ms").Should(Equal(0))
+ })
+
+ It("waits for a running scan before triggering a new one", func() {
+ libraryRepo.SetData(model.Libraries{{ID: 1, Name: "Library", Path: tempDir}})
+ scanner.SetScanning(true)
+
+ library := model.Library{ID: 1, Name: "Library", Path: tempDir, PIDAlbum: "folder"}
+ Expect(repo.Update(ctx, "1", library)).To(Succeed())
+
+ Consistently(func() int {
+ return scanner.GetScanAllCallCount()
+ }, "200ms", "20ms").Should(Equal(0))
+
+ scanner.SetScanning(false)
+ Eventually(func() int {
+ return scanner.GetScanAllCallCount()
+ }, "3s", "20ms").Should(Equal(1))
+ })
+
It("does not trigger scan when library creation fails", func() {
// Try to create library with invalid data (empty name)
library := &model.Library{Path: tempDir}
diff --git a/core/metrics/insights.go b/core/metrics/insights.go
index 4a78a7f3f..1d2ff5df4 100644
--- a/core/metrics/insights.go
+++ b/core/metrics/insights.go
@@ -10,6 +10,7 @@ import (
"path/filepath"
"runtime"
"runtime/debug"
+ "slices"
"strings"
"sync"
"sync/atomic"
@@ -269,9 +270,13 @@ func (c *insightsCollector) collect(ctx context.Context) []byte {
if err != nil {
log.Trace(ctx, "Error reading radios count", err)
}
- data.Library.Libraries, err = c.ds.Library().CountAll(ctx)
+ libs, err := c.ds.Library().GetAll(ctx)
if err != nil {
- log.Trace(ctx, "Error reading libraries count", err)
+ log.Trace(ctx, "Error reading libraries", err)
+ }
+ data.Library.Libraries = int64(len(libs))
+ if slices.ContainsFunc(libs, func(lib model.Library) bool { return lib.PIDAlbum != "" || lib.PIDTrack != "" }) {
+ data.Config.HasCustomPID = true
}
data.Library.ActiveUsers, err = c.ds.User().CountAll(ctx, model.QueryOptions{
Filters: squirrel.Gt{"last_access_at": time.Now().Add(-7 * 24 * time.Hour)},
diff --git a/core/playlists/import.go b/core/playlists/import.go
index 658bd92dc..b5991b095 100644
--- a/core/playlists/import.go
+++ b/core/playlists/import.go
@@ -78,8 +78,8 @@ func (s *playlists) resolveFolder(ctx context.Context, dir string) (*model.Folde
if err != nil {
return nil, err
}
- matcher := newLibraryMatcher(libs)
- lib, ok := matcher.findLibrary(dir)
+ matcher := model.NewLibraryMatcher(libs)
+ lib, ok := matcher.FindLibrary(dir)
if !ok {
return nil, fmt.Errorf("%w: %s", errNotInLibrary, dir)
}
diff --git a/core/playlists/parse_m3u.go b/core/playlists/parse_m3u.go
index 9610e9dbb..ab95b8850 100644
--- a/core/playlists/parse_m3u.go
+++ b/core/playlists/parse_m3u.go
@@ -1,13 +1,11 @@
package playlists
import (
- "cmp"
"context"
"fmt"
"io"
"net/url"
"path/filepath"
- "slices"
"strings"
"time"
@@ -156,61 +154,9 @@ func (r pathResolution) ToQualifiedString() (string, error) {
return fmt.Sprintf("%d:%s", r.libraryID, filepath.ToSlash(relativePath)), nil
}
-// libraryMatcher holds sorted libraries with cleaned paths for efficient path matching.
-type libraryMatcher struct {
- libraries model.Libraries
- cleanedPaths []string
-}
-
-// findLibraryForPath finds which library contains the given absolute path.
-// Returns library ID and path, or 0 and empty string if not found.
-func (lm *libraryMatcher) findLibraryForPath(absolutePath string) (int, string) {
- lib, ok := lm.findLibrary(absolutePath)
- if !ok {
- return 0, ""
- }
- return lib.ID, filepath.Clean(lib.Path)
-}
-
-// findLibrary checks if the absolute path is under any of the library paths.
-func (lm *libraryMatcher) findLibrary(absolutePath string) (model.Library, bool) {
- // Check sorted libraries (longest path first) to find the best match
- for i, cleanLibPath := range lm.cleanedPaths {
- // Check if absolutePath is under this library path
- if strings.HasPrefix(absolutePath, cleanLibPath) {
- // Ensure it's a proper path boundary (not just a prefix)
- if len(absolutePath) == len(cleanLibPath) || absolutePath[len(cleanLibPath)] == filepath.Separator {
- return lm.libraries[i], true
- }
- }
- }
- return model.Library{}, false
-}
-
-// newLibraryMatcher creates a libraryMatcher with libraries sorted by path length (longest first).
-// This ensures correct matching when library paths are prefixes of each other.
-// Example: /music-classical must be checked before /music
-// Otherwise, /music-classical/track.mp3 would match /music instead of /music-classical
-func newLibraryMatcher(libs model.Libraries) *libraryMatcher {
- // Sort libraries by path length (descending) to ensure longest paths match first.
- slices.SortFunc(libs, func(i, j model.Library) int {
- return cmp.Compare(len(j.Path), len(i.Path)) // Reverse order for descending
- })
-
- // Pre-clean all library paths once for efficient matching
- cleanedPaths := make([]string, len(libs))
- for i, lib := range libs {
- cleanedPaths[i] = filepath.Clean(lib.Path)
- }
- return &libraryMatcher{
- libraries: libs,
- cleanedPaths: cleanedPaths,
- }
-}
-
// pathResolver handles path resolution logic for playlist imports.
type pathResolver struct {
- matcher *libraryMatcher
+ matcher *model.LibraryMatcher
}
// newPathResolver creates a pathResolver with libraries loaded from the datastore.
@@ -219,7 +165,7 @@ func newPathResolver(ctx context.Context, ds model.DataStore) (*pathResolver, er
if err != nil {
return nil, err
}
- matcher := newLibraryMatcher(libs)
+ matcher := model.NewLibraryMatcher(libs)
return &pathResolver{matcher: matcher}, nil
}
@@ -246,14 +192,14 @@ func (r *pathResolver) resolvePath(line string, folder *model.Folder) pathResolu
// a pathResolution with the library information. Returns an invalid resolution if
// the path is not found in any library.
func (r *pathResolver) findInLibraries(absolutePath string) pathResolution {
- libID, libPath := r.matcher.findLibraryForPath(absolutePath)
- if libID == 0 {
+ lib, ok := r.matcher.FindLibrary(absolutePath)
+ if !ok {
return pathResolution{valid: false}
}
return pathResolution{
absolutePath: absolutePath,
- libraryPath: libPath,
- libraryID: libID,
+ libraryPath: filepath.Clean(lib.Path),
+ libraryID: lib.ID,
valid: true,
}
}
@@ -288,7 +234,7 @@ func (r *pathResolver) resolvePaths(ctx context.Context, folder *model.Folder, l
// HTTP(S) URLs are stored as-is (gated by EnableM3UExternalAlbumArt).
// Local paths (file://, absolute, or relative) are resolved to an absolute path
// and validated against known library boundaries via matcher.
-func resolveImageURL(value string, folder *model.Folder, matcher *libraryMatcher, owner model.User) string {
+func resolveImageURL(value string, folder *model.Folder, matcher *model.LibraryMatcher, owner model.User) string {
value = strings.TrimSpace(value)
if value == "" {
return ""
@@ -308,7 +254,7 @@ func resolveImageURL(value string, folder *model.Folder, matcher *libraryMatcher
return ""
}
- lib, ok := matcher.findLibrary(localPath)
+ lib, ok := matcher.FindLibrary(localPath)
// A playlist without a folder (API upload, or CLI import from outside all libraries) may only use the owner's libraries.
if !ok || (folder == nil && !owner.HasLibraryAccess(lib.ID)) {
return ""
diff --git a/core/playlists/parse_m3u_test.go b/core/playlists/parse_m3u_test.go
index b6a3a96f9..ced6c2b16 100644
--- a/core/playlists/parse_m3u_test.go
+++ b/core/playlists/parse_m3u_test.go
@@ -9,187 +9,6 @@ import (
. "github.com/onsi/gomega"
)
-var _ = Describe("libraryMatcher", func() {
- var ds *tests.MockDataStore
- var mockLibRepo *tests.MockLibraryRepo
- ctx := context.Background()
-
- BeforeEach(func() {
- tests.SkipOnWindows("path separator bug (#TBD-path-sep-playlists)")
- mockLibRepo = &tests.MockLibraryRepo{}
- ds = &tests.MockDataStore{
- MockedLibrary: mockLibRepo,
- }
- })
-
- // Helper function to create a libraryMatcher from the mock datastore
- createMatcher := func(ds model.DataStore) *libraryMatcher {
- libs, err := ds.Library().GetAll(ctx)
- Expect(err).ToNot(HaveOccurred())
- return newLibraryMatcher(libs)
- }
-
- Describe("Longest library path matching", func() {
- It("matches the longest library path when multiple libraries share a prefix", func() {
- // Setup libraries with prefix conflicts
- mockLibRepo.SetData([]model.Library{
- {ID: 1, Path: "/music"},
- {ID: 2, Path: "/music-classical"},
- {ID: 3, Path: "/music-classical/opera"},
- })
-
- matcher := createMatcher(ds)
-
- // Test that longest path matches first and returns correct library ID
- testCases := []struct {
- path string
- expectedLibID int
- expectedLibPath string
- }{
- {"/music-classical/opera/track.mp3", 3, "/music-classical/opera"},
- {"/music-classical/track.mp3", 2, "/music-classical"},
- {"/music/track.mp3", 1, "/music"},
- {"/music-classical/opera/subdir/file.mp3", 3, "/music-classical/opera"},
- }
-
- for _, tc := range testCases {
- libID, libPath := matcher.findLibraryForPath(tc.path)
- Expect(libID).To(Equal(tc.expectedLibID), "Path %s should match library ID %d, but got %d", tc.path, tc.expectedLibID, libID)
- Expect(libPath).To(Equal(tc.expectedLibPath), "Path %s should match library path %s, but got %s", tc.path, tc.expectedLibPath, libPath)
- }
- })
-
- It("handles libraries with similar prefixes but different structures", func() {
- mockLibRepo.SetData([]model.Library{
- {ID: 1, Path: "/home/user/music"},
- {ID: 2, Path: "/home/user/music-backup"},
- })
-
- matcher := createMatcher(ds)
-
- // Test that music-backup library is matched correctly
- libID, libPath := matcher.findLibraryForPath("/home/user/music-backup/track.mp3")
- Expect(libID).To(Equal(2))
- Expect(libPath).To(Equal("/home/user/music-backup"))
-
- // Test that music library is still matched correctly
- libID, libPath = matcher.findLibraryForPath("/home/user/music/track.mp3")
- Expect(libID).To(Equal(1))
- Expect(libPath).To(Equal("/home/user/music"))
- })
-
- It("matches path that is exactly the library root", func() {
- mockLibRepo.SetData([]model.Library{
- {ID: 1, Path: "/music"},
- {ID: 2, Path: "/music-classical"},
- })
-
- matcher := createMatcher(ds)
-
- // Exact library path should match
- libID, libPath := matcher.findLibraryForPath("/music-classical")
- Expect(libID).To(Equal(2))
- Expect(libPath).To(Equal("/music-classical"))
- })
-
- It("handles complex nested library structures", func() {
- mockLibRepo.SetData([]model.Library{
- {ID: 1, Path: "/media"},
- {ID: 2, Path: "/media/audio"},
- {ID: 3, Path: "/media/audio/classical"},
- {ID: 4, Path: "/media/audio/classical/baroque"},
- })
-
- matcher := createMatcher(ds)
-
- testCases := []struct {
- path string
- expectedLibID int
- expectedLibPath string
- }{
- {"/media/audio/classical/baroque/bach/track.mp3", 4, "/media/audio/classical/baroque"},
- {"/media/audio/classical/mozart/track.mp3", 3, "/media/audio/classical"},
- {"/media/audio/rock/track.mp3", 2, "/media/audio"},
- {"/media/video/movie.mp4", 1, "/media"},
- }
-
- for _, tc := range testCases {
- libID, libPath := matcher.findLibraryForPath(tc.path)
- Expect(libID).To(Equal(tc.expectedLibID), "Path %s should match library ID %d", tc.path, tc.expectedLibID)
- Expect(libPath).To(Equal(tc.expectedLibPath), "Path %s should match library path %s", tc.path, tc.expectedLibPath)
- }
- })
- })
-
- Describe("Edge cases", func() {
- It("handles empty library list", func() {
- mockLibRepo.SetData([]model.Library{})
-
- matcher := createMatcher(ds)
- Expect(matcher).ToNot(BeNil())
-
- // Should not match anything
- libID, libPath := matcher.findLibraryForPath("/music/track.mp3")
- Expect(libID).To(Equal(0))
- Expect(libPath).To(BeEmpty())
- })
-
- It("handles single library", func() {
- mockLibRepo.SetData([]model.Library{
- {ID: 1, Path: "/music"},
- })
-
- matcher := createMatcher(ds)
-
- libID, libPath := matcher.findLibraryForPath("/music/track.mp3")
- Expect(libID).To(Equal(1))
- Expect(libPath).To(Equal("/music"))
- })
-
- It("handles libraries with special characters in paths", func() {
- mockLibRepo.SetData([]model.Library{
- {ID: 1, Path: "/music[test]"},
- {ID: 2, Path: "/music(backup)"},
- })
-
- matcher := createMatcher(ds)
- Expect(matcher).ToNot(BeNil())
-
- // Special characters should match literally
- libID, libPath := matcher.findLibraryForPath("/music[test]/track.mp3")
- Expect(libID).To(Equal(1))
- Expect(libPath).To(Equal("/music[test]"))
- })
- })
-
- Describe("Path matching order", func() {
- It("ensures longest paths match first", func() {
- mockLibRepo.SetData([]model.Library{
- {ID: 1, Path: "/a"},
- {ID: 2, Path: "/ab"},
- {ID: 3, Path: "/abc"},
- })
-
- matcher := createMatcher(ds)
-
- // Verify that longer paths match correctly (not cut off by shorter prefix)
- testCases := []struct {
- path string
- expectedLibID int
- }{
- {"/abc/file.mp3", 3},
- {"/ab/file.mp3", 2},
- {"/a/file.mp3", 1},
- }
-
- for _, tc := range testCases {
- libID, _ := matcher.findLibraryForPath(tc.path)
- Expect(libID).To(Equal(tc.expectedLibID), "Path %s should match library ID %d", tc.path, tc.expectedLibID)
- }
- })
- })
-})
-
var _ = Describe("pathResolver", func() {
var ds *tests.MockDataStore
var mockLibRepo *tests.MockLibraryRepo
diff --git a/db/migrations/20260929221042_add_library_pid_columns.sql b/db/migrations/20260929221042_add_library_pid_columns.sql
new file mode 100644
index 000000000..487512287
--- /dev/null
+++ b/db/migrations/20260929221042_add_library_pid_columns.sql
@@ -0,0 +1,18 @@
+-- +goose Up
+-- +goose StatementBegin
+alter table library add column pid_album varchar default '' not null;
+alter table library add column pid_track varchar default '' not null;
+alter table library add column scanned_pid_album varchar default '' not null;
+alter table library add column scanned_pid_track varchar default '' not null;
+
+-- Every library was scanned with the global PID config, so seed it as their scanned config.
+-- This way the upgrade does not trigger a full rescan.
+update library set
+ scanned_pid_album = coalesce((select value from property where id = 'PIDAlbum'), ''),
+ scanned_pid_track = coalesce((select value from property where id = 'PIDTrack'), '');
+
+delete from property where id in ('PIDAlbum', 'PIDTrack');
+-- +goose StatementEnd
+
+-- +goose Down
+SELECT 1;
diff --git a/model/library.go b/model/library.go
index 1e33222ac..e80d22c89 100644
--- a/model/library.go
+++ b/model/library.go
@@ -1,10 +1,13 @@
package model
import (
+ "cmp"
"context"
+ "strings"
"time"
"github.com/deluan/rest"
+ "github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/utils/slice"
)
@@ -27,6 +30,38 @@ type Library struct {
TotalSize int64 `json:"totalSize" db:"total_size"`
TotalDuration float64 `json:"totalDuration" db:"total_duration"`
DefaultNewUsers bool `json:"defaultNewUsers" db:"default_new_users"`
+ PIDAlbum string `json:"pidAlbum" db:"pid_album"`
+ PIDTrack string `json:"pidTrack" db:"pid_track"`
+ ScannedPIDAlbum string `json:"-" db:"scanned_pid_album"`
+ ScannedPIDTrack string `json:"-" db:"scanned_pid_track"`
+}
+
+// PIDConfig holds the persistent ID specs used to compute track and album IDs.
+type PIDConfig struct {
+ Track string
+ Album string
+}
+
+// EffectivePID returns the PID specs in effect for this library: its own overrides, falling back to
+// the global config.
+func (l Library) EffectivePID() PIDConfig {
+ return PIDConfig{
+ Track: cmp.Or(l.PIDTrack, conf.Server.PID.Track),
+ Album: cmp.Or(l.PIDAlbum, conf.Server.PID.Album),
+ }
+}
+
+// PIDChanged reports whether the effective PID specs differ from the ones used by the last finished
+// scan of this library. A library that was never scanned counts as changed.
+func (l Library) PIDChanged() bool {
+ pid := l.EffectivePID()
+ return !strings.EqualFold(l.ScannedPIDAlbum, pid.Album) || !strings.EqualFold(l.ScannedPIDTrack, pid.Track)
+}
+
+// NeedsPIDRescan reports whether the library has content imported with an old PID config, so it must be
+// rescanned in full. A library that never finished a scan has nothing to regroup.
+func (l Library) NeedsPIDRescan() bool {
+ return !l.LastScanAt.IsZero() && l.PIDChanged()
}
const (
@@ -59,6 +94,8 @@ type LibraryRepository interface {
// TODO These methods should be moved to a core service
ScanBegin(ctx context.Context, id int, fullScan bool) error
ScanEnd(ctx context.Context, id int) error
+ // SetScannedPID records the PID specs used by the last finished scan of the library
+ SetScannedPID(ctx context.Context, id int, pid PIDConfig) error
ScanInProgress(ctx context.Context) (bool, error)
RefreshStats(ctx context.Context, id int) error
}
diff --git a/model/library_matcher.go b/model/library_matcher.go
new file mode 100644
index 000000000..83af96f9f
--- /dev/null
+++ b/model/library_matcher.go
@@ -0,0 +1,57 @@
+package model
+
+import (
+ "cmp"
+ "path/filepath"
+ "slices"
+ "strings"
+)
+
+// LibraryMatcher finds the library that contains an absolute path.
+type LibraryMatcher struct {
+ libraries Libraries
+ cleanedPaths []string
+}
+
+// NewLibraryMatcher sorts the libraries longest path first, so /music-classical is checked before /music.
+func NewLibraryMatcher(libs Libraries) *LibraryMatcher {
+ libs = slices.Clone(libs)
+ slices.SortFunc(libs, func(i, j Library) int {
+ return cmp.Compare(len(j.Path), len(i.Path))
+ })
+ cleanedPaths := make([]string, len(libs))
+ for i, lib := range libs {
+ cleanedPaths[i] = filepath.Clean(lib.Path)
+ }
+ return &LibraryMatcher{libraries: libs, cleanedPaths: cleanedPaths}
+}
+
+// FindLibrary returns the library whose path contains absolutePath.
+func (lm *LibraryMatcher) FindLibrary(absolutePath string) (Library, bool) {
+ for i, libPath := range lm.cleanedPaths {
+ // A cleaned path only ends with a separator when it is a filesystem root
+ if strings.HasPrefix(absolutePath, libPath) && (len(absolutePath) == len(libPath) ||
+ absolutePath[len(libPath)] == filepath.Separator || strings.HasSuffix(libPath, string(filepath.Separator))) {
+ return lm.libraries[i], true
+ }
+ }
+ return Library{}, false
+}
+
+// LibraryRelativePath rebases an absolute path onto the library root, as the scanner's io/fs sees it
+// (forward slashes). Relative paths, and absolute paths outside the library root, are returned unchanged.
+func LibraryRelativePath(libPath, path string) string {
+ if !filepath.IsAbs(path) {
+ return path
+ }
+ // The library root may be relative (e.g. the default "./music"); it resolves against the same cwd
+ absLib, err := filepath.Abs(libPath)
+ if err != nil {
+ return path
+ }
+ rel, err := filepath.Rel(absLib, path)
+ if err != nil || !filepath.IsLocal(rel) {
+ return path
+ }
+ return filepath.ToSlash(rel)
+}
diff --git a/model/library_matcher_test.go b/model/library_matcher_test.go
new file mode 100644
index 000000000..09e6f7e33
--- /dev/null
+++ b/model/library_matcher_test.go
@@ -0,0 +1,91 @@
+package model_test
+
+import (
+ "os"
+ "path/filepath"
+
+ "github.com/navidrome/navidrome/model"
+ . "github.com/onsi/ginkgo/v2"
+ . "github.com/onsi/gomega"
+)
+
+var _ = Describe("LibraryMatcher", func() {
+ // Paths are written Unix-style and converted, so they use the OS separator, as filepath.Abs output does
+ find := func(libs model.Libraries, path string) int {
+ for i := range libs {
+ libs[i].Path = filepath.FromSlash(libs[i].Path)
+ }
+ lib, ok := model.NewLibraryMatcher(libs).FindLibrary(filepath.FromSlash(path))
+ if !ok {
+ return 0
+ }
+ return lib.ID
+ }
+
+ DescribeTable("matches the longest library path",
+ func(libs model.Libraries, path string, expectedID int) {
+ Expect(find(libs, path)).To(Equal(expectedID))
+ },
+ Entry("nested library", model.Libraries{{ID: 1, Path: "/music"}, {ID: 2, Path: "/music-classical"}, {ID: 3, Path: "/music-classical/opera"}}, "/music-classical/opera/subdir/track.mp3", 3),
+ Entry("sibling with a shared prefix", model.Libraries{{ID: 1, Path: "/music"}, {ID: 2, Path: "/music-classical"}}, "/music-classical/track.mp3", 2),
+ Entry("shorter library", model.Libraries{{ID: 1, Path: "/music"}, {ID: 2, Path: "/music-classical"}}, "/music/track.mp3", 1),
+ Entry("exact library root", model.Libraries{{ID: 1, Path: "/music"}, {ID: 2, Path: "/music-classical"}}, "/music-classical", 2),
+ Entry("deeply nested libraries", model.Libraries{{ID: 1, Path: "/media"}, {ID: 2, Path: "/media/audio"}, {ID: 3, Path: "/media/audio/classical"}, {ID: 4, Path: "/media/audio/classical/baroque"}}, "/media/audio/classical/mozart/track.mp3", 3),
+ Entry("prefix that is not a path boundary", model.Libraries{{ID: 1, Path: "/a"}, {ID: 2, Path: "/ab"}, {ID: 3, Path: "/abc"}}, "/ab/file.mp3", 2),
+ Entry("special characters match literally", model.Libraries{{ID: 1, Path: "/music[test]"}, {ID: 2, Path: "/music(backup)"}}, "/music[test]/track.mp3", 1),
+ Entry("library path with a trailing slash", model.Libraries{{ID: 1, Path: "/music/"}}, "/music/track.mp3", 1),
+ Entry("library at the filesystem root", model.Libraries{{ID: 1, Path: "/"}}, "/music/track.mp3", 1),
+ Entry("nested library under a root library", model.Libraries{{ID: 1, Path: "/"}, {ID: 2, Path: "/music"}}, "/music/track.mp3", 2),
+ )
+
+ It("does not match a path outside every library", func() {
+ Expect(find(model.Libraries{{ID: 1, Path: "/music"}}, "/music-backup/track.mp3")).To(BeZero())
+ })
+
+ It("does not match anything without libraries", func() {
+ Expect(find(nil, "/music/track.mp3")).To(BeZero())
+ })
+
+ It("does not reorder the caller's libraries", func() {
+ libs := model.Libraries{{ID: 1, Path: "/a"}, {ID: 2, Path: "/abc"}}
+ model.NewLibraryMatcher(libs)
+ Expect(libs.IDs()).To(Equal([]int{1, 2}))
+ })
+})
+
+var _ = Describe("LibraryRelativePath", func() {
+ // Paths are built with filepath so the "absolute" cases stay absolute on every OS
+ // (a Unix-style "/foo" is not absolute on Windows).
+ libRoot, _ := filepath.Abs(filepath.Join("jukebox", "collection"))
+ outside, _ := filepath.Abs(filepath.Join("somewhere", "else"))
+
+ It("returns a relative path unchanged", func() {
+ Expect(model.LibraryRelativePath(libRoot, "_Collection")).To(Equal("_Collection"))
+ })
+
+ It("rebases an absolute target when the library root is relative", func() {
+ cwd, err := os.Getwd()
+ Expect(err).ToNot(HaveOccurred())
+ Expect(model.LibraryRelativePath(filepath.Join("music", "library"), filepath.Join(cwd, "music", "library", "rock"))).To(Equal("rock"))
+ })
+
+ It("rebases an absolute path that equals the library root to '.'", func() {
+ Expect(model.LibraryRelativePath(libRoot, libRoot)).To(Equal("."))
+ })
+
+ It("rebases an absolute path under the library root", func() {
+ Expect(model.LibraryRelativePath(libRoot, filepath.Join(libRoot, "_Collection"))).To(Equal("_Collection"))
+ })
+
+ It("handles a trailing slash on the library path", func() {
+ Expect(model.LibraryRelativePath(libRoot+string(filepath.Separator), filepath.Join(libRoot, "_Collection"))).To(Equal("_Collection"))
+ })
+
+ It("leaves an absolute path outside the library root unchanged", func() {
+ Expect(model.LibraryRelativePath(libRoot, outside)).To(Equal(outside))
+ })
+
+ It("returns an empty path unchanged", func() {
+ Expect(model.LibraryRelativePath(libRoot, "")).To(Equal(""))
+ })
+})
diff --git a/model/library_test.go b/model/library_test.go
new file mode 100644
index 000000000..4e799e13f
--- /dev/null
+++ b/model/library_test.go
@@ -0,0 +1,73 @@
+package model_test
+
+import (
+ "encoding/json"
+ "time"
+
+ "github.com/navidrome/navidrome/conf"
+ "github.com/navidrome/navidrome/conf/configtest"
+ "github.com/navidrome/navidrome/model"
+ . "github.com/onsi/ginkgo/v2"
+ . "github.com/onsi/gomega"
+)
+
+var _ = Describe("Library PID config", func() {
+ BeforeEach(func() {
+ DeferCleanup(configtest.SetupConfig())
+ conf.Server.PID.Album = "global_album"
+ conf.Server.PID.Track = "global_track"
+ })
+
+ Describe("EffectivePID", func() {
+ It("falls back to the global config", func() {
+ Expect(model.Library{}.EffectivePID()).To(Equal(model.PIDConfig{Track: "global_track", Album: "global_album"}))
+ })
+ It("uses the library overrides", func() {
+ lib := model.Library{PIDAlbum: "folder", PIDTrack: "title"}
+ Expect(lib.EffectivePID()).To(Equal(model.PIDConfig{Track: "title", Album: "folder"}))
+ })
+ })
+
+ Describe("PIDChanged", func() {
+ It("is false when the scanned specs match, ignoring case", func() {
+ lib := model.Library{ScannedPIDAlbum: "GLOBAL_ALBUM", ScannedPIDTrack: "global_track"}
+ Expect(lib.PIDChanged()).To(BeFalse())
+ })
+ It("is true when the album override differs from the scanned spec", func() {
+ lib := model.Library{PIDAlbum: "folder", ScannedPIDAlbum: "global_album", ScannedPIDTrack: "global_track"}
+ Expect(lib.PIDChanged()).To(BeTrue())
+ })
+ It("is true when only the track spec changed", func() {
+ lib := model.Library{PIDTrack: "title", ScannedPIDAlbum: "global_album", ScannedPIDTrack: "global_track"}
+ Expect(lib.PIDChanged()).To(BeTrue())
+ })
+ It("is true when the global config changed for a library without overrides", func() {
+ lib := model.Library{ScannedPIDAlbum: "old_album", ScannedPIDTrack: "global_track"}
+ Expect(lib.PIDChanged()).To(BeTrue())
+ })
+ It("is true for a library that was never scanned", func() {
+ Expect(model.Library{}.PIDChanged()).To(BeTrue())
+ })
+ })
+
+ Describe("NeedsPIDRescan", func() {
+ It("is false for a library that never finished a scan", func() {
+ Expect(model.Library{PIDAlbum: "folder"}.NeedsPIDRescan()).To(BeFalse())
+ })
+ It("is true for a scanned library whose PID config changed", func() {
+ lib := model.Library{PIDAlbum: "folder", ScannedPIDAlbum: "global_album", ScannedPIDTrack: "global_track", LastScanAt: time.Now()}
+ Expect(lib.NeedsPIDRescan()).To(BeTrue())
+ })
+ It("is false for a scanned library whose PID config did not change", func() {
+ lib := model.Library{ScannedPIDAlbum: "global_album", ScannedPIDTrack: "global_track", LastScanAt: time.Now()}
+ Expect(lib.NeedsPIDRescan()).To(BeFalse())
+ })
+ })
+
+ It("does not expose the scanned specs in JSON", func() {
+ data, err := json.Marshal(model.Library{PIDAlbum: "folder", ScannedPIDAlbum: "secret_album", ScannedPIDTrack: "secret_track"})
+ Expect(err).ToNot(HaveOccurred())
+ Expect(string(data)).To(ContainSubstring(`"pidAlbum":"folder"`))
+ Expect(string(data)).ToNot(ContainSubstring("secret_"))
+ })
+})
diff --git a/model/metadata/map_mediafile.go b/model/metadata/map_mediafile.go
index 6d12feba9..2135824d2 100644
--- a/model/metadata/map_mediafile.go
+++ b/model/metadata/map_mediafile.go
@@ -8,15 +8,14 @@ import (
"math"
"strconv"
- "github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/str"
)
-func (md Metadata) ToMediaFile(libID int, folderID string) model.MediaFile {
+func (md Metadata) ToMediaFile(lib model.Library, folderID string) model.MediaFile {
mf := model.MediaFile{
- LibraryID: libID,
+ LibraryID: lib.ID,
FolderID: folderID,
Tags: maps.Clone(md.tags),
}
@@ -84,8 +83,9 @@ func (md Metadata) ToMediaFile(libID int, folderID string) model.MediaFile {
mf.AlbumArtist = md.mapDisplayAlbumArtist(mf)
// Persistent IDs
- mf.PID = md.trackPID(mf)
- mf.AlbumID = md.albumID(mf, conf.Server.PID.Album)
+ pid := lib.EffectivePID()
+ mf.PID = md.trackPID(mf, pid)
+ mf.AlbumID = md.albumID(mf, pid.Album)
// BFR These IDs will go away once the UI handle multiple participants.
// BFR For Legacy Subsonic compatibility, we will set them in the API handlers
diff --git a/model/metadata/map_mediafile_test.go b/model/metadata/map_mediafile_test.go
index baaf8fab5..c19398841 100644
--- a/model/metadata/map_mediafile_test.go
+++ b/model/metadata/map_mediafile_test.go
@@ -30,9 +30,23 @@ var _ = Describe("ToMediaFile", func() {
var toMediaFile = func(tags model.RawTags) model.MediaFile {
props.Tags = tags
md = metadata.New("filepath", props)
- return md.ToMediaFile(1, "folderID")
+ return md.ToMediaFile(model.Library{ID: 1}, "folderID")
}
+ Describe("Persistent IDs", func() {
+ It("uses the library PID config for the album ID and for albumid in the track spec", func() {
+ props.Tags = model.RawTags{"ALBUM": {"Kind of Blue"}, "TITLE": {"So What"}}
+ md = metadata.New("Jazz/Loose/01.mp3", props)
+
+ byTags := md.ToMediaFile(model.Library{ID: 1, PIDAlbum: "album", PIDTrack: "albumid,title"}, "folderID")
+ byFolder := md.ToMediaFile(model.Library{ID: 1, PIDAlbum: "folder", PIDTrack: "albumid,title"}, "folderID")
+
+ Expect(byFolder.AlbumID).ToNot(Equal(byTags.AlbumID))
+ Expect(byFolder.AlbumID).To(Equal(md.AlbumID(byFolder, "folder")))
+ Expect(byFolder.PID).ToNot(Equal(byTags.PID))
+ })
+ })
+
Describe("Dates", func() {
It("should parse properly tagged dates ", func() {
mf = toMediaFile(model.RawTags{
diff --git a/model/metadata/map_participants_test.go b/model/metadata/map_participants_test.go
index ec66e12b9..db652fb8b 100644
--- a/model/metadata/map_participants_test.go
+++ b/model/metadata/map_participants_test.go
@@ -38,7 +38,7 @@ var _ = Describe("Participants", func() {
var toMediaFile = func(tags model.RawTags) model.MediaFile {
props.Tags = tags
md = metadata.New("filepath", props)
- return md.ToMediaFile(1, "folderID")
+ return md.ToMediaFile(model.Library{ID: 1}, "folderID")
}
Describe("ARTIST(S) tags", func() {
diff --git a/model/metadata/metadata_test.go b/model/metadata/metadata_test.go
index c84d93981..a1a675006 100644
--- a/model/metadata/metadata_test.go
+++ b/model/metadata/metadata_test.go
@@ -323,7 +323,7 @@ var _ = Describe("Metadata", func() {
tag: {tagValue},
}
md = metadata.New(filePath, props)
- return md.ToMediaFile(0, "0")
+ return md.ToMediaFile(model.Library{}, "0")
}
DescribeTable("Gain",
diff --git a/model/metadata/persistent_ids.go b/model/metadata/persistent_ids.go
index db315dc6b..b66ce824a 100644
--- a/model/metadata/persistent_ids.go
+++ b/model/metadata/persistent_ids.go
@@ -2,11 +2,11 @@ package metadata
import (
"cmp"
+ "errors"
"fmt"
"path/filepath"
"strings"
- "github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
@@ -22,12 +22,13 @@ type hashFunc = func(...string) string
// attributes. Attributes can be either tags or processed values like folder,
// albumid, albumartistid, etc. For each field, it gets all its attribute values
// and concatenates them, then hashes the result. If a field is empty, it is
-// skipped and the function looks for the next field.
+// skipped and the function looks for the next field. albumSpec is the album PID
+// spec used to resolve the `albumid` attribute.
//
// Taking hash as a parameter (instead of closing over it in a factory) keeps
// mf on the stack: closing over mf would force the whole ~1KB MediaFile to the
// heap on every call.
-func computePID(mf model.MediaFile, md Metadata, spec string, prependLibId bool, hash hashFunc) string {
+func computePID(mf model.MediaFile, md Metadata, spec, albumSpec string, prependLibId bool, hash hashFunc) string {
switch spec {
case "track_legacy":
return legacyTrackID(mf, prependLibId)
@@ -41,7 +42,7 @@ func computePID(mf model.MediaFile, md Metadata, spec string, prependLibId bool,
values := make([]string, len(attributes))
hasValue := false
for i, attr := range attributes {
- v := getPIDAttr(mf, md, attr, prependLibId, spec, hash)
+ v := getPIDAttr(mf, md, attr, prependLibId, spec, albumSpec, hash)
if v != "" {
hasValue = true
}
@@ -58,15 +59,15 @@ func computePID(mf model.MediaFile, md Metadata, spec string, prependLibId bool,
return hash(pid)
}
-func getPIDAttr(mf model.MediaFile, md Metadata, attr string, prependLibId bool, spec string, hash hashFunc) string {
+func getPIDAttr(mf model.MediaFile, md Metadata, attr string, prependLibId bool, spec, albumSpec string, hash hashFunc) string {
attr = strings.TrimSpace(strings.ToLower(attr))
switch attr {
case "albumid":
- if spec == conf.Server.PID.Album {
+ if spec == albumSpec {
log.Error("Recursive PID definition detected, ignoring `albumid`", "spec", spec)
return ""
}
- return computePID(mf, md, conf.Server.PID.Album, prependLibId, hash)
+ return computePID(mf, md, albumSpec, albumSpec, prependLibId, hash)
case "folder":
return filepath.Dir(mf.Path)
case "albumartistid":
@@ -79,18 +80,50 @@ func getPIDAttr(mf model.MediaFile, md Metadata, attr string, prependLibId bool,
return md.String(model.TagName(attr))
}
-func (md Metadata) trackPID(mf model.MediaFile) string {
- return computePID(mf, md, conf.Server.PID.Track, true, id.NewHash)
+// ValidatePIDSpec checks a PID override before it is stored; empty means "use the global config".
+// Aliases resolve to empty at scan time: accepted only in track specs, because the default one uses them.
+func ValidatePIDSpec(spec string, isAlbum bool) error {
+ switch {
+ case spec == "", isAlbum && spec == "album_legacy", !isAlbum && spec == "track_legacy":
+ return nil
+ }
+ for field := range strings.SplitSeq(spec, "|") {
+ for attr := range strings.SplitSeq(field, ",") {
+ attr = strings.TrimSpace(strings.ToLower(attr))
+ switch attr {
+ case "":
+ return fmt.Errorf("empty attribute in %q", spec)
+ case "albumid":
+ if isAlbum {
+ return errors.New("albumid cannot be used in an album PID")
+ }
+ case "folder", "albumartistid":
+ default:
+ name, ok := model.CanonicalTagName(attr)
+ if !ok {
+ return fmt.Errorf("unknown attribute %q", attr)
+ }
+ if isAlbum && string(name) != attr {
+ return fmt.Errorf("use the tag name %q instead of its alias %q", name, attr)
+ }
+ }
+ }
+ }
+ return nil
+}
+
+func (md Metadata) trackPID(mf model.MediaFile, pid model.PIDConfig) string {
+ return computePID(mf, md, pid.Track, pid.Album, true, id.NewHash)
}
func (md Metadata) albumID(mf model.MediaFile, pidConf string) string {
- return computePID(mf, md, pidConf, true, id.NewHash)
+ return computePID(mf, md, pidConf, pidConf, true, id.NewHash)
}
// BFR Must be configurable?
func (md Metadata) artistID(name string) string {
mf := model.MediaFile{AlbumArtist: name}
- return computePID(mf, md, "albumartistid", false, id.NewHash)
+ return computePID(mf, md, "albumartistid", "", false, id.NewHash)
}
func (md Metadata) mapTrackTitle() string {
diff --git a/model/metadata/persistent_ids_test.go b/model/metadata/persistent_ids_test.go
index 8e38bbd42..9f6eaf1f4 100644
--- a/model/metadata/persistent_ids_test.go
+++ b/model/metadata/persistent_ids_test.go
@@ -3,8 +3,7 @@ package metadata
import (
"strings"
- "github.com/navidrome/navidrome/conf"
- "github.com/navidrome/navidrome/conf/configtest"
+ "github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
@@ -13,16 +12,18 @@ import (
var _ = Describe("getPID", func() {
var (
- md Metadata
- mf model.MediaFile
- sum hashFunc
+ md Metadata
+ mf model.MediaFile
+ sum hashFunc
+ albumSpec string
)
getPID := func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string {
- return computePID(mf, md, spec, prependLibId, sum)
+ return computePID(mf, md, spec, albumSpec, prependLibId, sum)
}
BeforeEach(func() {
sum = func(s ...string) string { return "(" + strings.Join(s, ",") + ")" }
+ albumSpec = consts.DefaultAlbumPID
})
Context("attributes are tags", func() {
@@ -66,8 +67,7 @@ var _ = Describe("getPID", func() {
Context("calculated attributes", func() {
BeforeEach(func() {
- DeferCleanup(configtest.SetupConfig())
- conf.Server.PID.Album = "musicbrainz_albumid|albumartistid,album,albumversion,releasedate"
+ albumSpec = "musicbrainz_albumid|albumartistid,album,albumversion,releasedate"
})
When("field is title", func() {
It("should return the pid", func() {
@@ -121,8 +121,8 @@ var _ = Describe("getPID", func() {
When("albumid configuration refers to albumid recursively", func() {
It("should avoid infinite recursion", func() {
// Reproduce the issue from #4920
- conf.Server.PID.Album = "albumid,album,albumversion,releasedate"
- spec := conf.Server.PID.Album
+ albumSpec = "albumid,album,albumversion,releasedate"
+ spec := albumSpec
md.tags = map[model.TagName][]string{
"album": {"Album Name"},
"albumversion": {"Version"},
@@ -205,8 +205,7 @@ var _ = Describe("getPID", func() {
})
When("prependLibId is true with nested albumid", func() {
It("should handle nested albumid calls correctly", func() {
- DeferCleanup(configtest.SetupConfig())
- conf.Server.PID.Album = "album"
+ albumSpec = "album"
spec := "albumid"
md.tags = map[model.TagName][]string{"album": {"Test Album"}}
mf.AlbumArtist = "Test Artist"
@@ -306,3 +305,34 @@ var _ = Describe("getPID", func() {
})
})
})
+
+var _ = Describe("ValidatePIDSpec", func() {
+ DescribeTable("accepts valid specs",
+ func(spec string, isAlbum bool) {
+ Expect(ValidatePIDSpec(spec, isAlbum)).To(Succeed())
+ },
+ Entry("empty, meaning the global config", "", true),
+ Entry("default album spec", consts.DefaultAlbumPID, true),
+ Entry("default track spec, which uses tag aliases", consts.DefaultTrackPID, false),
+ Entry("folder", "folder", true),
+ Entry("album legacy", "album_legacy", true),
+ Entry("track legacy", "track_legacy", false),
+ Entry("computed attributes", "albumartistid,album|title", true),
+ Entry("albumid in a track spec", "albumid,title", false),
+ Entry("spaces and mixed case", "MusicBrainz_AlbumID | Folder", true),
+ )
+
+ DescribeTable("rejects invalid specs",
+ func(spec string, isAlbum bool, msg string) {
+ Expect(ValidatePIDSpec(spec, isAlbum)).To(MatchError(ContainSubstring(msg)))
+ },
+ Entry("unknown tag", "albmversion", true, `unknown attribute "albmversion"`),
+ Entry("empty field", "album||title", true, "empty attribute"),
+ Entry("empty attribute", "album,,title", true, "empty attribute"),
+ Entry("trailing separator", "album|", true, "empty attribute"),
+ Entry("albumid in an album spec", "albumid,album", true, "albumid"),
+ Entry("tag alias in an album spec", "talb", true, `use the tag name "album" instead of its alias "talb"`),
+ Entry("track legacy in an album spec", "track_legacy", true, `unknown attribute "track_legacy"`),
+ Entry("album legacy in a track spec", "album_legacy", false, `unknown attribute "album_legacy"`),
+ )
+})
diff --git a/model/scanner.go b/model/scanner.go
index 36c9007fb..d22c3d0d6 100644
--- a/model/scanner.go
+++ b/model/scanner.go
@@ -2,12 +2,15 @@ package model
import (
"context"
+ "errors"
"fmt"
"strconv"
"strings"
"time"
)
+var ErrAlreadyScanning = errors.New("already scanning")
+
// ScanTarget represents a specific folder within a library to be scanned.
// NOTE: This struct is used as a map key, so it should only contain comparable types.
type ScanTarget struct {
diff --git a/model/tag_mappings.go b/model/tag_mappings.go
index ce7d2f37b..5a8168754 100644
--- a/model/tag_mappings.go
+++ b/model/tag_mappings.go
@@ -195,6 +195,28 @@ func TagMappings() map[TagName]TagConf {
return mappings
}
+// CanonicalTagName returns the mapped tag that name is, or is an alias of. Tags are stored under this name.
+func CanonicalTagName(name string) (TagName, bool) {
+ tagName, ok := tagNameIndex()[TagName(name).ToLower()]
+ return tagName, ok
+}
+
+// tagNameIndex maps every tag name and alias to its tag name. Names are added last, so they win over aliases
+// (musicbrainz_trackid is a tag and also an alias of musicbrainz_recordingid).
+var tagNameIndex = sync.OnceValue(func() map[TagName]TagName {
+ mappings := TagMappings()
+ index := make(map[TagName]TagName, len(mappings))
+ for name, tag := range mappings {
+ for _, alias := range tag.Aliases {
+ index[TagName(alias)] = name
+ }
+ }
+ for name := range mappings {
+ index[name] = name
+ }
+ return index
+})
+
func TagRolesConf() TagConf {
_, cfg := parseMappings()
return cfg.Roles
diff --git a/model/tag_mappings_test.go b/model/tag_mappings_test.go
index e582c3f2f..91e54e5d4 100644
--- a/model/tag_mappings_test.go
+++ b/model/tag_mappings_test.go
@@ -192,3 +192,22 @@ var _ = Describe("TagConf", func() {
})
})
})
+
+var _ = Describe("CanonicalTagName", func() {
+ DescribeTable("resolves tag names and aliases",
+ func(name string, expected TagName) {
+ tagName, ok := CanonicalTagName(name)
+ Expect(ok).To(BeTrue())
+ Expect(tagName).To(Equal(expected))
+ },
+ Entry("tag name", "album", TagAlbum),
+ Entry("alias", "talb", TagAlbum),
+ Entry("mixed case alias", "TALB", TagAlbum),
+ Entry("tag name that is also an alias of another tag", "musicbrainz_trackid", TagMusicBrainzTrackID),
+ )
+
+ It("does not resolve an unknown name", func() {
+ _, ok := CanonicalTagName("nosuchtag")
+ Expect(ok).To(BeFalse())
+ })
+})
diff --git a/persistence/library_repository.go b/persistence/library_repository.go
index bf6b8995e..85da65cbc 100644
--- a/persistence/library_repository.go
+++ b/persistence/library_repository.go
@@ -93,6 +93,8 @@ func (r *libraryRepository) Put(ctx context.Context, l *model.Library, colsToUpd
"path": l.Path,
"remote_path": l.RemotePath,
"default_new_users": l.DefaultNewUsers,
+ "pid_album": l.PIDAlbum,
+ "pid_track": l.PIDTrack,
}, colsToUpdate...)
cols["updated_at"] = l.UpdatedAt
sq := Update(r.tableName).SetMap(cols).Where(Eq{"id": l.ID})
@@ -176,6 +178,15 @@ func (r *libraryRepository) ScanEnd(ctx context.Context, id int) error {
return err
}
+func (r *libraryRepository) SetScannedPID(ctx context.Context, id int, pid model.PIDConfig) error {
+ sq := Update(r.tableName).
+ Set("scanned_pid_album", pid.Album).
+ Set("scanned_pid_track", pid.Track).
+ Where(Eq{"id": id})
+ _, err := r.executeSQL(ctx, sq)
+ return err
+}
+
func (r *libraryRepository) ScanInProgress(ctx context.Context) (bool, error) {
query := r.newSelect(ctx).Where(NotEq{"last_scan_started_at": time.Time{}})
count, err := r.count(ctx, query)
diff --git a/persistence/library_repository_test.go b/persistence/library_repository_test.go
index 0ff470861..bf485a06f 100644
--- a/persistence/library_repository_test.go
+++ b/persistence/library_repository_test.go
@@ -270,6 +270,38 @@ var _ = Describe("LibraryRepository", func() {
})
})
+ Describe("PID config", func() {
+ It("stores the overrides, and Put never touches the scanned specs", func() {
+ lib := &model.Library{Name: "PID Library", Path: "/music/pid", PIDAlbum: "folder", PIDTrack: "title"}
+ Expect(repo.Put(ctx, lib)).To(Succeed())
+ Expect(repo.SetScannedPID(ctx, lib.ID, model.PIDConfig{Album: "folder", Track: "title"})).To(Succeed())
+
+ // An update coming from the REST API has no scanned specs. It must not clear them
+ update := &model.Library{ID: lib.ID, Name: "PID Library", Path: "/music/pid", PIDTrack: "title"}
+ Expect(repo.Put(ctx, update)).To(Succeed())
+
+ saved, err := repo.Get(ctx, lib.ID)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(saved.PIDAlbum).To(BeEmpty())
+ Expect(saved.PIDTrack).To(Equal("title"))
+ Expect(saved.ScannedPIDAlbum).To(Equal("folder"))
+ Expect(saved.ScannedPIDTrack).To(Equal("title"))
+ })
+
+ It("keeps the overrides when a partial update does not send them", func() {
+ lib := &model.Library{Name: "Partial", Path: "/music/partial", PIDAlbum: "folder", PIDTrack: "title"}
+ Expect(repo.Put(ctx, lib)).To(Succeed())
+
+ Expect(repo.Put(ctx, &model.Library{ID: lib.ID, Name: "Renamed"}, "name")).To(Succeed())
+
+ saved, err := repo.Get(ctx, lib.ID)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(saved.Name).To(Equal("Renamed"))
+ Expect(saved.PIDAlbum).To(Equal("folder"))
+ Expect(saved.PIDTrack).To(Equal("title"))
+ })
+ })
+
Describe("Delete", func() {
var adminRepo model.LibraryRepository
var artistRepo model.ArtistRepository
diff --git a/resources/i18n/pt-br.json b/resources/i18n/pt-br.json
index f0d8a0e06..238919259 100644
--- a/resources/i18n/pt-br.json
+++ b/resources/i18n/pt-br.json
@@ -328,11 +328,22 @@
"totalDuration": "Duração",
"defaultNewUsers": "Padrão para Novos Usuários",
"createdAt": "Data de Criação",
- "updatedAt": "Últ. Atualização"
+ "updatedAt": "Últ. Atualização",
+ "pidAlbum": "Agrupamento de álbuns",
+ "pidTrack": "Identificação das faixas"
},
"sections": {
"basic": "Informações Básicas",
- "statistics": "Estatísticas"
+ "statistics": "Estatísticas",
+ "pid": "IDs Persistentes"
+ },
+ "pid": {
+ "global": "Usar configuração global (%{value})",
+ "folder": "Pasta (um álbum por pasta)",
+ "custom": "Personalizado",
+ "spec": "Especificação do PID",
+ "help": "Tags e atributos que identificam um item. Consulte a sintaxe na documentação:",
+ "docs": "IDs Persistentes"
},
"actions": {
"scan": "Scanear Biblioteca",
@@ -362,7 +373,9 @@
"messages": {
"deleteConfirm": "Tem certeza que deseja excluir esta biblioteca? Isso removerá todos os dados associados.",
"scanInProgress": "Scan em progresso...",
- "noLibrariesAssigned": "Nenhuma biblioteca atribuída a este usuário"
+ "noLibrariesAssigned": "Nenhuma biblioteca atribuída a este usuário",
+ "pidChangeTitle": "Alterar os IDs persistentes?",
+ "pidChangeConfirm": "Ao salvar, os álbuns desta biblioteca serão reagrupados e as faixas serão identificadas novamente. Um scan completo da biblioteca começará imediatamente. As marcações como favoritas, as classificações e as contagens de reprodução das faixas serão mantidas. Os favoritos e as classificações dos álbuns serão transferidos para os novos álbuns quando um álbum antigo corresponder a um novo."
}
},
"plugin": {
diff --git a/scanner/controller.go b/scanner/controller.go
index 5eed6c58d..1b13c1846 100644
--- a/scanner/controller.go
+++ b/scanner/controller.go
@@ -25,7 +25,7 @@ import (
)
var (
- ErrAlreadyScanning = errors.New("already scanning")
+ ErrAlreadyScanning = model.ErrAlreadyScanning
)
func New(rootCtx context.Context, ds model.DataStore, broker events.Broker,
@@ -304,14 +304,14 @@ func LockForMaintenance() (func(), bool) {
return scanMaintenanceMux.Unlock, true
}
-// EffectiveFullScan reports whether a scan was requested as full or will resume an interrupted
-// full scan in one of the included libraries.
+// EffectiveFullScan reports whether a scan was requested as full, will resume an interrupted full scan,
+// or will rescan a library in full because its PID config changed, in one of the included libraries.
func EffectiveFullScan(ctx context.Context, ds model.DataStore, fullScan bool, targets []model.ScanTarget) bool {
if fullScan {
return true
}
return anyIncludedLibrary(ctx, ds, targets, func(library model.Library) bool {
- return library.FullScanInProgress
+ return library.FullScanInProgress || library.NeedsPIDRescan()
})
}
diff --git a/scanner/controller_test.go b/scanner/controller_test.go
index bdcb99eda..974540e32 100644
--- a/scanner/controller_test.go
+++ b/scanner/controller_test.go
@@ -2,6 +2,7 @@ package scanner_test
import (
"context"
+ "time"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/consts"
@@ -70,14 +71,21 @@ var _ = Describe("EffectiveFullScan", func() {
var ds *tests.MockDataStore
BeforeEach(func() {
+ pid := model.Library{}.EffectivePID()
libraries := &tests.MockLibraryRepo{}
libraries.SetData(model.Libraries{
- {ID: 1, FullScanInProgress: true},
- {ID: 2},
+ {ID: 1, FullScanInProgress: true, ScannedPIDAlbum: pid.Album, ScannedPIDTrack: pid.Track},
+ {ID: 2, ScannedPIDAlbum: pid.Album, ScannedPIDTrack: pid.Track},
+ {ID: 3, LastScanAt: time.Now(), PIDAlbum: "folder", ScannedPIDAlbum: pid.Album, ScannedPIDTrack: pid.Track},
})
ds = &tests.MockDataStore{MockedLibrary: libraries}
})
+ It("detects a library that needs a full rescan for a PID change", func() {
+ targets := []model.ScanTarget{{LibraryID: 3, FolderPath: "."}}
+ Expect(scanner.EffectiveFullScan(GinkgoT().Context(), ds, false, targets)).To(BeTrue())
+ })
+
It("detects an interrupted full scan in a targeted library", func() {
targets := []model.ScanTarget{{LibraryID: 1, FolderPath: "."}}
Expect(scanner.EffectiveFullScan(context.Background(), ds, false, targets)).To(BeTrue())
diff --git a/scanner/phase_1_folders.go b/scanner/phase_1_folders.go
index 7b6a6b097..4edaecacd 100644
--- a/scanner/phase_1_folders.go
+++ b/scanner/phase_1_folders.go
@@ -40,6 +40,7 @@ func createPhaseFolders(ctx context.Context, state *scanState, ds model.DataStor
if err != nil {
log.Error(ctx, "Scanner: Error creating scan context", "lib", lib.Name, err)
state.sendError(err)
+ state.markFailed(lib.ID)
continue
}
jobs = append(jobs, job)
@@ -51,12 +52,13 @@ func createPhaseFolders(ctx context.Context, state *scanState, ds model.DataStor
}
type scanJob struct {
- lib model.Library
- fs storage.MusicFS
- lastUpdates map[string]model.FolderUpdateInfo // Holds last update info for all (DB) folders in this library
- targetFolders []string // Specific folders to scan (including all descendants)
- lock sync.Mutex
- numFolders atomic.Int64
+ lib model.Library
+ fs storage.MusicFS
+ lastUpdates map[string]model.FolderUpdateInfo // Holds last update info for all (DB) folders in this library
+ targetFolders []string // Specific folders to scan (including all descendants)
+ prevAlbumPIDConf string // Album PID spec of the last finished scan, only when it differs from the current one
+ lock sync.Mutex
+ numFolders atomic.Int64
}
func newScanJob(ctx context.Context, ds model.DataStore, lib model.Library, fullScan bool, targetFolders []string) (*scanJob, error) {
@@ -77,16 +79,32 @@ func newScanJob(ctx context.Context, ds model.DataStore, lib model.Library, full
return nil, fmt.Errorf("getting fs for library: %w", err)
}
+ pid := lib.EffectivePID()
+ if lib.NeedsPIDRescan() {
+ msg := "Scanner: PID config changed, rescanning library in full"
+ if len(targetFolders) > 0 {
+ msg = "Scanner: PID config changed, rescanning target folders in full"
+ }
+ log.Info(ctx, msg, "lib", lib.Name, "targetFolders", targetFolders,
+ "album", pid.Album, "track", pid.Track, "scannedAlbum", lib.ScannedPIDAlbum, "scannedTrack", lib.ScannedPIDTrack)
+ fullScan = true
+ }
+ var prevAlbumPIDConf string
+ if lib.ScannedPIDAlbum != pid.Album {
+ prevAlbumPIDConf = lib.ScannedPIDAlbum
+ }
+
// Ensure FullScanInProgress reflects the current scan request.
// This is important when resuming an interrupted quick scan as a full scan:
// the DB may have FullScanInProgress=false, but we need it true for isOutdated() to work correctly.
lib.FullScanInProgress = lib.FullScanInProgress || fullScan
return &scanJob{
- lib: lib,
- fs: fsys,
- lastUpdates: lastUpdates,
- targetFolders: targetFolders,
+ lib: lib,
+ fs: fsys,
+ lastUpdates: lastUpdates,
+ targetFolders: targetFolders,
+ prevAlbumPIDConf: prevAlbumPIDConf,
}, nil
}
@@ -122,14 +140,13 @@ func (j *scanJob) createFolderEntry(path string) *folderEntry {
// The phaseFolders struct implements the phase interface, providing methods to produce
// folder entries, process folders, persist changes to the database, and log the results.
type phaseFolders struct {
- jobs []*scanJob
- ds model.DataStore
- ctx context.Context //nolint:containedctx // phase runs under a single scan ctx
- walkCtx context.Context //nolint:containedctx // cancelled when a folder fails to persist, so the walk stops early
- stopWalk context.CancelCauseFunc
- state *scanState
- prevAlbumPIDConf string
- imageChanges *imageChangeCollector
+ jobs []*scanJob
+ ds model.DataStore
+ ctx context.Context //nolint:containedctx // phase runs under a single scan ctx
+ walkCtx context.Context //nolint:containedctx // cancelled when a folder fails to persist, so the walk stops early
+ stopWalk context.CancelCauseFunc
+ state *scanState
+ imageChanges *imageChangeCollector
}
func (p *phaseFolders) description() string {
@@ -138,12 +155,6 @@ func (p *phaseFolders) description() string {
func (p *phaseFolders) producer() ppl.Producer[*folderEntry] {
return ppl.NewProducer(func(put func(entry *folderEntry)) error {
- var err error
- p.prevAlbumPIDConf, err = p.ds.Property().DefaultGet(p.ctx, consts.PIDAlbumKey, "")
- if err != nil {
- return fmt.Errorf("getting album PID conf: %w", err)
- }
-
// TODO Parallelize multiple job when we have multiple libraries
var total int64
var totalChanged int64
@@ -173,7 +184,7 @@ func (p *phaseFolders) producer() ppl.Producer[*folderEntry] {
// Check if folder is outdated
if folder.isOutdated() {
- if !p.state.fullScan {
+ if !folder.job.lib.FullScanInProgress {
// Ancestor folders need a row even with no files of their own: artwork
// resolution climbs them, and an image added later needs a state to diff.
if folder.isEmpty() && folder.isNew() {
@@ -239,7 +250,7 @@ func (p *phaseFolders) processFolder(entry *folderEntry) (*folderEntry, error) {
for afPath, af := range entry.audioFiles {
fullPath := path.Join(entry.path, afPath)
dbTrack, foundInDB := dbTracks[fullPath]
- if !foundInDB || p.state.fullScan {
+ if !foundInDB || entry.job.lib.FullScanInProgress {
filesToImport[fullPath] = dbTrack
} else {
info, err := af.Info()
@@ -289,18 +300,18 @@ func (p *phaseFolders) loadTagsFromFiles(entry *folderEntry, toImport map[string
}
for filePath, info := range allInfo {
md := metadata.New(filePath, info)
- track := md.ToMediaFile(entry.job.lib.ID, entry.id)
+ track := md.ToMediaFile(entry.job.lib, entry.id)
tracks = append(tracks, track)
for _, t := range track.Tags.FlattenAll() {
uniqueTags[t.ID] = t
}
// Keep track of any album ID changes, to reassign annotations later
- prevAlbumID := ""
+ prevAlbumID := track.AlbumID
if prev := toImport[filePath]; prev != nil {
prevAlbumID = prev.AlbumID
- } else {
- prevAlbumID = md.AlbumID(track, p.prevAlbumPIDConf)
+ } else if entry.job.prevAlbumPIDConf != "" {
+ prevAlbumID = md.AlbumID(track, entry.job.prevAlbumPIDConf)
}
_, ok := entry.albumIDMap[track.AlbumID]
if prevAlbumID != track.AlbumID && !ok {
@@ -453,7 +464,7 @@ func (p *phaseFolders) persistFolder(ctx context.Context, tx model.DataStore, en
if len(queueItems) > 0 {
queue := tx.ArtworkQueue()
enqueue := queue.Enqueue
- if p.state.fullScan {
+ if entry.job.lib.FullScanInProgress {
enqueue = queue.EnqueueIfMissing
}
if err := enqueue(ctx, queueItems...); err != nil {
diff --git a/scanner/scanner.go b/scanner/scanner.go
index cd2fe3c8d..305c443f4 100644
--- a/scanner/scanner.go
+++ b/scanner/scanner.go
@@ -4,13 +4,11 @@ import (
"context"
"fmt"
"maps"
- "path/filepath"
"slices"
"sync/atomic"
"time"
ppl "github.com/google/go-pipeline/pkg/pipeline"
- "github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/playlists"
"github.com/navidrome/navidrome/log"
@@ -32,6 +30,7 @@ type scanState struct {
libraries model.Libraries // Store libraries list for consistency across phases
targets map[int][]string // Optional: map[libraryID][]folderPaths for selective scans
totalLibraryCount int // Total number of libraries (unfiltered), for cross-library move detection
+ failedLibs map[int]bool // Libraries that could not be scanned in this run
}
func (s *scanState) sendProgress(info *ProgressInfo) {
@@ -48,29 +47,15 @@ func (s *scanState) sendWarning(msg string) {
s.sendProgress(&ProgressInfo{Warning: msg})
}
-func (s *scanState) sendError(err error) {
- s.sendProgress(&ProgressInfo{Error: err.Error()})
+func (s *scanState) markFailed(libID int) {
+ if s.failedLibs == nil {
+ s.failedLibs = map[int]bool{}
+ }
+ s.failedLibs[libID] = true
}
-// libraryRelativePath rebases an absolute scan target path onto the library root, since the
-// scanner's fs.FS only accepts paths relative to it. Relative paths, and absolute paths outside
-// the library root, are returned unchanged.
-func libraryRelativePath(libPath, folderPath string) string {
- if !filepath.IsAbs(folderPath) {
- return folderPath
- }
- // The library root may be relative (e.g. the default "./music"); it must be made absolute
- // to match against an absolute target, and it resolves against the same cwd as the scanner's fs.
- absLib, err := filepath.Abs(libPath)
- if err != nil {
- return folderPath
- }
- rel, err := filepath.Rel(absLib, folderPath)
- if err != nil || !filepath.IsLocal(rel) {
- return folderPath
- }
- // The scanner's fs.FS is an io/fs, which always uses forward slashes.
- return filepath.ToSlash(rel)
+func (s *scanState) sendError(err error) {
+ s.sendProgress(&ProgressInfo{Error: err.Error()})
}
func (s *scannerImpl) scanFolders(ctx context.Context, fullScan bool, targets []model.ScanTarget, progress chan<- *ProgressInfo) {
@@ -104,7 +89,7 @@ func (s *scannerImpl) scanFolders(ctx context.Context, fullScan bool, targets []
})
for _, target := range targets {
- folderPath := libraryRelativePath(libPaths[target.LibraryID], target.FolderPath)
+ folderPath := model.LibraryRelativePath(libPaths[target.LibraryID], target.FolderPath)
if folderPath == "" {
folderPath = "."
}
@@ -137,6 +122,10 @@ func (s *scannerImpl) scanFolders(ctx context.Context, fullScan bool, targets []
// if there was a full scan in progress, force a full scan
if !state.fullScan {
for _, lib := range state.libraries {
+ // A pending PID rescan already restarts in full through its own job
+ if lib.NeedsPIDRescan() {
+ continue
+ }
if lib.FullScanInProgress {
log.Info(ctx, "Scanner: Interrupted full scan detected", "lib", lib.Name)
state.fullScan = true
@@ -215,10 +204,13 @@ func (s *scannerImpl) prepareLibrariesForScan(ctx context.Context, state *scanSt
var successfulLibs []model.Library
for _, lib := range state.libraries {
- if lib.LastScanStartedAt.IsZero() {
+ // A library with a changed PID config restarts its scan: resuming would skip the folders that
+ // the interrupted scan already processed with the old config
+ pidRescan := lib.NeedsPIDRescan()
+ if lib.LastScanStartedAt.IsZero() || pidRescan {
// This is a new scan - mark it as started
err := s.ds.WithTxRetry(ctx, func(ctx context.Context, tx model.DataStore) error {
- return tx.Library().ScanBegin(ctx, lib.ID, state.fullScan)
+ return tx.Library().ScanBegin(ctx, lib.ID, state.fullScan || pidRescan)
}, "scanner: begin library scan")
if err != nil {
log.Error(ctx, "Scanner: Error marking scan start", "lib", lib.Name, err)
@@ -340,11 +332,12 @@ func (s *scannerImpl) runUpdateLibraries(ctx context.Context, state *scanState)
if err := tx.Library().ScanEnd(ctx, lib.ID); err != nil {
return fmt.Errorf("updating last scan completed for %s: %w", lib.Name, err)
}
- if err := tx.Property().Put(ctx, consts.PIDTrackKey, conf.Server.PID.Track); err != nil {
- return fmt.Errorf("updating track PID conf: %w", err)
- }
- if err := tx.Property().Put(ctx, consts.PIDAlbumKey, conf.Server.PID.Album); err != nil {
- return fmt.Errorf("updating album PID conf: %w", err)
+ // A selective scan covers only part of the library, so the rest may still use the old PID
+ // config. A library that could not be scanned did not apply it either.
+ if !state.isSelectiveScan() && !state.failedLibs[lib.ID] {
+ if err := tx.Library().SetScannedPID(ctx, lib.ID, lib.EffectivePID()); err != nil {
+ return fmt.Errorf("updating PID conf for %s: %w", lib.Name, err)
+ }
}
if state.changesDetected.Load() {
log.Debug(ctx, "Scanner: Refreshing library stats", "lib", lib.Name)
diff --git a/scanner/scanner_internal_test.go b/scanner/scanner_internal_test.go
index 0778bd6ec..e8abb7c7d 100644
--- a/scanner/scanner_internal_test.go
+++ b/scanner/scanner_internal_test.go
@@ -4,8 +4,6 @@ package scanner
import (
"context"
"errors"
- "os"
- "path/filepath"
"sync/atomic"
ppl "github.com/google/go-pipeline/pkg/pipeline"
@@ -13,43 +11,6 @@ import (
. "github.com/onsi/gomega"
)
-var _ = Describe("libraryRelativePath", func() {
- // Paths are built with filepath so the "absolute" cases stay absolute on every OS
- // (a Unix-style "/foo" is not absolute on Windows).
- libRoot, _ := filepath.Abs(filepath.Join("jukebox", "collection"))
- outside, _ := filepath.Abs(filepath.Join("somewhere", "else"))
-
- It("returns a relative path unchanged", func() {
- Expect(libraryRelativePath(libRoot, "_Collection")).To(Equal("_Collection"))
- })
-
- It("rebases an absolute target when the library root is relative", func() {
- cwd, err := os.Getwd()
- Expect(err).ToNot(HaveOccurred())
- Expect(libraryRelativePath(filepath.Join("music", "library"), filepath.Join(cwd, "music", "library", "rock"))).To(Equal("rock"))
- })
-
- It("rebases an absolute path that equals the library root to '.'", func() {
- Expect(libraryRelativePath(libRoot, libRoot)).To(Equal("."))
- })
-
- It("rebases an absolute path under the library root", func() {
- Expect(libraryRelativePath(libRoot, filepath.Join(libRoot, "_Collection"))).To(Equal("_Collection"))
- })
-
- It("handles a trailing slash on the library path", func() {
- Expect(libraryRelativePath(libRoot+string(filepath.Separator), filepath.Join(libRoot, "_Collection"))).To(Equal("_Collection"))
- })
-
- It("leaves an absolute path outside the library root unchanged", func() {
- Expect(libraryRelativePath(libRoot, outside)).To(Equal(outside))
- })
-
- It("returns an empty path unchanged", func() {
- Expect(libraryRelativePath(libRoot, "")).To(Equal(""))
- })
-})
-
type mockPhase struct {
num int
produceFunc func() ppl.Producer[int]
diff --git a/scanner/scanner_multilibrary_test.go b/scanner/scanner_multilibrary_test.go
index 546baf756..f6634c875 100644
--- a/scanner/scanner_multilibrary_test.go
+++ b/scanner/scanner_multilibrary_test.go
@@ -835,4 +835,170 @@ var _ = Describe("Scanner - Multi-Library", Ordered, func() {
Expect(lastError).To(BeEmpty())
})
})
+
+ Context("Per-library PID config", func() {
+ albumsOf := func(libID int) model.Albums {
+ // The mock datastore's GC is a no-op, so run the real one to purge the albums left
+ // empty by a regroup, as the scanner does in production
+ Expect(ds.RealDS.GC(ctx)).To(Succeed())
+ albums, err := ds.Album().GetAll(ctx, model.QueryOptions{
+ Filters: squirrel.Eq{"library_id": libID, "missing": false},
+ Sort: "name",
+ })
+ Expect(err).ToNot(HaveOccurred())
+ return albums
+ }
+ trackByTitle := func(libID int, title string) model.MediaFile {
+ mfs, err := ds.MediaFile().GetAll(ctx, model.QueryOptions{
+ Filters: squirrel.Eq{"library_id": libID, "title": title},
+ })
+ Expect(err).ToNot(HaveOccurred())
+ Expect(mfs).To(HaveLen(1))
+ return mfs[0]
+ }
+ rockTitles := func() []string {
+ mfs, err := ds.MediaFile().GetAll(ctx, model.QueryOptions{Filters: squirrel.Eq{"library_id": lib1.ID}})
+ Expect(err).ToNot(HaveOccurred())
+ return slice.Map(mfs, func(mf model.MediaFile) string { return mf.Title })
+ }
+ // changeRockInDB edits the rock track in the DB only. A full rescan of the rock library would
+ // restore the title from the file tags, a quick scan leaves it alone.
+ changeRockInDB := func() {
+ _, err := db.Db().ExecContext(ctx, "update media_file set title = 'Changed In DB' where library_id = ?", lib1.ID)
+ Expect(err).ToNot(HaveOccurred())
+ }
+ // changeBlueTrainInDB does the same for one jazz track
+ changeBlueTrainInDB := func() {
+ _, err := db.Db().ExecContext(ctx, "update media_file set title = 'Blue Train In DB' where library_id = ? and title = 'Blue Train'", lib2.ID)
+ Expect(err).ToNot(HaveOccurred())
+ }
+
+ BeforeEach(func() {
+ beatles := template(_t{"albumartist": "The Beatles", "album": "Abbey Road", "year": 1969})
+ _ = createFS("rock", fstest.MapFS{
+ "The Beatles/Abbey Road/01 - Come Together.mp3": beatles(track(1, "Come Together")),
+ })
+
+ miles := template(_t{"albumartist": "Miles Davis", "album": "Kind of Blue", "year": 1959})
+ coltrane := template(_t{"albumartist": "John Coltrane", "album": "Giant Steps", "year": 1960})
+ blueTrain := template(_t{"albumartist": "John Coltrane", "album": "Blue Train", "year": 1957})
+ _ = createFS("jazz", fstest.MapFS{
+ "Loose/01 - So What.mp3": miles(track(1, "So What")),
+ "Loose/02 - Giant Steps.mp3": coltrane(track(1, "Giant Steps")),
+ "Coltrane/Blue Train/01 - Blue Train.mp3": blueTrain(track(1, "Blue Train")),
+ })
+ })
+
+ It("regroups only the library whose PID config changed, keeping annotations", func() {
+ Expect(runScanner(ctx, true)).To(Succeed())
+ Expect(albumsOf(lib2.ID)).To(HaveLen(3))
+
+ // Star Blue Train, to check the star follows the album to its new ID
+ oldBlueTrain := trackByTitle(lib2.ID, "Blue Train")
+ Expect(ds.Album().SetStar(ctx, true, oldBlueTrain.AlbumID)).To(Succeed())
+ changeRockInDB()
+
+ lib2.PIDAlbum = "folder"
+ Expect(ds.Library().Put(ctx, &lib2)).To(Succeed())
+ Expect(runScanner(ctx, false)).To(Succeed())
+
+ // Jazz is grouped by folder now: "Loose" is one album
+ Expect(albumsOf(lib2.ID)).To(HaveLen(2))
+ Expect(trackByTitle(lib2.ID, "So What").AlbumID).To(Equal(trackByTitle(lib2.ID, "Giant Steps").AlbumID))
+
+ newBlueTrain := trackByTitle(lib2.ID, "Blue Train")
+ Expect(newBlueTrain.AlbumID).ToNot(Equal(oldBlueTrain.AlbumID))
+ album, err := ds.Album().Get(ctx, newBlueTrain.AlbumID)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(album.Starred).To(BeTrue())
+
+ // Rock only got a quick scan
+ Expect(rockTitles()).To(ConsistOf("Changed In DB"))
+
+ jazz, err := ds.Library().Get(ctx, lib2.ID)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(jazz.ScannedPIDAlbum).To(Equal("folder"))
+ Expect(jazz.PIDChanged()).To(BeFalse())
+ rock, err := ds.Library().Get(ctx, lib1.ID)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(rock.PIDChanged()).To(BeFalse())
+ })
+
+ It("rescans only libraries that follow the global config", func() {
+ lib2.PIDAlbum = "folder"
+ Expect(ds.Library().Put(ctx, &lib2)).To(Succeed())
+ Expect(runScanner(ctx, true)).To(Succeed())
+ changeRockInDB()
+ changeBlueTrainInDB()
+
+ conf.Server.PID.Album = "album"
+ Expect(runScanner(ctx, false)).To(Succeed())
+
+ // Rock follows the global config, so it was rescanned in full and its title restored
+ Expect(rockTitles()).To(ConsistOf("Come Together"))
+ // Jazz has its own override, so it only got a quick scan
+ trackByTitle(lib2.ID, "Blue Train In DB")
+ jazz, err := ds.Library().Get(ctx, lib2.ID)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(jazz.ScannedPIDAlbum).To(Equal("folder"))
+ })
+
+ It("restarts an interrupted scan when the PID config changed meanwhile", func() {
+ Expect(runScanner(ctx, true)).To(Succeed())
+
+ // Simulate a quick scan of jazz that was interrupted after it had processed every folder:
+ // the folders were updated after the (old) scan start time
+ _, err := db.Db().ExecContext(ctx, "update library set last_scan_started_at = ?, full_scan_in_progress = false where id = ?",
+ time.Now().Add(-time.Hour), lib2.ID)
+ Expect(err).ToNot(HaveOccurred())
+
+ lib2.PIDAlbum = "folder"
+ Expect(ds.Library().Put(ctx, &lib2)).To(Succeed())
+ Expect(runScanner(ctx, false)).To(Succeed())
+
+ // Every folder was revisited with the new config
+ Expect(albumsOf(lib2.ID)).To(HaveLen(2))
+ })
+
+ It("does not turn an interrupted PID rescan into a full scan of every library", func() {
+ Expect(runScanner(ctx, true)).To(Succeed())
+ changeRockInDB()
+ lib2.PIDAlbum = "folder"
+ Expect(ds.Library().Put(ctx, &lib2)).To(Succeed())
+
+ // Simulate a PID full scan of jazz that was interrupted
+ Expect(ds.Library().ScanBegin(ctx, lib2.ID, true)).To(Succeed())
+ Expect(runScanner(ctx, false)).To(Succeed())
+
+ // Rock only got a quick scan, jazz was rescanned with the new config
+ Expect(rockTitles()).To(ConsistOf("Changed In DB"))
+ Expect(albumsOf(lib2.ID)).To(HaveLen(2))
+ })
+
+ It("does not record the PID config for a library that could not be scanned", func() {
+ Expect(runScanner(ctx, true)).To(Succeed())
+ broken := model.Library{Name: "Broken", Path: "unregistered:///music", PIDAlbum: "folder"}
+ Expect(ds.Library().Put(ctx, &broken)).To(Succeed())
+
+ // The scan reports an error for the broken library, and still finishes the others
+ _ = runScanner(ctx, false)
+
+ reloaded, err := ds.Library().Get(ctx, broken.ID)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(reloaded.PIDChanged()).To(BeTrue())
+ })
+
+ It("does not record the PID config after a selective scan", func() {
+ Expect(runScanner(ctx, true)).To(Succeed())
+ lib2.PIDAlbum = "folder"
+ Expect(ds.Library().Put(ctx, &lib2)).To(Succeed())
+
+ _, err := s.ScanFolders(ctx, false, []model.ScanTarget{{LibraryID: lib2.ID, FolderPath: "Loose"}})
+ Expect(err).ToNot(HaveOccurred())
+
+ jazz, err := ds.Library().Get(ctx, lib2.ID)
+ Expect(err).ToNot(HaveOccurred())
+ Expect(jazz.PIDChanged()).To(BeTrue())
+ })
+ })
})
diff --git a/server/nativeapi/inspect.go b/server/nativeapi/inspect.go
index f1e6c4539..61013dd5a 100644
--- a/server/nativeapi/inspect.go
+++ b/server/nativeapi/inspect.go
@@ -22,7 +22,12 @@ func doInspect(ctx context.Context, ds model.DataStore, id string) (*core.Inspec
return nil, model.ErrNotFound
}
- return core.Inspect(file.AbsolutePath(), file.LibraryID, file.FolderID)
+ lib, err := ds.Library().Get(ctx, file.LibraryID)
+ if err != nil {
+ return nil, err
+ }
+
+ return core.Inspect(file.AbsolutePath(), *lib, file.FolderID)
}
func inspect(ds model.DataStore) http.HandlerFunc {
diff --git a/server/serve_index.go b/server/serve_index.go
index 4b093b953..167197403 100644
--- a/server/serve_index.go
+++ b/server/serve_index.go
@@ -58,6 +58,8 @@ func serveIndex(ds model.DataStore, fs fs.FS, shareInfo *model.Share) http.Handl
"uiSearchDebounceMs": conf.Server.UISearchDebounceMs,
"uiCoverArtSize": conf.Server.UICoverArtSize,
"enableCoverAnimation": conf.Server.EnableCoverAnimation,
+ "pidAlbum": conf.Server.PID.Album,
+ "pidTrack": conf.Server.PID.Track,
"enableNowPlaying": conf.Server.EnableNowPlaying,
"playbackReportIntervalMs": conf.Server.UIPlaybackReportInterval.Milliseconds(),
"gaTrackingId": conf.Server.GATrackingID,
diff --git a/server/serve_index_test.go b/server/serve_index_test.go
index e2df55c4b..277513768 100644
--- a/server/serve_index_test.go
+++ b/server/serve_index_test.go
@@ -89,6 +89,8 @@ var _ = Describe("serveIndex", func() {
Entry("uiSearchDebounceMs", func() { conf.Server.UISearchDebounceMs = 500 }, "uiSearchDebounceMs", float64(500)),
Entry("uiCoverArtSize", func() { conf.Server.UICoverArtSize = 300 }, "uiCoverArtSize", float64(300)),
Entry("enableCoverAnimation", func() { conf.Server.EnableCoverAnimation = true }, "enableCoverAnimation", true),
+ Entry("pidAlbum", func() { conf.Server.PID.Album = "folder" }, "pidAlbum", "folder"),
+ Entry("pidTrack", func() { conf.Server.PID.Track = "title" }, "pidTrack", "title"),
Entry("enableNowPlaying", func() { conf.Server.EnableNowPlaying = true }, "enableNowPlaying", true),
Entry("gaTrackingId", func() { conf.Server.GATrackingID = "UA-12345" }, "gaTrackingId", "UA-12345"),
Entry("defaultDownloadableShare", func() { conf.Server.DefaultDownloadableShare = true }, "defaultDownloadableShare", true),
diff --git a/tests/mock_library_repo.go b/tests/mock_library_repo.go
index e21dcccce..6de2b9265 100644
--- a/tests/mock_library_repo.go
+++ b/tests/mock_library_repo.go
@@ -145,6 +145,17 @@ func (m *MockLibraryRepo) ScanEnd(_ context.Context, id int) error {
return nil
}
+func (m *MockLibraryRepo) SetScannedPID(_ context.Context, id int, pid model.PIDConfig) error {
+ if m.Err != nil {
+ return m.Err
+ }
+ if lib, ok := m.Data[id]; ok {
+ lib.ScannedPIDAlbum, lib.ScannedPIDTrack = pid.Album, pid.Track
+ m.Data[id] = lib
+ }
+ return nil
+}
+
func (m *MockLibraryRepo) ScanInProgress(_ context.Context) (bool, error) {
if m.Err != nil {
return false, m.Err
diff --git a/ui/src/config.js b/ui/src/config.js
index e406e47cf..62b3cb822 100644
--- a/ui/src/config.js
+++ b/ui/src/config.js
@@ -32,6 +32,8 @@ const defaultConfig = {
listenBrainzEnabled: true,
enableExternalServices: true,
enableCoverAnimation: true,
+ pidAlbum: 'musicbrainz_albumid|albumartistid,album,albumversion,releasedate', // See consts.DefaultAlbumPID
+ pidTrack: 'musicbrainz_trackid|albumid,discnumber,tracknumber,title', // See consts.DefaultTrackPID
enableNowPlaying: true,
playbackReportIntervalMs: 60000,
devShowArtistPage: true,
diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json
index a04b6e311..f694ea75f 100644
--- a/ui/src/i18n/en.json
+++ b/ui/src/i18n/en.json
@@ -331,11 +331,22 @@
"totalDuration": "Duration",
"defaultNewUsers": "Default for New Users",
"createdAt": "Created",
- "updatedAt": "Updated"
+ "updatedAt": "Updated",
+ "pidAlbum": "Album grouping",
+ "pidTrack": "Track identity"
},
"sections": {
"basic": "Basic Information",
- "statistics": "Statistics"
+ "statistics": "Statistics",
+ "pid": "Persistent IDs"
+ },
+ "pid": {
+ "global": "Use global setting (%{value})",
+ "folder": "Folder (one album per folder)",
+ "custom": "Custom",
+ "spec": "PID spec",
+ "help": "Tags and attributes that identify an item. See the documentation for the syntax:",
+ "docs": "Persistent IDs"
},
"actions": {
"scan": "Scan Library",
@@ -365,7 +376,9 @@
"messages": {
"deleteConfirm": "Are you sure you want to delete this library? This will remove all associated data and user access.",
"scanInProgress": "Scan in progress...",
- "noLibrariesAssigned": "No libraries assigned to this user"
+ "noLibrariesAssigned": "No libraries assigned to this user",
+ "pidChangeTitle": "Change persistent IDs?",
+ "pidChangeConfirm": "This regroups albums and tracks in this library. A full rescan of this library starts now. Track stars, ratings and play counts are kept. Album stars and ratings move to the new albums where an old album maps to a new one."
}
},
"plugin": {
diff --git a/ui/src/library/LibraryCreate.jsx b/ui/src/library/LibraryCreate.jsx
index 0e69964b6..8166bb2f3 100644
--- a/ui/src/library/LibraryCreate.jsx
+++ b/ui/src/library/LibraryCreate.jsx
@@ -1,4 +1,5 @@
import React, { useCallback } from 'react'
+import PropTypes from 'prop-types'
import {
Create,
SimpleForm,
@@ -10,7 +11,34 @@ import {
useNotify,
useRedirect,
} from 'react-admin'
+import { Typography } from '@material-ui/core'
+import { makeStyles } from '@material-ui/core/styles'
import { Title } from '../common'
+import { PIDInputs } from './PIDInput'
+
+const useStyles = makeStyles((theme) => ({
+ spaced: { marginTop: theme.spacing(3) },
+}))
+
+// SimpleForm passes form props (variant, record, ...) to its children, so Typography can't be used directly
+const SectionTitle = ({ label, spaced }) => {
+ const translate = useTranslate()
+ const classes = useStyles()
+ return (
+
+ {translate(label)}
+
+ )
+}
+
+SectionTitle.propTypes = {
+ label: PropTypes.string.isRequired,
+ spaced: PropTypes.bool,
+}
const LibraryCreate = (props) => {
const translate = useTranslate()
@@ -73,9 +101,12 @@ const LibraryCreate = (props) => {
return (
} {...props}>
+
+
+
)
diff --git a/ui/src/library/LibraryEdit.jsx b/ui/src/library/LibraryEdit.jsx
index 53d17ac7f..c42c7ac4b 100644
--- a/ui/src/library/LibraryEdit.jsx
+++ b/ui/src/library/LibraryEdit.jsx
@@ -1,9 +1,11 @@
-import React, { useCallback } from 'react'
+import React, { useCallback, useState } from 'react'
+import PropTypes from 'prop-types'
import {
Edit,
FormWithRedirect,
TextInput,
BooleanInput,
+ Confirm,
required,
SaveButton,
useTranslate,
@@ -22,6 +24,9 @@ import {
ReadOnlySizeField,
Title,
} from '../common'
+import config from '../config'
+import { PIDInputs } from './PIDInput'
+import { pidConfigChanged } from './pidPresets'
const useStyles = makeStyles({
toolbar: {
@@ -53,8 +58,131 @@ const CustomToolbar = ({ showDelete, ...props }) => (
)
-const LibraryEdit = (props) => {
+export const LibraryEditForm = ({ formProps, canEditPath, canDelete }) => {
const translate = useTranslate()
+ const [confirmOpen, setConfirmOpen] = useState(false)
+
+ // Every submit path (Save button and Enter key) goes through here, so a PID change always asks first
+ const submit = () => {
+ if (
+ pidConfigChanged(
+ formProps.form.getState().values,
+ formProps.record,
+ config,
+ )
+ ) {
+ setConfirmOpen(true)
+ return
+ }
+ formProps.handleSubmit()
+ }
+
+ const handleConfirm = () => {
+ setConfirmOpen(false)
+ formProps.handleSubmit()
+ }
+
+ return (
+
+ )
+}
+
+LibraryEditForm.propTypes = {
+ formProps: PropTypes.object.isRequired,
+ canEditPath: PropTypes.bool,
+ canDelete: PropTypes.bool,
+}
+
+const LibraryEdit = (props) => {
const [mutate] = useMutation()
const notify = useNotify()
const redirect = useRedirect()
@@ -93,91 +221,11 @@ const LibraryEdit = (props) => {
{...props}
save={save}
render={(formProps) => (
-
+
)}
/>
diff --git a/ui/src/library/LibraryEdit.test.jsx b/ui/src/library/LibraryEdit.test.jsx
new file mode 100644
index 000000000..926adc839
--- /dev/null
+++ b/ui/src/library/LibraryEdit.test.jsx
@@ -0,0 +1,125 @@
+import * as React from 'react'
+import { TestContext } from 'ra-test'
+import {
+ FormWithRedirect,
+ RecordContextProvider,
+ SaveContextProvider,
+} from 'react-admin'
+import {
+ cleanup,
+ fireEvent,
+ render,
+ screen,
+ waitFor,
+ within,
+} from '@testing-library/react'
+import { describe, it, expect, vi, afterEach } from 'vitest'
+import { LibraryEditForm } from './LibraryEdit'
+import config from '../config'
+
+const record = {
+ id: '2',
+ name: 'Jazz',
+ path: '/music/jazz',
+ pidAlbum: '',
+ pidTrack: '',
+}
+
+// Edit provides a save context in the app. SaveButton only reads these setters from it
+const saveContext = {
+ save: vi.fn(),
+ setOnSuccess: vi.fn(),
+ setOnFailure: vi.fn(),
+ setTransform: vi.fn(),
+}
+
+const renderForm = (save) =>
+ render(
+
+
+
+ (
+
+ )}
+ />
+
+
+ ,
+ )
+
+const chooseAlbumGrouping = (optionText) => {
+ fireEvent.mouseDown(
+ screen.getByLabelText('resources.library.fields.pidAlbum'),
+ )
+ fireEvent.click(within(screen.getByRole('listbox')).getByText(optionText))
+}
+
+const dialogTitle = 'resources.library.messages.pidChangeTitle'
+
+describe('LibraryEditForm', () => {
+ afterEach(cleanup)
+
+ it('saves directly when the PID config did not change', async () => {
+ const save = vi.fn()
+ renderForm(save)
+ fireEvent.change(screen.getByLabelText(/resources.library.fields.name/), {
+ target: { value: 'Jazz Renamed' },
+ })
+ fireEvent.click(screen.getByText('ra.action.save'))
+ await waitFor(() => expect(save).toHaveBeenCalled())
+ expect(screen.queryByText(dialogTitle)).not.toBeInTheDocument()
+ })
+
+ it('asks before saving a PID change, and Cancel keeps the edits', async () => {
+ const save = vi.fn()
+ renderForm(save)
+ chooseAlbumGrouping('resources.library.pid.folder')
+ fireEvent.click(screen.getByText('ra.action.save'))
+
+ expect(await screen.findByText(dialogTitle)).toBeInTheDocument()
+ expect(save).not.toHaveBeenCalled()
+
+ fireEvent.click(screen.getByText('ra.action.cancel'))
+ await waitFor(() =>
+ expect(screen.queryByText(dialogTitle)).not.toBeInTheDocument(),
+ )
+ expect(save).not.toHaveBeenCalled()
+ expect(screen.getByText('resources.library.pid.folder')).toBeInTheDocument()
+ })
+
+ it('saves the PID change after Confirm', async () => {
+ const save = vi.fn()
+ renderForm(save)
+ chooseAlbumGrouping('resources.library.pid.folder')
+ fireEvent.click(screen.getByText('ra.action.save'))
+ fireEvent.click(await screen.findByText('ra.action.confirm'))
+
+ await waitFor(() => expect(save).toHaveBeenCalled())
+ expect(save.mock.calls[0][0]).toMatchObject({ pidAlbum: 'folder' })
+ })
+
+ it('pre-fills a Custom spec with the global spec', () => {
+ renderForm(vi.fn())
+ chooseAlbumGrouping('resources.library.pid.custom')
+ expect(screen.getByLabelText(/resources.library.pid.spec/)).toHaveValue(
+ config.pidAlbum,
+ )
+ })
+
+ it('asks before saving when the form is submitted with Enter', async () => {
+ const save = vi.fn()
+ const { container } = renderForm(save)
+ chooseAlbumGrouping('resources.library.pid.folder')
+ fireEvent.submit(container.querySelector('form'))
+
+ expect(await screen.findByText(dialogTitle)).toBeInTheDocument()
+ expect(save).not.toHaveBeenCalled()
+ })
+})
diff --git a/ui/src/library/PIDInput.jsx b/ui/src/library/PIDInput.jsx
new file mode 100644
index 000000000..6481dc392
--- /dev/null
+++ b/ui/src/library/PIDInput.jsx
@@ -0,0 +1,114 @@
+import React, { useState } from 'react'
+import PropTypes from 'prop-types'
+import { TextInput, required, useTranslate } from 'react-admin'
+import { useField } from 'react-final-form'
+import { FormHelperText, Link, MenuItem, TextField } from '@material-ui/core'
+import { makeStyles } from '@material-ui/core/styles'
+import {
+ PID_CUSTOM,
+ PID_FOLDER,
+ PID_GLOBAL,
+ pidModeFromValue,
+ pidValueForMode,
+} from './pidPresets'
+import config from '../config'
+import { docsUrl } from '../utils'
+
+const PID_DOCS_URL = docsUrl('/docs/usage/pids/')
+
+const useStyles = makeStyles((theme) => ({
+ help: { marginBottom: theme.spacing(1) },
+}))
+
+// PIDInput edits a library PID override: use the global setting, a preset, or a custom spec
+export const PIDInput = ({ source, label, globalValue, allowFolder }) => {
+ const translate = useTranslate()
+ const classes = useStyles()
+ const { input } = useField(source)
+ // Local state, so choosing Custom shows the text box before anything is typed
+ const [mode, setMode] = useState(() =>
+ pidModeFromValue(input.value, allowFolder),
+ )
+
+ const choices = [
+ {
+ id: PID_GLOBAL,
+ name: translate('resources.library.pid.global', { value: globalValue }),
+ },
+ ...(allowFolder
+ ? [{ id: PID_FOLDER, name: translate('resources.library.pid.folder') }]
+ : []),
+ { id: PID_CUSTOM, name: translate('resources.library.pid.custom') },
+ ]
+
+ const handleModeChange = (event) => {
+ const newMode = event.target.value
+ setMode(newMode)
+ input.onChange(pidValueForMode(newMode, globalValue))
+ }
+
+ return (
+ <>
+
+ {choices.map((choice) => (
+
+ ))}
+
+ {mode === PID_CUSTOM && (
+ <>
+
+
+ {translate('resources.library.pid.help')}{' '}
+
+ {translate('resources.library.pid.docs')}
+
+
+
+ )}
+
+ )
+}
+
+PIDInput.propTypes = {
+ source: PropTypes.string.isRequired,
+ label: PropTypes.string.isRequired,
+ globalValue: PropTypes.string,
+ allowFolder: PropTypes.bool,
+}
+
+export const PIDInputs = () => {
+ const translate = useTranslate()
+ return (
+ <>
+
+
+
+ )
+}
diff --git a/ui/src/library/pidPresets.js b/ui/src/library/pidPresets.js
new file mode 100644
index 000000000..0483fc691
--- /dev/null
+++ b/ui/src/library/pidPresets.js
@@ -0,0 +1,33 @@
+export const PID_GLOBAL = 'global'
+export const PID_FOLDER = 'folder'
+export const PID_CUSTOM = 'custom'
+
+export const pidModeFromValue = (value, allowFolder) => {
+ const v = (value || '').trim()
+ if (v === '') return PID_GLOBAL
+ if (allowFolder && v === PID_FOLDER) return PID_FOLDER
+ return PID_CUSTOM
+}
+
+// Returns the value to store for a dropdown choice. Custom starts from the global spec
+export const pidValueForMode = (mode, globalValue) => {
+ switch (mode) {
+ case PID_GLOBAL:
+ return ''
+ case PID_FOLDER:
+ return PID_FOLDER
+ default:
+ return globalValue || ''
+ }
+}
+
+// Reports whether the form values change the effective PID spec of the saved record. Like the
+// server, it trims, treats empty as the global value and compares case-insensitively
+export const pidConfigChanged = (values, record, globals) => {
+ const effective = (value, field) =>
+ ((value || '').trim() || globals[field] || '').toLowerCase()
+ return ['pidAlbum', 'pidTrack'].some(
+ (field) =>
+ effective(values[field], field) !== effective(record[field], field),
+ )
+}
diff --git a/ui/src/library/pidPresets.test.js b/ui/src/library/pidPresets.test.js
new file mode 100644
index 000000000..dab1c82e2
--- /dev/null
+++ b/ui/src/library/pidPresets.test.js
@@ -0,0 +1,65 @@
+import { describe, it, expect } from 'vitest'
+import {
+ PID_CUSTOM,
+ PID_FOLDER,
+ PID_GLOBAL,
+ pidConfigChanged,
+ pidModeFromValue,
+ pidValueForMode,
+} from './pidPresets'
+
+describe('pidModeFromValue', () => {
+ it('maps an empty value to the global setting', () => {
+ expect(pidModeFromValue('', true)).toBe(PID_GLOBAL)
+ expect(pidModeFromValue(undefined, true)).toBe(PID_GLOBAL)
+ })
+ it('maps folder to the Folder preset when allowed', () => {
+ expect(pidModeFromValue('folder', true)).toBe(PID_FOLDER)
+ })
+ it('maps folder to Custom when the Folder preset is not offered', () => {
+ expect(pidModeFromValue('folder', false)).toBe(PID_CUSTOM)
+ })
+ it('maps any other value to Custom', () => {
+ expect(pidModeFromValue('album|title', true)).toBe(PID_CUSTOM)
+ })
+})
+
+describe('pidValueForMode', () => {
+ it('stores an empty value for the global setting', () => {
+ expect(pidValueForMode(PID_GLOBAL, 'album')).toBe('')
+ })
+ it('stores folder for the Folder preset', () => {
+ expect(pidValueForMode(PID_FOLDER, '')).toBe('folder')
+ })
+ it('starts Custom from the global spec', () => {
+ expect(pidValueForMode(PID_CUSTOM, 'album|title')).toBe('album|title')
+ expect(pidValueForMode(PID_CUSTOM, undefined)).toBe('')
+ })
+})
+
+describe('pidConfigChanged', () => {
+ const record = { pidAlbum: 'folder', pidTrack: '' }
+ const globals = {
+ pidAlbum: 'musicbrainz_albumid|albumartistid,album',
+ pidTrack: 'musicbrainz_trackid|albumid,discnumber,tracknumber,title',
+ }
+ it.each([
+ ['nothing changed', { pidAlbum: 'folder', pidTrack: '' }, false],
+ ['a missing value equals an empty one', { pidAlbum: 'folder' }, false],
+ [
+ 'Custom set to the global value',
+ { pidAlbum: 'folder', pidTrack: globals.pidTrack },
+ false,
+ ],
+ ['a case-only change', { pidAlbum: 'FOLDER', pidTrack: '' }, false],
+ [
+ 'a whitespace-only change',
+ { pidAlbum: ' folder ', pidTrack: ' ' },
+ false,
+ ],
+ ['the album PID changed', { pidAlbum: '', pidTrack: '' }, true],
+ ['the track PID changed', { pidAlbum: 'folder', pidTrack: 'title' }, true],
+ ])('%s', (_, values, expected) => {
+ expect(pidConfigChanged(values, record, globals)).toBe(expected)
+ })
+})