mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-18 07:30:29 +00:00
1f3034f022
* 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.
488 lines
18 KiB
Go
488 lines
18 KiB
Go
package playlists_test
|
|
|
|
import (
|
|
"context"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
|
|
"github.com/navidrome/navidrome/conf"
|
|
"github.com/navidrome/navidrome/conf/configtest"
|
|
"github.com/navidrome/navidrome/core/artwork"
|
|
"github.com/navidrome/navidrome/core/playlists"
|
|
"github.com/navidrome/navidrome/model"
|
|
"github.com/navidrome/navidrome/model/criteria"
|
|
"github.com/navidrome/navidrome/model/request"
|
|
"github.com/navidrome/navidrome/tests"
|
|
. "github.com/onsi/ginkgo/v2"
|
|
. "github.com/onsi/gomega"
|
|
)
|
|
|
|
var _ = Describe("Playlists", func() {
|
|
var ds *tests.MockDataStore
|
|
var ps playlists.Playlists
|
|
var mockPlsRepo *tests.MockPlaylistRepo
|
|
ctx := context.Background()
|
|
|
|
BeforeEach(func() {
|
|
mockPlsRepo = tests.CreateMockPlaylistRepo()
|
|
ds = &tests.MockDataStore{
|
|
MockedPlaylist: mockPlsRepo,
|
|
MockedLibrary: &tests.MockLibraryRepo{},
|
|
}
|
|
ctx = request.WithUser(ctx, model.User{ID: "123"})
|
|
})
|
|
|
|
Describe("Delete", func() {
|
|
var mockTracks *tests.MockPlaylistTrackRepo
|
|
|
|
BeforeEach(func() {
|
|
mockTracks = &tests.MockPlaylistTrackRepo{AddCount: 3}
|
|
mockPlsRepo.Data = map[string]*model.Playlist{
|
|
"pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"},
|
|
}
|
|
mockPlsRepo.TracksRepo = mockTracks
|
|
ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds))
|
|
})
|
|
|
|
It("allows owner to delete their playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.Delete(ctx, "pls-1")
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(mockPlsRepo.Deleted).To(ContainElement("pls-1"))
|
|
})
|
|
|
|
It("allows admin to delete any playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "admin-1", IsAdmin: true})
|
|
err := ps.Delete(ctx, "pls-1")
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(mockPlsRepo.Deleted).To(ContainElement("pls-1"))
|
|
})
|
|
|
|
It("denies non-owner, non-admin from deleting", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "other-user", IsAdmin: false})
|
|
err := ps.Delete(ctx, "pls-1")
|
|
Expect(err).To(MatchError(model.ErrNotAuthorized))
|
|
Expect(mockPlsRepo.Deleted).To(BeEmpty())
|
|
})
|
|
|
|
It("returns error when playlist not found", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.Delete(ctx, "nonexistent")
|
|
Expect(err).To(Equal(model.ErrNotFound))
|
|
})
|
|
})
|
|
|
|
Describe("Tracks", func() {
|
|
var mockTracks *tests.MockPlaylistTrackRepo
|
|
|
|
BeforeEach(func() {
|
|
mockTracks = &tests.MockPlaylistTrackRepo{}
|
|
mockPlsRepo.Data = map[string]*model.Playlist{
|
|
"pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"},
|
|
}
|
|
mockPlsRepo.TracksRepo = mockTracks
|
|
ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds))
|
|
})
|
|
|
|
It("returns the playlist's track repository", func() {
|
|
Expect(ps.Tracks(ctx, "pls-1")).To(BeIdenticalTo(mockTracks))
|
|
})
|
|
|
|
It("returns ErrNotFound for an unknown or invisible playlist", func() {
|
|
_, err := ps.Tracks(ctx, "nonexistent")
|
|
Expect(err).To(MatchError(model.ErrNotFound))
|
|
})
|
|
})
|
|
|
|
Describe("Create", func() {
|
|
BeforeEach(func() {
|
|
mockPlsRepo.Data = map[string]*model.Playlist{
|
|
"pls-1": {ID: "pls-1", Name: "Existing", OwnerID: "user-1"},
|
|
"pls-2": {ID: "pls-2", Name: "Other's", OwnerID: "other-user"},
|
|
"pls-smart": {ID: "pls-smart", Name: "Smart", OwnerID: "user-1",
|
|
Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}},
|
|
"pls-synced": {ID: "pls-synced", Name: "Synced", OwnerID: "user-1", Sync: true},
|
|
"pls-synced-other": {ID: "pls-synced-other", Name: "Other's Synced", OwnerID: "other-user", Sync: true, Public: true},
|
|
}
|
|
ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds))
|
|
})
|
|
|
|
It("creates a new playlist with owner set from context", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
id, err := ps.Create(ctx, "", "New Playlist", []string{"song-1", "song-2"})
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(id).ToNot(BeEmpty())
|
|
Expect(mockPlsRepo.Last.Name).To(Equal("New Playlist"))
|
|
Expect(mockPlsRepo.Last.OwnerID).To(Equal("user-1"))
|
|
})
|
|
|
|
It("replaces tracks on existing playlist when owner matches", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
id, err := ps.Create(ctx, "pls-1", "", []string{"song-3"})
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(id).To(Equal("pls-1"))
|
|
Expect(mockPlsRepo.Last.Tracks).To(HaveLen(1))
|
|
})
|
|
|
|
It("allows admin to replace tracks on any playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "admin-1", IsAdmin: true})
|
|
id, err := ps.Create(ctx, "pls-2", "", []string{"song-3"})
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(id).To(Equal("pls-2"))
|
|
})
|
|
|
|
It("denies non-owner, non-admin from replacing tracks on existing playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
_, err := ps.Create(ctx, "pls-2", "", []string{"song-3"})
|
|
Expect(err).To(MatchError(model.ErrNotAuthorized))
|
|
})
|
|
|
|
It("returns error when existing playlistId not found", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
_, err := ps.Create(ctx, "nonexistent", "", []string{"song-1"})
|
|
Expect(err).To(Equal(model.ErrNotFound))
|
|
})
|
|
|
|
It("denies replacing tracks on a smart playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
_, err := ps.Create(ctx, "pls-smart", "", []string{"song-1"})
|
|
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
|
|
})
|
|
|
|
It("denies replacing tracks on a synced playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
_, err := ps.Create(ctx, "pls-synced", "", []string{"song-1"})
|
|
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
|
|
})
|
|
|
|
It("denies a non-owner with authorization, not a conflict, on a public synced playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
_, err := ps.Create(ctx, "pls-synced-other", "", []string{"song-1"})
|
|
Expect(err).To(MatchError(model.ErrNotAuthorized))
|
|
})
|
|
})
|
|
|
|
Describe("Update", func() {
|
|
var mockTracks *tests.MockPlaylistTrackRepo
|
|
|
|
BeforeEach(func() {
|
|
mockTracks = &tests.MockPlaylistTrackRepo{AddCount: 2}
|
|
mockPlsRepo.Data = map[string]*model.Playlist{
|
|
"pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"},
|
|
"pls-other": {ID: "pls-other", Name: "Other's", OwnerID: "other-user"},
|
|
"pls-smart": {ID: "pls-smart", Name: "Smart", OwnerID: "user-1",
|
|
Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}},
|
|
"pls-synced": {ID: "pls-synced", Name: "Synced", OwnerID: "user-1", Sync: true},
|
|
}
|
|
mockPlsRepo.TracksRepo = mockTracks
|
|
ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds))
|
|
})
|
|
|
|
It("allows owner to update their playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.Update(ctx, "pls-1", new("Updated Name"), nil, nil, nil, nil)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
})
|
|
|
|
It("allows admin to update any playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "admin-1", IsAdmin: true})
|
|
err := ps.Update(ctx, "pls-other", new("Updated Name"), nil, nil, nil, nil)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
})
|
|
|
|
It("denies non-owner, non-admin from updating", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "other-user", IsAdmin: false})
|
|
err := ps.Update(ctx, "pls-1", new("Updated Name"), nil, nil, nil, nil)
|
|
Expect(err).To(MatchError(model.ErrNotAuthorized))
|
|
})
|
|
|
|
It("returns error when playlist not found", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.Update(ctx, "nonexistent", new("Updated Name"), nil, nil, nil, nil)
|
|
Expect(err).To(Equal(model.ErrNotFound))
|
|
})
|
|
|
|
It("denies adding tracks to a smart playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.Update(ctx, "pls-smart", nil, nil, nil, []string{"song-1"}, nil)
|
|
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
|
|
})
|
|
|
|
It("denies removing tracks from a smart playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.Update(ctx, "pls-smart", nil, nil, nil, nil, []int{0})
|
|
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
|
|
})
|
|
|
|
It("allows metadata updates on a smart playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.Update(ctx, "pls-smart", new("Updated Smart"), nil, nil, nil, nil)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
})
|
|
|
|
It("denies adding tracks to a synced playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.Update(ctx, "pls-synced", nil, nil, nil, []string{"song-1"}, nil)
|
|
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
|
|
})
|
|
|
|
It("allows metadata updates on a synced playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.Update(ctx, "pls-synced", new("Renamed Synced"), nil, nil, nil, nil)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
})
|
|
})
|
|
|
|
Describe("AddTracks", func() {
|
|
var mockTracks *tests.MockPlaylistTrackRepo
|
|
|
|
BeforeEach(func() {
|
|
mockTracks = &tests.MockPlaylistTrackRepo{AddCount: 2}
|
|
mockPlsRepo.Data = map[string]*model.Playlist{
|
|
"pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"},
|
|
"pls-smart": {ID: "pls-smart", Name: "Smart", OwnerID: "user-1",
|
|
Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}},
|
|
"pls-other": {ID: "pls-other", Name: "Other's", OwnerID: "other-user"},
|
|
"pls-synced": {ID: "pls-synced", Name: "Synced", OwnerID: "user-1", Sync: true},
|
|
}
|
|
mockPlsRepo.TracksRepo = mockTracks
|
|
ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds))
|
|
})
|
|
|
|
It("allows owner to add tracks", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
count, err := ps.AddTracks(ctx, "pls-1", []string{"song-1", "song-2"})
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(count).To(Equal(2))
|
|
Expect(mockTracks.AddedIds).To(ConsistOf("song-1", "song-2"))
|
|
})
|
|
|
|
It("allows admin to add tracks to any playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "admin-1", IsAdmin: true})
|
|
count, err := ps.AddTracks(ctx, "pls-other", []string{"song-1"})
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(count).To(Equal(2))
|
|
})
|
|
|
|
It("denies non-owner, non-admin", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "other-user", IsAdmin: false})
|
|
_, err := ps.AddTracks(ctx, "pls-1", []string{"song-1"})
|
|
Expect(err).To(MatchError(model.ErrNotAuthorized))
|
|
})
|
|
|
|
It("denies editing smart playlists", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
_, err := ps.AddTracks(ctx, "pls-smart", []string{"song-1"})
|
|
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
|
|
})
|
|
|
|
It("denies editing synced playlists", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
_, err := ps.AddTracks(ctx, "pls-synced", []string{"song-1"})
|
|
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
|
|
})
|
|
|
|
It("returns error when playlist not found", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
_, err := ps.AddTracks(ctx, "nonexistent", []string{"song-1"})
|
|
Expect(err).To(Equal(model.ErrNotFound))
|
|
})
|
|
})
|
|
|
|
Describe("RemoveTracks", func() {
|
|
var mockTracks *tests.MockPlaylistTrackRepo
|
|
|
|
BeforeEach(func() {
|
|
mockTracks = &tests.MockPlaylistTrackRepo{}
|
|
mockPlsRepo.Data = map[string]*model.Playlist{
|
|
"pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"},
|
|
"pls-smart": {ID: "pls-smart", Name: "Smart", OwnerID: "user-1",
|
|
Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}},
|
|
}
|
|
mockPlsRepo.TracksRepo = mockTracks
|
|
ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds))
|
|
})
|
|
|
|
It("allows owner to remove tracks", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.RemoveTracks(ctx, "pls-1", []string{"track-1", "track-2"})
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(mockTracks.DeletedIds).To(ConsistOf("track-1", "track-2"))
|
|
})
|
|
|
|
It("denies on smart playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.RemoveTracks(ctx, "pls-smart", []string{"track-1"})
|
|
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
|
|
})
|
|
|
|
It("denies non-owner", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "other-user", IsAdmin: false})
|
|
err := ps.RemoveTracks(ctx, "pls-1", []string{"track-1"})
|
|
Expect(err).To(MatchError(model.ErrNotAuthorized))
|
|
})
|
|
})
|
|
|
|
Describe("ReorderTrack", func() {
|
|
var mockTracks *tests.MockPlaylistTrackRepo
|
|
|
|
BeforeEach(func() {
|
|
mockTracks = &tests.MockPlaylistTrackRepo{}
|
|
mockPlsRepo.Data = map[string]*model.Playlist{
|
|
"pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"},
|
|
"pls-smart": {ID: "pls-smart", Name: "Smart", OwnerID: "user-1",
|
|
Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}},
|
|
}
|
|
mockPlsRepo.TracksRepo = mockTracks
|
|
ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds))
|
|
})
|
|
|
|
It("allows owner to reorder", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.ReorderTrack(ctx, "pls-1", 1, 3)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(mockTracks.Reordered).To(BeTrue())
|
|
})
|
|
|
|
It("denies on smart playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.ReorderTrack(ctx, "pls-smart", 1, 3)
|
|
Expect(err).To(MatchError(model.ErrPlaylistNotEditable))
|
|
})
|
|
})
|
|
|
|
Describe("SetImage", func() {
|
|
var tmpDir string
|
|
|
|
BeforeEach(func() {
|
|
DeferCleanup(configtest.SetupConfig())
|
|
tmpDir = GinkgoT().TempDir()
|
|
conf.Server.DataFolder = conf.NewDir(tmpDir)
|
|
|
|
mockPlsRepo.Data = map[string]*model.Playlist{
|
|
"pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"},
|
|
"pls-other": {ID: "pls-other", Name: "Other's", OwnerID: "other-user"},
|
|
}
|
|
ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds))
|
|
})
|
|
|
|
It("saves image file and updates UploadedImage", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
reader := strings.NewReader("fake image data")
|
|
err := ps.SetImage(ctx, "pls-1", reader, ".jpg")
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
Expect(mockPlsRepo.Last.UploadedImage).To(Equal("pls-1_my_playlist.jpg"))
|
|
absPath := filepath.Join(tmpDir, "artwork", "playlist", "pls-1_my_playlist.jpg")
|
|
data, err := os.ReadFile(absPath)
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(string(data)).To(Equal("fake image data"))
|
|
})
|
|
|
|
It("removes old image when replacing", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
|
|
// Upload first image
|
|
err := ps.SetImage(ctx, "pls-1", strings.NewReader("first"), ".png")
|
|
Expect(err).ToNot(HaveOccurred())
|
|
oldPath := filepath.Join(tmpDir, "artwork", "playlist", "pls-1_my_playlist.png")
|
|
Expect(oldPath).To(BeAnExistingFile())
|
|
|
|
// Upload replacement image
|
|
err = ps.SetImage(ctx, "pls-1", strings.NewReader("second"), ".jpg")
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(oldPath).ToNot(BeAnExistingFile())
|
|
newPath := filepath.Join(tmpDir, "artwork", "playlist", "pls-1_my_playlist.jpg")
|
|
Expect(newPath).To(BeAnExistingFile())
|
|
})
|
|
|
|
It("allows admin to set image on any playlist", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "admin-1", IsAdmin: true})
|
|
err := ps.SetImage(ctx, "pls-other", strings.NewReader("data"), ".jpg")
|
|
Expect(err).ToNot(HaveOccurred())
|
|
})
|
|
|
|
It("denies non-owner", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "other-user", IsAdmin: false})
|
|
err := ps.SetImage(ctx, "pls-1", strings.NewReader("data"), ".jpg")
|
|
Expect(err).To(MatchError(model.ErrNotAuthorized))
|
|
})
|
|
|
|
It("returns error when playlist not found", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.SetImage(ctx, "nonexistent", strings.NewReader("data"), ".jpg")
|
|
Expect(err).To(Equal(model.ErrNotFound))
|
|
})
|
|
})
|
|
|
|
Describe("RemoveImage", func() {
|
|
var tmpDir string
|
|
|
|
BeforeEach(func() {
|
|
DeferCleanup(configtest.SetupConfig())
|
|
tmpDir = GinkgoT().TempDir()
|
|
conf.Server.DataFolder = conf.NewDir(tmpDir)
|
|
|
|
// Create a real image file on disk
|
|
imgDir := filepath.Join(tmpDir, "artwork", "playlist")
|
|
Expect(os.MkdirAll(imgDir, 0755)).To(Succeed())
|
|
Expect(os.WriteFile(filepath.Join(imgDir, "pls-1.jpg"), []byte("img data"), 0600)).To(Succeed())
|
|
|
|
mockPlsRepo.Data = map[string]*model.Playlist{
|
|
"pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1", UploadedImage: "pls-1.jpg"},
|
|
"pls-empty": {ID: "pls-empty", Name: "No Cover", OwnerID: "user-1"},
|
|
"pls-other": {ID: "pls-other", Name: "Other's", OwnerID: "other-user"},
|
|
}
|
|
ps = playlists.NewPlaylists(ds, artwork.NewUploader(ds))
|
|
})
|
|
|
|
It("removes file and clears UploadedImage", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.RemoveImage(ctx, "pls-1")
|
|
Expect(err).ToNot(HaveOccurred())
|
|
|
|
Expect(mockPlsRepo.Last.UploadedImage).To(BeEmpty())
|
|
absPath := filepath.Join(tmpDir, "artwork", "playlist", "pls-1.jpg")
|
|
Expect(absPath).ToNot(BeAnExistingFile())
|
|
})
|
|
|
|
It("succeeds even if playlist has no image", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.RemoveImage(ctx, "pls-empty")
|
|
Expect(err).ToNot(HaveOccurred())
|
|
Expect(mockPlsRepo.Last.UploadedImage).To(BeEmpty())
|
|
})
|
|
|
|
It("clears the resolved artwork state and re-queues after removing an upload", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
Expect(ds.Artwork(ctx).PutItemArtwork(&model.ItemArtwork{
|
|
ItemKind: "pl", ItemID: "pls-1", Hash: "oldhash", Source: "upload",
|
|
})).To(Succeed())
|
|
|
|
Expect(ps.RemoveImage(ctx, "pls-1")).To(Succeed())
|
|
|
|
_, err := ds.Artwork(ctx).GetItemArtwork(model.KindPlaylistArtwork, "pls-1", model.ImageTypePrimary)
|
|
Expect(err).To(MatchError(model.ErrNotFound))
|
|
queued, _ := ds.ArtworkQueue(ctx).DequeueBatch(100)
|
|
Expect(queued).To(ContainElement(SatisfyAll(
|
|
HaveField("ItemKind", "pl"),
|
|
HaveField("ItemID", "pls-1"),
|
|
HaveField("Priority", model.ArtworkPriorityBump),
|
|
)))
|
|
})
|
|
|
|
It("denies non-owner", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "other-user", IsAdmin: false})
|
|
err := ps.RemoveImage(ctx, "pls-1")
|
|
Expect(err).To(MatchError(model.ErrNotAuthorized))
|
|
})
|
|
|
|
It("returns error when playlist not found", func() {
|
|
ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
|
|
err := ps.RemoveImage(ctx, "nonexistent")
|
|
Expect(err).To(Equal(model.ErrNotFound))
|
|
})
|
|
})
|
|
})
|