diff --git a/core/scrobbler/play_tracker.go b/core/scrobbler/play_tracker.go index b0164701..198223b2 100644 --- a/core/scrobbler/play_tracker.go +++ b/core/scrobbler/play_tracker.go @@ -16,10 +16,10 @@ import ( "github.com/navidrome/navidrome/utils/singleton" ) -const nowPlayingExpire = 60 * time.Minute +const maxNowPlayingExpire = 60 * time.Minute type NowPlayingInfo struct { - TrackID string + MediaFile model.MediaFile Start time.Time Username string PlayerId string @@ -46,50 +46,58 @@ type playTracker struct { func GetPlayTracker(ds model.DataStore, broker events.Broker) PlayTracker { return singleton.GetInstance(func() *playTracker { - m := ttlcache.NewCache() - m.SkipTTLExtensionOnHit(true) - _ = m.SetTTL(nowPlayingExpire) - p := &playTracker{ds: ds, playMap: m, broker: broker} - p.scrobblers = make(map[string]Scrobbler) - for name, constructor := range constructors { - s := constructor(ds) - if conf.Server.DevEnableBufferedScrobble { - s = newBufferedScrobbler(ds, s, name) - } - p.scrobblers[name] = s - } - return p + return newPlayTracker(ds, broker) }) } +// This constructor only exists for testing. For normal usage, the PlayTracker has to be a singleton, returned by +// the GetPlayTracker function above +func newPlayTracker(ds model.DataStore, broker events.Broker) *playTracker { + m := ttlcache.NewCache() + m.SkipTTLExtensionOnHit(true) + _ = m.SetTTL(maxNowPlayingExpire) + p := &playTracker{ds: ds, playMap: m, broker: broker} + p.scrobblers = make(map[string]Scrobbler) + for name, constructor := range constructors { + s := constructor(ds) + if conf.Server.DevEnableBufferedScrobble { + s = newBufferedScrobbler(ds, s, name) + } + p.scrobblers[name] = s + } + return p +} + func (p *playTracker) NowPlaying(ctx context.Context, playerId string, playerName string, trackId string) error { + mf, err := p.ds.MediaFile(ctx).Get(trackId) + if err != nil { + log.Error(ctx, "Error retrieving mediaFile", "id", trackId, err) + return err + } + user, _ := request.UserFrom(ctx) info := NowPlayingInfo{ - TrackID: trackId, + MediaFile: *mf, Start: time.Now(), Username: user.UserName, PlayerId: playerId, PlayerName: playerName, } - _ = p.playMap.Set(playerId, info) + + ttl := time.Duration(int(mf.Duration)+5) * time.Second + _ = p.playMap.SetWithTTL(playerId, info, ttl) player, _ := request.PlayerFrom(ctx) if player.ScrobbleEnabled { - p.dispatchNowPlaying(ctx, user.ID, trackId) + p.dispatchNowPlaying(ctx, user.ID, mf) } return nil } -func (p *playTracker) dispatchNowPlaying(ctx context.Context, userId string, trackId string) { - t, err := p.ds.MediaFile(ctx).Get(trackId) - if err != nil { - log.Error(ctx, "Error retrieving mediaFile", "id", trackId, err) - return - } +func (p *playTracker) dispatchNowPlaying(ctx context.Context, userId string, t *model.MediaFile) { if t.Artist == consts.UnknownArtist { log.Debug(ctx, "Ignoring external NowPlaying update for track with unknown artist", "track", t.Title, "artist", t.Artist) return } - // TODO Parallelize for name, s := range p.scrobblers { if !s.IsAuthorized(ctx, userId) { continue @@ -103,7 +111,7 @@ func (p *playTracker) dispatchNowPlaying(ctx context.Context, userId string, tra } } -func (p *playTracker) GetNowPlaying(ctx context.Context) ([]NowPlayingInfo, error) { +func (p *playTracker) GetNowPlaying(_ context.Context) ([]NowPlayingInfo, error) { var res []NowPlayingInfo for _, playerId := range p.playMap.GetKeys() { value, err := p.playMap.Get(playerId) diff --git a/core/scrobbler/play_tracker_test.go b/core/scrobbler/play_tracker_test.go index f7412a01..fa074fb2 100644 --- a/core/scrobbler/play_tracker_test.go +++ b/core/scrobbler/play_tracker_test.go @@ -37,7 +37,7 @@ var _ = Describe("PlayTracker", func() { Register("fake", func(ds model.DataStore) Scrobbler { return &fake }) - tracker = GetPlayTracker(ds, events.GetBroker()) + tracker = newPlayTracker(ds, events.GetBroker()) track = model.MediaFile{ ID: "123", @@ -93,16 +93,13 @@ var _ = Describe("PlayTracker", func() { }) Describe("GetNowPlaying", func() { - BeforeEach(func() { - ctx = context.Background() - }) It("returns current playing music", func() { track2 := track track2.ID = "456" - _ = ds.MediaFile(ctx).Put(&track) - ctx = request.WithUser(ctx, model.User{UserName: "user-1"}) + _ = ds.MediaFile(ctx).Put(&track2) + ctx = request.WithUser(context.Background(), model.User{UserName: "user-1"}) _ = tracker.NowPlaying(ctx, "player-1", "player-one", "123") - ctx = request.WithUser(ctx, model.User{UserName: "user-2"}) + ctx = request.WithUser(context.Background(), model.User{UserName: "user-2"}) _ = tracker.NowPlaying(ctx, "player-2", "player-two", "456") playing, err := tracker.GetNowPlaying(ctx) @@ -112,12 +109,12 @@ var _ = Describe("PlayTracker", func() { Expect(playing[0].PlayerId).To(Equal("player-2")) Expect(playing[0].PlayerName).To(Equal("player-two")) Expect(playing[0].Username).To(Equal("user-2")) - Expect(playing[0].TrackID).To(Equal("456")) + Expect(playing[0].MediaFile.ID).To(Equal("456")) Expect(playing[1].PlayerId).To(Equal("player-1")) Expect(playing[1].PlayerName).To(Equal("player-one")) Expect(playing[1].Username).To(Equal("user-1")) - Expect(playing[1].TrackID).To(Equal("123")) + Expect(playing[1].MediaFile.ID).To(Equal("123")) }) }) diff --git a/server/subsonic/album_lists.go b/server/subsonic/album_lists.go index 4944f8d5..0f68857f 100644 --- a/server/subsonic/album_lists.go +++ b/server/subsonic/album_lists.go @@ -153,12 +153,7 @@ func (api *Router) GetNowPlaying(r *http.Request) (*responses.Subsonic, error) { response.NowPlaying = &responses.NowPlaying{} response.NowPlaying.Entry = make([]responses.NowPlayingEntry, len(npInfo)) for i, np := range npInfo { - mf, err := api.ds.MediaFile(ctx).Get(np.TrackID) - if err != nil { - return nil, err - } - - response.NowPlaying.Entry[i].Child = childFromMediaFile(ctx, *mf) + response.NowPlaying.Entry[i].Child = childFromMediaFile(ctx, np.MediaFile) response.NowPlaying.Entry[i].UserName = np.Username response.NowPlaying.Entry[i].MinutesAgo = int(time.Since(np.Start).Minutes()) response.NowPlaying.Entry[i].PlayerId = i + 1 // Fake numeric playerId, it does not seem to be used for anything