From f9fc3cd1c6255d4d6fe95248980e5b0e5ec70a78 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 25 Jun 2025 16:54:23 -0400 Subject: [PATCH] prepend libraryID for track and album PIDs Signed-off-by: Deluan --- model/library.go | 5 + model/metadata/legacy_ids.go | 13 ++- model/metadata/map_mediafile.go | 7 +- model/metadata/persistent_ids.go | 34 +++--- model/metadata/persistent_ids_test.go | 148 +++++++++++++++++++++++--- persistence/library_repository.go | 8 +- scanner/phase_1_folders.go | 2 + 7 files changed, 178 insertions(+), 39 deletions(-) diff --git a/model/library.go b/model/library.go index fd0247a6e..b7524d4bc 100644 --- a/model/library.go +++ b/model/library.go @@ -24,6 +24,11 @@ type Library struct { TotalDuration float64 `json:"totalDuration" db:"total_duration"` } +const ( + DefaultLibraryID = 1 + DefaultLibraryName = "Music Library" +) + type Libraries []Library type LibraryRepository interface { diff --git a/model/metadata/legacy_ids.go b/model/metadata/legacy_ids.go index 25025ea19..0a3bf0bf3 100644 --- a/model/metadata/legacy_ids.go +++ b/model/metadata/legacy_ids.go @@ -14,11 +14,15 @@ import ( // These are the legacy ID functions that were used in the original Navidrome ID generation. // They are kept here for backwards compatibility with existing databases. -func legacyTrackID(mf model.MediaFile) string { - return fmt.Sprintf("%x", md5.Sum([]byte(mf.Path))) +func legacyTrackID(mf model.MediaFile, prependLibId bool) string { + id := mf.Path + if prependLibId && mf.LibraryID != model.DefaultLibraryID { + id = fmt.Sprintf("%d\\%s", mf.LibraryID, id) + } + return fmt.Sprintf("%x", md5.Sum([]byte(id))) } -func legacyAlbumID(md Metadata) string { +func legacyAlbumID(mf model.MediaFile, md Metadata, prependLibId bool) string { releaseDate := legacyReleaseDate(md) albumPath := strings.ToLower(fmt.Sprintf("%s\\%s", legacyMapAlbumArtistName(md), legacyMapAlbumName(md))) if !conf.Server.Scanner.GroupAlbumReleases { @@ -26,6 +30,9 @@ func legacyAlbumID(md Metadata) string { albumPath = fmt.Sprintf("%s\\%s", albumPath, releaseDate) } } + if prependLibId && mf.LibraryID != model.DefaultLibraryID { + albumPath = fmt.Sprintf("%d\\%s", mf.LibraryID, albumPath) + } return fmt.Sprintf("%x", md5.Sum([]byte(albumPath))) } diff --git a/model/metadata/map_mediafile.go b/model/metadata/map_mediafile.go index 591b618a3..c64e8c724 100644 --- a/model/metadata/map_mediafile.go +++ b/model/metadata/map_mediafile.go @@ -7,9 +7,9 @@ import ( "math" "strconv" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/model/id" "github.com/navidrome/navidrome/utils/str" ) @@ -77,7 +77,7 @@ func (md Metadata) ToMediaFile(libID int, folderID string) model.MediaFile { // Persistent IDs mf.PID = md.trackPID(mf) - mf.AlbumID = md.albumID(mf) + mf.AlbumID = md.albumID(mf, conf.Server.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 @@ -104,8 +104,7 @@ func (md Metadata) ToMediaFile(libID int, folderID string) model.MediaFile { } func (md Metadata) AlbumID(mf model.MediaFile, pidConf string) string { - getPID := createGetPID(id.NewHash) - return getPID(mf, md, pidConf) + return md.albumID(mf, pidConf) } func (md Metadata) mapGain(rg, r128 model.TagName) *float64 { diff --git a/model/metadata/persistent_ids.go b/model/metadata/persistent_ids.go index 95e93c2fa..b45882946 100644 --- a/model/metadata/persistent_ids.go +++ b/model/metadata/persistent_ids.go @@ -2,6 +2,7 @@ package metadata import ( "cmp" + "fmt" "path/filepath" "strings" @@ -21,13 +22,15 @@ type hashFunc = func(...string) string // Attributes can be either tags or some processed values like folder, albumid, albumartistid, etc. // For each field, it gets all its attributes values and concatenates them, then hashes the result. // If a field is empty, it is skipped and the function looks for the next field. -func createGetPID(hash hashFunc) func(mf model.MediaFile, md Metadata, spec string) string { - var getPID func(mf model.MediaFile, md Metadata, spec string) string - getAttr := func(mf model.MediaFile, md Metadata, attr string) string { +type getPIDFunc = func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string + +func createGetPID(hash hashFunc) getPIDFunc { + var getPID getPIDFunc + getAttr := func(mf model.MediaFile, md Metadata, attr string, prependLibId bool) string { attr = strings.TrimSpace(strings.ToLower(attr)) switch attr { case "albumid": - return getPID(mf, md, conf.Server.PID.Album) + return getPID(mf, md, conf.Server.PID.Album, prependLibId) case "folder": return filepath.Dir(mf.Path) case "albumartistid": @@ -39,14 +42,14 @@ func createGetPID(hash hashFunc) func(mf model.MediaFile, md Metadata, spec stri } return md.String(model.TagName(attr)) } - getPID = func(mf model.MediaFile, md Metadata, spec string) string { + getPID = func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string { pid := "" fields := strings.Split(spec, "|") for _, field := range fields { attributes := strings.Split(field, ",") hasValue := false values := slice.Map(attributes, func(attr string) string { - v := getAttr(mf, md, attr) + v := getAttr(mf, md, attr, prependLibId) if v != "" { hasValue = true } @@ -57,32 +60,35 @@ func createGetPID(hash hashFunc) func(mf model.MediaFile, md Metadata, spec stri break } } + if prependLibId { + pid = fmt.Sprintf("%d\\%s", mf.LibraryID, pid) + } return hash(pid) } - return func(mf model.MediaFile, md Metadata, spec string) string { + return func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string { switch spec { case "track_legacy": - return legacyTrackID(mf) + return legacyTrackID(mf, prependLibId) case "album_legacy": - return legacyAlbumID(md) + return legacyAlbumID(mf, md, prependLibId) } - return getPID(mf, md, spec) + return getPID(mf, md, spec, prependLibId) } } func (md Metadata) trackPID(mf model.MediaFile) string { - return createGetPID(id.NewHash)(mf, md, conf.Server.PID.Track) + return createGetPID(id.NewHash)(mf, md, conf.Server.PID.Track, true) } -func (md Metadata) albumID(mf model.MediaFile) string { - return createGetPID(id.NewHash)(mf, md, conf.Server.PID.Album) +func (md Metadata) albumID(mf model.MediaFile, pidConf string) string { + return createGetPID(id.NewHash)(mf, md, pidConf, true) } // BFR Must be configurable? func (md Metadata) artistID(name string) string { mf := model.MediaFile{AlbumArtist: name} - return createGetPID(id.NewHash)(mf, md, "albumartistid") + return createGetPID(id.NewHash)(mf, md, "albumartistid", false) } func (md Metadata) mapTrackTitle() string { diff --git a/model/metadata/persistent_ids_test.go b/model/metadata/persistent_ids_test.go index d07b36331..7ae0c91f7 100644 --- a/model/metadata/persistent_ids_test.go +++ b/model/metadata/persistent_ids_test.go @@ -15,7 +15,7 @@ var _ = Describe("getPID", func() { md Metadata mf model.MediaFile sum hashFunc - getPID func(mf model.MediaFile, md Metadata, spec string) string + getPID getPIDFunc ) BeforeEach(func() { @@ -28,7 +28,7 @@ var _ = Describe("getPID", func() { When("no attributes were present", func() { It("should return empty pid", func() { md.tags = map[model.TagName][]string{} - pid := getPID(mf, md, spec) + pid := getPID(mf, md, spec, false) Expect(pid).To(Equal("()")) }) }) @@ -40,7 +40,7 @@ var _ = Describe("getPID", func() { "discnumber": {"1"}, "tracknumber": {"1"}, } - Expect(getPID(mf, md, spec)).To(Equal("(mbtrackid)")) + Expect(getPID(mf, md, spec, false)).To(Equal("(mbtrackid)")) }) }) When("only first field is present", func() { @@ -48,7 +48,7 @@ var _ = Describe("getPID", func() { md.tags = map[model.TagName][]string{ "musicbrainz_trackid": {"mbtrackid"}, } - Expect(getPID(mf, md, spec)).To(Equal("(mbtrackid)")) + Expect(getPID(mf, md, spec, false)).To(Equal("(mbtrackid)")) }) }) When("first is empty, but second field is present", func() { @@ -57,7 +57,7 @@ var _ = Describe("getPID", func() { "album": {"album name"}, "discnumber": {"1"}, } - Expect(getPID(mf, md, spec)).To(Equal("(album name\\1\\)")) + Expect(getPID(mf, md, spec, false)).To(Equal("(album name\\1\\)")) }) }) }) @@ -73,7 +73,7 @@ var _ = Describe("getPID", func() { md.tags = map[model.TagName][]string{"title": {"title"}} md.filePath = "/path/to/file.mp3" mf.Title = "Title" - Expect(getPID(mf, md, spec)).To(Equal("(Title)")) + Expect(getPID(mf, md, spec, false)).To(Equal("(Title)")) }) }) When("field is folder", func() { @@ -81,7 +81,7 @@ var _ = Describe("getPID", func() { spec := "folder|title" md.tags = map[model.TagName][]string{"title": {"title"}} mf.Path = "/path/to/file.mp3" - Expect(getPID(mf, md, spec)).To(Equal("(/path/to)")) + Expect(getPID(mf, md, spec, false)).To(Equal("(/path/to)")) }) }) When("field is albumid", func() { @@ -94,7 +94,7 @@ var _ = Describe("getPID", func() { "releasedate": {"2021-01-01"}, } mf.AlbumArtist = "Album Artist" - Expect(getPID(mf, md, spec)).To(Equal("(((album artist)\\album name\\version\\2021-01-01))")) + Expect(getPID(mf, md, spec, false)).To(Equal("(((album artist)\\album name\\version\\2021-01-01))")) }) }) When("field is albumartistid", func() { @@ -104,14 +104,14 @@ var _ = Describe("getPID", func() { "albumartist": {"Album Artist"}, } mf.AlbumArtist = "Album Artist" - Expect(getPID(mf, md, spec)).To(Equal("((album artist))")) + Expect(getPID(mf, md, spec, false)).To(Equal("((album artist))")) }) }) When("field is album", func() { It("should return the pid", func() { spec := "album|title" md.tags = map[model.TagName][]string{"album": {"Album Name"}} - Expect(getPID(mf, md, spec)).To(Equal("(album name)")) + Expect(getPID(mf, md, spec, false)).To(Equal("(album name)")) }) }) }) @@ -123,7 +123,7 @@ var _ = Describe("getPID", func() { md.tags = map[model.TagName][]string{ "album": {"album name"}, } - Expect(getPID(mf, md, spec)).To(Equal("(album name)")) + Expect(getPID(mf, md, spec, false)).To(Equal("(album name)")) }) }) When("the spec has spaces", func() { @@ -133,7 +133,7 @@ var _ = Describe("getPID", func() { "albumartist": {"Album Artist"}, "album": {"album name"}, } - Expect(getPID(mf, md, spec)).To(Equal("(Album Artist\\album name)")) + Expect(getPID(mf, md, spec, false)).To(Equal("(Album Artist\\album name)")) }) }) When("the spec has mixed case fields", func() { @@ -143,7 +143,129 @@ var _ = Describe("getPID", func() { "albumartist": {"Album Artist"}, "album": {"album name"}, } - Expect(getPID(mf, md, spec)).To(Equal("(Album Artist\\album name)")) + Expect(getPID(mf, md, spec, false)).To(Equal("(Album Artist\\album name)")) + }) + }) + }) + + Context("prependLibId functionality", func() { + BeforeEach(func() { + mf.LibraryID = 42 + }) + When("prependLibId is true", func() { + It("should prepend library ID to the hash input", func() { + spec := "album" + md.tags = map[model.TagName][]string{"album": {"Test Album"}} + pid := getPID(mf, md, spec, true) + // The hash function should receive "42\test album" as input + Expect(pid).To(Equal("(42\\test album)")) + }) + }) + When("prependLibId is false", func() { + It("should not prepend library ID to the hash input", func() { + spec := "album" + md.tags = map[model.TagName][]string{"album": {"Test Album"}} + pid := getPID(mf, md, spec, false) + // The hash function should receive "test album" as input + Expect(pid).To(Equal("(test album)")) + }) + }) + When("prependLibId is true with complex spec", func() { + It("should prepend library ID to the final hash input", func() { + spec := "musicbrainz_trackid|album,tracknumber" + md.tags = map[model.TagName][]string{ + "album": {"Test Album"}, + "tracknumber": {"1"}, + } + pid := getPID(mf, md, spec, true) + // Should use the fallback field and prepend library ID + Expect(pid).To(Equal("(42\\test album\\1)")) + }) + }) + When("prependLibId is true with nested albumid", func() { + It("should handle nested albumid calls correctly", func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.PID.Album = "album" + spec := "albumid" + md.tags = map[model.TagName][]string{"album": {"Test Album"}} + mf.AlbumArtist = "Test Artist" + pid := getPID(mf, md, spec, true) + // The albumid call should also use prependLibId=true + Expect(pid).To(Equal("(42\\(42\\test album))")) + }) + }) + }) + + Context("legacy specs", func() { + Context("track_legacy", func() { + When("library ID is default (1)", func() { + It("should not prepend library ID even when prependLibId is true", func() { + mf.Path = "/path/to/track.mp3" + mf.LibraryID = 1 // Default library ID + // With default library, both should be the same + pidTrue := getPID(mf, md, "track_legacy", true) + pidFalse := getPID(mf, md, "track_legacy", false) + Expect(pidTrue).To(Equal(pidFalse)) + Expect(pidTrue).NotTo(BeEmpty()) + }) + }) + When("library ID is non-default", func() { + It("should prepend library ID when prependLibId is true", func() { + mf.Path = "/path/to/track.mp3" + mf.LibraryID = 2 // Non-default library ID + pidTrue := getPID(mf, md, "track_legacy", true) + pidFalse := getPID(mf, md, "track_legacy", false) + Expect(pidTrue).NotTo(Equal(pidFalse)) + Expect(pidTrue).NotTo(BeEmpty()) + Expect(pidFalse).NotTo(BeEmpty()) + }) + }) + When("library ID is non-default but prependLibId is false", func() { + It("should not prepend library ID", func() { + mf.Path = "/path/to/track.mp3" + mf.LibraryID = 3 + mf2 := mf + mf2.LibraryID = 1 // Default library + pidNonDefault := getPID(mf, md, "track_legacy", false) + pidDefault := getPID(mf2, md, "track_legacy", false) + // Should be the same since prependLibId=false + Expect(pidNonDefault).To(Equal(pidDefault)) + }) + }) + }) + Context("album_legacy", func() { + When("library ID is default (1)", func() { + It("should not prepend library ID even when prependLibId is true", func() { + md.tags = map[model.TagName][]string{"album": {"Test Album"}} + mf.LibraryID = 1 // Default library ID + pidTrue := getPID(mf, md, "album_legacy", true) + pidFalse := getPID(mf, md, "album_legacy", false) + Expect(pidTrue).To(Equal(pidFalse)) + Expect(pidTrue).NotTo(BeEmpty()) + }) + }) + When("library ID is non-default", func() { + It("should prepend library ID when prependLibId is true", func() { + md.tags = map[model.TagName][]string{"album": {"Test Album"}} + mf.LibraryID = 2 // Non-default library ID + pidTrue := getPID(mf, md, "album_legacy", true) + pidFalse := getPID(mf, md, "album_legacy", false) + Expect(pidTrue).NotTo(Equal(pidFalse)) + Expect(pidTrue).NotTo(BeEmpty()) + Expect(pidFalse).NotTo(BeEmpty()) + }) + }) + When("library ID is non-default but prependLibId is false", func() { + It("should not prepend library ID", func() { + md.tags = map[model.TagName][]string{"album": {"Test Album"}} + mf.LibraryID = 3 + mf2 := mf + mf2.LibraryID = 1 // Default library + pidNonDefault := getPID(mf, md, "album_legacy", false) + pidDefault := getPID(mf2, md, "album_legacy", false) + // Should be the same since prependLibId=false + Expect(pidNonDefault).To(Equal(pidDefault)) + }) }) }) }) diff --git a/persistence/library_repository.go b/persistence/library_repository.go index bfe3fd75d..12ce5ce19 100644 --- a/persistence/library_repository.go +++ b/persistence/library_repository.go @@ -71,7 +71,7 @@ func (r *libraryRepository) GetPath(id int) (string, error) { } func (r *libraryRepository) Put(l *model.Library) error { - if l.ID == 1 { + if l.ID == model.DefaultLibraryID { currentLib, err := r.Get(1) // if we are creating it, it's ok. if err == nil { // it exists, so we are updating it @@ -131,19 +131,17 @@ ON CONFLICT (user_id, library_id) DO NOTHING;`, return nil } -const hardCodedMusicFolderID = 1 - // TODO Remove this method when we have a proper UI to add libraries // This is a temporary method to store the music folder path from the config in the DB func (r *libraryRepository) StoreMusicFolder() error { sq := Update(r.tableName).Set("path", conf.Server.MusicFolder). Set("updated_at", time.Now()). - Where(Eq{"id": hardCodedMusicFolderID}) + Where(Eq{"id": model.DefaultLibraryID}) _, err := r.executeSQL(sq) if err != nil { libLock.Lock() defer libLock.Unlock() - libCache[hardCodedMusicFolderID] = conf.Server.MusicFolder + libCache[model.DefaultLibraryID] = conf.Server.MusicFolder } return err } diff --git a/scanner/phase_1_folders.go b/scanner/phase_1_folders.go index 233e78b15..3b97ed38c 100644 --- a/scanner/phase_1_folders.go +++ b/scanner/phase_1_folders.go @@ -424,12 +424,14 @@ func (p *phaseFolders) persistAlbum(repo model.AlbumRepository, a *model.Album, if prevID == "" { return nil } + // Reassign annotation from previous album to new album log.Trace(p.ctx, "Reassigning album annotations", "from", prevID, "to", a.ID, "album", a.Name) if err := repo.ReassignAnnotation(prevID, a.ID); err != nil { log.Warn(p.ctx, "Scanner: Could not reassign annotations", "from", prevID, "to", a.ID, "album", a.Name, err) p.state.sendWarning(fmt.Sprintf("Could not reassign annotations from %s to %s ('%s'): %v", prevID, a.ID, a.Name, err)) } + // Keep created_at field from previous instance of the album if err := repo.CopyAttributes(prevID, a.ID, "created_at"); err != nil { // Silently ignore when the previous album is not found