From 0954928b14f74f84654da5da62dfea256f3e6490 Mon Sep 17 00:00:00 2001 From: Deluan Date: Mon, 24 Feb 2025 20:39:26 -0500 Subject: [PATCH] fix(server): import playlists with absolute paths Signed-off-by: Deluan --- core/playlists.go | 58 +++++++++--- core/playlists_test.go | 92 +++++++++++--------- tests/fixtures/playlists/subfolder2/pls2.m3u | 6 +- 3 files changed, 101 insertions(+), 55 deletions(-) diff --git a/core/playlists.go b/core/playlists.go index 2aa538b69..a29dfa247 100644 --- a/core/playlists.go +++ b/core/playlists.go @@ -188,20 +188,14 @@ func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, folder *m if !model.IsAudioFile(line) { continue } - line = filepath.Clean(line) - if folder != nil && !filepath.IsAbs(line) { - line = filepath.Join(folder.AbsolutePath(), line) - var err error - line, err = filepath.Rel(folder.LibraryPath, line) - if err != nil { - log.Trace(ctx, "Error getting relative path", "playlist", pls.Name, "path", line, "folder", folder, err) - continue - } - } filteredLines = append(filteredLines, line) } - filteredLines = slice.Map(filteredLines, filepath.ToSlash) - found, err := mediaFileRepository.FindByPaths(filteredLines) + paths, err := s.normalizePaths(ctx, pls, folder, filteredLines) + if err != nil { + log.Warn(ctx, "Error normalizing paths in playlist", "playlist", pls.Name, err) + continue + } + found, err := mediaFileRepository.FindByPaths(paths) if err != nil { log.Warn(ctx, "Error reading files from DB", "playlist", pls.Name, err) continue @@ -210,7 +204,7 @@ func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, folder *m for idx := range found { existing[strings.ToLower(found[idx].Path)] = idx } - for _, path := range filteredLines { + for _, path := range paths { idx, ok := existing[strings.ToLower(path)] if ok { mfs = append(mfs, found[idx]) @@ -228,6 +222,44 @@ func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, folder *m return nil } +// TODO This won't work for multiple libraries +func (s *playlists) normalizePaths(ctx context.Context, pls *model.Playlist, folder *model.Folder, lines []string) ([]string, error) { + libs, err := s.ds.Library(ctx).GetAll() + if err != nil { + return nil, err + } + // Normalize paths to be relative to the library root. Search for roots in any library + var res []string + for idx, line := range lines { + var libPath string + var filePath string + if folder != nil && !filepath.IsAbs(line) { + libPath = folder.LibraryPath + filePath = filepath.Join(folder.AbsolutePath(), line) + } else { + for _, lib := range libs { + if strings.HasPrefix(line, lib.Path) { + libPath = lib.Path + filePath = line + break + } + } + } + if libPath != "" { + var err error + filePath, err = filepath.Rel(libPath, filePath) + if err != nil { + log.Trace(ctx, "Error getting relative path", "playlist", pls.Name, "path", line, "folder", folder, err) + continue + } + res = append(res, filePath) + } else { + log.Warn(ctx, "Path in playlist not found in any library", "path", line, "line", idx) + } + } + return slice.Map(res, filepath.ToSlash), nil +} + func (s *playlists) updatePlaylist(ctx context.Context, newPls *model.Playlist) error { owner, _ := request.UserFrom(ctx) diff --git a/core/playlists_test.go b/core/playlists_test.go index 7f39523a8..7241dd1b5 100644 --- a/core/playlists_test.go +++ b/core/playlists_test.go @@ -20,15 +20,20 @@ import ( var _ = Describe("Playlists", func() { var ds *tests.MockDataStore var ps Playlists - var mp mockedPlaylist + var mockPlsRepo mockedPlaylistRepo + var mockLibRepo *tests.MockLibraryRepo ctx := context.Background() BeforeEach(func() { - mp = mockedPlaylist{} + mockPlsRepo = mockedPlaylistRepo{} + mockLibRepo = &tests.MockLibraryRepo{} ds = &tests.MockDataStore{ - MockedPlaylist: &mp, + MockedPlaylist: &mockPlsRepo, + MockedLibrary: mockLibRepo, } ctx = request.WithUser(ctx, model.User{ID: "123"}) + // Path should be libPath, but we want to match the root folder referenced in the m3u, which is `/` + mockLibRepo.SetData([]model.Library{{ID: 1, Path: "/"}}) }) Describe("ImportFile", func() { @@ -48,15 +53,14 @@ var _ = Describe("Playlists", func() { Describe("M3U", func() { It("parses well-formed playlists", func() { - // get absolute path for "tests/fixtures" folder pls, err := ps.ImportFile(ctx, folder, "pls1.m3u") Expect(err).ToNot(HaveOccurred()) Expect(pls.OwnerID).To(Equal("123")) Expect(pls.Tracks).To(HaveLen(3)) Expect(pls.Tracks[0].Path).To(Equal("tests/fixtures/playlists/test.mp3")) Expect(pls.Tracks[1].Path).To(Equal("tests/fixtures/playlists/test.ogg")) - Expect(pls.Tracks[2].Path).To(Equal("/tests/fixtures/01 Invisible (RED) Edit Version.mp3")) - Expect(mp.last).To(Equal(pls)) + Expect(pls.Tracks[2].Path).To(Equal("tests/fixtures/01 Invisible (RED) Edit Version.mp3")) + Expect(mockPlsRepo.last).To(Equal(pls)) }) It("parses playlists using LF ending", func() { @@ -76,7 +80,7 @@ var _ = Describe("Playlists", func() { It("parses well-formed playlists", func() { pls, err := ps.ImportFile(ctx, folder, "recently_played.nsp") Expect(err).ToNot(HaveOccurred()) - Expect(mp.last).To(Equal(pls)) + Expect(mockPlsRepo.last).To(Equal(pls)) Expect(pls.OwnerID).To(Equal("123")) Expect(pls.Name).To(Equal("Recently Played")) Expect(pls.Comment).To(Equal("Recently played tracks")) @@ -98,79 +102,87 @@ var _ = Describe("Playlists", func() { repo = &mockedMediaFileFromListRepo{} ds.MockedMediaFile = repo ps = NewPlaylists(ds) + mockLibRepo.SetData([]model.Library{{ID: 1, Path: "/music"}}) ctx = request.WithUser(ctx, model.User{ID: "123"}) }) It("parses well-formed playlists", func() { repo.data = []string{ - "tests/fixtures/test.mp3", - "tests/fixtures/test.ogg", - "/tests/fixtures/01 Invisible (RED) Edit Version.mp3", + "tests/test.mp3", + "tests/test.ogg", + "tests/01 Invisible (RED) Edit Version.mp3", } - f, _ := os.Open("tests/fixtures/playlists/pls-with-name.m3u") - defer f.Close() + m3u := strings.Join([]string{ + "#PLAYLIST:playlist 1", + "/music/tests/test.mp3", + "/music/tests/test.ogg", + "file:///music/tests/01%20Invisible%20(RED)%20Edit%20Version.mp3", + }, "\n") + f := strings.NewReader(m3u) + pls, err := ps.ImportM3U(ctx, f) Expect(err).ToNot(HaveOccurred()) Expect(pls.OwnerID).To(Equal("123")) Expect(pls.Name).To(Equal("playlist 1")) Expect(pls.Sync).To(BeFalse()) Expect(pls.Tracks).To(HaveLen(3)) - Expect(pls.Tracks[0].Path).To(Equal("tests/fixtures/test.mp3")) - Expect(pls.Tracks[1].Path).To(Equal("tests/fixtures/test.ogg")) - Expect(pls.Tracks[2].Path).To(Equal("/tests/fixtures/01 Invisible (RED) Edit Version.mp3")) - Expect(mp.last).To(Equal(pls)) - f.Close() - + Expect(pls.Tracks[0].Path).To(Equal("tests/test.mp3")) + Expect(pls.Tracks[1].Path).To(Equal("tests/test.ogg")) + Expect(pls.Tracks[2].Path).To(Equal("tests/01 Invisible (RED) Edit Version.mp3")) + Expect(mockPlsRepo.last).To(Equal(pls)) }) It("sets the playlist name as a timestamp if the #PLAYLIST directive is not present", func() { repo.data = []string{ - "tests/fixtures/test.mp3", - "tests/fixtures/test.ogg", - "/tests/fixtures/01 Invisible (RED) Edit Version.mp3", + "tests/test.mp3", + "tests/test.ogg", + "/tests/01 Invisible (RED) Edit Version.mp3", } - f, _ := os.Open("tests/fixtures/playlists/pls-without-name.m3u") - defer f.Close() + m3u := strings.Join([]string{ + "/music/tests/test.mp3", + "/music/tests/test.ogg", + }, "\n") + f := strings.NewReader(m3u) pls, err := ps.ImportM3U(ctx, f) Expect(err).ToNot(HaveOccurred()) _, err = time.Parse(time.RFC3339, pls.Name) Expect(err).ToNot(HaveOccurred()) - Expect(pls.Tracks).To(HaveLen(3)) + Expect(pls.Tracks).To(HaveLen(2)) }) It("returns only tracks that exist in the database and in the same other as the m3u", func() { repo.data = []string{ - "test1.mp3", - "test2.mp3", - "test3.mp3", + "album1/test1.mp3", + "album2/test2.mp3", + "album3/test3.mp3", } m3u := strings.Join([]string{ - "test3.mp3", - "test1.mp3", - "test4.mp3", - "test2.mp3", + "/music/album3/test3.mp3", + "/music/album1/test1.mp3", + "/music/album4/test4.mp3", + "/music/album2/test2.mp3", }, "\n") f := strings.NewReader(m3u) pls, err := ps.ImportM3U(ctx, f) Expect(err).ToNot(HaveOccurred()) Expect(pls.Tracks).To(HaveLen(3)) - Expect(pls.Tracks[0].Path).To(Equal("test3.mp3")) - Expect(pls.Tracks[1].Path).To(Equal("test1.mp3")) - Expect(pls.Tracks[2].Path).To(Equal("test2.mp3")) + Expect(pls.Tracks[0].Path).To(Equal("album3/test3.mp3")) + Expect(pls.Tracks[1].Path).To(Equal("album1/test1.mp3")) + Expect(pls.Tracks[2].Path).To(Equal("album2/test2.mp3")) }) It("is case-insensitive when comparing paths", func() { repo.data = []string{ - "tEsT1.Mp3", + "abc/tEsT1.Mp3", } m3u := strings.Join([]string{ - "TeSt1.mP3", + "/music/ABC/TeSt1.mP3", }, "\n") f := strings.NewReader(m3u) pls, err := ps.ImportM3U(ctx, f) Expect(err).ToNot(HaveOccurred()) Expect(pls.Tracks).To(HaveLen(1)) - Expect(pls.Tracks[0].Path).To(Equal("tEsT1.Mp3")) + Expect(pls.Tracks[0].Path).To(Equal("abc/tEsT1.Mp3")) }) }) @@ -254,16 +266,16 @@ func (r *mockedMediaFileFromListRepo) FindByPaths([]string) (model.MediaFiles, e return mfs, nil } -type mockedPlaylist struct { +type mockedPlaylistRepo struct { last *model.Playlist model.PlaylistRepository } -func (r *mockedPlaylist) FindByPath(string) (*model.Playlist, error) { +func (r *mockedPlaylistRepo) FindByPath(string) (*model.Playlist, error) { return nil, model.ErrNotFound } -func (r *mockedPlaylist) Put(pls *model.Playlist) error { +func (r *mockedPlaylistRepo) Put(pls *model.Playlist) error { r.last = pls return nil } diff --git a/tests/fixtures/playlists/subfolder2/pls2.m3u b/tests/fixtures/playlists/subfolder2/pls2.m3u index af745ba59..cfe699471 100644 --- a/tests/fixtures/playlists/subfolder2/pls2.m3u +++ b/tests/fixtures/playlists/subfolder2/pls2.m3u @@ -1,2 +1,4 @@ -test.mp3 -test.ogg +../test.mp3 +../test.ogg +/tests/fixtures/01%20Invisible%20(RED)%20Edit%20Version.mp3 +/invalid/path/xyz.mp3 \ No newline at end of file