From 19fa6a2a307d1e738480e520040e5d1c1f3b8bc7 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 19 Sep 2026 17:34:28 -0400 Subject: [PATCH] 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. --- persistence/criteria_sql.go | 15 ++++++++++++- persistence/criteria_sql_test.go | 5 +++-- persistence/smart_playlist_repository.go | 11 +++++++--- persistence/smart_playlist_repository_test.go | 21 +++++++++++++++++++ 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/persistence/criteria_sql.go b/persistence/criteria_sql.go index c61d9b3bf..d2f817438 100644 --- a/persistence/criteria_sql.go +++ b/persistence/criteria_sql.go @@ -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 @@ -348,7 +349,7 @@ func (c smartPlaylistCriteria) inList(values map[string]any, negate bool) (squir 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": playlistPath} + condition = squirrel.Eq{"playlist.path": pathVariants(playlistPath)} } else { return nil, errors.New("playlist id or path not given") } @@ -377,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} diff --git a/persistence/criteria_sql_test.go b/persistence/criteria_sql_test.go index 905edfded..ef91ec989 100644 --- a/persistence/criteria_sql_test.go +++ b/persistence/criteria_sql_test.go @@ -47,9 +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 [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 [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 = ? AND playlist.public = ?))", "/music/x.nsp", 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), diff --git a/persistence/smart_playlist_repository.go b/persistence/smart_playlist_repository.go index ea28e1563..b6042739c 100644 --- a/persistence/smart_playlist_repository.go +++ b/persistence/smart_playlist_repository.go @@ -6,6 +6,7 @@ import ( . "github.com/Masterminds/squirrel" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" + "golang.org/x/text/unicode/norm" ) // PlaylistRepository methods to handle smart playlists, which are defined by criteria and automatically populated @@ -102,7 +103,11 @@ func (r *playlistRepository) refreshChildPlaylists(pls *model.Playlist, rulesSQL conditions = append(conditions, Eq{"playlist.id": childPlaylistIds}) } if len(childPlaylistPaths) > 0 { - conditions = append(conditions, Eq{"playlist.path": childPlaylistPaths}) + lookupPaths := make([]string, 0, len(childPlaylistPaths)) + for _, path := range childPlaylistPaths { + lookupPaths = append(lookupPaths, pathVariants(path)...) + } + conditions = append(conditions, Eq{"playlist.path": lookupPaths}) } childPlaylists, err := r.GetAll(model.QueryOptions{Filters: conditions}) @@ -115,7 +120,7 @@ func (r *playlistRepository) refreshChildPlaylists(pls *model.Playlist, rulesSQL for i := range childPlaylists { found[childPlaylists[i].ID] = struct{}{} if childPlaylists[i].Path != "" { - found[childPlaylists[i].Path] = struct{}{} + found[norm.NFC.String(childPlaylists[i].Path)] = struct{}{} } r.refreshSmartPlaylist(&childPlaylists[i]) } @@ -126,7 +131,7 @@ func (r *playlistRepository) refreshChildPlaylists(pls *model.Playlist, rulesSQL } for _, path := range childPlaylistPaths { - if _, ok := found[path]; !ok { + 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) } } diff --git a/persistence/smart_playlist_repository_test.go b/persistence/smart_playlist_repository_test.go index b462191bd..a12d6b077 100644 --- a/persistence/smart_playlist_repository_test.go +++ b/persistence/smart_playlist_repository_test.go @@ -197,6 +197,27 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() { 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: "/música/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úsica/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