New NowPlaying and Skip detection implementation

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

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))
+	entries := make(Entries, len(npInfo))
-	for i, np := range *npInfo {
+	for i, np := range npInfo {
 		mf, err := g.mfRepository.Get(np.TrackId)
 		if err != nil {
 			return nil, err

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
+	info := NowPlayingInfo{}
-	m.info.Username = username
+	info.TrackId = trackId
-	m.info.Start = time.Now()
+	info.Username = username
-	m.info.PlayerId = playerId
+	info.Start = time.Now()
-	m.info.PlayerName = playerName
+	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) {
+func (m *MockNowPlaying) Dequeue(playerId int) (*NowPlayingInfo, error) {
-	r := m.info
+	if len(m.data) == 0 {
-	m.info = NowPlayingInfo{}
+		return nil, nil
-	return &r, nil
+	}
+	l := len(m.data)
+	info := m.data[l-1]
+	m.data = m.data[:l-1]
+
+	return &info, nil
 }
 
-func (m *MockNowPlaying) Current() NowPlayingInfo {
+func (m *MockNowPlaying) Head(playerId int) (*NowPlayingInfo, error) {
-	return m.info
+	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
 }

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
+	// Insert at the head of the queue
-	Clear(playerId int) (*NowPlayingInfo, error)
+	Enqueue(playerId int, playerName string, trackId, username string) error
-	GetAll() (*[]NowPlayingInfo, 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)
 }

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)
+	NowPlaying(playerId int, playerName, trackId, username string) (*domain.MediaFile, error)
-	DetectSkipped(playerId int, trackId string, submission bool) (bool, 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) {
+func (s *scrobbler) Register(playerId int, trackId string, playDate time.Time) (*domain.MediaFile, error) {
-	np, err := s.npRepo.Clear(playerId)
+	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 {
-		return false, err
+			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)
 }

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")
+			scrobbler.NowPlaying(1, "DSub", "1", "deluan")
-			Convey("And I start playing the other song without scrobbling the first one", func() {
+			Convey("And I play the other song without scrobbling the first one", func() {
-				skip, err := scrobbler.DetectSkipped(1, "2", false)
+				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)
-			})
 		})
 	})
 }

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++
 		}
 	}

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 {
+func nowPlayingKeyName(playerId int) string {
-	if id == "" {
+	return fmt.Sprintf("%s:%d", nowPlayingKeyPrefix, playerId)
-		return errors.New("Id is required")
+}
-	}
+
+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) {
+func (r *nowPlayingRepository) Head(playerId int) (*engine.NowPlayingInfo, error) {
-	val, err := Db().GetSet(nowPlayingKeyName, nil)
+	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) {
+// TODO Will not work for multiple players
-	val, err := Db().Get(nowPlayingKeyName)
+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)