Merge f6c6c732eda49d20b35a2ce5868a58c42dfb6a5d into be201a2d7972d9580bef5533bfcb8bc49a2675d2

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 {

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

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)

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")

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)

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{

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)
 

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() {

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)",

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}
         />
       )}
+      <FormControlLabel
+        disabled={!apiKey || linked !== true || checkingLink}
+        control={
+          <Switch
+            id={'lastfm-sync-favorites'}
+            color="primary"
+            checked={syncFavorites}
+            disabled={!apiKey || linked !== true || checkingLink}
+            onChange={toggleSyncFavorites}
+          />
+        }
+        label={
+          <span>{translate('menu.personal.options.lastfmSyncFavorites')}</span>
+        }
+      />
       {!apiKey && (
         <FormHelperText id="scrobble-lastfm-disabled-helper-text">
           {translate('menu.personal.options.lastfmNotConfigured')}