diff --git a/core/playlists.go b/core/playlists.go index 16a6d5e26..bc870bd24 100644 --- a/core/playlists.go +++ b/core/playlists.go @@ -54,9 +54,9 @@ func (s *playlists) ImportM3U(ctx context.Context, reader io.Reader) (*model.Pla pls := &model.Playlist{ OwnerID: owner.ID, Public: false, - Sync: true, + Sync: false, } - pls, err := s.parseM3U(ctx, pls, "", reader) + err := s.parseM3U(ctx, pls, "", reader) if err != nil { log.Error(ctx, "Error parsing playlist", err) return nil, err @@ -84,10 +84,11 @@ func (s *playlists) parsePlaylist(ctx context.Context, playlistFile string, base extension := strings.ToLower(filepath.Ext(playlistFile)) switch extension { case ".nsp": - return s.parseNSP(ctx, pls, file) + err = s.parseNSP(ctx, pls, file) default: - return s.parseM3U(ctx, pls, baseDir, file) + err = s.parseM3U(ctx, pls, baseDir, file) } + return pls, err } func (s *playlists) newSyncedPlaylist(baseDir string, playlistFile string) (*model.Playlist, error) { @@ -111,14 +112,14 @@ func (s *playlists) newSyncedPlaylist(baseDir string, playlistFile string) (*mod return pls, nil } -func (s *playlists) parseNSP(ctx context.Context, pls *model.Playlist, file io.Reader) (*model.Playlist, error) { +func (s *playlists) parseNSP(ctx context.Context, pls *model.Playlist, file io.Reader) error { nsp := &nspFile{} reader := jsoncommentstrip.NewReader(file) dec := json.NewDecoder(reader) err := dec.Decode(nsp) if err != nil { log.Error(ctx, "Error parsing SmartPlaylist", "playlist", pls.Name, err) - return nil, err + return err } pls.Rules = &nsp.Criteria if nsp.Name != "" { @@ -127,20 +128,18 @@ func (s *playlists) parseNSP(ctx context.Context, pls *model.Playlist, file io.R if nsp.Comment != "" { pls.Comment = nsp.Comment } - return pls, nil + return nil } -func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, baseDir string, reader io.Reader) (*model.Playlist, error) { +func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, baseDir string, reader io.Reader) error { mediaFileRepository := s.ds.MediaFile(ctx) var mfs model.MediaFiles for lines := range slice.CollectChunks(slice.LinesFrom(reader), 400) { - var filteredLines []string + filteredLines := make([]string, 0, len(lines)) for _, line := range lines { line := strings.TrimSpace(line) if strings.HasPrefix(line, "#PLAYLIST:") { - if split := strings.Split(line, ":"); len(split) >= 2 { - pls.Name = split[1] - } + pls.Name = line[len("#PLAYLIST:"):] continue } // Skip empty lines and extended info @@ -161,10 +160,18 @@ func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, baseDir s log.Warn(ctx, "Error reading files from DB", "playlist", pls.Name, err) continue } - if len(found) != len(filteredLines) { - logMissingFiles(ctx, pls, filteredLines, found) + existing := make(map[string]int, len(found)) + for idx := range found { + existing[found[idx].Path] = idx + } + for _, path := range filteredLines { + idx, ok := existing[path] + if ok { + mfs = append(mfs, found[idx]) + } else { + log.Warn(ctx, "Path in playlist not found", "playlist", pls.Name, "path", path) + } } - mfs = append(mfs, found...) } if pls.Name == "" { pls.Name = time.Now().Format(time.RFC3339) @@ -172,20 +179,7 @@ func (s *playlists) parseM3U(ctx context.Context, pls *model.Playlist, baseDir s pls.Tracks = nil pls.AddMediaFiles(mfs) - return pls, nil -} - -func logMissingFiles(ctx context.Context, pls *model.Playlist, lines []string, found model.MediaFiles) { - missing := make(map[string]bool) - for _, line := range lines { - missing[line] = true - } - for _, mf := range found { - delete(missing, mf.Path) - } - for path := range missing { - log.Warn(ctx, "Path in playlist not found", "playlist", pls.Name, "path", path) - } + return nil } func (s *playlists) updatePlaylist(ctx context.Context, newPls *model.Playlist) error { diff --git a/core/playlists_test.go b/core/playlists_test.go index 54f391cee..ca1ddbfe6 100644 --- a/core/playlists_test.go +++ b/core/playlists_test.go @@ -3,19 +3,20 @@ package core import ( "context" "os" + "strconv" + "strings" "time" + "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/criteria" "github.com/navidrome/navidrome/model/request" - - "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) var _ = Describe("Playlists", func() { - var ds model.DataStore + var ds *tests.MockDataStore var ps Playlists var mp mockedPlaylist ctx := context.Background() @@ -23,8 +24,7 @@ var _ = Describe("Playlists", func() { BeforeEach(func() { mp = mockedPlaylist{} ds = &tests.MockDataStore{ - MockedMediaFile: &mockedMediaFile{}, - MockedPlaylist: &mp, + MockedPlaylist: &mp, } ctx = request.WithUser(ctx, model.User{ID: "123"}) }) @@ -32,12 +32,13 @@ var _ = Describe("Playlists", func() { Describe("ImportFile", func() { BeforeEach(func() { ps = NewPlaylists(ds) + ds.MockedMediaFile = &mockedMediaFileRepo{} }) Describe("M3U", func() { It("parses well-formed playlists", func() { pls, err := ps.ImportFile(ctx, "tests/fixtures", "playlists/pls1.m3u") - Expect(err).To(BeNil()) + 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/test.mp3")) @@ -48,13 +49,13 @@ var _ = Describe("Playlists", func() { It("parses playlists using LF ending", func() { pls, err := ps.ImportFile(ctx, "tests/fixtures/playlists", "lf-ended.m3u") - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(pls.Tracks).To(HaveLen(2)) }) It("parses playlists using CR ending (old Mac format)", func() { pls, err := ps.ImportFile(ctx, "tests/fixtures/playlists", "cr-ended.m3u") - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(pls.Tracks).To(HaveLen(2)) }) }) @@ -62,7 +63,7 @@ var _ = Describe("Playlists", func() { Describe("NSP", func() { It("parses well-formed playlists", func() { pls, err := ps.ImportFile(ctx, "tests/fixtures", "playlists/recently_played.nsp") - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) Expect(mp.last).To(Equal(pls)) Expect(pls.OwnerID).To(Equal("123")) Expect(pls.Name).To(Equal("Recently Played")) @@ -76,18 +77,28 @@ var _ = Describe("Playlists", func() { }) Describe("ImportM3U", func() { + var repo *mockedMediaFileFromListRepo BeforeEach(func() { + repo = &mockedMediaFileFromListRepo{} + ds.MockedMediaFile = repo ps = NewPlaylists(ds) ctx = request.WithUser(ctx, model.User{ID: "123"}) }) It("parses well-formed playlists", func() { - f, _ := os.Open("tests/fixtures/playlists/pls-post-with-name.m3u") + repo.data = []string{ + "tests/fixtures/test.mp3", + "tests/fixtures/test.ogg", + "/tests/fixtures/01 Invisible (RED) Edit Version.mp3", + } + f, _ := os.Open("tests/fixtures/playlists/pls-with-name.m3u") defer f.Close() pls, err := ps.ImportM3U(ctx, f) + Expect(err).ToNot(HaveOccurred()) Expect(pls.OwnerID).To(Equal("123")) Expect(pls.Name).To(Equal("playlist 1")) - Expect(err).To(BeNil()) + 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")) @@ -97,32 +108,72 @@ var _ = Describe("Playlists", func() { }) It("sets the playlist name as a timestamp if the #PLAYLIST directive is not present", func() { - f, _ := os.Open("tests/fixtures/playlists/pls-post.m3u") + repo.data = []string{ + "tests/fixtures/test.mp3", + "tests/fixtures/test.ogg", + "/tests/fixtures/01 Invisible (RED) Edit Version.mp3", + } + f, _ := os.Open("tests/fixtures/playlists/pls-without-name.m3u") defer f.Close() pls, err := ps.ImportM3U(ctx, f) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) _, err = time.Parse(time.RFC3339, pls.Name) - Expect(err).To(BeNil()) + Expect(err).ToNot(HaveOccurred()) + Expect(pls.Tracks).To(HaveLen(3)) + }) + + 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", + } + m3u := strings.Join([]string{ + "test3.mp3", + "test1.mp3", + "test4.mp3", + "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")) }) }) }) -type mockedMediaFile struct { +// mockedMediaFileRepo's FindByPaths method returns a list of MediaFiles with the same paths as the input +type mockedMediaFileRepo struct { model.MediaFileRepository } -func (r *mockedMediaFile) FindByPath(s string) (*model.MediaFile, error) { - return &model.MediaFile{ - ID: "123", - Path: s, - }, nil +func (r *mockedMediaFileRepo) FindByPaths(paths []string) (model.MediaFiles, error) { + var mfs model.MediaFiles + for idx, path := range paths { + mfs = append(mfs, model.MediaFile{ + ID: strconv.Itoa(idx), + Path: path, + }) + } + return mfs, nil } -func (r *mockedMediaFile) FindByPaths(paths []string) (model.MediaFiles, error) { +// mockedMediaFileFromListRepo's FindByPaths method returns a list of MediaFiles based on the data field +type mockedMediaFileFromListRepo struct { + model.MediaFileRepository + data []string +} + +func (r *mockedMediaFileFromListRepo) FindByPaths([]string) (model.MediaFiles, error) { var mfs model.MediaFiles - for _, path := range paths { - mf, _ := r.FindByPath(path) - mfs = append(mfs, *mf) + for idx, path := range r.data { + mfs = append(mfs, model.MediaFile{ + ID: strconv.Itoa(idx), + Path: path, + }) } return mfs, nil } diff --git a/tests/fixtures/playlists/pls-post-with-name.m3u b/tests/fixtures/playlists/pls-with-name.m3u similarity index 100% rename from tests/fixtures/playlists/pls-post-with-name.m3u rename to tests/fixtures/playlists/pls-with-name.m3u diff --git a/tests/fixtures/playlists/pls-post.m3u b/tests/fixtures/playlists/pls-without-name.m3u similarity index 100% rename from tests/fixtures/playlists/pls-post.m3u rename to tests/fixtures/playlists/pls-without-name.m3u