From c222efacd23c73d9427725bd9906ff1f05492985 Mon Sep 17 00:00:00 2001 From: David Date: Sat, 14 Mar 2026 13:32:28 -0500 Subject: [PATCH 1/2] feat: Add support for referencing playlists using paths Signed-off-by: David --- model/criteria/criteria.go | 12 ++++++++ model/criteria/criteria_test.go | 22 ++++++++++---- model/criteria/operators.go | 48 ++++++++++++++++++++---------- model/criteria/operators_test.go | 4 ++- persistence/playlist_repository.go | 9 ++++++ 5 files changed, 72 insertions(+), 23 deletions(-) diff --git a/model/criteria/criteria.go b/model/criteria/criteria.go index 278acf34c..5919cd781 100644 --- a/model/criteria/criteria.go +++ b/model/criteria/criteria.go @@ -164,6 +164,18 @@ func (c Criteria) ChildPlaylistIds() []string { return nil } +func (c Criteria) ChildPlaylistPaths() []string { + if c.Expression == nil { + return nil + } + + if parent := c.Expression.(interface{ ChildPlaylistPaths() (paths []string) }); parent != nil { + return parent.ChildPlaylistPaths() + } + + return nil +} + func (c Criteria) MarshalJSON() ([]byte, error) { aux := struct { All []Expression `json:"all,omitempty"` diff --git a/model/criteria/criteria_test.go b/model/criteria/criteria_test.go index a76b3fc1f..c13f89110 100644 --- a/model/criteria/criteria_test.go +++ b/model/criteria/criteria_test.go @@ -407,19 +407,23 @@ var _ = Describe("Criteria", func() { Context("with child playlists", func() { var ( - topLevelInPlaylistID string - topLevelNotInPlaylistID string - nestedAnyInPlaylistID string - nestedAnyNotInPlaylistID string - nestedAllInPlaylistID string - nestedAllNotInPlaylistID string + topLevelInPlaylistID string + topLevelInPlaylistPath string + topLevelNotInPlaylistID string + nestedAnyInPlaylistID string + nestedAnyNotInPlaylistID string + nestedAllInPlaylistID string + nestedAllNotInPlaylistID string + nestedAnyNotInPlaylistPath string ) BeforeEach(func() { topLevelInPlaylistID = uuid.NewString() + topLevelInPlaylistPath = "./test.nsp" topLevelNotInPlaylistID = uuid.NewString() nestedAnyInPlaylistID = uuid.NewString() nestedAnyNotInPlaylistID = uuid.NewString() + nestedAnyNotInPlaylistPath = "../not-in-playlist.m3u" nestedAllInPlaylistID = uuid.NewString() nestedAllNotInPlaylistID = uuid.NewString() @@ -427,10 +431,12 @@ var _ = Describe("Criteria", func() { goObj = Criteria{ Expression: All{ InPlaylist{"id": topLevelInPlaylistID}, + InPlaylist{"path": topLevelInPlaylistPath}, NotInPlaylist{"id": topLevelNotInPlaylistID}, Any{ InPlaylist{"id": nestedAnyInPlaylistID}, NotInPlaylist{"id": nestedAnyNotInPlaylistID}, + NotInPlaylist{"path": nestedAnyNotInPlaylistPath}, }, All{ InPlaylist{"id": nestedAllInPlaylistID}, @@ -443,6 +449,10 @@ var _ = Describe("Criteria", func() { ids := goObj.ChildPlaylistIds() gomega.Expect(ids).To(gomega.ConsistOf(topLevelInPlaylistID, topLevelNotInPlaylistID, nestedAnyInPlaylistID, nestedAnyNotInPlaylistID, nestedAllInPlaylistID, nestedAllNotInPlaylistID)) }) + It("extracts all child smart playlist paths from expression criteria", func() { + ids := goObj.ChildPlaylistPaths() + gomega.Expect(ids).To(gomega.ConsistOf(topLevelInPlaylistPath, nestedAnyNotInPlaylistPath)) + }) It("extracts child smart playlist IDs from deeply nested expression", func() { goObj = Criteria{ Expression: Any{ diff --git a/model/criteria/operators.go b/model/criteria/operators.go index 336f914de..1e611d273 100644 --- a/model/criteria/operators.go +++ b/model/criteria/operators.go @@ -27,6 +27,10 @@ func (all All) ChildPlaylistIds() (ids []string) { return extractPlaylistIds(all) } +func (all All) ChildPlaylistPaths() (paths []string) { + return extractPlaylistPaths(all) +} + type ( Any squirrel.Or Or = Any @@ -44,6 +48,10 @@ func (any Any) ChildPlaylistIds() (ids []string) { return extractPlaylistIds(any) } +func (any Any) ChildPlaylistPaths() (paths []string) { + return extractPlaylistPaths(any) +} + type Is squirrel.Eq type Eq = Is @@ -300,10 +308,13 @@ func (ipl NotInPlaylist) MarshalJSON() ([]byte, error) { } func inList(m map[string]any, negate bool) (sql string, args []any, err error) { - var playlistid string - var ok bool - if playlistid, ok = m["id"].(string); !ok { - return "", nil, errors.New("playlist id not given") + var condition squirrel.Sqlizer + if playlistId, ok := m["id"].(string); ok { + condition = squirrel.Eq{"pl.playlist_id": playlistId} + } else if playlistPath, ok := m["path"].(string); ok { + condition = squirrel.Eq{"playlist.path": playlistPath} + } else { + return "", nil, errors.New("playlist id or path not given") } // Subquery to fetch all media files that are contained in given playlist @@ -312,8 +323,9 @@ func inList(m map[string]any, negate bool) (sql string, args []any, err error) { From("playlist_tracks pl"). LeftJoin("playlist on pl.playlist_id = playlist.id"). Where(squirrel.And{ - squirrel.Eq{"pl.playlist_id": playlistid}, + condition, squirrel.Eq{"playlist.public": 1}}) + subQText, subQArgs, err := subQuery.PlaceholderFormat(squirrel.Question).ToSql() if err != nil { @@ -326,28 +338,32 @@ func inList(m map[string]any, negate bool) (sql string, args []any, err error) { } } -func extractPlaylistIds(inputRule any) (ids []string) { - var id string - var ok bool - +func extractPlaylistField(inputRule any, field string) (values []string) { switch rule := inputRule.(type) { case Any: for _, rules := range rule { - ids = append(ids, extractPlaylistIds(rules)...) + values = append(values, extractPlaylistField(rules, field)...) } case All: for _, rules := range rule { - ids = append(ids, extractPlaylistIds(rules)...) + values = append(values, extractPlaylistField(rules, field)...) } case InPlaylist: - if id, ok = rule["id"].(string); ok { - ids = append(ids, id) + if value, ok := rule[field].(string); ok { + values = append(values, value) } case NotInPlaylist: - if id, ok = rule["id"].(string); ok { - ids = append(ids, id) + if value, ok := rule[field].(string); ok { + values = append(values, value) } } - return } + +func extractPlaylistIds(inputRule any) (ids []string) { + return extractPlaylistField(inputRule, "id") +} + +func extractPlaylistPaths(inputRule any) (paths []string) { + return extractPlaylistField(inputRule, "path") +} diff --git a/model/criteria/operators_test.go b/model/criteria/operators_test.go index 5f756f97d..4921d599a 100644 --- a/model/criteria/operators_test.go +++ b/model/criteria/operators_test.go @@ -46,8 +46,10 @@ var _ = Describe("Operators", func() { Entry("after", After{"lastPlayed": rangeStart}, "annotation.play_date > ?", rangeStart), // InPlaylist and NotInPlaylist are special cases - Entry("inPlaylist", InPlaylist{"id": "deadbeef-dead-beef"}, "media_file.id IN "+ + Entry("inPlaylist [id]", InPlaylist{"id": "deadbeef-dead-beef"}, "media_file.id IN "+ "(SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ? AND playlist.public = ?))", "deadbeef-dead-beef", 1), + Entry("inPlaylist [path]", InPlaylist{"path": "lacuslacus.nsp"}, "media_file.id IN "+ + "(SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (playlist.path = ? AND playlist.public = ?))", "lacuslacus.nsp", 1), Entry("notInPlaylist", NotInPlaylist{"id": "deadbeef-dead-beef"}, "media_file.id NOT IN "+ "(SELECT media_file_id FROM playlist_tracks pl LEFT JOIN playlist on pl.playlist_id = playlist.id WHERE (pl.playlist_id = ? AND playlist.public = ?))", "deadbeef-dead-beef", 1), diff --git a/persistence/playlist_repository.go b/persistence/playlist_repository.go index 8d1bbe0f8..45ae77d5f 100644 --- a/persistence/playlist_repository.go +++ b/persistence/playlist_repository.go @@ -230,6 +230,15 @@ func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool { rules := *pls.Rules // If the playlist depends on other playlists, recursively refresh them first + childPlaylistPaths := rules.ChildPlaylistPaths() + for _, path := range childPlaylistPaths { + childPls, err := r.FindByPath(path) + if err != nil { + log.Error(r.ctx, "Error loading child playlist", "id", pls.ID, "childId", path, err) + return false + } + r.refreshSmartPlaylist(childPls) + } childPlaylistIds := rules.ChildPlaylistIds() for _, id := range childPlaylistIds { childPls, err := r.Get(id) From 7fd3fc34427b482a96164f3667742bfdd38c2f4e Mon Sep 17 00:00:00 2001 From: David Date: Mon, 16 Mar 2026 21:41:47 -0500 Subject: [PATCH 2/2] feat: Support relative playlist paths in smartlists Signed-off-by: David --- model/criteria/operators.go | 4 +- model/playlist.go | 37 ++++++++++++++ model/playlist_test.go | 80 ++++++++++++++++++++++++++++++ persistence/playlist_repository.go | 21 ++++---- 4 files changed, 131 insertions(+), 11 deletions(-) diff --git a/model/criteria/operators.go b/model/criteria/operators.go index 1e611d273..5b9c6a586 100644 --- a/model/criteria/operators.go +++ b/model/criteria/operators.go @@ -333,9 +333,9 @@ func inList(m map[string]any, negate bool) (sql string, args []any, err error) { } if negate { return "media_file.id NOT IN (" + subQText + ")", subQArgs, nil - } else { - return "media_file.id IN (" + subQText + ")", subQArgs, nil } + + return "media_file.id IN (" + subQText + ")", subQArgs, nil } func extractPlaylistField(inputRule any, field string) (values []string) { diff --git a/model/playlist.go b/model/playlist.go index e2f93993d..604cf6510 100644 --- a/model/playlist.go +++ b/model/playlist.go @@ -1,6 +1,7 @@ package model import ( + "path/filepath" "slices" "strconv" "time" @@ -117,6 +118,42 @@ func (pls Playlist) UploadedImagePath() string { return UploadedImagePath(consts.EntityPlaylist, pls.UploadedImage) } +func (pls Playlist) NormalizeChildPaths() { + if pls.Rules.Expression == nil { + return + } + + normalizePlaylistPaths(pls.Rules.Expression, pls.Path) +} + +func normalizePlaylistPaths(inputRule any, referencingPlaylistPath string) { + switch rule := inputRule.(type) { + case criteria.Any: + for _, rules := range rule { + normalizePlaylistPaths(rules, referencingPlaylistPath) + } + case criteria.All: + for _, rules := range rule { + normalizePlaylistPaths(rules, referencingPlaylistPath) + } + case criteria.InPlaylist: + dir := filepath.Dir(referencingPlaylistPath) + if path, ok := rule["path"].(string); ok { + if !filepath.IsAbs(path) { + rule["path"] = filepath.Clean(filepath.Join(dir, path)) + } + } + case criteria.NotInPlaylist: + dir := filepath.Dir(referencingPlaylistPath) + if path, ok := rule["path"].(string); ok { + if !filepath.IsAbs(path) { + rule["path"] = filepath.Clean(filepath.Join(dir, path)) + } + } + } + return +} + type Playlists []Playlist type PlaylistRepository interface { diff --git a/model/playlist_test.go b/model/playlist_test.go index 9ed24f00f..2f8a9222e 100644 --- a/model/playlist_test.go +++ b/model/playlist_test.go @@ -2,6 +2,7 @@ package model_test import ( "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" @@ -43,4 +44,83 @@ var _ = Describe("Playlist", func() { Expect(pls.ToM3U8()).To(Equal(expected)) }) }) + + Describe("NormalizeChildPaths()", func() { + It("normalizes file paths", func() { + pls := model.Playlist{Rules: &criteria.Criteria{ + Expression: criteria.All{ + criteria.InPlaylist{"path": "/test/my-test-path.m3u"}, + criteria.InPlaylist{"path": "../my-test-path.m3u"}, + criteria.NotInPlaylist{"path": "/not-test/not-my-test-path.m3u"}, + criteria.Any{ + criteria.InPlaylist{"path": "../../in-the-test.nsp"}, + criteria.NotInPlaylist{"path": "./sibling.nsp"}, + criteria.All{ + criteria.InPlaylist{"path": "/other-root/other.m3u"}, + criteria.NotInPlaylist{"path": "../../../out-of-containment.nsp"}, + }, + }, + }, + }, + Path: "/test/nested/my-playlist.nsp"} + + pls.NormalizeChildPaths() + Expect(pls.Rules).Should(BeEquivalentTo(&criteria.Criteria{ + Expression: criteria.All{ + criteria.InPlaylist{"path": "/test/my-test-path.m3u"}, + criteria.InPlaylist{"path": "/test/my-test-path.m3u"}, + criteria.NotInPlaylist{"path": "/not-test/not-my-test-path.m3u"}, + criteria.Any{ + criteria.InPlaylist{"path": "/in-the-test.nsp"}, + criteria.NotInPlaylist{"path": "/test/nested/sibling.nsp"}, + criteria.All{ + criteria.InPlaylist{"path": "/other-root/other.m3u"}, + criteria.NotInPlaylist{"path": "/out-of-containment.nsp"}, + }, + }, + }, + })) + }) + + It("normalizes various file paths", func() { + // Absolute path + pls := model.Playlist{ID: "123"} + pls.Rules = &criteria.Criteria{ + Expression: criteria.All{ + criteria.InPlaylist{"path": "/test/my-test-path.m3u"}, + }, + } + + pls.NormalizeChildPaths() + Expect(pls.Rules).NotTo(BeNil()) + }) + + It("handles relative paths correctly", func() { + pls := model.Playlist{ID: "123", Path: "/test/my-playlist.m3u"} + pls.Rules = &criteria.Criteria{ + Expression: criteria.All{ + criteria.InPlaylist{"path": "../my-test-path.m3u"}, + }, + } + + pls.NormalizeChildPaths() + Expect(pls.Rules).Should(BeEquivalentTo(&criteria.Criteria{ + Expression: criteria.All{ + criteria.InPlaylist{"path": "/my-test-path.m3u"}, + }, + })) + }) + + It("ignores non-path entries", func() { + pls := model.Playlist{ID: "123"} + pls.Rules = &criteria.Criteria{ + Expression: criteria.All{ + criteria.InPlaylist{"path": "/not-test/not-my-test-path.m3u"}, + }, + } + + pls.NormalizeChildPaths() + Expect(pls.Rules).NotTo(BeNil()) + }) + }) }) diff --git a/persistence/playlist_repository.go b/persistence/playlist_repository.go index 45ae77d5f..c1bf833c6 100644 --- a/persistence/playlist_repository.go +++ b/persistence/playlist_repository.go @@ -230,15 +230,6 @@ func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool { rules := *pls.Rules // If the playlist depends on other playlists, recursively refresh them first - childPlaylistPaths := rules.ChildPlaylistPaths() - for _, path := range childPlaylistPaths { - childPls, err := r.FindByPath(path) - if err != nil { - log.Error(r.ctx, "Error loading child playlist", "id", pls.ID, "childId", path, err) - return false - } - r.refreshSmartPlaylist(childPls) - } childPlaylistIds := rules.ChildPlaylistIds() for _, id := range childPlaylistIds { childPls, err := r.Get(id) @@ -249,6 +240,18 @@ func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool { r.refreshSmartPlaylist(childPls) } + pls.NormalizeChildPaths() + childPlaylistPaths := rules.ChildPlaylistPaths() + for _, path := range childPlaylistPaths { + log.Info(r.ctx, "Loading child playlist", "id", pls.ID, "childId", path, err) + childPls, err := r.FindByPath(path) + if err != nil { + log.Error(r.ctx, "Error loading child playlist", "id", pls.ID, "childId", path, err) + return false + } + r.refreshSmartPlaylist(childPls) + } + sq := Select("row_number() over (order by "+rules.OrderBy()+") as id", "'"+pls.ID+"' as playlist_id", "media_file.id as media_file_id"). From("media_file").LeftJoin("annotation on ("+ "annotation.item_id = media_file.id"+