Add tests to /scrobble endpoint

server/subsonic/media_annotation.go

@@ -18,12 +18,12 @@ import (
 
 type MediaAnnotationController struct {
 	ds          model.DataStore
-	scrobbler scrobbler.PlayTracker
+	playTracker scrobbler.PlayTracker
 	broker      events.Broker
 }
 
-func NewMediaAnnotationController(ds model.DataStore, scrobbler scrobbler.PlayTracker, broker events.Broker) *MediaAnnotationController {
+func NewMediaAnnotationController(ds model.DataStore, playTracker scrobbler.PlayTracker, broker events.Broker) *MediaAnnotationController {
-	return &MediaAnnotationController{ds: ds, scrobbler: scrobbler, broker: broker}
+	return &MediaAnnotationController{ds: ds, playTracker: playTracker, broker: broker}
 }
 
 func (c *MediaAnnotationController) SetRating(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
@@ -115,71 +115,6 @@ func (c *MediaAnnotationController) Unstar(w http.ResponseWriter, r *http.Reques
 	return newResponse(), nil
 }
 
-func (c *MediaAnnotationController) Scrobble(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
-	ids, err := requiredParamStrings(r, "id")
-	if err != nil {
-		return nil, err
-	}
-	times := utils.ParamTimes(r, "time")
-	if len(times) > 0 && len(times) != len(ids) {
-		return nil, newError(responses.ErrorGeneric, "Wrong number of timestamps: %d, should be %d", len(times), len(ids))
-	}
-	submission := utils.ParamBool(r, "submission", true)
-	ctx := r.Context()
-
-	if submission {
-		err := c.scrobblerSubmit(ctx, ids, times)
-		if err != nil {
-			log.Error(ctx, "Error registering scrobbles", "ids", ids, "times", times, err)
-		}
-	} else {
-		err := c.scrobblerNowPlaying(ctx, ids[0])
-		if err != nil {
-			log.Error(ctx, "Error setting NowPlaying", "id", ids[0], err)
-		}
-	}
-
-	return newResponse(), nil
-}
-
-func (c *MediaAnnotationController) scrobblerSubmit(ctx context.Context, ids []string, times []time.Time) error {
-	var submissions []scrobbler.Submission
-	log.Debug(ctx, "Scrobbling tracks", "ids", ids, "times", times)
-	for i, id := range ids {
-		var t time.Time
-		if len(times) > 0 {
-			t = times[i]
-		} else {
-			t = time.Now()
-		}
-		submissions = append(submissions, scrobbler.Submission{TrackID: id, Timestamp: t})
-	}
-
-	return c.scrobbler.Submit(ctx, submissions)
-}
-
-func (c *MediaAnnotationController) scrobblerNowPlaying(ctx context.Context, trackId string) error {
-	mf, err := c.ds.MediaFile(ctx).Get(trackId)
-	if err != nil {
-		return err
-	}
-	if mf == nil {
-		return fmt.Errorf(`ID "%s" not found`, trackId)
-	}
-
-	player, _ := request.PlayerFrom(ctx)
-	username, _ := request.UsernameFrom(ctx)
-	client, _ := request.ClientFrom(ctx)
-	clientId, ok := request.ClientUniqueIdFrom(ctx)
-	if !ok {
-		clientId = player.ID
-	}
-
-	log.Info("Now Playing", "title", mf.Title, "artist", mf.Artist, "user", username, "player", player.Name)
-	err = c.scrobbler.NowPlaying(ctx, clientId, client, trackId)
-	return err
-}
-
 func (c *MediaAnnotationController) setStar(ctx context.Context, star bool, ids ...string) error {
 	if len(ids) == 0 {
 		return nil
@@ -236,3 +171,68 @@ func (c *MediaAnnotationController) setStar(ctx context.Context, star bool, ids
 	}
 	return nil
 }
+
+func (c *MediaAnnotationController) Scrobble(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
+	ids, err := requiredParamStrings(r, "id")
+	if err != nil {
+		return nil, err
+	}
+	times := utils.ParamTimes(r, "time")
+	if len(times) > 0 && len(times) != len(ids) {
+		return nil, newError(responses.ErrorGeneric, "Wrong number of timestamps: %d, should be %d", len(times), len(ids))
+	}
+	submission := utils.ParamBool(r, "submission", true)
+	ctx := r.Context()
+
+	if submission {
+		err := c.scrobblerSubmit(ctx, ids, times)
+		if err != nil {
+			log.Error(ctx, "Error registering scrobbles", "ids", ids, "times", times, err)
+		}
+	} else {
+		err := c.scrobblerNowPlaying(ctx, ids[0])
+		if err != nil {
+			log.Error(ctx, "Error setting NowPlaying", "id", ids[0], err)
+		}
+	}
+
+	return newResponse(), nil
+}
+
+func (c *MediaAnnotationController) scrobblerSubmit(ctx context.Context, ids []string, times []time.Time) error {
+	var submissions []scrobbler.Submission
+	log.Debug(ctx, "Scrobbling tracks", "ids", ids, "times", times)
+	for i, id := range ids {
+		var t time.Time
+		if len(times) > 0 {
+			t = times[i]
+		} else {
+			t = time.Now()
+		}
+		submissions = append(submissions, scrobbler.Submission{TrackID: id, Timestamp: t})
+	}
+
+	return c.playTracker.Submit(ctx, submissions)
+}
+
+func (c *MediaAnnotationController) scrobblerNowPlaying(ctx context.Context, trackId string) error {
+	mf, err := c.ds.MediaFile(ctx).Get(trackId)
+	if err != nil {
+		return err
+	}
+	if mf == nil {
+		return fmt.Errorf(`ID "%s" not found`, trackId)
+	}
+
+	player, _ := request.PlayerFrom(ctx)
+	username, _ := request.UsernameFrom(ctx)
+	client, _ := request.ClientFrom(ctx)
+	clientId, ok := request.ClientUniqueIdFrom(ctx)
+	if !ok {
+		clientId = player.ID
+	}
+
+	log.Info("Now Playing", "title", mf.Title, "artist", mf.Artist, "user", username, "player", player.Name)
+	err = c.playTracker.NowPlaying(ctx, clientId, client, trackId)
+	return err
+}

server/subsonic/media_annotation_test.go

@@ -0,0 +1,146 @@
+package subsonic
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"time"
+
+	"github.com/navidrome/navidrome/model/request"
+
+	"github.com/navidrome/navidrome/core/scrobbler"
+	"github.com/navidrome/navidrome/model"
+	"github.com/navidrome/navidrome/server/events"
+	"github.com/navidrome/navidrome/tests"
+	"github.com/navidrome/navidrome/utils"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("MediaAnnotationController", func() {
+	var controller *MediaAnnotationController
+	var w *httptest.ResponseRecorder
+	var ds model.DataStore
+	var playTracker *fakePlayTracker
+	var eventBroker *fakeEventBroker
+	var ctx context.Context
+
+	BeforeEach(func() {
+		ctx = context.Background()
+		ds = &tests.MockDataStore{}
+		playTracker = &fakePlayTracker{}
+		eventBroker = &fakeEventBroker{}
+		controller = NewMediaAnnotationController(ds, playTracker, eventBroker)
+		w = httptest.NewRecorder()
+	})
+
+	Describe("Scrobble", func() {
+		It("submit all scrobbles with only the id", func() {
+			submissionTime := time.Now()
+			r := newGetRequest("id=12", "id=34")
+
+			_, err := controller.Scrobble(w, r)
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(playTracker.Submissions).To(HaveLen(2))
+			Expect(playTracker.Submissions[0].Timestamp).To(BeTemporally(">", submissionTime))
+			Expect(playTracker.Submissions[0].TrackID).To(Equal("12"))
+			Expect(playTracker.Submissions[1].Timestamp).To(BeTemporally(">", submissionTime))
+			Expect(playTracker.Submissions[1].TrackID).To(Equal("34"))
+		})
+
+		It("submit all scrobbles with respective times", func() {
+			time1 := time.Now().Add(-20 * time.Minute)
+			t1 := utils.ToMillis(time1)
+			time2 := time.Now().Add(-10 * time.Minute)
+			t2 := utils.ToMillis(time2)
+			r := newGetRequest("id=12", "id=34", fmt.Sprintf("time=%d", t1), fmt.Sprintf("time=%d", t2))
+
+			_, err := controller.Scrobble(w, r)
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(playTracker.Submissions).To(HaveLen(2))
+			Expect(playTracker.Submissions[0].Timestamp).To(BeTemporally("~", time1))
+			Expect(playTracker.Submissions[0].TrackID).To(Equal("12"))
+			Expect(playTracker.Submissions[1].Timestamp).To(BeTemporally("~", time2))
+			Expect(playTracker.Submissions[1].TrackID).To(Equal("34"))
+		})
+
+		It("checks if number of ids match number of times", func() {
+			r := newGetRequest("id=12", "id=34", "time=1111")
+
+			_, err := controller.Scrobble(w, r)
+
+			Expect(err).To(HaveOccurred())
+			Expect(playTracker.Submissions).To(BeEmpty())
+		})
+
+		Context("submission=false", func() {
+			var req *http.Request
+			BeforeEach(func() {
+				_ = ds.MediaFile(ctx).Put(&model.MediaFile{ID: "12"})
+				ctx = request.WithPlayer(ctx, model.Player{ID: "player-1"})
+				req = newGetRequest("id=12", "submission=false")
+				req = req.WithContext(ctx)
+			})
+
+			It("does not scrobble", func() {
+				_, err := controller.Scrobble(w, req)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(playTracker.Submissions).To(BeEmpty())
+			})
+
+			It("registers a NowPlaying", func() {
+				_, err := controller.Scrobble(w, req)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(playTracker.Playing).To(HaveLen(1))
+				Expect(playTracker.Playing).To(HaveKey("player-1"))
+			})
+		})
+	})
+})
+
+type fakePlayTracker struct {
+	Submissions []scrobbler.Submission
+	Playing     map[string]string
+	Error       error
+}
+
+func (f *fakePlayTracker) NowPlaying(ctx context.Context, playerId string, playerName string, trackId string) error {
+	if f.Error != nil {
+		return f.Error
+	}
+	if f.Playing == nil {
+		f.Playing = make(map[string]string)
+	}
+	f.Playing[playerId] = trackId
+	return nil
+}
+
+func (f *fakePlayTracker) GetNowPlaying(ctx context.Context) ([]scrobbler.NowPlayingInfo, error) {
+	return nil, f.Error
+}
+
+func (f *fakePlayTracker) Submit(ctx context.Context, submissions []scrobbler.Submission) error {
+	if f.Error != nil {
+		return f.Error
+	}
+	f.Submissions = append(f.Submissions, submissions...)
+	return nil
+}
+
+var _ scrobbler.PlayTracker = (*fakePlayTracker)(nil)
+
+type fakeEventBroker struct {
+	http.Handler
+	Events []events.Event
+}
+
+func (f *fakeEventBroker) SendMessage(ctx context.Context, event events.Event) {
+	f.Events = append(f.Events, event)
+}
+
+var _ events.Broker = (*fakeEventBroker)(nil)