From 043f79d746291234f06c28cdda7a4cd8f327dbca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Fri, 13 Jun 2025 00:06:08 -0400 Subject: [PATCH] feat(ui): add EnableNowPlaying configuration (default true) (#4219) * Add EnableNowPlaying config option * Return 501 for disabled NowPlaying * chore(tests): remove get_now_playing_route test * Disable now playing events when disabled * fix(tests): add mutex for thread-safe access to scrobble buffer Signed-off-by: Deluan --------- Signed-off-by: Deluan --- conf/configuration.go | 2 + core/metrics/insights.go | 1 + core/metrics/insights/data.go | 1 + core/scrobbler/play_tracker.go | 17 +++++--- core/scrobbler/play_tracker_test.go | 18 ++++++++ server/serve_index.go | 1 + server/serve_index_test.go | 11 +++++ server/subsonic/api.go | 6 ++- tests/mock_data_store.go | 4 ++ tests/mock_scrobble_buffer_repo.go | 12 ++++++ ui/src/config.js | 1 + ui/src/eventStream.js | 5 ++- ui/src/layout/AppBar.jsx | 6 +-- ui/src/layout/AppBar.test.jsx | 65 +++++++++++++++++++++++++++++ 14 files changed, 139 insertions(+), 11 deletions(-) create mode 100644 ui/src/layout/AppBar.test.jsx diff --git a/conf/configuration.go b/conf/configuration.go index c3a08bbfa..dc7e75b7e 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -80,6 +80,7 @@ type configOptions struct { DefaultUIVolume int EnableReplayGain bool EnableCoverAnimation bool + EnableNowPlaying bool GATrackingID string EnableLogRedacting bool AuthRequestLimit int @@ -491,6 +492,7 @@ func setViperDefaults() { viper.SetDefault("defaultuivolume", consts.DefaultUIVolume) viper.SetDefault("enablereplaygain", true) viper.SetDefault("enablecoveranimation", true) + viper.SetDefault("enablenowplaying", true) viper.SetDefault("enablesharing", false) viper.SetDefault("shareurl", "") viper.SetDefault("defaultshareexpiration", 8760*time.Hour) diff --git a/core/metrics/insights.go b/core/metrics/insights.go index 6076be0a5..29284a908 100644 --- a/core/metrics/insights.go +++ b/core/metrics/insights.go @@ -176,6 +176,7 @@ var staticData = sync.OnceValue(func() insights.Data { data.Config.DefaultBackgroundURLSet = conf.Server.UILoginBackgroundURL == consts.DefaultUILoginBackgroundURL data.Config.EnableArtworkPrecache = conf.Server.EnableArtworkPrecache data.Config.EnableCoverAnimation = conf.Server.EnableCoverAnimation + data.Config.EnableNowPlaying = conf.Server.EnableNowPlaying data.Config.EnableDownloads = conf.Server.EnableDownloads data.Config.EnableSharing = conf.Server.EnableSharing data.Config.EnableStarRating = conf.Server.EnableStarRating diff --git a/core/metrics/insights/data.go b/core/metrics/insights/data.go index 9df547b4a..85c1ad18b 100644 --- a/core/metrics/insights/data.go +++ b/core/metrics/insights/data.go @@ -60,6 +60,7 @@ type Data struct { EnableJukebox bool `json:"enableJukebox,omitempty"` EnablePrometheus bool `json:"enablePrometheus,omitempty"` EnableCoverAnimation bool `json:"enableCoverAnimation,omitempty"` + EnableNowPlaying bool `json:"enableNowPlaying,omitempty"` SessionTimeout uint64 `json:"sessionTimeout,omitempty"` SearchFullString bool `json:"searchFullString,omitempty"` RecentlyAddedByModTime bool `json:"recentlyAddedByModTime,omitempty"` diff --git a/core/scrobbler/play_tracker.go b/core/scrobbler/play_tracker.go index 0f9b8c170..caa7868e5 100644 --- a/core/scrobbler/play_tracker.go +++ b/core/scrobbler/play_tracker.go @@ -5,6 +5,7 @@ import ( "sort" "time" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" @@ -51,10 +52,12 @@ func GetPlayTracker(ds model.DataStore, broker events.Broker) PlayTracker { func newPlayTracker(ds model.DataStore, broker events.Broker) *playTracker { m := cache.NewSimpleCache[string, NowPlayingInfo]() p := &playTracker{ds: ds, playMap: m, broker: broker} - m.OnExpiration(func(_ string, _ NowPlayingInfo) { - ctx := events.BroadcastToAll(context.Background()) - broker.SendMessage(ctx, &events.NowPlayingCount{Count: m.Len()}) - }) + if conf.Server.EnableNowPlaying { + m.OnExpiration(func(_ string, _ NowPlayingInfo) { + ctx := events.BroadcastToAll(context.Background()) + broker.SendMessage(ctx, &events.NowPlayingCount{Count: m.Len()}) + }) + } p.scrobblers = make(map[string]Scrobbler) var enabled []string for name, constructor := range constructors { @@ -89,8 +92,10 @@ func (p *playTracker) NowPlaying(ctx context.Context, playerId string, playerNam ttl := time.Duration(int(mf.Duration)+5) * time.Second _ = p.playMap.AddWithTTL(playerId, info, ttl) - ctx = events.BroadcastToAll(ctx) - p.broker.SendMessage(ctx, &events.NowPlayingCount{Count: p.playMap.Len()}) + if conf.Server.EnableNowPlaying { + ctx = events.BroadcastToAll(ctx) + p.broker.SendMessage(ctx, &events.NowPlayingCount{Count: p.playMap.Len()}) + } player, _ := request.PlayerFrom(ctx) if player.ScrobbleEnabled { p.dispatchNowPlaying(ctx, user.ID, mf) diff --git a/core/scrobbler/play_tracker_test.go b/core/scrobbler/play_tracker_test.go index d540e6faa..72bb446e4 100644 --- a/core/scrobbler/play_tracker_test.go +++ b/core/scrobbler/play_tracker_test.go @@ -7,6 +7,8 @@ import ( "sync" "time" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/model" @@ -29,6 +31,7 @@ var _ = Describe("PlayTracker", func() { var fake fakeScrobbler BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) ctx = context.Background() ctx = request.WithUser(ctx, model.User{ID: "u-1"}) ctx = request.WithPlayer(ctx, model.Player{ScrobbleEnabled: true}) @@ -113,6 +116,13 @@ var _ = Describe("PlayTracker", func() { Expect(ok).To(BeTrue()) Expect(evt.Count).To(Equal(1)) }) + + It("does not send event when disabled", func() { + conf.Server.EnableNowPlaying = false + err := tracker.NowPlaying(ctx, "player-1", "player-one", "123") + Expect(err).ToNot(HaveOccurred()) + Expect(eventBroker.getEvents()).To(BeEmpty()) + }) }) Describe("GetNowPlaying", func() { @@ -151,6 +161,14 @@ var _ = Describe("PlayTracker", func() { Expect(ok).To(BeTrue()) Expect(evt.Count).To(Equal(0)) }) + + It("does not send event when disabled", func() { + conf.Server.EnableNowPlaying = false + tracker = newPlayTracker(ds, eventBroker) + info := NowPlayingInfo{MediaFile: track, Start: time.Now(), Username: "user"} + _ = tracker.(*playTracker).playMap.AddWithTTL("player-2", info, 10*time.Millisecond) + Consistently(func() int { return len(eventBroker.getEvents()) }).Should(Equal(0)) + }) }) Describe("Submit", func() { diff --git a/server/serve_index.go b/server/serve_index.go index 1e55743f0..19ecc7b35 100644 --- a/server/serve_index.go +++ b/server/serve_index.go @@ -55,6 +55,7 @@ func serveIndex(ds model.DataStore, fs fs.FS, shareInfo *model.Share) http.Handl "defaultLanguage": conf.Server.DefaultLanguage, "defaultUIVolume": conf.Server.DefaultUIVolume, "enableCoverAnimation": conf.Server.EnableCoverAnimation, + "enableNowPlaying": conf.Server.EnableNowPlaying, "gaTrackingId": conf.Server.GATrackingID, "losslessFormats": strings.ToUpper(strings.Join(mime.LosslessFormats, ",")), "devActivityPanel": conf.Server.DevActivityPanel, diff --git a/server/serve_index_test.go b/server/serve_index_test.go index fd0d42193..3944414d9 100644 --- a/server/serve_index_test.go +++ b/server/serve_index_test.go @@ -196,6 +196,17 @@ var _ = Describe("serveIndex", func() { Expect(config).To(HaveKeyWithValue("enableCoverAnimation", true)) }) + It("sets the enableNowPlaying", func() { + conf.Server.EnableNowPlaying = true + r := httptest.NewRequest("GET", "/index.html", nil) + w := httptest.NewRecorder() + + serveIndex(ds, fs, nil)(w, r) + + config := extractAppConfig(w.Body.String()) + Expect(config).To(HaveKeyWithValue("enableNowPlaying", true)) + }) + It("sets the gaTrackingId", func() { conf.Server.GATrackingID = "UA-12345" r := httptest.NewRequest("GET", "/index.html", nil) diff --git a/server/subsonic/api.go b/server/subsonic/api.go index fd8c3af28..632734c3c 100644 --- a/server/subsonic/api.go +++ b/server/subsonic/api.go @@ -110,7 +110,11 @@ func (api *Router) routes() http.Handler { hr(r, "getAlbumList2", api.GetAlbumList2) h(r, "getStarred", api.GetStarred) h(r, "getStarred2", api.GetStarred2) - h(r, "getNowPlaying", api.GetNowPlaying) + if conf.Server.EnableNowPlaying { + h(r, "getNowPlaying", api.GetNowPlaying) + } else { + h501(r, "getNowPlaying") + } h(r, "getRandomSongs", api.GetRandomSongs) h(r, "getSongsByGenre", api.GetSongsByGenre) }) diff --git a/tests/mock_data_store.go b/tests/mock_data_store.go index b146a3b56..02a03e56e 100644 --- a/tests/mock_data_store.go +++ b/tests/mock_data_store.go @@ -2,6 +2,7 @@ package tests import ( "context" + "sync" "github.com/navidrome/navidrome/model" ) @@ -25,6 +26,7 @@ type MockDataStore struct { MockedUserProps model.UserPropsRepository MockedScrobbleBuffer model.ScrobbleBufferRepository MockedRadio model.RadioRepository + scrobbleBufferMu sync.Mutex } func (db *MockDataStore) Library(ctx context.Context) model.LibraryRepository { @@ -193,6 +195,8 @@ func (db *MockDataStore) Player(ctx context.Context) model.PlayerRepository { } func (db *MockDataStore) ScrobbleBuffer(ctx context.Context) model.ScrobbleBufferRepository { + db.scrobbleBufferMu.Lock() + defer db.scrobbleBufferMu.Unlock() if db.MockedScrobbleBuffer == nil { if db.RealDS != nil { db.MockedScrobbleBuffer = db.RealDS.ScrobbleBuffer(ctx) diff --git a/tests/mock_scrobble_buffer_repo.go b/tests/mock_scrobble_buffer_repo.go index 407c673eb..5865f423a 100644 --- a/tests/mock_scrobble_buffer_repo.go +++ b/tests/mock_scrobble_buffer_repo.go @@ -1,6 +1,7 @@ package tests import ( + "sync" "time" "github.com/navidrome/navidrome/model" @@ -9,6 +10,7 @@ import ( type MockedScrobbleBufferRepo struct { Error error Data model.ScrobbleEntries + mu sync.RWMutex } func CreateMockedScrobbleBufferRepo() *MockedScrobbleBufferRepo { @@ -19,6 +21,8 @@ func (m *MockedScrobbleBufferRepo) UserIDs(service string) ([]string, error) { if m.Error != nil { return nil, m.Error } + m.mu.RLock() + defer m.mu.RUnlock() userIds := make(map[string]struct{}) for _, e := range m.Data { if e.Service == service { @@ -36,6 +40,8 @@ func (m *MockedScrobbleBufferRepo) Enqueue(service, userId, mediaFileId string, if m.Error != nil { return m.Error } + m.mu.Lock() + defer m.mu.Unlock() m.Data = append(m.Data, model.ScrobbleEntry{ MediaFile: model.MediaFile{ID: mediaFileId}, Service: service, @@ -50,6 +56,8 @@ func (m *MockedScrobbleBufferRepo) Next(service, userId string) (*model.Scrobble if m.Error != nil { return nil, m.Error } + m.mu.RLock() + defer m.mu.RUnlock() for _, e := range m.Data { if e.Service == service && e.UserID == userId { return &e, nil @@ -62,6 +70,8 @@ func (m *MockedScrobbleBufferRepo) Dequeue(entry *model.ScrobbleEntry) error { if m.Error != nil { return m.Error } + m.mu.Lock() + defer m.mu.Unlock() newData := model.ScrobbleEntries{} for _, e := range m.Data { if e.Service == entry.Service && e.UserID == entry.UserID && e.PlayTime == entry.PlayTime && e.MediaFile.ID == entry.MediaFile.ID { @@ -77,5 +87,7 @@ func (m *MockedScrobbleBufferRepo) Length() (int64, error) { if m.Error != nil { return 0, m.Error } + m.mu.RLock() + defer m.mu.RUnlock() return int64(len(m.Data)), nil } diff --git a/ui/src/config.js b/ui/src/config.js index 1a89019ba..c94a6ffb9 100644 --- a/ui/src/config.js +++ b/ui/src/config.js @@ -29,6 +29,7 @@ const defaultConfig = { listenBrainzEnabled: true, enableExternalServices: true, enableCoverAnimation: true, + enableNowPlaying: true, devShowArtistPage: true, devUIShowConfig: true, enableReplayGain: true, diff --git a/ui/src/eventStream.js b/ui/src/eventStream.js index 33a7f6c9e..7ab91056e 100644 --- a/ui/src/eventStream.js +++ b/ui/src/eventStream.js @@ -2,6 +2,7 @@ import { baseUrl } from './utils' import throttle from 'lodash.throttle' import { processEvent, serverDown } from './actions' import { REST_URL } from './consts' +import config from './config' const newEventStream = async () => { let url = baseUrl(`${REST_URL}/events`) @@ -33,7 +34,9 @@ const startEventStream = async (dispatchFn) => { throttledEventHandler(dispatchFn), ) newStream.addEventListener('refreshResource', eventHandler(dispatchFn)) - newStream.addEventListener('nowPlayingCount', eventHandler(dispatchFn)) + if (config.enableNowPlaying) { + newStream.addEventListener('nowPlayingCount', eventHandler(dispatchFn)) + } newStream.addEventListener('keepAlive', eventHandler(dispatchFn)) newStream.onerror = (e) => { // eslint-disable-next-line no-console diff --git a/ui/src/layout/AppBar.jsx b/ui/src/layout/AppBar.jsx index 5690c4264..eefcc908d 100644 --- a/ui/src/layout/AppBar.jsx +++ b/ui/src/layout/AppBar.jsx @@ -120,9 +120,9 @@ const CustomUserMenu = ({ onClick, ...rest }) => { return ( <> - {config.devActivityPanel && permissions === 'admin' && ( - - )} + {config.devActivityPanel && + permissions === 'admin' && + config.enableNowPlaying && } {config.devActivityPanel && permissions === 'admin' && } diff --git a/ui/src/layout/AppBar.test.jsx b/ui/src/layout/AppBar.test.jsx new file mode 100644 index 000000000..f39dd75cb --- /dev/null +++ b/ui/src/layout/AppBar.test.jsx @@ -0,0 +1,65 @@ +import React from 'react' +import { render, screen } from '@testing-library/react' +import { describe, it, beforeEach, vi } from 'vitest' +import { Provider } from 'react-redux' +import { createStore, combineReducers } from 'redux' +import { activityReducer } from '../reducers' +import AppBar from './AppBar' +import config from '../config' + +let store + +vi.mock('react-admin', () => ({ + AppBar: ({ userMenu }) =>
{userMenu}
, + useTranslate: () => (x) => x, + usePermissions: () => ({ permissions: 'admin' }), + getResources: () => [], +})) + +vi.mock('./NowPlayingPanel', () => ({ + default: () =>
, +})) +vi.mock('./ActivityPanel', () => ({ + default: () =>
, +})) +vi.mock('./PersonalMenu', () => ({ + default: () =>
, +})) +vi.mock('./UserMenu', () => ({ + default: ({ children }) =>
{children}
, +})) +vi.mock('../dialogs/Dialogs', () => ({ + Dialogs: () =>
, +})) +vi.mock('../dialogs', () => ({ + AboutDialog: () =>
, +})) + +describe('', () => { + beforeEach(() => { + config.devActivityPanel = true + config.enableNowPlaying = true + store = createStore(combineReducers({ activity: activityReducer }), { + activity: { nowPlayingCount: 0 }, + }) + }) + + it('renders NowPlayingPanel when enabled', () => { + render( + + + , + ) + expect(screen.getByTestId('now-playing-panel')).toBeInTheDocument() + }) + + it('hides NowPlayingPanel when disabled', () => { + config.enableNowPlaying = false + render( + + + , + ) + expect(screen.queryByTestId('now-playing-panel')).toBeNull() + }) +})