nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-01 07:21:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge e55719267fb9c146d31a48e6cb49b93f96dd510e into e80a7937e86e715e4022ef8dac18301a25f9bf4e

Browse Source
This commit is contained in:
David Vedvick 2026-07-02 03:55:28 +00:00 committed by GitHub
commit 8331ee6c0e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 247 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
model/criteria/criteria.go
View File

@ -74,6 +74,21 @@ func (c Criteria) ChildPlaylistIds() []string {
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)
}
func (c Criteria) MarshalJSON() ([]byte, error) {
aux := struct {
All []Expression `json:"all,omitempty"`

22
model/criteria/criteria_test.go
View File

@ -235,19 +235,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()
@ -255,10 +259,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},
@ -271,6 +277,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{

40
model/criteria/operators.go
View File

@ -1,5 +1,7 @@
package criteria
import "github.com/navidrome/navidrome/log"
// Conjunctions need to implement this interface, to allow Criteria to extract child playlist IDs recursively
type conjunction interface {
ChildPlaylistIds() []string
@ -20,6 +22,10 @@ func (all All) ChildPlaylistIds() (ids []string) {
return extractPlaylistIds(all)
}
func (all All) ChildPlaylistPaths() (paths []string) {
return extractPlaylistPaths(all)
}
type (
Any []Expression
Or = Any
@ -35,6 +41,10 @@ func (any Any) ChildPlaylistIds() (ids []string) {
return extractPlaylistIds(any)
}
func (any Any) ChildPlaylistPaths() (paths []string) {
return extractPlaylistPaths(any)
}
type Is map[string]any
type Eq = Is
@ -172,28 +182,36 @@ 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)
if value, ok := rule[field].(string); ok {
values = append(values, value)
} else {
log.Warn("Playlist field not a string", field)
}
case NotInPlaylist:
if id, ok = rule["id"].(string); ok {
ids = append(ids, id)
if value, ok := rule[field].(string); ok {
values = append(values, value)
} else {
log.Warn("Playlist field not a string", field)
}
}
return
}
func extractPlaylistIds(inputRule any) (ids []string) {
return extractPlaylistField(inputRule, "id")
}
func extractPlaylistPaths(inputRule any) (paths []string) {
return extractPlaylistField(inputRule, "path")
}

68
model/playlist.go
View File

@ -1,6 +1,8 @@
package model
import (
"maps"
"path/filepath"
"slices"
"strconv"
"time"
@ -117,6 +119,72 @@ func (pls Playlist) UploadedImagePath() string {
return UploadedImagePath(consts.EntityPlaylist, pls.UploadedImage)
}
func (pls Playlist) WithNormalizeChildPaths() Playlist {
if pls.Rules == nil || pls.Rules.Expression == nil {
return pls
}
plsClone := pls
plsClone.Rules = &criteria.Criteria{
Sort: pls.Rules.Sort,
Limit: pls.Rules.Limit,
LimitPercent: pls.Rules.LimitPercent,
Offset: pls.Rules.Offset,
Order: pls.Rules.Order,
Expression: normalizePlaylistPaths(pls.Rules.Expression, pls.Path),
}
return plsClone
}
func normalizePlaylistPaths(inputRule criteria.Expression, referencingPlaylistPath string) criteria.Expression {
if referencingPlaylistPath == "" {
return inputRule
}
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:
inPlaylist := maps.Clone(rule)
if path, ok := rule["path"].(string); ok {
if path == "" {
return inPlaylist
}
if !filepath.IsAbs(path) {
dir := filepath.Dir(referencingPlaylistPath)
inPlaylist["path"] = filepath.Clean(filepath.Join(dir, path))
}
}
return inPlaylist
case criteria.NotInPlaylist:
notInPlaylist := maps.Clone(rule)
if path, ok := rule["path"].(string); ok {
if path == "" {
return notInPlaylist
}
if !filepath.IsAbs(path) {
dir := filepath.Dir(referencingPlaylistPath)
notInPlaylist["path"] = filepath.Clean(filepath.Join(dir, path))
}
}
return notInPlaylist
}
return inputRule
}
type Playlists []Playlist
type PlaylistRepository interface {

65
model/playlist_test.go
View File

@ -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,68 @@ var _ = Describe("Playlist", func() {
Expect(pls.ToM3U8()).To(Equal(expected))
})
})
Describe("NormalizeChildPaths()", func() {
It("normalizes file paths", func() {
tests.SkipOnWindows("path separator bug (#TBD-path-sep-model)")
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.Eq{"artist": "Bob Dealin'"},
criteria.Any{
criteria.InPlaylist{"path": "../../in-the-test.nsp"},
criteria.NotInPlaylist{"path": "./sibling.nsp"},
criteria.NotInPlaylist{"path": ""},
criteria.All{
criteria.InPlaylist{"path": "/other-root/other.m3u"},
criteria.NotInPlaylist{"path": "../../../out-of-containment.nsp"},
criteria.InPlaylist{"id": "94d8ba52-7aca-40e2-af82-4cb09c43d710"},
},
},
},
},
Path: "/test/nested/my-playlist.nsp"}
newPls := pls.WithNormalizeChildPaths()
Expect(newPls.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.Eq{"artist": "Bob Dealin'"},
criteria.Any{
criteria.InPlaylist{"path": "/in-the-test.nsp"},
criteria.NotInPlaylist{"path": "/test/nested/sibling.nsp"},
criteria.NotInPlaylist{"path": ""},
criteria.All{
criteria.InPlaylist{"path": "/other-root/other.m3u"},
criteria.NotInPlaylist{"path": "/out-of-containment.nsp"},
criteria.InPlaylist{"id": "94d8ba52-7aca-40e2-af82-4cb09c43d710"},
},
},
},
}))
})
It("skips normalization when playlist path is empty", func() {
pls := model.Playlist{
Rules: &criteria.Criteria{
Expression: criteria.All{
criteria.InPlaylist{"path": "../my-test-path.m3u"},
},
},
Path: ""}
newPls := pls.WithNormalizeChildPaths()
Expect(newPls.Rules).Should(BeEquivalentTo(&criteria.Criteria{
Expression: criteria.All{
criteria.InPlaylist{"path": "../my-test-path.m3u"},
},
}))
})
})
})

