arian/navidrome
1
0
Fork 0
Code Issues Pull Requests 1 Packages Projects Releases Wiki Activity

refactor: move playlist business logic from repositories to service layer (#5027)

Browse Source 
* refactor: move playlist business logic from repositories to core.Playlists service

Move authorization, permission checks, and orchestration logic from
playlist repositories to the core.Playlists service, following the
existing pattern used by core.Share and core.Library.

Changes:
- Expand core.Playlists interface with read, mutation, track management,
  and REST adapter methods
- Add playlistRepositoryWrapper for REST Save/Update/Delete with
  permission checks (follows Share/Library pattern)
- Simplify persistence/playlist_repository.go: remove isWritable(),
  auth checks from Delete()/Put()/updatePlaylist()
- Simplify persistence/playlist_track_repository.go: remove
  isTracksEditable() and permission checks from Add/Delete/Reorder
- Update Subsonic API handlers to route through service
- Update Native API handlers to accept core.Playlists instead of
  model.DataStore

* test: add coverage for playlist service methods and REST wrapper

Add 30 new tests covering the service methods added during the playlist
refactoring:

- Delete: owner, admin, denied, not found
- Create: new playlist, replace tracks, admin bypass, denied, not found
- AddTracks: owner, admin, denied, smart playlist, not found
- RemoveTracks: owner, smart playlist denied, non-owner denied
- ReorderTrack: owner, smart playlist denied
- NewRepository wrapper: Save (owner assignment, ID clearing),
  Update (owner, admin, denied, ownership change, not found),
  Delete (delegation with permission checks)

Expand mockedPlaylistRepo with Get, Delete, Tracks, GetWithTracks, and
rest.Persistable methods. Add mockedPlaylistTrackRepo for track
operation verification.

* fix: add authorization check to playlist Update method

Added ownership verification to the Subsonic Update endpoint in the
playlist service layer. The authorization check was present in the old
repository code but was not carried over during the refactoring to the
service layer, allowing any authenticated user to modify playlists they
don't own via the Subsonic API. Also added corresponding tests for the
Update method's permission logic.

* refactor: improve playlist permission checks and error handling, add e2e tests

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: rename core.Playlists to playlists package and update references

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: rename playlists_internal_test.go to parse_m3u_test.go and update tests; add new parse_nsp.go and rest_adapter.go files

Signed-off-by: Deluan <deluan@navidrome.org>

* fix: block track mutations on smart playlists in Create and Update

Create now rejects replacing tracks on smart playlists (pre-existing
gap). Update now uses checkTracksEditable instead of checkWritable
when track changes are requested, restoring the protection that was
removed from the repository layer during the refactoring. Metadata-only
updates on smart playlists remain allowed.

* test: add smart playlist protection tests to ensure readonly behavior and mutation restrictions

* refactor: optimize track removal and renumbering in playlists

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: implement track reordering in playlists with SQL updates

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: wrap track deletion and reordering in transactions for consistency

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: remove unused getTracks method from playlistTrackRepository

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: optimize playlist track renumbering with CTE-based UPDATE

Replace the DELETE + re-INSERT renumbering strategy with a two-step
UPDATE approach using a materialized CTE and ROW_NUMBER() window
function. The previous approach (SELECT all IDs, DELETE all tracks,
re-INSERT in chunks of 200) required 13 SQL operations for a 2000-track
playlist. The new approach uses just 2 UPDATEs: first negating all IDs
to clear the positive space, then assigning sequential positions via
UPDATE...FROM with a CTE. This avoids the UNIQUE constraint violations
that affected the original correlated subquery while reducing per-delete
request time from ~110ms to ~12ms on a 2000-track playlist.

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: rename New function to NewPlaylists for clarity

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor: update mock playlist repository and tests for consistency

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan Quintão
2026-02-21 19:57:13 -05:00
committed by GitHub
parent 76c01566a9
commit 7ad2907719
40 changed files with 2131 additions and 619 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/e2e/e2e_suite_test.go
+22 -3
View File
@@ -21,6 +21,7 @@ import (
"github.com/navidrome/navidrome/core/external"
"github.com/navidrome/navidrome/core/metrics"
"github.com/navidrome/navidrome/core/playback"
"github.com/navidrome/navidrome/core/playlists"
"github.com/navidrome/navidrome/core/scrobbler"
"github.com/navidrome/navidrome/core/storage/storagetest"
"github.com/navidrome/navidrome/db"
@@ -69,6 +70,14 @@ var (
Name: "Admin User",
IsAdmin: true,
}
// Regular (non-admin) user for permission tests
regularUser = model.User{
ID: "regular-1",
UserName: "regular",
Name: "Regular User",
IsAdmin: false,
}
)
func createFS(files fstest.MapFS) storagetest.FakeFS {
@@ -288,19 +297,29 @@ var _ = BeforeSuite(func() {
adminUserWithPass.NewPassword = "password"
Expect(initDS.User(ctx).Put(&adminUserWithPass)).To(Succeed())
regularUserWithPass := regularUser
regularUserWithPass.NewPassword = "password"
Expect(initDS.User(ctx).Put(&regularUserWithPass)).To(Succeed())
lib = model.Library{ID: 1, Name: "Music Library", Path: "fake:///music"}
Expect(initDS.Library(ctx).Put(&lib)).To(Succeed())
Expect(initDS.User(ctx).SetUserLibraries(adminUser.ID, []int{lib.ID})).To(Succeed())
Expect(initDS.User(ctx).SetUserLibraries(regularUser.ID, []int{lib.ID})).To(Succeed())
loadedUser, err := initDS.User(ctx).FindByUsername(adminUser.UserName)
Expect(err).ToNot(HaveOccurred())
adminUser.Libraries = loadedUser.Libraries
loadedRegular, err := initDS.User(ctx).FindByUsername(regularUser.UserName)
Expect(err).ToNot(HaveOccurred())
regularUser.Libraries = loadedRegular.Libraries
ctx = request.WithUser(GinkgoT().Context(), adminUser)
buildTestFS()
s := scanner.New(ctx, initDS, artwork.NoopCacheWarmer(), events.NoopBroker(),
core.NewPlaylists(initDS), metrics.NewNoopInstance())
playlists.NewPlaylists(initDS), metrics.NewNoopInstance())
_, err = s.ScanAll(ctx, true)
Expect(err).ToNot(HaveOccurred())
@@ -334,7 +353,7 @@ func setupTestDB() {
// Create the Subsonic Router with real DS + noop stubs
s := scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(),
core.NewPlaylists(ds), metrics.NewNoopInstance())
playlists.NewPlaylists(ds), metrics.NewNoopInstance())
router = subsonic.New(
ds,
noopArtwork{},
@@ -344,7 +363,7 @@ func setupTestDB() {
noopProvider{},
s,
events.NoopBroker(),
core.NewPlaylists(ds),
playlists.NewPlaylists(ds),
noopPlayTracker{},
core.NewShare(ds),
playback.PlaybackServer(nil),
server/e2e/subsonic_multilibrary_test.go
+2 -2
View File
@@ -6,9 +6,9 @@ import (
"github.com/Masterminds/squirrel"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/core"
"github.com/navidrome/navidrome/core/artwork"
"github.com/navidrome/navidrome/core/metrics"
"github.com/navidrome/navidrome/core/playlists"
"github.com/navidrome/navidrome/core/storage/storagetest"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/scanner"
@@ -53,7 +53,7 @@ var _ = Describe("Multi-Library Support", Ordered, func() {
// Run incremental scan to import lib2 content (lib1 files unchanged → skipped)
s := scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(),
core.NewPlaylists(ds), metrics.NewNoopInstance())
playlists.NewPlaylists(ds), metrics.NewNoopInstance())
_, err = s.ScanAll(ctx, false)
Expect(err).ToNot(HaveOccurred())
server/e2e/subsonic_playlists_test.go
+428 -18
View File
@@ -1,7 +1,11 @@
package e2e
import (
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/criteria"
"github.com/navidrome/navidrome/server/subsonic/responses"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
@@ -15,9 +19,9 @@ var _ = Describe("Playlist Endpoints", Ordered, func() {
setupTestDB()
// Look up song IDs from scanned data for playlist operations
songs, err := ds.MediaFile(ctx).GetAll(model.QueryOptions{Sort: "title", Max: 3})
songs, err := ds.MediaFile(ctx).GetAll(model.QueryOptions{Sort: "title", Max: 6})
Expect(err).ToNot(HaveOccurred())
Expect(len(songs)).To(BeNumerically(">=", 3))
Expect(len(songs)).To(BeNumerically(">=", 5))
for _, s := range songs {
songIDs = append(songIDs, s.ID)
}
@@ -32,24 +36,30 @@ var _ = Describe("Playlist Endpoints", Ordered, func() {
})
It("createPlaylist creates a new playlist with songs", func() {
resp := doReq("createPlaylist", "name", "Test Playlist", "songId", songIDs[0], "songId", songIDs[1])
resp := doReq("createPlaylist", "name", "Test Playlist",
"songId", songIDs[0], "songId", songIDs[1], "songId", songIDs[2])
Expect(resp.Status).To(Equal(responses.StatusOK))
Expect(resp.Playlist).ToNot(BeNil())
Expect(resp.Playlist.Name).To(Equal("Test Playlist"))
Expect(resp.Playlist.SongCount).To(Equal(int32(2)))
Expect(resp.Playlist.SongCount).To(Equal(int32(3)))
Expect(resp.Playlist.Entry).To(HaveLen(3))
Expect(resp.Playlist.Entry[0].Id).To(Equal(songIDs[0]))
Expect(resp.Playlist.Entry[1].Id).To(Equal(songIDs[1]))
Expect(resp.Playlist.Entry[2].Id).To(Equal(songIDs[2]))
playlistID = resp.Playlist.Id
})
It("getPlaylist returns playlist with tracks", func() {
It("getPlaylist returns playlist with tracks in order", func() {
resp := doReq("getPlaylist", "id", playlistID)
Expect(resp.Status).To(Equal(responses.StatusOK))
Expect(resp.Playlist).ToNot(BeNil())
Expect(resp.Playlist.Name).To(Equal("Test Playlist"))
Expect(resp.Playlist.Entry).To(HaveLen(2))
Expect(resp.Playlist.Entry).To(HaveLen(3))
Expect(resp.Playlist.Entry[0].Id).To(Equal(songIDs[0]))
Expect(resp.Playlist.Entry[1].Id).To(Equal(songIDs[1]))
Expect(resp.Playlist.Entry[2].Id).To(Equal(songIDs[2]))
})
It("createPlaylist without name or playlistId returns error", func() {
@@ -59,40 +69,150 @@ var _ = Describe("Playlist Endpoints", Ordered, func() {
Expect(resp.Error).ToNot(BeNil())
})
It("createPlaylist with playlistId replaces tracks on existing playlist", func() {
// Replace tracks: the playlist had [song0, song1, song2], replace with [song3, song4]
resp := doReq("createPlaylist", "playlistId", playlistID,
"songId", songIDs[3], "songId", songIDs[4])
Expect(resp.Status).To(Equal(responses.StatusOK))
Expect(resp.Playlist).ToNot(BeNil())
Expect(resp.Playlist.Id).To(Equal(playlistID))
Expect(resp.Playlist.SongCount).To(Equal(int32(2)))
Expect(resp.Playlist.Entry).To(HaveLen(2))
Expect(resp.Playlist.Entry[0].Id).To(Equal(songIDs[3]))
Expect(resp.Playlist.Entry[1].Id).To(Equal(songIDs[4]))
})
It("updatePlaylist can rename the playlist", func() {
resp := doReq("updatePlaylist", "playlistId", playlistID, "name", "Renamed Playlist")
Expect(resp.Status).To(Equal(responses.StatusOK))
// Verify the rename
resp = doReq("getPlaylist", "id", playlistID)
Expect(resp.Playlist.Name).To(Equal("Renamed Playlist"))
// Tracks should be unchanged
Expect(resp.Playlist.SongCount).To(Equal(int32(2)))
})
It("updatePlaylist can set comment", func() {
resp := doReq("updatePlaylist", "playlistId", playlistID, "comment", "My favorite songs")
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReq("getPlaylist", "id", playlistID)
Expect(resp.Playlist.Comment).To(Equal("My favorite songs"))
})
It("updatePlaylist can set public visibility", func() {
resp := doReq("updatePlaylist", "playlistId", playlistID, "public", "true")
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReq("getPlaylist", "id", playlistID)
Expect(resp.Playlist.Public).To(BeTrue())
})
It("updatePlaylist can add songs", func() {
resp := doReq("updatePlaylist", "playlistId", playlistID, "songIdToAdd", songIDs[2])
// Playlist currently has [song3, song4], add song0
resp := doReq("updatePlaylist", "playlistId", playlistID, "songIdToAdd", songIDs[0])
Expect(resp.Status).To(Equal(responses.StatusOK))
// Verify the song was added
resp = doReq("getPlaylist", "id", playlistID)
Expect(resp.Playlist.SongCount).To(Equal(int32(3)))
Expect(resp.Playlist.Entry).To(HaveLen(3))
Expect(resp.Playlist.Entry[0].Id).To(Equal(songIDs[3]))
Expect(resp.Playlist.Entry[1].Id).To(Equal(songIDs[4]))
Expect(resp.Playlist.Entry[2].Id).To(Equal(songIDs[0]))
})
It("updatePlaylist can remove songs by index", func() {
// Remove the first song (index 0)
resp := doReq("updatePlaylist", "playlistId", playlistID, "songIndexToRemove", "0")
It("updatePlaylist can add multiple songs at once", func() {
// Playlist currently has [song3, song4, song0], add song1 and song2
resp := doReq("updatePlaylist", "playlistId", playlistID,
"songIdToAdd", songIDs[1], "songIdToAdd", songIDs[2])
Expect(resp.Status).To(Equal(responses.StatusOK))
// Verify the song was removed
resp = doReq("getPlaylist", "id", playlistID)
Expect(resp.Playlist.SongCount).To(Equal(int32(5)))
Expect(resp.Playlist.Entry).To(HaveLen(5))
})
It("updatePlaylist can remove songs by index and verifies correct songs remain", func() {
// Playlist has [song3, song4, song0, song1, song2]
// Remove index 0 (song3) and index 2 (song0)
resp := doReq("updatePlaylist", "playlistId", playlistID,
"songIndexToRemove", "0", "songIndexToRemove", "2")
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReq("getPlaylist", "id", playlistID)
Expect(resp.Playlist.SongCount).To(Equal(int32(3)))
Expect(resp.Playlist.Entry).To(HaveLen(3))
Expect(resp.Playlist.Entry[0].Id).To(Equal(songIDs[4]))
Expect(resp.Playlist.Entry[1].Id).To(Equal(songIDs[1]))
Expect(resp.Playlist.Entry[2].Id).To(Equal(songIDs[2]))
})
It("updatePlaylist can remove and add songs in a single call", func() {
// Playlist has [song4, song1, song2]
// Remove index 1 (song1) and add song3
resp := doReq("updatePlaylist", "playlistId", playlistID,
"songIndexToRemove", "1", "songIdToAdd", songIDs[3])
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReq("getPlaylist", "id", playlistID)
Expect(resp.Playlist.SongCount).To(Equal(int32(3)))
Expect(resp.Playlist.Entry).To(HaveLen(3))
Expect(resp.Playlist.Entry[0].Id).To(Equal(songIDs[4]))
Expect(resp.Playlist.Entry[1].Id).To(Equal(songIDs[2]))
Expect(resp.Playlist.Entry[2].Id).To(Equal(songIDs[3]))
})
It("updatePlaylist can combine metadata change with track removal", func() {
// Playlist has [song4, song2, song3]
// Rename + remove index 0 (song4)
resp := doReq("updatePlaylist", "playlistId", playlistID,
"name", "Final Playlist", "songIndexToRemove", "0")
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReq("getPlaylist", "id", playlistID)
Expect(resp.Playlist.Name).To(Equal("Final Playlist"))
Expect(resp.Playlist.SongCount).To(Equal(int32(2)))
Expect(resp.Playlist.Entry).To(HaveLen(2))
Expect(resp.Playlist.Entry[0].Id).To(Equal(songIDs[2]))
Expect(resp.Playlist.Entry[1].Id).To(Equal(songIDs[3]))
})
It("updatePlaylist can remove all songs from playlist", func() {
// Playlist has [song2, song3] — remove both
resp := doReq("updatePlaylist", "playlistId", playlistID,
"songIndexToRemove", "0", "songIndexToRemove", "1")
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReq("getPlaylist", "id", playlistID)
Expect(resp.Playlist.SongCount).To(Equal(int32(0)))
Expect(resp.Playlist.Entry).To(BeEmpty())
})
It("updatePlaylist can add songs to an empty playlist", func() {
resp := doReq("updatePlaylist", "playlistId", playlistID,
"songIdToAdd", songIDs[0])
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReq("getPlaylist", "id", playlistID)
Expect(resp.Playlist.SongCount).To(Equal(int32(1)))
Expect(resp.Playlist.Entry).To(HaveLen(1))
Expect(resp.Playlist.Entry[0].Id).To(Equal(songIDs[0]))
})
It("updatePlaylist without playlistId returns error", func() {
resp := doReq("updatePlaylist", "name", "No ID")
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error).ToNot(BeNil())
})
It("getPlaylists shows the playlist", func() {
resp := doReq("getPlaylists")
Expect(resp.Status).To(Equal(responses.StatusOK))
Expect(resp.Playlists.Playlist).To(HaveLen(1))
Expect(resp.Playlists.Playlist[0].Id).To(Equal(playlistID))
})
It("deletePlaylist removes the playlist", func() {
@@ -107,4 +227,294 @@ var _ = Describe("Playlist Endpoints", Ordered, func() {
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error).ToNot(BeNil())
})
It("getPlaylists returns empty after deletion", func() {
resp := doReq("getPlaylists")
Expect(resp.Status).To(Equal(responses.StatusOK))
Expect(resp.Playlists.Playlist).To(BeEmpty())
})
Describe("Playlist Permissions", Ordered, func() {
var songIDs []string
var adminPrivateID string
var adminPublicID string
var regularPlaylistID string
BeforeAll(func() {
setupTestDB()
songs, err := ds.MediaFile(ctx).GetAll(model.QueryOptions{Sort: "title", Max: 6})
Expect(err).ToNot(HaveOccurred())
Expect(len(songs)).To(BeNumerically(">=", 3))
for _, s := range songs {
songIDs = append(songIDs, s.ID)
}
})
It("admin creates a private playlist", func() {
resp := doReqWithUser(adminUser, "createPlaylist", "name", "Admin Private",
"songId", songIDs[0], "songId", songIDs[1])
Expect(resp.Status).To(Equal(responses.StatusOK))
adminPrivateID = resp.Playlist.Id
})
It("admin creates a public playlist", func() {
resp := doReqWithUser(adminUser, "createPlaylist", "name", "Admin Public",
"songId", songIDs[0], "songId", songIDs[1])
Expect(resp.Status).To(Equal(responses.StatusOK))
adminPublicID = resp.Playlist.Id
// Make it public
resp = doReqWithUser(adminUser, "updatePlaylist",
"playlistId", adminPublicID, "public", "true")
Expect(resp.Status).To(Equal(responses.StatusOK))
})
It("regular user creates a playlist", func() {
resp := doReqWithUser(regularUser, "createPlaylist", "name", "Regular Playlist",
"songId", songIDs[0])
Expect(resp.Status).To(Equal(responses.StatusOK))
regularPlaylistID = resp.Playlist.Id
})
// --- Private playlist: regular user gets "not found" (repo hides it entirely) ---
It("regular user cannot see admin's private playlist", func() {
resp := doReqWithUser(regularUser, "getPlaylist", "id", adminPrivateID)
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error).ToNot(BeNil())
})
It("regular user cannot update admin's private playlist (not found)", func() {
resp := doReqWithUser(regularUser, "updatePlaylist",
"playlistId", adminPrivateID, "name", "Hacked")
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error).ToNot(BeNil())
})
It("regular user cannot delete admin's private playlist (not found)", func() {
resp := doReqWithUser(regularUser, "deletePlaylist", "id", adminPrivateID)
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error).ToNot(BeNil())
})
// --- Public playlist: regular user can see but cannot modify (authorization fail, code 50) ---
It("regular user can see admin's public playlist", func() {
resp := doReqWithUser(regularUser, "getPlaylist", "id", adminPublicID)
Expect(resp.Status).To(Equal(responses.StatusOK))
Expect(resp.Playlist.Name).To(Equal("Admin Public"))
})
It("regular user cannot update admin's public playlist", func() {
resp := doReqWithUser(regularUser, "updatePlaylist",
"playlistId", adminPublicID, "name", "Hacked")
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error).ToNot(BeNil())
Expect(resp.Error.Code).To(Equal(int32(50)))
})
It("regular user cannot add songs to admin's public playlist", func() {
resp := doReqWithUser(regularUser, "updatePlaylist",
"playlistId", adminPublicID, "songIdToAdd", songIDs[2])
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error.Code).To(Equal(int32(50)))
})
It("regular user cannot remove songs from admin's public playlist", func() {
resp := doReqWithUser(regularUser, "updatePlaylist",
"playlistId", adminPublicID, "songIndexToRemove", "0")
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error.Code).To(Equal(int32(50)))
})
It("regular user cannot delete admin's public playlist", func() {
resp := doReqWithUser(regularUser, "deletePlaylist", "id", adminPublicID)
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error.Code).To(Equal(int32(50)))
})
It("regular user cannot replace tracks on admin's public playlist via createPlaylist", func() {
resp := doReqWithUser(regularUser, "createPlaylist",
"playlistId", adminPublicID, "songId", songIDs[2])
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error).ToNot(BeNil())
})
// --- Regular user can manage their own playlists ---
It("regular user can update their own playlist", func() {
resp := doReqWithUser(regularUser, "updatePlaylist",
"playlistId", regularPlaylistID, "name", "My Updated Playlist")
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReqWithUser(regularUser, "getPlaylist", "id", regularPlaylistID)
Expect(resp.Playlist.Name).To(Equal("My Updated Playlist"))
})
It("regular user can add songs to their own playlist", func() {
resp := doReqWithUser(regularUser, "updatePlaylist",
"playlistId", regularPlaylistID, "songIdToAdd", songIDs[1])
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReqWithUser(regularUser, "getPlaylist", "id", regularPlaylistID)
Expect(resp.Playlist.SongCount).To(Equal(int32(2)))
})
It("regular user can delete their own playlist", func() {
resp := doReqWithUser(regularUser, "deletePlaylist", "id", regularPlaylistID)
Expect(resp.Status).To(Equal(responses.StatusOK))
})
// --- Admin can manage any user's playlists ---
It("admin can update any user's playlist", func() {
resp := doReqWithUser(regularUser, "createPlaylist", "name", "To Be Admin-Edited",
"songId", songIDs[0])
Expect(resp.Status).To(Equal(responses.StatusOK))
plsID := resp.Playlist.Id
resp = doReqWithUser(adminUser, "updatePlaylist",
"playlistId", plsID, "name", "Admin Edited")
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReqWithUser(adminUser, "getPlaylist", "id", plsID)
Expect(resp.Playlist.Name).To(Equal("Admin Edited"))
})
It("admin can delete any user's playlist", func() {
resp := doReqWithUser(regularUser, "createPlaylist", "name", "To Be Admin-Deleted",
"songId", songIDs[0])
Expect(resp.Status).To(Equal(responses.StatusOK))
plsID := resp.Playlist.Id
resp = doReqWithUser(adminUser, "deletePlaylist", "id", plsID)
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReqWithUser(adminUser, "getPlaylist", "id", plsID)
Expect(resp.Status).To(Equal(responses.StatusFailed))
})
// --- Verify admin's playlists are unchanged ---
It("admin's private playlist is unchanged after failed regular user operations", func() {
resp := doReqWithUser(adminUser, "getPlaylist", "id", adminPrivateID)
Expect(resp.Status).To(Equal(responses.StatusOK))
Expect(resp.Playlist.Name).To(Equal("Admin Private"))
Expect(resp.Playlist.SongCount).To(Equal(int32(2)))
})
It("admin's public playlist is unchanged after failed regular user operations", func() {
resp := doReqWithUser(adminUser, "getPlaylist", "id", adminPublicID)
Expect(resp.Status).To(Equal(responses.StatusOK))
Expect(resp.Playlist.Name).To(Equal("Admin Public"))
Expect(resp.Playlist.SongCount).To(Equal(int32(2)))
})
})
Describe("Smart Playlist Protection", Ordered, func() {
var smartPlaylistID string
var songID string
BeforeAll(func() {
setupTestDB()
// Look up a song ID for mutation tests
songs, err := ds.MediaFile(ctx).GetAll(model.QueryOptions{Sort: "title", Max: 1})
Expect(err).ToNot(HaveOccurred())
Expect(songs).ToNot(BeEmpty())
songID = songs[0].ID
// Insert a smart playlist directly into the DB
smartPls := &model.Playlist{
Name: "Smart Playlist",
OwnerID: adminUser.ID,
Public: false,
Rules: &criteria.Criteria{Expression: criteria.Contains{"title": ""}},
}
Expect(ds.Playlist(ctx).Put(smartPls)).To(Succeed())
smartPlaylistID = smartPls.ID
})
It("getPlaylist returns smart playlist with readonly flag and validUntil", func() {
resp := doReq("getPlaylist", "id", smartPlaylistID)
Expect(resp.Status).To(Equal(responses.StatusOK))
Expect(resp.Playlist.Name).To(Equal("Smart Playlist"))
Expect(resp.Playlist.OpenSubsonicPlaylist).ToNot(BeNil())
Expect(resp.Playlist.OpenSubsonicPlaylist.Readonly).To(BeTrue())
expectedValidUntil := time.Now().Add(conf.Server.SmartPlaylistRefreshDelay)
Expect(*resp.Playlist.OpenSubsonicPlaylist.ValidUntil).To(BeTemporally("~", expectedValidUntil, time.Second))
})
It("createPlaylist rejects replacing tracks on smart playlist", func() {
resp := doReq("createPlaylist", "playlistId", smartPlaylistID, "songId", songID)
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error).ToNot(BeNil())
Expect(resp.Error.Code).To(Equal(int32(50)))
})
It("updatePlaylist rejects adding songs to smart playlist", func() {
resp := doReq("updatePlaylist", "playlistId", smartPlaylistID,
"songIdToAdd", songID)
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error).ToNot(BeNil())
Expect(resp.Error.Code).To(Equal(int32(50)))
})
It("updatePlaylist rejects removing songs from smart playlist", func() {
resp := doReq("updatePlaylist", "playlistId", smartPlaylistID,
"songIndexToRemove", "0")
Expect(resp.Status).To(Equal(responses.StatusFailed))
Expect(resp.Error).ToNot(BeNil())
Expect(resp.Error.Code).To(Equal(int32(50)))
})
It("updatePlaylist allows renaming smart playlist", func() {
resp := doReq("updatePlaylist", "playlistId", smartPlaylistID,
"name", "Renamed Smart")
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReq("getPlaylist", "id", smartPlaylistID)
Expect(resp.Playlist.Name).To(Equal("Renamed Smart"))
})
It("updatePlaylist allows setting comment on smart playlist", func() {
resp := doReq("updatePlaylist", "playlistId", smartPlaylistID,
"comment", "Auto-generated playlist")
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReq("getPlaylist", "id", smartPlaylistID)
Expect(resp.Playlist.Comment).To(Equal("Auto-generated playlist"))
})
It("deletePlaylist can delete smart playlist", func() {
resp := doReq("deletePlaylist", "id", smartPlaylistID)
Expect(resp.Status).To(Equal(responses.StatusOK))
resp = doReq("getPlaylist", "id", smartPlaylistID)
Expect(resp.Status).To(Equal(responses.StatusFailed))
})
})
})
server/e2e/subsonic_scan_test.go
-2
View File
@@ -22,8 +22,6 @@ var _ = Describe("Scan Endpoints", func() {
})
It("startScan requires admin user", func() {
regularUser := createUser("user-2", "regular", "Regular User", false)
resp := doReqWithUser(regularUser, "startScan")
Expect(resp.Status).To(Equal(responses.StatusFailed))
server/nativeapi/native_api.go
+11 -10
View File
@@ -14,6 +14,7 @@ import (
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/core"
"github.com/navidrome/navidrome/core/metrics"
playlistsvc "github.com/navidrome/navidrome/core/playlists"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
@@ -37,7 +38,7 @@ type Router struct {
http.Handler
ds model.DataStore
share core.Share
playlists core.Playlists
playlists playlistsvc.Playlists
insights metrics.Insights
libs core.Library
users core.User
@@ -45,7 +46,7 @@ type Router struct {
pluginManager PluginManager
}
func New(ds model.DataStore, share core.Share, playlists core.Playlists, insights metrics.Insights, libraryService core.Library, userService core.User, maintenance core.Maintenance, pluginManager PluginManager) *Router {
func New(ds model.DataStore, share core.Share, playlists playlistsvc.Playlists, insights metrics.Insights, libraryService core.Library, userService core.User, maintenance core.Maintenance, pluginManager PluginManager) *Router {
r := &Router{ds: ds, share: share, playlists: playlists, insights: insights, libs: libraryService, users: userService, maintenance: maintenance, pluginManager: pluginManager}
r.Handler = r.routes()
return r
@@ -121,7 +122,7 @@ func (api *Router) RX(r chi.Router, pathPrefix string, constructor rest.Reposito
func (api *Router) addPlaylistRoute(r chi.Router) {
constructor := func(ctx context.Context) rest.Repository {
return api.ds.Resource(ctx, model.Playlist{})
return api.playlists.NewRepository(ctx)
}
r.Route("/playlist", func(r chi.Router) {
@@ -146,26 +147,26 @@ func (api *Router) addPlaylistRoute(r chi.Router) {
func (api *Router) addPlaylistTrackRoute(r chi.Router) {
r.Route("/playlist/{playlistId}/tracks", func(r chi.Router) {
r.Get("/", func(w http.ResponseWriter, r *http.Request) {
getPlaylist(api.ds)(w, r)
getPlaylist(api.playlists)(w, r)
})
r.With(server.URLParamsMiddleware).Route("/", func(r chi.Router) {
r.Delete("/", func(w http.ResponseWriter, r *http.Request) {
deleteFromPlaylist(api.ds)(w, r)
deleteFromPlaylist(api.playlists)(w, r)
})
r.Post("/", func(w http.ResponseWriter, r *http.Request) {
addToPlaylist(api.ds)(w, r)
addToPlaylist(api.playlists)(w, r)
})
})
r.Route("/{id}", func(r chi.Router) {
r.Use(server.URLParamsMiddleware)
r.Get("/", func(w http.ResponseWriter, r *http.Request) {
getPlaylistTrack(api.ds)(w, r)
getPlaylistTrack(api.playlists)(w, r)
})
r.Put("/", func(w http.ResponseWriter, r *http.Request) {
reorderItem(api.ds)(w, r)
reorderItem(api.playlists)(w, r)
})
r.Delete("/", func(w http.ResponseWriter, r *http.Request) {
deleteFromPlaylist(api.ds)(w, r)
deleteFromPlaylist(api.playlists)(w, r)
})
})
})
@@ -173,7 +174,7 @@ func (api *Router) addPlaylistTrackRoute(r chi.Router) {
func (api *Router) addSongPlaylistsRoute(r chi.Router) {
r.With(server.URLParamsMiddleware).Get("/song/{id}/playlists", func(w http.ResponseWriter, r *http.Request) {
getSongPlaylists(api.ds)(w, r)
getSongPlaylists(api.playlists)(w, r)
})
}
server/nativeapi/playlists.go
+33 -37
View File
@@ -11,7 +11,7 @@ import (
"github.com/deluan/rest"
"github.com/go-chi/chi/v5"
"github.com/navidrome/navidrome/core"
"github.com/navidrome/navidrome/core/playlists"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/utils/req"
@@ -19,10 +19,10 @@ import (
type restHandler = func(rest.RepositoryConstructor, ...rest.Logger) http.HandlerFunc
func playlistTracksHandler(ds model.DataStore, handler restHandler, refreshSmartPlaylist func(*http.Request) bool) http.HandlerFunc {
func playlistTracksHandler(pls playlists.Playlists, handler restHandler, refreshSmartPlaylist func(*http.Request) bool) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
plsId := chi.URLParam(r, "playlistId")
tracks := ds.Playlist(r.Context()).Tracks(plsId, refreshSmartPlaylist(r))
tracks := pls.TracksRepository(r.Context(), plsId, refreshSmartPlaylist(r))
if tracks == nil {
http.Error(w, "not found", http.StatusNotFound)
return
@@ -31,27 +31,27 @@ func playlistTracksHandler(ds model.DataStore, handler restHandler, refreshSmart
}
}
func getPlaylist(ds model.DataStore) http.HandlerFunc {
handler := playlistTracksHandler(ds, rest.GetAll, func(r *http.Request) bool {
func getPlaylist(pls playlists.Playlists) http.HandlerFunc {
handler := playlistTracksHandler(pls, rest.GetAll, func(r *http.Request) bool {
return req.Params(r).Int64Or("_start", 0) == 0
})
return func(w http.ResponseWriter, r *http.Request) {
if strings.ToLower(r.Header.Get("accept")) == "audio/x-mpegurl" {
handleExportPlaylist(ds)(w, r)
handleExportPlaylist(pls)(w, r)
return
}
handler(w, r)
}
}
func getPlaylistTrack(ds model.DataStore) http.HandlerFunc {
return playlistTracksHandler(ds, rest.Get, func(*http.Request) bool { return true })
func getPlaylistTrack(pls playlists.Playlists) http.HandlerFunc {
return playlistTracksHandler(pls, rest.Get, func(*http.Request) bool { return true })
}
func createPlaylistFromM3U(playlists core.Playlists) http.HandlerFunc {
func createPlaylistFromM3U(pls playlists.Playlists) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
pls, err := playlists.ImportM3U(ctx, r.Body)
pl, err := pls.ImportM3U(ctx, r.Body)
if err != nil {
log.Error(r.Context(), "Error parsing playlist", err)
// TODO: consider returning StatusBadRequest for playlists that are malformed
@@ -59,7 +59,7 @@ func createPlaylistFromM3U(playlists core.Playlists) http.HandlerFunc {
return
}
w.WriteHeader(http.StatusCreated)
_, err = w.Write([]byte(pls.ToM3U8())) //nolint:gosec
_, err = w.Write([]byte(pl.ToM3U8())) //nolint:gosec
if err != nil {
log.Error(ctx, "Error sending m3u contents", err)
http.Error(w, err.Error(), http.StatusInternalServerError)
@@ -68,45 +68,41 @@ func createPlaylistFromM3U(playlists core.Playlists) http.HandlerFunc {
}
}
func handleExportPlaylist(ds model.DataStore) http.HandlerFunc {
func handleExportPlaylist(pls playlists.Playlists) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
plsRepo := ds.Playlist(ctx)
plsId := chi.URLParam(r, "playlistId")
pls, err := plsRepo.GetWithTracks(plsId, true, false)
playlist, err := pls.GetWithTracks(ctx, plsId)
if errors.Is(err, model.ErrNotFound) {
log.Warn(r.Context(), "Playlist not found", "playlistId", plsId)
log.Warn(ctx, "Playlist not found", "playlistId", plsId)
http.Error(w, "not found", http.StatusNotFound)
return
}
if err != nil {
log.Error(r.Context(), "Error retrieving the playlist", "playlistId", plsId, err)
log.Error(ctx, "Error retrieving the playlist", "playlistId", plsId, err)
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
log.Debug(ctx, "Exporting playlist as M3U", "playlistId", plsId, "name", pls.Name)
log.Debug(ctx, "Exporting playlist as M3U", "playlistId", plsId, "name", playlist.Name)
w.Header().Set("Content-Type", "audio/x-mpegurl")
disposition := fmt.Sprintf("attachment; filename=\"%s.m3u\"", pls.Name)
disposition := fmt.Sprintf("attachment; filename=\"%s.m3u\"", playlist.Name)
w.Header().Set("Content-Disposition", disposition)
_, err = w.Write([]byte(pls.ToM3U8())) //nolint:gosec
_, err = w.Write([]byte(playlist.ToM3U8())) //nolint:gosec
if err != nil {
log.Error(ctx, "Error sending playlist", "name", pls.Name)
log.Error(ctx, "Error sending playlist", "name", playlist.Name)
return
}
}
}
func deleteFromPlaylist(ds model.DataStore) http.HandlerFunc {
func deleteFromPlaylist(pls playlists.Playlists) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
p := req.Params(r)
playlistId, _ := p.String(":playlistId")
ids, _ := p.Strings("id")
err := ds.WithTxImmediate(func(tx model.DataStore) error {
tracksRepo := tx.Playlist(r.Context()).Tracks(playlistId, true)
return tracksRepo.Delete(ids...)
})
err := pls.RemoveTracks(r.Context(), playlistId, ids)
if len(ids) == 1 && errors.Is(err, model.ErrNotFound) {
log.Warn(r.Context(), "Track not found in playlist", "playlistId", playlistId, "id", ids[0])
http.Error(w, "not found", http.StatusNotFound)
@@ -121,7 +117,7 @@ func deleteFromPlaylist(ds model.DataStore) http.HandlerFunc {
}
}
func addToPlaylist(ds model.DataStore) http.HandlerFunc {
func addToPlaylist(pls playlists.Playlists) http.HandlerFunc {
type addTracksPayload struct {
Ids []string `json:"ids"`
AlbumIds []string `json:"albumIds"`
@@ -130,6 +126,7 @@ func addToPlaylist(ds model.DataStore) http.HandlerFunc {
}
return func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
p := req.Params(r)
playlistId, _ := p.String(":playlistId")
var payload addTracksPayload
@@ -138,24 +135,23 @@ func addToPlaylist(ds model.DataStore) http.HandlerFunc {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
tracksRepo := ds.Playlist(r.Context()).Tracks(playlistId, true)
count, c := 0, 0
if c, err = tracksRepo.Add(payload.Ids); err != nil {
if c, err = pls.AddTracks(ctx, playlistId, payload.Ids); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
count += c
if c, err = tracksRepo.AddAlbums(payload.AlbumIds); err != nil {
if c, err = pls.AddAlbums(ctx, playlistId, payload.AlbumIds); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
count += c
if c, err = tracksRepo.AddArtists(payload.ArtistIds); err != nil {
if c, err = pls.AddArtists(ctx, playlistId, payload.ArtistIds); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
count += c
if c, err = tracksRepo.AddDiscs(payload.Discs); err != nil {
if c, err = pls.AddDiscs(ctx, playlistId, payload.Discs); err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
@@ -169,12 +165,13 @@ func addToPlaylist(ds model.DataStore) http.HandlerFunc {
}
}
func reorderItem(ds model.DataStore) http.HandlerFunc {
func reorderItem(pls playlists.Playlists) http.HandlerFunc {
type reorderPayload struct {
InsertBefore string `json:"insert_before"`
}
return func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
p := req.Params(r)
playlistId, _ := p.String(":playlistId")
id := p.IntOr(":id", 0)
@@ -193,9 +190,8 @@ func reorderItem(ds model.DataStore) http.HandlerFunc {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
tracksRepo := ds.Playlist(r.Context()).Tracks(playlistId, true)
err = tracksRepo.Reorder(id, newPos)
if errors.Is(err, rest.ErrPermissionDenied) {
err = pls.ReorderTrack(ctx, playlistId, id, newPos)
if errors.Is(err, model.ErrNotAuthorized) {
http.Error(w, err.Error(), http.StatusForbidden)
return
}
@@ -211,11 +207,11 @@ func reorderItem(ds model.DataStore) http.HandlerFunc {
}
}
func getSongPlaylists(ds model.DataStore) http.HandlerFunc {
func getSongPlaylists(svc playlists.Playlists) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
p := req.Params(r)
trackId, _ := p.String(":id")
playlists, err := ds.Playlist(r.Context()).GetPlaylists(trackId)
playlists, err := svc.GetPlaylists(r.Context(), trackId)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
server/nativeapi/playlists_test.go
+18 -9
View File
@@ -1,6 +1,7 @@
package nativeapi
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
@@ -11,6 +12,7 @@ import (
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/core/playlists"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/server"
"github.com/navidrome/navidrome/tests"
@@ -48,11 +50,19 @@ func (m *mockPlaylistTrackRepo) Read(id string) (any, error) {
return nil, rest.ErrNotFound
}
type mockPlaylistsService struct {
playlists.Playlists
tracksRepo rest.Repository
}
func (m *mockPlaylistsService) TracksRepository(_ context.Context, _ string, _ bool) rest.Repository {
return m.tracksRepo
}
var _ = Describe("Playlist Tracks Endpoint", func() {
var (
router http.Handler
ds *tests.MockDataStore
plsRepo *tests.MockPlaylistRepo
plsSvc *mockPlaylistsService
userRepo *tests.MockedUserRepo
w *httptest.ResponseRecorder
)
@@ -61,11 +71,10 @@ var _ = Describe("Playlist Tracks Endpoint", func() {
DeferCleanup(configtest.SetupConfig())
conf.Server.SessionTimeout = time.Minute
plsRepo = &tests.MockPlaylistRepo{}
plsSvc = &mockPlaylistsService{}
userRepo = tests.CreateMockUserRepo()
ds = &tests.MockDataStore{
MockedPlaylist: plsRepo,
ds := &tests.MockDataStore{
MockedUser: userRepo,
MockedProperty: &tests.MockedPropertyRepo{},
}
@@ -82,7 +91,7 @@ var _ = Describe("Playlist Tracks Endpoint", func() {
err := userRepo.Put(&testUser)
Expect(err).ToNot(HaveOccurred())
nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil)
nativeRouter := New(ds, nil, plsSvc, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil)
router = server.JWTVerifier(nativeRouter)
w = httptest.NewRecorder()
})
@@ -105,7 +114,7 @@ var _ = Describe("Playlist Tracks Endpoint", func() {
})
It("returns tracks when playlist exists", func() {
plsRepo.TracksReturn = &mockPlaylistTrackRepo{
plsSvc.tracksRepo = &mockPlaylistTrackRepo{
tracks: model.PlaylistTracks{
{ID: "1", MediaFileID: "mf-1", PlaylistID: "pls-1"},
{ID: "2", MediaFileID: "mf-2", PlaylistID: "pls-1"},
@@ -135,7 +144,7 @@ var _ = Describe("Playlist Tracks Endpoint", func() {
})
It("returns the track when playlist exists", func() {
plsRepo.TracksReturn = &mockPlaylistTrackRepo{
plsSvc.tracksRepo = &mockPlaylistTrackRepo{
tracks: model.PlaylistTracks{
{ID: "1", MediaFileID: "mf-1", PlaylistID: "pls-1"},
},
@@ -154,7 +163,7 @@ var _ = Describe("Playlist Tracks Endpoint", func() {
})
It("returns 404 when track does not exist in playlist", func() {
plsRepo.TracksReturn = &mockPlaylistTrackRepo{
plsSvc.tracksRepo = &mockPlaylistTrackRepo{
tracks: model.PlaylistTracks{},
}
server/subsonic/api.go
+5 -2
View File
@@ -16,6 +16,7 @@ import (
"github.com/navidrome/navidrome/core/external"
"github.com/navidrome/navidrome/core/metrics"
"github.com/navidrome/navidrome/core/playback"
playlistsvc "github.com/navidrome/navidrome/core/playlists"
"github.com/navidrome/navidrome/core/scrobbler"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
@@ -40,7 +41,7 @@ type Router struct {
archiver core.Archiver
players core.Players
provider external.Provider
playlists core.Playlists
playlists playlistsvc.Playlists
scanner model.Scanner
broker events.Broker
scrobbler scrobbler.PlayTracker
@@ -51,7 +52,7 @@ type Router struct {
func New(ds model.DataStore, artwork artwork.Artwork, streamer core.MediaStreamer, archiver core.Archiver,
players core.Players, provider external.Provider, scanner model.Scanner, broker events.Broker,
playlists core.Playlists, scrobbler scrobbler.PlayTracker, share core.Share, playback playback.PlaybackServer,
playlists playlistsvc.Playlists, scrobbler scrobbler.PlayTracker, share core.Share, playback playback.PlaybackServer,
metrics metrics.Metrics,
) *Router {
r := &Router{
@@ -290,6 +291,8 @@ func mapToSubsonicError(err error) subError {
err = newError(responses.ErrorGeneric, err.Error())
case errors.Is(err, model.ErrNotFound):
err = newError(responses.ErrorDataNotFound, "data not found")
case errors.Is(err, model.ErrNotAuthorized):
err = newError(responses.ErrorAuthorizationFail)
default:
err = newError(responses.ErrorGeneric, fmt.Sprintf("Internal Server Error: %s", err))
}
server/subsonic/playlists.go
+4 -32
View File
@@ -19,7 +19,7 @@ import (
func (api *Router) GetPlaylists(r *http.Request) (*responses.Subsonic, error) {
ctx := r.Context()
allPls, err := api.ds.Playlist(ctx).GetAll(model.QueryOptions{Sort: "name"})
allPls, err := api.playlists.GetAll(ctx, model.QueryOptions{Sort: "name"})
if err != nil {
log.Error(r, err)
return nil, err
@@ -42,7 +42,7 @@ func (api *Router) GetPlaylist(r *http.Request) (*responses.Subsonic, error) {
}
func (api *Router) getPlaylist(ctx context.Context, id string) (*responses.Subsonic, error) {
pls, err := api.ds.Playlist(ctx).GetWithTracks(id, true, false)
pls, err := api.playlists.GetWithTracks(ctx, id)
if errors.Is(err, model.ErrNotFound) {
log.Error(ctx, err.Error(), "id", id)
return nil, newError(responses.ErrorDataNotFound, "playlist not found")
@@ -60,34 +60,6 @@ func (api *Router) getPlaylist(ctx context.Context, id string) (*responses.Subso
return response, nil
}
func (api *Router) create(ctx context.Context, playlistId, name string, ids []string) (string, error) {
err := api.ds.WithTxImmediate(func(tx model.DataStore) error {
owner := getUser(ctx)
var pls *model.Playlist
var err error
if playlistId != "" {
pls, err = tx.Playlist(ctx).Get(playlistId)
if err != nil {
return err
}
if owner.ID != pls.OwnerID {
return model.ErrNotAuthorized
}
} else {
pls = &model.Playlist{Name: name}
pls.OwnerID = owner.ID
}
pls.Tracks = nil
pls.AddMediaFilesByID(ids)
err = tx.Playlist(ctx).Put(pls)
playlistId = pls.ID
return err
})
return playlistId, err
}
func (api *Router) CreatePlaylist(r *http.Request) (*responses.Subsonic, error) {
ctx := r.Context()
p := req.Params(r)
@@ -97,7 +69,7 @@ func (api *Router) CreatePlaylist(r *http.Request) (*responses.Subsonic, error)
if playlistId == "" && name == "" {
return nil, errors.New("required parameter name is missing")
}
id, err := api.create(ctx, playlistId, name, songIds)
id, err := api.playlists.Create(ctx, playlistId, name, songIds)
if err != nil {
log.Error(r, err)
return nil, err
@@ -111,7 +83,7 @@ func (api *Router) DeletePlaylist(r *http.Request) (*responses.Subsonic, error)
if err != nil {
return nil, err
}
err = api.ds.Playlist(r.Context()).Delete(id)
err = api.playlists.Delete(r.Context(), id)
if errors.Is(err, model.ErrNotAuthorized) {
return nil, newError(responses.ErrorAuthorizationFail)
}
server/subsonic/playlists_test.go
+3 -3
View File
@@ -5,7 +5,7 @@ import (
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/core"
"github.com/navidrome/navidrome/core/playlists"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/criteria"
"github.com/navidrome/navidrome/model/request"
@@ -14,7 +14,7 @@ import (
. "github.com/onsi/gomega"
)
var _ core.Playlists = (*fakePlaylists)(nil)
var _ playlists.Playlists = (*fakePlaylists)(nil)
var _ = Describe("buildPlaylist", func() {
var router *Router
@@ -272,7 +272,7 @@ var _ = Describe("UpdatePlaylist", func() {
})
type fakePlaylists struct {
core.Playlists
playlists.Playlists
lastPlaylistID string
lastName *string
lastComment *string