mirror of
https://github.com/navidrome/navidrome.git
synced 2026-11-02 08:27:56 +01:00
* feat(playlists): add average_rating column to playlist table
* feat(playlists): store and read per-user starred/rating annotations
* feat(playlists): clean up annotations when a playlist is deleted
* feat(subsonic): route star/unstar of a playlist to the playlist repository
* feat(subsonic): route setRating of a playlist to the playlist repository
* test(subsonic): guard that playlist responses never expose annotations
* fix(playlists): clean stale mis-typed annotations on upgrade; cover GetAll read-back
* fix(playlists): scope annotation join by item_type and harden delete
Address code-review findings on the playlist-annotations branch:
- withAnnotation: add an item_type predicate to the LEFT JOIN so a
mis-typed annotation row sharing an id can no longer leak into (or
duplicate) another entity's read. Correct for every caller since each
repo writes annotations with item_type = tableName. Regression test added.
- migration: reclassify legacy media_file-typed rows for playlist ids to
item_type='playlist' (instead of deleting them), preserving users' prior
playlist star/rating; run before the average_rating backfill so those
ratings are included.
- playlist Delete: replace the per-request full-table cleanAnnotations()
anti-join with a targeted, permission-safe (rows-affected gated),
best-effort delete so a cleanup failure no longer misreports an
already-committed delete as an error.
- MockPlaylistRepo: implement GetAll/IncPlayCount/ReassignAnnotation to
remove the dead All field and the nil-interface panic traps.
- test: use slices.IndexFunc instead of a hand-rolled find loop.
* feat(playlists): streamline playlist deletion by relying on annotation sweep
* docs(playlists): trim comments in annotation migration and test
Condense the verbose comments added in this branch per the project's
comment-minimalism guideline, keeping only the non-obvious rationale.
The migration's reclassify block is shortened while preserving the safety
invariant (playlist and media_file ids never collide, so the item_type
rewrite touches only mis-typed rows and cannot violate the unique key) and
the ordering note. The redundant 'Populate average_rating' comment is
dropped since the UPDATE is self-evident. The repository test's leakage
comment is condensed to two lines. No code behavior changes.
* refactor(subsonic): resolve setStar targets via GetEntityByID
Replace setStar's Album/Artist/Playlist Exists probe chain with a single
model.GetEntityByID lookup and a type switch, mirroring setRating. This
removes three per-id existence queries and keeps the two annotation paths
consistent.
An id that resolves to no known entity is logged and skipped rather than
filed as a spurious media_file annotation, and a lookup failure on one id no
longer aborts the whole batch. Also drop a duplicate empty-ids guard.
* refactor(playlists): drop no-op reclassify/backfill from migration
The average_rating migration carried two data-fix UPDATEs that are no-ops on
any real database:
- The media_file->playlist reclassification only matches rows no released
build ever created: playlists were never annotatable, so star/setRating of
a playlist id was never written as item_type='playlist'. Any stray
media_file-typed row for a playlist id is already removed by the media_file
annotation GC sweep (item_id not in media_file).
- The average_rating backfill runs before any item_type='playlist' row can
exist, so it can only ever write the default 0. Going forward SetRating
keeps average_rating current via updateAvgRating.
Reduce the migration to the column add/drop.
* refactor(persistence): bind annotation join params, derive idField from tableName
Address PR review: use Squirrel parameter binding for item_type/user_id in
the shared withAnnotation join instead of string concatenation, and pass
r.tableName+".id" from selectPlaylist so the join field stays consistent
with the surrounding r.tableName usage.
* fix(subsonic): surface datastore errors in setStar instead of skipping
Address PR review: setStar swallowed every GetEntityByID error and continued,
so a real datastore failure would still commit the transaction and emit a
refresh event as if the star succeeded. Skip only on model.ErrNotFound (an
unknown id); return any other error so the request fails and rolls back.
* test(subsonic): assert absent JSON keys instead of substring matches
Address PR review: substring checks are brittle ("starred" matches "starredAt",
"rating" matches "userRating"). Unmarshal the response and assert the
annotation keys are absent.
* fix(subsonic): skip refresh broadcast when a star request changes nothing
Address PR review (Codex): once setStar began skipping unknown ids, a request
containing only unresolvable ids left the RefreshResource empty, which
SendMessage serializes as a {*:*} wildcard that forces every client to
refresh. Only broadcast when at least one id was actually starred.
* fix(db): rebase playlist average_rating migration timestamp past master
The 20260708011823 migration predated the newest migration merged to
master (20260712211040_add_primary_key...), which Goose would silently
skip on already-upgraded databases. Rename it to a current timestamp so
it applies in order.
296 lines
7.6 KiB
Go
296 lines
7.6 KiB
Go
package subsonic
|
|
|
|
import (
|
|
"context"
|
|
"errors"
|
|
"fmt"
|
|
"math"
|
|
"net/http"
|
|
"time"
|
|
|
|
"github.com/navidrome/navidrome/core/scrobbler"
|
|
"github.com/navidrome/navidrome/log"
|
|
"github.com/navidrome/navidrome/model"
|
|
"github.com/navidrome/navidrome/model/request"
|
|
"github.com/navidrome/navidrome/server/events"
|
|
"github.com/navidrome/navidrome/server/subsonic/responses"
|
|
"github.com/navidrome/navidrome/utils/req"
|
|
)
|
|
|
|
func (api *Router) SetRating(r *http.Request) (*responses.Subsonic, error) {
|
|
p := req.Params(r)
|
|
id, err := p.String("id")
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
rating, err := p.Int("rating")
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
log.Debug(r, "Setting rating", "rating", rating, "id", id)
|
|
err = api.setRating(r.Context(), id, rating)
|
|
if err != nil {
|
|
log.Error(r, err)
|
|
return nil, err
|
|
}
|
|
|
|
return newResponse(), nil
|
|
}
|
|
|
|
func (api *Router) setRating(ctx context.Context, id string, rating int) error {
|
|
var repo model.AnnotatedRepository
|
|
var resource string
|
|
|
|
entity, err := model.GetEntityByID(ctx, api.ds, id)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
switch entity.(type) {
|
|
case *model.Artist:
|
|
repo = api.ds.Artist(ctx)
|
|
resource = "artist"
|
|
case *model.Album:
|
|
repo = api.ds.Album(ctx)
|
|
resource = "album"
|
|
case *model.Playlist:
|
|
repo = api.ds.Playlist(ctx)
|
|
resource = "playlist"
|
|
default:
|
|
repo = api.ds.MediaFile(ctx)
|
|
resource = "song"
|
|
}
|
|
err = repo.SetRating(rating, id)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
event := &events.RefreshResource{}
|
|
api.broker.SendMessage(ctx, event.With(resource, id))
|
|
return nil
|
|
}
|
|
|
|
func (api *Router) Star(r *http.Request) (*responses.Subsonic, error) {
|
|
p := req.Params(r)
|
|
ids, _ := p.Strings("id")
|
|
albumIds, _ := p.Strings("albumId")
|
|
artistIds, _ := p.Strings("artistId")
|
|
if len(ids)+len(albumIds)+len(artistIds) == 0 {
|
|
return nil, newError(responses.ErrorMissingParameter, "Required id parameter is missing")
|
|
}
|
|
ids = append(ids, albumIds...)
|
|
ids = append(ids, artistIds...)
|
|
|
|
err := api.setStar(r.Context(), true, ids...)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
return newResponse(), nil
|
|
}
|
|
|
|
func (api *Router) Unstar(r *http.Request) (*responses.Subsonic, error) {
|
|
p := req.Params(r)
|
|
ids, _ := p.Strings("id")
|
|
albumIds, _ := p.Strings("albumId")
|
|
artistIds, _ := p.Strings("artistId")
|
|
if len(ids)+len(albumIds)+len(artistIds) == 0 {
|
|
return nil, newError(responses.ErrorMissingParameter, "Required id parameter is missing")
|
|
}
|
|
ids = append(ids, albumIds...)
|
|
ids = append(ids, artistIds...)
|
|
|
|
err := api.setStar(r.Context(), false, ids...)
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
return newResponse(), nil
|
|
}
|
|
|
|
func (api *Router) setStar(ctx context.Context, star bool, ids ...string) error {
|
|
if len(ids) == 0 {
|
|
log.Warn(ctx, "Cannot star/unstar an empty list of ids")
|
|
return nil
|
|
}
|
|
log.Debug(ctx, "Changing starred", "ids", ids, "starred", star)
|
|
err := api.ds.WithTxImmediate(func(tx model.DataStore) error {
|
|
event := &events.RefreshResource{}
|
|
changed := false
|
|
for _, id := range ids {
|
|
var repo model.AnnotatedRepository
|
|
var resource string
|
|
entity, err := model.GetEntityByID(ctx, tx, id)
|
|
if err != nil {
|
|
if !errors.Is(err, model.ErrNotFound) {
|
|
return err
|
|
}
|
|
log.Warn(ctx, "Cannot star/unstar unknown id, skipping", "id", id)
|
|
continue
|
|
}
|
|
switch entity.(type) {
|
|
case *model.Artist:
|
|
repo = tx.Artist(ctx)
|
|
resource = "artist"
|
|
case *model.Album:
|
|
repo = tx.Album(ctx)
|
|
resource = "album"
|
|
case *model.Playlist:
|
|
repo = tx.Playlist(ctx)
|
|
resource = "playlist"
|
|
default:
|
|
repo = tx.MediaFile(ctx)
|
|
resource = "song"
|
|
}
|
|
if err := repo.SetStar(star, id); err != nil {
|
|
return err
|
|
}
|
|
event = event.With(resource, id)
|
|
changed = true
|
|
}
|
|
// Skip the broadcast when nothing changed: an empty RefreshResource
|
|
// serializes as a "{*:*}" wildcard, forcing every client to refresh.
|
|
if changed {
|
|
api.broker.SendMessage(ctx, event)
|
|
}
|
|
return nil
|
|
})
|
|
if err != nil {
|
|
log.Error(ctx, err)
|
|
return err
|
|
}
|
|
return nil
|
|
}
|
|
|
|
func (api *Router) Scrobble(r *http.Request) (*responses.Subsonic, error) {
|
|
p := req.Params(r)
|
|
ids, err := p.Strings("id")
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
times, _ := p.Times("time")
|
|
if len(times) > 0 && len(times) != len(ids) {
|
|
return nil, newError(responses.ErrorGeneric, "Wrong number of timestamps: %d, should be %d", len(times), len(ids))
|
|
}
|
|
submission := p.BoolOr("submission", true)
|
|
position := p.IntOr("position", 0)
|
|
ctx := r.Context()
|
|
|
|
if submission {
|
|
err := api.scrobblerSubmit(ctx, ids, times)
|
|
if err != nil {
|
|
log.Error(ctx, "Error registering scrobbles", "ids", ids, "times", times, err)
|
|
}
|
|
} else {
|
|
err := api.scrobblerNowPlaying(ctx, ids[0], position)
|
|
if err != nil {
|
|
log.Error(ctx, "Error setting NowPlaying", "id", ids[0], err)
|
|
}
|
|
}
|
|
|
|
return newResponse(), nil
|
|
}
|
|
|
|
func (api *Router) scrobblerSubmit(ctx context.Context, ids []string, times []time.Time) error {
|
|
var submissions []scrobbler.Submission
|
|
log.Debug(ctx, "Scrobbling tracks", "ids", ids, "times", times)
|
|
for i, id := range ids {
|
|
var t time.Time
|
|
if len(times) > 0 {
|
|
t = times[i]
|
|
} else {
|
|
t = time.Now()
|
|
}
|
|
submissions = append(submissions, scrobbler.Submission{TrackID: id, Timestamp: t})
|
|
}
|
|
|
|
return api.scrobbler.Submit(ctx, submissions)
|
|
}
|
|
|
|
func (api *Router) scrobblerNowPlaying(ctx context.Context, trackId string, position int) error {
|
|
mf, err := api.ds.MediaFile(ctx).Get(trackId)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
if mf == nil {
|
|
return fmt.Errorf(`ID "%s" not found`, trackId)
|
|
}
|
|
|
|
player, _ := request.PlayerFrom(ctx)
|
|
username, _ := request.UsernameFrom(ctx)
|
|
client, _ := request.ClientFrom(ctx)
|
|
clientId, ok := request.ClientUniqueIdFrom(ctx)
|
|
if !ok {
|
|
clientId = player.ID
|
|
}
|
|
|
|
log.Info(ctx, "Now Playing", "title", mf.Title, "artist", mf.Artist, "user", username, "player", player.Name, "position", position)
|
|
return api.scrobbler.ReportPlayback(ctx, scrobbler.ReportPlaybackParams{
|
|
MediaId: trackId,
|
|
PositionMs: int64(position) * 1000,
|
|
State: scrobbler.StatePlaying,
|
|
PlaybackRate: 1.0,
|
|
ClientId: clientId,
|
|
ClientName: client,
|
|
})
|
|
}
|
|
|
|
func (api *Router) ReportPlayback(r *http.Request) (*responses.Subsonic, error) {
|
|
p := req.Params(r)
|
|
mediaId, err := p.String("mediaId")
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
mediaType, err := p.String("mediaType")
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
positionMs, err := p.Int64("positionMs")
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
if positionMs < 0 {
|
|
return nil, newError(responses.ErrorGeneric, "positionMs must be non-negative")
|
|
}
|
|
state, err := p.String("state")
|
|
if err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
if !scrobbler.ValidStates[state] {
|
|
return nil, newError(responses.ErrorGeneric, "Invalid state: %s", state)
|
|
}
|
|
|
|
playbackRate := p.Float64Or("playbackRate", 1.0)
|
|
if math.IsNaN(playbackRate) || math.IsInf(playbackRate, 0) || playbackRate <= 0 {
|
|
return nil, newError(responses.ErrorGeneric, "playbackRate must be a finite positive number")
|
|
}
|
|
ignoreScrobble := p.BoolOr("ignoreScrobble", false)
|
|
|
|
ctx := r.Context()
|
|
if mediaType != "song" {
|
|
log.Warn(ctx, "reportPlayback received unsupported mediaType", "mediaType", mediaType, "mediaId", mediaId)
|
|
}
|
|
|
|
player, _ := request.PlayerFrom(ctx)
|
|
client, _ := request.ClientFrom(ctx)
|
|
clientId, ok := request.ClientUniqueIdFrom(ctx)
|
|
if !ok {
|
|
clientId = player.ID
|
|
}
|
|
|
|
err = api.scrobbler.ReportPlayback(ctx, scrobbler.ReportPlaybackParams{
|
|
MediaId: mediaId,
|
|
PositionMs: positionMs,
|
|
State: state,
|
|
PlaybackRate: playbackRate,
|
|
IgnoreScrobble: ignoreScrobble,
|
|
ClientId: clientId,
|
|
ClientName: client,
|
|
})
|
|
if err != nil {
|
|
log.Error(ctx, "Error in ReportPlayback", "mediaId", mediaId, "state", state, err)
|
|
return nil, err
|
|
}
|
|
|
|
return newResponse(), nil
|
|
}
|