12
persistence/criteria_sql.go
View File

@ -338,11 +338,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 {
condition = squirrel.Eq{"pl.playlist_id": playlistId}
} else if playlistPath, ok := values["path"].(string); ok && playlistPath != "" {
condition = squirrel.Eq{"playlist.path": 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})

10
persistence/criteria_sql_test.go
View File

@ -47,7 +47,8 @@ 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 = ? 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("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),
@ -263,6 +264,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")))

25
persistence/smart_playlist_repository.go
View File

@ -31,17 +31,18 @@ func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool {
return false
}
rulesSQL := newSmartPlaylistCriteria(*pls.Rules, withSmartPlaylistOwner(*usr))
normalisedPls := pls.WithNormalizeChildPaths()
rulesSQL := newSmartPlaylistCriteria(*normalisedPls.Rules, withSmartPlaylistOwner(*usr))
if !r.refreshChildPlaylists(pls, rulesSQL) {
if !r.refreshChildPlaylists(&normalisedPls, rulesSQL) {
return false
}
if err := r.resolvePercentageLimit(pls, &rulesSQL, usr.ID); err != nil {
if err := r.resolvePercentageLimit(&normalisedPls, &rulesSQL, usr.ID); err != nil {
return false
}
sq := r.buildSmartPlaylistQuery(pls, rulesSQL, usr.ID)
sq := r.buildSmartPlaylistQuery(&normalisedPls, 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)
@ -91,19 +92,23 @@ func (r *playlistRepository) shouldRefreshSmartPlaylist(pls *model.Playlist, usr
// 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 {
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}})
childPlaylists, err := r.GetAll(model.QueryOptions{Filters: Or{Eq{"playlist.id": childPlaylistIds}, Eq{"playlist.path": childPlaylistPaths}}})
if err != nil {
log.Error(r.ctx, "Error loading child playlists for smart playlist refresh", "playlist", pls.Name, "id", pls.ID, "childIds", childPlaylistIds, 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{}{}
if childPlaylists[i].Path != "" {
found[childPlaylists[i].Path] = struct{}{}
}
r.refreshSmartPlaylist(&childPlaylists[i])
}
for _, id := range childPlaylistIds {
@ -111,6 +116,12 @@ func (r *playlistRepository) refreshChildPlaylists(pls *model.Playlist, rulesSQL
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[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
}

23
persistence/smart_playlist_repository_test.go
View File

@ -71,13 +71,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())
@ -95,14 +105,19 @@ 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))
})
})