mirror of
https://github.com/navidrome/navidrome.git
synced 2026-07-30 16:56:22 -04: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.
207 lines
6.2 KiB
Go
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
|
|
}
|