Merge e510f3460426fc8d7e913d92b1f935ac1d5ee8e3 into e03ce871773c434b95ec2a623243c6397df160f5

This commit is contained in:
Deluan Quintão 2026-09-29 11:21:25 -04:00 committed by GitHub
commit ff0117c8f8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 228 additions and 45 deletions

View File

@ -221,6 +221,8 @@ type PlaylistRepository interface {
FindByPath(ctx context.Context, path string) (*Playlist, error)
Tracks(ctx context.Context, playlistId string, refreshSmartPlaylist bool) PlaylistTrackRepository
GetPlaylists(ctx context.Context, mediaFileId string) (Playlists, error)
// Evaluate refreshes a smart playlist's tracks as its owner, ignoring visibility and the refresh delay.
Evaluate(ctx context.Context, id string) error
}
type PlaylistTrack struct {

View File

@ -260,6 +260,17 @@ func (r *playlistRepository) selectPlaylist(ctx context.Context, options ...mode
return r.withAnnotation(ctx, sel, r.tableName+".id")
}
// inTx runs fn in a transaction, joining the caller's if one is already open.
func (r *playlistRepository) inTx(fn func(tx *playlistRepository) error) error {
conn, ok := r.db.(*dbx.DB)
if !ok {
return fn(r)
}
return conn.Transactional(func(tx *dbx.Tx) error {
return fn(NewPlaylistRepository(tx).(*playlistRepository))
})
}
func (r *playlistRepository) updateTracks(ctx context.Context, id string, tracks model.MediaFiles) error {
ids := make([]string, len(tracks))
for i := range tracks {

View File

@ -10,7 +10,6 @@ import (
"github.com/deluan/rest"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/pocketbase/dbx"
)
@ -78,7 +77,7 @@ func (r *shareRepository) loadMedia(ctx context.Context, share *model.Share) err
return And{cond, Eq{"missing": false}}
}
// Load as the share owner so their library access is applied, whoever renders the share.
ownerCtx, err := r.ownerContext(ctx, share)
ownerCtx, err := r.ownerContext(ctx, share.UserID)
if err != nil {
return err
}
@ -128,19 +127,6 @@ func (r *shareRepository) loadMedia(ctx context.Context, share *model.Share) err
return nil
}
// ownerContext returns a context scoped to the share owner, so repository
// queries apply the owner's library access when a public share is rendered.
func (r *shareRepository) ownerContext(ctx context.Context, share *model.Share) (context.Context, error) {
owner, err := NewUserRepository(r.db).Get(ctx, share.UserID)
if err != nil {
return nil, fmt.Errorf("loading share owner %q: %w", share.UserID, err)
}
if owner == nil {
return nil, fmt.Errorf("share owner %q not found", share.UserID)
}
return request.WithUser(ctx, *owner), nil
}
func sortByIdPosition(mfs model.MediaFiles, ids []string) model.MediaFiles {
m := map[string]int{}
for i, mf := range mfs {

View File

@ -2,6 +2,9 @@ package persistence
import (
"context"
"encoding/json"
"errors"
"fmt"
"slices"
"time"
@ -18,6 +21,23 @@ import (
// in its criteria. To optimize performance, it only refreshes when necessary based on the last evaluated time and
// configured refresh delay.
func (r *playlistRepository) Evaluate(ctx context.Context, id string) error {
var res dbPlaylist
if err := r.queryOne(ctx, r.selectPlaylist(ctx).Where(Eq{"playlist.id": id}), &res); err != nil {
return err
}
pls := res.Playlist
ownerCtx, err := r.ownerContext(ctx, pls.OwnerID)
if err != nil {
return err
}
pls.EvaluatedAt = nil
if !r.refreshSmartPlaylist(ownerCtx, &pls) {
return fmt.Errorf("evaluating smart playlist %s", id)
}
return nil
}
// refreshSmartPlaylist evaluates the criteria of a smart playlist and updates its tracks accordingly.
func (r *playlistRepository) refreshSmartPlaylist(ctx context.Context, pls *model.Playlist) bool {
return r.refreshSmartPlaylistTree(ctx, pls, map[string]struct{}{})
@ -39,12 +59,6 @@ func (r *playlistRepository) refreshSmartPlaylistTree(ctx context.Context, pls *
log.Debug(ctx, "Refreshing smart playlist", "playlist", pls.Name, "id", pls.ID)
start := time.Now()
del := Delete("playlist_tracks").Where(Eq{"playlist_id": pls.ID})
if _, err := r.executeSQL(ctx, del); err != nil {
log.Error(ctx, "Error deleting old smart playlist tracks", "playlist", pls.Name, "id", pls.ID, err)
return false
}
rulesSQL := newSmartPlaylistCriteria(*pls.NormalizedRules(), withSmartPlaylistOwner(*usr))
if !r.refreshChildPlaylists(ctx, pls, rulesSQL, visited) {
@ -55,37 +69,56 @@ func (r *playlistRepository) refreshSmartPlaylistTree(ctx context.Context, pls *
return false
}
sq := r.buildSmartPlaylistQuery(ctx, pls, rulesSQL, usr.ID)
sq, err := r.addCriteria(sq, rulesSQL)
sq, err := r.addCriteria(r.buildSmartPlaylistQuery(ctx, rulesSQL, usr.ID), rulesSQL)
if err != nil {
log.Error(ctx, "Error building smart playlist criteria", "playlist", pls.Name, "id", pls.ID, err)
return false
}
insSql := Insert("playlist_tracks").Columns("id", "playlist_id", "media_file_id").Select(sq)
if _, err = r.executeSQL(ctx, insSql); err != nil {
// Evaluate the criteria before writing, so the write lock is only held for the short replace below
var ids []string
if err = r.queryAllSlice(ctx, sq, &ids); err != nil && !errors.Is(err, model.ErrNotFound) {
log.Error(ctx, "Error evaluating smart playlist criteria", "playlist", pls.Name, "id", pls.ID, err)
return false
}
err = r.inTx(func(tx *playlistRepository) error { return tx.replaceSmartPlaylistTracks(ctx, pls, ids) })
if err != nil {
log.Error(ctx, "Error refreshing smart playlist tracks", "playlist", pls.Name, "id", pls.ID, err)
return false
}
if err = r.refreshCounters(ctx, pls); err != nil {
log.Error(ctx, "Error updating smart playlist stats", "playlist", pls.Name, "id", pls.ID, err)
return false
}
// Reuse the stamp refreshCounters just wrote, so evaluated_at and updated_at agree
now := pls.UpdatedAt
updSql := Update(r.tableName).Set("evaluated_at", now).Where(Eq{"id": pls.ID})
if _, err = r.executeSQL(ctx, updSql); err != nil {
log.Error(ctx, "Error updating smart playlist", "playlist", pls.Name, "id", pls.ID, err)
return false
}
pls.EvaluatedAt = &now
log.Debug(ctx, "Refreshed playlist", "playlist", pls.Name, "id", pls.ID, "numTracks", pls.SongCount, "elapsed", time.Since(start))
return true
}
func (r *playlistRepository) replaceSmartPlaylistTracks(ctx context.Context, pls *model.Playlist, ids []string) error {
if _, err := r.executeSQL(ctx, Delete("playlist_tracks").Where(Eq{"playlist_id": pls.ID})); err != nil {
return err
}
if len(ids) > 0 {
idsJSON, err := json.Marshal(ids)
if err != nil {
return err
}
ins := Expr("INSERT INTO playlist_tracks (id, playlist_id, media_file_id) SELECT key + 1, ?, value FROM json_each(?)",
pls.ID, string(idsJSON))
if _, err = r.executeSQL(ctx, ins); err != nil {
return err
}
}
if err := r.refreshCounters(ctx, pls); err != nil {
return err
}
// Reuse the stamp refreshCounters just wrote, so evaluated_at and updated_at agree
now := pls.UpdatedAt
if _, err := r.executeSQL(ctx, Update(r.tableName).Set("evaluated_at", now).Where(Eq{"id": pls.ID})); err != nil {
return err
}
pls.EvaluatedAt = &now
return nil
}
// shouldRefreshSmartPlaylist determines if a smart playlist needs to be refreshed based on its type, last evaluated
// time, and ownership.
func (r *playlistRepository) shouldRefreshSmartPlaylist(ctx context.Context, pls *model.Playlist, usr *model.User) bool {
@ -176,12 +209,10 @@ func (r *playlistRepository) resolvePercentageLimit(ctx context.Context, pls *mo
return nil
}
// buildSmartPlaylistQuery constructs the SQL query to select media files matching the smart playlist criteria,
// including the joins its fields require and library filtering.
func (r *playlistRepository) buildSmartPlaylistQuery(ctx context.Context, pls *model.Playlist, rulesSQL smartPlaylistCriteria, userID string) SelectBuilder {
orderBy := rulesSQL.orderBy()
sq := Select("row_number() over (order by "+orderBy+") as id", "'"+pls.ID+"' as playlist_id", "media_file.id as media_file_id").
From("media_file")
// buildSmartPlaylistQuery constructs the SQL query to select the ids of media files matching the smart playlist
// criteria, including the joins its fields require and library filtering.
func (r *playlistRepository) buildSmartPlaylistQuery(ctx context.Context, rulesSQL smartPlaylistCriteria, userID string) SelectBuilder {
sq := Select("media_file.id").From("media_file")
sq = rulesSQL.applyRequiredJoins(sq, userID)
sq = r.applyLibraryFilter(ctx, sq, "media_file")
return sq

View File

@ -330,6 +330,47 @@ var _ = Describe("PlaylistRepository - Smart Playlists", func() {
})
})
Describe("Evaluate", func() {
dayRules := func() *criteria.Criteria {
return &criteria.Criteria{
Expression: criteria.All{criteria.Contains{"title": "Day"}},
RefreshDelay: 24 * time.Hour,
}
}
It("evaluates even when the refresh delay has not elapsed", func() {
evaluatedAt := time.Now().Add(-1 * time.Hour)
pls := model.Playlist{Name: "Evaluate Delay", OwnerID: "userid", Rules: dayRules(), EvaluatedAt: &evaluatedAt}
Expect(repo.Put(ctx, &pls)).To(Succeed())
DeferCleanup(func() { _ = repo.Delete(ctx, pls.ID) })
Expect(repo.Evaluate(ctx, pls.ID)).To(Succeed())
got, err := repo.Get(ctx, pls.ID)
Expect(err).ToNot(HaveOccurred())
Expect(*got.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second))
Expect(got.SongCount).To(Equal(1))
})
It("evaluates as the owner, even when the caller cannot see the playlist", func() {
pls := model.Playlist{Name: "Evaluate Owner", OwnerID: regularUser.ID, Rules: dayRules()}
Expect(repo.Put(ctx, &pls)).To(Succeed())
DeferCleanup(func() { _ = repo.Delete(ctx, pls.ID) })
otherCtx := request.WithUser(log.NewContext(GinkgoT().Context()), thirdUser)
Expect(repo.Evaluate(otherCtx, pls.ID)).To(Succeed())
got, err := repo.Get(ctx, pls.ID)
Expect(err).ToNot(HaveOccurred())
Expect(got.EvaluatedAt).ToNot(BeNil())
Expect(got.SongCount).To(Equal(1))
})
It("returns ErrNotFound for an unknown playlist", func() {
Expect(repo.Evaluate(ctx, "nonexistent-id")).To(MatchError(model.ErrNotFound))
})
})
Describe("Playlist Track Sorting", func() {
var testPlaylistID string

View File

@ -58,6 +58,18 @@ func loggedUser(ctx context.Context) *model.User {
}
}
// ownerContext scopes ctx to the given user, so queries apply that user's library access and annotations.
func (r sqlRepository) ownerContext(ctx context.Context, userID string) (context.Context, error) {
owner, err := NewUserRepository(r.db).Get(ctx, userID)
if err != nil {
return nil, fmt.Errorf("loading owner %q: %w", userID, err)
}
if owner == nil {
return nil, fmt.Errorf("owner %q not found", userID)
}
return request.WithUser(ctx, *owner), nil
}
// ownerFilter returns the predicate restricting access to rows owned by the logged-in user, for
// tables with a user_id column. It returns nil for admins and for headless/system contexts (invalid
// user), meaning "no ownership restriction". Callers should skip the WHERE clause when it is nil.

View File

@ -144,6 +144,10 @@ func (p *phasePlaylists) processPlaylistsInFolder(folder *model.Folder) (*model.
continue
}
if pls.IsSmartPlaylist() {
// A nil EvaluatedAt means the playlist is new or its file changed
if pls.ID != "" && pls.EvaluatedAt == nil {
p.scanState.queueSmartPlaylist(pls.ID)
}
log.Debug("Scanner: Imported smart playlist", "name", pls.Name, "lastUpdated", pls.UpdatedAt, "path", pls.Path, "elapsed", time.Since(started))
} else {
log.Debug("Scanner: Imported playlist", "name", pls.Name, "lastUpdated", pls.UpdatedAt, "path", pls.Path, "numTracks", len(pls.Tracks), "elapsed", time.Since(started))

View File

@ -6,12 +6,14 @@ import (
"os"
"path/filepath"
"sort"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/playlists"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/criteria"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
@ -213,6 +215,27 @@ var _ = Describe("phasePlaylists", func() {
)))
})
It("queues only smart playlists that were never evaluated", func() {
libPath := GinkgoT().TempDir()
folder := &model.Folder{LibraryPath: libPath, Path: "path/to", Name: "folder"}
_ = os.MkdirAll(folder.AbsolutePath(), 0755)
for _, name := range []string{"new.nsp", "evaluated.nsp", "regular.m3u"} {
_ = os.WriteFile(filepath.Join(folder.AbsolutePath(), name), []byte{}, 0600)
}
rules := &criteria.Criteria{Expression: criteria.All{criteria.Contains{"title": "Day"}}}
pls.On("ImportFromFolder", mock.Anything, folder, "new.nsp").
Return(&model.Playlist{ID: "new", Rules: rules}, nil)
pls.On("ImportFromFolder", mock.Anything, folder, "evaluated.nsp").
Return(&model.Playlist{ID: "evaluated", Rules: rules, EvaluatedAt: new(time.Now())}, nil)
pls.On("ImportFromFolder", mock.Anything, folder, "regular.m3u").
Return(&model.Playlist{ID: "regular"}, nil)
_, err := phase.processPlaylistsInFolder(folder)
Expect(err).ToNot(HaveOccurred())
Expect(state.smartPlaylistsToEvaluate()).To(ConsistOf("new"))
})
It("reports an error if there is an error reading files", func() {
tests.SkipOnWindows("relies on Unix /etc filesystem")
progress := make(chan *ProgressInfo)

View File

@ -6,6 +6,7 @@ import (
"maps"
"path/filepath"
"slices"
"sync"
"sync/atomic"
"time"
@ -32,6 +33,21 @@ type scanState struct {
libraries model.Libraries // Store libraries list for consistency across phases
targets map[int][]string // Optional: map[libraryID][]folderPaths for selective scans
totalLibraryCount int // Total number of libraries (unfiltered), for cross-library move detection
smartPlaylistsMu sync.Mutex
smartPlaylists []string
}
func (s *scanState) queueSmartPlaylist(id string) {
s.smartPlaylistsMu.Lock()
defer s.smartPlaylistsMu.Unlock()
s.smartPlaylists = append(s.smartPlaylists, id)
}
func (s *scanState) smartPlaylistsToEvaluate() []string {
s.smartPlaylistsMu.Lock()
defer s.smartPlaylistsMu.Unlock()
return s.smartPlaylists
}
func (s *scanState) sendProgress(info *ProgressInfo) {
@ -187,6 +203,9 @@ func (s *scannerImpl) scanFolders(ctx context.Context, fullScan bool, targets []
// Update last_scan_completed_at for all libraries
s.runUpdateLibraries(ctx, &state),
// Evaluate new/changed smart playlists last, so their rules see the final library state
s.runEvaluateSmartPlaylists(ctx, &state),
)
if err != nil {
log.Error(ctx, "Scanner: Finished with error", "duration", time.Since(startTime), err)
@ -332,6 +351,21 @@ func (s *scannerImpl) runRefreshStats(ctx context.Context, state *scanState) fun
}
}
// Failures are logged but never fail the scan: the playlist is still evaluated when it is next read.
func (s *scannerImpl) runEvaluateSmartPlaylists(ctx context.Context, state *scanState) func() error {
return func() error {
for _, id := range state.smartPlaylistsToEvaluate() {
start := time.Now()
if err := s.ds.Playlist().Evaluate(ctx, id); err != nil {
log.Warn(ctx, "Scanner: Could not evaluate smart playlist", "id", id, err)
continue
}
log.Debug(ctx, "Scanner: Evaluated smart playlist", "id", id, "elapsed", time.Since(start))
}
return nil
}
}
func (s *scannerImpl) runUpdateLibraries(ctx context.Context, state *scanState) func() error {
return func() error {
start := time.Now()

View File

@ -9,6 +9,7 @@ import (
"sync/atomic"
ppl "github.com/google/go-pipeline/pkg/pipeline"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
@ -135,3 +136,32 @@ var _ = Describe("runPhase", func() {
Expect(counter.Load()).To(Equal(int64(3)))
})
})
var _ = Describe("runEvaluateSmartPlaylists", func() {
var ctx context.Context
var plsRepo *tests.MockPlaylistRepo
var s *scannerImpl
var state *scanState
BeforeEach(func() {
ctx = GinkgoT().Context()
plsRepo = tests.CreateMockPlaylistRepo()
s = &scannerImpl{ds: &tests.MockDataStore{MockedPlaylist: plsRepo}}
state = &scanState{}
})
It("evaluates every queued smart playlist", func() {
state.queueSmartPlaylist("p1")
state.queueSmartPlaylist("p2")
Expect(s.runEvaluateSmartPlaylists(ctx, state)()).To(Succeed())
Expect(plsRepo.Evaluated).To(Equal([]string{"p1", "p2"}))
})
It("does not fail the scan when an evaluation fails", func() {
plsRepo.SetError(true)
state.queueSmartPlaylist("p1")
Expect(s.runEvaluateSmartPlaylists(ctx, state)()).To(Succeed())
})
})

View File

@ -30,6 +30,7 @@ type MockPlaylistRepo struct {
Err bool
TracksRepo model.PlaylistTrackRepository
TracksRefreshed bool
Evaluated []string
}
func (m *MockPlaylistRepo) SetError(err bool) {
@ -186,4 +187,12 @@ func (m *MockPlaylistRepo) CountAll(_ context.Context, _ ...model.QueryOptions)
return int64(len(m.Data)), nil
}
func (m *MockPlaylistRepo) Evaluate(_ context.Context, id string) error {
if m.Err {
return errors.New("error")
}
m.Evaluated = append(m.Evaluated, id)
return nil
}
var _ model.PlaylistRepository = (*MockPlaylistRepo)(nil)