diff --git a/internal/app/app.go b/internal/app/app.go index c43614e..b8358f9 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -1289,11 +1289,12 @@ func buildTagMetadata(trackMeta map[string]any, title, source, trackID string, o if discTotal == 0 { discTotal = jsonutil.IntFromAny(trackMeta["numberOfVolumes"]) } - if discTotal == 0 && opts.albumDiscTotal > 0 { + if !opts.forPlaylist && discTotal == 0 && opts.albumDiscTotal > 0 { discTotal = opts.albumDiscTotal } if opts.forPlaylist { - discTotal = 1 + discNumber = 0 + discTotal = 0 } if !opts.forPlaylist && discNumber == 0 { discNumber = 1 diff --git a/internal/app/app_test.go b/internal/app/app_test.go index 86aed23..95c3681 100644 --- a/internal/app/app_test.go +++ b/internal/app/app_test.go @@ -490,6 +490,43 @@ func TestBuildTagMetadataUsesAlbumArtistOverride(t *testing.T) { } } +func TestBuildTagMetadataPlaylistOmitsDiscTags(t *testing.T) { + meta := map[string]any{ + "title": "One Step Too Far", + "track_number": float64(15), + "media_number": float64(2), + "numberOfVolumes": float64(2), + "numberOfTracks": float64(18), + "performer": map[string]any{"name": "Faithless"}, + "artist": map[string]any{"name": "Faithless"}, + "release_date": "2005-01-01", + "release_date_original": "2005-01-01", + "album": map[string]any{ + "id": "23324600", + "title": "Greatest Hits (Deluxe)", + "artist": map[string]any{"name": "Faithless"}, + }, + } + playlistCfg := config.DefaultConfigData().Metadata + applyPlaylistMetadataOverrides(meta, playlistCfg, "Road Trip", 3) + tags := buildTagMetadata(meta, "One Step Too Far", "tidal", "23324615", ripTrackOptions{forPlaylist: true, playlistName: "Road Trip", playlistPos: 3, total: 20}) + if tags.Album != "Road Trip" { + t.Fatalf("album = %q, want Road Trip", tags.Album) + } + if tags.TrackNumber != 3 { + t.Fatalf("track number = %d, want 3", tags.TrackNumber) + } + if tags.TrackTotal != 20 { + t.Fatalf("track total = %d, want 20", tags.TrackTotal) + } + if tags.DiscNumber != 0 { + t.Fatalf("disc number = %d, want 0", tags.DiscNumber) + } + if tags.DiscTotal != 0 { + t.Fatalf("disc total = %d, want 0", tags.DiscTotal) + } +} + func TestTrackOutputPathFallsBackToDisc1(t *testing.T) { tmp := t.TempDir() d := config.DefaultConfigData() diff --git a/internal/provider/tidal/client.go b/internal/provider/tidal/client.go index a49c180..eda8626 100644 --- a/internal/provider/tidal/client.go +++ b/internal/provider/tidal/client.go @@ -22,12 +22,13 @@ import ( ) const ( - baseURL = "https://api.tidalhifi.com/v1" - lyricsAPIv1 = "https://api.tidal.com/v1" - openAPIV2 = "https://openapi.tidal.com/v2" - authURL = "https://auth.tidal.com/v1/oauth2" - clientID = "fX2JxdmntZWK0ixT" - clientSec = "1Nm5AfDAjxrgJFJbKNWLeAyKGVGmINuXPPLHVXAvxAg=" + baseURL = "https://api.tidalhifi.com/v1" + lyricsAPIv1 = "https://api.tidal.com/v1" + openAPIV2 = "https://openapi.tidal.com/v2" + authURL = "https://auth.tidal.com/v1/oauth2" + clientID = "fX2JxdmntZWK0ixT" + clientSec = "1Nm5AfDAjxrgJFJbKNWLeAyKGVGmINuXPPLHVXAvxAg=" + tidalRequestAttempts = 3 ) var qualityMap = map[int]string{ @@ -843,11 +844,111 @@ func resolvePlaylistURL(baseRaw, refRaw string) string { return baseURL.ResolveReference(refURL).String() } -func (c *Client) apiRequest(ctx context.Context, path string, params url.Values, base string) (map[string]any, int, error) { - if err := c.limiter.Wait(ctx); err != nil { - return nil, 0, err - } +func shouldRetryStatus(status int) bool { + return status == http.StatusTooManyRequests || status >= http.StatusInternalServerError +} +func retryDelay(retryAfter string, attempt int) time.Duration { + retryAfter = strings.TrimSpace(retryAfter) + if retryAfter != "" { + if seconds, err := strconv.Atoi(retryAfter); err == nil && seconds >= 0 { + return time.Duration(seconds) * time.Second + } + if when, err := http.ParseTime(retryAfter); err == nil { + if delay := time.Until(when); delay > 0 { + return delay + } + return 0 + } + } + return time.Duration(attempt+1) * 500 * time.Millisecond +} + +func waitRetry(ctx context.Context, delay time.Duration) error { + if delay <= 0 { + return nil + } + timer := time.NewTimer(delay) + defer timer.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-timer.C: + return nil + } +} + +func parseAPIResponseBody(body []byte, status int) (map[string]any, error) { + out := map[string]any{} + if len(body) == 0 { + return out, nil + } + if err := json.Unmarshal(body, &out); err != nil { + if status < http.StatusOK || status >= http.StatusMultipleChoices { + if raw := strings.TrimSpace(string(body)); raw != "" { + out["raw"] = raw + } + return out, nil + } + return nil, err + } + return out, nil +} + +func readAPIResponse(resp *http.Response) (map[string]any, int, string, error) { + defer func() { _ = resp.Body.Close() }() + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, resp.StatusCode, resp.Header.Get("Retry-After"), err + } + parsed, err := parseAPIResponseBody(body, resp.StatusCode) + return parsed, resp.StatusCode, resp.Header.Get("Retry-After"), err +} + +func (c *Client) doJSONWithRetry(ctx context.Context, newRequest func() (*http.Request, error)) (map[string]any, int, error) { + var lastStatus int + for attempt := 0; attempt < tidalRequestAttempts; attempt++ { + if err := c.limiter.Wait(ctx); err != nil { + return nil, 0, err + } + req, err := newRequest() + if err != nil { + return nil, 0, err + } + resp, err := c.http.Do(req) + if err != nil { + if attempt+1 < tidalRequestAttempts { + if waitErr := waitRetry(ctx, retryDelay("", attempt)); waitErr != nil { + return nil, 0, waitErr + } + continue + } + return nil, 0, err + } + + parsed, status, retryAfter, err := readAPIResponse(resp) + lastStatus = status + if err != nil { + if attempt+1 < tidalRequestAttempts { + if waitErr := waitRetry(ctx, retryDelay(retryAfter, attempt)); waitErr != nil { + return nil, 0, waitErr + } + continue + } + return nil, status, err + } + if shouldRetryStatus(status) && attempt+1 < tidalRequestAttempts { + if waitErr := waitRetry(ctx, retryDelay(retryAfter, attempt)); waitErr != nil { + return nil, 0, waitErr + } + continue + } + return parsed, status, nil + } + return map[string]any{}, lastStatus, nil +} + +func (c *Client) apiRequest(ctx context.Context, path string, params url.Values, base string) (map[string]any, int, error) { if params == nil { params = url.Values{} } @@ -863,65 +964,31 @@ func (c *Client) apiRequest(ctx context.Context, path string, params url.Values, reqURL += "?" + params.Encode() } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) - if err != nil { - return nil, 0, err - } - req.Header.Set("Authorization", "Bearer "+c.cfg.Session.Tidal.AccessToken) - req.Header.Set("User-Agent", "streamrip-go/0.1") - - resp, err := c.http.Do(req) - if err != nil { - return nil, 0, err - } - defer func() { _ = resp.Body.Close() }() - - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, resp.StatusCode, err - } - parsed := map[string]any{} - if len(body) > 0 { - if err = json.Unmarshal(body, &parsed); err != nil { - return nil, resp.StatusCode, err + return c.doJSONWithRetry(ctx, func() (*http.Request, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, reqURL, nil) + if err != nil { + return nil, err } - } - - return parsed, resp.StatusCode, nil + req.Header.Set("Authorization", "Bearer "+c.cfg.Session.Tidal.AccessToken) + req.Header.Set("User-Agent", "streamrip-go/0.1") + return req, nil + }) } func (c *Client) apiPost(ctx context.Context, endpoint string, form url.Values, basicAuth bool) (map[string]any, int, error) { - if err := c.limiter.Wait(ctx); err != nil { - return nil, 0, err - } - - req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewBufferString(form.Encode())) - if err != nil { - return nil, 0, err - } - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("User-Agent", "streamrip-go/0.1") - if basicAuth { - auth := base64.StdEncoding.EncodeToString([]byte(clientID + ":" + clientSec)) - req.Header.Set("Authorization", "Basic "+auth) - } - - resp, err := c.http.Do(req) - if err != nil { - return nil, 0, err - } - defer func() { _ = resp.Body.Close() }() - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, resp.StatusCode, err - } - out := map[string]any{} - if len(body) > 0 { - if err = json.Unmarshal(body, &out); err != nil { - return nil, resp.StatusCode, err + return c.doJSONWithRetry(ctx, func() (*http.Request, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewBufferString(form.Encode())) + if err != nil { + return nil, err } - } - return out, resp.StatusCode, nil + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("User-Agent", "streamrip-go/0.1") + if basicAuth { + auth := base64.StdEncoding.EncodeToString([]byte(clientID + ":" + clientSec)) + req.Header.Set("Authorization", "Basic "+auth) + } + return req, nil + }) } func stringify(v any) string { diff --git a/internal/provider/tidal/client_test.go b/internal/provider/tidal/client_test.go index 9a86892..d3f94b2 100644 --- a/internal/provider/tidal/client_test.go +++ b/internal/provider/tidal/client_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "net/url" "reflect" "strconv" "testing" @@ -238,6 +239,83 @@ func TestGetMetadataTrackIgnoresLyricsEndpointFailure(t *testing.T) { } } +func TestAPIRequestRetriesTooManyRequests(t *testing.T) { + calls := 0 + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/tracks/42" { + w.WriteHeader(http.StatusNotFound) + return + } + calls++ + if calls == 1 { + w.Header().Set("Retry-After", "0") + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte("slow down")) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{"id": 42, "title": "Song"}) + })) + defer ts.Close() + + cfgData := config.DefaultConfigData() + cfgData.Downloads.RequestsPerMinute = 0 + cfgData.Tidal.AccessToken = "token" + cfgData.Tidal.CountryCode = "US" + c := New(&config.Config{File: cfgData, Session: cfgData}) + c.baseURL = ts.URL + "/v1" + + resp, status, err := c.apiRequest(context.Background(), "tracks/42", nil, c.baseURL) + if err != nil { + t.Fatalf("apiRequest() err = %v", err) + } + if status != http.StatusOK { + t.Fatalf("status = %d, want %d", status, http.StatusOK) + } + if calls != 2 { + t.Fatalf("calls = %d, want 2", calls) + } + if stringify(resp["title"]) != "Song" { + t.Fatalf("title = %q, want Song", stringify(resp["title"])) + } +} + +func TestAPIPostRetriesTooManyRequests(t *testing.T) { + calls := 0 + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/token" { + w.WriteHeader(http.StatusNotFound) + return + } + calls++ + if calls == 1 { + w.Header().Set("Retry-After", "0") + w.WriteHeader(http.StatusTooManyRequests) + _, _ = w.Write([]byte("slow down")) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{"access_token": "fresh-token"}) + })) + defer ts.Close() + + cfgData := config.DefaultConfigData() + cfgData.Downloads.RequestsPerMinute = 0 + c := New(&config.Config{File: cfgData, Session: cfgData}) + + resp, status, err := c.apiPost(context.Background(), ts.URL+"/token", url.Values{"grant_type": []string{"refresh_token"}}, false) + if err != nil { + t.Fatalf("apiPost() err = %v", err) + } + if status != http.StatusOK { + t.Fatalf("status = %d, want %d", status, http.StatusOK) + } + if calls != 2 { + t.Fatalf("calls = %d, want 2", calls) + } + if stringify(resp["access_token"]) != "fresh-token" { + t.Fatalf("access_token = %q, want fresh-token", stringify(resp["access_token"])) + } +} + func TestGetDownloadablePrefersAtmosWhenEnabled(t *testing.T) { var calls []string allImmersive := true