From a7509c9ff73d3a0ebee702c66d56d47cfc863b7f Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 22 Jun 2021 14:00:44 -0400 Subject: [PATCH] Send NowPlaying and Scrobbles to Last.fm --- consts/consts.go | 2 +- core/agents/agents_test.go | 13 ++- core/agents/lastfm/agent.go | 109 +++++++++++++++------- core/agents/lastfm/agent_test.go | 98 ++++++++++++++++++-- core/agents/lastfm/auth_router.go | 15 ++-- core/agents/lastfm/client.go | 80 +++++++++++++++-- core/agents/lastfm/responses.go | 55 ++++++++++++ core/agents/spotify/spotify.go | 7 +- core/scrobbler/broker.go | 37 +++++--- core/scrobbler/broker_test.go | 119 +++++++++++++++++++++++++ core/scrobbler/interfaces.go | 8 +- core/scrobbler/scrobbler_suite_test.go | 17 ++++ server/subsonic/media_annotation.go | 2 +- tests/mock_mediafile_repo.go | 17 +++- utils/cached_http_client_test.go | 2 +- 15 files changed, 503 insertions(+), 78 deletions(-) create mode 100644 core/scrobbler/broker_test.go create mode 100644 core/scrobbler/scrobbler_suite_test.go diff --git a/consts/consts.go b/consts/consts.go index 52477b744..8a8b16b4b 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -46,7 +46,7 @@ const ( PlaceholderAlbumArt = "navidrome-600x600.png" PlaceholderAvatar = "logo-192x192.png" - DefaultCachedHttpClientTTL = 10 * time.Second + DefaultHttpClientTimeOut = 10 * time.Second ) // Cache options diff --git a/core/agents/agents_test.go b/core/agents/agents_test.go index 67bb0e13d..b25727b88 100644 --- a/core/agents/agents_test.go +++ b/core/agents/agents_test.go @@ -4,6 +4,9 @@ import ( "context" "errors" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + "github.com/navidrome/navidrome/conf" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -12,15 +15,17 @@ import ( var _ = Describe("Agents", func() { var ctx context.Context var cancel context.CancelFunc + var ds model.DataStore BeforeEach(func() { ctx, cancel = context.WithCancel(context.Background()) + ds = &tests.MockDataStore{} }) Describe("Placeholder", func() { var ag *Agents BeforeEach(func() { conf.Server.Agents = "" - ag = New(ctx) + ag = New(ds) }) It("calls the placeholder GetBiography", func() { @@ -41,16 +46,16 @@ var _ = Describe("Agents", func() { var mock *mockAgent BeforeEach(func() { mock = &mockAgent{} - Register("fake", func(ctx context.Context) Interface { + Register("fake", func(ds model.DataStore) Interface { return mock }) - Register("empty", func(ctx context.Context) Interface { + Register("empty", func(ds model.DataStore) Interface { return struct { Interface }{} }) conf.Server.Agents = "empty,fake" - ag = New(ctx) + ag = New(ds) Expect(ag.AgentName()).To(Equal("agents")) }) diff --git a/core/agents/lastfm/agent.go b/core/agents/lastfm/agent.go index 998e46885..1d2637bd3 100644 --- a/core/agents/lastfm/agent.go +++ b/core/agents/lastfm/agent.go @@ -18,23 +18,27 @@ const ( ) type lastfmAgent struct { - ctx context.Context - ds model.DataStore - apiKey string - secret string - lang string - client *Client + ds model.DataStore + sessionKeys *sessionKeys + apiKey string + secret string + lang string + client *Client } func lastFMConstructor(ds model.DataStore) *lastfmAgent { l := &lastfmAgent{ - ds: ds, - lang: conf.Server.LastFM.Language, - apiKey: conf.Server.LastFM.ApiKey, - secret: conf.Server.LastFM.Secret, + ds: ds, + lang: conf.Server.LastFM.Language, + apiKey: conf.Server.LastFM.ApiKey, + secret: conf.Server.LastFM.Secret, + sessionKeys: &sessionKeys{ds: ds}, } - hc := utils.NewCachedHTTPClient(http.DefaultClient, consts.DefaultCachedHttpClientTTL) - l.client = NewClient(l.apiKey, l.secret, l.lang, hc) + hc := &http.Client{ + Timeout: consts.DefaultHttpClientTimeOut, + } + chc := utils.NewCachedHTTPClient(hc, consts.DefaultHttpClientTimeOut) + l.client = NewClient(l.apiKey, l.secret, l.lang, chc) return l } @@ -43,7 +47,7 @@ func (l *lastfmAgent) AgentName() string { } func (l *lastfmAgent) GetMBID(ctx context.Context, id string, name string) (string, error) { - a, err := l.callArtistGetInfo(name, "") + a, err := l.callArtistGetInfo(ctx, name, "") if err != nil { return "", err } @@ -54,7 +58,7 @@ func (l *lastfmAgent) GetMBID(ctx context.Context, id string, name string) (stri } func (l *lastfmAgent) GetURL(ctx context.Context, id, name, mbid string) (string, error) { - a, err := l.callArtistGetInfo(name, mbid) + a, err := l.callArtistGetInfo(ctx, name, mbid) if err != nil { return "", err } @@ -65,7 +69,7 @@ func (l *lastfmAgent) GetURL(ctx context.Context, id, name, mbid string) (string } func (l *lastfmAgent) GetBiography(ctx context.Context, id, name, mbid string) (string, error) { - a, err := l.callArtistGetInfo(name, mbid) + a, err := l.callArtistGetInfo(ctx, name, mbid) if err != nil { return "", err } @@ -76,7 +80,7 @@ func (l *lastfmAgent) GetBiography(ctx context.Context, id, name, mbid string) ( } func (l *lastfmAgent) GetSimilar(ctx context.Context, id, name, mbid string, limit int) ([]agents.Artist, error) { - resp, err := l.callArtistGetSimilar(name, mbid, limit) + resp, err := l.callArtistGetSimilar(ctx, name, mbid, limit) if err != nil { return nil, err } @@ -94,7 +98,7 @@ func (l *lastfmAgent) GetSimilar(ctx context.Context, id, name, mbid string, lim } func (l *lastfmAgent) GetTopSongs(ctx context.Context, id, artistName, mbid string, count int) ([]agents.Song, error) { - resp, err := l.callArtistGetTopTracks(artistName, mbid, count) + resp, err := l.callArtistGetTopTracks(ctx, artistName, mbid, count) if err != nil { return nil, err } @@ -111,54 +115,91 @@ func (l *lastfmAgent) GetTopSongs(ctx context.Context, id, artistName, mbid stri return res, nil } -func (l *lastfmAgent) callArtistGetInfo(name string, mbid string) (*Artist, error) { - a, err := l.client.ArtistGetInfo(l.ctx, name, mbid) +func (l *lastfmAgent) callArtistGetInfo(ctx context.Context, name string, mbid string) (*Artist, error) { + a, err := l.client.ArtistGetInfo(ctx, name, mbid) lfErr, isLastFMError := err.(*lastFMError) if mbid != "" && ((err == nil && a.Name == "[unknown]") || (isLastFMError && lfErr.Code == 6)) { - log.Warn(l.ctx, "LastFM/artist.getInfo could not find artist by mbid, trying again", "artist", name, "mbid", mbid) - return l.callArtistGetInfo(name, "") + log.Warn(ctx, "LastFM/artist.getInfo could not find artist by mbid, trying again", "artist", name, "mbid", mbid) + return l.callArtistGetInfo(ctx, name, "") } if err != nil { - log.Error(l.ctx, "Error calling LastFM/artist.getInfo", "artist", name, "mbid", mbid, err) + log.Error(ctx, "Error calling LastFM/artist.getInfo", "artist", name, "mbid", mbid, err) return nil, err } return a, nil } -func (l *lastfmAgent) callArtistGetSimilar(name string, mbid string, limit int) ([]Artist, error) { - s, err := l.client.ArtistGetSimilar(l.ctx, name, mbid, limit) +func (l *lastfmAgent) callArtistGetSimilar(ctx context.Context, name string, mbid string, limit int) ([]Artist, error) { + s, err := l.client.ArtistGetSimilar(ctx, name, mbid, limit) lfErr, isLastFMError := err.(*lastFMError) if mbid != "" && ((err == nil && s.Attr.Artist == "[unknown]") || (isLastFMError && lfErr.Code == 6)) { - log.Warn(l.ctx, "LastFM/artist.getSimilar could not find artist by mbid, trying again", "artist", name, "mbid", mbid) - return l.callArtistGetSimilar(name, "", limit) + log.Warn(ctx, "LastFM/artist.getSimilar could not find artist by mbid, trying again", "artist", name, "mbid", mbid) + return l.callArtistGetSimilar(ctx, name, "", limit) } if err != nil { - log.Error(l.ctx, "Error calling LastFM/artist.getSimilar", "artist", name, "mbid", mbid, err) + log.Error(ctx, "Error calling LastFM/artist.getSimilar", "artist", name, "mbid", mbid, err) return nil, err } return s.Artists, nil } -func (l *lastfmAgent) callArtistGetTopTracks(artistName, mbid string, count int) ([]Track, error) { - t, err := l.client.ArtistGetTopTracks(l.ctx, artistName, mbid, count) +func (l *lastfmAgent) callArtistGetTopTracks(ctx context.Context, artistName, mbid string, count int) ([]Track, error) { + t, err := l.client.ArtistGetTopTracks(ctx, artistName, mbid, count) lfErr, isLastFMError := err.(*lastFMError) if mbid != "" && ((err == nil && t.Attr.Artist == "[unknown]") || (isLastFMError && lfErr.Code == 6)) { - log.Warn(l.ctx, "LastFM/artist.getTopTracks could not find artist by mbid, trying again", "artist", artistName, "mbid", mbid) - return l.callArtistGetTopTracks(artistName, "", count) + log.Warn(ctx, "LastFM/artist.getTopTracks could not find artist by mbid, trying again", "artist", artistName, "mbid", mbid) + return l.callArtistGetTopTracks(ctx, artistName, "", count) } if err != nil { - log.Error(l.ctx, "Error calling LastFM/artist.getTopTracks", "artist", artistName, "mbid", mbid, err) + log.Error(ctx, "Error calling LastFM/artist.getTopTracks", "artist", artistName, "mbid", mbid, err) return nil, err } return t.Track, nil } -func (l *lastfmAgent) NowPlaying(c context.Context, track *model.MediaFile) error { +func (l *lastfmAgent) NowPlaying(ctx context.Context, userId string, track *model.MediaFile) error { + sk, err := l.sessionKeys.get(ctx, userId) + if err != nil { + return err + } + err = l.client.UpdateNowPlaying(ctx, sk, ScrobbleInfo{ + artist: track.Artist, + track: track.Title, + album: track.Album, + trackNumber: track.TrackNumber, + mbid: track.MbzTrackID, + duration: int(track.Duration), + albumArtist: track.AlbumArtist, + }) + if err != nil { + return err + } return nil } -func (l *lastfmAgent) Scrobble(ctx context.Context, scrobbles []scrobbler.Scrobble) error { +func (l *lastfmAgent) Scrobble(ctx context.Context, userId string, scrobbles []scrobbler.Scrobble) error { + sk, err := l.sessionKeys.get(ctx, userId) + if err != nil { + return err + } + + // TODO Implement batch scrobbling + for _, s := range scrobbles { + err = l.client.Scrobble(ctx, sk, ScrobbleInfo{ + artist: s.Artist, + track: s.Title, + album: s.Album, + trackNumber: s.TrackNumber, + mbid: s.MbzTrackID, + duration: int(s.Duration), + albumArtist: s.AlbumArtist, + timestamp: s.TimeStamp, + }) + if err != nil { + return err + } + } return nil } diff --git a/core/agents/lastfm/agent_test.go b/core/agents/lastfm/agent_test.go index 77a49709a..efb86c903 100644 --- a/core/agents/lastfm/agent_test.go +++ b/core/agents/lastfm/agent_test.go @@ -7,6 +7,14 @@ import ( "io/ioutil" "net/http" "os" + "strconv" + "time" + + "github.com/navidrome/navidrome/core/scrobbler" + + "github.com/navidrome/navidrome/model/request" + + "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/core/agents" @@ -21,15 +29,21 @@ const ( ) var _ = Describe("lastfmAgent", func() { + var ds model.DataStore + var ctx context.Context + BeforeEach(func() { + ds = &tests.MockDataStore{} + ctx = context.Background() + }) Describe("lastFMConstructor", func() { It("uses configured api key and language", func() { conf.Server.LastFM.ApiKey = "123" conf.Server.LastFM.Secret = "secret" conf.Server.LastFM.Language = "pt" - agent := lastFMConstructor(context.Background()) - Expect(agent.(*lastfmAgent).apiKey).To(Equal("123")) - Expect(agent.(*lastfmAgent).secret).To(Equal("secret")) - Expect(agent.(*lastfmAgent).lang).To(Equal("pt")) + agent := lastFMConstructor(ds) + Expect(agent.apiKey).To(Equal("123")) + Expect(agent.secret).To(Equal("secret")) + Expect(agent.lang).To(Equal("pt")) }) }) @@ -39,7 +53,7 @@ var _ = Describe("lastfmAgent", func() { BeforeEach(func() { httpClient = &tests.FakeHttpClient{} client := NewClient("API_KEY", "SECRET", "pt", httpClient) - agent = lastFMConstructor(context.Background()).(*lastfmAgent) + agent = lastFMConstructor(ds) agent.client = client }) @@ -97,7 +111,7 @@ var _ = Describe("lastfmAgent", func() { BeforeEach(func() { httpClient = &tests.FakeHttpClient{} client := NewClient("API_KEY", "SECRET", "pt", httpClient) - agent = lastFMConstructor(context.Background()).(*lastfmAgent) + agent = lastFMConstructor(ds) agent.client = client }) @@ -158,7 +172,7 @@ var _ = Describe("lastfmAgent", func() { BeforeEach(func() { httpClient = &tests.FakeHttpClient{} client := NewClient("API_KEY", "SECRET", "pt", httpClient) - agent = lastFMConstructor(context.Background()).(*lastfmAgent) + agent = lastFMConstructor(ds) agent.client = client }) @@ -212,4 +226,74 @@ var _ = Describe("lastfmAgent", func() { }) }) }) + + Describe("Scrobbling", func() { + var agent *lastfmAgent + var httpClient *tests.FakeHttpClient + var track *model.MediaFile + BeforeEach(func() { + ctx = request.WithUser(ctx, model.User{ID: "user-1"}) + _ = ds.Property(ctx).Put(sessionKeyPropertyPrefix+"user-1", "SK-1") + httpClient = &tests.FakeHttpClient{} + client := NewClient("API_KEY", "SECRET", "en", httpClient) + agent = lastFMConstructor(ds) + agent.client = client + track = &model.MediaFile{ + ID: "123", + Title: "Track Title", + Album: "Track Album", + Artist: "Track Artist", + AlbumArtist: "Track AlbumArtist", + TrackNumber: 1, + Duration: 180, + MbzTrackID: "mbz-123", + } + }) + + Describe("NowPlaying", func() { + It("calls Last.fm with correct params", func() { + httpClient.Res = http.Response{Body: ioutil.NopCloser(bytes.NewBufferString("{}")), StatusCode: 200} + + err := agent.NowPlaying(ctx, "user-1", track) + + Expect(err).ToNot(HaveOccurred()) + Expect(httpClient.SavedRequest.Method).To(Equal(http.MethodPost)) + sentParams := httpClient.SavedRequest.URL.Query() + Expect(sentParams.Get("method")).To(Equal("track.updateNowPlaying")) + Expect(sentParams.Get("sk")).To(Equal("SK-1")) + Expect(sentParams.Get("track")).To(Equal(track.Title)) + Expect(sentParams.Get("album")).To(Equal(track.Album)) + Expect(sentParams.Get("artist")).To(Equal(track.Artist)) + Expect(sentParams.Get("albumArtist")).To(Equal(track.AlbumArtist)) + Expect(sentParams.Get("trackNumber")).To(Equal(strconv.Itoa(track.TrackNumber))) + Expect(sentParams.Get("duration")).To(Equal(strconv.FormatFloat(float64(track.Duration), 'G', -1, 32))) + Expect(sentParams.Get("mbid")).To(Equal(track.MbzTrackID)) + }) + }) + + Describe("Scrobble", func() { + It("calls Last.fm with correct params", func() { + ts := time.Now() + scrobbles := []scrobbler.Scrobble{{MediaFile: *track, TimeStamp: ts}} + httpClient.Res = http.Response{Body: ioutil.NopCloser(bytes.NewBufferString("{}")), StatusCode: 200} + + err := agent.Scrobble(ctx, "user-1", scrobbles) + + Expect(err).ToNot(HaveOccurred()) + Expect(httpClient.SavedRequest.Method).To(Equal(http.MethodPost)) + sentParams := httpClient.SavedRequest.URL.Query() + Expect(sentParams.Get("method")).To(Equal("track.scrobble")) + Expect(sentParams.Get("sk")).To(Equal("SK-1")) + Expect(sentParams.Get("track")).To(Equal(track.Title)) + Expect(sentParams.Get("album")).To(Equal(track.Album)) + Expect(sentParams.Get("artist")).To(Equal(track.Artist)) + Expect(sentParams.Get("albumArtist")).To(Equal(track.AlbumArtist)) + Expect(sentParams.Get("trackNumber")).To(Equal(strconv.Itoa(track.TrackNumber))) + Expect(sentParams.Get("duration")).To(Equal(strconv.FormatFloat(float64(track.Duration), 'G', -1, 32))) + Expect(sentParams.Get("mbid")).To(Equal(track.MbzTrackID)) + Expect(sentParams.Get("timestamp")).To(Equal(strconv.FormatInt(ts.Unix(), 10))) + }) + }) + }) + }) diff --git a/core/agents/lastfm/auth_router.go b/core/agents/lastfm/auth_router.go index 04f033580..3a57507b9 100644 --- a/core/agents/lastfm/auth_router.go +++ b/core/agents/lastfm/auth_router.go @@ -7,6 +7,8 @@ import ( "net/http" "time" + "github.com/navidrome/navidrome/consts" + "github.com/deluan/rest" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" @@ -32,13 +34,16 @@ type Router struct { func NewRouter(ds model.DataStore) *Router { r := &Router{ - ds: ds, - apiKey: conf.Server.LastFM.ApiKey, - secret: conf.Server.LastFM.Secret, + ds: ds, + apiKey: conf.Server.LastFM.ApiKey, + secret: conf.Server.LastFM.Secret, + sessionKeys: &sessionKeys{ds: ds}, } - r.sessionKeys = &sessionKeys{ds: ds} r.Handler = r.routes() - r.client = NewClient(r.apiKey, r.secret, "en", http.DefaultClient) + hc := &http.Client{ + Timeout: consts.DefaultHttpClientTimeOut, + } + r.client = NewClient(r.apiKey, r.secret, "en", hc) return r } diff --git a/core/agents/lastfm/client.go b/core/agents/lastfm/client.go index b16178954..cf7534da3 100644 --- a/core/agents/lastfm/client.go +++ b/core/agents/lastfm/client.go @@ -6,12 +6,15 @@ import ( "encoding/hex" "encoding/json" "fmt" + "io/ioutil" "net/http" "net/url" "sort" "strconv" "strings" + "time" + "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/utils" ) @@ -49,7 +52,7 @@ func (c *Client) ArtistGetInfo(ctx context.Context, name string, mbid string) (* params.Add("artist", name) params.Add("mbid", mbid) params.Add("lang", c.lang) - response, err := c.makeRequest(params, false) + response, err := c.makeRequest(http.MethodGet, params, false) if err != nil { return nil, err } @@ -62,7 +65,7 @@ func (c *Client) ArtistGetSimilar(ctx context.Context, name string, mbid string, params.Add("artist", name) params.Add("mbid", mbid) params.Add("limit", strconv.Itoa(limit)) - response, err := c.makeRequest(params, false) + response, err := c.makeRequest(http.MethodGet, params, false) if err != nil { return nil, err } @@ -75,7 +78,7 @@ func (c *Client) ArtistGetTopTracks(ctx context.Context, name string, mbid strin params.Add("artist", name) params.Add("mbid", mbid) params.Add("limit", strconv.Itoa(limit)) - response, err := c.makeRequest(params, false) + response, err := c.makeRequest(http.MethodGet, params, false) if err != nil { return nil, err } @@ -86,7 +89,7 @@ func (c *Client) GetToken(ctx context.Context) (string, error) { params := url.Values{} params.Add("method", "auth.getToken") c.sign(params) - response, err := c.makeRequest(params, true) + response, err := c.makeRequest(http.MethodGet, params, true) if err != nil { return "", err } @@ -97,14 +100,74 @@ func (c *Client) GetSession(ctx context.Context, token string) (string, error) { params := url.Values{} params.Add("method", "auth.getSession") params.Add("token", token) - response, err := c.makeRequest(params, true) + response, err := c.makeRequest(http.MethodGet, params, true) if err != nil { return "", err } return response.Session.Key, nil } -func (c *Client) makeRequest(params url.Values, signed bool) (*Response, error) { +type ScrobbleInfo struct { + artist string + track string + album string + trackNumber int + mbid string + duration int + albumArtist string + timestamp time.Time +} + +func (c *Client) UpdateNowPlaying(ctx context.Context, sessionKey string, info ScrobbleInfo) error { + params := url.Values{} + params.Add("method", "track.updateNowPlaying") + params.Add("artist", info.artist) + params.Add("track", info.track) + params.Add("album", info.album) + params.Add("trackNumber", strconv.Itoa(info.trackNumber)) + params.Add("mbid", info.mbid) + params.Add("duration", strconv.Itoa(info.duration)) + params.Add("albumArtist", info.albumArtist) + params.Add("sk", sessionKey) + resp, err := c.makeRequest(http.MethodPost, params, true) + if err != nil { + return err + } + if resp.NowPlaying.IgnoredMessage.Code != "" { + log.Warn(ctx, "LastFM: NowPlaying was ignored", "code", resp.NowPlaying.IgnoredMessage.Code, + "text", resp.NowPlaying.IgnoredMessage.Text) + } + return nil +} + +func (c *Client) Scrobble(ctx context.Context, sessionKey string, info ScrobbleInfo) error { + params := url.Values{} + params.Add("method", "track.scrobble") + params.Add("timestamp", strconv.FormatInt(info.timestamp.Unix(), 10)) + params.Add("artist", info.artist) + params.Add("track", info.track) + params.Add("album", info.album) + params.Add("trackNumber", strconv.Itoa(info.trackNumber)) + params.Add("mbid", info.mbid) + params.Add("duration", strconv.Itoa(info.duration)) + params.Add("albumArtist", info.albumArtist) + params.Add("sk", sessionKey) + resp, err := c.makeRequest(http.MethodPost, params, true) + if err != nil { + return err + } + if resp.Scrobbles.Scrobble.IgnoredMessage.Code != "0" { + log.Warn(ctx, "LastFM: Scrobble was ignored", "code", resp.Scrobbles.Scrobble.IgnoredMessage.Code, + "text", resp.Scrobbles.Scrobble.IgnoredMessage.Text) + } + if resp.Scrobbles.Attr.Accepted != 1 { + log.Warn(ctx, "LastFM: Scrobble was not accepted", "code", resp.Scrobbles.Scrobble.IgnoredMessage.Code, + "text", resp.Scrobbles.Scrobble.IgnoredMessage.Text) + } + return nil +} + +func (c *Client) makeRequest(method string, params url.Values, signed bool) (*Response, error) { params.Add("format", "json") params.Add("api_key", c.apiKey) @@ -112,7 +175,7 @@ func (c *Client) makeRequest(params url.Values, signed bool) (*Response, error) c.sign(params) } - req, _ := http.NewRequest("GET", apiBaseUrl, nil) + req, _ := http.NewRequest(method, apiBaseUrl, nil) req.URL.RawQuery = params.Encode() resp, err := c.hc.Do(req) @@ -121,7 +184,8 @@ func (c *Client) makeRequest(params url.Values, signed bool) (*Response, error) } defer resp.Body.Close() - decoder := json.NewDecoder(resp.Body) + body, _ := ioutil.ReadAll(resp.Body) + decoder := json.NewDecoder(strings.NewReader(string(body))) var response Response jsonErr := decoder.Decode(&response) diff --git a/core/agents/lastfm/responses.go b/core/agents/lastfm/responses.go index a685ecc25..b87563c0d 100644 --- a/core/agents/lastfm/responses.go +++ b/core/agents/lastfm/responses.go @@ -8,6 +8,8 @@ type Response struct { Message string `json:"message"` Token string `json:"token"` Session Session `json:"session"` + NowPlaying NowPlaying `json:"nowplaying"` + Scrobbles Scrobbles `json:"scrobbles"` } type Artist struct { @@ -67,3 +69,56 @@ type Session struct { Key string `json:"key"` Subscriber int `json:"subscriber"` } + +type NowPlaying struct { + Artist struct { + Corrected string `json:"corrected"` + Text string `json:"#text"` + } `json:"artist"` + IgnoredMessage struct { + Code string `json:"code"` + Text string `json:"#text"` + } `json:"ignoredMessage"` + Album struct { + Corrected string `json:"corrected"` + Text string `json:"#text"` + } `json:"album"` + AlbumArtist struct { + Corrected string `json:"corrected"` + Text string `json:"#text"` + } `json:"albumArtist"` + Track struct { + Corrected string `json:"corrected"` + Text string `json:"#text"` + } `json:"track"` +} + +type Scrobbles struct { + Attr struct { + Accepted int `json:"accepted"` + Ignored int `json:"ignored"` + } `json:"@attr"` + Scrobble struct { + Artist struct { + Corrected string `json:"corrected"` + Text string `json:"#text"` + } `json:"artist"` + IgnoredMessage struct { + Code string `json:"code"` + Text string `json:"#text"` + } `json:"ignoredMessage"` + AlbumArtist struct { + Corrected string `json:"corrected"` + Text string `json:"#text"` + } `json:"albumArtist"` + Timestamp string `json:"timestamp"` + Album struct { + Corrected string `json:"corrected"` + Text string `json:"#text"` + } `json:"album"` + Track struct { + Corrected string `json:"corrected"` + Text string `json:"#text"` + } `json:"track"` + } `json:"scrobble"` +} diff --git a/core/agents/spotify/spotify.go b/core/agents/spotify/spotify.go index 697e43b19..0f5389e50 100644 --- a/core/agents/spotify/spotify.go +++ b/core/agents/spotify/spotify.go @@ -31,8 +31,11 @@ func spotifyConstructor(ds model.DataStore) agents.Interface { id: conf.Server.Spotify.ID, secret: conf.Server.Spotify.Secret, } - hc := utils.NewCachedHTTPClient(http.DefaultClient, consts.DefaultCachedHttpClientTTL) - l.client = NewClient(l.id, l.secret, hc) + hc := &http.Client{ + Timeout: consts.DefaultHttpClientTimeOut, + } + chc := utils.NewCachedHTTPClient(hc, consts.DefaultHttpClientTimeOut) + l.client = NewClient(l.id, l.secret, chc) return l } diff --git a/core/scrobbler/broker.go b/core/scrobbler/broker.go index 4f7bd0af4..532ec2d79 100644 --- a/core/scrobbler/broker.go +++ b/core/scrobbler/broker.go @@ -26,7 +26,7 @@ type NowPlayingInfo struct { type Broker interface { NowPlaying(ctx context.Context, playerId string, playerName string, trackId string) error GetNowPlaying(ctx context.Context) ([]NowPlayingInfo, error) - Submit(ctx context.Context, playerId int, trackId string, playTime time.Time) error + Submit(ctx context.Context, trackId string, playTime time.Time) error } type broker struct { @@ -45,20 +45,20 @@ func GetBroker(ds model.DataStore) Broker { } func (s *broker) NowPlaying(ctx context.Context, playerId string, playerName string, trackId string) error { - username, _ := request.UsernameFrom(ctx) + user, _ := request.UserFrom(ctx) info := NowPlayingInfo{ TrackID: trackId, Start: time.Now(), - Username: username, + Username: user.UserName, PlayerId: playerId, PlayerName: playerName, } _ = s.playMap.Set(playerId, info) - s.dispatchNowPlaying(ctx, trackId) + s.dispatchNowPlaying(ctx, user.ID, trackId) return nil } -func (s *broker) dispatchNowPlaying(ctx context.Context, trackId string) { +func (s *broker) dispatchNowPlaying(ctx context.Context, userId string, trackId string) { t, err := s.ds.MediaFile(ctx).Get(trackId) if err != nil { log.Error(ctx, "Error retrieving mediaFile", "id", trackId, err) @@ -68,10 +68,8 @@ func (s *broker) dispatchNowPlaying(ctx context.Context, trackId string) { for name, constructor := range scrobblers { log.Debug(ctx, "Sending NowPlaying info", "scrobbler", name, "track", t.Title, "artist", t.Artist) err := func() error { - ctx, cancel := context.WithTimeout(ctx, 5*time.Second) - defer cancel() s := constructor(s.ds) - return s.NowPlaying(ctx, t) + return s.NowPlaying(ctx, userId, t) }() if err != nil { log.Error(ctx, "Error sending NowPlayingInfo", "scrobbler", name, "track", t.Title, "artist", t.Artist, err) @@ -96,8 +94,27 @@ func (s *broker) GetNowPlaying(ctx context.Context) ([]NowPlayingInfo, error) { return res, nil } -func (s *broker) Submit(ctx context.Context, playerId int, trackId string, playTime time.Time) error { - panic("implement me") +func (s *broker) Submit(ctx context.Context, trackId string, playTime time.Time) error { + u, _ := request.UserFrom(ctx) + t, err := s.ds.MediaFile(ctx).Get(trackId) + if err != nil { + log.Error(ctx, "Error retrieving mediaFile", "id", trackId, err) + return err + } + scrobbles := []Scrobble{{MediaFile: *t, TimeStamp: playTime}} + // TODO Parallelize + for name, constructor := range scrobblers { + log.Debug(ctx, "Sending NowPlaying info", "scrobbler", name, "track", t.Title, "artist", t.Artist) + err := func() error { + s := constructor(s.ds) + return s.Scrobble(ctx, u.ID, scrobbles) + }() + if err != nil { + log.Error(ctx, "Error sending NowPlayingInfo", "scrobbler", name, "track", t.Title, "artist", t.Artist, err) + return err + } + } + return nil } var scrobblers map[string]Constructor diff --git a/core/scrobbler/broker_test.go b/core/scrobbler/broker_test.go new file mode 100644 index 000000000..e558158e6 --- /dev/null +++ b/core/scrobbler/broker_test.go @@ -0,0 +1,119 @@ +package scrobbler + +import ( + "context" + "time" + + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +var _ = Describe("Broker", func() { + var ctx context.Context + var ds model.DataStore + var broker Broker + var track model.MediaFile + var fake *fakeScrobbler + BeforeEach(func() { + ctx = context.Background() + ctx = request.WithUser(ctx, model.User{ID: "u-1"}) + ds = &tests.MockDataStore{} + broker = GetBroker(ds) + fake = &fakeScrobbler{} + Register("fake", func(ds model.DataStore) Scrobbler { + return fake + }) + + track = model.MediaFile{ + ID: "123", + Title: "Track Title", + Album: "Track Album", + Artist: "Track Artist", + AlbumArtist: "Track AlbumArtist", + TrackNumber: 1, + Duration: 180, + MbzTrackID: "mbz-123", + } + _ = ds.MediaFile(ctx).Put(&track) + }) + + Describe("NowPlaying", func() { + It("sends track to agent", func() { + err := broker.NowPlaying(ctx, "player-1", "player-one", "123") + Expect(err).ToNot(HaveOccurred()) + Expect(fake.UserID).To(Equal("u-1")) + Expect(fake.Track.ID).To(Equal("123")) + }) + }) + + 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"}) + _ = broker.NowPlaying(ctx, "player-1", "player-one", "123") + ctx = request.WithUser(ctx, model.User{UserName: "user-2"}) + _ = broker.NowPlaying(ctx, "player-2", "player-two", "456") + + playing, err := broker.GetNowPlaying(ctx) + + Expect(err).ToNot(HaveOccurred()) + Expect(playing).To(HaveLen(2)) + 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[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")) + }) + }) + + Describe("Submit", func() { + It("sends track to agent", func() { + ctx = request.WithUser(ctx, model.User{ID: "u-1", UserName: "user-1"}) + ts := time.Now() + + err := broker.Submit(ctx, "123", ts) + + Expect(err).ToNot(HaveOccurred()) + Expect(fake.UserID).To(Equal("u-1")) + Expect(fake.Scrobbles[0].ID).To(Equal("123")) + }) + }) + +}) + +type fakeScrobbler struct { + UserID string + Track *model.MediaFile + Scrobbles []Scrobble + Error error +} + +func (f *fakeScrobbler) NowPlaying(ctx context.Context, userId string, track *model.MediaFile) error { + if f.Error != nil { + return f.Error + } + f.UserID = userId + f.Track = track + return nil +} + +func (f *fakeScrobbler) Scrobble(ctx context.Context, userId string, scrobbles []Scrobble) error { + if f.Error != nil { + return f.Error + } + f.UserID = userId + f.Scrobbles = scrobbles + return nil +} diff --git a/core/scrobbler/interfaces.go b/core/scrobbler/interfaces.go index 52b0bf3e8..024794a03 100644 --- a/core/scrobbler/interfaces.go +++ b/core/scrobbler/interfaces.go @@ -8,13 +8,13 @@ import ( ) type Scrobble struct { - Track *model.MediaFile - TimeStamp *time.Time + model.MediaFile + TimeStamp time.Time } type Scrobbler interface { - NowPlaying(context.Context, *model.MediaFile) error - Scrobble(context.Context, []Scrobble) error + NowPlaying(ctx context.Context, userId string, track *model.MediaFile) error + Scrobble(ctx context.Context, userId string, scrobbles []Scrobble) error } type Constructor func(ds model.DataStore) Scrobbler diff --git a/core/scrobbler/scrobbler_suite_test.go b/core/scrobbler/scrobbler_suite_test.go new file mode 100644 index 000000000..d32028ab5 --- /dev/null +++ b/core/scrobbler/scrobbler_suite_test.go @@ -0,0 +1,17 @@ +package scrobbler + +import ( + "testing" + + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestAgents(t *testing.T) { + tests.Init(t, false) + log.SetLevel(log.LevelCritical) + RegisterFailHandler(Fail) + RunSpecs(t, "Scrobbler Test Suite") +} diff --git a/server/subsonic/media_annotation.go b/server/subsonic/media_annotation.go index 4dbec7a3e..9888e805b 100644 --- a/server/subsonic/media_annotation.go +++ b/server/subsonic/media_annotation.go @@ -186,7 +186,7 @@ func (c *MediaAnnotationController) scrobblerRegister(ctx context.Context, track } else { log.Info("Scrobbled", "title", mf.Title, "artist", mf.Artist, "user", username) } - + _ = c.scrobbler.Submit(ctx, trackId, playTime) return mf, err } diff --git a/tests/mock_mediafile_repo.go b/tests/mock_mediafile_repo.go index 341973514..84ac4a2c9 100644 --- a/tests/mock_mediafile_repo.go +++ b/tests/mock_mediafile_repo.go @@ -3,11 +3,15 @@ package tests import ( "errors" + "github.com/google/uuid" + "github.com/navidrome/navidrome/model" ) func CreateMockMediaFileRepo() *MockMediaFileRepo { - return &MockMediaFileRepo{} + return &MockMediaFileRepo{ + data: make(map[string]model.MediaFile), + } } type MockMediaFileRepo struct { @@ -45,6 +49,17 @@ func (m *MockMediaFileRepo) Get(id string) (*model.MediaFile, error) { return nil, model.ErrNotFound } +func (m *MockMediaFileRepo) Put(mf *model.MediaFile) error { + if m.err { + return errors.New("error!") + } + if mf.ID == "" { + mf.ID = uuid.NewString() + } + m.data[mf.ID] = *mf + return nil +} + func (m *MockMediaFileRepo) FindByAlbum(artistId string) (model.MediaFiles, error) { if m.err { return nil, errors.New("Error!") diff --git a/utils/cached_http_client_test.go b/utils/cached_http_client_test.go index c7357328d..71263b9e9 100644 --- a/utils/cached_http_client_test.go +++ b/utils/cached_http_client_test.go @@ -25,7 +25,7 @@ var _ = Describe("CachedHttpClient", func() { header = r.Header.Get("head") _, _ = fmt.Fprintf(w, "Hello, %s", r.URL.Query()["name"]) })) - chc = NewCachedHTTPClient(http.DefaultClient, consts.DefaultCachedHttpClientTTL) + chc = NewCachedHTTPClient(http.DefaultClient, consts.DefaultHttpClientTimeOut) }) AfterEach(func() {