Files
navidrome/persistence/sql_annotations.go
Deluan Quintão ca27335d06 feat(playlists): per-user starred/rating annotations (backend) (#5749)
* 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.
2026-07-14 07:38:25 -04:00

207 lines
6.2 KiB
Go

package persistence
import (
"database/sql"
"errors"
"fmt"
"regexp"
"sort"
"strings"
"sync"
"time"
. "github.com/Masterminds/squirrel"
"github.com/fatih/structs"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
)
const annotationTable = "annotation"
// annotationColumns are the columns withAnnotation's LEFT JOIN contributes, derived from
// model.Annotations so the set tracks schema changes. average_rating is excluded: it lives on the
// base table, not the annotation join.
var annotationColumns = sync.OnceValue(func() map[string]struct{} {
cols := map[string]struct{}{}
for name := range structs.Map(model.Annotations{}) {
if name == "average_rating" {
continue
}
cols[name] = struct{}{}
}
return cols
})
// annotationColumnRE matches any annotation column as a whole word. The word boundaries keep the
// base-table column average_rating from matching the annotation column rating (Go's \b treats '_'
// as a word char). It is case-insensitive because SQLite column names are, so a raw filter using
// e.g. "RATING" must still be detected.
var annotationColumnRE = sync.OnceValue(func() *regexp.Regexp {
cols := make([]string, 0, len(annotationColumns()))
for col := range annotationColumns() {
cols = append(cols, regexp.QuoteMeta(col))
}
sort.Strings(cols) // map iteration is random; sort for a stable pattern
return regexp.MustCompile(`(?i)\b(?:` + strings.Join(cols, "|") + `)\b`)
})
// filtersNeedAnnotation reports whether the rendered query references an annotation column, i.e.
// whether the annotation LEFT JOIN must be kept. Scanning the rendered SQL catches every filter
// path. The placeholder column is needed because squirrel won't render a column-less SELECT; on a
// render error, keep the join to be safe.
func filtersNeedAnnotation(query SelectBuilder) bool {
sql, _, err := query.Columns("1").ToSql()
if err != nil {
return true
}
return annotationColumnRE().MatchString(sql)
}
func (r sqlRepository) withAnnotation(query SelectBuilder, idField string) SelectBuilder {
userID := loggedUser(r.ctx).ID
if userID == invalidUserId {
return query.Columns(fmt.Sprintf("%s.average_rating", r.tableName))
}
query = query.
LeftJoin("annotation on ("+
"annotation.item_id = "+idField+
" AND annotation.item_type = ?"+
" AND annotation.user_id = ?)", r.tableName, userID).
Columns(
"coalesce(starred, 0) as starred",
"coalesce(rating, 0) as rating",
"starred_at",
"play_date",
"rated_at",
)
if conf.Server.AlbumPlayCountMode == consts.AlbumPlayCountModeNormalized && r.tableName == "album" {
query = query.Columns(
fmt.Sprintf("round(coalesce(round(cast(play_count as float) / coalesce(%[1]s.song_count, 1), 1), 0)) as play_count", r.tableName),
)
} else {
query = query.Columns("coalesce(play_count, 0) as play_count")
}
query = query.Columns(fmt.Sprintf("%s.average_rating", r.tableName))
return query
}
func annotationBoolFilter(field string) func(string, any) Sqlizer {
return func(_ string, value any) Sqlizer {
v, ok := value.(string)
if !ok {
return nil
}
if strings.ToLower(v) == "true" {
return Expr(fmt.Sprintf("COALESCE(%s, 0) > 0", field))
}
return Expr(fmt.Sprintf("COALESCE(%s, 0) = 0", field))
}
}
func (r sqlRepository) annId(itemID ...string) And {
userID := loggedUser(r.ctx).ID
return And{
Eq{annotationTable + ".user_id": userID},
Eq{annotationTable + ".item_type": r.tableName},
Eq{annotationTable + ".item_id": itemID},
}
}
func (r sqlRepository) annUpsert(values map[string]any, itemIDs ...string) error {
upd := Update(annotationTable).Where(r.annId(itemIDs...))
for f, v := range values {
upd = upd.Set(f, v)
}
c, err := r.executeSQL(upd)
if c == 0 || errors.Is(err, sql.ErrNoRows) {
userID := loggedUser(r.ctx).ID
for _, itemID := range itemIDs {
values["user_id"] = userID
values["item_type"] = r.tableName
values["item_id"] = itemID
ins := Insert(annotationTable).SetMap(values)
_, err = r.executeSQL(ins)
if err != nil {
return err
}
}
}
return err
}
func (r sqlRepository) SetStar(starred bool, ids ...string) error {
starredAt := time.Now()
return r.annUpsert(map[string]any{"starred": starred, "starred_at": starredAt}, ids...)
}
func (r sqlRepository) SetRating(rating int, itemID string) error {
ratedAt := time.Now()
err := r.annUpsert(map[string]any{"rating": rating, "rated_at": ratedAt}, itemID)
if err != nil {
return err
}
return r.updateAvgRating(itemID)
}
func (r sqlRepository) updateAvgRating(itemID string) error {
upd := Update(r.tableName).
Where(Eq{"id": itemID}).
Set("average_rating", Expr(
"coalesce((select round(avg(rating), 2) from annotation where item_id = ? and item_type = ? and rating > 0), 0)",
itemID, r.tableName,
))
_, err := r.executeSQL(upd)
return err
}
func (r sqlRepository) IncPlayCount(itemID string, ts time.Time) error {
upd := Update(annotationTable).Where(r.annId(itemID)).
Set("play_count", Expr("play_count+1")).
Set("play_date", Expr("max(ifnull(play_date,''),?)", ts))
c, err := r.executeSQL(upd)
if c == 0 || errors.Is(err, sql.ErrNoRows) {
userID := loggedUser(r.ctx).ID
values := map[string]any{}
values["user_id"] = userID
values["item_type"] = r.tableName
values["item_id"] = itemID
values["play_count"] = 1
values["play_date"] = ts
ins := Insert(annotationTable).SetMap(values)
_, err = r.executeSQL(ins)
if err != nil {
return err
}
}
return err
}
func (r sqlRepository) ReassignAnnotation(prevID string, newID string) error {
if prevID == newID || prevID == "" || newID == "" {
return nil
}
upd := Update(annotationTable).Where(And{
Eq{annotationTable + ".item_type": r.tableName},
Eq{annotationTable + ".item_id": prevID},
}).Set("item_id", newID)
_, err := r.executeSQL(upd)
return err
}
func (r sqlRepository) cleanAnnotations() error {
del := Delete(annotationTable).Where(Eq{"item_type": r.tableName}).Where("item_id not in (select id from " + r.tableName + ")")
c, err := r.executeSQL(del)
if err != nil {
return fmt.Errorf("error cleaning up %s annotations: %w", r.tableName, err)
}
if c > 0 {
log.Debug(r.ctx, "Clean-up annotations", "table", r.tableName, "totalDeleted", c)
}
return nil
}