From bc4c72eb42805acdd1f3622c8e7e9dddd48d5209 Mon Sep 17 00:00:00 2001 From: junkerderprovinz Date: Sun, 9 Aug 2026 16:52:07 +0200 Subject: [PATCH 1/3] fix(model): make FolderID, folder names and ToM3U8 paths OS-independent (#5381) Signed-off-by: junkerderprovinz --- model/folder.go | 16 +++++++++++----- model/folder_test.go | 3 --- model/mediafile.go | 8 +++++--- model/mediafile_test.go | 5 ----- model/playlist_test.go | 2 -- 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/model/folder.go b/model/folder.go index 701fca8cd..8ca52ee4e 100644 --- a/model/folder.go +++ b/model/folder.go @@ -43,15 +43,21 @@ func (f Folder) String() string { // FolderID generates a unique ID for a folder in a library. // The ID is generated based on the library ID and the folder path relative to the library root. // Any leading or trailing slashes are removed from the folder path. -func FolderID(lib Library, path string) string { - path = strings.TrimPrefix(path, lib.Path) - path = strings.TrimPrefix(path, string(os.PathSeparator)) - path = filepath.Clean(path) - key := fmt.Sprintf("%d:%s", lib.ID, path) +func FolderID(lib Library, folderPath string) string { + folderPath = strings.TrimPrefix(folderPath, lib.Path) + folderPath = strings.TrimPrefix(folderPath, string(os.PathSeparator)) + // Folder IDs must be stable across operating systems, so normalize to + // forward slashes (path.Clean, not filepath.Clean) before hashing; + // otherwise a nested path on Windows would hash with back-slashes. + folderPath = path.Clean(filepath.ToSlash(folderPath)) + key := fmt.Sprintf("%d:%s", lib.ID, folderPath) return id.NewHash(key) } func NewFolder(lib Library, folderPath string) *Folder { + // path.Split/path.Clean below only treat "/" as a separator, so normalize + // OS separators first to derive dir/name correctly on Windows. + folderPath = filepath.ToSlash(folderPath) newID := FolderID(lib, folderPath) dir, name := path.Split(folderPath) dir = path.Clean(dir) diff --git a/model/folder_test.go b/model/folder_test.go index 4c1b4c2b7..0535f6987 100644 --- a/model/folder_test.go +++ b/model/folder_test.go @@ -7,7 +7,6 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/id" - "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -67,7 +66,6 @@ var _ = Describe("Folder", func() { When("the folder has multiple subdirs", func() { It("should return the correct folder ID", func() { - tests.SkipOnWindows("path separator bug (#TBD-path-sep-model)") folderPath := filepath.FromSlash("/music/rock/metal") expectedID := id.NewHash("1:rock/metal") Expect(model.FolderID(lib, folderPath)).To(Equal(expectedID)) @@ -77,7 +75,6 @@ var _ = Describe("Folder", func() { Describe("NewFolder", func() { It("should create a new SubFolder with the correct attributes", func() { - tests.SkipOnWindows("path separator bug (#TBD-path-sep-model)") folderPath := filepath.FromSlash("rock/metal") folder := model.NewFolder(lib, folderPath) diff --git a/model/mediafile.go b/model/mediafile.go index 0c56b4825..6fa82df9a 100644 --- a/model/mediafile.go +++ b/model/mediafile.go @@ -527,11 +527,13 @@ func (mfs MediaFiles) ToM3U8(title string, absolutePaths bool) string { buf.WriteString(fmt.Sprintf("#PLAYLIST:%s\n", title)) for _, t := range mfs { buf.WriteString(fmt.Sprintf("#EXTINF:%.f,%s - %s\n", t.Duration, t.Artist, t.Title)) + p := t.Path if absolutePaths { - buf.WriteString(t.AbsolutePath() + "\n") - } else { - buf.WriteString(t.Path + "\n") + p = t.AbsolutePath() } + // Emit forward slashes so the exported .m3u8 is consistent across OSes + // (AbsolutePath returns native separators on Windows). + buf.WriteString(filepath.ToSlash(p) + "\n") } return buf.String() } diff --git a/model/mediafile_test.go b/model/mediafile_test.go index 9ca3489bb..6ba0ce3be 100644 --- a/model/mediafile_test.go +++ b/model/mediafile_test.go @@ -6,7 +6,6 @@ import ( "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" ) @@ -477,9 +476,6 @@ var _ = Describe("MediaFiles", func() { DescribeTable("generates correct output", func(absolutePaths bool, expectedContent string) { - if absolutePaths { - tests.SkipOnWindows("path separator bug (#TBD-path-sep-model)") - } result := mfs.ToM3U8("Multi Track", absolutePaths) Expect(result).To(Equal(expectedContent)) }, @@ -500,7 +496,6 @@ var _ = Describe("MediaFiles", func() { Context("path variations", func() { It("handles different path structures", func() { - tests.SkipOnWindows("path separator bug (#TBD-path-sep-model)") mfs = MediaFiles{ {Title: "Root", Artist: "Artist", Duration: 60, Path: "song.mp3", LibraryPath: "/lib"}, {Title: "Nested", Artist: "Artist", Duration: 60, Path: "deep/nested/song.mp3", LibraryPath: "/lib"}, diff --git a/model/playlist_test.go b/model/playlist_test.go index 7ffb69b63..12b051a8e 100644 --- a/model/playlist_test.go +++ b/model/playlist_test.go @@ -8,7 +8,6 @@ import ( "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/criteria" - "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -34,7 +33,6 @@ var _ = Describe("Playlist", func() { } }) It("generates the correct M3U format", func() { - tests.SkipOnWindows("path separator bug (#TBD-path-sep-model)") expected := `#EXTM3U #PLAYLIST:Mellow sunset #EXTINF:378,Morcheeba feat. Kurt Wagner - What New York Couples Fight About From 5d71b6e0d52723decfd8b0a57ef406c46e17cf9f Mon Sep 17 00:00:00 2001 From: junkerderprovinz Date: Fri, 9 Oct 2026 20:49:40 +0200 Subject: [PATCH 2/3] test(model): fix path-separator specs in the tests instead of production Folder IDs are persisted, so the Windows IDs stay as they are and the spec pins them. NewFolder gets the forward-slash paths the scanner passes, and the M3U expectations use the OS separator. Signed-off-by: junkerderprovinz --- model/folder.go | 16 +++++----------- model/folder_test.go | 7 +++++-- model/mediafile.go | 8 +++----- model/mediafile_test.go | 8 +++++--- model/playlist_test.go | 2 +- 5 files changed, 19 insertions(+), 22 deletions(-) diff --git a/model/folder.go b/model/folder.go index 8ca52ee4e..701fca8cd 100644 --- a/model/folder.go +++ b/model/folder.go @@ -43,21 +43,15 @@ func (f Folder) String() string { // FolderID generates a unique ID for a folder in a library. // The ID is generated based on the library ID and the folder path relative to the library root. // Any leading or trailing slashes are removed from the folder path. -func FolderID(lib Library, folderPath string) string { - folderPath = strings.TrimPrefix(folderPath, lib.Path) - folderPath = strings.TrimPrefix(folderPath, string(os.PathSeparator)) - // Folder IDs must be stable across operating systems, so normalize to - // forward slashes (path.Clean, not filepath.Clean) before hashing; - // otherwise a nested path on Windows would hash with back-slashes. - folderPath = path.Clean(filepath.ToSlash(folderPath)) - key := fmt.Sprintf("%d:%s", lib.ID, folderPath) +func FolderID(lib Library, path string) string { + path = strings.TrimPrefix(path, lib.Path) + path = strings.TrimPrefix(path, string(os.PathSeparator)) + path = filepath.Clean(path) + key := fmt.Sprintf("%d:%s", lib.ID, path) return id.NewHash(key) } func NewFolder(lib Library, folderPath string) *Folder { - // path.Split/path.Clean below only treat "/" as a separator, so normalize - // OS separators first to derive dir/name correctly on Windows. - folderPath = filepath.ToSlash(folderPath) newID := FolderID(lib, folderPath) dir, name := path.Split(folderPath) dir = path.Clean(dir) diff --git a/model/folder_test.go b/model/folder_test.go index 0535f6987..30a810ff3 100644 --- a/model/folder_test.go +++ b/model/folder_test.go @@ -67,7 +67,9 @@ var _ = Describe("Folder", func() { When("the folder has multiple subdirs", func() { It("should return the correct folder ID", func() { folderPath := filepath.FromSlash("/music/rock/metal") - expectedID := id.NewHash("1:rock/metal") + // IDs are persisted and hashed with the OS separator, so changing + // this on Windows would need a migration. + expectedID := id.NewHash("1:" + filepath.FromSlash("rock/metal")) Expect(model.FolderID(lib, folderPath)).To(Equal(expectedID)) }) }) @@ -75,7 +77,8 @@ var _ = Describe("Folder", func() { Describe("NewFolder", func() { It("should create a new SubFolder with the correct attributes", func() { - folderPath := filepath.FromSlash("rock/metal") + // The scanner walks an fs.FS, so folder paths always use forward slashes + folderPath := "rock/metal" folder := model.NewFolder(lib, folderPath) Expect(folder.LibraryID).To(Equal(lib.ID)) diff --git a/model/mediafile.go b/model/mediafile.go index 6fa82df9a..0c56b4825 100644 --- a/model/mediafile.go +++ b/model/mediafile.go @@ -527,13 +527,11 @@ func (mfs MediaFiles) ToM3U8(title string, absolutePaths bool) string { buf.WriteString(fmt.Sprintf("#PLAYLIST:%s\n", title)) for _, t := range mfs { buf.WriteString(fmt.Sprintf("#EXTINF:%.f,%s - %s\n", t.Duration, t.Artist, t.Title)) - p := t.Path if absolutePaths { - p = t.AbsolutePath() + buf.WriteString(t.AbsolutePath() + "\n") + } else { + buf.WriteString(t.Path + "\n") } - // Emit forward slashes so the exported .m3u8 is consistent across OSes - // (AbsolutePath returns native separators on Windows). - buf.WriteString(filepath.ToSlash(p) + "\n") } return buf.String() } diff --git a/model/mediafile_test.go b/model/mediafile_test.go index 6ba0ce3be..392b940b8 100644 --- a/model/mediafile_test.go +++ b/model/mediafile_test.go @@ -1,6 +1,7 @@ package model_test import ( + "path/filepath" "time" "github.com/navidrome/navidrome/conf" @@ -483,9 +484,10 @@ var _ = Describe("MediaFiles", func() { false, "#EXTM3U\n#PLAYLIST:Multi Track\n#EXTINF:120,Artist A - Song One\na/song1.mp3\n#EXTINF:241,Artist B - Song Two\nb/song2.mp3\n#EXTINF:90,Artist with Ümläuts - Song with \"quotes\" & ampersands\nspecial/file.mp3\n", ), + // AbsolutePath joins with the OS separator Entry("absolute paths", true, - "#EXTM3U\n#PLAYLIST:Multi Track\n#EXTINF:120,Artist A - Song One\n/music/a/song1.mp3\n#EXTINF:241,Artist B - Song Two\n/music/b/song2.mp3\n#EXTINF:90,Artist with Ümläuts - Song with \"quotes\" & ampersands\n/música/special/file.mp3\n", + filepath.FromSlash("#EXTM3U\n#PLAYLIST:Multi Track\n#EXTINF:120,Artist A - Song One\n/music/a/song1.mp3\n#EXTINF:241,Artist B - Song Two\n/music/b/song2.mp3\n#EXTINF:90,Artist with Ümläuts - Song with \"quotes\" & ampersands\n/música/special/file.mp3\n"), ), Entry("special characters", false, @@ -506,8 +508,8 @@ var _ = Describe("MediaFiles", func() { Expect(relativeResult).To(ContainSubstring("deep/nested/song.mp3\n")) absoluteResult := mfs.ToM3U8("Test", true) - Expect(absoluteResult).To(ContainSubstring("/lib/song.mp3\n")) - Expect(absoluteResult).To(ContainSubstring("/lib/deep/nested/song.mp3\n")) + Expect(absoluteResult).To(ContainSubstring(filepath.FromSlash("/lib/song.mp3\n"))) + Expect(absoluteResult).To(ContainSubstring(filepath.FromSlash("/lib/deep/nested/song.mp3\n"))) }) }) }) diff --git a/model/playlist_test.go b/model/playlist_test.go index 12b051a8e..124d2e503 100644 --- a/model/playlist_test.go +++ b/model/playlist_test.go @@ -44,7 +44,7 @@ var _ = Describe("Playlist", func() { #EXTINF:164,Legião Urbana - On the Way Home /music/library/Legião Urbana/Música p_ acampamentos/02-05 On the Way Home.mp3 ` - Expect(pls.ToM3U8()).To(Equal(expected)) + Expect(pls.ToM3U8()).To(Equal(filepath.FromSlash(expected))) }) }) From 1d15beb2eb6a7202f3b6fcf58a3549e0f986703b Mon Sep 17 00:00:00 2001 From: junkerderprovinz Date: Fri, 9 Oct 2026 20:53:47 +0200 Subject: [PATCH 3/3] test(model): clarify where NewFolder paths come from Signed-off-by: junkerderprovinz --- model/folder_test.go | 2 +- model/mediafile_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/model/folder_test.go b/model/folder_test.go index 30a810ff3..4f38973bb 100644 --- a/model/folder_test.go +++ b/model/folder_test.go @@ -77,7 +77,7 @@ var _ = Describe("Folder", func() { Describe("NewFolder", func() { It("should create a new SubFolder with the correct attributes", func() { - // The scanner walks an fs.FS, so folder paths always use forward slashes + // The scanner passes fs.FS paths here, which use forward slashes on every OS. folderPath := "rock/metal" folder := model.NewFolder(lib, folderPath) diff --git a/model/mediafile_test.go b/model/mediafile_test.go index 392b940b8..fc35e9fde 100644 --- a/model/mediafile_test.go +++ b/model/mediafile_test.go @@ -484,7 +484,7 @@ var _ = Describe("MediaFiles", func() { false, "#EXTM3U\n#PLAYLIST:Multi Track\n#EXTINF:120,Artist A - Song One\na/song1.mp3\n#EXTINF:241,Artist B - Song Two\nb/song2.mp3\n#EXTINF:90,Artist with Ümläuts - Song with \"quotes\" & ampersands\nspecial/file.mp3\n", ), - // AbsolutePath joins with the OS separator + // AbsolutePath joins with the OS separator. Entry("absolute paths", true, filepath.FromSlash("#EXTM3U\n#PLAYLIST:Multi Track\n#EXTINF:120,Artist A - Song One\n/music/a/song1.mp3\n#EXTINF:241,Artist B - Song Two\n/music/b/song2.mp3\n#EXTINF:90,Artist with Ümläuts - Song with \"quotes\" & ampersands\n/música/special/file.mp3\n"),