From a5f72f267883198350f603bef31a5f42c89fa4b0 Mon Sep 17 00:00:00 2001 From: David Date: Sat, 14 Mar 2026 13:32:28 -0500 Subject: [PATCH] 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 | 34 ++++++++++++++++-------- persistence/criteria_sql.go | 12 ++++++--- persistence/criteria_sql_test.go | 3 ++- persistence/smart_playlist_repository.go | 14 +++++++--- 6 files changed, 72 insertions(+), 25 deletions(-) diff --git a/model/criteria/criteria.go b/model/criteria/criteria.go index 31d208d08..c213872ad 100644 --- a/model/criteria/criteria.go +++ b/model/criteria/criteria.go @@ -74,6 +74,18 @@ func (c Criteria) ChildPlaylistIds() []string { return slices.Compact(ids) } +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 092cfd36a..c59df2708 100644 --- a/model/criteria/criteria_test.go +++ b/model/criteria/criteria_test.go @@ -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{ diff --git a/model/criteria/operators.go b/model/criteria/operators.go index 3ddd77f8b..5fb59f266 100644 --- a/model/criteria/operators.go +++ b/model/criteria/operators.go @@ -22,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 @@ -37,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 @@ -178,28 +186,32 @@ 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) } 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/persistence/criteria_sql.go b/persistence/criteria_sql.go index a1bae3170..ef6a979ca 100644 --- a/persistence/criteria_sql.go +++ b/persistence/criteria_sql.go @@ -310,11 +310,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 { + 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}) diff --git a/persistence/criteria_sql_test.go b/persistence/criteria_sql_test.go index ae2695a4d..126910a5d 100644 --- a/persistence/criteria_sql_test.go +++ b/persistence/criteria_sql_test.go @@ -45,7 +45,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}, "COALESCE(album_annotation.rating, 0) > ?", 3), Entry("artist annotation", criteria.Is{"artistLoved": true}, "COALESCE(artist_annotation.starred, false) = ?", true), diff --git a/persistence/smart_playlist_repository.go b/persistence/smart_playlist_repository.go index 54f316152..926bfdc7f 100644 --- a/persistence/smart_playlist_repository.go +++ b/persistence/smart_playlist_repository.go @@ -91,19 +91,21 @@ 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{}{} + found[childPlaylists[i].Path] = struct{}{} r.refreshSmartPlaylist(&childPlaylists[i]) } for _, id := range childPlaylistIds { @@ -111,6 +113,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 }