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

fix(smartplaylists): handle empty playlist paths

Browse Source 
Signed-off-by: David <dvedvick@gmail.com>
This commit is contained in:
David 2026-05-27 21:09:14 -05:00
parent 7328bbbba0
commit 8877d06b5c
5 changed files with 53 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
model/playlist.go
View File

@ -118,8 +118,8 @@ func (pls Playlist) UploadedImagePath() string {
return UploadedImagePath(consts.EntityPlaylist, pls.UploadedImage)
}
func (pls Playlist) NormalizeChildPaths() {
if pls.Rules.Expression == nil {
func (pls *Playlist) NormalizeChildPaths() {
if pls.Rules == nil || pls.Rules.Expression == nil {
return
}
@ -127,6 +127,10 @@ func (pls Playlist) NormalizeChildPaths() {
}
func normalizePlaylistPaths(inputRule any, referencingPlaylistPath string) {
if referencingPlaylistPath == "" {
return
}
switch rule := inputRule.(type) {
case criteria.Any:
for _, rules := range rule {
@ -139,6 +143,10 @@ func normalizePlaylistPaths(inputRule any, referencingPlaylistPath string) {
case criteria.InPlaylist:
dir := filepath.Dir(referencingPlaylistPath)
if path, ok := rule["path"].(string); ok {
if path == "" {
return
}
if !filepath.IsAbs(path) {
rule["path"] = filepath.Clean(filepath.Join(dir, path))
}
@ -146,6 +154,10 @@ func normalizePlaylistPaths(inputRule any, referencingPlaylistPath string) {
case criteria.NotInPlaylist:
dir := filepath.Dir(referencingPlaylistPath)
if path, ok := rule["path"].(string); ok {
if path == "" {
return
}
if !filepath.IsAbs(path) {
rule["path"] = filepath.Clean(filepath.Join(dir, path))
}

71
model/playlist_test.go
View File

@ -47,21 +47,26 @@ var _ = Describe("Playlist", func() {
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"},
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.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"}
pls.NormalizeChildPaths()
@ -73,54 +78,32 @@ var _ = Describe("Playlist", func() {
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("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"},
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"},
},
},
}
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"},
},
}
Path: ""}
pls.NormalizeChildPaths()
Expect(pls.Rules).Should(BeEquivalentTo(&criteria.Criteria{
Expression: criteria.All{
criteria.InPlaylist{"path": "/my-test-path.m3u"},
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())
})
})
})

2
persistence/criteria_sql.go
View File

@ -315,7 +315,7 @@ func (c smartPlaylistCriteria) inList(values map[string]any, negate bool) (squir
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 {
} 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")

7
persistence/criteria_sql_test.go
View File

@ -157,6 +157,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")))
})
Describe("sort", func() {
It("sorts by regular fields", func() {
Expect(newSmartPlaylistCriteria(criteria.Criteria{Sort: "title"}).OrderBy()).To(Equal("media_file.title asc"))

7
persistence/smart_playlist_repository.go
View File

@ -91,13 +91,12 @@ 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()
pls.NormalizeChildPaths()
childPlaylistPaths := rulesSQL.ChildPlaylistPaths()
if len(childPlaylistIds) == 0 && len(childPlaylistPaths) == 0 {
return true
}
pls.NormalizeChildPaths()
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)
@ -107,7 +106,9 @@ func (r *playlistRepository) refreshChildPlaylists(pls *model.Playlist, rulesSQL
found := make(map[string]struct{}, len(childPlaylists)*2)
for i := range childPlaylists {
found[childPlaylists[i].ID] = struct{}{}
found[childPlaylists[i].Path] = struct{}{}
if childPlaylists[i].Path != "" {
found[childPlaylists[i].Path] = struct{}{}
}
r.refreshSmartPlaylist(&childPlaylists[i])
}
for _, id := range childPlaylistIds {