mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 02:17:25 +02:00
feat(smartplaylists): add support for referencing playlists using paths (#5187)
* feat: Add support for referencing playlists using paths Signed-off-by: David <dvedvick@gmail.com> * feat: Support relative playlist paths in smartlists Signed-off-by: David <dvedvick@gmail.com> * fix(smartplaylists): protect against nil panic Signed-off-by: David <dvedvick@gmail.com> * fix(smartplaylists): refreshing child playlists Signed-off-by: David <dvedvick@gmail.com> * chore(smartplaylists): log field parsing error Signed-off-by: David <dvedvick@gmail.com> * fix(smartplaylists): handle empty playlist paths Signed-off-by: David <dvedvick@gmail.com> * refactor(smartplaylists): make NormalizeChildPaths non-mutating Signed-off-by: David <dvedvick@gmail.com> * fix(smartplaylists): stop warning on every inPlaylist rule without the looked-up field Rules that reference a playlist by id have no path field, and the reverse, so the warning fired on every refresh. The log call also had a bad argument count. * fix(smartplaylists): ignore empty inPlaylist id and path references An empty path matched every playlist without a file path, including the referencing playlist itself, so the refresh recursed until the stack overflowed. An empty id also shadowed a valid path in the same rule. * fix(smartplaylists): match inPlaylist paths in both NFC and NFD forms A playlist path is stored in the Unicode form the filesystem reports, which can differ from the form typed in the .nsp file. The exact comparison then found no playlist for names with accents. * fix(smartplaylists): keep all criteria fields when normalizing child paths The field-by-field copy dropped RefreshDelay. * fix(smartplaylists): clean absolute inPlaylist path references Only relative references were cleaned, so an absolute reference such as /music/./child.nsp never matched the stored /music/child.nsp. * fix(smartplaylists): stop infinite recursion on playlists that reference each other Two smart playlists referencing each other, by id or by path, recursed until the stack overflowed and the server died. The refresh now tracks visited playlists. * fix(smartplaylists): resolve inPlaylist path references with OS-native separators Playlist.Path is OS-native, but references in a .nsp file use forward slashes. On Windows they never matched, and a leading slash was not seen as absolute. The specs now build OS-native paths, so they also run on Windows. * fix(smartplaylists): warn when a relative inPlaylist path cannot be resolved A playlist created in the UI has no file path, so a relative reference silently matched nothing. * refactor(smartplaylists): simplify child playlist reference handling Share one extractor for child ids and paths, return only the normalized rules instead of a playlist copy, and resolve each path reference in a single switch. * test(smartplaylists): store the Unicode child path in OS-native form Playlist.Path is OS-native, so on Windows the forward-slash fixture never matched the normalized reference. --------- Signed-off-by: David <dvedvick@gmail.com> Co-authored-by: Deluan Quintão <deluan@navidrome.org>
This commit is contained in:
parent
672c0af580
commit
49f626c00a
9 changed files with 370 additions and 51 deletions
|
|
@ -64,18 +64,21 @@ func (c Criteria) IsPercentageLimit() bool {
|
|||
}
|
||||
|
||||
func (c Criteria) ChildPlaylistIds() []string {
|
||||
if c.Expression == nil {
|
||||
return nil
|
||||
}
|
||||
return c.childPlaylistRefs(conjunction.ChildPlaylistIds)
|
||||
}
|
||||
|
||||
func (c Criteria) ChildPlaylistPaths() []string {
|
||||
return c.childPlaylistRefs(conjunction.ChildPlaylistPaths)
|
||||
}
|
||||
|
||||
func (c Criteria) childPlaylistRefs(extract func(conjunction) []string) []string {
|
||||
parent, ok := c.Expression.(conjunction)
|
||||
if !ok {
|
||||
return nil
|
||||
}
|
||||
|
||||
ids := parent.ChildPlaylistIds()
|
||||
slices.Sort(ids)
|
||||
return slices.Compact(ids)
|
||||
refs := extract(parent)
|
||||
slices.Sort(refs)
|
||||
return slices.Compact(refs)
|
||||
}
|
||||
|
||||
func (c Criteria) MarshalJSON() ([]byte, error) {
|
||||
|
|
|
|||
|
|
@ -323,19 +323,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()
|
||||
|
|
@ -343,10 +347,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},
|
||||
|
|
@ -359,6 +365,18 @@ 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() {
|
||||
paths := goObj.ChildPlaylistPaths()
|
||||
gomega.Expect(paths).To(gomega.ConsistOf(topLevelInPlaylistPath, nestedAnyNotInPlaylistPath))
|
||||
})
|
||||
It("ignores empty child playlist paths", func() {
|
||||
c := Criteria{Expression: All{InPlaylist{"path": ""}, NotInPlaylist{"path": ""}}}
|
||||
gomega.Expect(c.ChildPlaylistPaths()).To(gomega.BeEmpty())
|
||||
})
|
||||
It("ignores empty child playlist ids", func() {
|
||||
c := Criteria{Expression: All{InPlaylist{"id": ""}, NotInPlaylist{"id": ""}}}
|
||||
gomega.Expect(c.ChildPlaylistIds()).To(gomega.BeEmpty())
|
||||
})
|
||||
It("extracts child smart playlist IDs from deeply nested expression", func() {
|
||||
goObj = Criteria{
|
||||
Expression: Any{
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
package criteria
|
||||
|
||||
// Conjunctions need to implement this interface, to allow Criteria to extract child playlist IDs recursively
|
||||
// Conjunctions need to implement this interface, to allow Criteria to extract child playlist references recursively
|
||||
type conjunction interface {
|
||||
ChildPlaylistIds() []string
|
||||
ChildPlaylistPaths() []string
|
||||
}
|
||||
|
||||
type (
|
||||
|
|
@ -16,9 +17,9 @@ func (all All) MarshalJSON() ([]byte, error) {
|
|||
return marshalConjunction("all", all)
|
||||
}
|
||||
|
||||
func (all All) ChildPlaylistIds() (ids []string) {
|
||||
return extractPlaylistIds(all)
|
||||
}
|
||||
func (all All) ChildPlaylistIds() []string { return extractPlaylistField(all, "id") }
|
||||
|
||||
func (all All) ChildPlaylistPaths() []string { return extractPlaylistField(all, "path") }
|
||||
|
||||
type (
|
||||
Any []Expression
|
||||
|
|
@ -31,9 +32,9 @@ func (any Any) MarshalJSON() ([]byte, error) {
|
|||
return marshalConjunction("any", any)
|
||||
}
|
||||
|
||||
func (any Any) ChildPlaylistIds() (ids []string) {
|
||||
return extractPlaylistIds(any)
|
||||
}
|
||||
func (any Any) ChildPlaylistIds() []string { return extractPlaylistField(any, "id") }
|
||||
|
||||
func (any Any) ChildPlaylistPaths() []string { return extractPlaylistField(any, "path") }
|
||||
|
||||
type Is map[string]any
|
||||
type Eq = Is
|
||||
|
|
@ -172,28 +173,20 @@ func (ip IsPresent) MarshalJSON() ([]byte, error) {
|
|||
|
||||
func (ip IsPresent) fields() map[string]any { return ip }
|
||||
|
||||
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)
|
||||
}
|
||||
case NotInPlaylist:
|
||||
if id, ok = rule["id"].(string); ok {
|
||||
ids = append(ids, id)
|
||||
case InPlaylist, NotInPlaylist:
|
||||
if value, ok := rule.(Expression).fields()[field].(string); ok && value != "" {
|
||||
values = append(values, value)
|
||||
}
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,12 +2,16 @@ package model
|
|||
|
||||
import (
|
||||
"iter"
|
||||
"maps"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"slices"
|
||||
"strconv"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/consts"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model/criteria"
|
||||
)
|
||||
|
||||
|
|
@ -137,6 +141,66 @@ func (pls Playlist) UploadedImagePath() string {
|
|||
return UploadedImagePath(consts.EntityPlaylist, pls.UploadedImage)
|
||||
}
|
||||
|
||||
// NormalizedRules returns the rules with child playlist paths resolved to absolute, OS-native paths.
|
||||
func (pls Playlist) NormalizedRules() *criteria.Criteria {
|
||||
if pls.Rules == nil || pls.Rules.Expression == nil {
|
||||
return pls.Rules
|
||||
}
|
||||
|
||||
rules := *pls.Rules
|
||||
rules.Expression = normalizePlaylistPaths(pls.Rules.Expression, pls.Path)
|
||||
return &rules
|
||||
}
|
||||
|
||||
func normalizePlaylistPaths(inputRule criteria.Expression, referencingPlaylistPath string) criteria.Expression {
|
||||
switch rule := inputRule.(type) {
|
||||
case criteria.Any:
|
||||
anyCriteria := make(criteria.Any, len(rule))
|
||||
for i, rules := range rule {
|
||||
anyCriteria[i] = normalizePlaylistPaths(rules, referencingPlaylistPath)
|
||||
}
|
||||
return anyCriteria
|
||||
case criteria.All:
|
||||
allCriteria := make(criteria.All, len(rule))
|
||||
for i, rules := range rule {
|
||||
allCriteria[i] = normalizePlaylistPaths(rules, referencingPlaylistPath)
|
||||
}
|
||||
return allCriteria
|
||||
case criteria.InPlaylist:
|
||||
return criteria.InPlaylist(normalizeChildPathRule(rule, referencingPlaylistPath))
|
||||
case criteria.NotInPlaylist:
|
||||
return criteria.NotInPlaylist(normalizeChildPathRule(rule, referencingPlaylistPath))
|
||||
}
|
||||
|
||||
return inputRule
|
||||
}
|
||||
|
||||
func normalizeChildPathRule(rule map[string]any, referencingPlaylistPath string) map[string]any {
|
||||
path, ok := rule["path"].(string)
|
||||
if !ok || path == "" {
|
||||
return rule
|
||||
}
|
||||
|
||||
// References use forward slashes to stay portable, while Playlist.Path is OS-native.
|
||||
path = filepath.FromSlash(path)
|
||||
switch {
|
||||
case isAbsPlaylistRef(path):
|
||||
path = filepath.Clean(path)
|
||||
case referencingPlaylistPath != "":
|
||||
path = filepath.Join(filepath.Dir(referencingPlaylistPath), path)
|
||||
default:
|
||||
log.Warn("Cannot resolve relative playlist reference: playlist has no file path", "reference", path)
|
||||
}
|
||||
normalized := maps.Clone(rule)
|
||||
normalized["path"] = path
|
||||
return normalized
|
||||
}
|
||||
|
||||
// filepath.IsAbs rejects a bare leading separator on Windows, but that is how Unix spells absolute.
|
||||
func isAbsPlaylistRef(path string) bool {
|
||||
return filepath.IsAbs(path) || os.IsPathSeparator(path[0])
|
||||
}
|
||||
|
||||
type Playlists []Playlist
|
||||
|
||||
type PlaylistCursor iter.Seq2[Playlist, error]
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package model_test
|
||||
|
||||
import (
|
||||
"path/filepath"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
|
|
@ -88,4 +89,102 @@ var _ = Describe("Playlist", func() {
|
|||
Expect(model.Playlist{Sync: true}.TracksEditable()).To(BeFalse())
|
||||
})
|
||||
})
|
||||
|
||||
Describe("NormalizedRules()", func() {
|
||||
// absPath builds an OS-native absolute path so these specs also run on Windows.
|
||||
absPath := func(parts ...string) string {
|
||||
abs, err := filepath.Abs(filepath.Join(parts...))
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
return abs
|
||||
}
|
||||
normalize := func(pls model.Playlist) criteria.Expression {
|
||||
return pls.NormalizedRules().Expression
|
||||
}
|
||||
|
||||
It("resolves relative references against the playlist folder", func() {
|
||||
pls := model.Playlist{
|
||||
Path: absPath("test", "nested", "my-playlist.nsp"),
|
||||
Rules: &criteria.Criteria{Expression: criteria.All{
|
||||
criteria.InPlaylist{"path": "../up.m3u"},
|
||||
criteria.NotInPlaylist{"path": "./sibling.nsp"},
|
||||
criteria.Any{criteria.InPlaylist{"path": "sub/deep.nsp"}},
|
||||
}},
|
||||
}
|
||||
Expect(normalize(pls)).To(BeEquivalentTo(criteria.All{
|
||||
criteria.InPlaylist{"path": absPath("test", "up.m3u")},
|
||||
criteria.NotInPlaylist{"path": absPath("test", "nested", "sibling.nsp")},
|
||||
criteria.Any{criteria.InPlaylist{"path": absPath("test", "nested", "sub", "deep.nsp")}},
|
||||
}))
|
||||
})
|
||||
|
||||
It("cleans absolute references", func() {
|
||||
dirty := absPath("music") + string(filepath.Separator) + "." + string(filepath.Separator) + "child.nsp"
|
||||
pls := model.Playlist{
|
||||
Path: absPath("test", "my-playlist.nsp"),
|
||||
Rules: &criteria.Criteria{Expression: criteria.All{criteria.NotInPlaylist{"path": dirty}}},
|
||||
}
|
||||
Expect(normalize(pls)).To(BeEquivalentTo(criteria.All{
|
||||
criteria.NotInPlaylist{"path": absPath("music", "child.nsp")},
|
||||
}))
|
||||
})
|
||||
|
||||
It("treats a leading slash as absolute on every OS", func() {
|
||||
pls := model.Playlist{
|
||||
Path: absPath("test", "my-playlist.nsp"),
|
||||
Rules: &criteria.Criteria{Expression: criteria.All{criteria.InPlaylist{"path": "/other/./root.m3u"}}},
|
||||
}
|
||||
Expect(normalize(pls)).To(BeEquivalentTo(criteria.All{
|
||||
criteria.InPlaylist{"path": filepath.FromSlash("/other/root.m3u")},
|
||||
}))
|
||||
})
|
||||
|
||||
It("leaves empty paths and id references untouched", func() {
|
||||
pls := model.Playlist{
|
||||
Path: absPath("test", "my-playlist.nsp"),
|
||||
Rules: &criteria.Criteria{Expression: criteria.All{
|
||||
criteria.InPlaylist{"path": ""},
|
||||
criteria.InPlaylist{"id": "94d8ba52-7aca-40e2-af82-4cb09c43d710"},
|
||||
criteria.Eq{"artist": "Bob Dealin"},
|
||||
}},
|
||||
}
|
||||
Expect(normalize(pls)).To(BeEquivalentTo(criteria.All{
|
||||
criteria.InPlaylist{"path": ""},
|
||||
criteria.InPlaylist{"id": "94d8ba52-7aca-40e2-af82-4cb09c43d710"},
|
||||
criteria.Eq{"artist": "Bob Dealin"},
|
||||
}))
|
||||
})
|
||||
|
||||
It("skips relative references when the playlist has no path", func() {
|
||||
pls := model.Playlist{
|
||||
Rules: &criteria.Criteria{Expression: criteria.All{criteria.InPlaylist{"path": "../up.m3u"}}},
|
||||
}
|
||||
Expect(normalize(pls)).To(BeEquivalentTo(criteria.All{
|
||||
criteria.InPlaylist{"path": filepath.FromSlash("../up.m3u")},
|
||||
}))
|
||||
})
|
||||
|
||||
It("preserves every other criteria field", func() {
|
||||
rules := criteria.Criteria{
|
||||
Expression: criteria.All{criteria.InPlaylist{"path": "child.nsp"}},
|
||||
Sort: "title",
|
||||
Order: "desc",
|
||||
Limit: 10,
|
||||
LimitPercent: 25,
|
||||
Offset: 5,
|
||||
RefreshDelay: 3 * time.Hour,
|
||||
}
|
||||
pls := model.Playlist{Path: absPath("test", "my-playlist.nsp"), Rules: &rules}
|
||||
|
||||
normalized := *pls.NormalizedRules()
|
||||
normalized.Expression = rules.Expression
|
||||
Expect(normalized).To(Equal(rules))
|
||||
})
|
||||
|
||||
It("does not mutate the original playlist rules", func() {
|
||||
original := criteria.All{criteria.InPlaylist{"path": "child.nsp"}}
|
||||
pls := model.Playlist{Path: absPath("test", "my-playlist.nsp"), Rules: &criteria.Criteria{Expression: original}}
|
||||
_ = pls.NormalizedRules()
|
||||
Expect(original[0]).To(BeEquivalentTo(criteria.InPlaylist{"path": "child.nsp"}))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import (
|
|||
"github.com/Masterminds/squirrel"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/criteria"
|
||||
"golang.org/x/text/unicode/norm"
|
||||
)
|
||||
|
||||
type smartPlaylistJoinType int
|
||||
|
|
@ -344,11 +345,15 @@ func startOfPeriod(numDays int64, from time.Time) string {
|
|||
}
|
||||
|
||||
func (c smartPlaylistCriteria) inList(values map[string]any, negate bool) (squirrel.Sqlizer, error) {
|
||||
playlistID, ok := values["id"].(string)
|
||||
if !ok {
|
||||
return nil, errors.New("playlist id not given")
|
||||
var condition squirrel.Sqlizer
|
||||
if playlistId, ok := values["id"].(string); ok && playlistId != "" {
|
||||
condition = squirrel.Eq{"pl.playlist_id": playlistId}
|
||||
} else if playlistPath, ok := values["path"].(string); ok && playlistPath != "" {
|
||||
condition = squirrel.Eq{"playlist.path": pathVariants(playlistPath)}
|
||||
} else {
|
||||
return nil, errors.New("playlist id or path not given")
|
||||
}
|
||||
filters := squirrel.And{squirrel.Eq{"pl.playlist_id": playlistID}}
|
||||
filters := squirrel.And{condition}
|
||||
if !c.owner.IsAdmin {
|
||||
if c.owner.ID == "" {
|
||||
filters = append(filters, squirrel.Eq{"playlist.public": 1})
|
||||
|
|
@ -373,6 +378,18 @@ func (c smartPlaylistCriteria) inList(values map[string]any, negate bool) (squir
|
|||
return squirrel.Expr("media_file.id IN ("+subSQL+")", subArgs...), nil
|
||||
}
|
||||
|
||||
// Filesystems disagree on the Unicode form of a name, so match the path in NFC and NFD.
|
||||
func pathVariants(path string) []string {
|
||||
variants := []string{path}
|
||||
if alt := norm.NFC.String(path); alt != path {
|
||||
variants = append(variants, alt)
|
||||
}
|
||||
if alt := norm.NFD.String(path); alt != path {
|
||||
variants = append(variants, alt)
|
||||
}
|
||||
return variants
|
||||
}
|
||||
|
||||
func jsonExpr(info criteria.FieldInfo, cond squirrel.Sqlizer, negate bool) squirrel.Sqlizer {
|
||||
if info.IsRole {
|
||||
return roleCond{role: info.Name(), cond: cond, not: negate}
|
||||
|
|
|
|||
|
|
@ -47,7 +47,10 @@ var _ = Describe("Smart playlist criteria SQL", func() {
|
|||
Entry("in range", criteria.InTheRange{"year": []int{1980, 1990}}, "(media_file.year >= ? AND media_file.year <= ?)", 1980, 1990),
|
||||
Entry("before", criteria.Before{"lastPlayed": time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)}, "annotation.play_date < ?", time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)),
|
||||
Entry("after", criteria.After{"lastPlayed": time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)}, "annotation.play_date > ?", time.Date(2021, 10, 1, 0, 0, 0, 0, time.Local)),
|
||||
Entry("in playlist", criteria.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("in playlist [path]", criteria.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 IN (?) AND playlist.public = ?))", "lacuslacus.nsp", 1),
|
||||
Entry("in playlist [id]", criteria.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("in playlist [empty id falls back to path]", criteria.InPlaylist{"id": "", "path": "/music/x.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 IN (?) AND playlist.public = ?))", "/music/x.nsp", 1),
|
||||
Entry("in playlist [decomposed unicode path]", criteria.InPlaylist{"path": "/m\u00fasica/x.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 IN (?,?) AND playlist.public = ?))", "/m\u00fasica/x.nsp", "/mu\u0301sica/x.nsp", 1),
|
||||
Entry("not in playlist", criteria.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),
|
||||
Entry("album annotation", criteria.Gt{"albumRating": 3}, "album_annotation.rating > ?", 3),
|
||||
Entry("artist annotation", criteria.Is{"artistLoved": true}, "artist_annotation.starred = ?", true),
|
||||
|
|
@ -268,6 +271,13 @@ var _ = Describe("Smart playlist criteria SQL", func() {
|
|||
Expect(err).To(MatchError(ContainSubstring("invalid boolean value for 'missing' expression")))
|
||||
})
|
||||
|
||||
It("returns an error when inPlaylist has empty path", func() {
|
||||
_, err := newSmartPlaylistCriteria(
|
||||
criteria.Criteria{Expression: criteria.InPlaylist{"path": ""}},
|
||||
withSmartPlaylistOwner(model.User{ID: "owner-id", IsAdmin: false})).where()
|
||||
Expect(err).To(MatchError(ContainSubstring("playlist id or path not given")))
|
||||
})
|
||||
|
||||
It("returns an error for a range over a tag/role field", func() {
|
||||
_, err := newSmartPlaylistCriteria(criteria.Criteria{Expression: criteria.InTheRange{"rate": []int{1, 5}}}).where()
|
||||
Expect(err).To(MatchError(ContainSubstring("range operator not supported for tag/role field")))
|
||||
|
|
|
|||
|
|
@ -1,11 +1,14 @@
|
|||
package persistence
|
||||
|
||||
import (
|
||||
"slices"
|
||||
"time"
|
||||
|
||||
. "github.com/Masterminds/squirrel"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/utils/slice"
|
||||
"golang.org/x/text/unicode/norm"
|
||||
)
|
||||
|
||||
// PlaylistRepository methods to handle smart playlists, which are defined by criteria and automatically populated
|
||||
|
|
@ -16,6 +19,17 @@ import (
|
|||
|
||||
// refreshSmartPlaylist evaluates the criteria of a smart playlist and updates its tracks accordingly.
|
||||
func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool {
|
||||
return r.refreshSmartPlaylistTree(pls, map[string]struct{}{})
|
||||
}
|
||||
|
||||
// The visited set stops playlists that reference each other from recursing forever.
|
||||
func (r *playlistRepository) refreshSmartPlaylistTree(pls *model.Playlist, visited map[string]struct{}) bool {
|
||||
if _, seen := visited[pls.ID]; seen {
|
||||
log.Trace(r.ctx, "Skipping already visited smart playlist", "playlist", pls.Name, "id", pls.ID)
|
||||
return false
|
||||
}
|
||||
visited[pls.ID] = struct{}{}
|
||||
|
||||
usr := loggedUser(r.ctx)
|
||||
if !r.shouldRefreshSmartPlaylist(pls, usr) {
|
||||
return false
|
||||
|
|
@ -30,9 +44,9 @@ func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool {
|
|||
return false
|
||||
}
|
||||
|
||||
rulesSQL := newSmartPlaylistCriteria(*pls.Rules, withSmartPlaylistOwner(*usr))
|
||||
rulesSQL := newSmartPlaylistCriteria(*pls.NormalizedRules(), withSmartPlaylistOwner(*usr))
|
||||
|
||||
if !r.refreshChildPlaylists(pls, rulesSQL) {
|
||||
if !r.refreshChildPlaylists(pls, rulesSQL, visited) {
|
||||
return false
|
||||
}
|
||||
|
||||
|
|
@ -89,28 +103,47 @@ func (r *playlistRepository) shouldRefreshSmartPlaylist(pls *model.Playlist, usr
|
|||
|
||||
// refreshChildPlaylists handles refreshing any child playlists that are referenced in the smart playlist criteria.
|
||||
// Returns false if child playlists could not be loaded (DB error), signaling the parent refresh should abort.
|
||||
func (r *playlistRepository) refreshChildPlaylists(pls *model.Playlist, rulesSQL smartPlaylistCriteria) bool {
|
||||
func (r *playlistRepository) refreshChildPlaylists(pls *model.Playlist, rulesSQL smartPlaylistCriteria, visited map[string]struct{}) bool {
|
||||
childPlaylistIds := rulesSQL.ChildPlaylistIds()
|
||||
if len(childPlaylistIds) == 0 {
|
||||
childPlaylistPaths := rulesSQL.ChildPlaylistPaths()
|
||||
if len(childPlaylistIds) == 0 && len(childPlaylistPaths) == 0 {
|
||||
return true
|
||||
}
|
||||
|
||||
childPlaylists, err := r.GetAll(model.QueryOptions{Filters: Eq{"playlist.id": childPlaylistIds}})
|
||||
var conditions Or
|
||||
if len(childPlaylistIds) > 0 {
|
||||
conditions = append(conditions, Eq{"playlist.id": childPlaylistIds})
|
||||
}
|
||||
if len(childPlaylistPaths) > 0 {
|
||||
lookupPaths := slices.Concat(slice.Map(childPlaylistPaths, pathVariants)...)
|
||||
conditions = append(conditions, Eq{"playlist.path": lookupPaths})
|
||||
}
|
||||
|
||||
childPlaylists, err := r.GetAll(model.QueryOptions{Filters: conditions})
|
||||
if err != nil {
|
||||
log.Error(r.ctx, "Error loading child playlists for smart playlist refresh", "playlist", pls.Name, "id", pls.ID, "childIds", childPlaylistIds, err)
|
||||
log.Error(r.ctx, "Error loading child playlists for smart playlist refresh", "playlist", pls.Name, "id", pls.ID, "childIds", childPlaylistIds, "childPaths", childPlaylistPaths, err)
|
||||
return false
|
||||
}
|
||||
|
||||
found := make(map[string]struct{}, len(childPlaylists))
|
||||
found := make(map[string]struct{}, len(childPlaylists)*2)
|
||||
for i := range childPlaylists {
|
||||
found[childPlaylists[i].ID] = struct{}{}
|
||||
r.refreshSmartPlaylist(&childPlaylists[i])
|
||||
if childPlaylists[i].Path != "" {
|
||||
found[norm.NFC.String(childPlaylists[i].Path)] = struct{}{}
|
||||
}
|
||||
r.refreshSmartPlaylistTree(&childPlaylists[i], visited)
|
||||
}
|
||||
for _, id := range childPlaylistIds {
|
||||
if _, ok := found[id]; !ok {
|
||||
log.Warn(r.ctx, "Referenced playlist is not accessible to smart playlist owner", "playlist", pls.Name, "id", pls.ID, "childId", id, "ownerId", pls.OwnerID)
|
||||
}
|
||||
}
|
||||
|
||||
for _, path := range childPlaylistPaths {
|
||||
if _, ok := found[norm.NFC.String(path)]; !ok {
|
||||
log.Warn(r.ctx, "Referenced playlist is not accessible to smart playlist owner", "playlist", pls.Name, "id", pls.ID, "path", path, "ownerId", pls.OwnerID)
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package persistence
|
||||
|
||||
import (
|
||||
"path/filepath"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
|
|
@ -124,13 +125,23 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() {
|
|||
criteria.Contains{"title": "Day"},
|
||||
},
|
||||
}
|
||||
nestedPls := model.Playlist{Name: "Nested", OwnerID: "userid", Public: true, Rules: childRules}
|
||||
nestedPls := model.Playlist{Name: "Nested [ID]", OwnerID: "userid", Public: true, Rules: childRules}
|
||||
Expect(repo.Put(&nestedPls)).To(Succeed())
|
||||
DeferCleanup(func() { _ = repo.Delete(nestedPls.ID) })
|
||||
|
||||
parentPls := model.Playlist{Name: "Parent", OwnerID: "userid", Rules: &criteria.Criteria{
|
||||
childRules = &criteria.Criteria{
|
||||
Expression: criteria.All{
|
||||
criteria.Eq{"artist": "シートベルツ"},
|
||||
},
|
||||
}
|
||||
nestedPathPls := model.Playlist{Name: "Nested [Path]", OwnerID: "userid", Path: "test.nsp", Public: true, Rules: childRules}
|
||||
Expect(repo.Put(&nestedPathPls)).To(Succeed())
|
||||
DeferCleanup(func() { _ = repo.Delete(nestedPathPls.ID) })
|
||||
|
||||
parentPls := model.Playlist{Name: "Parent", OwnerID: "userid", Rules: &criteria.Criteria{
|
||||
Expression: criteria.Any{
|
||||
criteria.InPlaylist{"id": nestedPls.ID},
|
||||
criteria.InPlaylist{"path": nestedPathPls.Path},
|
||||
},
|
||||
}}
|
||||
Expect(repo.Put(&parentPls)).To(Succeed())
|
||||
|
|
@ -148,17 +159,88 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() {
|
|||
Expect(*pls.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
|
||||
|
||||
// Parent should have tracks from the nested playlist
|
||||
Expect(pls.Tracks).To(HaveLen(1))
|
||||
Expect(pls.Tracks).To(HaveLen(2))
|
||||
Expect(pls.Tracks[0].MediaFileID).To(Equal(songDayInALife.ID))
|
||||
|
||||
// Nested playlist should now have been refreshed (EvaluatedAt set)
|
||||
// Nested playlists should now have been refreshed (EvaluatedAt set)
|
||||
nestedPlsAfterParentGet, err := repo.Get(nestedPls.ID)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(nestedPlsAfterParentGet.EvaluatedAt).ToNot(BeNil())
|
||||
Expect(*nestedPlsAfterParentGet.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
|
||||
|
||||
nestedPlsAfterParentGet, err = repo.Get(nestedPathPls.ID)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(nestedPlsAfterParentGet.EvaluatedAt).ToNot(BeNil())
|
||||
Expect(*nestedPlsAfterParentGet.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
|
||||
})
|
||||
})
|
||||
|
||||
It("does not recurse forever when two smart playlists reference each other", func() {
|
||||
conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
|
||||
|
||||
plsA := model.Playlist{Name: "Cycle A", OwnerID: "userid", Public: true, Rules: &criteria.Criteria{
|
||||
Expression: criteria.All{criteria.Contains{"title": "Day"}},
|
||||
}}
|
||||
Expect(repo.Put(&plsA)).To(Succeed())
|
||||
DeferCleanup(func() { _ = repo.Delete(plsA.ID) })
|
||||
|
||||
plsB := model.Playlist{Name: "Cycle B", OwnerID: "userid", Public: true, Rules: &criteria.Criteria{
|
||||
Expression: criteria.All{criteria.InPlaylist{"id": plsA.ID}},
|
||||
}}
|
||||
Expect(repo.Put(&plsB)).To(Succeed())
|
||||
DeferCleanup(func() { _ = repo.Delete(plsB.ID) })
|
||||
|
||||
plsA.Rules = &criteria.Criteria{Expression: criteria.All{criteria.InPlaylist{"id": plsB.ID}}}
|
||||
Expect(repo.Put(&plsA)).To(Succeed())
|
||||
|
||||
_, err := repo.GetWithTracks(plsA.ID, true, false)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
})
|
||||
|
||||
It("does not treat an empty path as a reference to every playlist without a path", func() {
|
||||
conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
|
||||
|
||||
bystander := model.Playlist{Name: "Bystander", OwnerID: "userid", Public: true, Rules: &criteria.Criteria{
|
||||
Expression: criteria.All{criteria.Contains{"title": "Day"}},
|
||||
}}
|
||||
Expect(repo.Put(&bystander)).To(Succeed())
|
||||
DeferCleanup(func() { _ = repo.Delete(bystander.ID) })
|
||||
|
||||
parent := model.Playlist{Name: "Empty Path", OwnerID: "userid", Public: true, Rules: &criteria.Criteria{
|
||||
Expression: criteria.All{criteria.InPlaylist{"path": ""}},
|
||||
}}
|
||||
Expect(repo.Put(&parent)).To(Succeed())
|
||||
DeferCleanup(func() { _ = repo.Delete(parent.ID) })
|
||||
|
||||
_, err := repo.GetWithTracks(parent.ID, true, false)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
reloaded, err := repo.Get(bystander.ID)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(reloaded.EvaluatedAt).To(BeNil())
|
||||
})
|
||||
|
||||
It("matches a child path stored in a different Unicode normalization form", func() {
|
||||
conf.Server.SmartPlaylistRefreshDelay = -1 * time.Second
|
||||
|
||||
child := model.Playlist{Name: "NFD Child", OwnerID: "userid", Public: true, Path: filepath.FromSlash("/mu\u0301sica/child.nsp"), Rules: &criteria.Criteria{
|
||||
Expression: criteria.All{criteria.Contains{"title": "Day"}},
|
||||
}}
|
||||
Expect(repo.Put(&child)).To(Succeed())
|
||||
DeferCleanup(func() { _ = repo.Delete(child.ID) })
|
||||
|
||||
parent := model.Playlist{Name: "NFC Parent", OwnerID: "userid", Rules: &criteria.Criteria{
|
||||
Expression: criteria.All{criteria.InPlaylist{"path": "/m\u00fasica/child.nsp"}},
|
||||
}}
|
||||
Expect(repo.Put(&parent)).To(Succeed())
|
||||
DeferCleanup(func() { _ = repo.Delete(parent.ID) })
|
||||
|
||||
pls, err := repo.GetWithTracks(parent.ID, true, false)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(pls.Tracks).To(HaveLen(1))
|
||||
Expect(pls.Tracks[0].MediaFileID).To(Equal(songDayInALife.ID))
|
||||
})
|
||||
|
||||
When("refresh delay has not expired", func() {
|
||||
It("should NOT refresh tracks for smart playlist referenced in parent smart playlist criteria", func() {
|
||||
conf.Server.SmartPlaylistRefreshDelay = 1 * time.Hour
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue