From b3b6a2eb1d7b78102c6758158a33daa0024fb4ab Mon Sep 17 00:00:00 2001 From: Joren Date: Sat, 3 Oct 2026 17:05:55 +0200 Subject: [PATCH] refactor: delete dead code, dedupe helpers, add tests for untested packages Dead code Removed, each verified to have no non-test caller: promptSearchSelection (77 lines, superseded by promptSearchSelectionMenu), AddPlaylistByTrackIDs, download.NewWithVerifySSL, qobuz.Client.GetTrackMetadata, tidal.Client.trackSupportsAtmos (test-only, and the comment above GetDownloadable already documented its removal from the live path) and its now-orphaned test, yandex.canonicalTrackRequestID (a no-op returning its trimmed input; the call site uses strings.TrimSpace directly) and the unused defaultEstimatedKbps constant. Config keys that did nothing qobuz.download_booklets and misc.check_for_updates were declared, defaulted and documented in config.toml.example but read by no code. The whole [youtube] block went too: there is no YouTube provider. go-toml ignores unknown keys, verified, so existing user configs still load. Duplicated helpers tidal.stringify and yandex.firstNonEmpty were byte-identical copies of jsonutil.StringFromAny and jsonutil.FirstNonEmpty. displayArtistNames existed three times, byte-identical, in internal/app, provider/yandex and provider/beatport; it now lives in jsonutil. The CLI's normalizeCodec delegates to config.NormalizeCodec so the flag and the config file are checked by the same rules. Tests for packages that had none internal/jsonutil (0% -> 100%) and internal/netutil (0% -> 78%), the latter covering redactURL, which is a security control, and the zero-timeout contract that internal/download and the qobuz body copy depend on. internal/config rose to 82% with coverage of the new Validate(), the 0600 permissions, CloneConfigData not sharing slices, and a concurrent UpdateTokens test that exercises the mutex under -race. Test suite is ~85x faster Provider tests built clients with the default 60 requests/minute, so the rate limiter slept 1s per HTTP call: deezer took 31s, tidal 11s, qobuz 8s. Each provider test package now builds its config through a testConfigData helper that disables throttling. Whole suite: ~60s -> 0.7s, and 2.8s under -race. Co-Authored-By: Claude Opus 5 --- cmd/rip/search.go | 78 -------- config.toml.example | 13 -- internal/app/app.go | 58 +----- internal/config/config.go | 39 ++-- internal/config/config_test.go | 187 ++++++++++++++++++++ internal/download/downloader.go | 4 - internal/jsonutil/jsonutil.go | 16 ++ internal/jsonutil/jsonutil_test.go | 184 +++++++++++++++++++ internal/netutil/http_test.go | 148 ++++++++++++++++ internal/provider/beatport/client.go | 21 +-- internal/provider/beatport/client_test.go | 19 +- internal/provider/deezer/client_test.go | 48 +++-- internal/provider/qobuz/client.go | 8 - internal/provider/qobuz/client_test.go | 18 +- internal/provider/soundcloud/client_test.go | 22 ++- internal/provider/tidal/client.go | 91 +++------- internal/provider/tidal/client_test.go | 65 +++---- internal/provider/yandex/client.go | 53 ++---- internal/provider/yandex/client_test.go | 21 ++- 19 files changed, 701 insertions(+), 392 deletions(-) create mode 100644 internal/jsonutil/jsonutil_test.go create mode 100644 internal/netutil/http_test.go diff --git a/cmd/rip/search.go b/cmd/rip/search.go index 9744baf..93dc49f 100644 --- a/cmd/rip/search.go +++ b/cmd/rip/search.go @@ -110,84 +110,6 @@ func parseSearchArgs(args []string, defaultLimit int) (searchOptions, error) { }, nil } -func promptSearchSelection(results []searchResult) ([]int, error) { - reader := bufio.NewReader(os.Stdin) - for { - fmt.Print("Select results to download (e.g. 1,3-5; a=all; q=cancel): ") - line, err := reader.ReadString('\n') - if err != nil { - return nil, err - } - line = strings.TrimSpace(line) - if line == "" || strings.EqualFold(line, "q") || strings.EqualFold(line, "quit") { - return nil, nil - } - if strings.EqualFold(line, "a") || strings.EqualFold(line, "all") { - out := make([]int, 0, len(results)) - for i := range results { - out = append(out, i) - } - return out, nil - } - - selected := map[int]struct{}{} - chunks := strings.Split(line, ",") - ok := true - for _, raw := range chunks { - part := strings.TrimSpace(raw) - if part == "" { - continue - } - if strings.Contains(part, "-") { - bounds := strings.SplitN(part, "-", 2) - if len(bounds) != 2 { - ok = false - break - } - start, err1 := strconv.Atoi(strings.TrimSpace(bounds[0])) - end, err2 := strconv.Atoi(strings.TrimSpace(bounds[1])) - if err1 != nil || err2 != nil || start <= 0 || end <= 0 || start > end { - ok = false - break - } - for i := start; i <= end; i++ { - if i > len(results) { - ok = false - break - } - selected[i-1] = struct{}{} - } - if !ok { - break - } - continue - } - idx, err := strconv.Atoi(part) - if err != nil || idx <= 0 || idx > len(results) { - ok = false - break - } - selected[idx-1] = struct{}{} - } - - if !ok || len(selected) == 0 { - fmt.Println("Invalid selection, try again.") - continue - } - - out := make([]int, 0, len(selected)) - for idx := range selected { - out = append(out, idx) - } - for i := 1; i < len(out); i++ { - for j := i; j > 0 && out[j] < out[j-1]; j-- { - out[j], out[j-1] = out[j-1], out[j] - } - } - return out, nil - } -} - func promptSearchSelectionMenu(source, mediaType, query string, results []searchResult) ([]int, error) { if len(results) == 0 { return nil, nil diff --git a/config.toml.example b/config.toml.example index d3857d7..69f0c32 100644 --- a/config.toml.example +++ b/config.toml.example @@ -21,9 +21,6 @@ verify_ssl = true # Quality ladder (global scale is 0-4, Qobuz supports 1-4): # 1 = MP3 320, 2 = FLAC 16/44.1, 3 = FLAC 24-bit <=96kHz, 4 = FLAC 24-bit <=192kHz quality = 3 -# Download booklet PDFs when available -download_booklets = true - # Authenticate using Qobuz auth token instead of email/password hash use_auth_token = false # If use_auth_token=true, set your user id. Otherwise set your email. @@ -101,14 +98,6 @@ quality = 0 client_id = "" app_version = "" -[youtube] -# Only 0 is currently supported -quality = 0 -# Download video streams together with audio when supported -download_videos = false -# Folder used for video outputs -video_downloads_folder = "/path/to/StreamripDownloads/YouTubeVideos" - [database] # Track IDs already downloaded are stored here and skipped next time downloads_enabled = true @@ -189,5 +178,3 @@ max_search_results = 100 [misc] # Metadata used for config compatibility checks version = "2.2.0" -# Notify when a new version is available -check_for_updates = true diff --git a/internal/app/app.go b/internal/app/app.go index 03a2c37..aff3efe 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -298,47 +298,6 @@ func (m *Main) AddByID(ctx context.Context, source, mediaType, id string) error return nil } -func (m *Main) AddPlaylistByTrackIDs(ctx context.Context, source, playlistID, playlistName string, trackIDs []string) error { - p, err := m.GetLoggedInProvider(ctx, source) - if err != nil { - return err - } - if strings.TrimSpace(playlistName) == "" { - playlistName = playlistID - } - ids := make([]string, 0, len(trackIDs)) - for _, id := range trackIDs { - id = strings.TrimSpace(id) - if id != "" { - ids = append(ids, id) - } - } - if len(ids) == 0 { - return fmt.Errorf("playlist %q has no track ids", playlistName) - } - - pending := media.PendingFunc{ - ResolveFn: func(context.Context) (media.Media, error) { - metaItems := make([]any, 0, len(ids)) - for _, id := range ids { - metaItems = append(metaItems, map[string]any{"id": id}) - } - playlistMeta := map[string]any{ - "name": playlistName, - "tracks": map[string]any{"items": metaItems}, - } - return media.MediaFunc{RipFn: func(ctx context.Context) error { - if m.Config.Session.CLI.TextOutput { - m.logf("Downloading playlist: %s\n", playlistName) - } - return m.ripPlaylist(ctx, p, source, playlistID, playlistMeta) - }}, nil - }, - } - m.Pending = append(m.Pending, pending) - return nil -} - func (m *Main) AddMixedPlaylistByTrackRefs(ctx context.Context, playlistID, playlistName string, refs []PlaylistTrackRef) error { if strings.TrimSpace(playlistName) == "" { playlistName = playlistID @@ -1728,11 +1687,11 @@ func extractAlbumArtist(albumMeta map[string]any) string { } } if len(names) > 0 { - return displayArtistNames(names) + return jsonutil.DisplayArtistNames(names) } } if names := stringSliceFromAny(albumMeta["artist_names"]); len(names) > 0 { - return displayArtistNames(names) + return jsonutil.DisplayArtistNames(names) } artist := jsonutil.NestedString(albumMeta, "artist", "name") if artist != "" { @@ -1740,16 +1699,3 @@ func extractAlbumArtist(albumMeta map[string]any) string { } return "Unknown" } - -func displayArtistNames(names []string) string { - switch len(names) { - case 0: - return "" - case 1: - return names[0] - case 2: - return names[0] + " & " + names[1] - default: - return strings.Join(names[:len(names)-1], ", ") + " & " + names[len(names)-1] - } -} diff --git a/internal/config/config.go b/internal/config/config.go index 037200a..cd93558 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -38,7 +38,6 @@ type ConfigData struct { Yandex YandexConfig `toml:"yandex"` Beatport BeatportConfig `toml:"beatport"` Soundcloud SoundcloudConfig `toml:"soundcloud"` - Youtube YoutubeConfig `toml:"youtube"` Database DatabaseConfig `toml:"database"` Conversion ConversionConfig `toml:"conversion"` QobuzFilters QobuzDiscographyFilterConfig `toml:"qobuz_filters"` @@ -61,13 +60,12 @@ type DownloadsConfig struct { } type QobuzConfig struct { - Quality int `toml:"quality"` - DownloadBooklets bool `toml:"download_booklets"` - UseAuthToken bool `toml:"use_auth_token"` - EmailOrUserID string `toml:"email_or_userid"` - PasswordOrToken string `toml:"password_or_token"` - AppID string `toml:"app_id"` - Secrets []string `toml:"secrets"` + Quality int `toml:"quality"` + UseAuthToken bool `toml:"use_auth_token"` + EmailOrUserID string `toml:"email_or_userid"` + PasswordOrToken string `toml:"password_or_token"` + AppID string `toml:"app_id"` + Secrets []string `toml:"secrets"` } type TidalConfig struct { @@ -112,12 +110,6 @@ type SoundcloudConfig struct { AppVersion string `toml:"app_version"` } -type YoutubeConfig struct { - Quality int `toml:"quality"` - DownloadVideos bool `toml:"download_videos"` - VideoDownloadsFolder string `toml:"video_downloads_folder"` -} - type DatabaseConfig struct { DownloadsEnabled bool `toml:"downloads_enabled"` DownloadsPath string `toml:"downloads_path"` @@ -177,8 +169,7 @@ type CLIConfig struct { } type MiscConfig struct { - Version string `toml:"version"` - CheckForUpdates bool `toml:"check_for_updates"` + Version string `toml:"version"` } func Load(path string) (*Config, error) { @@ -366,9 +357,8 @@ func DefaultConfigData() ConfigData { VerifySSL: true, }, Qobuz: QobuzConfig{ - Quality: 3, - DownloadBooklets: true, - UseAuthToken: false, + Quality: 3, + UseAuthToken: false, }, Tidal: TidalConfig{ Quality: 3, @@ -389,11 +379,6 @@ func DefaultConfigData() ConfigData { Soundcloud: SoundcloudConfig{ Quality: 0, }, - Youtube: YoutubeConfig{ - Quality: 0, - DownloadVideos: false, - VideoDownloadsFolder: filepath.Join(downloadsFolder, "YouTubeVideos"), - }, Database: DatabaseConfig{ DownloadsEnabled: true, DownloadsPath: filepath.Join(appDir, "downloads.db"), @@ -437,8 +422,7 @@ func DefaultConfigData() ConfigData { MaxSearchResults: 100, }, Misc: MiscConfig{ - Version: CurrentConfigVersion, - CheckForUpdates: true, + Version: CurrentConfigVersion, }, } @@ -498,9 +482,6 @@ func applyRuntimeDefaults(data *ConfigData) { if data.Database.FailedDownloadsPath == "" { data.Database.FailedDownloadsPath = filepath.Join(appDir, "failed_downloads.db") } - if data.Youtube.VideoDownloadsFolder == "" { - data.Youtube.VideoDownloadsFolder = filepath.Join(data.Downloads.Folder, "YouTubeVideos") - } } // CloneConfigData returns a deep copy of in. The slice fields must not be diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 64febc7..28f4e85 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -2,8 +2,11 @@ package config import ( "errors" + "fmt" "os" "path/filepath" + "strings" + "sync" "testing" "github.com/pelletier/go-toml/v2" @@ -139,3 +142,187 @@ func TestExampleConfigIsValid(t *testing.T) { t.Errorf("config.toml.example version = %q, want %q", data.Misc.Version, CurrentConfigVersion) } } + +func TestValidateRejectsBadValues(t *testing.T) { + tests := []struct { + name string + mutate func(*ConfigData) + want string + }{ + {"qobuz quality 0", func(d *ConfigData) { d.Qobuz.Quality = 0 }, "qobuz.quality"}, + {"qobuz quality 5", func(d *ConfigData) { d.Qobuz.Quality = 5 }, "qobuz.quality"}, + {"beatport quality 0", func(d *ConfigData) { d.Beatport.Quality = 0 }, "beatport.quality"}, + {"tidal quality 5", func(d *ConfigData) { d.Tidal.Quality = 5 }, "tidal.quality"}, + {"deezer quality 3", func(d *ConfigData) { d.Deezer.Quality = 3 }, "deezer.quality"}, + {"negative quality", func(d *ConfigData) { d.Tidal.Quality = -1 }, "tidal.quality"}, + {"max_connections 0", func(d *ConfigData) { d.Downloads.MaxConnections = 0 }, "max_connections"}, + {"max_connections negative", func(d *ConfigData) { d.Downloads.MaxConnections = -5 }, "max_connections"}, + {"requests_per_minute 0", func(d *ConfigData) { d.Downloads.RequestsPerMinute = 0 }, "requests_per_minute"}, + {"truncate_to 0", func(d *ConfigData) { d.Filepaths.TruncateTo = 0 }, "truncate_to"}, + {"bogus codec", func(d *ConfigData) { + d.Conversion.Enabled = true + d.Conversion.Codec = "bogus" + }, "conversion.codec"}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + d := DefaultConfigData() + applyRuntimeDefaults(&d) + tc.mutate(&d) + err := Validate(&d) + if err == nil { + t.Fatalf("Validate() = nil, want an error mentioning %q", tc.want) + } + if !strings.Contains(err.Error(), tc.want) { + t.Fatalf("Validate() = %q, want it to mention %q", err, tc.want) + } + }) + } +} + +func TestValidateAcceptsValidValues(t *testing.T) { + d := DefaultConfigData() + applyRuntimeDefaults(&d) + // Negative requests_per_minute disables throttling, which is legitimate. + d.Downloads.RequestsPerMinute = -1 + d.Soundcloud.Quality = 1 + d.Deezer.Quality = 2 + d.Qobuz.Quality = 4 + if err := Validate(&d); err != nil { + t.Fatalf("Validate() = %v, want nil", err) + } +} + +func TestValidateNormalizesCodec(t *testing.T) { + d := DefaultConfigData() + applyRuntimeDefaults(&d) + d.Conversion.Enabled = true + d.Conversion.Codec = " ogg " + if err := Validate(&d); err != nil { + t.Fatalf("Validate() = %v", err) + } + if d.Conversion.Codec != "VORBIS" { + t.Fatalf("codec = %q, want VORBIS", d.Conversion.Codec) + } +} + +func TestNormalizeCodec(t *testing.T) { + for in, want := range map[string]string{ + "flac": "FLAC", "FLAC": "FLAC", " mp3 ": "MP3", + "ogg": "VORBIS", "vorbis": "VORBIS", "alac": "ALAC", "aac": "AAC", + } { + got, err := NormalizeCodec(in) + if err != nil { + t.Errorf("NormalizeCodec(%q) err = %v", in, err) + continue + } + if got != want { + t.Errorf("NormalizeCodec(%q) = %q, want %q", in, got, want) + } + } + if _, err := NormalizeCodec("wav"); err == nil { + t.Error("NormalizeCodec(wav) should fail") + } +} + +func TestCloneConfigDataDoesNotShareSlices(t *testing.T) { + a := DefaultConfigData() + a.Qobuz.Secrets = []string{"s1", "s2"} + a.Metadata.Exclude = []string{"e1"} + + b := CloneConfigData(a) + b.Qobuz.Secrets[0] = "mutated" + b.Metadata.Exclude[0] = "mutated" + + if a.Qobuz.Secrets[0] != "s1" { + t.Error("Qobuz.Secrets is shared between copies") + } + if a.Metadata.Exclude[0] != "e1" { + t.Error("Metadata.Exclude is shared between copies") + } +} + +func TestSaveFileUsesOwnerOnlyPermissions(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + cfg := &Config{Path: path, File: DefaultConfigData()} + if err := cfg.SaveFile(); err != nil { + t.Fatalf("SaveFile() = %v", err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + // The file holds plaintext passwords, ARL cookies and OAuth tokens. + if perm := info.Mode().Perm(); perm != 0o600 { + t.Fatalf("config permissions = %04o, want 0600", perm) + } +} + +func TestSaveFileTightensExistingWorldReadableFile(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "config.toml") + if err := os.WriteFile(path, []byte("version = \"x\"\n"), 0o644); err != nil { + t.Fatal(err) + } + cfg := &Config{Path: path, File: DefaultConfigData()} + if err := cfg.SaveFile(); err != nil { + t.Fatal(err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Fatalf("permissions = %04o, want 0600 after rewrite", perm) + } +} + +func TestUpdateTokensIsSafeWithoutAPath(t *testing.T) { + cfg := &Config{File: DefaultConfigData(), Session: DefaultConfigData()} + err := cfg.UpdateTokens(func(file, session *ConfigData) { + file.Deezer.RefreshToken = "rt" + session.Deezer.RefreshToken = "rt" + }) + if err != nil { + t.Fatalf("UpdateTokens() = %v", err) + } + // The in-memory update must still happen even with nothing to write to. + if cfg.File.Deezer.RefreshToken != "rt" || cfg.Session.Deezer.RefreshToken != "rt" { + t.Fatal("UpdateTokens did not apply the mutation") + } +} + +func TestUpdateTokensPersistsAndIsConcurrencySafe(t *testing.T) { + path := filepath.Join(t.TempDir(), "config.toml") + cfg := &Config{Path: path, File: DefaultConfigData(), Session: DefaultConfigData()} + + var wg sync.WaitGroup + for i := 0; i < 16; i++ { + wg.Add(1) + go func(n int) { + defer wg.Done() + token := fmt.Sprintf("token-%d", n) + if err := cfg.UpdateTokens(func(file, session *ConfigData) { + file.Tidal.AccessToken = token + session.Tidal.AccessToken = token + }); err != nil { + t.Errorf("UpdateTokens: %v", err) + } + }(i) + } + wg.Wait() + + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var round ConfigData + if err := toml.Unmarshal(raw, &round); err != nil { + t.Fatalf("concurrent writes produced an unparseable config: %v", err) + } + if round.Tidal.AccessToken == "" { + t.Error("no token persisted") + } +} diff --git a/internal/download/downloader.go b/internal/download/downloader.go index eadedba..3d503de 100644 --- a/internal/download/downloader.go +++ b/internal/download/downloader.go @@ -43,10 +43,6 @@ func New() *Downloader { return NewWithOptions(true, true, 0) } -func NewWithVerifySSL(verifySSL bool) *Downloader { - return NewWithOptions(verifySSL, true, 0) -} - func NewWithOptions(verifySSL bool, showProgress bool, maxConnsPerHost int) *Downloader { forceProgress := strings.EqualFold(os.Getenv("STREAMRIP_GO_FORCE_PROGRESS"), "1") || strings.EqualFold(os.Getenv("STREAMRIP_GO_FORCE_PROGRESS"), "true") interactive := showProgress && (forceProgress || (term.IsTerminal(int(os.Stderr.Fd())) && strings.ToLower(os.Getenv("TERM")) != "dumb")) diff --git a/internal/jsonutil/jsonutil.go b/internal/jsonutil/jsonutil.go index d0219ca..ddc7497 100644 --- a/internal/jsonutil/jsonutil.go +++ b/internal/jsonutil/jsonutil.go @@ -129,3 +129,19 @@ func TitleCase(s string) string { } return string(r) } + +// DisplayArtistNames joins artist names the way the providers present them: +// "A", "A & B", or "A, B & C". Previously duplicated byte-for-byte in +// internal/app, provider/yandex and provider/beatport. +func DisplayArtistNames(names []string) string { + switch len(names) { + case 0: + return "" + case 1: + return names[0] + case 2: + return names[0] + " & " + names[1] + default: + return strings.Join(names[:len(names)-1], ", ") + " & " + names[len(names)-1] + } +} diff --git a/internal/jsonutil/jsonutil_test.go b/internal/jsonutil/jsonutil_test.go new file mode 100644 index 0000000..f1cf88e --- /dev/null +++ b/internal/jsonutil/jsonutil_test.go @@ -0,0 +1,184 @@ +package jsonutil + +import "testing" + +func TestStringFromAny(t *testing.T) { + tests := []struct { + name string + in any + want string + }{ + {"string", "hi", "hi"}, + {"int", 7, "7"}, + {"int64", int64(8), "8"}, + {"float whole", float64(9), "9"}, + {"float fractional", 44.1, "44.1"}, + {"float no trailing zeroes", float64(100), "100"}, + {"nil", nil, ""}, + {"bool unsupported", true, ""}, + {"map unsupported", map[string]any{}, ""}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := StringFromAny(tc.in); got != tc.want { + t.Fatalf("StringFromAny(%#v) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +func TestIntFromAny(t *testing.T) { + tests := []struct { + in any + want int + }{ + {5, 5}, + {int32(6), 6}, + {int64(7), 7}, + {8.9, 8}, + {"10", 10}, + {" 11 ", 11}, + {"not a number", 0}, + {nil, 0}, + {true, 0}, + } + for _, tc := range tests { + if got := IntFromAny(tc.in); got != tc.want { + t.Errorf("IntFromAny(%#v) = %d, want %d", tc.in, got, tc.want) + } + } +} + +func TestFloatFromAny(t *testing.T) { + tests := []struct { + in any + want float64 + }{ + {1.5, 1.5}, + {2, 2}, + {int64(3), 3}, + {"4.5", 0}, // strings are deliberately not parsed here + {nil, 0}, + } + for _, tc := range tests { + if got := FloatFromAny(tc.in); got != tc.want { + t.Errorf("FloatFromAny(%#v) = %v, want %v", tc.in, got, tc.want) + } + } +} + +func TestBoolFromAny(t *testing.T) { + tests := []struct { + in any + want bool + }{ + {true, true}, + {false, false}, + {"true", true}, + {"TRUE", true}, + {" yes ", true}, + {"1", true}, + {"0", false}, + {"no", false}, + {1, true}, + {0, false}, + {int64(2), true}, + {0.0, false}, + {1.5, true}, + {nil, false}, + } + for _, tc := range tests { + if got := BoolFromAny(tc.in); got != tc.want { + t.Errorf("BoolFromAny(%#v) = %v, want %v", tc.in, got, tc.want) + } + } +} + +func TestFirstNonEmpty(t *testing.T) { + if got := FirstNonEmpty("", " ", "found", "later"); got != "found" { + t.Errorf("got %q", got) + } + if got := FirstNonEmpty(" trimmed "); got != "trimmed" { + t.Errorf("got %q, want trimmed", got) + } + if got := FirstNonEmpty("", " "); got != "" { + t.Errorf("got %q, want empty", got) + } + if got := FirstNonEmpty(); got != "" { + t.Errorf("got %q, want empty", got) + } +} + +func TestNestedMap(t *testing.T) { + m := map[string]any{"album": map[string]any{"title": "X"}, "n": 1} + if got := NestedMap(m, "album")["title"]; got != "X" { + t.Errorf("got %v", got) + } + if got := NestedMap(m, "missing"); len(got) != 0 { + t.Errorf("missing key should give empty map, got %v", got) + } + if got := NestedMap(m, "n"); len(got) != 0 { + t.Errorf("non-map value should give empty map, got %v", got) + } +} + +func TestNestedAnyAndString(t *testing.T) { + m := map[string]any{ + "album": map[string]any{ + "artist": map[string]any{"name": "Nina Simone", "id": float64(42)}, + }, + } + if got := NestedString(m, "album", "artist", "name"); got != "Nina Simone" { + t.Errorf("got %q", got) + } + if got := NestedString(m, "album", "artist", "id"); got != "42" { + t.Errorf("numeric id = %q, want 42", got) + } + if got := NestedString(m, "album", "nope", "name"); got != "" { + t.Errorf("missing path = %q, want empty", got) + } + if got := NestedAny(m, "album", "artist", "name"); got != "Nina Simone" { + t.Errorf("got %v", got) + } + if got := NestedAny(m); got == nil { + t.Error("no keys should return the map itself") + } + // Traversing through a non-map must not panic. + if got := NestedAny(m, "album", "artist", "name", "deeper"); got != nil { + t.Errorf("over-deep path = %v, want nil", got) + } +} + +func TestTitleCase(t *testing.T) { + tests := []struct{ in, want string }{ + {"qobuz", "Qobuz"}, + {"Tidal", "Tidal"}, + {"", ""}, + {"1st", "1st"}, + {"ä", "ä"}, // only ASCII is uppercased + } + for _, tc := range tests { + if got := TitleCase(tc.in); got != tc.want { + t.Errorf("TitleCase(%q) = %q, want %q", tc.in, got, tc.want) + } + } +} + +func TestDisplayArtistNames(t *testing.T) { + tests := []struct { + in []string + want string + }{ + {nil, ""}, + {[]string{}, ""}, + {[]string{"A"}, "A"}, + {[]string{"A", "B"}, "A & B"}, + {[]string{"A", "B", "C"}, "A, B & C"}, + {[]string{"A", "B", "C", "D"}, "A, B, C & D"}, + } + for _, tc := range tests { + if got := DisplayArtistNames(tc.in); got != tc.want { + t.Errorf("DisplayArtistNames(%v) = %q, want %q", tc.in, got, tc.want) + } + } +} diff --git a/internal/netutil/http_test.go b/internal/netutil/http_test.go new file mode 100644 index 0000000..9f06553 --- /dev/null +++ b/internal/netutil/http_test.go @@ -0,0 +1,148 @@ +package netutil + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" +) + +func TestRedactURLHidesCredentials(t *testing.T) { + tests := []struct { + name string + raw string + contains []string + absent []string + }{ + { + name: "user auth token", + raw: "https://api.example.com/track?user_auth_token=sekrit&track_id=42", + contains: []string{"user_auth_token=REDACTED", "track_id=42"}, + absent: []string{"sekrit"}, + }, + { + name: "request signature", + raw: "https://api.example.com/x?request_sig=abc123&app_id=9", + contains: []string{"request_sig=REDACTED", "app_id=9"}, + absent: []string{"abc123"}, + }, + { + name: "several at once", + raw: "https://api.example.com/x?password=p&access_token=a&refresh_token=r&id=1", + contains: []string{"password=REDACTED", "access_token=REDACTED", "refresh_token=REDACTED", "id=1"}, + absent: []string{"password=p", "access_token=a", "refresh_token=r"}, + }, + { + name: "nothing sensitive is left untouched", + raw: "https://api.example.com/track/42?limit=10", + contains: []string{"limit=10", "/track/42"}, + }, + { + name: "no query string", + raw: "https://api.example.com/track/42", + contains: []string{"https://api.example.com/track/42"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + u, err := url.Parse(tc.raw) + if err != nil { + t.Fatal(err) + } + got := redactURL(u) + for _, want := range tc.contains { + if !strings.Contains(got, want) { + t.Errorf("redactURL(%q) = %q, missing %q", tc.raw, got, want) + } + } + for _, bad := range tc.absent { + if strings.Contains(got, bad) { + t.Errorf("redactURL(%q) = %q, leaked %q", tc.raw, got, bad) + } + } + }) + } +} + +func TestRedactURLNil(t *testing.T) { + if got := redactURL(nil); got != "" { + t.Errorf("redactURL(nil) = %q, want empty", got) + } +} + +func TestIsSensitiveParam(t *testing.T) { + sensitive := []string{ + "user_auth_token", "api_token", "access_token", "refresh_token", + "request_sig", "signature", "password", "secret", "token", "code", "auth", "key", + } + for _, name := range sensitive { + if !isSensitiveParam(name) { + t.Errorf("isSensitiveParam(%q) = false, want true", name) + } + } + for _, name := range []string{"track_id", "limit", "offset", "app_id", "quality"} { + if isSensitiveParam(name) { + t.Errorf("isSensitiveParam(%q) = true, want false", name) + } + } +} + +func TestNewHTTPClientHonoursVerifySSL(t *testing.T) { + insecure := NewHTTPClient(5*time.Second, false, 4) + tr, ok := insecure.Transport.(*loggingTransport) + if !ok { + t.Fatalf("transport is %T, want *loggingTransport", insecure.Transport) + } + base, ok := tr.base.(*http.Transport) + if !ok { + t.Fatalf("base transport is %T", tr.base) + } + if !base.TLSClientConfig.InsecureSkipVerify { + t.Error("verifySSL=false should set InsecureSkipVerify") + } + + secure := NewHTTPClient(5*time.Second, true, 4) + base = secure.Transport.(*loggingTransport).base.(*http.Transport) + if base.TLSClientConfig.InsecureSkipVerify { + t.Error("verifySSL=true must not skip verification") + } + if secure.Timeout != 5*time.Second { + t.Errorf("Timeout = %v, want 5s", secure.Timeout) + } +} + +func TestNewHTTPClientZeroTimeoutMeansNoTimeout(t *testing.T) { + // internal/download and the qobuz body copy rely on this: a zero timeout + // must not be replaced with a default, or long track downloads abort. + c := NewHTTPClient(0, true, 0) + if c.Timeout != 0 { + t.Errorf("Timeout = %v, want 0", c.Timeout) + } +} + +func TestNewHTTPClientDefaultsMaxConns(t *testing.T) { + c := NewHTTPClient(time.Second, true, 0) + base := c.Transport.(*loggingTransport).base.(*http.Transport) + if base.MaxIdleConnsPerHost != defaultMaxConnsPerHost { + t.Errorf("MaxIdleConnsPerHost = %d, want %d", base.MaxIdleConnsPerHost, defaultMaxConnsPerHost) + } +} + +func TestClientPerformsRequest(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusTeapot) + })) + defer ts.Close() + + resp, err := NewHTTPClient(5*time.Second, true, 2).Get(ts.URL + "/x?token=sekrit") + if err != nil { + t.Fatal(err) + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusTeapot { + t.Errorf("status = %d", resp.StatusCode) + } +} diff --git a/internal/provider/beatport/client.go b/internal/provider/beatport/client.go index a56ac1c..abcd137 100644 --- a/internal/provider/beatport/client.go +++ b/internal/provider/beatport/client.go @@ -546,7 +546,7 @@ func (c *Client) normalizeRelease(raw map[string]any, tracks []any) map[string]a func (c *Client) normalizeTrack(raw map[string]any) map[string]any { release := mapAny(raw["release"]) trackArtistNames := artistNames(raw["artists"]) - artistName := displayArtistNames(trackArtistNames) + artistName := jsonutil.DisplayArtistNames(trackArtistNames) artistID := firstArtistID(raw["artists"]) albumArtist := strings.TrimSpace(jsonutil.StringFromAny(release["album_artist_name"])) if albumArtist == "" { @@ -592,7 +592,7 @@ func (c *Client) normalizeTrackListItem(raw map[string]any) map[string]any { "id": jsonutil.StringFromAny(raw["id"]), "title": jsonutil.StringFromAny(raw["name"]), "version": jsonutil.StringFromAny(raw["mix_name"]), - "artist": map[string]any{"name": displayArtistNames(names)}, + "artist": map[string]any{"name": jsonutil.DisplayArtistNames(names)}, "album": map[string]any{"id": jsonutil.NestedString(raw, "release", "id"), "title": jsonutil.NestedString(raw, "release", "name")}, "track_number": jsonutil.IntFromAny(raw["number"]), } @@ -712,32 +712,19 @@ func artistNames(v any) []string { return names } -func displayArtistNames(names []string) string { - switch len(names) { - case 0: - return "" - case 1: - return names[0] - case 2: - return names[0] + " & " + names[1] - default: - return strings.Join(names[:len(names)-1], ", ") + " & " + names[len(names)-1] - } -} - func (c *Client) releaseArtistName(v any) string { items := sliceAny(v) threshold := c.cfg.Session.Beatport.VariousArtistsThreshold if threshold >= 0 && len(items) > threshold { return "Various Artists" } - return displayArtistNames(artistNames(v)) + return jsonutil.DisplayArtistNames(artistNames(v)) } func (c *Client) releaseArtistNameForTracks(releaseArtists any, tracks []any) string { common := commonReleaseArtists(releaseArtists, tracks) if len(common) > 0 { - return displayArtistNames(artistNames(common)) + return jsonutil.DisplayArtistNames(artistNames(common)) } return c.releaseArtistName(releaseArtists) } diff --git a/internal/provider/beatport/client_test.go b/internal/provider/beatport/client_test.go index 389344e..b0ebb44 100644 --- a/internal/provider/beatport/client_test.go +++ b/internal/provider/beatport/client_test.go @@ -9,12 +9,13 @@ import ( "time" "streamrip-go/internal/config" + "streamrip-go/internal/jsonutil" ) func newTestClient(t *testing.T, handler http.Handler) (*Client, func()) { t.Helper() ts := httptest.NewServer(handler) - cfg := &config.Config{Session: config.DefaultConfigData(), File: config.DefaultConfigData()} + cfg := &config.Config{Session: testConfigData(), File: testConfigData()} cfg.Session.Beatport.Username = "user" cfg.Session.Beatport.Password = "pass" cfg.Session.Beatport.AccessToken = "token" @@ -199,7 +200,7 @@ func TestAlbumMetadataUsesArtistPresentOnEveryTrack(t *testing.T) { } func TestReleaseArtistCollapseCanBeDisabled(t *testing.T) { - cfg := &config.Config{Session: config.DefaultConfigData(), File: config.DefaultConfigData()} + cfg := &config.Config{Session: testConfigData(), File: testConfigData()} cfg.Session.Beatport.VariousArtistsThreshold = -1 c := New(cfg) got := c.releaseArtistName([]any{ @@ -214,13 +215,13 @@ func TestReleaseArtistCollapseCanBeDisabled(t *testing.T) { } func TestDisplayArtistNamesUsesCreditPunctuation(t *testing.T) { - got := displayArtistNames(artistNames([]any{ + got := jsonutil.DisplayArtistNames(artistNames([]any{ map[string]any{"name": "A"}, map[string]any{"name": "B"}, map[string]any{"name": "C"}, })) if got != "A, B & C" { - t.Fatalf("displayArtistNames() = %q", got) + t.Fatalf("DisplayArtistNames() = %q", got) } } @@ -265,3 +266,13 @@ func TestArtistMetadataUsesReleases(t *testing.T) { t.Fatalf("unexpected album: %+v", album) } } + +// testConfigData returns defaults with request throttling disabled. +// +// At the default 60 requests/minute the rate limiter sleeps 1s per request, +// which accounted for most of this suite's runtime (deezer alone was 31s). +func testConfigData() config.ConfigData { + d := config.DefaultConfigData() + d.Downloads.RequestsPerMinute = -1 + return d +} diff --git a/internal/provider/deezer/client_test.go b/internal/provider/deezer/client_test.go index 48b35af..bbb174d 100644 --- a/internal/provider/deezer/client_test.go +++ b/internal/provider/deezer/client_test.go @@ -24,7 +24,7 @@ func TestSearchTrack(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true @@ -74,7 +74,7 @@ func TestGetMetadataArtistPaginatesAlbums(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true @@ -135,7 +135,7 @@ func TestGetMetadataAlbumPaginatesTracks(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true @@ -189,7 +189,7 @@ func TestGetMetadataPlaylistPaginatesTracks(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true @@ -241,7 +241,7 @@ func TestGetDownloadableNativeCipher(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Deezer.ARL = "arl" c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true @@ -289,7 +289,7 @@ func TestGetDownloadablePrefersNoneCipherWhenAvailable(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Deezer.ARL = "arl" c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true @@ -354,7 +354,7 @@ func TestLoginPrefersARLFlowOverRefreshShortcut(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Deezer.ARL = "arl" cfgData.Deezer.RefreshToken = "refresh-token" c := New(&config.Config{File: cfgData, Session: cfgData}) @@ -386,7 +386,7 @@ func TestLoginPrefersARLFlowOverRefreshShortcut(t *testing.T) { } func TestGetDownloadableRequiresARL(t *testing.T) { - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Deezer.ARL = "" c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true @@ -409,7 +409,7 @@ func TestGetDownloadableDRMError(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Deezer.ARL = "arl" c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true @@ -448,7 +448,7 @@ func TestGetTrackTokenPrefersPipeToken(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true c.jwt = "jwt-token" @@ -499,7 +499,7 @@ func TestGetDownloadableUsesPipeTrackToken(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Deezer.ARL = "arl" c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true @@ -541,7 +541,7 @@ func TestGetMetadataAddsLyricsFromPipe(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true c.jwt = "jwt" @@ -594,7 +594,7 @@ func TestLoginWithCredentials(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Deezer.Email = "tidal1@alpin.sbs" cfgData.Deezer.Password = "tidal1@alpin.sbs" c := New(&config.Config{File: cfgData, Session: cfgData}) @@ -662,7 +662,7 @@ func TestMobileAuthIncludesAppContextParams(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) origGateway := gatewayURL @@ -703,7 +703,7 @@ func TestMobileUserAutologIncludesRefreshToken(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.sid = "sid123" c.userID = "42" @@ -748,7 +748,7 @@ func TestLoginWithRefreshToken(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Deezer.RefreshToken = "refresh-token" c := New(&config.Config{File: cfgData, Session: cfgData}) @@ -782,7 +782,7 @@ func TestRefreshJWTHTTPError(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.refresh = "refresh-token" @@ -802,7 +802,7 @@ func TestRefreshLicenseFromPipeGraphQLError(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.jwt = "jwt-token" @@ -848,7 +848,7 @@ func TestRefreshLicenseFromPipeUsesMobileGraphQLShape(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.jwt = "jwt-token" @@ -863,3 +863,13 @@ func TestRefreshLicenseFromPipeUsesMobileGraphQLShape(t *testing.T) { t.Fatalf("license = %q, want license-token", c.license) } } + +// testConfigData returns defaults with request throttling disabled. +// +// At the default 60 requests/minute the rate limiter sleeps 1s per request, +// which accounted for most of this suite's runtime (deezer alone was 31s). +func testConfigData() config.ConfigData { + d := config.DefaultConfigData() + d.Downloads.RequestsPerMinute = -1 + return d +} diff --git a/internal/provider/qobuz/client.go b/internal/provider/qobuz/client.go index c6d113f..dbec8f2 100644 --- a/internal/provider/qobuz/client.go +++ b/internal/provider/qobuz/client.go @@ -269,14 +269,6 @@ func (c *Client) GetMetadata(ctx context.Context, item, mediaType string) (map[s return resp, nil } -func (c *Client) GetTrackMetadata(ctx context.Context, id string) (*TrackMetadata, error) { - raw, err := c.GetMetadata(ctx, id, "track") - if err != nil { - return nil, err - } - return ParseTrackMetadata(raw) -} - func (c *Client) Search(ctx context.Context, mediaType, query string, limit int) ([]map[string]any, error) { if !c.LoggedIn() { return nil, errNotLoggedIn diff --git a/internal/provider/qobuz/client_test.go b/internal/provider/qobuz/client_test.go index 38eb598..cb30cad 100644 --- a/internal/provider/qobuz/client_test.go +++ b/internal/provider/qobuz/client_test.go @@ -208,7 +208,7 @@ func TestGetArtistPagination(t *testing.T) { func newTestClient(t *testing.T) *Client { t.Helper() - d := config.DefaultConfigData() + d := testConfigData() d.Qobuz.AppID = "12345" cfg := &config.Config{File: d, Session: d} return New(cfg) @@ -248,7 +248,7 @@ func TestLoginRefreshesAppCredentialsWhenSecretInvalid(t *testing.T) { })) defer ts.Close() - d := config.DefaultConfigData() + d := testConfigData() d.Qobuz.EmailOrUserID = "user@example.com" d.Qobuz.PasswordOrToken = "hash" d.Qobuz.AppID = "old-app" @@ -302,7 +302,7 @@ func TestLoginRetriesAfterRefreshingAppCredentials(t *testing.T) { })) defer ts.Close() - d := config.DefaultConfigData() + d := testConfigData() d.Qobuz.EmailOrUserID = "user@example.com" d.Qobuz.PasswordOrToken = "hash" d.Qobuz.AppID = "old-app" @@ -396,7 +396,7 @@ func qobuzSecretSig(requestTS, secret string) string { } func TestRefreshAppCredentialsRejectsEmptyData(t *testing.T) { - d := config.DefaultConfigData() + d := testConfigData() c := New(&config.Config{File: d, Session: d}) c.fetchCfg = func(context.Context) (string, []string, error) { return "", []string{" "}, nil @@ -406,3 +406,13 @@ func TestRefreshAppCredentialsRejectsEmptyData(t *testing.T) { t.Fatalf("expected error for empty refreshed app credentials") } } + +// testConfigData returns defaults with request throttling disabled. +// +// At the default 60 requests/minute the rate limiter sleeps 1s per request, +// which accounted for most of this suite's runtime (deezer alone was 31s). +func testConfigData() config.ConfigData { + d := config.DefaultConfigData() + d.Downloads.RequestsPerMinute = -1 + return d +} diff --git a/internal/provider/soundcloud/client_test.go b/internal/provider/soundcloud/client_test.go index 57778e9..f33658b 100644 --- a/internal/provider/soundcloud/client_test.go +++ b/internal/provider/soundcloud/client_test.go @@ -14,7 +14,7 @@ import ( ) func TestGetTrackMetadataAndDownloadable(t *testing.T) { - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true c.run = func(_ context.Context, _ string, args ...string) ([]byte, error) { @@ -49,7 +49,7 @@ func TestGetTrackMetadataAndDownloadable(t *testing.T) { } func TestGetPlaylistMetadata(t *testing.T) { - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true c.run = func(_ context.Context, _ string, args ...string) ([]byte, error) { @@ -81,7 +81,7 @@ func TestGetPlaylistMetadata(t *testing.T) { } func TestSearchTrack(t *testing.T) { - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true c.run = func(_ context.Context, _ string, args ...string) ([]byte, error) { @@ -122,7 +122,7 @@ func TestSearchPlaylist(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true c.http = ts.Client() @@ -167,7 +167,7 @@ func TestSearchPlaylistAcceptsDotsInPath(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.loggedIn = true c.http = ts.Client() @@ -203,7 +203,7 @@ func TestSearchPlaylistAcceptsDotsInPath(t *testing.T) { } func TestLoginShowsYtDlpHint(t *testing.T) { - cfgData := config.DefaultConfigData() + cfgData := testConfigData() c := New(&config.Config{File: cfgData, Session: cfgData}) c.bin = "definitely-not-a-real-yt-dlp-bin" err := c.Login(context.Background()) @@ -254,3 +254,13 @@ func TestCanonicalSoundcloudURLAcceptsSubdomain(t *testing.T) { t.Fatalf("canonical url = %q, want %q", got, "https://soundcloud.com/a/b") } } + +// testConfigData returns defaults with request throttling disabled. +// +// At the default 60 requests/minute the rate limiter sleeps 1s per request, +// which accounted for most of this suite's runtime (deezer alone was 31s). +func testConfigData() config.ConfigData { + d := config.DefaultConfigData() + d.Downloads.RequestsPerMinute = -1 + return d +} diff --git a/internal/provider/tidal/client.go b/internal/provider/tidal/client.go index e4eb89c..666f393 100644 --- a/internal/provider/tidal/client.go +++ b/internal/provider/tidal/client.go @@ -120,10 +120,10 @@ func (c *Client) Login(ctx context.Context) error { return fmt.Errorf("tidal login failed: status=%d body=%v", status, resp) } - if v := stringify(resp["countryCode"]); v != "" { + if v := jsonutil.StringFromAny(resp["countryCode"]); v != "" { c.cfg.Session.Tidal.CountryCode = v } - if v := stringify(resp["userId"]); v != "" { + if v := jsonutil.StringFromAny(resp["userId"]); v != "" { c.cfg.Session.Tidal.UserID = v } @@ -159,12 +159,12 @@ func (c *Client) refreshAccessToken(ctx context.Context) error { return fmt.Errorf("tidal token refresh failed: status=%d body=%v", status, resp) } - newToken := stringify(resp["access_token"]) + newToken := jsonutil.StringFromAny(resp["access_token"]) if newToken == "" { return errors.New("tidal token refresh missing access_token") } - newRefresh := stringify(resp["refresh_token"]) + newRefresh := jsonutil.StringFromAny(resp["refresh_token"]) expiresIn := int64(jsonutil.IntFromAny(resp["expires_in"])) if expiresIn <= 0 { expiresIn = 7 * 24 * 3600 @@ -263,8 +263,8 @@ func (c *Client) fetchTrackLyrics(ctx context.Context, trackID string) (string, return "", "" } - lyrics := strings.TrimSpace(stringify(resp["lyrics"])) - lrc := strings.TrimSpace(stringify(resp["subtitles"])) + lyrics := strings.TrimSpace(jsonutil.StringFromAny(resp["lyrics"])) + lrc := strings.TrimSpace(jsonutil.StringFromAny(resp["subtitles"])) return lyrics, lrc } @@ -339,30 +339,6 @@ func (c *Client) GetDownloadable(ctx context.Context, trackID string, quality in return c.getDownloadableFromTrackManifest(ctx, trackID, quality) } -func (c *Client) trackSupportsAtmos(ctx context.Context, trackID string) bool { - resp, status, err := c.apiRequest(ctx, "tracks/"+trackID, url.Values{}, c.baseURL) - if err != nil || status != http.StatusOK { - return false - } - if modes, ok := resp["audioModes"].([]any); ok { - for _, mode := range modes { - if strings.Contains(strings.ToUpper(stringify(mode)), "ATMOS") { - return true - } - } - } - if mm, ok := resp["mediaMetadata"].(map[string]any); ok { - if tags, ok := mm["tags"].([]any); ok { - for _, tag := range tags { - if strings.Contains(strings.ToUpper(stringify(tag)), "ATMOS") { - return true - } - } - } - } - return false -} - func (c *Client) getAtmosDownloadable(ctx context.Context, trackID string) (*provider.Downloadable, error) { var lastErr error for _, aq := range atmosAudioQualities { @@ -407,18 +383,18 @@ func (c *Client) getAtmosDownloadable(ctx context.Context, trackID string) (*pro } func playbackLooksAtmos(resp map[string]any) bool { - if strings.Contains(strings.ToUpper(stringify(resp["audioMode"])), "ATMOS") { + if strings.Contains(strings.ToUpper(jsonutil.StringFromAny(resp["audioMode"])), "ATMOS") { return true } if modes, ok := resp["audioModes"].([]any); ok { for _, raw := range modes { - if strings.Contains(strings.ToUpper(stringify(raw)), "ATMOS") { + if strings.Contains(strings.ToUpper(jsonutil.StringFromAny(raw)), "ATMOS") { return true } } } - manifestB64 := stringify(resp["manifest"]) + manifestB64 := jsonutil.StringFromAny(resp["manifest"]) if manifestB64 == "" { return false } @@ -430,17 +406,17 @@ func playbackLooksAtmos(resp map[string]any) bool { if err = json.Unmarshal(b, &manifest); err != nil { return false } - if strings.Contains(strings.ToUpper(stringify(manifest["audioMode"])), "ATMOS") { + if strings.Contains(strings.ToUpper(jsonutil.StringFromAny(manifest["audioMode"])), "ATMOS") { return true } if modes, ok := manifest["audioModes"].([]any); ok { for _, raw := range modes { - if strings.Contains(strings.ToUpper(stringify(raw)), "ATMOS") { + if strings.Contains(strings.ToUpper(jsonutil.StringFromAny(raw)), "ATMOS") { return true } } } - codec := strings.ToLower(stringify(manifest["codecs"])) + codec := strings.ToLower(jsonutil.StringFromAny(manifest["codecs"])) return strings.Contains(codec, "ec-3") || strings.Contains(codec, "eac3") || strings.Contains(codec, "joc") || strings.Contains(codec, "atmos") } @@ -524,7 +500,7 @@ func (c *Client) fetchArtistAlbums(ctx context.Context, artistID string) ([]map[ if wrapped, ok := itm["item"].(map[string]any); ok { itm = wrapped } - id := stringify(itm["id"]) + id := jsonutil.StringFromAny(itm["id"]) if id == "" { continue } @@ -576,14 +552,14 @@ func (c *Client) getDownloadableFromTrackManifestForFormats(ctx context.Context, if !ok { return nil, errors.New("tidal trackManifests missing attributes") } - uri := stringify(attrs["uri"]) + uri := jsonutil.StringFromAny(attrs["uri"]) if uri == "" { return nil, errors.New("tidal trackManifests missing uri") } attrFormats, _ := attrs["formats"].([]any) ext := "m4a" for _, f := range attrFormats { - fv := strings.ToUpper(stringify(f)) + fv := strings.ToUpper(jsonutil.StringFromAny(f)) if strings.Contains(fv, "FLAC") { ext = "flac" break @@ -630,7 +606,7 @@ func (c *Client) GetVideoDownloadable(ctx context.Context, videoID string) (*pro return nil, fmt.Errorf("tidal video playbackinfo failed: status=%d", status) } - manifestB64 := stringify(resp["manifest"]) + manifestB64 := jsonutil.StringFromAny(resp["manifest"]) if manifestB64 == "" { return nil, errors.New("tidal video manifest missing") } @@ -646,7 +622,7 @@ func (c *Client) GetVideoDownloadable(ctx context.Context, videoID string) (*pro if !ok || len(urls) == 0 { return nil, errors.New("tidal video manifest urls missing") } - masterURL := stringify(urls[0]) + masterURL := jsonutil.StringFromAny(urls[0]) if masterURL == "" { return nil, errors.New("tidal video master url missing") } @@ -677,7 +653,7 @@ func (c *Client) GetVideoDownloadable(ctx context.Context, videoID string) (*pro } func downloadableFromPlaybackManifest(resp map[string]any) *provider.Downloadable { - manifestB64 := stringify(resp["manifest"]) + manifestB64 := jsonutil.StringFromAny(resp["manifest"]) if manifestB64 == "" { return nil } @@ -693,11 +669,11 @@ func downloadableFromPlaybackManifest(resp map[string]any) *provider.Downloadabl if !ok || len(urls) == 0 { return nil } - streamURL := stringify(urls[0]) + streamURL := jsonutil.StringFromAny(urls[0]) if streamURL == "" { return nil } - codec := strings.ToLower(stringify(manifest["codecs"])) + codec := strings.ToLower(jsonutil.StringFromAny(manifest["codecs"])) ext := "m4a" if strings.Contains(codec, "flac") { ext = "flac" @@ -708,14 +684,14 @@ func downloadableFromPlaybackManifest(resp map[string]any) *provider.Downloadabl if profile.Container == "" { profile = tidalAudioProfileFromExtension(ext) } - audioQuality := strings.ToUpper(strings.TrimSpace(stringify(resp["audioQuality"]))) + audioQuality := strings.ToUpper(strings.TrimSpace(jsonutil.StringFromAny(resp["audioQuality"]))) if audioQuality == "" { - audioQuality = strings.ToUpper(strings.TrimSpace(stringify(manifest["audioQuality"]))) + audioQuality = strings.ToUpper(strings.TrimSpace(jsonutil.StringFromAny(manifest["audioQuality"]))) } if audioQuality != "" { profile = applyTidalAudioQuality(profile, audioQuality) } - if strings.Contains(strings.ToUpper(stringify(resp["audioMode"])), "ATMOS") { + if strings.Contains(strings.ToUpper(jsonutil.StringFromAny(resp["audioMode"])), "ATMOS") { profile = tidalAtmosAudioProfile() } return &provider.Downloadable{URL: streamURL, Extension: ext, Source: "tidal", Audio: profile} @@ -724,7 +700,7 @@ func downloadableFromPlaybackManifest(resp map[string]any) *provider.Downloadabl func tidalAudioProfileFromFormats(formats []any) provider.AudioProfile { best := provider.AudioProfile{} for _, raw := range formats { - f := strings.ToUpper(strings.TrimSpace(stringify(raw))) + f := strings.ToUpper(strings.TrimSpace(jsonutil.StringFromAny(raw))) switch { case strings.Contains(f, "EAC3") || strings.Contains(f, "JOC") || strings.Contains(f, "ATMOS"): return tidalAtmosAudioProfile() @@ -1003,28 +979,13 @@ func (c *Client) apiPost(ctx context.Context, endpoint string, form url.Values, }) } -func stringify(v any) string { - switch t := v.(type) { - case string: - return t - case int: - return strconv.Itoa(t) - case int64: - return strconv.FormatInt(t, 10) - case float64: - return strconv.FormatFloat(t, 'f', -1, 64) - default: - return "" - } -} - func enrichTidalImage(meta map[string]any) { if _, ok := meta["image"].(map[string]any); ok { return } - cover := stringify(meta["cover"]) + cover := jsonutil.StringFromAny(meta["cover"]) if cover == "" { - cover = stringify(meta["squareImage"]) + cover = jsonutil.StringFromAny(meta["squareImage"]) } if cover == "" { return diff --git a/internal/provider/tidal/client_test.go b/internal/provider/tidal/client_test.go index d3f94b2..69e0abf 100644 --- a/internal/provider/tidal/client_test.go +++ b/internal/provider/tidal/client_test.go @@ -12,10 +12,11 @@ import ( "testing" "streamrip-go/internal/config" + "streamrip-go/internal/jsonutil" ) func TestLoginMissingToken(t *testing.T) { - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Tidal.AccessToken = "" c := New(&config.Config{File: cfgData, Session: cfgData}) err := c.Login(context.Background()) @@ -37,7 +38,7 @@ func TestSearch(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Tidal.AccessToken = "token" cfgData.Tidal.CountryCode = "US" c := New(&config.Config{File: cfgData, Session: cfgData}) @@ -73,7 +74,7 @@ func TestGetVideoDownloadable(t *testing.T) { })) defer server.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Tidal.AccessToken = "token" cfgData.Tidal.CountryCode = "US" c := New(&config.Config{File: cfgData, Session: cfgData}) @@ -143,7 +144,7 @@ func TestGetMetadataArtistPaginatesAlbums(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Tidal.AccessToken = "token" cfgData.Tidal.CountryCode = "US" c := New(&config.Config{File: cfgData, Session: cfgData}) @@ -185,7 +186,7 @@ func TestGetMetadataTrackAddsLyricsAndSyncedLyrics(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Tidal.AccessToken = "token" cfgData.Tidal.CountryCode = "MY" c := New(&config.Config{File: cfgData, Session: cfgData}) @@ -197,10 +198,10 @@ func TestGetMetadataTrackAddsLyricsAndSyncedLyrics(t *testing.T) { if err != nil { t.Fatalf("GetMetadata() err = %v", err) } - if got := stringify(meta["lyrics"]); got != "plain lyrics line" { + if got := jsonutil.StringFromAny(meta["lyrics"]); got != "plain lyrics line" { t.Fatalf("lyrics = %q, want plain lyrics line", got) } - if got := stringify(meta["lyrics_synced"]); got != "[00:00.00]plain lyrics line" { + if got := jsonutil.StringFromAny(meta["lyrics_synced"]); got != "[00:00.00]plain lyrics line" { t.Fatalf("lyrics_synced = %q, want synced lrc", got) } } @@ -219,7 +220,7 @@ func TestGetMetadataTrackIgnoresLyricsEndpointFailure(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Tidal.AccessToken = "token" cfgData.Tidal.CountryCode = "US" c := New(&config.Config{File: cfgData, Session: cfgData}) @@ -257,7 +258,7 @@ func TestAPIRequestRetriesTooManyRequests(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Downloads.RequestsPerMinute = 0 cfgData.Tidal.AccessToken = "token" cfgData.Tidal.CountryCode = "US" @@ -274,8 +275,8 @@ func TestAPIRequestRetriesTooManyRequests(t *testing.T) { if calls != 2 { t.Fatalf("calls = %d, want 2", calls) } - if stringify(resp["title"]) != "Song" { - t.Fatalf("title = %q, want Song", stringify(resp["title"])) + if jsonutil.StringFromAny(resp["title"]) != "Song" { + t.Fatalf("title = %q, want Song", jsonutil.StringFromAny(resp["title"])) } } @@ -297,7 +298,7 @@ func TestAPIPostRetriesTooManyRequests(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Downloads.RequestsPerMinute = 0 c := New(&config.Config{File: cfgData, Session: cfgData}) @@ -311,8 +312,8 @@ func TestAPIPostRetriesTooManyRequests(t *testing.T) { 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"])) + if jsonutil.StringFromAny(resp["access_token"]) != "fresh-token" { + t.Fatalf("access_token = %q, want fresh-token", jsonutil.StringFromAny(resp["access_token"])) } } @@ -342,7 +343,7 @@ func TestGetDownloadablePrefersAtmosWhenEnabled(t *testing.T) { })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Tidal.AccessToken = "token" cfgData.Tidal.CountryCode = "US" cfgData.Tidal.PreferAtmos = true @@ -377,28 +378,6 @@ func TestPlaybackLooksAtmosFromManifest(t *testing.T) { } } -func TestTrackSupportsAtmosFromTags(t *testing.T) { - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != "/v1/tracks/42" { - w.WriteHeader(http.StatusNotFound) - return - } - _ = json.NewEncoder(w).Encode(map[string]any{"id": 42, "audioModes": []any{"STEREO"}, "mediaMetadata": map[string]any{"tags": []any{"LOSSLESS", "DOLBY_ATMOS"}}}) - })) - defer ts.Close() - - cfgData := config.DefaultConfigData() - cfgData.Tidal.AccessToken = "token" - cfgData.Tidal.CountryCode = "US" - c := New(&config.Config{File: cfgData, Session: cfgData}) - c.loggedIn = true - c.baseURL = ts.URL + "/v1" - - if !c.trackSupportsAtmos(context.Background(), "42") { - t.Fatalf("expected atmos support from mediaMetadata tags") - } -} - func TestFormatsForQuality(t *testing.T) { tests := []struct { name string @@ -448,7 +427,7 @@ func TestGetDownloadableLosslessUsesTrackManifestWhenPlaybackIsAAC(t *testing.T) })) defer ts.Close() - cfgData := config.DefaultConfigData() + cfgData := testConfigData() cfgData.Tidal.AccessToken = "token" cfgData.Tidal.CountryCode = "US" c := New(&config.Config{File: cfgData, Session: cfgData}) @@ -473,3 +452,13 @@ func TestGetDownloadableLosslessUsesTrackManifestWhenPlaybackIsAAC(t *testing.T) t.Fatalf("unexpected audio profile: %+v", d.Audio) } } + +// testConfigData returns defaults with request throttling disabled. +// +// At the default 60 requests/minute the rate limiter sleeps 1s per request, +// which accounted for most of this suite's runtime (deezer alone was 31s). +func testConfigData() config.ConfigData { + d := config.DefaultConfigData() + d.Downloads.RequestsPerMinute = -1 + return d +} diff --git a/internal/provider/yandex/client.go b/internal/provider/yandex/client.go index 270e8fb..2da05e3 100644 --- a/internal/provider/yandex/client.go +++ b/internal/provider/yandex/client.go @@ -36,7 +36,6 @@ const ( requestAttempts = 3 desktopWindowsSignKey = "kzqU4XhfCaY6B6JTHODeq5" legacyMP3SignSalt = "XGRlBW9FXlekgbPrRHuSiA" - defaultEstimatedKbps = 50000 ) var ErrMissingYandexToken = errors.New("missing yandex access_token") @@ -180,7 +179,7 @@ func (c *Client) Close() error { } func (c *Client) getTrackMetadata(ctx context.Context, item string) (map[string]any, error) { - trackRef := canonicalTrackRequestID(item) + trackRef := strings.TrimSpace(item) form := url.Values{} form.Set("trackIds", trackRef) form.Set("withMixData", "true") @@ -577,7 +576,7 @@ func normalizeTrack(raw map[string]any, fallbackID string, artistSeparator strin trackID := canonicalTrackRefFromRaw(raw, fallbackID) albumRaw := firstAlbum(raw) artistNames := artistNames(raw) - artistName := displayArtistNames(artistNames) + artistName := jsonutil.DisplayArtistNames(artistNames) albumArtist := joinAlbumArtists(albumRaw, artistSeparator) if albumArtist == "" { albumArtist = artistName @@ -596,7 +595,7 @@ func normalizeTrack(raw map[string]any, fallbackID string, artistSeparator strin "tracks_count": trackTotal, "media_number": discNumber, "numberOfVolumes": discTotal, - "release_date": firstNonEmpty(jsonutil.StringFromAny(raw["releaseDate"]), jsonutil.StringFromAny(albumRaw["releaseDate"])), + "release_date": jsonutil.FirstNonEmpty(jsonutil.StringFromAny(raw["releaseDate"]), jsonutil.StringFromAny(albumRaw["releaseDate"])), "parental_warning": isExplicit(raw), "explicit": isExplicit(raw), "source_track_id": jsonutil.StringFromAny(raw["realId"]), @@ -684,9 +683,9 @@ func normalizePlaylist(raw map[string]any) map[string]any { } } owner, _ := raw["owner"].(map[string]any) - ownerName := firstNonEmpty(jsonutil.StringFromAny(owner["name"]), jsonutil.StringFromAny(owner["login"])) + ownerName := jsonutil.FirstNonEmpty(jsonutil.StringFromAny(owner["name"]), jsonutil.StringFromAny(owner["login"])) return map[string]any{ - "id": firstNonEmpty(jsonutil.StringFromAny(raw["playlistUuid"]), jsonutil.StringFromAny(raw["kind"])), + "id": jsonutil.FirstNonEmpty(jsonutil.StringFromAny(raw["playlistUuid"]), jsonutil.StringFromAny(raw["kind"])), "name": jsonutil.StringFromAny(raw["title"]), "title": jsonutil.StringFromAny(raw["title"]), "description": jsonutil.StringFromAny(raw["description"]), @@ -738,8 +737,8 @@ func normalizeArtistSearchItem(raw map[string]any) map[string]any { func normalizePlaylistSearchItem(raw map[string]any) map[string]any { owner, _ := raw["owner"].(map[string]any) - ownerID := firstNonEmpty(jsonutil.StringFromAny(owner["uid"]), jsonutil.StringFromAny(raw["uid"])) - ownerName := firstNonEmpty(jsonutil.StringFromAny(owner["name"]), jsonutil.StringFromAny(owner["login"])) + ownerID := jsonutil.FirstNonEmpty(jsonutil.StringFromAny(owner["uid"]), jsonutil.StringFromAny(raw["uid"])) + ownerName := jsonutil.FirstNonEmpty(jsonutil.StringFromAny(owner["name"]), jsonutil.StringFromAny(owner["login"])) return map[string]any{ "id": ownerID + ":" + jsonutil.StringFromAny(raw["kind"]), "title": jsonutil.StringFromAny(raw["title"]), @@ -783,7 +782,7 @@ func firstArtistID(raw map[string]any) string { } func joinArtists(raw map[string]any, artistSeparator string) string { - return displayArtistNames(artistNames(raw)) + return jsonutil.DisplayArtistNames(artistNames(raw)) } func artistNames(raw map[string]any) []string { @@ -805,7 +804,7 @@ func joinAlbumArtists(albumRaw map[string]any, artistSeparator string) string { if len(albumRaw) == 0 { return "" } - return displayArtistNames(albumArtistNames(albumRaw)) + return jsonutil.DisplayArtistNames(albumArtistNames(albumRaw)) } func albumArtistNames(albumRaw map[string]any) []string { @@ -823,19 +822,6 @@ func albumArtistNames(albumRaw map[string]any) []string { return parts } -func displayArtistNames(names []string) string { - switch len(names) { - case 0: - return "" - case 1: - return names[0] - case 2: - return names[0] + " & " + names[1] - default: - return strings.Join(names[:len(names)-1], ", ") + " & " + names[len(names)-1] - } -} - func (c *Client) artistSeparator() string { if c.cfg.Session.Metadata.ArtistSeparator == "" { return "; " @@ -852,19 +838,11 @@ func trackNumbers(albumRaw map[string]any) (int, int) { return trackNumber, firstPositiveInt(jsonutil.IntFromAny(position["volume"]), 1) } -func canonicalTrackRequestID(item string) string { - item = strings.TrimSpace(item) - if item == "" { - return item - } - return item -} - func canonicalTrackRefFromRaw(raw map[string]any, fallback string) string { if strings.Contains(strings.TrimSpace(fallback), ":") { return strings.TrimSpace(fallback) } - trackID := firstNonEmpty(jsonutil.StringFromAny(raw["realId"]), jsonutil.StringFromAny(raw["id"])) + trackID := jsonutil.FirstNonEmpty(jsonutil.StringFromAny(raw["realId"]), jsonutil.StringFromAny(raw["id"])) albumID := jsonutil.StringFromAny(firstAlbum(raw)["id"]) if trackID == "" { trackID = strings.TrimSpace(fallback) @@ -931,7 +909,7 @@ func imageMapFromTrack(raw map[string]any, albumRaw map[string]any) map[string]a } func imageMapFromURI(raw map[string]any) map[string]any { - uri := firstNonEmpty(jsonutil.StringFromAny(raw["ogImage"]), jsonutil.StringFromAny(raw["coverUri"])) + uri := jsonutil.FirstNonEmpty(jsonutil.StringFromAny(raw["ogImage"]), jsonutil.StringFromAny(raw["coverUri"])) if uri == "" { if cover, ok := raw["cover"].(map[string]any); ok { uri = jsonutil.StringFromAny(cover["uri"]) @@ -1049,12 +1027,3 @@ func firstPositiveInt(vals ...int) int { } return 0 } - -func firstNonEmpty(vals ...string) string { - for _, v := range vals { - if strings.TrimSpace(v) != "" { - return strings.TrimSpace(v) - } - } - return "" -} diff --git a/internal/provider/yandex/client_test.go b/internal/provider/yandex/client_test.go index fd88a30..7d74adc 100644 --- a/internal/provider/yandex/client_test.go +++ b/internal/provider/yandex/client_test.go @@ -60,7 +60,7 @@ func TestGetDownloadableUsesModernGetFileInfo(t *testing.T) { })) defer ts.Close() - d := config.DefaultConfigData() + d := testConfigData() d.Downloads.RequestsPerMinute = 0 d.Yandex.AccessToken = "token" c := New(&config.Config{File: d, Session: d}) @@ -132,7 +132,7 @@ func TestGetMetadataTrackUsesModernTracksEndpoint(t *testing.T) { })) defer ts.Close() - d := config.DefaultConfigData() + d := testConfigData() d.Downloads.RequestsPerMinute = 0 d.Yandex.AccessToken = "token" c := New(&config.Config{File: d, Session: d}) @@ -167,13 +167,6 @@ func TestGetMetadataTrackUsesModernTracksEndpoint(t *testing.T) { } } -func TestDisplayArtistNamesUsesCreditPunctuation(t *testing.T) { - got := displayArtistNames([]string{"A", "B", "C"}) - if got != "A, B & C" { - t.Fatalf("displayArtistNames() = %q", got) - } -} - func TestLegacyDirectURLBuildsPlayableMP3URL(t *testing.T) { url, err := legacyDirectURL(&legacyDownloadInfoXML{ Host: "example.test", @@ -189,3 +182,13 @@ func TestLegacyDirectURLBuildsPlayableMP3URL(t *testing.T) { t.Fatalf("legacyDirectURL() = %q, want %q", url, want) } } + +// testConfigData returns defaults with request throttling disabled. +// +// At the default 60 requests/minute the rate limiter sleeps 1s per request, +// which accounted for most of this suite's runtime (deezer alone was 31s). +func testConfigData() config.ConfigData { + d := config.DefaultConfigData() + d.Downloads.RequestsPerMinute = -1 + return d +}