nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-10-18 07:30:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(smartplaylists): simplify child playlist reference handling

Browse Source 
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.
This commit is contained in:
Deluan 2026-09-19 17:43:46 -04:00
parent 4553661909
commit 1e5ad0d3f8
6 changed files with 42 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
model/criteria/criteria.go
View File

@ -64,33 +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)
}
func (c Criteria) ChildPlaylistPaths() []string {
if c.Expression == nil {
return nil
}
parent, ok := c.Expression.(interface{ ChildPlaylistPaths() []string })
if !ok {
return nil
}
paths := parent.ChildPlaylistPaths()
slices.Sort(paths)
return slices.Compact(paths)
refs := extract(parent)
slices.Sort(refs)
return slices.Compact(refs)
}
func (c Criteria) MarshalJSON() ([]byte, error) {

4
model/criteria/criteria_test.go
View File

@ -366,8 +366,8 @@ var _ = Describe("Criteria", func() {
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))
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": ""}}}

27
model/criteria/operators.go
View File

@ -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,13 +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() (paths []string) {
return extractPlaylistPaths(all)
}
func (all All) ChildPlaylistPaths() []string { return extractPlaylistField(all, "path") }
type (
Any []Expression
@ -35,13 +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() (paths []string) {
return extractPlaylistPaths(any)
}
func (any Any) ChildPlaylistPaths() []string { return extractPlaylistField(any, "path") }
type Is map[string]any
type Eq = Is
@ -197,11 +190,3 @@ func extractPlaylistField(inputRule any, field string) (values []string) {
}
return
}
func extractPlaylistIds(inputRule any) (ids []string) {
return extractPlaylistField(inputRule, "id")
}
func extractPlaylistPaths(inputRule any) (paths []string) {
return extractPlaylistField(inputRule, "path")
}

26
model/playlist.go
View File

@ -141,16 +141,15 @@ func (pls Playlist) UploadedImagePath() string {
return UploadedImagePath(consts.EntityPlaylist, pls.UploadedImage)
}
func (pls Playlist) WithNormalizeChildPaths() Playlist {
// 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
return pls.Rules
}
rules := *pls.Rules
rules.Expression = normalizePlaylistPaths(pls.Rules.Expression, pls.Path)
plsClone := pls
plsClone.Rules = &rules
return plsClone
return &rules
}
func normalizePlaylistPaths(inputRule criteria.Expression, referencingPlaylistPath string) criteria.Expression {
@ -177,22 +176,23 @@ func normalizePlaylistPaths(inputRule criteria.Expression, referencingPlaylistPa
}
func normalizeChildPathRule(rule map[string]any, referencingPlaylistPath string) map[string]any {
normalized := maps.Clone(rule)
path, ok := rule["path"].(string)
if !ok || path == "" {
return normalized
return rule
}
// References use forward slashes to stay portable, while Playlist.Path is OS-native.
path = filepath.FromSlash(path)
normalized["path"] = path
if isAbsPlaylistRef(path) {
normalized["path"] = filepath.Clean(path)
} else if referencingPlaylistPath != "" {
normalized["path"] = filepath.Clean(filepath.Join(filepath.Dir(referencingPlaylistPath), path))
} else {
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
}

8
model/playlist_test.go
View File

@ -90,7 +90,7 @@ var _ = Describe("Playlist", func() {
})
})
Describe("WithNormalizeChildPaths()", func() {
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...))
@ -98,7 +98,7 @@ var _ = Describe("Playlist", func() {
return abs
}
normalize := func(pls model.Playlist) criteria.Expression {
return pls.WithNormalizeChildPaths().Rules.Expression
return pls.NormalizedRules().Expression
}
It("resolves relative references against the playlist folder", func() {
@ -175,7 +175,7 @@ var _ = Describe("Playlist", func() {
}
pls := model.Playlist{Path: absPath("test", "my-playlist.nsp"), Rules: &rules}
normalized := *pls.WithNormalizeChildPaths().Rules
normalized := *pls.NormalizedRules()
normalized.Expression = rules.Expression
Expect(normalized).To(Equal(rules))
})
@ -183,7 +183,7 @@ var _ = Describe("Playlist", func() {
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.WithNormalizeChildPaths()
_ = pls.NormalizedRules()
Expect(original[0]).To(BeEquivalentTo(criteria.InPlaylist{"path": "child.nsp"}))
})
})

16
persistence/smart_playlist_repository.go
View File

@ -1,11 +1,13 @@
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"
)
@ -42,18 +44,17 @@ func (r *playlistRepository) refreshSmartPlaylistTree(pls *model.Playlist, visit
return false
}
normalisedPls := pls.WithNormalizeChildPaths()
rulesSQL := newSmartPlaylistCriteria(*normalisedPls.Rules, withSmartPlaylistOwner(*usr))
rulesSQL := newSmartPlaylistCriteria(*pls.NormalizedRules(), withSmartPlaylistOwner(*usr))
if !r.refreshChildPlaylists(&normalisedPls, rulesSQL, visited) {
if !r.refreshChildPlaylists(pls, rulesSQL, visited) {
return false
}
if err := r.resolvePercentageLimit(&normalisedPls, &rulesSQL, usr.ID); err != nil {
if err := r.resolvePercentageLimit(pls, &rulesSQL, usr.ID); err != nil {
return false
}
sq := r.buildSmartPlaylistQuery(&normalisedPls, rulesSQL, usr.ID)
sq := r.buildSmartPlaylistQuery(pls, rulesSQL, usr.ID)
sq, err := r.addCriteria(sq, rulesSQL)
if err != nil {
log.Error(r.ctx, "Error building smart playlist criteria", "playlist", pls.Name, "id", pls.ID, err)
@ -114,10 +115,7 @@ func (r *playlistRepository) refreshChildPlaylists(pls *model.Playlist, rulesSQL
conditions = append(conditions, Eq{"playlist.id": childPlaylistIds})
}
if len(childPlaylistPaths) > 0 {
lookupPaths := make([]string, 0, len(childPlaylistPaths))
for _, path := range childPlaylistPaths {
lookupPaths = append(lookupPaths, pathVariants(path)...)
}
lookupPaths := slices.Concat(slice.Map(childPlaylistPaths, pathVariants)...)
conditions = append(conditions, Eq{"playlist.path": lookupPaths})
}