mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00: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.
213 lines
7.1 KiB
Go
213 lines
7.1 KiB
Go
package persistence
|
|
|
|
import (
|
|
"context"
|
|
"database/sql"
|
|
"reflect"
|
|
"time"
|
|
|
|
"github.com/navidrome/navidrome/db"
|
|
"github.com/navidrome/navidrome/log"
|
|
"github.com/navidrome/navidrome/model"
|
|
"github.com/navidrome/navidrome/utils/run"
|
|
"github.com/pocketbase/dbx"
|
|
)
|
|
|
|
type SQLStore struct {
|
|
db dbx.Builder
|
|
}
|
|
|
|
func New(conn *sql.DB) model.DataStore {
|
|
return &SQLStore{db: dbx.NewFromDB(conn, db.Driver)}
|
|
}
|
|
|
|
func (s *SQLStore) Album(ctx context.Context) model.AlbumRepository {
|
|
return NewAlbumRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Artist(ctx context.Context) model.ArtistRepository {
|
|
return NewArtistRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) MediaFile(ctx context.Context) model.MediaFileRepository {
|
|
return NewMediaFileRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Library(ctx context.Context) model.LibraryRepository {
|
|
return NewLibraryRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Folder(ctx context.Context) model.FolderRepository {
|
|
return newFolderRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Genre(ctx context.Context) model.GenreRepository {
|
|
return NewGenreRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Tag(ctx context.Context) model.TagRepository {
|
|
return NewTagRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) PlayQueue(ctx context.Context) model.PlayQueueRepository {
|
|
return NewPlayQueueRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Playlist(ctx context.Context) model.PlaylistRepository {
|
|
return NewPlaylistRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Property(ctx context.Context) model.PropertyRepository {
|
|
return NewPropertyRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Radio(ctx context.Context) model.RadioRepository {
|
|
return NewRadioRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) UserProps(ctx context.Context) model.UserPropsRepository {
|
|
return NewUserPropsRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Share(ctx context.Context) model.ShareRepository {
|
|
return NewShareRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) User(ctx context.Context) model.UserRepository {
|
|
return NewUserRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Transcoding(ctx context.Context) model.TranscodingRepository {
|
|
return NewTranscodingRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Player(ctx context.Context) model.PlayerRepository {
|
|
return NewPlayerRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) ScrobbleBuffer(ctx context.Context) model.ScrobbleBufferRepository {
|
|
return NewScrobbleBufferRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Scrobble(ctx context.Context) model.ScrobbleRepository {
|
|
return NewScrobbleRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Plugin(ctx context.Context) model.PluginRepository {
|
|
return NewPluginRepository(ctx, s.getDBXBuilder())
|
|
}
|
|
|
|
func (s *SQLStore) Resource(ctx context.Context, m any) model.ResourceRepository {
|
|
switch m.(type) {
|
|
case model.User:
|
|
return s.User(ctx).(model.ResourceRepository)
|
|
case model.Transcoding:
|
|
return s.Transcoding(ctx).(model.ResourceRepository)
|
|
case model.Player:
|
|
return s.Player(ctx).(model.ResourceRepository)
|
|
case model.Artist:
|
|
return s.Artist(ctx).(model.ResourceRepository)
|
|
case model.Album:
|
|
return s.Album(ctx).(model.ResourceRepository)
|
|
case model.MediaFile:
|
|
return s.MediaFile(ctx).(model.ResourceRepository)
|
|
case model.Genre:
|
|
return s.Genre(ctx).(model.ResourceRepository)
|
|
case model.Playlist:
|
|
return s.Playlist(ctx).(model.ResourceRepository)
|
|
case model.Radio:
|
|
return s.Radio(ctx).(model.ResourceRepository)
|
|
case model.Share:
|
|
return s.Share(ctx).(model.ResourceRepository)
|
|
case model.Tag:
|
|
return s.Tag(ctx).(model.ResourceRepository)
|
|
case model.Plugin:
|
|
return s.Plugin(ctx).(model.ResourceRepository)
|
|
case model.Scrobble:
|
|
return s.Scrobble(ctx).(model.ResourceRepository)
|
|
}
|
|
log.Error("Resource not implemented", "model", reflect.TypeOf(m).Name())
|
|
return nil
|
|
}
|
|
|
|
func (s *SQLStore) WithTx(block func(tx model.DataStore) error, scope ...string) error {
|
|
var msg string
|
|
if len(scope) > 0 {
|
|
msg = scope[0]
|
|
}
|
|
start := time.Now()
|
|
conn, inTx := s.db.(*dbx.DB)
|
|
if !inTx {
|
|
log.Trace("Nested Transaction started", "scope", msg)
|
|
conn = dbx.NewFromDB(db.Db(), db.Driver)
|
|
} else {
|
|
log.Trace("Transaction started", "scope", msg)
|
|
}
|
|
return conn.Transactional(func(tx *dbx.Tx) error {
|
|
newDb := &SQLStore{db: tx}
|
|
err := block(newDb)
|
|
if !inTx {
|
|
log.Trace("Nested Transaction finished", "scope", msg, "elapsed", time.Since(start), err)
|
|
} else {
|
|
log.Trace("Transaction finished", "scope", msg, "elapsed", time.Since(start), err)
|
|
}
|
|
return err
|
|
})
|
|
}
|
|
|
|
func (s *SQLStore) WithTxImmediate(block func(tx model.DataStore) error, scope ...string) error {
|
|
ctx := context.Background()
|
|
return s.WithTx(func(tx model.DataStore) error {
|
|
// Workaround to force the transaction to be upgraded to immediate mode to avoid deadlocks
|
|
// See https://berthub.eu/articles/posts/a-brief-post-on-sqlite3-database-locked-despite-timeout/
|
|
_ = tx.Property(ctx).Put("tmp_lock_flag", "")
|
|
defer func() {
|
|
_ = tx.Property(ctx).Delete("tmp_lock_flag")
|
|
}()
|
|
|
|
return block(tx)
|
|
}, scope...)
|
|
}
|
|
|
|
func (s *SQLStore) GC(ctx context.Context, libraryIDs ...int) error {
|
|
trace := func(ctx context.Context, msg string, f func() error) func() error {
|
|
return func() error {
|
|
start := time.Now()
|
|
err := f()
|
|
log.Debug(ctx, "GC: "+msg, "elapsed", time.Since(start), err)
|
|
return err
|
|
}
|
|
}
|
|
|
|
// If libraryIDs are provided, scope operations to those libraries where possible
|
|
scoped := len(libraryIDs) > 0
|
|
if scoped {
|
|
log.Debug(ctx, "GC: Running selective garbage collection", "libraryIDs", libraryIDs)
|
|
}
|
|
|
|
err := run.Sequentially(
|
|
trace(ctx, "purge empty albums", func() error { return s.Album(ctx).(*albumRepository).purgeEmpty(libraryIDs...) }),
|
|
trace(ctx, "purge empty artists", func() error { return s.Artist(ctx).(*artistRepository).purgeEmpty() }),
|
|
trace(ctx, "mark missing artists", func() error { return s.Artist(ctx).(*artistRepository).markMissing() }),
|
|
trace(ctx, "purge empty folders", func() error { return s.Folder(ctx).(*folderRepository).purgeEmpty(libraryIDs...) }),
|
|
trace(ctx, "clean album annotations", func() error { return s.Album(ctx).(*albumRepository).cleanAnnotations() }),
|
|
trace(ctx, "clean artist annotations", func() error { return s.Artist(ctx).(*artistRepository).cleanAnnotations() }),
|
|
trace(ctx, "clean media file annotations", func() error { return s.MediaFile(ctx).(*mediaFileRepository).cleanAnnotations() }),
|
|
trace(ctx, "clean playlist annotations", func() error { return s.Playlist(ctx).(*playlistRepository).cleanAnnotations() }),
|
|
trace(ctx, "clean media file bookmarks", func() error { return s.MediaFile(ctx).(*mediaFileRepository).cleanBookmarks() }),
|
|
trace(ctx, "purge non used tags", func() error { return s.Tag(ctx).(*tagRepository).purgeUnused() }),
|
|
trace(ctx, "remove orphan playlist tracks", func() error { return s.Playlist(ctx).(*playlistRepository).removeOrphans() }),
|
|
)
|
|
if err != nil {
|
|
log.Error(ctx, "Error tidying up database", err)
|
|
}
|
|
return err
|
|
}
|
|
|
|
func (s *SQLStore) getDBXBuilder() dbx.Builder {
|
|
if s.db == nil {
|
|
return dbx.NewFromDB(db.Db(), db.Driver)
|
|
}
|
|
return s.db
|
|
}
|