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