diff --git a/adapters/lastfm/agent.go b/adapters/lastfm/agent.go index dbd73c30d..a4262a0b8 100644 --- a/adapters/lastfm/agent.go +++ b/adapters/lastfm/agent.go @@ -23,8 +23,9 @@ import ( ) const ( - lastFMAgentName = "lastfm" - sessionKeyProperty = "LastFMSessionKey" + lastFMAgentName = "lastfm" + sessionKeyProperty = "LastFMSessionKey" + syncFavoritesProperty = "LastFMSyncFavorites" ) var ignoredContent = []string{ @@ -392,8 +393,9 @@ func (l *lastfmAgent) Scrobble(ctx context.Context, userId string, s scrobbler.S log.Debug(ctx, "Skipping Last.fm scrobble for short song", "track", s.Title, "duration", s.Duration) return nil } + artist := l.getArtistForScrobble(&s.MediaFile, model.RoleArtist, s.Artist) err = l.client.scrobble(ctx, sk, ScrobbleInfo{ - artist: l.getArtistForScrobble(&s.MediaFile, model.RoleArtist, s.Artist), + artist: artist, track: s.Title, album: s.Album, trackNumber: s.TrackNumber, @@ -402,20 +404,33 @@ func (l *lastfmAgent) Scrobble(ctx context.Context, userId string, s scrobbler.S albumArtist: l.getArtistForScrobble(&s.MediaFile, model.RoleAlbumArtist, s.AlbumArtist), timestamp: s.TimeStamp, }) - if err == nil { - return nil + if err != nil { + var lfErr *lastFMError + isLastFMError := errors.As(err, &lfErr) + if !isLastFMError { + log.Warn(ctx, "Last.fm client.scrobble returned error", "track", s.Title, err) + return errors.Join(err, scrobbler.ErrRetryLater) + } + // 11: service offline; 16: temporarily unavailable. Rate limiting is mapped by the client. + if lfErr.Code == 11 || lfErr.Code == 16 || errors.Is(err, scrobbler.ErrRetryLater) { + return errors.Join(err, scrobbler.ErrRetryLater) + } + return errors.Join(err, scrobbler.ErrUnrecoverable) } - var lfErr *lastFMError - isLastFMError := errors.As(err, &lfErr) - if !isLastFMError { - log.Warn(ctx, "Last.fm client.scrobble returned error", "track", s.Title, err) - return errors.Join(err, scrobbler.ErrRetryLater) + if s.Starred && syncFavoritesEnabled(ctx, l.ds.UserProps(), userId) { + if loveErr := l.client.love(ctx, sk, artist, s.Title); loveErr != nil { + log.Warn(ctx, "Last.fm client.love returned error", "track", s.Title, loveErr) + } } - // 11: service offline; 16: temporarily unavailable. Rate limiting is mapped by the client. - if lfErr.Code == 11 || lfErr.Code == 16 || errors.Is(err, scrobbler.ErrRetryLater) { - return errors.Join(err, scrobbler.ErrRetryLater) + return nil +} + +func syncFavoritesEnabled(ctx context.Context, props model.UserPropsRepository, userId string) bool { + v, err := props.DefaultGet(ctx, userId, syncFavoritesProperty, "false") + if err != nil { + log.Warn(ctx, "Could not read Last.fm favorite sync setting", "userId", userId, err) } - return errors.Join(err, scrobbler.ErrUnrecoverable) + return v == "true" } func (l *lastfmAgent) IsAuthorized(ctx context.Context, userId string) bool { diff --git a/adapters/lastfm/agent_test.go b/adapters/lastfm/agent_test.go index 0f68bd46b..fa6bc9d6b 100644 --- a/adapters/lastfm/agent_test.go +++ b/adapters/lastfm/agent_test.go @@ -452,6 +452,58 @@ var _ = Describe("lastfmAgent", func() { Expect(sentParams.Get("timestamp")).To(Equal(strconv.FormatInt(ts.Unix(), 10))) }) + It("does not love a track that is not starred", func() { + httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString("{}")), StatusCode: 200} + + err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()}) + + Expect(err).ToNot(HaveOccurred()) + Expect(httpClient.RequestCount).To(Equal(1)) + body, _ := io.ReadAll(httpClient.SavedRequest.Body) + sentParams, _ := url.ParseQuery(string(body)) + Expect(sentParams.Get("method")).To(Equal("track.scrobble")) + }) + + It("loves a starred track after scrobbling it", func() { + track.Starred = true + Expect(ds.UserProps().Put(ctx, "user-1", syncFavoritesProperty, "true")).To(Succeed()) + recorder := &recordingHttpClient{} + agent.client = newClient("API_KEY", "SECRET", recorder) + + err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()}) + + Expect(err).ToNot(HaveOccurred()) + Expect(recorder.methods()).To(Equal([]string{"track.scrobble", "track.love"})) + love := recorder.params[1] + Expect(love.Get("artist")).To(Equal(track.Artist)) + Expect(love.Get("track")).To(Equal(track.Title)) + Expect(love.Get("sk")).To(Equal("SK-1")) + }) + + It("does not love a starred track when favorite sync is off", func() { + track.Starred = true + Expect(ds.UserProps().Put(ctx, "user-1", syncFavoritesProperty, "false")).To(Succeed()) + recorder := &recordingHttpClient{} + agent.client = newClient("API_KEY", "SECRET", recorder) + + err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()}) + + Expect(err).ToNot(HaveOccurred()) + Expect(recorder.methods()).To(Equal([]string{"track.scrobble"})) + }) + + It("still scrobbles when loving the track fails", func() { + track.Starred = true + Expect(ds.UserProps().Put(ctx, "user-1", syncFavoritesProperty, "true")).To(Succeed()) + recorder := &recordingHttpClient{fail: map[string]bool{"track.love": true}} + agent.client = newClient("API_KEY", "SECRET", recorder) + + err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()}) + + Expect(err).ToNot(HaveOccurred()) + Expect(recorder.methods()).To(Equal([]string{"track.scrobble", "track.love"})) + }) + When("ScrobbleFirstArtistOnly is true", func() { BeforeEach(func() { conf.Server.LastFM.ScrobbleFirstArtistOnly = true @@ -469,6 +521,20 @@ var _ = Describe("lastfmAgent", func() { Expect(sentParams.Get("artist")).To(Equal("First Artist")) Expect(sentParams.Get("albumArtist")).To(Equal("First Album Artist")) }) + + It("loves the track under the first artist", func() { + track.Starred = true + Expect(ds.UserProps().Put(ctx, "user-1", syncFavoritesProperty, "true")).To(Succeed()) + recorder := &recordingHttpClient{} + agent.client = newClient("API_KEY", "SECRET", recorder) + + err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()}) + + Expect(err).ToNot(HaveOccurred()) + Expect(recorder.methods()).To(Equal([]string{"track.scrobble", "track.love"})) + Expect(recorder.params[1].Get("artist")).To(Equal("First Artist")) + Expect(recorder.params[1].Get("track")).To(Equal(track.Title)) + }) }) It("skips songs with less than 31 seconds", func() { @@ -481,6 +547,17 @@ var _ = Describe("lastfmAgent", func() { Expect(httpClient.SavedRequest).To(BeNil()) }) + It("does not love a short song even when it is starred", func() { + track.Duration = 29 + track.Starred = true + httpClient.Res = http.Response{Body: io.NopCloser(bytes.NewBufferString("{}")), StatusCode: 200} + + err := agent.Scrobble(ctx, "user-1", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()}) + + Expect(err).ToNot(HaveOccurred()) + Expect(httpClient.RequestCount).To(BeZero()) + }) + It("returns ErrNotAuthorized if user is not linked", func() { err := agent.Scrobble(ctx, "user-2", scrobbler.Scrobble{MediaFile: *track, TimeStamp: time.Now()}) Expect(err).To(MatchError(scrobbler.ErrNotAuthorized)) @@ -729,3 +806,39 @@ func (c *langAwareHttpClient) Do(req *http.Request) (*http.Response, error) { Body: io.NopCloser(bytes.NewBufferString(`{}`)), }, nil } + +type recordingHttpClient struct { + params []url.Values + fail map[string]bool +} + +func (c *recordingHttpClient) methods() []string { + methods := make([]string, len(c.params)) + for i, p := range c.params { + methods[i] = p.Get("method") + } + return methods +} + +func (c *recordingHttpClient) Do(req *http.Request) (*http.Response, error) { + body, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + params, err := url.ParseQuery(string(body)) + if err != nil { + return nil, err + } + c.params = append(c.params, params) + + status := http.StatusOK + payload := `{}` + if c.fail[params.Get("method")] { + status = http.StatusBadRequest + payload = `{"error":11,"message":"Service Offline - This service is temporarily offline. Try again later."}` + } + return &http.Response{ + StatusCode: status, + Body: io.NopCloser(bytes.NewBufferString(payload)), + }, nil +} diff --git a/adapters/lastfm/auth_router.go b/adapters/lastfm/auth_router.go index ec1bb9290..014796839 100644 --- a/adapters/lastfm/auth_router.go +++ b/adapters/lastfm/auth_router.go @@ -4,6 +4,7 @@ import ( "bytes" "context" _ "embed" + "encoding/json" "errors" "net/http" "time" @@ -56,6 +57,7 @@ func (s *Router) routes() http.Handler { r.Get("/link", s.getLinkStatus) r.Delete("/link", s.unlink) + r.Put("/favorites", s.setSyncFavorites) }) r.Get("/link/callback", s.callback) @@ -76,6 +78,7 @@ func (s *Router) getLinkStatus(w http.ResponseWriter, r *http.Request) { return } resp["status"] = key != "" + resp["syncFavorites"] = syncFavoritesEnabled(r.Context(), s.ds.UserProps(), u.ID) linkToken, err := createLinkToken(u.ID) if err != nil { log.Error(r.Context(), "Could not create LastFM link token", "userId", u.ID, err) @@ -86,6 +89,26 @@ func (s *Router) getLinkStatus(w http.ResponseWriter, r *http.Request) { _ = rest.RespondWithJSON(w, http.StatusOK, resp) } +func (s *Router) setSyncFavorites(w http.ResponseWriter, r *http.Request) { + u, _ := request.UserFrom(r.Context()) + var body struct { + Enabled bool `json:"enabled"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + _ = rest.RespondWithError(w, http.StatusBadRequest, "invalid request") + return + } + value := "false" + if body.Enabled { + value = "true" + } + if err := s.ds.UserProps().Put(r.Context(), u.ID, syncFavoritesProperty, value); err != nil { + _ = rest.RespondWithError(w, http.StatusInternalServerError, err.Error()) + return + } + _ = rest.RespondWithJSON(w, http.StatusOK, map[string]bool{"enabled": body.Enabled}) +} + func (s *Router) unlink(w http.ResponseWriter, r *http.Request) { u, _ := request.UserFrom(r.Context()) err := s.sessionKeys.Delete(r.Context(), u.ID) diff --git a/adapters/lastfm/auth_router_test.go b/adapters/lastfm/auth_router_test.go index 476daf443..553ed167f 100644 --- a/adapters/lastfm/auth_router_test.go +++ b/adapters/lastfm/auth_router_test.go @@ -75,6 +75,7 @@ var _ = Describe("auth_router", func() { Expect(json.Unmarshal(rec.Body.Bytes(), &body)).To(Succeed()) Expect(body["apiKey"]).To(Equal("API_KEY")) Expect(body["status"]).To(Equal(false)) + Expect(body["syncFavorites"]).To(Equal(false)) token, ok := body["linkToken"].(string) Expect(ok).To(BeTrue()) Expect(token).ToNot(BeEmpty()) @@ -85,6 +86,31 @@ var _ = Describe("auth_router", func() { }) }) + Describe("setSyncFavorites", func() { + It("saves the choice and reports it on the link status", func() { + req := httptest.NewRequest(http.MethodPut, "/favorites", bytes.NewBufferString(`{"enabled":false}`)) + ctx := request.WithUser(req.Context(), model.User{ID: victimID}) + req = req.WithContext(ctx) + rec := httptest.NewRecorder() + + router.setSyncFavorites(rec, req) + + Expect(rec.Code).To(Equal(http.StatusOK)) + stored, err := userProps.Get(GinkgoT().Context(), victimID, syncFavoritesProperty) + Expect(err).ToNot(HaveOccurred()) + Expect(stored).To(Equal("false")) + + statusReq := httptest.NewRequest(http.MethodGet, "/link", nil) + statusReq = statusReq.WithContext(request.WithUser(statusReq.Context(), model.User{ID: victimID})) + statusRec := httptest.NewRecorder() + router.getLinkStatus(statusRec, statusReq) + + var body map[string]any + Expect(json.Unmarshal(statusRec.Body.Bytes(), &body)).To(Succeed()) + Expect(body["syncFavorites"]).To(Equal(false)) + }) + }) + Describe("callback", func() { It("stores the session key under the user encoded in the signed token", func() { stubGetSessionOK("LEGIT_SESSION") diff --git a/adapters/lastfm/client.go b/adapters/lastfm/client.go index 7b2601cf9..6fdba09bb 100644 --- a/adapters/lastfm/client.go +++ b/adapters/lastfm/client.go @@ -200,6 +200,16 @@ func (c *client) scrobble(ctx context.Context, sessionKey string, info ScrobbleI return nil } +func (c *client) love(ctx context.Context, sessionKey, artist, track string) error { + params := url.Values{} + params.Add("method", "track.love") + params.Add("artist", artist) + params.Add("track", track) + params.Add("sk", sessionKey) + _, err := c.makeRequest(ctx, http.MethodPost, params, true) + return err +} + func (c *client) makeRequest(ctx context.Context, method string, params url.Values, signed bool) (*Response, error) { params.Add("format", "json") params.Add("api_key", c.apiKey) diff --git a/adapters/lastfm/client_test.go b/adapters/lastfm/client_test.go index f72c84b91..5d31460b6 100644 --- a/adapters/lastfm/client_test.go +++ b/adapters/lastfm/client_test.go @@ -259,6 +259,32 @@ var _ = Describe("client", func() { }) }) + Describe("love", func() { + It("sends parameters in request body for POST", func() { + httpClient.Res = http.Response{ + Body: io.NopCloser(bytes.NewBufferString(`{}`)), + StatusCode: 200, + } + + err := client.love(context.Background(), "SESSION_KEY", "U2", "One") + Expect(err).To(BeNil()) + + req := httpClient.SavedRequest + Expect(req.Method).To(Equal(http.MethodPost)) + Expect(req.Header.Get("Content-Type")).To(Equal("application/x-www-form-urlencoded")) + Expect(req.URL.RawQuery).To(BeEmpty()) + + body, _ := io.ReadAll(req.Body) + bodyParams, _ := url.ParseQuery(string(body)) + Expect(bodyParams.Get("method")).To(Equal("track.love")) + Expect(bodyParams.Get("artist")).To(Equal("U2")) + Expect(bodyParams.Get("track")).To(Equal("One")) + Expect(bodyParams.Get("sk")).To(Equal("SESSION_KEY")) + Expect(bodyParams.Get("api_key")).To(Equal("API_KEY")) + Expect(bodyParams.Get("api_sig")).ToNot(BeEmpty()) + }) + }) + Describe("updateNowPlaying", func() { It("sends parameters in request body for POST", func() { httpClient.Res = http.Response{ diff --git a/persistence/scrobble_buffer_repository.go b/persistence/scrobble_buffer_repository.go index 0daf2b0e7..aebbef7ec 100644 --- a/persistence/scrobble_buffer_repository.go +++ b/persistence/scrobble_buffer_repository.go @@ -63,13 +63,14 @@ func (r *scrobbleBufferRepository) Enqueue(ctx context.Context, service, userId, } func (r *scrobbleBufferRepository) Next(ctx context.Context, service string, userId string) (*model.ScrobbleEntry, error) { - // Put `s.*` last or else m.id overrides s.id - sql := Select().Columns("m.*, s.*"). + // `s.*` follows `m.*` so the buffer id wins. starred is selected after that. + sql := Select().Columns("m.*, s.*", "coalesce(annotation.starred, 0) as starred"). From(r.tableName+" s"). LeftJoin("media_file m on m.id = s.media_file_id"). + LeftJoin("annotation on annotation.item_id = m.id AND annotation.item_type = 'media_file' AND annotation.user_id = s.user_id"). Where(And{ Eq{"service": service}, - Eq{"user_id": userId}, + Eq{"s.user_id": userId}, }). OrderBy("play_time", "s.rowid").Limit(1) diff --git a/persistence/scrobble_buffer_repository_test.go b/persistence/scrobble_buffer_repository_test.go index 38b0e4e63..b470fecfa 100644 --- a/persistence/scrobble_buffer_repository_test.go +++ b/persistence/scrobble_buffer_repository_test.go @@ -189,6 +189,55 @@ var _ = Describe("ScrobbleBufferRepository", func() { Entry("Service with one user", "b", "2222", timeD, "1004", "2"), ) + It("returns the owner's starred flag", func() { + insertManually("a", "userid", "1001", timeA.Add(-time.Hour)) + ins := squirrel.Insert("annotation").SetMap(map[string]any{ + "user_id": "userid", + "item_id": "1001", + "item_type": "media_file", + "starred": true, + }) + _, err := rawRepo.executeSQL(ctx, ins) + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(func() { + del := squirrel.Delete("annotation").Where(squirrel.Eq{ + "user_id": "userid", "item_id": "1001", "item_type": "media_file", + }) + _, err := rawRepo.executeSQL(ctx, del) + Expect(err).ToNot(HaveOccurred()) + }) + + entry, err := scrobble.Next(ctx, "a", "userid") + Expect(err).ToNot(HaveOccurred()) + Expect(entry).ToNot(BeNil()) + Expect(entry.MediaFileID).To(Equal("1001")) + Expect(entry.Starred).To(BeTrue()) + }) + + It("ignores a star that belongs to someone else", func() { + insertManually("a", "userid", "1001", timeA.Add(-time.Hour)) + ins := squirrel.Insert("annotation").SetMap(map[string]any{ + "user_id": "2222", + "item_id": "1001", + "item_type": "media_file", + "starred": true, + }) + _, err := rawRepo.executeSQL(ctx, ins) + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(func() { + del := squirrel.Delete("annotation").Where(squirrel.Eq{ + "user_id": "2222", "item_id": "1001", "item_type": "media_file", + }) + _, err := rawRepo.executeSQL(ctx, del) + Expect(err).ToNot(HaveOccurred()) + }) + + entry, err := scrobble.Next(ctx, "a", "userid") + Expect(err).ToNot(HaveOccurred()) + Expect(entry).ToNot(BeNil()) + Expect(entry.MediaFileID).To(Equal("1001")) + Expect(entry.Starred).To(BeFalse()) + }) }) Describe("Discard", func() { diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index f694ea75f..ee6fbe669 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -635,6 +635,7 @@ "lastfmLinkFailure": "Last.fm could not be linked", "lastfmUnlinkSuccess": "Last.fm unlinked and scrobbling disabled", "lastfmUnlinkFailure": "Last.fm could not be unlinked", + "lastfmSyncFavoritesFailure": "Could not save the Last.fm favorite sync setting", "listenBrainzLinkSuccess": "ListenBrainz successfully linked and scrobbling enabled as user: %{user}", "listenBrainzLinkFailure": "ListenBrainz could not be linked: %{error}", "listenBrainzUnlinkSuccess": "ListenBrainz unlinked and scrobbling disabled", @@ -681,6 +682,7 @@ "desktop_notifications": "Desktop Notifications", "lastfmNotConfigured": "Last.fm API-Key is not configured", "lastfmScrobbling": "Scrobble to Last.fm", + "lastfmSyncFavorites": "Sync favorites to Last.fm", "listenBrainzScrobbling": "Scrobble to ListenBrainz", "replaygain": "ReplayGain Mode", "preAmp": "ReplayGain PreAmp (dB)", diff --git a/ui/src/personal/LastfmScrobbleToggle.jsx b/ui/src/personal/LastfmScrobbleToggle.jsx index c8e07328f..618ff647d 100644 --- a/ui/src/personal/LastfmScrobbleToggle.jsx +++ b/ui/src/personal/LastfmScrobbleToggle.jsx @@ -65,6 +65,7 @@ export const LastfmScrobbleToggle = (props) => { const [linked, setLinked] = useState(null) const [checkingLink, setCheckingLink] = useState(false) const [apiKey, setApiKey] = useState(false) + const [syncFavorites, setSyncFavorites] = useState(false) const openedTab = useRef() useEffect(() => { @@ -72,6 +73,7 @@ export const LastfmScrobbleToggle = (props) => { .then((response) => { setLinked(response.json.status === true) setApiKey(response.json.apiKey) + setSyncFavorites(response.json.syncFavorites === true) }) .catch(() => { setLinked(false) @@ -111,6 +113,16 @@ export const LastfmScrobbleToggle = (props) => { }) } + const toggleSyncFavorites = (event) => { + const next = event.target.checked + httpClient('/api/lastfm/favorites', { + method: 'PUT', + body: JSON.stringify({ enabled: next }), + }) + .then(() => setSyncFavorites(next)) + .catch(() => notify('message.lastfmSyncFavoritesFailure', 'warning')) + } + const toggleScrobble = () => { if (!linked) { startLink() @@ -147,6 +159,21 @@ export const LastfmScrobbleToggle = (props) => { openedTab={openedTab} /> )} + + } + label={ + {translate('menu.personal.options.lastfmSyncFavorites')} + } + /> {!apiKey && ( {translate('menu.personal.options.lastfmNotConfigured')}