diff --git a/server/jellyfin/README.md b/server/jellyfin/README.md index 9315ac4f9..48e69cb05 100644 --- a/server/jellyfin/README.md +++ b/server/jellyfin/README.md @@ -158,8 +158,8 @@ and `MusicArtist` queries accept it too, matching artists credited on an album o `Years`; `StudioIds` (record labels, as listed by `GET Studios`); `SearchTerm`; `Filters` (`IsFavorite`, `IsFavoriteOrLikes`, `IsPlayed`, `IsUnplayed`) and the standalone `isFavorite`/`isPlayed` booleans it can also be expressed as — `Filters` wins when both are sent, as -in Jellyfin; `Likes`, `Dislikes`, `IsFolder`, `IsNotFolder` and `IsResumable` have no Navidrome -equivalent and are ignored; `SortBy`/`SortOrder` (every recognized key is applied in order, so secondary keys break ties; +in Jellyfin; `Likes` and `Dislikes` are not supported, and `IsFolder`, `IsNotFolder` and +`IsResumable` have no Navidrome equivalent, so all five are ignored; `SortBy`/`SortOrder` (every recognized key is applied in order, so secondary keys break ties; unrecognized keys are skipped, and `Random` always sorts alone); `StartIndex`/`Limit`; and `Ids` (batch fetch by id). `Recursive=false` with a library `ParentId` returns direct children only (no tracks — no track is a library's direct child). @@ -176,7 +176,7 @@ returns direct children only (no tracks — no track is a library's direct child | Artists / genres / labels | `GET Artists`, `GET Artists/AlbumArtists`, `GET Genres`, `GET MusicGenres`, `GET Studios`, `GET Items/Filters` | | Similar / mixes | `GET Artists/{itemId}/Similar`, `GET Items/{itemId}/Similar`, `GET Albums/{itemId}/Similar`, `GET {Items,Songs,Albums,Artists,Playlists}/{itemId}/InstantMix`, `GET Artists/InstantMix?id=`, `GET MusicGenres/InstantMix?id=` | | Images | `GET Items/{itemId}/Images/{type}[/{index}]` (public), `POST`/`DELETE Items/{itemId}/Images/{type}` (playlist cover, authenticated) | -| Favorites / ratings for songs, albums, artists, and playlists | `POST`/`DELETE UserFavoriteItems/{itemId}`, `POST`/`DELETE Users/{userId}/FavoriteItems/{itemId}`, `POST`/`DELETE Users/{userId}/Items/{itemId}/Rating`, `GET UserItems/{itemId}/UserData`, `GET Users/{userId}/Items/{itemId}/UserData` | +| Favorites / ratings for songs, albums, artists, and playlists | `POST`/`DELETE UserFavoriteItems/{itemId}`, `POST`/`DELETE Users/{userId}/FavoriteItems/{itemId}`, `POST`/`DELETE UserItems/{itemId}/Rating`, `POST`/`DELETE Users/{userId}/Items/{itemId}/Rating`, `GET`/`POST UserItems/{itemId}/UserData`, `GET`/`POST Users/{userId}/Items/{itemId}/UserData` | | Streaming | `GET Audio/{itemId}/stream[.{container}]`, `GET Audio/{itemId}/universal`, `GET Audio/{itemId}/main.m3u8`, `GET Items/{itemId}/File`, `GET Items/{itemId}/Download`, `GET`/`POST Items/{itemId}/PlaybackInfo` (`HEAD` too on stream, universal, File, Download and images; a transcode HEAD answers without starting it) | | Lyrics | `GET Audio/{itemId}/Lyrics` | | Playback reporting | `POST Sessions/Playing`, `POST Sessions/Playing/Progress`, `POST Sessions/Playing/Stopped`, `POST Sessions/Playing/Ping` (no-op), `POST Sessions/Capabilities[/Full]` | @@ -402,12 +402,20 @@ make test PKG=./server/jellyfin/... in the playlist, so ids change when entries are inserted or removed; re-read the list after an edit. Real Jellyfin uses the song id. Clients that echo `PlaylistItemId` back (Finamp) work; clients that send the song id as `EntryIds` or to `Move` (Jellify) get `404`. -- **Rating uses the `rating` param.** `POST Users/{userId}/Items/{itemId}/Rating` reads a `rating` - (0-10) and stores it as 0-5 stars. Jellyfin's `likes` boolean is not read, so a client that sends - only `likes` clears the rating. -- **Missing endpoints.** `UserPlayedItems` (mark played/unplayed), `Search/Hints` and the non-legacy - `UserItems/{itemId}/Rating` are not implemented. Play counts only change through playback - reporting. +- **Ratings are stored as 0-5 stars.** Jellyfin's 0-10 `Rating` is rounded to the nearest star + (any nonzero value keeps at least one), so it reads back as an even number. `likes=true` stores + 5 stars and `likes=false` 1 star, like Jellyfin's 10 and 1. A rating of 0 clears the rating. +- **`POST .../Rating` needs a value.** Jellyfin clears the rating when `likes` is missing; Navidrome + answers `400` and keeps it, as it does for a malformed or out-of-range value. Use + `DELETE .../Rating` to clear. A numeric `rating` param (0-10) is also accepted when `likes` is + absent. +- **`POST .../UserData` writes only `Rating`, `Likes` and `IsFavorite`.** `Played`, `PlayCount`, + `LastPlayedDate` and `PlaybackPositionTicks` are ignored. A body that is not a single JSON + object, or a `Rating` outside 0-10, is a `400` and nothing is written. +- **Annotation writes can't target another user.** Rating and UserData writes whose `userId` names + a different user get `403`, even for admins; Jellyfin lets admins edit other users' data. +- **Missing endpoints.** `UserPlayedItems` (mark played/unplayed) and `Search/Hints` are not + implemented. Play counts only change through playback reporting. - **Some `Fields` are never emitted.** `ProviderIds`, `People`, `Etag` and `DateLastMediaAdded` are accepted but never returned. - **Blurhashes can be missing.** `ImageBlurHashes` carries the real blurhash that the artwork diff --git a/server/jellyfin/annotations.go b/server/jellyfin/annotations.go index a589798be..1bb0b7846 100644 --- a/server/jellyfin/annotations.go +++ b/server/jellyfin/annotations.go @@ -1,47 +1,52 @@ package jellyfin import ( + "encoding/json" "errors" - "math" + "io" "net/http" + "strconv" + "github.com/go-chi/chi/v5" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/events" "github.com/navidrome/navidrome/server/jellyfin/dto" - "github.com/navidrome/navidrome/utils/req" ) -// resolveAnnotated finds which annotated repo owns id, returning the resource name used in -// refreshResource events. Albums and songs 404 when the user can't access their library; artists -// span libraries (library_artist), so have no single LibraryID to gate on and rely on list-time -// scoping. PlaylistRepository.Get enforces playlist visibility. When repo is nil the response has -// already been written, so callers must return without writing the annotation. -func (api *Router) resolveAnnotated(w http.ResponseWriter, r *http.Request, id string) (repo model.AnnotatedRepository, resource string) { +type annotatedItem struct { + repo model.AnnotatedRepository + resource string // name used in refreshResource events + ann model.Annotations +} + +// resolveAnnotated finds id's repo and the caller's annotations. Only albums and songs are library-gated +// (artists span libraries; playlists gate in Get). When !ok, the response has already been written. +func (api *Router) resolveAnnotated(w http.ResponseWriter, r *http.Request, id string) (item annotatedItem, ok bool) { ctx := r.Context() entity, err := model.GetEntityByID(ctx, api.ds, id) if err != nil && !errors.Is(err, model.ErrNotFound) { api.internalError(w, r, err) - return nil, "" + return annotatedItem{}, false } u, _ := request.UserFrom(ctx) switch e := entity.(type) { case *model.Album: if u.HasLibraryAccess(e.LibraryID) { - return api.ds.Album(), "album" + return annotatedItem{api.ds.Album(), "album", e.Annotations}, true } case *model.Artist: - return api.ds.Artist(), "artist" + return annotatedItem{api.ds.Artist(), "artist", e.Annotations}, true case *model.MediaFile: if u.HasLibraryAccess(e.LibraryID) { - return api.ds.MediaFile(), "song" + return annotatedItem{api.ds.MediaFile(), "song", e.Annotations}, true } case *model.Playlist: - return api.ds.Playlist(), "playlist" + return annotatedItem{api.ds.Playlist(), "playlist", e.Annotations}, true } // Unknown ids, inaccessible-library items and non-annotatable entities (radios) all read as absent. http.Error(w, "Not Found", http.StatusNotFound) - return nil, "" + return annotatedItem{}, false } // getUserItemData returns the caller's play/favorite/rating state for a single item. Jellify @@ -65,65 +70,135 @@ func (api *Router) getUserItemData(w http.ResponseWriter, r *http.Request) { api.ok(w, r, data) } -func (api *Router) setFavorite(w http.ResponseWriter, r *http.Request, starred bool) { +// userDataChange holds already-validated writes; a nil field leaves that annotation untouched. +type userDataChange struct { + rating *int // Navidrome stars; 0 clears + favorite *bool +} + +// applyUserData writes change for the itemId path param and answers with the item's resulting +// UserData, as Jellyfin does. +func (api *Router) applyUserData(w http.ResponseWriter, r *http.Request, change userDataChange) { id, ok := itemIDParam(w, r, "itemId") if !ok { return } - repo, resource := api.resolveAnnotated(w, r, id) - if repo == nil { + item, ok := api.resolveAnnotated(w, r, id) + if !ok { return } - if err := repo.SetStar(r.Context(), starred, id); err != nil { - api.internalError(w, r, err) - return + ctx := r.Context() + if change.rating != nil { + if err := item.repo.SetRating(ctx, *change.rating, id); err != nil { + api.internalError(w, r, err) + return + } + item.ann.Rating = *change.rating } - api.broker.SendMessage(r.Context(), (&events.RefreshResource{}).With(resource, id)) - encodedID := dto.EncodeID(id) - api.ok(w, r, &dto.UserItemDataDto{IsFavorite: starred, Key: encodedID, ItemId: encodedID}) + if change.favorite != nil { + if err := item.repo.SetStar(ctx, *change.favorite, id); err != nil { + api.internalError(w, r, err) + return + } + item.ann.Starred = *change.favorite + } + if change.rating != nil || change.favorite != nil { + api.broker.SendMessage(ctx, (&events.RefreshResource{}).With(item.resource, id)) + } + api.ok(w, r, dto.UserData(item.ann, id)) +} + +// applyCallerUserData is applyUserData for routes that may name a userId: only the caller's own +// annotations can be written, so naming another user is refused. Undecodable ids are ignored. +func (api *Router) applyCallerUserData(w http.ResponseWriter, r *http.Request, change userDataChange) { + raw := chi.URLParam(r, "userId") + if raw == "" { + raw = r.URL.Query().Get("userid") + } + if target, ok := dto.DecodeID(raw); ok { + if u, _ := request.UserFrom(r.Context()); target != u.ID { + http.Error(w, "Forbidden", http.StatusForbidden) + return + } + } + api.applyUserData(w, r, change) +} + +func (api *Router) markFavorite(w http.ResponseWriter, r *http.Request) { + api.applyUserData(w, r, userDataChange{favorite: new(true)}) } -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) + api.applyUserData(w, r, userDataChange{favorite: new(false)}) } -func (api *Router) setItemRating(w http.ResponseWriter, r *http.Request, rating int) { - id, ok := itemIDParam(w, r, "itemId") - if !ok { - return - } - repo, resource := api.resolveAnnotated(w, r, id) - if repo == nil { - return - } - if err := repo.SetRating(r.Context(), rating, id); err != nil { - api.internalError(w, r, err) - return - } - api.broker.SendMessage(r.Context(), (&events.RefreshResource{}).With(resource, id)) - encodedID := dto.EncodeID(id) - d := &dto.UserItemDataDto{Key: encodedID, ItemId: encodedID} - if rating > 0 { - jfRating := float64(rating) * 2 // Navidrome 0-5 -> Jellyfin 0-10, mirrors dto.UserData - d.Rating = &jfRating - } - api.ok(w, r, d) -} - -// setRating maps Jellyfin's 0-10 rating (a nullable double, so fractional values are valid) to -// Navidrome's 0-5 stars. A nonzero rating floors at one star: rounding to 0 would clear it, since -// SetRating(0) is the delete path. +// setRating reads Jellyfin's likes flag, or the numeric rating extension older clients send. Unlike +// Jellyfin, a missing or malformed value is a 400 rather than a clear: clearing is DELETE's job. func (api *Router) setRating(w http.ResponseWriter, r *http.Request) { - jfRating := req.Params(r).Float64Or("rating", 0) - jfRating = min(max(jfRating, 0), 10) // clamp: a client sending e.g. Rating=100 must not write an out-of-domain rating - rating := int(math.Round(jfRating / 2)) - if jfRating > 0 { - rating = max(rating, 1) + q := r.URL.Query() + var stars int + switch { + case q.Has("likes"): + likes, err := strconv.ParseBool(q.Get("likes")) + if err != nil { + http.Error(w, "likes must be true or false", http.StatusBadRequest) + return + } + stars = dto.StarsFromLikes(likes) + case q.Has("rating"): + rating, err := strconv.ParseFloat(q.Get("rating"), 64) + s, ok := dto.StarsFromRating(rating) + if err != nil || !ok { + http.Error(w, "rating must be a number from 0 to 10", http.StatusBadRequest) + return + } + stars = s + default: + http.Error(w, "likes is required", http.StatusBadRequest) + return } - api.setItemRating(w, r, rating) + api.applyCallerUserData(w, r, userDataChange{rating: &stars}) } func (api *Router) removeRating(w http.ResponseWriter, r *http.Request) { - api.setItemRating(w, r, 0) + api.applyCallerUserData(w, r, userDataChange{rating: new(0)}) +} + +// updateUserItemData writes Rating, Likes and IsFavorite; omitted or null fields are left untouched. +// Play state is ignored because it only changes through playback reporting. +func (api *Router) updateUserItemData(w http.ResponseWriter, r *http.Request) { + body, ok := decodeUserDataBody(w, r) + if !ok { + return + } + change := userDataChange{favorite: body.IsFavorite} + // Jellyfin applies Likes before Rating, so Rating wins when a body carries both. + switch { + case body.Rating != nil: + stars, ok := dto.StarsFromRating(*body.Rating) + if !ok { + http.Error(w, "Rating must be from 0 to 10", http.StatusBadRequest) + return + } + change.rating = &stars + case body.Likes != nil: + change.rating = new(dto.StarsFromLikes(*body.Likes)) + } + api.applyCallerUserData(w, r, change) +} + +const maxUserDataBody = 64 << 10 + +// decodeUserDataBody accepts exactly one JSON object; Decode alone would ignore anything after it. +func decodeUserDataBody(w http.ResponseWriter, r *http.Request) (*dto.UpdateUserItemDataDto, bool) { + dec := json.NewDecoder(http.MaxBytesReader(w, r.Body, maxUserDataBody)) + var body *dto.UpdateUserItemDataDto + err := dec.Decode(&body) + if err == nil && body != nil { + if _, err = dec.Token(); errors.Is(err, io.EOF) { + return body, true + } + } + http.Error(w, "Bad Request", http.StatusBadRequest) + return nil, false } diff --git a/server/jellyfin/annotations_test.go b/server/jellyfin/annotations_test.go index a436503ba..69d73ed05 100644 --- a/server/jellyfin/annotations_test.go +++ b/server/jellyfin/annotations_test.go @@ -5,6 +5,8 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" + "time" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" @@ -21,7 +23,7 @@ var _ = Describe("Annotations", func() { var broker *fakeEventBroker // alice has access to library 1 only. ctxUser := func() context.Context { - return request.WithUser(context.Background(), model.User{ID: testID("u1"), UserName: "alice", Libraries: model.Libraries{{ID: 1, Name: "Music"}}}) + return request.WithUser(GinkgoT().Context(), model.User{ID: testID("u1"), UserName: "alice", Libraries: model.Libraries{{ID: 1, Name: "Music"}}}) } BeforeEach(func() { @@ -30,6 +32,16 @@ var _ = Describe("Annotations", func() { api = &Router{ds: ds, broker: broker} }) + songWith := func(ann model.Annotations) *tests.MockMediaFileRepo { + mfRepo := ds.MediaFile().(*tests.MockMediaFileRepo) + mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song", LibraryID: 1, Annotations: ann}}) + return mfRepo + } + songReq := func(method, target, body string, params ...string) *http.Request { + r := httptest.NewRequest(method, target, strings.NewReader(body)).WithContext(ctxUser()) + return withChiURLParam(r, append([]string{"itemId", dto.EncodeID(testID("s1"))}, params...)...) + } + Describe("markFavorite / unmarkFavorite", func() { It("stars a song and returns IsFavorite=true", func() { mfRepo := ds.MediaFile().(*tests.MockMediaFileRepo) @@ -268,41 +280,275 @@ var _ = Describe("Annotations", func() { Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) }) - It("clamps a Rating above 10 to Navidrome's max (5)", func() { - mfRepo := ds.MediaFile().(*tests.MockMediaFileRepo) - mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song", LibraryID: 1}}) - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/Users/u1/Items/s1/Rating?Rating=100", nil).WithContext(ctxUser()) - r = withChiURLParam(r, "itemId", dto.EncodeID(testID("s1"))) - invoke(api.setRating, w, r) - Expect(w.Code).To(Equal(http.StatusOK)) - Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(5)) - }) - - It("clamps a negative Rating to Navidrome's min (0)", func() { - mfRepo := ds.MediaFile().(*tests.MockMediaFileRepo) - mfRepo.SetData(model.MediaFiles{{ID: testID("s1"), Title: "Song", LibraryID: 1}}) - w := httptest.NewRecorder() - r := httptest.NewRequest("POST", "/Users/u1/Items/s1/Rating?Rating=-5", nil).WithContext(ctxUser()) - r = withChiURLParam(r, "itemId", dto.EncodeID(testID("s1"))) - invoke(api.setRating, w, r) - Expect(w.Code).To(Equal(http.StatusOK)) - Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(0)) - }) - It("emits a refreshResource event when rating a song", func() { - mfRepo := ds.MediaFile().(*tests.MockMediaFileRepo) - mfRepo.SetData(model.MediaFiles{{ID: testID("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", dto.EncodeID(testID("s1"))) - invoke(api.setRating, w, r) + songWith(model.Annotations{}) + invoke(api.setRating, httptest.NewRecorder(), songReq("POST", "/Users/u1/Items/s1/Rating?Rating=8", "")) Expect(broker.Events).To(HaveLen(1)) Expect(broker.Events[0].Data(broker.Events[0])).To(Equal(`{"song":["` + testID("s1") + `"]}`)) }) + + Context("Jellyfin likes semantics", func() { + It("stores likes=true as 5 stars and reports Rating 10 / Likes true", func() { + mfRepo := songWith(model.Annotations{}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/UserItems/s1/Rating?likes=true", "")) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(5)) + d := parseUserData(w) + Expect(*d.Rating).To(Equal(10.0)) + Expect(*d.Likes).To(BeTrue()) + }) + + It("stores likes=false as 1 star (Jellyfin's dislike is Rating 1), not a clear", func() { + mfRepo := songWith(model.Annotations{Rating: 4}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/UserItems/s1/Rating?Likes=False", "")) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(1)) + d := parseUserData(w) + Expect(*d.Rating).To(Equal(2.0)) + Expect(*d.Likes).To(BeFalse()) + }) + + It("prefers likes over the numeric rating extension when both are sent", func() { + mfRepo := songWith(model.Annotations{}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/UserItems/s1/Rating?likes=false&rating=10", "")) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(1)) + }) + + It("still clears on an explicit rating=0 from numeric clients", func() { + mfRepo := songWith(model.Annotations{Rating: 4}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/UserItems/s1/Rating?rating=0", "")) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(0)) + Expect(parseUserData(w).Rating).To(BeNil()) + }) + + It("keeps the favorite and play state and reports them in the response", func() { + playDate := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) + mfRepo := songWith(model.Annotations{Starred: true, PlayCount: 3, PlayDate: &playDate}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/UserItems/s1/Rating?likes=true", "")) + Expect(w.Code).To(Equal(http.StatusOK)) + song := mfRepo.Data[testID("s1")] + Expect(song.Starred).To(BeTrue()) + Expect(song.PlayCount).To(Equal(int64(3))) + Expect(song.PlayDate).To(Equal(&playDate)) + d := parseUserData(w) + Expect(d.IsFavorite).To(BeTrue()) + Expect(d.PlayCount).To(Equal(3)) + Expect(d.Played).To(BeTrue()) + }) + + DescribeTable("rejects a missing or malformed value without touching the stored rating", + func(query string) { + mfRepo := songWith(model.Annotations{Rating: 4}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/UserItems/s1/Rating"+query, "")) + Expect(w.Code).To(Equal(http.StatusBadRequest)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + Expect(broker.Events).To(BeEmpty()) + }, + Entry("no params (Jellyfin would clear)", ""), + Entry("unrelated params only", "?userId=x&foo=1"), + Entry("empty likes", "?likes="), + Entry("non-boolean likes", "?likes=maybe"), + Entry("non-boolean likes even with a valid rating", "?likes=maybe&rating=8"), + Entry("empty rating", "?rating="), + Entry("non-numeric rating", "?rating=abc"), + Entry("NaN rating", "?rating=NaN"), + Entry("infinite rating", "?rating=Inf"), + Entry("rating above 10", "?Rating=100"), + Entry("negative rating", "?Rating=-5"), + ) + }) + + Context("authorization", func() { + DescribeTable("refuses a userId naming another user and leaves the rating", + func(method, target string, params ...string) { + handler := api.setRating + if method == "DELETE" { + handler = api.removeRating + } + mfRepo := songWith(model.Annotations{Rating: 4}) + w := httptest.NewRecorder() + invoke(handler, w, songReq(method, target, "", params...)) + Expect(w.Code).To(Equal(http.StatusForbidden)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + Expect(broker.Events).To(BeEmpty()) + }, + Entry("path userId", "POST", "/Users/u2/Items/s1/Rating?likes=true", "userId", dto.EncodeID(testID("u2"))), + Entry("query userId on the /UserItems form", "POST", "/UserItems/s1/Rating?likes=true&userId="+dto.EncodeID(testID("u2"))), + Entry("clearing", "DELETE", "/Users/u2/Items/s1/Rating", "userId", dto.EncodeID(testID("u2"))), + ) + + It("accepts the caller's own userId", func() { + mfRepo := songWith(model.Annotations{}) + w := httptest.NewRecorder() + invoke(api.setRating, w, songReq("POST", "/Users/u1/Items/s1/Rating?likes=true", "", "userId", dto.EncodeID(testID("u1")))) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(5)) + }) + }) + }) + + Describe("updateUserItemData", func() { + post := func(body string, params ...string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + invoke(api.updateUserItemData, w, songReq("POST", "/UserItems/s1/UserData", body, params...)) + return w + } + + It("maps a 0-10 Rating to stars and answers with the full UserData", func() { + mfRepo := songWith(model.Annotations{Starred: true, PlayCount: 2}) + w := post(`{"Rating":8}`) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + d := parseUserData(w) + Expect(*d.Rating).To(Equal(8.0)) + Expect(*d.Likes).To(BeTrue()) + Expect(d.IsFavorite).To(BeTrue()) + Expect(d.PlayCount).To(Equal(2)) + Expect(d.ItemId).To(Equal(dto.EncodeID(testID("s1")))) + Expect(broker.Events).To(HaveLen(1)) + }) + + DescribeTable("stores the personal rating", + func(body string, before, expected int) { + mfRepo := songWith(model.Annotations{Rating: before}) + Expect(post(body).Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(expected)) + }, + Entry("Likes true", `{"Likes":true}`, 0, 5), + Entry("Likes false (Rating 1)", `{"Likes":false}`, 4, 1), + Entry("Rating wins over Likes, as Jellyfin applies Likes first", `{"Likes":true,"Rating":2}`, 0, 1), + Entry("fractional Rating rounds to the nearest star", `{"Rating":7.5}`, 0, 4), + Entry("small nonzero Rating floors at one star", `{"Rating":0.4}`, 0, 1), + Entry("Rating 0 clears", `{"Rating":0}`, 4, 0), + ) + + DescribeTable("leaves the rating untouched when Rating and Likes are omitted or null", + func(body string, wantEvents int) { + mfRepo := songWith(model.Annotations{Rating: 4}) + w := post(body) + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + Expect(*parseUserData(w).Rating).To(Equal(8.0)) + Expect(broker.Events).To(HaveLen(wantEvents)) + }, + Entry("empty object writes nothing", `{}`, 0), + Entry("explicit nulls write nothing", `{"Rating":null,"Likes":null,"IsFavorite":null}`, 0), + Entry("favorite only", `{"IsFavorite":true}`, 1), + ) + + It("sets and unsets the favorite without touching the rating", func() { + mfRepo := songWith(model.Annotations{Rating: 4}) + Expect(post(`{"IsFavorite":true}`).Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Starred).To(BeTrue()) + Expect(post(`{"IsFavorite":false}`).Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Starred).To(BeFalse()) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + }) + + It("ignores play-state fields, which only change through playback reporting", func() { + mfRepo := songWith(model.Annotations{PlayCount: 2}) + w := post(`{"Played":true,"PlayCount":99,"LastPlayedDate":"2026-01-01T00:00:00Z","PlaybackPositionTicks":5}`) + Expect(w.Code).To(Equal(http.StatusOK)) + song := mfRepo.Data[testID("s1")] + Expect(song.PlayCount).To(Equal(int64(2))) + Expect(song.PlayDate).To(BeNil()) + Expect(broker.Events).To(BeEmpty()) + }) + + It("leaves state unchanged when a client echoes back the UserData it read", func() { + mfRepo := songWith(model.Annotations{Rating: 3, Starred: true, PlayCount: 1}) + echo, err := json.Marshal(dto.UserData(mfRepo.Data[testID("s1")].Annotations, testID("s1"))) + Expect(err).ToNot(HaveOccurred()) + Expect(post(string(echo)).Code).To(Equal(http.StatusOK)) + song := mfRepo.Data[testID("s1")] + Expect(song.Rating).To(Equal(3)) + Expect(song.Starred).To(BeTrue()) + Expect(song.PlayCount).To(Equal(int64(1))) + }) + + DescribeTable("rejects an invalid body without writing anything", + func(body string) { + mfRepo := songWith(model.Annotations{Rating: 4}) + Expect(post(body).Code).To(Equal(http.StatusBadRequest)) + song := mfRepo.Data[testID("s1")] + Expect(song.Rating).To(Equal(4)) + Expect(song.Starred).To(BeFalse()) + Expect(broker.Events).To(BeEmpty()) + }, + Entry("empty body", ""), + Entry("malformed JSON", `{"Rating":`), + Entry("Rating above 10, even with a valid IsFavorite", `{"IsFavorite":true,"Rating":10.5}`), + Entry("negative Rating", `{"IsFavorite":true,"Rating":-1}`), + Entry("Rating as a string", `{"Rating":"8"}`), + Entry("Likes as a string", `{"Likes":"yes"}`), + Entry("IsFavorite as a number", `{"IsFavorite":1}`), + Entry("JSON null", `null`), + Entry("JSON array", `[]`), + Entry("trailing garbage after the object", `{"Rating":8} x`), + Entry("a second JSON value after the object", `{"Rating":8}{"Rating":2}`), + Entry("a second object after whitespace", "{\"Rating\":8}\n{}"), + ) + + It("accepts trailing whitespace after the object", func() { + mfRepo := songWith(model.Annotations{}) + Expect(post("{\"Rating\":8}\r\n\t ").Code).To(Equal(http.StatusOK)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + }) + + It("rejects a body larger than the limit", func() { + mfRepo := songWith(model.Annotations{Rating: 4}) + big := `{"Rating":8,"Pad":"` + strings.Repeat("x", 70<<10) + `"}` + Expect(post(big).Code).To(Equal(http.StatusBadRequest)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + }) + + Context("authorization", func() { + It("refuses a userId naming another user without writing", func() { + mfRepo := songWith(model.Annotations{Rating: 4}) + w := post(`{"Rating":10,"IsFavorite":true}`, "userId", dto.EncodeID(testID("u2"))) + Expect(w.Code).To(Equal(http.StatusForbidden)) + Expect(mfRepo.Data[testID("s1")].Rating).To(Equal(4)) + Expect(mfRepo.Data[testID("s1")].Starred).To(BeFalse()) + }) + + It("accepts the caller's own userId", func() { + songWith(model.Annotations{}) + Expect(post(`{"Rating":10}`, "userId", dto.EncodeID(testID("u1"))).Code).To(Equal(http.StatusOK)) + }) + + It("returns 404 and writes nothing for an album in a library the user can't access", func() { + albumRepo := ds.Album().(*tests.MockAlbumRepo) + albumRepo.SetData(model.Albums{{ID: testID("a1"), Name: "One", LibraryID: 2}}) + w := httptest.NewRecorder() + r := httptest.NewRequest("POST", "/UserItems/a1/UserData", strings.NewReader(`{"Rating":10,"IsFavorite":true}`)).WithContext(ctxUser()) + invoke(api.updateUserItemData, w, withChiURLParam(r, "itemId", dto.EncodeID(testID("a1")))) + Expect(w.Code).To(Equal(http.StatusNotFound)) + Expect(albumRepo.Data[testID("a1")].Rating).To(Equal(0)) + Expect(albumRepo.Data[testID("a1")].Starred).To(BeFalse()) + }) + + It("returns 404 for an unknown item", func() { + w := post(`{"Rating":10}`) + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) + }) }) }) +func parseUserData(w *httptest.ResponseRecorder) dto.UserItemDataDto { + var d dto.UserItemDataDto + Expect(json.Unmarshal(w.Body.Bytes(), &d)).To(Succeed()) + return d +} + type fakeEventBroker struct { http.Handler Events []events.Event diff --git a/server/jellyfin/api.go b/server/jellyfin/api.go index 4a471d1f0..b9fd32150 100644 --- a/server/jellyfin/api.go +++ b/server/jellyfin/api.go @@ -146,6 +146,8 @@ func (api *Router) routes() http.Handler { r.Delete("/userfavoriteitems/{itemId}", api.unmarkFavorite) r.Post("/users/{userId}/favoriteitems/{itemId}", api.markFavorite) r.Delete("/users/{userId}/favoriteitems/{itemId}", api.unmarkFavorite) + r.Post("/useritems/{itemId}/rating", api.setRating) + r.Delete("/useritems/{itemId}/rating", api.removeRating) r.Post("/users/{userId}/items/{itemId}/rating", api.setRating) r.Delete("/users/{userId}/items/{itemId}/rating", api.removeRating) @@ -153,6 +155,8 @@ func (api *Router) routes() http.Handler { // /Users/{userId}/Items is the legacy spelling. r.Get("/useritems/{itemId}/userdata", api.getUserItemData) r.Get("/users/{userId}/items/{itemId}/userdata", api.getUserItemData) + r.Post("/useritems/{itemId}/userdata", api.updateUserItemData) + r.Post("/users/{userId}/items/{itemId}/userdata", api.updateUserItemData) r.Get("/artists/{itemId}/similar", api.getSimilarArtists) r.Get("/items/{itemId}/similar", api.getSimilarItems) diff --git a/server/jellyfin/dto/dto.go b/server/jellyfin/dto/dto.go index 4a28eb313..318011d89 100644 --- a/server/jellyfin/dto/dto.go +++ b/server/jellyfin/dto/dto.go @@ -40,6 +40,14 @@ type UserItemDataDto struct { Key string `json:"Key"` ItemId string `json:"ItemId,omitempty"` LastPlayedDate *string `json:"LastPlayedDate,omitempty"` + Likes *bool `json:"Likes,omitempty"` +} + +// UpdateUserItemDataDto is the POST .../UserData body; other Jellyfin fields are accepted and ignored. +type UpdateUserItemDataDto struct { + Rating *float64 `json:"Rating"` + Likes *bool `json:"Likes"` + IsFavorite *bool `json:"IsFavorite"` } type BaseItemDto struct { diff --git a/server/jellyfin/dto/mappers.go b/server/jellyfin/dto/mappers.go index fc7e329de..620df783f 100644 --- a/server/jellyfin/dto/mappers.go +++ b/server/jellyfin/dto/mappers.go @@ -3,6 +3,7 @@ package dto import ( "cmp" "fmt" + "math" "time" "github.com/navidrome/navidrome/conf" @@ -121,6 +122,30 @@ func MediaSourceFromMediaFile(mf model.MediaFile) MediaSourceInfo { } } +// minLikeRating is Jellyfin's UserItemData.MinLikeValue: a rating at or above it reads as Likes. +const minLikeRating = 6.5 + +// StarsFromRating maps Jellyfin's 0-10 rating to 0-5 stars, rejecting out-of-range values and NaN. +// A nonzero rating floors at one star, since 0 stars means "not rated". +func StarsFromRating(rating float64) (int, bool) { + if !(rating >= 0 && rating <= 10) { + return 0, false + } + stars := int(math.Round(rating / 2)) + if rating > 0 { + stars = max(stars, 1) + } + return stars, true +} + +// StarsFromLikes mirrors Jellyfin, which stores likes as Rating 10 and dislikes as Rating 1. +func StarsFromLikes(likes bool) int { + if likes { + return 5 + } + return 1 +} + func UserData(a model.Annotations, itemID string) *UserItemDataDto { // Callers pass the raw model id; encode here so Key/ItemId match the encoded Id on the BaseItemDto. encodedID := EncodeID(itemID) @@ -134,6 +159,7 @@ func UserData(a model.Annotations, itemID string) *UserItemDataDto { if a.Rating > 0 { r := float64(a.Rating) * 2 // Navidrome 0-5 -> Jellyfin 0-10 d.Rating = &r + d.Likes = new(r >= minLikeRating) } if s := JellyfinDate(a.PlayDate); s != "" { d.LastPlayedDate = &s diff --git a/server/jellyfin/dto/mappers_test.go b/server/jellyfin/dto/mappers_test.go index 259904673..3e4ba941f 100644 --- a/server/jellyfin/dto/mappers_test.go +++ b/server/jellyfin/dto/mappers_test.go @@ -478,6 +478,30 @@ var _ = Describe("mappers", func() { Expect(item.ImageBlurHashes).To(BeNil()) }) + DescribeTable("UserData derives Likes from the rating like Jellyfin (rating >= 6.5)", + func(stars int, expected *bool) { + d := UserData(model.Annotations{Rating: stars}, testID("s1")) + Expect(d.Likes).To(Equal(expected)) + }, + Entry("unrated has no Likes", 0, nil), + Entry("1 star (Jellyfin 2) dislikes", 1, new(false)), + Entry("3 stars (Jellyfin 6) dislikes", 3, new(false)), + Entry("4 stars (Jellyfin 8) likes", 4, new(true)), + Entry("5 stars (Jellyfin 10) likes", 5, new(true)), + ) + + It("emits Rating as a whole number and omits Likes when unrated", func() { + rated, err := json.Marshal(UserData(model.Annotations{Rating: 4}, testID("s1"))) + Expect(err).ToNot(HaveOccurred()) + Expect(string(rated)).To(ContainSubstring(`"Rating":8,`)) + Expect(string(rated)).To(ContainSubstring(`"Likes":true`)) + + unrated, err := json.Marshal(UserData(model.Annotations{}, testID("s1"))) + Expect(err).ToNot(HaveOccurred()) + Expect(string(unrated)).ToNot(ContainSubstring(`"Rating"`)) + Expect(string(unrated)).ToNot(ContainSubstring(`"Likes"`)) + }) + It("emits DateCreated and SortName on playlists, like albums and artists", func() { p := model.Playlist{ID: testID("pl-1"), Name: "Chill", CreatedAt: time.Date(2026, 7, 1, 12, 0, 0, 0, time.UTC)} Expect(PlaylistToBaseItem(p, nil).DateCreated).To(Equal("2026-07-01T12:00:00.0000000Z")) diff --git a/server/jellyfin/e2e/annotations_test.go b/server/jellyfin/e2e/annotations_test.go index 182fd5436..014751f6e 100644 --- a/server/jellyfin/e2e/annotations_test.go +++ b/server/jellyfin/e2e/annotations_test.go @@ -130,13 +130,78 @@ var _ = Describe("Annotations", func() { Expect(*itemUserData(id).Rating).To(Equal(float64(8))) }) - It("clamps an out-of-range rating to the valid domain", func() { + It("rejects an out-of-range rating and keeps the stored one", func() { id := albumID("Help!") + Expect(post("/Users/admin-1/Items/"+enc(id)+"/Rating?Rating=8", "").Code).To(Equal(http.StatusOK)) + Expect(post("/Users/admin-1/Items/"+enc(id)+"/Rating?Rating=100", "").Code).To(Equal(http.StatusBadRequest)) + Expect(*itemUserData(id).Rating).To(Equal(float64(8))) + }) + + It("stores Jellyfin likes on the current /UserItems route instead of clearing", func() { + id := songID("So What") + var liked dto.UserItemDataDto + parseInto(post("/UserItems/"+enc(id)+"/Rating?likes=true", ""), &liked) + Expect(*liked.Rating).To(Equal(float64(10))) + Expect(*liked.Likes).To(BeTrue()) + + var disliked dto.UserItemDataDto + parseInto(post("/Users/admin-1/Items/"+enc(id)+"/Rating?likes=false", ""), &disliked) + Expect(*disliked.Rating).To(Equal(float64(2))) + Expect(*disliked.Likes).To(BeFalse()) + Expect(*itemUserData(id).Likes).To(BeFalse()) + + Expect(del("/UserItems/" + enc(id) + "/Rating").Code).To(Equal(http.StatusOK)) + Expect(itemUserData(id).Rating).To(BeNil()) + Expect(itemUserData(id).Likes).To(BeNil()) + }) + + It("does not clear a rating when the request has no value", func() { + id := songID("So What") + Expect(post("/UserItems/"+enc(id)+"/Rating?likes=true", "").Code).To(Equal(http.StatusOK)) + Expect(post("/UserItems/"+enc(id)+"/Rating", "").Code).To(Equal(http.StatusBadRequest)) + Expect(post("/UserItems/"+enc(id)+"/Rating?likes=sure", "").Code).To(Equal(http.StatusBadRequest)) + Expect(*itemUserData(id).Rating).To(Equal(float64(10))) + }) + }) + + Describe("POST /UserItems/{id}/UserData", func() { + It("sets Rating, Likes and IsFavorite on both route spellings", func() { + id := songID("Come Together") var data dto.UserItemDataDto - parseInto(post("/Users/admin-1/Items/"+enc(id)+"/Rating?Rating=100", ""), &data) - // 100 clamps to 10 (Jellyfin) -> 5 (Navidrome) -> 10 back out. - Expect(data.Rating).ToNot(BeNil()) - Expect(*data.Rating).To(Equal(float64(10))) + parseInto(post("/UserItems/"+enc(id)+"/UserData", `{"Rating":6}`), &data) + Expect(*data.Rating).To(Equal(float64(6))) + Expect(*data.Likes).To(BeFalse()) + + parseInto(post("/Users/"+enc(adminUser.ID)+"/Items/"+enc(id)+"/UserData", `{"Likes":true,"IsFavorite":true}`), &data) + stored := itemUserData(id) + Expect(*stored.Rating).To(Equal(float64(10))) + Expect(*stored.Likes).To(BeTrue()) + Expect(stored.IsFavorite).To(BeTrue()) + }) + + It("leaves omitted fields and play state untouched", func() { + id := songID("Come Together") + Expect(post("/UserItems/"+enc(id)+"/UserData", `{"Rating":8,"IsFavorite":true}`).Code).To(Equal(http.StatusOK)) + before := itemUserData(id) + + Expect(post("/UserItems/"+enc(id)+"/UserData", `{"Played":true,"PlayCount":42,"LastPlayedDate":"2026-01-01T00:00:00Z"}`).Code).To(Equal(http.StatusOK)) + Expect(post("/UserItems/"+enc(id)+"/UserData", `{"IsFavorite":false}`).Code).To(Equal(http.StatusOK)) + + after := itemUserData(id) + Expect(*after.Rating).To(Equal(float64(8))) + Expect(after.IsFavorite).To(BeFalse()) + Expect(after.PlayCount).To(Equal(before.PlayCount)) + Expect(after.Played).To(Equal(before.Played)) + Expect(after.LastPlayedDate).To(Equal(before.LastPlayedDate)) + }) + + It("rejects invalid bodies without writing", func() { + id := songID("Come Together") + Expect(post("/UserItems/"+enc(id)+"/UserData", `{"Rating":11,"IsFavorite":true}`).Code).To(Equal(http.StatusBadRequest)) + Expect(post("/UserItems/"+enc(id)+"/UserData", `{"Rating":8} {"Rating":2}`).Code).To(Equal(http.StatusBadRequest)) + stored := itemUserData(id) + Expect(stored.Rating).To(BeNil()) + Expect(stored.IsFavorite).To(BeFalse()) }) }) }) diff --git a/server/jellyfin/e2e/multiuser_test.go b/server/jellyfin/e2e/multiuser_test.go index 905d4e16f..f4f2c43d2 100644 --- a/server/jellyfin/e2e/multiuser_test.go +++ b/server/jellyfin/e2e/multiuser_test.go @@ -4,6 +4,7 @@ import ( "net/http" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server/jellyfin/dto" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -28,6 +29,36 @@ var _ = Describe("Multi-user access control", func() { }) }) + Describe("personal ratings", func() { + userData := func(u model.User, id string) *dto.UserItemDataDto { + var data dto.UserItemDataDto + parseInto(getAs(u, "/UserItems/"+enc(id)+"/UserData"), &data) + return &data + } + + It("keeps each user's rating separate", func() { + id := songID("So What") + Expect(post("/UserItems/"+enc(id)+"/UserData", `{"Rating":10}`).Code).To(Equal(http.StatusOK)) + Expect(postAs(regularUser, "/UserItems/"+enc(id)+"/Rating?likes=false", "").Code).To(Equal(http.StatusOK)) + + Expect(*userData(adminUser, id).Rating).To(Equal(float64(10))) + Expect(*userData(regularUser, id).Rating).To(Equal(float64(2))) + }) + + It("refuses writes that name another user, even for an admin, and changes nothing", func() { + id := songID("So What") + Expect(postAs(regularUser, "/UserItems/"+enc(id)+"/UserData", `{"Rating":4}`).Code).To(Equal(http.StatusOK)) + + Expect(postAs(regularUser, "/Users/"+enc(adminUser.ID)+"/Items/"+enc(id)+"/UserData", `{"Rating":10}`).Code).To(Equal(http.StatusForbidden)) + Expect(post("/Users/"+enc(regularUser.ID)+"/Items/"+enc(id)+"/Rating?likes=true", "").Code).To(Equal(http.StatusForbidden)) + Expect(del("/Users/" + enc(regularUser.ID) + "/Items/" + enc(id) + "/Rating").Code).To(Equal(http.StatusForbidden)) + Expect(post("/UserItems/"+enc(id)+"/UserData?userId="+enc(regularUser.ID), `{"Rating":10}`).Code).To(Equal(http.StatusForbidden)) + + Expect(userData(adminUser, id).Rating).To(BeNil()) + Expect(*userData(regularUser, id).Rating).To(Equal(float64(4))) + }) + }) + Describe("private playlists", func() { It("does not expose another user's private playlist", func() { adminPl := createPlaylist("Admin Private", nil) @@ -53,6 +84,7 @@ var _ = Describe("Multi-user access control", func() { adminPl := createPlaylist("Admin Private", nil) Expect(postAs(regularUser, "/Users/user-1/FavoriteItems/"+enc(adminPl), "").Code).To(Equal(http.StatusNotFound)) Expect(postAs(regularUser, "/Users/user-1/Items/"+enc(adminPl)+"/Rating?Rating=10", "").Code).To(Equal(http.StatusNotFound)) + Expect(postAs(regularUser, "/UserItems/"+enc(adminPl)+"/UserData", `{"Rating":10}`).Code).To(Equal(http.StatusNotFound)) }) It("lets each user manage their own playlist", func() { diff --git a/server/jellyfin/items_test.go b/server/jellyfin/items_test.go index 18789a34b..bdf0be253 100644 --- a/server/jellyfin/items_test.go +++ b/server/jellyfin/items_test.go @@ -18,11 +18,13 @@ import ( . "github.com/onsi/gomega" ) -// withChiURLParam simulates chi's routing having captured a path parameter, since these -// tests call handlers directly instead of going through the full router. -func withChiURLParam(r *http.Request, key, value string) *http.Request { +// withChiURLParam simulates chi's routing having captured path parameters (key, value pairs), since +// these tests call handlers directly instead of going through the full router. +func withChiURLParam(r *http.Request, kv ...string) *http.Request { rctx := chi.NewRouteContext() - rctx.URLParams.Add(key, value) + for i := 0; i+1 < len(kv); i += 2 { + rctx.URLParams.Add(kv[i], kv[i+1]) + } return r.WithContext(context.WithValue(r.Context(), chi.RouteCtxKey, rctx)) }