mirror of
https://github.com/navidrome/navidrome.git
synced 2026-11-15 06:38:50 +01:00
feat(jellyfin): favorites and rating write-back with library access control
Adds POST/DELETE handlers for /Users/{userId}/FavoriteItems/{itemId} and
/Users/{userId}/Items/{itemId}/Rating. A shared resolveAnnotated helper
probes album/artist/media file (mirroring getItem's order) and 404s before
writing if the user lacks access to the album/song's library; artists are
exempt since they span multiple libraries. Ratings are halved coming in
(Jellyfin 0-10 -> Navidrome 0-5) to match the doubling in dto.UserData.
Also teaches MockMediaFileRepo and MockArtistRepo's SetStar/SetRating (and
MockAlbumRepo's, previously a no-op) to actually mutate the backing data so
write-back can be asserted in tests.
This commit is contained in:
parent
7e26f1fa8c
commit
d28adb2676
6 changed files with 316 additions and 0 deletions
88
server/jellyfin/annotations.go
Normal file
88
server/jellyfin/annotations.go
Normal file
|
|
@ -0,0 +1,88 @@
|
|||
package jellyfin
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/server/jellyfin/dto"
|
||||
"github.com/navidrome/navidrome/utils/req"
|
||||
)
|
||||
|
||||
// resolveAnnotated finds which repo (album, artist or media file) owns id, mirroring getItem's
|
||||
// probe order and access control: albums and songs each belong to exactly one library and 404
|
||||
// when the current user can't access it, so an id can't be used to favorite/rate content outside
|
||||
// the user's libraries by guessing. Artists span multiple libraries (via library_artist), so —
|
||||
// same as getItem — there's no single LibraryID to check; access for them relies on list-time
|
||||
// scoping and the persistence layer's defense-in-depth.
|
||||
// ok is false, and the response has already been written, when the id doesn't resolve to any
|
||||
// entity or access is denied; callers must return immediately without writing the annotation.
|
||||
func (api *Router) resolveAnnotated(w http.ResponseWriter, r *http.Request, id string) (repo model.AnnotatedRepository, ok bool) {
|
||||
ctx := r.Context()
|
||||
u, _ := request.UserFrom(ctx)
|
||||
if al, err := api.ds.Album(ctx).Get(id); err == nil {
|
||||
if !u.HasLibraryAccess(al.LibraryID) {
|
||||
http.Error(w, "Not Found", http.StatusNotFound)
|
||||
return nil, false
|
||||
}
|
||||
return api.ds.Album(ctx), true
|
||||
}
|
||||
if _, err := api.ds.Artist(ctx).Get(id); err == nil {
|
||||
return api.ds.Artist(ctx), true
|
||||
}
|
||||
if mf, err := api.ds.MediaFile(ctx).Get(id); err == nil {
|
||||
if !u.HasLibraryAccess(mf.LibraryID) {
|
||||
http.Error(w, "Not Found", http.StatusNotFound)
|
||||
return nil, false
|
||||
}
|
||||
return api.ds.MediaFile(ctx), true
|
||||
}
|
||||
http.Error(w, "Not Found", http.StatusNotFound)
|
||||
return nil, false
|
||||
}
|
||||
|
||||
func (api *Router) setFavorite(w http.ResponseWriter, r *http.Request, starred bool) {
|
||||
id := chi.URLParam(r, "itemId")
|
||||
repo, ok := api.resolveAnnotated(w, r, id)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
if err := repo.SetStar(starred, id); err != nil {
|
||||
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
api.ok(w, r, &dto.UserItemDataDto{IsFavorite: starred, Key: id, ItemId: id})
|
||||
}
|
||||
|
||||
func (api *Router) markFavorite(w http.ResponseWriter, r *http.Request) { api.setFavorite(w, r, true) }
|
||||
func (api *Router) unmarkFavorite(w http.ResponseWriter, r *http.Request) {
|
||||
api.setFavorite(w, r, false)
|
||||
}
|
||||
|
||||
func (api *Router) setItemRating(w http.ResponseWriter, r *http.Request, rating int) {
|
||||
id := chi.URLParam(r, "itemId")
|
||||
repo, ok := api.resolveAnnotated(w, r, id)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
if err := repo.SetRating(rating, id); err != nil {
|
||||
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
d := &dto.UserItemDataDto{Key: id, ItemId: id}
|
||||
if rating > 0 {
|
||||
jfRating := float64(rating) * 2 // Navidrome 0-5 -> Jellyfin 0-10, mirrors dto.UserData
|
||||
d.Rating = &jfRating
|
||||
}
|
||||
api.ok(w, r, d)
|
||||
}
|
||||
|
||||
func (api *Router) setRating(w http.ResponseWriter, r *http.Request) {
|
||||
rating := req.Params(r).IntOr("Rating", 0) / 2 // Jellyfin 0-10 -> Navidrome 0-5
|
||||
api.setItemRating(w, r, rating)
|
||||
}
|
||||
|
||||
func (api *Router) removeRating(w http.ResponseWriter, r *http.Request) {
|
||||
api.setItemRating(w, r, 0)
|
||||
}
|
||||
171
server/jellyfin/annotations_test.go
Normal file
171
server/jellyfin/annotations_test.go
Normal file
|
|
@ -0,0 +1,171 @@
|
|||
package jellyfin
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/server/jellyfin/dto"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
var _ = Describe("Annotations", func() {
|
||||
var api *Router
|
||||
var ds *tests.MockDataStore
|
||||
// alice has access to library 1 only.
|
||||
ctxUser := func() context.Context {
|
||||
return request.WithUser(context.Background(), model.User{ID: "u1", UserName: "alice", Libraries: model.Libraries{{ID: 1, Name: "Music"}}})
|
||||
}
|
||||
|
||||
BeforeEach(func() {
|
||||
ds = &tests.MockDataStore{}
|
||||
api = &Router{ds: ds}
|
||||
})
|
||||
|
||||
Describe("markFavorite / unmarkFavorite", func() {
|
||||
It("stars a song and returns IsFavorite=true", func() {
|
||||
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
|
||||
mfRepo.SetData(model.MediaFiles{{ID: "s1", Title: "Song", LibraryID: 1}})
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/s1", nil).WithContext(ctxUser())
|
||||
r = withChiURLParam(r, "itemId", "s1")
|
||||
api.markFavorite(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
var d dto.UserItemDataDto
|
||||
Expect(json.Unmarshal(w.Body.Bytes(), &d)).To(Succeed())
|
||||
Expect(d.IsFavorite).To(BeTrue())
|
||||
Expect(mfRepo.Data["s1"].Starred).To(BeTrue())
|
||||
})
|
||||
|
||||
It("stars an album and returns IsFavorite=true", func() {
|
||||
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
|
||||
albumRepo.SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 1}})
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/a1", nil).WithContext(ctxUser())
|
||||
r = withChiURLParam(r, "itemId", "a1")
|
||||
api.markFavorite(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
var d dto.UserItemDataDto
|
||||
Expect(json.Unmarshal(w.Body.Bytes(), &d)).To(Succeed())
|
||||
Expect(d.IsFavorite).To(BeTrue())
|
||||
Expect(albumRepo.Data["a1"].Starred).To(BeTrue())
|
||||
})
|
||||
|
||||
It("stars an artist without checking library access (artists span multiple libraries)", func() {
|
||||
artistRepo := ds.Artist(context.Background()).(*tests.MockArtistRepo)
|
||||
artistRepo.SetData(model.Artists{{ID: "ar1", Name: "Artist"}})
|
||||
w := httptest.NewRecorder()
|
||||
// alice only has access to library 1, but artists aren't gated per-library.
|
||||
r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/ar1", nil).WithContext(ctxUser())
|
||||
r = withChiURLParam(r, "itemId", "ar1")
|
||||
api.markFavorite(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
var d dto.UserItemDataDto
|
||||
Expect(json.Unmarshal(w.Body.Bytes(), &d)).To(Succeed())
|
||||
Expect(d.IsFavorite).To(BeTrue())
|
||||
Expect(artistRepo.Data["ar1"].Starred).To(BeTrue())
|
||||
})
|
||||
|
||||
It("unstars a song and returns IsFavorite=false", func() {
|
||||
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
|
||||
mfRepo.SetData(model.MediaFiles{{ID: "s1", Title: "Song", LibraryID: 1, Annotations: model.Annotations{Starred: true}}})
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("DELETE", "/Users/u1/FavoriteItems/s1", nil).WithContext(ctxUser())
|
||||
r = withChiURLParam(r, "itemId", "s1")
|
||||
api.unmarkFavorite(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
var d dto.UserItemDataDto
|
||||
Expect(json.Unmarshal(w.Body.Bytes(), &d)).To(Succeed())
|
||||
Expect(d.IsFavorite).To(BeFalse())
|
||||
Expect(mfRepo.Data["s1"].Starred).To(BeFalse())
|
||||
})
|
||||
|
||||
It("returns 404 and does not star an album in a library the user can't access", func() {
|
||||
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
|
||||
albumRepo.SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 2}})
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/a1", nil).WithContext(ctxUser()) // only has access to library 1
|
||||
r = withChiURLParam(r, "itemId", "a1")
|
||||
api.markFavorite(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusNotFound))
|
||||
Expect(albumRepo.Data["a1"].Starred).To(BeFalse())
|
||||
})
|
||||
|
||||
It("returns 404 and does not star a song in a library the user can't access", func() {
|
||||
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
|
||||
mfRepo.SetData(model.MediaFiles{{ID: "s1", Title: "Song", LibraryID: 2}})
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/s1", nil).WithContext(ctxUser()) // only has access to library 1
|
||||
r = withChiURLParam(r, "itemId", "s1")
|
||||
api.markFavorite(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusNotFound))
|
||||
Expect(mfRepo.Data["s1"].Starred).To(BeFalse())
|
||||
})
|
||||
|
||||
It("returns 404 when the id doesn't match any entity", func() {
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("POST", "/Users/u1/FavoriteItems/missing", nil).WithContext(ctxUser())
|
||||
r = withChiURLParam(r, "itemId", "missing")
|
||||
api.markFavorite(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusNotFound))
|
||||
})
|
||||
})
|
||||
|
||||
Describe("setRating / removeRating", func() {
|
||||
It("maps a Jellyfin 0-10 rating to Navidrome's 0-5 scale", func() {
|
||||
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
|
||||
mfRepo.SetData(model.MediaFiles{{ID: "s1", Title: "Song", LibraryID: 1}})
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("POST", "/Users/u1/Items/s1/Rating?Rating=8", nil).WithContext(ctxUser())
|
||||
r = withChiURLParam(r, "itemId", "s1")
|
||||
api.setRating(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
Expect(mfRepo.Data["s1"].Rating).To(Equal(4))
|
||||
var d dto.UserItemDataDto
|
||||
Expect(json.Unmarshal(w.Body.Bytes(), &d)).To(Succeed())
|
||||
Expect(d.Rating).NotTo(BeNil())
|
||||
Expect(*d.Rating).To(Equal(8.0))
|
||||
})
|
||||
|
||||
It("rates an album", func() {
|
||||
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
|
||||
albumRepo.SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 1}})
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("POST", "/Users/u1/Items/a1/Rating?Rating=10", nil).WithContext(ctxUser())
|
||||
r = withChiURLParam(r, "itemId", "a1")
|
||||
api.setRating(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
Expect(albumRepo.Data["a1"].Rating).To(Equal(5))
|
||||
})
|
||||
|
||||
It("removes a rating", func() {
|
||||
mfRepo := ds.MediaFile(context.Background()).(*tests.MockMediaFileRepo)
|
||||
mfRepo.SetData(model.MediaFiles{{ID: "s1", Title: "Song", LibraryID: 1, Annotations: model.Annotations{Rating: 4}}})
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("DELETE", "/Users/u1/Items/s1/Rating", nil).WithContext(ctxUser())
|
||||
r = withChiURLParam(r, "itemId", "s1")
|
||||
api.removeRating(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusOK))
|
||||
Expect(mfRepo.Data["s1"].Rating).To(Equal(0))
|
||||
var d dto.UserItemDataDto
|
||||
Expect(json.Unmarshal(w.Body.Bytes(), &d)).To(Succeed())
|
||||
Expect(d.Rating).To(BeNil())
|
||||
})
|
||||
|
||||
It("returns 404 and does not rate an album in a library the user can't access", func() {
|
||||
albumRepo := ds.Album(context.Background()).(*tests.MockAlbumRepo)
|
||||
albumRepo.SetData(model.Albums{{ID: "a1", Name: "One", LibraryID: 2}})
|
||||
w := httptest.NewRecorder()
|
||||
r := httptest.NewRequest("POST", "/Users/u1/Items/a1/Rating?Rating=10", nil).WithContext(ctxUser()) // only has access to library 1
|
||||
r = withChiURLParam(r, "itemId", "a1")
|
||||
api.setRating(w, r)
|
||||
Expect(w.Code).To(Equal(http.StatusNotFound))
|
||||
Expect(albumRepo.Data["a1"].Rating).To(Equal(0))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -68,6 +68,11 @@ func (api *Router) routes() http.Handler {
|
|||
r.Get("/Users/{userId}/Items/{itemId}", api.getItem)
|
||||
r.Get("/Users/{userId}/Items/Latest", api.getLatest)
|
||||
|
||||
r.Post("/Users/{userId}/FavoriteItems/{itemId}", api.markFavorite)
|
||||
r.Delete("/Users/{userId}/FavoriteItems/{itemId}", api.unmarkFavorite)
|
||||
r.Post("/Users/{userId}/Items/{itemId}/Rating", api.setRating)
|
||||
r.Delete("/Users/{userId}/Items/{itemId}/Rating", api.removeRating)
|
||||
|
||||
r.Get("/Artists", api.getArtists)
|
||||
r.Get("/Artists/AlbumArtists", api.getArtists)
|
||||
r.Get("/Genres", api.getGenres)
|
||||
|
|
|
|||
|
|
@ -174,6 +174,9 @@ func (m *MockAlbumRepo) SetRating(rating int, itemID string) error {
|
|||
if m.Err {
|
||||
return errors.New("unexpected error")
|
||||
}
|
||||
if d, ok := m.Data[itemID]; ok {
|
||||
d.Rating = rating
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
|
|
@ -182,6 +185,11 @@ func (m *MockAlbumRepo) SetStar(starred bool, itemIDs ...string) error {
|
|||
if m.Err {
|
||||
return errors.New("unexpected error")
|
||||
}
|
||||
for _, id := range itemIDs {
|
||||
if d, ok := m.Data[id]; ok {
|
||||
d.Starred = starred
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -73,6 +73,28 @@ func (m *MockArtistRepo) IncPlayCount(id string, timestamp time.Time) error {
|
|||
return model.ErrNotFound
|
||||
}
|
||||
|
||||
func (m *MockArtistRepo) SetStar(starred bool, itemIDs ...string) error {
|
||||
if m.Err {
|
||||
return errors.New("error")
|
||||
}
|
||||
for _, id := range itemIDs {
|
||||
if d, ok := m.Data[id]; ok {
|
||||
d.Starred = starred
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *MockArtistRepo) SetRating(rating int, itemID string) error {
|
||||
if m.Err {
|
||||
return errors.New("error")
|
||||
}
|
||||
if d, ok := m.Data[itemID]; ok {
|
||||
d.Rating = rating
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *MockArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, error) {
|
||||
if len(options) > 0 {
|
||||
m.Options = options[0]
|
||||
|
|
|
|||
|
|
@ -154,6 +154,28 @@ func (m *MockMediaFileRepo) IncPlayCount(id string, timestamp time.Time) error {
|
|||
return model.ErrNotFound
|
||||
}
|
||||
|
||||
func (m *MockMediaFileRepo) SetStar(starred bool, itemIDs ...string) error {
|
||||
if m.Err {
|
||||
return errors.New("error")
|
||||
}
|
||||
for _, id := range itemIDs {
|
||||
if d, ok := m.Data[id]; ok {
|
||||
d.Starred = starred
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *MockMediaFileRepo) SetRating(rating int, itemID string) error {
|
||||
if m.Err {
|
||||
return errors.New("error")
|
||||
}
|
||||
if d, ok := m.Data[itemID]; ok {
|
||||
d.Rating = rating
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *MockMediaFileRepo) FindByAlbum(artistId string) (model.MediaFiles, error) {
|
||||
if m.Err {
|
||||
return nil, errors.New("error")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue