2022-12-21 14:37:08 -05:00
|
|
|
package model_test
|
|
|
|
|
|
|
|
|
|
import (
|
feat(smartplaylists): add support for referencing playlists using paths (#5187)
* feat: Add support for referencing playlists using paths
Signed-off-by: David <dvedvick@gmail.com>
* feat: Support relative playlist paths in smartlists
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): protect against nil panic
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): refreshing child playlists
Signed-off-by: David <dvedvick@gmail.com>
* chore(smartplaylists): log field parsing error
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): handle empty playlist paths
Signed-off-by: David <dvedvick@gmail.com>
* refactor(smartplaylists): make NormalizeChildPaths non-mutating
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): stop warning on every inPlaylist rule without the looked-up field
Rules that reference a playlist by id have no path field, and the reverse, so
the warning fired on every refresh. The log call also had a bad argument count.
* fix(smartplaylists): ignore empty inPlaylist id and path references
An empty path matched every playlist without a file path, including the
referencing playlist itself, so the refresh recursed until the stack overflowed.
An empty id also shadowed a valid path in the same rule.
* 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.
* fix(smartplaylists): keep all criteria fields when normalizing child paths
The field-by-field copy dropped RefreshDelay.
* fix(smartplaylists): clean absolute inPlaylist path references
Only relative references were cleaned, so an absolute reference such as
/music/./child.nsp never matched the stored /music/child.nsp.
* fix(smartplaylists): stop infinite recursion on playlists that reference each other
Two smart playlists referencing each other, by id or by path, recursed until the
stack overflowed and the server died. The refresh now tracks visited playlists.
* fix(smartplaylists): resolve inPlaylist path references with OS-native separators
Playlist.Path is OS-native, but references in a .nsp file use forward slashes.
On Windows they never matched, and a leading slash was not seen as absolute.
The specs now build OS-native paths, so they also run on Windows.
* fix(smartplaylists): warn when a relative inPlaylist path cannot be resolved
A playlist created in the UI has no file path, so a relative reference silently
matched nothing.
* refactor(smartplaylists): simplify child playlist reference handling
Share one extractor for child ids and paths, return only the normalized rules
instead of a playlist copy, and resolve each path reference in a single switch.
* test(smartplaylists): store the Unicode child path in OS-native form
Playlist.Path is OS-native, so on Windows the forward-slash fixture never matched
the normalized reference.
---------
Signed-off-by: David <dvedvick@gmail.com>
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
2026-09-19 20:05:58 -05:00
|
|
|
"path/filepath"
|
2026-07-16 22:20:35 -04:00
|
|
|
"time"
|
|
|
|
|
|
|
|
|
|
"github.com/navidrome/navidrome/conf"
|
|
|
|
|
"github.com/navidrome/navidrome/conf/configtest"
|
2022-12-21 14:37:08 -05:00
|
|
|
"github.com/navidrome/navidrome/model"
|
2026-07-16 22:20:35 -04:00
|
|
|
"github.com/navidrome/navidrome/model/criteria"
|
2026-04-19 13:16:47 -04:00
|
|
|
"github.com/navidrome/navidrome/tests"
|
2022-12-21 14:37:08 -05:00
|
|
|
. "github.com/onsi/ginkgo/v2"
|
|
|
|
|
. "github.com/onsi/gomega"
|
|
|
|
|
)
|
|
|
|
|
|
|
|
|
|
var _ = Describe("Playlist", func() {
|
|
|
|
|
Describe("ToM3U8()", func() {
|
|
|
|
|
var pls model.Playlist
|
|
|
|
|
BeforeEach(func() {
|
|
|
|
|
pls = model.Playlist{Name: "Mellow sunset"}
|
|
|
|
|
pls.Tracks = model.PlaylistTracks{
|
|
|
|
|
{MediaFile: model.MediaFile{Artist: "Morcheeba feat. Kurt Wagner", Title: "What New York Couples Fight About",
|
2025-05-27 12:37:57 -04:00
|
|
|
Duration: 377.84,
|
|
|
|
|
LibraryPath: "/music/library", Path: "Morcheeba/Charango/01-06 What New York Couples Fight About.mp3"}},
|
2022-12-21 14:37:08 -05:00
|
|
|
{MediaFile: model.MediaFile{Artist: "A Tribe Called Quest", Title: "Description of a Fool (Groove Armada's Acoustic mix)",
|
2025-05-27 12:37:57 -04:00
|
|
|
Duration: 374.49,
|
|
|
|
|
LibraryPath: "/music/library", Path: "Groove Armada/Back to Mine_ Groove Armada/01-01 Description of a Fool (Groove Armada's Acoustic mix).mp3"}},
|
2022-12-21 14:37:08 -05:00
|
|
|
{MediaFile: model.MediaFile{Artist: "Lou Reed", Title: "Walk on the Wild Side",
|
2025-05-27 12:37:57 -04:00
|
|
|
Duration: 253.1,
|
|
|
|
|
LibraryPath: "/music/library", Path: "Lou Reed/Walk on the Wild Side_ The Best of Lou Reed/01-06 Walk on the Wild Side.m4a"}},
|
2022-12-21 14:37:08 -05:00
|
|
|
{MediaFile: model.MediaFile{Artist: "Legião Urbana", Title: "On the Way Home",
|
2025-05-27 12:37:57 -04:00
|
|
|
Duration: 163.89,
|
|
|
|
|
LibraryPath: "/music/library", Path: "Legião Urbana/Música p_ acampamentos/02-05 On the Way Home.mp3"}},
|
2022-12-21 14:37:08 -05:00
|
|
|
}
|
|
|
|
|
})
|
|
|
|
|
It("generates the correct M3U format", func() {
|
2026-04-19 13:16:47 -04:00
|
|
|
tests.SkipOnWindows("path separator bug (#TBD-path-sep-model)")
|
2022-12-21 14:37:08 -05:00
|
|
|
expected := `#EXTM3U
|
|
|
|
|
#PLAYLIST:Mellow sunset
|
|
|
|
|
#EXTINF:378,Morcheeba feat. Kurt Wagner - What New York Couples Fight About
|
|
|
|
|
/music/library/Morcheeba/Charango/01-06 What New York Couples Fight About.mp3
|
|
|
|
|
#EXTINF:374,A Tribe Called Quest - Description of a Fool (Groove Armada's Acoustic mix)
|
|
|
|
|
/music/library/Groove Armada/Back to Mine_ Groove Armada/01-01 Description of a Fool (Groove Armada's Acoustic mix).mp3
|
|
|
|
|
#EXTINF:253,Lou Reed - Walk on the Wild Side
|
|
|
|
|
/music/library/Lou Reed/Walk on the Wild Side_ The Best of Lou Reed/01-06 Walk on the Wild Side.m4a
|
|
|
|
|
#EXTINF:164,Legião Urbana - On the Way Home
|
|
|
|
|
/music/library/Legião Urbana/Música p_ acampamentos/02-05 On the Way Home.mp3
|
|
|
|
|
`
|
|
|
|
|
Expect(pls.ToM3U8()).To(Equal(expected))
|
|
|
|
|
})
|
|
|
|
|
})
|
2026-07-16 22:20:35 -04:00
|
|
|
|
|
|
|
|
Describe("RefreshDelay", func() {
|
|
|
|
|
BeforeEach(func() {
|
|
|
|
|
DeferCleanup(configtest.SetupConfig())
|
|
|
|
|
conf.Server.SmartPlaylistRefreshDelay = 5 * time.Second
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("returns the global config value when rules have no refreshDelay", func() {
|
|
|
|
|
pls := model.Playlist{Rules: &criteria.Criteria{Expression: criteria.All{criteria.Is{"loved": true}}}}
|
|
|
|
|
Expect(pls.RefreshDelay()).To(Equal(5 * time.Second))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("returns the per-playlist value when set", func() {
|
|
|
|
|
pls := model.Playlist{Rules: &criteria.Criteria{
|
|
|
|
|
Expression: criteria.All{criteria.Is{"loved": true}},
|
|
|
|
|
RefreshDelay: 24 * time.Hour,
|
|
|
|
|
}}
|
|
|
|
|
Expect(pls.RefreshDelay()).To(Equal(24 * time.Hour))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("returns the global value for non-smart playlists", func() {
|
|
|
|
|
pls := model.Playlist{}
|
|
|
|
|
Expect(pls.RefreshDelay()).To(Equal(5 * time.Second))
|
|
|
|
|
})
|
|
|
|
|
})
|
fix(playlist): block track edits on synced playlists across all APIs (#5984)
* fix(playlist): block track edits on synced playlists across all APIs
A synced playlist's tracks come from its source file, so any track edit made
through the UI or an API was silently reverted on the next scan. Track mutations
funnel through two service guards, checkTracksEditable (incremental edits) and
Create (wholesale replace, used by Subsonic createPlaylist and Jellyfin's
replace path), which each duplicated the smart-playlist check. Both now consult
a shared model.Playlist.TracksEditable() predicate, so the native, Subsonic, and
Jellyfin paths are all locked: track edits return ErrNotAuthorized (403, or
Subsonic error 50) instead of being accepted and lost. Metadata-only edits
(name, comment, public, the sync flag itself) still go through checkWritable and
are unaffected. In the UI, a synced playlist's track list becomes read-only,
mirroring how smart playlists already behave.
* fix(playlist): return 409 Conflict for non-editable playlist track edits
The previous commit rejected track edits on smart and synced playlists with
ErrNotAuthorized (403). That conflates two different things: a 403 says the
caller lacks permission, but a synced or smart playlist's tracks are immutable
for everyone, including the owner and admins. It is a property of the resource,
not the caller.
Introduce ErrPlaylistNotEditable and return it from both track-edit guards. The
Native and Jellyfin APIs now map it to 409 Conflict; Subsonic maps it to error
50, the closest code it has (it has no read-only concept). The Native track
handlers previously mapped this rejection inconsistently (400 on add, 500 on
remove, 403 on reorder) through a new shared writePlaylistError helper. Genuine
authorization failures (non-owner, non-admin) still return ErrNotAuthorized.
* fix(playlist): surface synced read-only state in picker, Jellyfin, and OpenSubsonic
Follow-up to the track-edit lock: the read-only state was enforced but not
advertised consistently, so clients still offered edits that the server rejects.
- UI: the Add to Playlist picker filtered targets by isWritable only, offering
synced playlists that then 409 on add. It now filters with canChangeTracks.
- Jellyfin: addToPlaylist/removeFromPlaylist hard-coded every error to 404, so a
locked playlist reported "not found" instead of 409. They now return 409 for
ErrPlaylistNotEditable while keeping the deliberate anti-probing 404 for every
other error (a non-owner never reaches ErrPlaylistNotEditable, so 409 leaks
nothing).
- OpenSubsonic: buildOSPlaylist marked only smart playlists readonly; owned
synced playlists advertised readonly=false. Readonly now also covers
!TracksEditable(), matching the existing smart-playlist treatment.
* fix(jellyfin): report CanEdit from playlist editability in permission probes
getPlaylistUsers and getPlaylistUser returned CanEdit: true unconditionally, so
Finamp (which probes this before showing edit controls) offered track editing on
synced/smart playlists whose add/remove requests now return 409. Both handlers
now fetch the playlist and set CanEdit from TracksEditable(), keeping the
deliberate non-owner looseness (CanEdit stays true for a normal playlist a
non-owner views) and mapping any lookup error to 404 like the sibling probes.
* fix(playlist): check ownership before editability when replacing tracks
Create checked TracksEditable() before ownership, so a non-owner replacing
another user's public smart/synced playlist (Jellyfin updatePlaylist with a
non-empty Ids list) received a 409 read-only conflict instead of a 403
authorization failure. The incremental guards check ownership first via
checkWritable; Create now matches that order. Subsonic is unaffected (both errors
map to code 50). Owners of their own smart/synced playlists still get the
read-only conflict.
* fix(jellyfin): return 403 for locked playlists, matching Jellyfin
Jellyfin itself refuses edits on its file-backed playlists with Forbid() (403):
PlaylistsController gates every mutation on OwnerUserId == caller or a share with
CanEdit, and playlists imported from .m3u files satisfy neither. Its CanEdit is
an ACL field, not a read-only marker, and Jellyfin core has no server-managed
playlist type at all.
Our Jellyfin routes exist to imitate that API, so ErrPlaylistNotEditable now maps
to 403 there instead of 409. The native API keeps 409 (a resource-state conflict
is the accurate REST answer where we define the contract) and Subsonic keeps
error 50, its closest code.
* chore(playlist): trim comments added by this branch
Several comments ran to three or four lines and carried rationale that belongs in
the commit history rather than the code: what Jellyfin does with its own
file-backed playlists, and restatements of the expressions directly below them.
Each block is now one or two lines covering only the non-obvious why.
2026-08-19 08:47:53 -04:00
|
|
|
|
|
|
|
|
Describe("TracksEditable", func() {
|
|
|
|
|
It("is true for a plain playlist", func() {
|
|
|
|
|
Expect(model.Playlist{}.TracksEditable()).To(BeTrue())
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("is false for a smart playlist", func() {
|
|
|
|
|
pls := model.Playlist{Rules: &criteria.Criteria{Expression: criteria.Is{"loved": true}}}
|
|
|
|
|
Expect(pls.TracksEditable()).To(BeFalse())
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("is false for a synced playlist", func() {
|
|
|
|
|
Expect(model.Playlist{Sync: true}.TracksEditable()).To(BeFalse())
|
|
|
|
|
})
|
|
|
|
|
})
|
feat(smartplaylists): add support for referencing playlists using paths (#5187)
* feat: Add support for referencing playlists using paths
Signed-off-by: David <dvedvick@gmail.com>
* feat: Support relative playlist paths in smartlists
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): protect against nil panic
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): refreshing child playlists
Signed-off-by: David <dvedvick@gmail.com>
* chore(smartplaylists): log field parsing error
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): handle empty playlist paths
Signed-off-by: David <dvedvick@gmail.com>
* refactor(smartplaylists): make NormalizeChildPaths non-mutating
Signed-off-by: David <dvedvick@gmail.com>
* fix(smartplaylists): stop warning on every inPlaylist rule without the looked-up field
Rules that reference a playlist by id have no path field, and the reverse, so
the warning fired on every refresh. The log call also had a bad argument count.
* fix(smartplaylists): ignore empty inPlaylist id and path references
An empty path matched every playlist without a file path, including the
referencing playlist itself, so the refresh recursed until the stack overflowed.
An empty id also shadowed a valid path in the same rule.
* 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.
* fix(smartplaylists): keep all criteria fields when normalizing child paths
The field-by-field copy dropped RefreshDelay.
* fix(smartplaylists): clean absolute inPlaylist path references
Only relative references were cleaned, so an absolute reference such as
/music/./child.nsp never matched the stored /music/child.nsp.
* fix(smartplaylists): stop infinite recursion on playlists that reference each other
Two smart playlists referencing each other, by id or by path, recursed until the
stack overflowed and the server died. The refresh now tracks visited playlists.
* fix(smartplaylists): resolve inPlaylist path references with OS-native separators
Playlist.Path is OS-native, but references in a .nsp file use forward slashes.
On Windows they never matched, and a leading slash was not seen as absolute.
The specs now build OS-native paths, so they also run on Windows.
* fix(smartplaylists): warn when a relative inPlaylist path cannot be resolved
A playlist created in the UI has no file path, so a relative reference silently
matched nothing.
* refactor(smartplaylists): simplify child playlist reference handling
Share one extractor for child ids and paths, return only the normalized rules
instead of a playlist copy, and resolve each path reference in a single switch.
* test(smartplaylists): store the Unicode child path in OS-native form
Playlist.Path is OS-native, so on Windows the forward-slash fixture never matched
the normalized reference.
---------
Signed-off-by: David <dvedvick@gmail.com>
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
2026-09-19 20:05:58 -05:00
|
|
|
|
|
|
|
|
Describe("NormalizedRules()", func() {
|
|
|
|
|
// absPath builds an OS-native absolute path so these specs also run on Windows.
|
|
|
|
|
absPath := func(parts ...string) string {
|
|
|
|
|
abs, err := filepath.Abs(filepath.Join(parts...))
|
|
|
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
|
return abs
|
|
|
|
|
}
|
|
|
|
|
normalize := func(pls model.Playlist) criteria.Expression {
|
|
|
|
|
return pls.NormalizedRules().Expression
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
It("resolves relative references against the playlist folder", func() {
|
|
|
|
|
pls := model.Playlist{
|
|
|
|
|
Path: absPath("test", "nested", "my-playlist.nsp"),
|
|
|
|
|
Rules: &criteria.Criteria{Expression: criteria.All{
|
|
|
|
|
criteria.InPlaylist{"path": "../up.m3u"},
|
|
|
|
|
criteria.NotInPlaylist{"path": "./sibling.nsp"},
|
|
|
|
|
criteria.Any{criteria.InPlaylist{"path": "sub/deep.nsp"}},
|
|
|
|
|
}},
|
|
|
|
|
}
|
|
|
|
|
Expect(normalize(pls)).To(BeEquivalentTo(criteria.All{
|
|
|
|
|
criteria.InPlaylist{"path": absPath("test", "up.m3u")},
|
|
|
|
|
criteria.NotInPlaylist{"path": absPath("test", "nested", "sibling.nsp")},
|
|
|
|
|
criteria.Any{criteria.InPlaylist{"path": absPath("test", "nested", "sub", "deep.nsp")}},
|
|
|
|
|
}))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("cleans absolute references", func() {
|
|
|
|
|
dirty := absPath("music") + string(filepath.Separator) + "." + string(filepath.Separator) + "child.nsp"
|
|
|
|
|
pls := model.Playlist{
|
|
|
|
|
Path: absPath("test", "my-playlist.nsp"),
|
|
|
|
|
Rules: &criteria.Criteria{Expression: criteria.All{criteria.NotInPlaylist{"path": dirty}}},
|
|
|
|
|
}
|
|
|
|
|
Expect(normalize(pls)).To(BeEquivalentTo(criteria.All{
|
|
|
|
|
criteria.NotInPlaylist{"path": absPath("music", "child.nsp")},
|
|
|
|
|
}))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("treats a leading slash as absolute on every OS", func() {
|
|
|
|
|
pls := model.Playlist{
|
|
|
|
|
Path: absPath("test", "my-playlist.nsp"),
|
|
|
|
|
Rules: &criteria.Criteria{Expression: criteria.All{criteria.InPlaylist{"path": "/other/./root.m3u"}}},
|
|
|
|
|
}
|
|
|
|
|
Expect(normalize(pls)).To(BeEquivalentTo(criteria.All{
|
|
|
|
|
criteria.InPlaylist{"path": filepath.FromSlash("/other/root.m3u")},
|
|
|
|
|
}))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("leaves empty paths and id references untouched", func() {
|
|
|
|
|
pls := model.Playlist{
|
|
|
|
|
Path: absPath("test", "my-playlist.nsp"),
|
|
|
|
|
Rules: &criteria.Criteria{Expression: criteria.All{
|
|
|
|
|
criteria.InPlaylist{"path": ""},
|
|
|
|
|
criteria.InPlaylist{"id": "94d8ba52-7aca-40e2-af82-4cb09c43d710"},
|
|
|
|
|
criteria.Eq{"artist": "Bob Dealin"},
|
|
|
|
|
}},
|
|
|
|
|
}
|
|
|
|
|
Expect(normalize(pls)).To(BeEquivalentTo(criteria.All{
|
|
|
|
|
criteria.InPlaylist{"path": ""},
|
|
|
|
|
criteria.InPlaylist{"id": "94d8ba52-7aca-40e2-af82-4cb09c43d710"},
|
|
|
|
|
criteria.Eq{"artist": "Bob Dealin"},
|
|
|
|
|
}))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("skips relative references when the playlist has no path", func() {
|
|
|
|
|
pls := model.Playlist{
|
|
|
|
|
Rules: &criteria.Criteria{Expression: criteria.All{criteria.InPlaylist{"path": "../up.m3u"}}},
|
|
|
|
|
}
|
|
|
|
|
Expect(normalize(pls)).To(BeEquivalentTo(criteria.All{
|
|
|
|
|
criteria.InPlaylist{"path": filepath.FromSlash("../up.m3u")},
|
|
|
|
|
}))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("preserves every other criteria field", func() {
|
|
|
|
|
rules := criteria.Criteria{
|
|
|
|
|
Expression: criteria.All{criteria.InPlaylist{"path": "child.nsp"}},
|
|
|
|
|
Sort: "title",
|
|
|
|
|
Order: "desc",
|
|
|
|
|
Limit: 10,
|
|
|
|
|
LimitPercent: 25,
|
|
|
|
|
Offset: 5,
|
|
|
|
|
RefreshDelay: 3 * time.Hour,
|
|
|
|
|
}
|
|
|
|
|
pls := model.Playlist{Path: absPath("test", "my-playlist.nsp"), Rules: &rules}
|
|
|
|
|
|
|
|
|
|
normalized := *pls.NormalizedRules()
|
|
|
|
|
normalized.Expression = rules.Expression
|
|
|
|
|
Expect(normalized).To(Equal(rules))
|
|
|
|
|
})
|
|
|
|
|
|
|
|
|
|
It("does not mutate the original playlist rules", func() {
|
|
|
|
|
original := criteria.All{criteria.InPlaylist{"path": "child.nsp"}}
|
|
|
|
|
pls := model.Playlist{Path: absPath("test", "my-playlist.nsp"), Rules: &criteria.Criteria{Expression: original}}
|
|
|
|
|
_ = pls.NormalizedRules()
|
|
|
|
|
Expect(original[0]).To(BeEquivalentTo(criteria.InPlaylist{"path": "child.nsp"}))
|
|
|
|
|
})
|
|
|
|
|
})
|
2022-12-21 14:37:08 -05:00
|
|
|
})
|