diff --git a/api/media_annotation.go b/api/media_annotation.go index 3457105fe..66b486f8d 100644 --- a/api/media_annotation.go +++ b/api/media_annotation.go @@ -38,13 +38,6 @@ func (c *MediaAnnotationController) Scrobble() { } else { t = time.Now() } - skip, err := c.scrobbler.DetectSkipped(playerId, id, submission) - if err != nil { - beego.Error("Error detecting skip:", err) - } - if skip { - beego.Info("Skipped previous song") - } if submission { mf, err := c.scrobbler.Register(playerId, id, t) if err != nil { @@ -53,7 +46,7 @@ func (c *MediaAnnotationController) Scrobble() { } beego.Info(fmt.Sprintf(`Scrobbled (%s) "%s" at %v`, id, mf.Title, t)) } else { - mf, err := c.scrobbler.NowPlaying(playerId, id, username, playerName) + mf, err := c.scrobbler.NowPlaying(playerId, playerName, id, username) if err != nil { beego.Error("Error setting", id, "as current song:", err) continue diff --git a/engine/list_generator.go b/engine/list_generator.go index 6df732b9b..b29a0df0f 100644 --- a/engine/list_generator.go +++ b/engine/list_generator.go @@ -103,8 +103,8 @@ func (g *listGenerator) GetNowPlaying() (Entries, error) { if err != nil { return nil, err } - entries := make(Entries, len(*npInfo)) - for i, np := range *npInfo { + entries := make(Entries, len(npInfo)) + for i, np := range npInfo { mf, err := g.mfRepository.Get(np.TrackId) if err != nil { return nil, err diff --git a/engine/mock_nowplaying_repo.go b/engine/mock_nowplaying_repo.go index 695d3d5ff..87c249502 100644 --- a/engine/mock_nowplaying_repo.go +++ b/engine/mock_nowplaying_repo.go @@ -11,7 +11,7 @@ func CreateMockNowPlayingRepo() *MockNowPlaying { type MockNowPlaying struct { NowPlayingRepository - info NowPlayingInfo + data []NowPlayingInfo err bool } @@ -19,24 +19,44 @@ func (m *MockNowPlaying) SetError(err bool) { m.err = err } -func (m *MockNowPlaying) Set(id, username string, playerId int, playerName string) error { +func (m *MockNowPlaying) Enqueue(playerId int, playerName string, trackId, username string) error { if m.err { return errors.New("Error!") } - m.info.TrackId = id - m.info.Username = username - m.info.Start = time.Now() - m.info.PlayerId = playerId - m.info.PlayerName = playerName + info := NowPlayingInfo{} + info.TrackId = trackId + info.Username = username + info.Start = time.Now() + info.PlayerId = playerId + info.PlayerName = playerName + + m.data = append(m.data, NowPlayingInfo{}) + copy(m.data[1:], m.data[0:]) + m.data[0] = info + return nil } -func (m *MockNowPlaying) Clear(playerId int) (*NowPlayingInfo, error) { - r := m.info - m.info = NowPlayingInfo{} - return &r, nil +func (m *MockNowPlaying) Dequeue(playerId int) (*NowPlayingInfo, error) { + if len(m.data) == 0 { + return nil, nil + } + l := len(m.data) + info := m.data[l-1] + m.data = m.data[:l-1] + + return &info, nil } -func (m *MockNowPlaying) Current() NowPlayingInfo { - return m.info +func (m *MockNowPlaying) Head(playerId int) (*NowPlayingInfo, error) { + if len(m.data) == 0 { + return nil, nil + } + info := m.data[0] + return &info, nil +} + +func (m *MockNowPlaying) ClearAll() { + m.data = make([]NowPlayingInfo, 0) + m.err = false } diff --git a/engine/nowplaying.go b/engine/nowplaying.go index c0bf13525..662d8d275 100644 --- a/engine/nowplaying.go +++ b/engine/nowplaying.go @@ -12,8 +12,17 @@ type NowPlayingInfo struct { PlayerName string } +// This repo has the semantics of a FIFO queue, for each playerId type NowPlayingRepository interface { - Set(trackId, username string, playerId int, playerName string) error - Clear(playerId int) (*NowPlayingInfo, error) - GetAll() (*[]NowPlayingInfo, error) + // Insert at the head of the queue + Enqueue(playerId int, playerName string, trackId, username string) error + + // Returns the element at the head of the queue (last inserted one) + Head(playerId int) (*NowPlayingInfo, error) + + // Removes and returns the element at the end of the queue + Dequeue(playerId int) (*NowPlayingInfo, error) + + // Returns all heads from all playerIds + GetAll() ([]*NowPlayingInfo, error) } diff --git a/engine/scrobbler.go b/engine/scrobbler.go index d0a69a55d..5fa58d2f8 100644 --- a/engine/scrobbler.go +++ b/engine/scrobbler.go @@ -5,14 +5,14 @@ import ( "fmt" "time" + "github.com/astaxie/beego" "github.com/deluan/gosonic/domain" "github.com/deluan/gosonic/itunesbridge" ) type Scrobbler interface { Register(playerId int, trackId string, playDate time.Time) (*domain.MediaFile, error) - NowPlaying(playerId int, trackId, username string, playerName string) (*domain.MediaFile, error) - DetectSkipped(playerId int, trackId string, submission bool) (bool, error) + NowPlaying(playerId int, playerName, trackId, username string) (*domain.MediaFile, error) } func NewScrobbler(itunes itunesbridge.ItunesControl, mr domain.MediaFileRepository, npr NowPlayingRepository) Scrobbler { @@ -25,39 +25,20 @@ type scrobbler struct { npRepo NowPlayingRepository } -func (s *scrobbler) DetectSkipped(playerId int, trackId string, submission bool) (bool, error) { - np, err := s.npRepo.Clear(playerId) - if err != nil { - return false, err +func (s *scrobbler) Register(playerId int, trackId string, playDate time.Time) (*domain.MediaFile, error) { + for { + np, err := s.npRepo.Dequeue(playerId) + if err != nil || np == nil || np.TrackId == trackId { + break + } + err = s.itunes.MarkAsSkipped(np.TrackId, np.Start.Add(time.Duration(1)*time.Minute)) + if err != nil { + beego.Warn("Error skipping track", np.TrackId) + } else { + beego.Debug("Skipped track", np.TrackId) + } } - if np == nil { - return false, nil - } - - if (submission && np.TrackId != trackId) || (!submission) { - return true, s.itunes.MarkAsSkipped(np.TrackId, time.Now()) - } - return false, nil -} - -func (s *scrobbler) Register(playerId int, id string, playDate time.Time) (*domain.MediaFile, error) { - mf, err := s.mfRepo.Get(id) - if err != nil { - return nil, err - } - - if mf == nil { - return nil, errors.New(fmt.Sprintf(`Id "%s" not found`, id)) - } - - if err := s.itunes.MarkAsPlayed(id, playDate); err != nil { - return nil, err - } - return mf, nil -} - -func (s *scrobbler) NowPlaying(playerId int, trackId, username string, playerName string) (*domain.MediaFile, error) { mf, err := s.mfRepo.Get(trackId) if err != nil { return nil, err @@ -67,5 +48,21 @@ func (s *scrobbler) NowPlaying(playerId int, trackId, username string, playerNam return nil, errors.New(fmt.Sprintf(`Id "%s" not found`, trackId)) } - return mf, s.npRepo.Set(trackId, username, playerId, playerName) + if err := s.itunes.MarkAsPlayed(trackId, playDate); err != nil { + return nil, err + } + return mf, nil +} + +func (s *scrobbler) NowPlaying(playerId int, playerName, trackId, username string) (*domain.MediaFile, error) { + mf, err := s.mfRepo.Get(trackId) + if err != nil { + return nil, err + } + + if mf == nil { + return nil, errors.New(fmt.Sprintf(`Id "%s" not found`, trackId)) + } + + return mf, s.npRepo.Enqueue(playerId, playerName, trackId, username) } diff --git a/engine/scrobbler_test.go b/engine/scrobbler_test.go index 6a95e43de..bddb89ce3 100644 --- a/engine/scrobbler_test.go +++ b/engine/scrobbler_test.go @@ -54,7 +54,7 @@ func TestScrobbler(t *testing.T) { }) Convey("When I inform the song that is now playing", func() { - mf, err := scrobbler.NowPlaying(1, "2", "deluan", "DSub") + mf, err := scrobbler.NowPlaying(1, "DSub", "2", "deluan") Convey("Then I get the song for that id back", func() { So(err, ShouldBeNil) @@ -62,7 +62,7 @@ func TestScrobbler(t *testing.T) { }) Convey("And it saves the song as the one current playing", func() { - info := npRepo.Current() + info, _ := npRepo.Head(1) So(info.TrackId, ShouldEqual, "2") So(info.Start, ShouldHappenBefore, time.Now()) So(info.Username, ShouldEqual, "deluan") @@ -76,32 +76,33 @@ func TestScrobbler(t *testing.T) { Reset(func() { itCtrl.played = make(map[string]time.Time) + itCtrl.skipped = make(map[string]time.Time) }) }) Convey("Given a DB with two songs", t, func() { - mfRepo.SetData(`[{"Id":"1","Title":"Femme Fatale"},{"Id":"2","Title":"Here She Comes Now"}]`, 2) + mfRepo.SetData(`[{"Id":"1","Title":"Femme Fatale"},{"Id":"2","Title":"Here She Comes Now"},{"Id":"3","Title":"Lady Godiva's Operation"}]`, 3) + itCtrl.skipped = make(map[string]time.Time) + npRepo.ClearAll() Convey("When I play one song", func() { - scrobbler.NowPlaying(1, "1", "deluan", "DSub") - Convey("And I start playing the other song without scrobbling the first one", func() { - skip, err := scrobbler.DetectSkipped(1, "2", false) + scrobbler.NowPlaying(1, "DSub", "1", "deluan") + Convey("And I play the other song without scrobbling the first one", func() { + scrobbler.NowPlaying(1, "DSub", "2", "deluan") + mf, err := scrobbler.Register(1, "2", time.Now()) Convey("Then the first song should be marked as skipped", func() { - So(skip, ShouldBeTrue) + So(mf.Id, ShouldEqual, "2") So(itCtrl.skipped, ShouldContainKey, "1") So(err, ShouldBeNil) }) }) Convey("And I scrobble it before starting to play the other song", func() { - skip, err := scrobbler.DetectSkipped(1, "1", true) + mf, err := scrobbler.Register(1, "1", time.Now()) Convey("Then the first song should NOT marked as skipped", func() { - So(skip, ShouldBeFalse) + So(mf.Id, ShouldEqual, "1") So(itCtrl.skipped, ShouldBeEmpty) So(err, ShouldBeNil) }) }) - Reset(func() { - itCtrl.skipped = make(map[string]time.Time) - }) }) }) } diff --git a/persistence/mock_mediafile_repo.go b/persistence/mock_mediafile_repo.go index a3d2104b6..0ebc7587e 100644 --- a/persistence/mock_mediafile_repo.go +++ b/persistence/mock_mediafile_repo.go @@ -14,7 +14,7 @@ func CreateMockMediaFileRepo() *MockMediaFile { type MockMediaFile struct { domain.MediaFileRepository - data map[string]*domain.MediaFile + data map[string]domain.MediaFile err bool } @@ -23,14 +23,14 @@ func (m *MockMediaFile) SetError(err bool) { } func (m *MockMediaFile) SetData(j string, size int) { - m.data = make(map[string]*domain.MediaFile) + m.data = make(map[string]domain.MediaFile) var l = make(domain.MediaFiles, size) err := json.Unmarshal([]byte(j), &l) if err != nil { fmt.Println("ERROR: ", err) } for _, a := range l { - m.data[a.Id] = &a + m.data[a.Id] = a } } @@ -47,7 +47,7 @@ func (m *MockMediaFile) Get(id string) (*domain.MediaFile, error) { return nil, errors.New("Error!") } if d, ok := m.data[id]; ok { - return d, nil + return &d, nil } return nil, domain.ErrNotFound } @@ -60,7 +60,7 @@ func (m *MockMediaFile) FindByAlbum(artistId string) (domain.MediaFiles, error) i := 0 for _, a := range m.data { if a.AlbumId == artistId { - res[i] = *a + res[i] = a i++ } } diff --git a/persistence/nowplaying_repository.go b/persistence/nowplaying_repository.go index 655f690d4..094c063ee 100644 --- a/persistence/nowplaying_repository.go +++ b/persistence/nowplaying_repository.go @@ -2,14 +2,14 @@ package persistence import ( "encoding/json" - "errors" + "fmt" "time" "github.com/deluan/gosonic/engine" ) var ( - nowPlayingKeyName = []byte("nowplaying") + nowPlayingKeyPrefix = []byte("nowplaying") ) type nowPlayingRepository struct { @@ -22,21 +22,29 @@ func NewNowPlayingRepository() engine.NowPlayingRepository { return r } -func (r *nowPlayingRepository) Set(id, username string, playerId int, playerName string) error { - if id == "" { - return errors.New("Id is required") - } +func nowPlayingKeyName(playerId int) string { + return fmt.Sprintf("%s:%d", nowPlayingKeyPrefix, playerId) +} + +func (r *nowPlayingRepository) Enqueue(playerId int, playerName, id, username string) error { m := &engine.NowPlayingInfo{TrackId: id, Username: username, Start: time.Now(), PlayerId: playerId, PlayerName: playerName} h, err := json.Marshal(m) if err != nil { return err } - return Db().SetEX(nowPlayingKeyName, int64(engine.NowPlayingExpire.Seconds()), []byte(h)) + + keyName := []byte(nowPlayingKeyName(playerId)) + + _, err = Db().LPush(keyName, []byte(h)) + Db().LExpire(keyName, int64(engine.NowPlayingExpire.Seconds())) + return err } -func (r *nowPlayingRepository) Clear(playerId int) (*engine.NowPlayingInfo, error) { - val, err := Db().GetSet(nowPlayingKeyName, nil) +func (r *nowPlayingRepository) Head(playerId int) (*engine.NowPlayingInfo, error) { + keyName := []byte(nowPlayingKeyName(playerId)) + + val, err := Db().LIndex(keyName, 0) if err != nil { return nil, err } @@ -48,20 +56,28 @@ func (r *nowPlayingRepository) Clear(playerId int) (*engine.NowPlayingInfo, erro return info, nil } -func (r *nowPlayingRepository) GetAll() (*[]engine.NowPlayingInfo, error) { - val, err := Db().Get(nowPlayingKeyName) +// TODO Will not work for multiple players +func (r *nowPlayingRepository) GetAll() ([]*engine.NowPlayingInfo, error) { + np, err := r.Head(1) + return []*engine.NowPlayingInfo{np}, err +} + +func (r *nowPlayingRepository) Dequeue(playerId int) (*engine.NowPlayingInfo, error) { + keyName := []byte(nowPlayingKeyName(playerId)) + + val, err := Db().RPop(keyName) if err != nil { return nil, err } if val == nil { - return &[]engine.NowPlayingInfo{}, nil + return nil, nil } info := &engine.NowPlayingInfo{} err = json.Unmarshal(val, info) if err != nil { - return &[]engine.NowPlayingInfo{}, nil + return nil, nil } - return &[]engine.NowPlayingInfo{*info}, nil + return info, nil } var _ engine.NowPlayingRepository = (*nowPlayingRepository)(nil)