diff --git a/cmd/rip/args.go b/cmd/rip/args.go index d303623..c26ad0e 100644 --- a/cmd/rip/args.go +++ b/cmd/rip/args.go @@ -244,18 +244,3 @@ func parseQuality(raw string, min int, max int) (int, error) { } return q, nil } - -func asString(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 "" - } -} diff --git a/cmd/rip/cmd_smoke.go b/cmd/rip/cmd_smoke.go index bb3bd1d..019b19a 100644 --- a/cmd/rip/cmd_smoke.go +++ b/cmd/rip/cmd_smoke.go @@ -8,6 +8,7 @@ import ( "streamrip-go/internal/app" "streamrip-go/internal/config" + "streamrip-go/internal/jsonutil" "streamrip-go/internal/provider" ) @@ -24,7 +25,7 @@ func cmdSoundcloudSmoke(ctx context.Context, cfg *config.Config, _ globalOptions fmt.Fprintf(os.Stderr, "soundcloud smoke error: %v\n", err) return 1 } - fmt.Printf("soundcloud oembed ok: title=%q author=%q provider=%q\n", asString(meta["title"]), asString(meta["author_name"]), asString(meta["provider_name"])) + fmt.Printf("soundcloud oembed ok: title=%q author=%q provider=%q\n", jsonutil.StringFromAny(meta["title"]), jsonutil.StringFromAny(meta["author_name"]), jsonutil.StringFromAny(meta["provider_name"])) return 0 } @@ -157,11 +158,11 @@ func cmdQobuzSearchSmoke(ctx context.Context, cfg *config.Config, _ globalOption if !ok { continue } - id := asString(item["id"]) - title := asString(item["title"]) - version := asString(item["version"]) - bitDepth := asString(item["maximum_bit_depth"]) - sr := asString(item["maximum_sampling_rate"]) + id := jsonutil.StringFromAny(item["id"]) + title := jsonutil.StringFromAny(item["title"]) + version := jsonutil.StringFromAny(item["version"]) + bitDepth := jsonutil.StringFromAny(item["maximum_bit_depth"]) + sr := jsonutil.StringFromAny(item["maximum_sampling_rate"]) if version != "" { title = title + " (" + version + ")" } @@ -211,7 +212,7 @@ func cmdTidalSearchSmoke(ctx context.Context, cfg *config.Config, _ globalOption if !ok { item = wrapper } - fmt.Printf("album_id=%s | %s\n", asString(item["id"]), asString(item["title"])) + fmt.Printf("album_id=%s | %s\n", jsonutil.StringFromAny(item["id"]), jsonutil.StringFromAny(item["title"])) } } @@ -244,9 +245,9 @@ func cmdTidalMetadataSmoke(ctx context.Context, cfg *config.Config, _ globalOpti fmt.Fprintf(os.Stderr, "metadata error: %v\n", err) return 1 } - title := asString(meta["title"]) + title := jsonutil.StringFromAny(meta["title"]) if title == "" { - title = asString(meta["name"]) + title = jsonutil.StringFromAny(meta["name"]) } trackCount := 0 if tracksMap, ok := meta["tracks"].(map[string]any); ok { @@ -298,9 +299,9 @@ func cmdTidalVideoSmoke(ctx context.Context, cfg *config.Config, _ globalOptions fmt.Fprintf(os.Stderr, "video downloadable error: %v\n", err) return 1 } - title := asString(meta["title"]) + title := jsonutil.StringFromAny(meta["title"]) if title == "" { - title = asString(meta["name"]) + title = jsonutil.StringFromAny(meta["name"]) } fmt.Printf("tidal video ok: id=%s title=%q ext=%s\n", videoID, title, d.Extension) fmt.Printf("stream_url=%s\n", d.URL) diff --git a/cmd/rip/cmd_test.go b/cmd/rip/cmd_test.go new file mode 100644 index 0000000..3d28538 --- /dev/null +++ b/cmd/rip/cmd_test.go @@ -0,0 +1,248 @@ +package main + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "streamrip-go/internal/config" +) + +// testConfig returns a config rooted in a temp dir, so command functions can +// run without touching the user's real config or download folder. +func testConfig(t *testing.T) *config.Config { + t.Helper() + tmp := t.TempDir() + d := config.DefaultConfigData() + d.Downloads.Folder = filepath.Join(tmp, "downloads") + d.Database.DownloadsEnabled = false + d.Database.FailedDownloadsEnabled = false + d.CLI.ProgressBars = false + return &config.Config{Path: filepath.Join(tmp, "config.toml"), File: d, Session: d} +} + +// The command functions became testable when main() was split into run() int +// plus one function per command; previously they were arms of a 1196-line +// switch that called os.Exit directly. + +func TestCommandsRejectMissingArguments(t *testing.T) { + tests := []struct { + name string + run func(ctx context.Context, cfg *config.Config, gopts globalOptions, args []string) int + args []string + }{ + {"url with no args", cmdURL, nil}, + {"file with no args", cmdFile, nil}, + {"config with no args", cmdConfig, nil}, + {"database with no args", cmdDatabase, nil}, + {"database without browse", cmdDatabase, []string{"downloads"}}, + {"database browse without table", cmdDatabase, []string{"browse"}}, + {"id with too few args", cmdID, []string{"qobuz", "album"}}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := tc.run(context.Background(), testConfig(t), globalOptions{}, tc.args) + if got != 2 { + t.Fatalf("exit code = %d, want 2 (usage error)", got) + } + }) + } +} + +func TestCmdDatabaseRejectsUnknownTable(t *testing.T) { + if got := cmdDatabase(context.Background(), testConfig(t), globalOptions{}, []string{"browse", "bogus"}); got != 2 { + t.Fatalf("exit code = %d, want 2", got) + } +} + +func TestCmdConfigRejectsUnknownSubcommand(t *testing.T) { + if got := cmdConfig(context.Background(), testConfig(t), globalOptions{}, []string{"bogus"}); got != 2 { + t.Fatalf("exit code = %d, want 2", got) + } +} + +func TestCmdConfigPathPrintsPath(t *testing.T) { + cfg := testConfig(t) + out := captureStdout(t, func() { + if got := cmdConfig(context.Background(), cfg, globalOptions{}, []string{"path"}); got != 0 { + t.Errorf("exit code = %d, want 0", got) + } + }) + if !strings.Contains(out, cfg.Path) { + t.Fatalf("output %q does not contain the config path %q", out, cfg.Path) + } +} + +func TestCmdConfigPathRejectsExtraArgument(t *testing.T) { + if got := cmdConfig(context.Background(), testConfig(t), globalOptions{}, []string{"path", "extra"}); got != 2 { + t.Fatalf("exit code = %d, want 2", got) + } +} + +func TestCmdFileReportsMissingFile(t *testing.T) { + missing := filepath.Join(t.TempDir(), "nope.txt") + if got := cmdFile(context.Background(), testConfig(t), globalOptions{}, []string{missing}); got != 1 { + t.Fatalf("exit code = %d, want 1 (read error)", got) + } +} + +func TestCmdFileRejectsUnknownOption(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "urls.txt") + if err := os.WriteFile(path, []byte("https://play.qobuz.com/album/abc\n"), 0o600); err != nil { + t.Fatal(err) + } + if got := cmdFile(context.Background(), testConfig(t), globalOptions{}, []string{path, "--bogus"}); got != 2 { + t.Fatalf("exit code = %d, want 2", got) + } +} + +func TestCmdURLRejectsInvalidURL(t *testing.T) { + // Nothing resolvable is queued, so this reports "nothing to rip" and exits 0. + out := captureStdout(t, func() { + if got := cmdURL(context.Background(), testConfig(t), globalOptions{}, []string{"not-a-url"}); got != 0 { + t.Errorf("exit code = %d, want 0", got) + } + }) + if !strings.Contains(out, "nothing to rip") { + t.Fatalf("output %q should report that nothing was queued", out) + } +} + +func TestCmdIDRejectsQualityBelowSourceFloor(t *testing.T) { + // Qobuz has no quality 0. + if got := cmdID(context.Background(), testConfig(t), globalOptions{}, []string{"qobuz", "album", "abc", "0"}); got != 2 { + t.Fatalf("exit code = %d, want 2", got) + } + if got := cmdID(context.Background(), testConfig(t), globalOptions{}, []string{"beatport", "album", "abc", "0"}); got != 2 { + t.Fatalf("beatport exit code = %d, want 2", got) + } +} + +func TestCmdIDRejectsUnknownOption(t *testing.T) { + if got := cmdID(context.Background(), testConfig(t), globalOptions{}, []string{"qobuz", "album", "abc", "--bogus"}); got != 2 { + t.Fatalf("exit code = %d, want 2", got) + } +} + +func TestSmokeCommandsRejectMissingArguments(t *testing.T) { + tests := []struct { + name string + run func(ctx context.Context, cfg *config.Config, gopts globalOptions, args []string) int + args []string + }{ + {"soundcloud", cmdSoundcloudSmoke, nil}, + {"qobuz", cmdQobuzSmoke, nil}, + {"qobuz convert", cmdQobuzConvertRipSmoke, []string{"id"}}, + {"qobuz search", cmdQobuzSearchSmoke, nil}, + {"tidal search", cmdTidalSearchSmoke, nil}, + {"tidal metadata", cmdTidalMetadataSmoke, []string{"track"}}, + {"tidal video", cmdTidalVideoSmoke, nil}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := tc.run(context.Background(), testConfig(t), globalOptions{}, tc.args); got != 2 { + t.Fatalf("exit code = %d, want 2", got) + } + }) + } +} + +// captureStdout swaps os.Stdout for a pipe while fn runs. The commands print +// user-facing output with fmt.Println rather than taking a writer. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + orig := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stdout = w + + done := make(chan string, 1) + go func() { + var sb strings.Builder + buf := make([]byte, 4096) + for { + n, readErr := r.Read(buf) + if n > 0 { + sb.Write(buf[:n]) + } + if readErr != nil { + break + } + } + done <- sb.String() + }() + + fn() + + _ = w.Close() + os.Stdout = orig + out := <-done + _ = r.Close() + return out +} + +func TestVersionOutput(t *testing.T) { + var sb strings.Builder + printVersion(&sb) + out := sb.String() + if !strings.HasPrefix(out, "rip ") { + t.Errorf("version output should start with \"rip \": %q", out) + } + for _, want := range []string{"go:", "target:"} { + if !strings.Contains(out, want) { + t.Errorf("version output missing %q: %q", want, out) + } + } +} + +func TestParseGlobalArgsVersionFlag(t *testing.T) { + for _, flag := range []string{"--version", "-V"} { + opts, err := parseGlobalArgs([]string{flag}) + if err != nil { + t.Fatalf("parseGlobalArgs(%q) error = %v", flag, err) + } + if !opts.version { + t.Errorf("parseGlobalArgs(%q) did not set version", flag) + } + } +} + +func TestApplyQualityOverrideCoversEverySource(t *testing.T) { + cfg := testConfig(t) + applyQualityOverride(cfg, 4) + + got := map[string]int{ + "qobuz": cfg.Session.Qobuz.Quality, + "tidal": cfg.Session.Tidal.Quality, + "deezer": cfg.Session.Deezer.Quality, + "soundcloud": cfg.Session.Soundcloud.Quality, + "yandex": cfg.Session.Yandex.Quality, + "beatport": cfg.Session.Beatport.Quality, + } + for source, q := range got { + if q != 4 { + t.Errorf("%s quality = %d, want 4; -q must reach every source", source, q) + } + } +} + +func TestApplyQualityOverrideClampsSourceFloor(t *testing.T) { + cfg := testConfig(t) + applyQualityOverride(cfg, 0) + + if cfg.Session.Qobuz.Quality != 1 { + t.Errorf("qobuz quality = %d, want it clamped to 1", cfg.Session.Qobuz.Quality) + } + if cfg.Session.Beatport.Quality != 1 { + t.Errorf("beatport quality = %d, want it clamped to 1", cfg.Session.Beatport.Quality) + } + if cfg.Session.Tidal.Quality != 0 { + t.Errorf("tidal quality = %d, want 0 preserved", cfg.Session.Tidal.Quality) + } +} diff --git a/cmd/rip/helpers.go b/cmd/rip/helpers.go index c430bb6..441fbbc 100644 --- a/cmd/rip/helpers.go +++ b/cmd/rip/helpers.go @@ -13,6 +13,7 @@ import ( "strings" "streamrip-go/internal/app" + "streamrip-go/internal/jsonutil" "streamrip-go/internal/urlparse" ) @@ -94,12 +95,12 @@ func parseFileInput(content []byte) ([]fileIDItem, []string, int, bool, error) { if !ok { return nil, nil, 0, true, fmt.Errorf("json item %d must be an object", i+1) } - source := strings.ToLower(strings.TrimSpace(asString(entry["source"]))) - mediaType := strings.ToLower(strings.TrimSpace(asString(entry["media_type"]))) + source := strings.ToLower(strings.TrimSpace(jsonutil.StringFromAny(entry["source"]))) + mediaType := strings.ToLower(strings.TrimSpace(jsonutil.StringFromAny(entry["media_type"]))) if mediaType == "" { - mediaType = strings.ToLower(strings.TrimSpace(asString(entry["mediaType"]))) + mediaType = strings.ToLower(strings.TrimSpace(jsonutil.StringFromAny(entry["mediaType"]))) } - id := strings.TrimSpace(asString(entry["id"])) + id := strings.TrimSpace(jsonutil.StringFromAny(entry["id"])) if source == "" || mediaType == "" || id == "" { return nil, nil, 0, true, fmt.Errorf("json item %d missing source/media_type/id", i+1) } diff --git a/cmd/rip/lastfm.go b/cmd/rip/lastfm.go index 2d96fb3..76135ef 100644 --- a/cmd/rip/lastfm.go +++ b/cmd/rip/lastfm.go @@ -13,6 +13,7 @@ import ( "time" "streamrip-go/internal/app" + "streamrip-go/internal/jsonutil" "streamrip-go/internal/netutil" "streamrip-go/internal/provider" ) @@ -241,8 +242,8 @@ func extractLastFMTitleArtistPairs(page string) []lastFMTrack { if len(dataPairs) > 0 { out := make([]lastFMTrack, 0, len(dataPairs)) for _, m := range dataPairs { - title := html.UnescapeString(strings.TrimSpace(firstNonEmpty(m[1], m[2]))) - artist := html.UnescapeString(strings.TrimSpace(firstNonEmpty(m[3], m[4]))) + title := html.UnescapeString(strings.TrimSpace(jsonutil.FirstNonEmpty(m[1], m[2]))) + artist := html.UnescapeString(strings.TrimSpace(jsonutil.FirstNonEmpty(m[3], m[4]))) if title == "" || artist == "" { continue } @@ -256,8 +257,8 @@ func extractLastFMTitleArtistPairs(page string) []lastFMTrack { titles := lastFMTitleTagsRe.FindAllStringSubmatch(page, -1) out := make([]lastFMTrack, 0, len(titles)/2) for i := 0; i+1 < len(titles); i += 2 { - titleRaw := strings.TrimSpace(firstNonEmpty(titles[i][1], titles[i][2])) - artistRaw := strings.TrimSpace(firstNonEmpty(titles[i+1][1], titles[i+1][2])) + titleRaw := strings.TrimSpace(jsonutil.FirstNonEmpty(titles[i][1], titles[i][2])) + artistRaw := strings.TrimSpace(jsonutil.FirstNonEmpty(titles[i+1][1], titles[i+1][2])) if strings.EqualFold(titleRaw, "Play on YouTube") || strings.EqualFold(artistRaw, "Play on YouTube") { continue } @@ -271,15 +272,6 @@ func extractLastFMTitleArtistPairs(page string) []lastFMTrack { return out } -func firstNonEmpty(items ...string) string { - for _, item := range items { - if strings.TrimSpace(item) != "" { - return strings.TrimSpace(item) - } - } - return "" -} - func extractLastFMTracksFromMirrorMarkdown(md string) (string, []lastFMTrack) { lines := strings.Split(strings.ReplaceAll(md, "\r\n", "\n"), "\n") title := "" diff --git a/cmd/rip/main.go b/cmd/rip/main.go index be2600a..7c67997 100644 --- a/cmd/rip/main.go +++ b/cmd/rip/main.go @@ -65,13 +65,13 @@ func run() int { cfg, err := config.Load(gopts.configPath) if err != nil { if errors.Is(err, config.ErrOutdatedConfig) { - resolvedPath, upErr := config.UpgradeOutdated(gopts.configPath) + resolvedPath, backupPath, upErr := config.UpgradeOutdated(gopts.configPath) if upErr != nil { fmt.Fprintf(os.Stderr, "config error: %v\n", err) fmt.Fprintf(os.Stderr, "config auto-upgrade failed: %v\n", upErr) return 1 } - fmt.Fprintf(os.Stderr, "config upgraded at %s\n", resolvedPath) + fmt.Fprintf(os.Stderr, "config upgraded at %s (previous copy kept at %s)\n", resolvedPath, backupPath) cfg, err = config.Load(gopts.configPath) } } diff --git a/cmd/rip/search.go b/cmd/rip/search.go index 93dc49f..a2c22fd 100644 --- a/cmd/rip/search.go +++ b/cmd/rip/search.go @@ -330,38 +330,38 @@ func normalizeSearchResults(source, mediaType string, pages []map[string]any) [] if !ok { continue } - id := asString(itm["id"]) - title := asString(itm["title"]) + id := jsonutil.StringFromAny(itm["id"]) + title := jsonutil.StringFromAny(itm["title"]) if title == "" { - title = asString(itm["name"]) + title = jsonutil.StringFromAny(itm["name"]) } - if version := asString(itm["version"]); version != "" { + if version := jsonutil.StringFromAny(itm["version"]); version != "" { title += " (" + version + ")" } - artist := nestedSearchString(itm, "artist", "name") + artist := jsonutil.NestedString(itm, "artist", "name") if artist == "" { - artist = nestedSearchString(itm, "performer", "name") + artist = jsonutil.NestedString(itm, "performer", "name") } - album := nestedSearchString(itm, "album", "title") - trackCount := searchInt(itm["tracks_count"]) + album := jsonutil.NestedString(itm, "album", "title") + trackCount := jsonutil.IntFromAny(itm["tracks_count"]) if trackCount == 0 { - trackCount = searchInt(itm["track_count"]) + trackCount = jsonutil.IntFromAny(itm["track_count"]) } explicit := searchBool(itm["parental_warning"]) - date := firstNonEmpty( - asString(itm["release_date_original"]), - asString(itm["release_date"]), - asString(itm["releaseDate"]), - asString(itm["streamStartDate"]), - nestedSearchString(itm, "album", "release_date_original"), - nestedSearchString(itm, "album", "release_date"), + date := jsonutil.FirstNonEmpty( + jsonutil.StringFromAny(itm["release_date_original"]), + jsonutil.StringFromAny(itm["release_date"]), + jsonutil.StringFromAny(itm["releaseDate"]), + jsonutil.StringFromAny(itm["streamStartDate"]), + jsonutil.NestedString(itm, "album", "release_date_original"), + jsonutil.NestedString(itm, "album", "release_date"), ) releases := 0 if mediaType == "artist" { - releases = firstPositiveInt( - searchInt(itm["albums_count"]), - searchInt(itm["albumsCount"]), - nestedSearchInt(itm, "albums", "total"), + releases = jsonutil.FirstPositiveInt( + jsonutil.IntFromAny(itm["albums_count"]), + jsonutil.IntFromAny(itm["albumsCount"]), + jsonutil.IntFromAny(jsonutil.NestedAny(itm, "albums", "total")), ) } appendUnique(searchResult{ID: id, Title: title, Artist: artist, Album: album, Date: date, Releases: releases, TrackCount: trackCount, Explicit: explicit}) @@ -379,39 +379,39 @@ func normalizeSearchResults(source, mediaType string, pages []map[string]any) [] if wrapped, ok := itm["item"].(map[string]any); ok { itm = wrapped } - id := asString(itm["id"]) - title := asString(itm["title"]) + id := jsonutil.StringFromAny(itm["id"]) + title := jsonutil.StringFromAny(itm["title"]) if title == "" { - title = asString(itm["name"]) + title = jsonutil.StringFromAny(itm["name"]) } - artist := nestedSearchString(itm, "artist", "name") + artist := jsonutil.NestedString(itm, "artist", "name") if artist == "" { if artists, ok := itm["artists"].([]any); ok && len(artists) > 0 { if a0, ok := artists[0].(map[string]any); ok { - artist = asString(a0["name"]) + artist = jsonutil.StringFromAny(a0["name"]) } } } - album := nestedSearchString(itm, "album", "title") - trackCount := searchInt(itm["numberOfTracks"]) + album := jsonutil.NestedString(itm, "album", "title") + trackCount := jsonutil.IntFromAny(itm["numberOfTracks"]) if trackCount == 0 { - trackCount = searchInt(itm["tracks_count"]) + trackCount = jsonutil.IntFromAny(itm["tracks_count"]) } explicit := searchBool(itm["explicit"]) - date := firstNonEmpty( - asString(itm["releaseDate"]), - asString(itm["streamStartDate"]), - asString(itm["release_date"]), - nestedSearchString(itm, "album", "releaseDate"), - nestedSearchString(itm, "album", "release_date"), + date := jsonutil.FirstNonEmpty( + jsonutil.StringFromAny(itm["releaseDate"]), + jsonutil.StringFromAny(itm["streamStartDate"]), + jsonutil.StringFromAny(itm["release_date"]), + jsonutil.NestedString(itm, "album", "releaseDate"), + jsonutil.NestedString(itm, "album", "release_date"), ) releases := 0 if mediaType == "artist" { - releases = firstPositiveInt( - searchInt(itm["numberOfAlbums"]), - searchInt(itm["albums_count"]), - searchInt(itm["albumsCount"]), - nestedSearchInt(itm, "albums", "total"), + releases = jsonutil.FirstPositiveInt( + jsonutil.IntFromAny(itm["numberOfAlbums"]), + jsonutil.IntFromAny(itm["albums_count"]), + jsonutil.IntFromAny(itm["albumsCount"]), + jsonutil.IntFromAny(jsonutil.NestedAny(itm, "albums", "total")), ) } appendUnique(searchResult{ID: id, Title: title, Artist: artist, Album: album, Date: date, Releases: releases, TrackCount: trackCount, Explicit: explicit}) @@ -431,25 +431,25 @@ func normalizeSearchResults(source, mediaType string, pages []map[string]any) [] if !ok { continue } - id := asString(itm["id"]) - title := asString(itm["title"]) + id := jsonutil.StringFromAny(itm["id"]) + title := jsonutil.StringFromAny(itm["title"]) if title == "" { - title = asString(itm["name"]) + title = jsonutil.StringFromAny(itm["name"]) } - artist := nestedSearchString(itm, "artist", "name") - album := nestedSearchString(itm, "album", "title") - trackCount := searchInt(itm["nb_tracks"]) + artist := jsonutil.NestedString(itm, "artist", "name") + album := jsonutil.NestedString(itm, "album", "title") + trackCount := jsonutil.IntFromAny(itm["nb_tracks"]) explicit := searchBool(itm["explicit_lyrics"]) - date := firstNonEmpty( - asString(itm["release_date"]), - nestedSearchString(itm, "album", "release_date"), + date := jsonutil.FirstNonEmpty( + jsonutil.StringFromAny(itm["release_date"]), + jsonutil.NestedString(itm, "album", "release_date"), ) releases := 0 if mediaType == "artist" { - releases = firstPositiveInt( - searchInt(itm["nb_album"]), - searchInt(itm["albums_count"]), - searchInt(itm["numberOfAlbums"]), + releases = jsonutil.FirstPositiveInt( + jsonutil.IntFromAny(itm["nb_album"]), + jsonutil.IntFromAny(itm["albums_count"]), + jsonutil.IntFromAny(itm["numberOfAlbums"]), ) } appendUnique(searchResult{ID: id, Title: title, Artist: artist, Album: album, Date: date, Releases: releases, TrackCount: trackCount, Explicit: explicit}) @@ -464,13 +464,13 @@ func normalizeSearchResults(source, mediaType string, pages []map[string]any) [] if !ok { continue } - id := asString(itm["id"]) - title := asString(itm["title"]) - artist := nestedSearchString(itm, "artist", "name") - trackCount := searchInt(itm["tracks_count"]) - date := firstNonEmpty( - asString(itm["release_date"]), - asString(itm["display_date"]), + id := jsonutil.StringFromAny(itm["id"]) + title := jsonutil.StringFromAny(itm["title"]) + artist := jsonutil.NestedString(itm, "artist", "name") + trackCount := jsonutil.IntFromAny(itm["tracks_count"]) + date := jsonutil.FirstNonEmpty( + jsonutil.StringFromAny(itm["release_date"]), + jsonutil.StringFromAny(itm["display_date"]), ) appendUnique(searchResult{ID: id, Title: title, Artist: artist, Date: date, TrackCount: trackCount}) } @@ -484,34 +484,34 @@ func normalizeSearchResults(source, mediaType string, pages []map[string]any) [] if !ok { continue } - id := asString(itm["id"]) - title := asString(itm["title"]) + id := jsonutil.StringFromAny(itm["id"]) + title := jsonutil.StringFromAny(itm["title"]) if title == "" { - title = asString(itm["name"]) + title = jsonutil.StringFromAny(itm["name"]) } - artist := nestedSearchString(itm, "artist", "name") + artist := jsonutil.NestedString(itm, "artist", "name") if artist == "" { - artist = nestedSearchString(itm, "performer", "name") + artist = jsonutil.NestedString(itm, "performer", "name") } - album := nestedSearchString(itm, "album", "title") - trackCount := firstPositiveInt( - searchInt(itm["trackCount"]), - searchInt(itm["track_count"]), - searchInt(itm["tracks_count"]), + album := jsonutil.NestedString(itm, "album", "title") + trackCount := jsonutil.FirstPositiveInt( + jsonutil.IntFromAny(itm["trackCount"]), + jsonutil.IntFromAny(itm["track_count"]), + jsonutil.IntFromAny(itm["tracks_count"]), ) explicit := searchBool(itm["explicit"]) - date := firstNonEmpty( - asString(itm["release_date"]), - asString(itm["releaseDate"]), - nestedSearchString(itm, "album", "release_date"), - nestedSearchString(itm, "album", "releaseDate"), + date := jsonutil.FirstNonEmpty( + jsonutil.StringFromAny(itm["release_date"]), + jsonutil.StringFromAny(itm["releaseDate"]), + jsonutil.NestedString(itm, "album", "release_date"), + jsonutil.NestedString(itm, "album", "releaseDate"), ) releases := 0 if mediaType == "artist" { - releases = firstPositiveInt( - searchInt(itm["albums_count"]), - searchInt(itm["numberOfAlbums"]), - nestedSearchInt(itm, "albums", "total"), + releases = jsonutil.FirstPositiveInt( + jsonutil.IntFromAny(itm["albums_count"]), + jsonutil.IntFromAny(itm["numberOfAlbums"]), + jsonutil.IntFromAny(jsonutil.NestedAny(itm, "albums", "total")), ) } appendUnique(searchResult{ID: id, Title: title, Artist: artist, Album: album, Date: date, Releases: releases, TrackCount: trackCount, Explicit: explicit}) @@ -526,18 +526,18 @@ func normalizeSearchResults(source, mediaType string, pages []map[string]any) [] if !ok { continue } - id := asString(itm["id"]) - title := asString(itm["title"]) + id := jsonutil.StringFromAny(itm["id"]) + title := jsonutil.StringFromAny(itm["title"]) if title == "" { - title = asString(itm["name"]) + title = jsonutil.StringFromAny(itm["name"]) } - if version := asString(itm["version"]); version != "" { + if version := jsonutil.StringFromAny(itm["version"]); version != "" { title += " (" + version + ")" } - artist := nestedSearchString(itm, "artist", "name") - album := nestedSearchString(itm, "album", "title") - trackCount := firstPositiveInt(searchInt(itm["tracks_count"]), searchInt(itm["track_count"])) - date := firstNonEmpty(asString(itm["release_date_original"]), asString(itm["release_date"]), nestedSearchString(itm, "album", "release_date_original")) + artist := jsonutil.NestedString(itm, "artist", "name") + album := jsonutil.NestedString(itm, "album", "title") + trackCount := jsonutil.FirstPositiveInt(jsonutil.IntFromAny(itm["tracks_count"]), jsonutil.IntFromAny(itm["track_count"])) + date := jsonutil.FirstNonEmpty(jsonutil.StringFromAny(itm["release_date_original"]), jsonutil.StringFromAny(itm["release_date"]), jsonutil.NestedString(itm, "album", "release_date_original")) appendUnique(searchResult{ID: id, Title: title, Artist: artist, Album: album, Date: date, TrackCount: trackCount}) } } @@ -608,55 +608,10 @@ func formatSearchOptionLabel(index int, mediaType string, r searchResult) string } } -func nestedSearchString(v map[string]any, keys ...string) string { - cur := any(v) - for _, key := range keys { - m, ok := cur.(map[string]any) - if !ok { - return "" - } - cur = m[key] - } - return asString(cur) -} - -func nestedSearchInt(v map[string]any, keys ...string) int { - cur := any(v) - for _, key := range keys { - m, ok := cur.(map[string]any) - if !ok { - return 0 - } - cur = m[key] - } - return searchInt(cur) -} - -func firstPositiveInt(values ...int) int { - for _, v := range values { - if v > 0 { - return v - } - } - return 0 -} - -func searchInt(v any) int { - switch t := v.(type) { - case int: - return t - case int64: - return int(t) - case float64: - return int(t) - case string: - i, _ := strconv.Atoi(t) - return i - default: - return 0 - } -} - +// searchBool is deliberately stricter than jsonutil.BoolFromAny: it accepts +// only a real bool. The explicit/parental-warning fields it reads are proper +// booleans in every provider's search payload, and widening this to coerce +// strings and numbers would change how results are labelled. func searchBool(v any) bool { b, ok := v.(bool) return ok && b diff --git a/internal/config/config.go b/internal/config/config.go index 603b5b0..1655cba 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strings" "sync" + "time" "github.com/pelletier/go-toml/v2" ) @@ -370,25 +371,42 @@ func NormalizeCodec(raw string) (string, error) { } } -func UpgradeOutdated(path string) (string, error) { +// UpgradeOutdated rewrites an outdated config at the current version. It runs +// automatically, without prompting, so it copies the original aside first: the +// rewrite re-serializes the whole file from the parsed struct, which drops any +// key this version does not know about. It returns the config path and the +// backup path. +// +// This is a version stamp, not a migration framework. There are no per-version +// transforms, so a future key rename still needs explicit handling here. +func UpgradeOutdated(path string) (resolved string, backup string, err error) { resolvedPath, err := resolvePath(path) if err != nil { - return "", err + return "", "", err } raw, err := os.ReadFile(resolvedPath) if err != nil { - return "", err + return "", "", err } + + backupPath := fmt.Sprintf("%s.bak-%d", resolvedPath, time.Now().Unix()) + if err = os.WriteFile(backupPath, raw, 0o600); err != nil { + return "", "", fmt.Errorf("could not back up %s before upgrading: %w", resolvedPath, err) + } + data := DefaultConfigData() if err = toml.Unmarshal(raw, &data); err != nil { - return "", err + return "", "", err } applyRuntimeDefaults(&data) data.Misc.Version = CurrentConfigVersion - if err = saveConfigData(resolvedPath, data); err != nil { - return "", err + if err = Validate(&data); err != nil { + return "", "", err } - return resolvedPath, nil + if err = saveConfigData(resolvedPath, data); err != nil { + return "", "", err + } + return resolvedPath, backupPath, nil } func (c *Config) SaveFile() error { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b75d3e9..b21b442 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -70,7 +70,7 @@ func TestUpgradeOutdatedConfig(t *testing.T) { t.Fatalf("saveConfigData() error = %v", err) } - resolved, err := UpgradeOutdated(path) + resolved, backup, err := UpgradeOutdated(path) if err != nil { t.Fatalf("UpgradeOutdated() error = %v", err) } @@ -78,6 +78,28 @@ func TestUpgradeOutdatedConfig(t *testing.T) { t.Fatalf("resolved path = %q, want %q", resolved, path) } + // The upgrade re-serializes the whole file and runs without prompting, so + // it must leave the original behind. + if backup == "" { + t.Fatal("UpgradeOutdated() returned no backup path") + } + backupRaw, err := os.ReadFile(backup) + if err != nil { + t.Fatalf("backup not written: %v", err) + } + var backedUp ConfigData + if err := toml.Unmarshal(backupRaw, &backedUp); err != nil { + t.Fatalf("backup is not valid toml: %v", err) + } + if backedUp.Misc.Version != "1.0.0" { + t.Errorf("backup version = %q, want the pre-upgrade 1.0.0", backedUp.Misc.Version) + } + if info, err := os.Stat(backup); err != nil { + t.Fatal(err) + } else if perm := info.Mode().Perm(); perm != 0o600 { + t.Errorf("backup permissions = %04o, want 0600 (it holds the same credentials)", perm) + } + cfg, err := Load(path) if err != nil { t.Fatalf("Load() after upgrade error = %v", err) diff --git a/internal/jsonutil/jsonutil.go b/internal/jsonutil/jsonutil.go index ddc7497..2cf28de 100644 --- a/internal/jsonutil/jsonutil.go +++ b/internal/jsonutil/jsonutil.go @@ -145,3 +145,16 @@ func DisplayArtistNames(names []string) string { return strings.Join(names[:len(names)-1], ", ") + " & " + names[len(names)-1] } } + +// FirstPositiveInt returns the first value greater than zero, or 0 if there is +// none. Useful for picking among optional metadata fields where 0 means +// "absent". Previously duplicated in provider/yandex, provider/beatport and +// cmd/rip. +func FirstPositiveInt(values ...int) int { + for _, v := range values { + if v > 0 { + return v + } + } + return 0 +} diff --git a/internal/jsonutil/jsonutil_test.go b/internal/jsonutil/jsonutil_test.go index f1cf88e..02f2369 100644 --- a/internal/jsonutil/jsonutil_test.go +++ b/internal/jsonutil/jsonutil_test.go @@ -182,3 +182,22 @@ func TestDisplayArtistNames(t *testing.T) { } } } + +func TestFirstPositiveInt(t *testing.T) { + tests := []struct { + in []int + want int + }{ + {nil, 0}, + {[]int{0, 0}, 0}, + {[]int{0, 5, 7}, 5}, + {[]int{-3, 0, 2}, 2}, + {[]int{9}, 9}, + {[]int{-1}, 0}, + } + for _, tc := range tests { + if got := FirstPositiveInt(tc.in...); got != tc.want { + t.Errorf("FirstPositiveInt(%v) = %d, want %d", tc.in, got, tc.want) + } + } +} diff --git a/internal/provider/beatport/client.go b/internal/provider/beatport/client.go index 1f16c2c..75045cd 100644 --- a/internal/provider/beatport/client.go +++ b/internal/provider/beatport/client.go @@ -532,7 +532,7 @@ func (c *Client) normalizeRelease(raw map[string]any, tracks []any) map[string]a "title": jsonutil.StringFromAny(raw["name"]), "artist": map[string]any{"name": artist}, "release_date_original": jsonutil.StringFromAny(raw["new_release_date"]), - "tracks_count": firstPositiveInt(jsonutil.IntFromAny(raw["track_count"]), len(items)), + "tracks_count": jsonutil.FirstPositiveInt(jsonutil.IntFromAny(raw["track_count"]), len(items)), "maximum_bit_depth": 16, "maximum_sampling_rate": "44.1", "image": imageMap(raw["image"]), @@ -648,7 +648,7 @@ func playlistMetadata(raw map[string]any, tracks []any) map[string]any { "id": jsonutil.StringFromAny(raw["id"]), "name": name, "title": name, - "tracks_count": firstPositiveInt(jsonutil.IntFromAny(raw["track_count"]), len(tracks)), + "tracks_count": jsonutil.FirstPositiveInt(jsonutil.IntFromAny(raw["track_count"]), len(tracks)), "image": imageMap(raw["image"]), "tracks": map[string]any{"items": tracks}, } @@ -829,15 +829,6 @@ func sliceAny(v any) []any { return s } -func firstPositiveInt(vals ...int) int { - for _, v := range vals { - if v > 0 { - return v - } - } - return 0 -} - func cloneValues(in url.Values) url.Values { out := url.Values{} for k, vals := range in { diff --git a/internal/provider/yandex/client.go b/internal/provider/yandex/client.go index 0f1b816..a20b20e 100644 --- a/internal/provider/yandex/client.go +++ b/internal/provider/yandex/client.go @@ -581,7 +581,7 @@ func normalizeTrack(raw map[string]any, fallbackID string, artistSeparator strin } trackNumber, discNumber := trackNumbers(albumRaw) trackTotal := jsonutil.IntFromAny(albumRaw["trackCount"]) - discTotal := firstPositiveInt( + discTotal := jsonutil.FirstPositiveInt( jsonutil.IntFromAny(raw["volumesCount"]), jsonutil.IntFromAny(albumRaw["volumesCount"]), ) @@ -726,7 +726,7 @@ func normalizeArtistSearchItem(raw map[string]any) map[string]any { "id": jsonutil.StringFromAny(raw["id"]), "title": jsonutil.StringFromAny(raw["name"]), "name": jsonutil.StringFromAny(raw["name"]), - "albums_count": firstPositiveInt( + "albums_count": jsonutil.FirstPositiveInt( jsonutil.IntFromAny(jsonutil.NestedAny(raw, "counts", "directAlbums")), jsonutil.IntFromAny(jsonutil.NestedAny(raw, "counts", "tracks")), ), @@ -833,7 +833,7 @@ func trackNumbers(albumRaw map[string]any) (int, int) { if trackNumber > 0 { trackNumber++ } - return trackNumber, firstPositiveInt(jsonutil.IntFromAny(position["volume"]), 1) + return trackNumber, jsonutil.FirstPositiveInt(jsonutil.IntFromAny(position["volume"]), 1) } func canonicalTrackRefFromRaw(raw map[string]any, fallback string) string { @@ -1019,15 +1019,6 @@ func waitRetry(ctx context.Context, delay time.Duration) error { } } -func firstPositiveInt(vals ...int) int { - for _, v := range vals { - if v > 0 { - return v - } - } - return 0 -} - // signKey and legacyMP3Salt read the vendor request-signing keys from config, // falling back to the built-in defaults. func (c *Client) signKey() string {