From aa453e4dc28893a3164042189b098381ada4e61b Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Mon, 13 Nov 2023 11:42:09 +0100 Subject: [PATCH] ListenBrainz: Fix love import and rate limit check --- backends/listenbrainz/client.go | 14 +++++-- backends/listenbrainz/listenbrainz.go | 29 ++++++++++----- backends/listenbrainz/listenbrainz_test.go | 19 +++++++++- backends/listenbrainz/models.go | 43 ++++++++++++---------- backends/listenbrainz/models_test.go | 14 +++++++ cmd/listens.go | 7 ++++ cmd/loves.go | 7 ++++ models/interfaces.go | 1 + 8 files changed, 101 insertions(+), 33 deletions(-) diff --git a/backends/listenbrainz/client.go b/backends/listenbrainz/client.go index adbcfa6..9611421 100644 --- a/backends/listenbrainz/client.go +++ b/backends/listenbrainz/client.go @@ -46,6 +46,7 @@ func NewClient(token string) Client { client.SetAuthScheme("Token") client.SetAuthToken(token) client.SetHeader("Accept", "application/json") + client.SetHeader("Content-Type", "application/json") // Handle rate limiting (see https://listenbrainz.readthedocs.io/en/latest/users/api/index.html#rate-limiting) client.SetRetryCount(5) @@ -54,6 +55,7 @@ func NewClient(token string) Client { return r.StatusCode() == http.StatusTooManyRequests }, ) + client.SetRetryMaxWaitTime(time.Duration(1 * time.Minute)) client.SetRetryAfter(func(client *resty.Client, resp *resty.Response) (time.Duration, error) { resetIn, err := strconv.Atoi(resp.Header().Get("X-RateLimit-Reset-In")) // fmt.Printf("R %v: %v, %v\n", resp.Request.URL, resetIn, err) @@ -68,6 +70,7 @@ func NewClient(token string) Client { func (c Client) GetListens(user string, maxTime time.Time, minTime time.Time) (result GetListensResult, err error) { const path = "/user/{username}/listens" + errorResult := ErrorResult{} response, err := c.HttpClient.R(). SetPathParam("username", user). SetQueryParams(map[string]string{ @@ -76,10 +79,11 @@ func (c Client) GetListens(user string, maxTime time.Time, minTime time.Time) (r "count": strconv.Itoa(c.MaxResults), }). SetResult(&result). + SetError(&errorResult). Get(path) if response.StatusCode() != 200 { - err = errors.New(response.String()) + err = errors.New(errorResult.Error) return } return @@ -87,6 +91,7 @@ func (c Client) GetListens(user string, maxTime time.Time, minTime time.Time) (r func (c Client) GetFeedback(user string, status int, offset int) (result GetFeedbackResult, err error) { const path = "/feedback/user/{username}/get-feedback" + errorResult := ErrorResult{} response, err := c.HttpClient.R(). SetPathParam("username", user). SetQueryParams(map[string]string{ @@ -96,10 +101,11 @@ func (c Client) GetFeedback(user string, status int, offset int) (result GetFeed "metadata": "true", }). SetResult(&result). + SetError(&errorResult). Get(path) if response.StatusCode() != 200 { - err = errors.New(response.String()) + err = errors.New(errorResult.Error) return } return @@ -107,13 +113,15 @@ func (c Client) GetFeedback(user string, status int, offset int) (result GetFeed func (c Client) SendFeedback(feedback Feedback) (result StatusResult, err error) { const path = "/feedback/recording-feedback" + errorResult := ErrorResult{} response, err := c.HttpClient.R(). SetBody(feedback). SetResult(&result). + SetError(&errorResult). Post(path) if response.StatusCode() != 200 { - err = errors.New(response.String()) + err = errors.New(errorResult.Error) return } return diff --git a/backends/listenbrainz/listenbrainz.go b/backends/listenbrainz/listenbrainz.go index d38bc67..4baa676 100644 --- a/backends/listenbrainz/listenbrainz.go +++ b/backends/listenbrainz/listenbrainz.go @@ -22,6 +22,7 @@ THE SOFTWARE. package listenbrainz import ( + "fmt" "slices" "time" @@ -113,6 +114,7 @@ func (b ListenBrainzApiBackend) ImportLoves(loves []models.Love, oldestTimestamp TotalCount: len(loves), ImportCount: 0, LastTimestamp: oldestTimestamp, + ImportErrors: make([]string, 0), } for _, love := range loves { if love.Created.Unix() <= oldestTimestamp.Unix() { @@ -121,16 +123,20 @@ func (b ListenBrainzApiBackend) ImportLoves(loves []models.Love, oldestTimestamp // TODO: Support love import without recording MBID if love.RecordingMbid != "" { - _, err := b.client.SendFeedback(Feedback{ + resp, err := b.client.SendFeedback(Feedback{ RecordingMbid: string(love.RecordingMbid), Score: 1, }) - if err != nil { + if err == nil && resp.Status == "ok" { result.ImportCount += 1 if love.Created.Unix() > result.LastTimestamp.Unix() { result.LastTimestamp = love.Created } + } else { + msg := fmt.Sprintf("Failed import of \"%s\" by %s: %v", + love.TrackName, love.ArtistName(), err.Error()) + result.ImportErrors = append(result.ImportErrors, msg) } } } @@ -166,17 +172,22 @@ func (f Feedback) ToLove() models.Love { RecordingMbid: recordingMbid, Created: time.Unix(f.Created, 0), Track: models.Track{ - TrackName: track.TrackName, - ReleaseName: track.ReleaseName, - ArtistNames: []string{track.ArtistName}, RecordingMbid: recordingMbid, - ReleaseMbid: models.MBID(track.MbidMapping.ReleaseMbid), - ArtistMbids: make([]models.MBID, 0, len(track.MbidMapping.ArtistMbids)), }, } - for _, artistMbid := range track.MbidMapping.ArtistMbids { - love.Track.ArtistMbids = append(love.Track.ArtistMbids, models.MBID(artistMbid)) + if track != nil { + love.Track.TrackName = track.TrackName + love.Track.ReleaseName = track.ReleaseName + love.ArtistNames = []string{track.ArtistName} + love.ReleaseMbid = models.MBID(track.MbidMapping.ReleaseMbid) + love.ArtistMbids = make([]models.MBID, 0, len(track.MbidMapping.ArtistMbids)) + + if track.MbidMapping != nil { + for _, artistMbid := range track.MbidMapping.ArtistMbids { + love.Track.ArtistMbids = append(love.Track.ArtistMbids, models.MBID(artistMbid)) + } + } } return love diff --git a/backends/listenbrainz/listenbrainz_test.go b/backends/listenbrainz/listenbrainz_test.go index f2f8bd3..66c3ab2 100644 --- a/backends/listenbrainz/listenbrainz_test.go +++ b/backends/listenbrainz/listenbrainz_test.go @@ -82,11 +82,11 @@ func TestListenBrainzFeedbackToLove(t *testing.T) { RecordingMbid: recordingMbid, Score: 1, UserName: "ousidecontext", - TrackMetadata: listenbrainz.Track{ + TrackMetadata: &listenbrainz.Track{ TrackName: "Oweynagat", ArtistName: "Dool", ReleaseName: "Here Now, There Then", - MbidMapping: listenbrainz.MbidMapping{ + MbidMapping: &listenbrainz.MbidMapping{ RecordingMbid: recordingMbid, ReleaseMbid: releaseMbid, ArtistMbids: []string{artistMbid}, @@ -106,3 +106,18 @@ func TestListenBrainzFeedbackToLove(t *testing.T) { require.Len(t, love.Track.ArtistMbids, 1) assert.Equal(models.MBID(artistMbid), love.Track.ArtistMbids[0]) } + +func TestListenBrainzPartialFeedbackToLove(t *testing.T) { + recordingMbid := "c0a1fc94-5f04-4a5f-bc09-e5de0c49cd12" + feedback := listenbrainz.Feedback{ + Created: 1699859066, + RecordingMbid: recordingMbid, + Score: 1, + } + love := feedback.ToLove() + assert := assert.New(t) + assert.Equal(time.Unix(1699859066, 0).Unix(), love.Created.Unix()) + assert.Equal(models.MBID(recordingMbid), love.RecordingMbid) + assert.Equal(models.MBID(recordingMbid), love.Track.RecordingMbid) + assert.Empty(love.Track.TrackName) +} diff --git a/backends/listenbrainz/models.go b/backends/listenbrainz/models.go index 0b8bc89..8b4f44c 100644 --- a/backends/listenbrainz/models.go +++ b/backends/listenbrainz/models.go @@ -48,25 +48,25 @@ type Listen struct { } type Track struct { - TrackName string `json:"track_name"` - ArtistName string `json:"artist_name"` - ReleaseName string `json:"release_name"` - AdditionalInfo map[string]any `json:"additional_info"` - MbidMapping MbidMapping `json:"mbid_mapping"` + TrackName string `json:"track_name,omitempty"` + ArtistName string `json:"artist_name,omitempty"` + ReleaseName string `json:"release_name,omitempty"` + AdditionalInfo map[string]any `json:"additional_info,omitempty"` + MbidMapping *MbidMapping `json:"mbid_mapping,omitempty"` } type MbidMapping struct { - RecordingName string `json:"recording_name"` - RecordingMbid string `json:"recording_mbid"` - ReleaseMbid string `json:"release_mbid"` - ArtistMbids []string `json:"artist_mbids"` - Artists []Artist `json:"artists"` + RecordingName string `json:"recording_name,omitempty"` + RecordingMbid string `json:"recording_mbid,omitempty"` + ReleaseMbid string `json:"release_mbid,omitempty"` + ArtistMbids []string `json:"artist_mbids,omitempty"` + Artists []Artist `json:"artists,omitempty"` } type Artist struct { - ArtistCreditName string `json:"artist_credit_name"` - ArtistMbid string `json:"artist_mbid"` - JoinPhrase string `json:"join_phrase"` + ArtistCreditName string `json:"artist_credit_name,omitempty"` + ArtistMbid string `json:"artist_mbid,omitempty"` + JoinPhrase string `json:"join_phrase,omitempty"` } type GetFeedbackResult struct { @@ -77,18 +77,23 @@ type GetFeedbackResult struct { } type Feedback struct { - Created int64 `json:"created"` - RecordingMbid string `json:"recording_mbid"` - RecordingMsid string `json:"recording_msid"` - Score int `json:"score"` - TrackMetadata Track `json:"track_metadata"` - UserName string `json:"user_id"` + Created int64 `json:"created,omitempty"` + RecordingMbid string `json:"recording_mbid,omitempty"` + RecordingMsid string `json:"recording_msid,omitempty"` + Score int `json:"score,omitempty"` + TrackMetadata *Track `json:"track_metadata,omitempty"` + UserName string `json:"user_id,omitempty"` } type StatusResult struct { Status string `json:"status"` } +type ErrorResult struct { + Code int `json:"code"` + Error string `json:"error"` +} + func (t Track) Duration() time.Duration { info := t.AdditionalInfo millisecondsF, ok := tryGetFloat[float64](info, "duration_ms") diff --git a/backends/listenbrainz/models_test.go b/backends/listenbrainz/models_test.go index ac212f2..8f75eb7 100644 --- a/backends/listenbrainz/models_test.go +++ b/backends/listenbrainz/models_test.go @@ -22,10 +22,12 @@ THE SOFTWARE. package listenbrainz_test import ( + "encoding/json" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uploadedlobster.com/scotty/backends/listenbrainz" ) @@ -157,3 +159,15 @@ func TestReleaseGroupMbid(t *testing.T) { } assert.Equal(t, expected, track.ReleaseGroupMbid()) } + +func TestMarshalPartialFeedback(t *testing.T) { + feedback := listenbrainz.Feedback{ + Created: 1699859066, + RecordingMbid: "c0a1fc94-5f04-4a5f-bc09-e5de0c49cd12", + } + b, err := json.Marshal(feedback) + require.NoError(t, err) + assert.Equal(t, + "{\"created\":1699859066,\"recording_mbid\":\"c0a1fc94-5f04-4a5f-bc09-e5de0c49cd12\"}", + string(b)) +} diff --git a/cmd/listens.go b/cmd/listens.go index f8f8000..ef08ade 100644 --- a/cmd/listens.go +++ b/cmd/listens.go @@ -50,6 +50,13 @@ var listensCmd = &cobra.Command{ cobra.CheckErr(err) fmt.Printf("Imported %v of %v listens (last timestamp %v)\n", result.ImportCount, result.TotalCount, result.LastTimestamp) + + if len(result.ImportErrors) > 0 { + fmt.Printf("\nDuring the import the following errors occurred:\n") + for _, err := range result.ImportErrors { + fmt.Printf("Error: %v\n", err) + } + } }, } diff --git a/cmd/loves.go b/cmd/loves.go index e6c16cd..406f414 100644 --- a/cmd/loves.go +++ b/cmd/loves.go @@ -50,6 +50,13 @@ var lovesCmd = &cobra.Command{ cobra.CheckErr(err) fmt.Printf("Imported %v of %v loves (last timestamp %v)\n", result.ImportCount, result.TotalCount, result.LastTimestamp) + + if len(result.ImportErrors) > 0 { + fmt.Printf("\nDuring the import the following errors occurred:\n") + for _, err := range result.ImportErrors { + fmt.Printf("Error: %v\n", err) + } + } }, } diff --git a/models/interfaces.go b/models/interfaces.go index 1e7063b..9ebecea 100644 --- a/models/interfaces.go +++ b/models/interfaces.go @@ -51,4 +51,5 @@ type ImportResult struct { TotalCount int ImportCount int LastTimestamp time.Time + ImportErrors []string }