diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index f66df2e75..e48e248c9 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -76,7 +76,8 @@ func CreateNativeAPIRouter(ctx context.Context) *nativeapi.Router { provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher) artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider) cacheWarmer := artwork.NewCacheWarmer(artworkArtwork, fileCache) - modelScanner := scanner.New(ctx, dataStore, cacheWarmer, broker, playlistsPlaylists, metricsMetrics) + smartPlaylistEvaluator := playlists.NewSmartPlaylistEvaluator(dataStore) + modelScanner := scanner.New(ctx, dataStore, cacheWarmer, broker, playlistsPlaylists, smartPlaylistEvaluator, metricsMetrics) watcher := scanner.GetWatcher(dataStore, modelScanner) library := core.NewLibrary(dataStore, modelScanner, watcher, broker, manager) user := core.NewUser(dataStore, manager) @@ -105,7 +106,8 @@ func CreateSubsonicAPIRouter(ctx context.Context) *subsonic.Router { cacheWarmer := artwork.NewCacheWarmer(artworkArtwork, fileCache) imageUploadService := core.NewImageUploadService() playlistsPlaylists := playlists.NewPlaylists(dataStore, imageUploadService) - modelScanner := scanner.New(ctx, dataStore, cacheWarmer, broker, playlistsPlaylists, metricsMetrics) + smartPlaylistEvaluator := playlists.NewSmartPlaylistEvaluator(dataStore) + modelScanner := scanner.New(ctx, dataStore, cacheWarmer, broker, playlistsPlaylists, smartPlaylistEvaluator, metricsMetrics) playTracker := scrobbler.GetPlayTracker(dataStore, broker, manager) playbackServer := playback.GetInstance(dataStore) lyricsLyrics := lyrics.NewLyrics(manager) @@ -177,7 +179,8 @@ func CreateScanner(ctx context.Context) model.Scanner { cacheWarmer := artwork.NewCacheWarmer(artworkArtwork, fileCache) imageUploadService := core.NewImageUploadService() playlistsPlaylists := playlists.NewPlaylists(dataStore, imageUploadService) - modelScanner := scanner.New(ctx, dataStore, cacheWarmer, broker, playlistsPlaylists, metricsMetrics) + smartPlaylistEvaluator := playlists.NewSmartPlaylistEvaluator(dataStore) + modelScanner := scanner.New(ctx, dataStore, cacheWarmer, broker, playlistsPlaylists, smartPlaylistEvaluator, metricsMetrics) return modelScanner } @@ -196,7 +199,8 @@ func CreateScanWatcher(ctx context.Context) scanner.Watcher { cacheWarmer := artwork.NewCacheWarmer(artworkArtwork, fileCache) imageUploadService := core.NewImageUploadService() playlistsPlaylists := playlists.NewPlaylists(dataStore, imageUploadService) - modelScanner := scanner.New(ctx, dataStore, cacheWarmer, broker, playlistsPlaylists, metricsMetrics) + smartPlaylistEvaluator := playlists.NewSmartPlaylistEvaluator(dataStore) + modelScanner := scanner.New(ctx, dataStore, cacheWarmer, broker, playlistsPlaylists, smartPlaylistEvaluator, metricsMetrics) watcher := scanner.GetWatcher(dataStore, modelScanner) return watcher } diff --git a/core/playlists/evaluator.go b/core/playlists/evaluator.go new file mode 100644 index 000000000..d0d3463ca --- /dev/null +++ b/core/playlists/evaluator.go @@ -0,0 +1,105 @@ +package playlists + +import ( + "context" + "maps" + "slices" + "sync" + "time" + + "github.com/navidrome/navidrome/core/auth" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" +) + +// SmartPlaylistEvaluator evaluates smart playlists in the background. +// Call Enqueue to queue a playlist for evaluation. The evaluation happens +// asynchronously in a background goroutine. +type SmartPlaylistEvaluator interface { + Enqueue(playlistID string) +} + +func NewSmartPlaylistEvaluator(ds model.DataStore) SmartPlaylistEvaluator { + e := &smartPlaylistEvaluator{ + ds: ds, + buffer: make(map[string]struct{}), + wakeSignal: make(chan struct{}, 1), + } + go e.run() + return e +} + +type smartPlaylistEvaluator struct { + ds model.DataStore + buffer map[string]struct{} + mutex sync.Mutex + wakeSignal chan struct{} +} + +func (e *smartPlaylistEvaluator) Enqueue(playlistID string) { + e.mutex.Lock() + defer e.mutex.Unlock() + e.buffer[playlistID] = struct{}{} + e.sendWakeSignal() +} + +func (e *smartPlaylistEvaluator) sendWakeSignal() { + select { + case e.wakeSignal <- struct{}{}: + default: + } +} + +func (e *smartPlaylistEvaluator) run() { + for { + e.waitSignal(10 * time.Second) + + e.mutex.Lock() + if len(e.buffer) == 0 { + e.mutex.Unlock() + continue + } + + batch := slices.Collect(maps.Keys(e.buffer)) + e.buffer = make(map[string]struct{}) + e.mutex.Unlock() + + e.processBatch(batch) + } +} + +func (e *smartPlaylistEvaluator) waitSignal(timeout time.Duration) { + timer := time.NewTimer(timeout) + defer timer.Stop() + select { + case <-timer.C: + case <-e.wakeSignal: + } +} + +func (e *smartPlaylistEvaluator) processBatch(batch []string) { + // Use admin context so userFilter() returns all playlists. + // Evaluate() internally uses pls.OwnerID for annotation JOINs. + ctx := auth.WithAdminUser(context.TODO(), e.ds) + + log.Debug(ctx, "Evaluating smart playlists in background", "count", len(batch)) + for _, id := range batch { + start := time.Now() + err := e.ds.Playlist(ctx).Evaluate(id) + if err != nil { + log.Error(ctx, "Error evaluating smart playlist in background", "id", id, err) + continue + } + log.Debug(ctx, "Smart playlist evaluation complete", "id", id, "elapsed", time.Since(start)) + } +} + +// NoopSmartPlaylistEvaluator returns an evaluator that does nothing. +// Used in CLI scan and test contexts. +func NoopSmartPlaylistEvaluator() SmartPlaylistEvaluator { + return &noopSmartPlaylistEvaluator{} +} + +type noopSmartPlaylistEvaluator struct{} + +func (n *noopSmartPlaylistEvaluator) Enqueue(string) {} diff --git a/core/playlists/evaluator_test.go b/core/playlists/evaluator_test.go new file mode 100644 index 000000000..8aad7c814 --- /dev/null +++ b/core/playlists/evaluator_test.go @@ -0,0 +1,17 @@ +package playlists_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/navidrome/navidrome/core/playlists" +) + +var _ = Describe("SmartPlaylistEvaluator", func() { + Describe("NoopSmartPlaylistEvaluator", func() { + It("does not panic when enqueuing", func() { + noop := playlists.NoopSmartPlaylistEvaluator() + Expect(func() { noop.Enqueue("some-id") }).ToNot(Panic()) + }) + }) +}) diff --git a/core/wire_providers.go b/core/wire_providers.go index a2fffa34f..c6cb92d93 100644 --- a/core/wire_providers.go +++ b/core/wire_providers.go @@ -21,6 +21,7 @@ var Set = wire.NewSet( NewPlayers, NewShare, playlists.NewPlaylists, + playlists.NewSmartPlaylistEvaluator, NewLibrary, NewUser, NewMaintenance, diff --git a/model/playlist.go b/model/playlist.go index e2f93993d..ff120c736 100644 --- a/model/playlist.go +++ b/model/playlist.go @@ -131,6 +131,7 @@ type PlaylistRepository interface { Delete(id string) error Tracks(playlistId string, refreshSmartPlaylist bool) PlaylistTrackRepository GetPlaylists(mediaFileId string) (Playlists, error) + Evaluate(id string) error } type PlaylistTrack struct { diff --git a/persistence/playlist_repository.go b/persistence/playlist_repository.go index 8d1bbe0f8..45f63cdac 100644 --- a/persistence/playlist_repository.go +++ b/persistence/playlist_repository.go @@ -310,6 +310,22 @@ func (r *playlistRepository) refreshSmartPlaylist(pls *model.Playlist) bool { return true } +func (r *playlistRepository) Evaluate(id string) error { + pls, err := r.Get(id) + if err != nil { + return err + } + if !pls.IsSmartPlaylist() { + return nil + } + // Reset EvaluatedAt so refreshSmartPlaylist won't skip due to delay check + pls.EvaluatedAt = nil + if !r.refreshSmartPlaylist(pls) { + return fmt.Errorf("failed to evaluate smart playlist %s", id) + } + return nil +} + func (r *playlistRepository) addSmartPlaylistAnnotationJoins(sq SelectBuilder, joins criteria.JoinType, userID string) SelectBuilder { if joins.Has(criteria.JoinAlbumAnnotation) { sq = sq.LeftJoin("annotation AS album_annotation ON ("+ diff --git a/persistence/playlist_repository_test.go b/persistence/playlist_repository_test.go index 88cb5f697..fc2ff2df6 100644 --- a/persistence/playlist_repository_test.go +++ b/persistence/playlist_repository_test.go @@ -254,6 +254,57 @@ var _ = Describe("PlaylistRepository", func() { }) }) + Describe("Evaluate", func() { + var testPlaylistID string + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + }) + + AfterEach(func() { + if testPlaylistID != "" { + _ = repo.Delete(testPlaylistID) + testPlaylistID = "" + } + }) + + It("evaluates a smart playlist and sets EvaluatedAt and SongCount", func() { + rules := &criteria.Criteria{ + Expression: criteria.All{ + criteria.Contains{"title": "Day"}, + }, + } + newPls := model.Playlist{Name: "Evaluate Test", OwnerID: "userid", Rules: rules} + Expect(repo.Put(&newPls)).To(Succeed()) + testPlaylistID = newPls.ID + + Expect(repo.Evaluate(newPls.ID)).To(Succeed()) + + saved, err := repo.Get(newPls.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(saved.EvaluatedAt).ToNot(BeNil()) + Expect(*saved.EvaluatedAt).To(BeTemporally("~", time.Now(), 2*time.Second)) + Expect(saved.SongCount).To(BeNumerically(">", 0)) + }) + + It("is a no-op for non-smart playlists", func() { + newPls := model.Playlist{Name: "Regular Playlist", OwnerID: "userid"} + Expect(repo.Put(&newPls)).To(Succeed()) + testPlaylistID = newPls.ID + + Expect(repo.Evaluate(newPls.ID)).To(Succeed()) + + saved, err := repo.Get(newPls.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(saved.EvaluatedAt).To(BeNil()) + }) + + It("returns ErrNotFound for a non-existent playlist ID", func() { + err := repo.Evaluate("nonexistent-id") + Expect(err).To(MatchError(model.ErrNotFound)) + }) + }) + Describe("Playlist Track Sorting", func() { var testPlaylistID string diff --git a/scanner/controller.go b/scanner/controller.go index 94248ffd0..69b020f6f 100644 --- a/scanner/controller.go +++ b/scanner/controller.go @@ -27,13 +27,14 @@ var ( ) func New(rootCtx context.Context, ds model.DataStore, cw artwork.CacheWarmer, broker events.Broker, - pls playlists.Playlists, m metrics.Metrics) model.Scanner { + pls playlists.Playlists, spe playlists.SmartPlaylistEvaluator, m metrics.Metrics) model.Scanner { c := &controller{ rootCtx: rootCtx, ds: ds, cw: cw, broker: broker, pls: pls, + spe: spe, metrics: m, devExternalScanner: conf.Server.DevExternalScanner, } @@ -47,7 +48,7 @@ func (s *controller) getScanner() scanner { if s.devExternalScanner { return &scannerExternal{} } - return &scannerImpl{ds: s.ds, cw: s.cw, pls: s.pls} + return &scannerImpl{ds: s.ds, cw: s.cw, pls: s.pls, spe: s.spe} } // CallScan starts an in-process scan of specific library/folder pairs. @@ -64,7 +65,7 @@ func CallScan(ctx context.Context, ds model.DataStore, pls playlists.Playlists, progress := make(chan *ProgressInfo, 100) go func() { defer close(progress) - scanner := &scannerImpl{ds: ds, cw: artwork.NoopCacheWarmer(), pls: pls} + scanner := &scannerImpl{ds: ds, cw: artwork.NoopCacheWarmer(), pls: pls, spe: playlists.NoopSmartPlaylistEvaluator()} scanner.scanFolders(ctx, fullScan, targets, progress) }() return progress, nil @@ -99,6 +100,7 @@ type controller struct { broker events.Broker metrics metrics.Metrics pls playlists.Playlists + spe playlists.SmartPlaylistEvaluator limiter *rate.Sometimes devExternalScanner bool count atomic.Uint32 diff --git a/scanner/controller_test.go b/scanner/controller_test.go index d60d432b4..ad71dc278 100644 --- a/scanner/controller_test.go +++ b/scanner/controller_test.go @@ -32,7 +32,7 @@ var _ = Describe("Controller", func() { DeferCleanup(configtest.SetupConfig()) ds = &tests.MockDataStore{RealDS: persistence.New(db.Db())} ds.MockedProperty = &tests.MockedPropertyRepo{} - ctrl = scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(), playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + ctrl = scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(), playlists.NewPlaylists(ds, core.NewImageUploadService()), playlists.NoopSmartPlaylistEvaluator(), metrics.NewNoopInstance()) }) It("includes last scan error", func() { diff --git a/scanner/phase_4_playlists.go b/scanner/phase_4_playlists.go index ab5f77ae0..3c8c6aedc 100644 --- a/scanner/phase_4_playlists.go +++ b/scanner/phase_4_playlists.go @@ -23,16 +23,19 @@ type phasePlaylists struct { ds model.DataStore pls playlists.Playlists cw artwork.CacheWarmer + spe playlists.SmartPlaylistEvaluator refreshed atomic.Uint32 } -func createPhasePlaylists(ctx context.Context, scanState *scanState, ds model.DataStore, pls playlists.Playlists, cw artwork.CacheWarmer) *phasePlaylists { +func createPhasePlaylists(ctx context.Context, scanState *scanState, ds model.DataStore, + pls playlists.Playlists, cw artwork.CacheWarmer, spe playlists.SmartPlaylistEvaluator) *phasePlaylists { return &phasePlaylists{ ctx: ctx, scanState: scanState, ds: ds, pls: pls, cw: cw, + spe: spe, } } @@ -105,7 +108,8 @@ func (p *phasePlaylists) processPlaylistsInFolder(folder *model.Folder) (*model. continue } if pls.IsSmartPlaylist() { - log.Debug("Scanner: Imported smart playlist", "name", pls.Name, "lastUpdated", pls.UpdatedAt, "path", pls.Path, "elapsed", time.Since(started)) + p.spe.Enqueue(pls.ID) + log.Debug(p.ctx, "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)) } diff --git a/scanner/phase_4_playlists_test.go b/scanner/phase_4_playlists_test.go index 06e6fa686..6d6facb82 100644 --- a/scanner/phase_4_playlists_test.go +++ b/scanner/phase_4_playlists_test.go @@ -42,7 +42,7 @@ var _ = Describe("phasePlaylists", func() { pls = &mockPlaylists{} cw = artwork.NoopCacheWarmer() state = &scanState{} - phase = createPhasePlaylists(ctx, state, ds, pls, cw) + phase = createPhasePlaylists(ctx, state, ds, pls, cw, playlists.NoopSmartPlaylistEvaluator()) }) Describe("description", func() { diff --git a/scanner/scanner.go b/scanner/scanner.go index 871b0c696..b7675afc8 100644 --- a/scanner/scanner.go +++ b/scanner/scanner.go @@ -24,6 +24,7 @@ type scannerImpl struct { ds model.DataStore cw artwork.CacheWarmer pls playlists.Playlists + spe playlists.SmartPlaylistEvaluator } // scanState holds the state of an in-progress scan, to be passed to the various phases @@ -148,7 +149,7 @@ func (s *scannerImpl) scanFolders(ctx context.Context, fullScan bool, targets [] runPhase[*model.Album](ctx, 3, createPhaseRefreshAlbums(ctx, &state, s.ds)), // Phase 4: Import/update playlists - runPhase[*model.Folder](ctx, 4, createPhasePlaylists(ctx, &state, s.ds, s.pls, s.cw)), + runPhase[*model.Folder](ctx, 4, createPhasePlaylists(ctx, &state, s.ds, s.pls, s.cw, s.spe)), ), // Final Steps (cannot be parallelized): diff --git a/scanner/scanner_benchmark_test.go b/scanner/scanner_benchmark_test.go index 8f0dcd340..60b7ace13 100644 --- a/scanner/scanner_benchmark_test.go +++ b/scanner/scanner_benchmark_test.go @@ -41,7 +41,7 @@ func BenchmarkScan(b *testing.B) { ds := persistence.New(db.Db()) conf.Server.DevExternalScanner = false s := scanner.New(context.Background(), ds, artwork.NoopCacheWarmer(), events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService()), playlists.NoopSmartPlaylistEvaluator(), metrics.NewNoopInstance()) fs := storagetest.FakeFS{} storagetest.Register("fake", &fs) diff --git a/scanner/scanner_multilibrary_test.go b/scanner/scanner_multilibrary_test.go index 3ae50933c..6972333e2 100644 --- a/scanner/scanner_multilibrary_test.go +++ b/scanner/scanner_multilibrary_test.go @@ -79,7 +79,7 @@ var _ = Describe("Scanner - Multi-Library", Ordered, func() { Expect(ds.User(ctx).Put(&adminUser)).To(Succeed()) s = scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService()), playlists.NoopSmartPlaylistEvaluator(), metrics.NewNoopInstance()) // Create two test libraries (let DB auto-assign IDs) lib1 = model.Library{Name: "Rock Collection", Path: "rock:///music"} diff --git a/scanner/scanner_selective_test.go b/scanner/scanner_selective_test.go index 6c70eb268..072870ac8 100644 --- a/scanner/scanner_selective_test.go +++ b/scanner/scanner_selective_test.go @@ -65,7 +65,7 @@ var _ = Describe("ScanFolders", Ordered, func() { Expect(ds.User(ctx).Put(&adminUser)).To(Succeed()) s = scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService()), playlists.NoopSmartPlaylistEvaluator(), metrics.NewNoopInstance()) lib = model.Library{ID: 1, Name: "Fake Library", Path: "fake:///music"} Expect(ds.Library(ctx).Put(&lib)).To(Succeed()) diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go index 7bf91d64f..9626f0ea5 100644 --- a/scanner/scanner_test.go +++ b/scanner/scanner_test.go @@ -85,7 +85,7 @@ var _ = Describe("Scanner", Ordered, func() { Expect(ds.User(ctx).Put(&adminUser)).To(Succeed()) s = scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService()), playlists.NoopSmartPlaylistEvaluator(), metrics.NewNoopInstance()) lib = model.Library{ID: 1, Name: "Fake Library", Path: "fake:///music"} Expect(ds.Library(ctx).Put(&lib)).To(Succeed()) diff --git a/server/e2e/e2e_suite_test.go b/server/e2e/e2e_suite_test.go index 4ad9e3daa..95d4d0b22 100644 --- a/server/e2e/e2e_suite_test.go +++ b/server/e2e/e2e_suite_test.go @@ -458,7 +458,7 @@ var _ = BeforeSuite(func() { buildTestFS() s := scanner.New(ctx, initDS, artwork.NoopCacheWarmer(), events.NoopBroker(), - playlists.NewPlaylists(initDS, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(initDS, core.NewImageUploadService()), playlists.NoopSmartPlaylistEvaluator(), metrics.NewNoopInstance()) _, err = s.ScanAll(ctx, true) Expect(err).ToNot(HaveOccurred()) @@ -502,7 +502,7 @@ func setupTestDB() { streamerSpy = &spyStreamer{} decider := stream.NewTranscodeDecider(ds, noopFFmpeg{}) s := scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService()), playlists.NoopSmartPlaylistEvaluator(), metrics.NewNoopInstance()) router = subsonic.New( ds, noopArtwork{}, diff --git a/server/e2e/subsonic_multilibrary_test.go b/server/e2e/subsonic_multilibrary_test.go index a837da124..88f4d4cd3 100644 --- a/server/e2e/subsonic_multilibrary_test.go +++ b/server/e2e/subsonic_multilibrary_test.go @@ -54,7 +54,7 @@ var _ = Describe("Multi-Library Support", Ordered, func() { // Run incremental scan to import lib2 content (lib1 files unchanged → skipped) s := scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService()), playlists.NoopSmartPlaylistEvaluator(), metrics.NewNoopInstance()) _, err = s.ScanAll(ctx, false) Expect(err).ToNot(HaveOccurred()) diff --git a/tests/mock_playlist_repo.go b/tests/mock_playlist_repo.go index 9bdc52152..9717e49d2 100644 --- a/tests/mock_playlist_repo.go +++ b/tests/mock_playlist_repo.go @@ -108,4 +108,11 @@ func (m *MockPlaylistRepo) CountAll(_ ...model.QueryOptions) (int64, error) { return int64(len(m.Data)), nil } +func (m *MockPlaylistRepo) Evaluate(_ string) error { + if m.Err { + return errors.New("error") + } + return nil +} + var _ model.PlaylistRepository = (*MockPlaylistRepo)(nil)