diff --git a/cmd/rip/args.go b/cmd/rip/args.go index 47ec727..5531a60 100644 --- a/cmd/rip/args.go +++ b/cmd/rip/args.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "os" "strconv" "strings" @@ -124,16 +125,10 @@ func parseGlobalArgs(args []string) (globalOptions, error) { return opts, nil } +// normalizeCodec delegates to config so the --codec flag and a codec set in +// the config file are validated by the same rules. func normalizeCodec(raw string) (string, error) { - codec := strings.ToUpper(strings.TrimSpace(raw)) - switch codec { - case "ALAC", "FLAC", "MP3", "AAC", "VORBIS": - return codec, nil - case "OGG": - return "VORBIS", nil - default: - return "", fmt.Errorf("unsupported codec %q (expected ALAC, FLAC, OGG, MP3, AAC)", raw) - } + return config.NormalizeCodec(raw) } func applyGlobalConfigOverrides(cfg *config.Config, opts globalOptions) { @@ -144,10 +139,7 @@ func applyGlobalConfigOverrides(cfg *config.Config, opts globalOptions) { cfg.Session.Database.DownloadsEnabled = false } if opts.qualitySet { - cfg.Session.Qobuz.Quality = opts.quality - cfg.Session.Tidal.Quality = opts.quality - cfg.Session.Deezer.Quality = opts.quality - cfg.Session.Soundcloud.Quality = opts.quality + applyQualityOverride(cfg, opts.quality) } if opts.codecSet { cfg.Session.Conversion.Enabled = true @@ -161,6 +153,50 @@ func applyGlobalConfigOverrides(cfg *config.Config, opts globalOptions) { } } +// minQualityForSource is the lowest quality id each source accepts. Qobuz and +// Beatport have no equivalent of the "lowest" tier that 0 means elsewhere. +var minQualityForSource = map[string]int{ + "qobuz": 1, + "beatport": 1, +} + +// applyQualityOverride applies a global -q/--quality to every source. +// +// It previously set only qobuz, tidal, deezer and soundcloud, so +// `rip -q 4 url ` was a silent no-op even though +// Main.qualityForSource reads Yandex and Beatport too. Sources with a higher +// floor are clamped rather than erroring, because one global flag has to work +// across a mixed queue. +func applyQualityOverride(cfg *config.Config, quality int) { + clamped := map[string]int{} + for _, source := range []string{"qobuz", "tidal", "deezer", "soundcloud", "yandex", "beatport"} { + q := quality + if min, ok := minQualityForSource[source]; ok && q < min { + q = min + clamped[source] = q + } + switch source { + case "qobuz": + cfg.Session.Qobuz.Quality = q + case "tidal": + cfg.Session.Tidal.Quality = q + case "deezer": + cfg.Session.Deezer.Quality = q + case "soundcloud": + cfg.Session.Soundcloud.Quality = q + case "yandex": + cfg.Session.Yandex.Quality = q + case "beatport": + cfg.Session.Beatport.Quality = q + } + } + for _, source := range []string{"qobuz", "beatport"} { + if q, ok := clamped[source]; ok { + fmt.Fprintf(os.Stderr, "note: %s has no quality %d; using %d for %s\n", source, quality, q, source) + } + } +} + func errorWithActionableHint(err error, opts globalOptions) string { if err == nil { return "" diff --git a/cmd/rip/main.go b/cmd/rip/main.go index 53df584..415ed4c 100644 --- a/cmd/rip/main.go +++ b/cmd/rip/main.go @@ -261,8 +261,11 @@ func run() int { } } def := config.DefaultConfigData() - cfg.File = def - cfg.Session = def + // Separate copies: assigning the same value to both halves made + // File.Qobuz.Secrets and Session.Qobuz.Secrets share one backing + // array. + cfg.File = config.CloneConfigData(def) + cfg.Session = config.CloneConfigData(def) if err = cfg.SaveFile(); err != nil { fmt.Fprintf(os.Stderr, "reset config error: %v\n", err) return 1 diff --git a/internal/config/config.go b/internal/config/config.go index 6c50c67..037200a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -213,9 +213,92 @@ func Load(path string) (*Config, error) { return nil, fmt.Errorf("%w: need to update from %q to %q", ErrOutdatedConfig, data.Misc.Version, CurrentConfigVersion) } + if err = Validate(&data); err != nil { + return nil, err + } + return &Config{Path: resolvedPath, File: data, Session: cloneConfigData(data)}, nil } +// qualityRange is the accepted quality id range per source. Qobuz and Beatport +// have no "lowest" tier, so their floor is 1. +var qualityRange = map[string][2]int{ + "qobuz": {1, 4}, + "tidal": {0, 4}, + "deezer": {0, 2}, + "soundcloud": {0, 1}, + "yandex": {0, 4}, + "beatport": {1, 4}, +} + +// Validate rejects config values that would otherwise fail much later, deep in +// a rip, or be silently patched over at individual call sites. Load previously +// checked nothing but misc.version, so a negative max_connections or a bogus +// conversion codec was accepted and only noticed mid-download. +// +// It normalizes what can be normalized (the conversion codec, which until now +// was only canonicalized when it arrived via --codec) and errors on the rest. +func Validate(data *ConfigData) error { + for source, bounds := range qualityRange { + var q int + switch source { + case "qobuz": + q = data.Qobuz.Quality + case "tidal": + q = data.Tidal.Quality + case "deezer": + q = data.Deezer.Quality + case "soundcloud": + q = data.Soundcloud.Quality + case "yandex": + q = data.Yandex.Quality + case "beatport": + q = data.Beatport.Quality + } + if q < bounds[0] || q > bounds[1] { + return fmt.Errorf("config: %s.quality is %d, must be %d-%d", source, q, bounds[0], bounds[1]) + } + } + + if data.Downloads.MaxConnections < 1 { + return fmt.Errorf("config: downloads.max_connections is %d, must be >= 1", data.Downloads.MaxConnections) + } + if data.Downloads.RequestsPerMinute == 0 { + return errors.New("config: downloads.requests_per_minute is 0; use a positive value, or a negative value to disable throttling") + } + if data.Filepaths.TruncateTo < 1 { + return fmt.Errorf("config: filepaths.truncate_to is %d, must be >= 1", data.Filepaths.TruncateTo) + } + + if data.Conversion.Enabled { + codec, err := NormalizeCodec(data.Conversion.Codec) + if err != nil { + return fmt.Errorf("config: conversion.codec: %w", err) + } + data.Conversion.Codec = codec + } + + // artwork.*_max_width uses -1 (and any non-positive value) to mean "do not + // downscale", so there is no lower bound to enforce here. + + return nil +} + +// NormalizeCodec canonicalizes a conversion codec name. It lives here, rather +// than only in the CLI flag parser, so a codec set in the config file goes +// through the same check. +func NormalizeCodec(raw string) (string, error) { + codec := strings.ToUpper(strings.TrimSpace(raw)) + switch codec { + case "ALAC", "FLAC", "MP3", "AAC", "VORBIS": + return codec, nil + case "OGG": + return "VORBIS", nil + default: + return "", fmt.Errorf("unsupported codec %q (expected ALAC, FLAC, OGG, MP3, AAC)", raw) + } +} + func UpgradeOutdated(path string) (string, error) { resolvedPath, err := resolvePath(path) if err != nil { @@ -420,6 +503,13 @@ func applyRuntimeDefaults(data *ConfigData) { } } +// CloneConfigData returns a deep copy of in. The slice fields must not be +// shared between Config.File and Config.Session, or writing one silently +// mutates the other. +func CloneConfigData(in ConfigData) ConfigData { + return cloneConfigData(in) +} + func cloneConfigData(in ConfigData) ConfigData { out := in out.Qobuz.Secrets = append([]string(nil), in.Qobuz.Secrets...) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 9f260a2..64febc7 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -5,6 +5,8 @@ import ( "os" "path/filepath" "testing" + + "github.com/pelletier/go-toml/v2" ) func TestDefaultConfigData(t *testing.T) { @@ -110,3 +112,30 @@ func TestSessionCloneDoesNotAliasSlices(t *testing.T) { t.Fatalf("file qobuz secrets unexpectedly mutated") } } + +func TestDefaultConfigPassesValidation(t *testing.T) { + d := DefaultConfigData() + applyRuntimeDefaults(&d) + if err := Validate(&d); err != nil { + t.Fatalf("shipped defaults fail validation: %v", err) + } +} + +// TestExampleConfigIsValid guards the file users are told to copy. +func TestExampleConfigIsValid(t *testing.T) { + raw, err := os.ReadFile(filepath.Join("..", "..", "config.toml.example")) + if err != nil { + t.Skipf("example config not found: %v", err) + } + data := DefaultConfigData() + if err := toml.Unmarshal(raw, &data); err != nil { + t.Fatalf("config.toml.example does not parse: %v", err) + } + applyRuntimeDefaults(&data) + if err := Validate(&data); err != nil { + t.Fatalf("config.toml.example fails validation: %v", err) + } + if data.Misc.Version != CurrentConfigVersion { + t.Errorf("config.toml.example version = %q, want %q", data.Misc.Version, CurrentConfigVersion) + } +} diff --git a/internal/download/downloader_test.go b/internal/download/downloader_test.go index b81fe98..c21e4e2 100644 --- a/internal/download/downloader_test.go +++ b/internal/download/downloader_test.go @@ -4,8 +4,8 @@ import ( "context" "crypto/aes" "crypto/cipher" - "errors" "encoding/hex" + "errors" "io" "net/http" "net/http/httptest" diff --git a/internal/provider/qobuz/client.go b/internal/provider/qobuz/client.go index 185963c..c6d113f 100644 --- a/internal/provider/qobuz/client.go +++ b/internal/provider/qobuz/client.go @@ -59,11 +59,12 @@ var ( ) type Client struct { - cfg *config.Config - http *http.Client - limiter *ratelimit.Limiter - baseURL string - fetchCfg func(ctx context.Context) (string, []string, error) + cfg *config.Config + http *http.Client + streamHTTP *http.Client + limiter *ratelimit.Limiter + baseURL string + fetchCfg func(ctx context.Context) (string, []string, error) // authMu guards loggedIn and the login/refresh path, which download // workers can reach concurrently. authMu sync.Mutex @@ -91,11 +92,16 @@ type mobileFileURL struct { func New(cfg *config.Config) *Client { return &Client{ - cfg: cfg, - http: netutil.NewHTTPClient(30*time.Second, cfg.Session.Downloads.VerifySSL, cfg.Session.Downloads.MaxConnections), - limiter: ratelimit.New(cfg.Session.Downloads.RequestsPerMinute), - baseURL: baseURL, - fetchCfg: nil, + cfg: cfg, + http: netutil.NewHTTPClient(30*time.Second, cfg.Session.Downloads.VerifySSL, cfg.Session.Downloads.MaxConnections), + // Separate client with no Timeout for streaming a whole track body. + // http.Client.Timeout bounds the entire request including the body + // read, so the 30s API client aborted any track that took longer than + // 30s to stream. internal/download does the same for this reason. + streamHTTP: netutil.NewHTTPClient(0, cfg.Session.Downloads.VerifySSL, cfg.Session.Downloads.MaxConnections), + limiter: ratelimit.New(cfg.Session.Downloads.RequestsPerMinute), + baseURL: baseURL, + fetchCfg: nil, } } @@ -1171,7 +1177,11 @@ func (c *Client) mobileCopyURLToWriter(ctx context.Context, sourceURL string, ou return err } c.setMobileHeaders(req) - resp, err := c.http.Do(req) + client := c.streamHTTP + if client == nil { + client = c.http + } + resp, err := client.Do(req) if err != nil { return err } @@ -1316,7 +1326,17 @@ func (c *Client) mobileDeriveTrackKey(encryptedKey string) ([]byte, error) { if err != nil { return nil, fmt.Errorf("decode mobile info: %w", err) } - reader := hkdf.New(func() hash.Hash { return sha256.New() }, hexDecodeOrNil(mobileAppSecret), salt, info) + // A non-hex secret used to be swallowed and HKDF keyed on nil, which + // derives a wrong KEK and yields silently corrupt audio rather than an + // error. + secretKey, err := hex.DecodeString(mobileAppSecret) + if err != nil { + return nil, fmt.Errorf("decode mobile app secret: %w", err) + } + if len(secretKey) == 0 { + return nil, errors.New("mobile app secret is empty") + } + reader := hkdf.New(func() hash.Hash { return sha256.New() }, secretKey, salt, info) kek := make([]byte, 16) if _, err = io.ReadFull(reader, kek); err != nil { return nil, fmt.Errorf("mobile hkdf derive failed: %w", err) @@ -1325,11 +1345,6 @@ func (c *Client) mobileDeriveTrackKey(encryptedKey string) ([]byte, error) { return unwrapQobuzTrackKey(encryptedKey, kek) } -func hexDecodeOrNil(s string) []byte { - b, _ := hex.DecodeString(s) - return b -} - func unwrapQobuzTrackKey(encryptedKey string, kek []byte) ([]byte, error) { parts := strings.SplitN(encryptedKey, ".", 3) if len(parts) != 3 || parts[0] != mobileSessionProf { diff --git a/internal/provider/soundcloud/client.go b/internal/provider/soundcloud/client.go index 81890da..593178c 100644 --- a/internal/provider/soundcloud/client.go +++ b/internal/provider/soundcloud/client.go @@ -16,7 +16,9 @@ import ( "streamrip-go/internal/config" "streamrip-go/internal/jsonutil" + "streamrip-go/internal/netutil" "streamrip-go/internal/provider" + "streamrip-go/internal/ratelimit" ) var errUnsupportedMediaType = errors.New("unsupported soundcloud media type") @@ -26,12 +28,13 @@ var soundcloudSearchBaseURL = "https://soundcloud.com" type commandRunner func(ctx context.Context, name string, args ...string) ([]byte, error) type Client struct { - cfg *config.Config - bin string - run commandRunner - http *http.Client - mu sync.Mutex - cache map[string]map[string]any + cfg *config.Config + bin string + run commandRunner + http *http.Client + limiter *ratelimit.Limiter + mu sync.Mutex + cache map[string]map[string]any // authMu guards loggedIn. Kept separate from mu, which guards cache, so a // cache operation can never block a login check. @@ -41,11 +44,16 @@ type Client struct { func New(cfg *config.Config) *Client { return &Client{ - cfg: cfg, - bin: "yt-dlp", - run: runCommand, - http: &http.Client{Timeout: 20 * time.Second}, - cache: map[string]map[string]any{}, + cfg: cfg, + bin: "yt-dlp", + run: runCommand, + // Use the shared constructor like every other provider: rolling our own + // client here meant soundcloud silently ignored downloads.verify_ssl and + // downloads.max_connections, and bypassed the transport that logs + // requests at -vv with credentials redacted. + http: netutil.NewHTTPClient(20*time.Second, cfg.Session.Downloads.VerifySSL, cfg.Session.Downloads.MaxConnections), + limiter: ratelimit.New(cfg.Session.Downloads.RequestsPerMinute), + cache: map[string]map[string]any{}, } } @@ -141,6 +149,11 @@ func (c *Client) searchPlaylists(ctx context.Context, query string, limit int) ( } req.Header.Set("User-Agent", "Mozilla/5.0") + if c.limiter != nil { + if err = c.limiter.Wait(ctx); err != nil { + return nil, err + } + } resp, err := c.http.Do(req) if err != nil { return nil, err diff --git a/internal/store/sqlite.go b/internal/store/sqlite.go index a0d02dc..5fb29bf 100644 --- a/internal/store/sqlite.go +++ b/internal/store/sqlite.go @@ -3,23 +3,40 @@ package store import ( "context" "database/sql" + "fmt" + "io" + "net/url" + "os" + "strings" "sync" + "time" _ "modernc.org/sqlite" ) type SQLite struct { - db *sql.DB - mu sync.Mutex + db *sql.DB + path string + mu sync.Mutex + + // hasLegacyRows is set when the database was migrated from the pre-source + // schema. Those rows have no source, so IsDownloaded has to consider them + // separately or the entire pre-migration library looks undownloaded. + hasLegacyRows bool + legacyWarned bool } func NewSQLite(path string) (*SQLite, error) { - db, err := sql.Open("sqlite", path) + db, err := sql.Open("sqlite", sqliteDSN(path)) if err != nil { return nil, err } + // One writer at a time. SQLite allows a single writer anyway, and the + // mutex below already serializes this process; capping the pool keeps the + // driver from opening connections that can only contend with each other. + db.SetMaxOpenConns(1) - s := &SQLite{db: db} + s := &SQLite{db: db, path: path} if err = s.init(); err != nil { _ = db.Close() return nil, err @@ -27,6 +44,70 @@ func NewSQLite(path string) (*SQLite, error) { return s, nil } +// sqliteDSN builds a file: URI with the pragmas this app needs. +// +// WAL lets a concurrent reader (for example `rip db browse`) run while a rip +// is writing, and busy_timeout makes a contended write wait instead of failing +// immediately with SQLITE_BUSY. The path is passed through url.URL so that a +// config path containing '?' or '#' is not parsed as query parameters. +func sqliteDSN(path string) string { + u := url.URL{Scheme: "file", Opaque: (&url.URL{Path: path}).EscapedPath()} + q := url.Values{} + q.Add("_pragma", "busy_timeout(10000)") + q.Add("_pragma", "journal_mode(WAL)") + q.Add("_pragma", "foreign_keys(on)") + return u.String() + "?" + q.Encode() +} + +// schemaVersion is stored in PRAGMA user_version so later migrations can be +// ordered explicitly instead of being inferred from PRAGMA table_info. +const schemaVersion = 1 + +func (s *SQLite) userVersion() (int, error) { + var v int + if err := s.db.QueryRow(`PRAGMA user_version`).Scan(&v); err != nil { + return 0, err + } + return v, nil +} + +func (s *SQLite) setUserVersion(v int) error { + _, err := s.db.Exec(fmt.Sprintf(`PRAGMA user_version = %d`, v)) + return err +} + +// backupFile copies the database aside before a destructive migration. A +// migration that loses the download history silently costs the user a full +// re-download, so keep a copy they can fall back to. +func (s *SQLite) backupFile() (string, error) { + if strings.TrimSpace(s.path) == "" { + return "", nil + } + src, err := os.Open(s.path) + if err != nil { + if os.IsNotExist(err) { + return "", nil + } + return "", err + } + defer func() { _ = src.Close() }() + + dstPath := fmt.Sprintf("%s.bak-%d", s.path, time.Now().Unix()) + dst, err := os.OpenFile(dstPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) + if err != nil { + return "", err + } + if _, err = io.Copy(dst, src); err != nil { + _ = dst.Close() + _ = os.Remove(dstPath) + return "", err + } + if err = dst.Close(); err != nil { + return "", err + } + return dstPath, nil +} + func (s *SQLite) init() error { if err := s.ensureDownloadsSchema(); err != nil { return err @@ -47,6 +128,20 @@ func (s *SQLite) init() error { } } + // A database migrated by an earlier version has sourceless rows but no + // in-memory flag, so detect them here rather than only at migration time. + if !s.hasLegacyRows { + if n, err := s.legacyRowCount(context.Background()); err == nil && n > 0 { + s.hasLegacyRows = true + } + } + + if v, err := s.userVersion(); err == nil && v < schemaVersion { + if err = s.setUserVersion(schemaVersion); err != nil { + return err + } + } + return nil } @@ -94,6 +189,14 @@ func (s *SQLite) ensureDownloadsSchema() error { return err } + // Legacy schema: a downloads table with no source column. Back the file up + // first, because the rows below can only be carried over without a source + // and that is not fully reversible. + backupPath, backupErr := s.backupFile() + if backupErr != nil { + return fmt.Errorf("could not back up %s before migrating: %w", s.path, backupErr) + } + tx, err := s.db.Begin() if err != nil { return err @@ -120,12 +223,27 @@ func (s *SQLite) ensureDownloadsSchema() error { if _, err = tx.Exec(`INSERT OR IGNORE INTO downloads(source, id) SELECT '', id FROM downloads_legacy`); err != nil { return err } - if _, err = tx.Exec(`DROP TABLE downloads_legacy`); err != nil { + // downloads_legacy is deliberately kept. Dropping it used to make the + // migration unrecoverable, and IsDownloaded needs the sourceless rows to + // avoid re-downloading everything the user already has. + + if err = tx.Commit(); err != nil { return err } - err = tx.Commit() - return err + s.hasLegacyRows = true + if backupPath != "" { + fmt.Fprintf(os.Stderr, "download database migrated to the source-aware schema; previous copy kept at %s\n", backupPath) + } + return nil +} + +// legacyRowCount reports how many pre-migration rows exist. Rows carried over +// from the old schema have an empty source. +func (s *SQLite) legacyRowCount(ctx context.Context) (int, error) { + var n int + err := s.db.QueryRowContext(ctx, `SELECT COUNT(1) FROM downloads WHERE source = ''`).Scan(&n) + return n, err } func (s *SQLite) IsDownloaded(ctx context.Context, source, id string) (bool, error) { @@ -137,6 +255,26 @@ func (s *SQLite) IsDownloaded(ctx context.Context, source, id string) (bool, err if err != nil { return false, err } + if count > 0 { + return true, nil + } + + // Rows migrated from the pre-source schema have an empty source, so an + // exact match on (source, id) misses them and the whole pre-migration + // library would look undownloaded. Matching on id alone can only ever + // over-match across sources, which costs a skipped re-download rather than + // a duplicated library. + if !s.hasLegacyRows { + return false, nil + } + err = s.db.QueryRowContext(ctx, `SELECT COUNT(1) FROM downloads WHERE source = '' AND id = ?`, id).Scan(&count) + if err != nil { + return false, err + } + if count > 0 && !s.legacyWarned { + s.legacyWarned = true + fmt.Fprintf(os.Stderr, "note: matching downloads recorded before sources were tracked; pass --force to re-download them\n") + } return count > 0, nil } diff --git a/internal/store/sqlite_test.go b/internal/store/sqlite_test.go index 2209f54..450c14c 100644 --- a/internal/store/sqlite_test.go +++ b/internal/store/sqlite_test.go @@ -2,7 +2,12 @@ package store import ( "context" + "database/sql" + "fmt" + "os" "path/filepath" + "strings" + "sync" "testing" ) @@ -48,3 +53,193 @@ func TestSQLiteStore(t *testing.T) { t.Fatalf("MarkFailed() error = %v", err) } } + +// makeLegacyDB creates a database with the pre-source-column schema and the +// given ids, as a pre-migration install would have on disk. +func makeLegacyDB(t *testing.T, path string, ids ...string) { + t.Helper() + db, err := sql.Open("sqlite", path) + if err != nil { + t.Fatal(err) + } + defer func() { _ = db.Close() }() + + if _, err = db.Exec(`CREATE TABLE downloads (id TEXT NOT NULL PRIMARY KEY)`); err != nil { + t.Fatal(err) + } + for _, id := range ids { + if _, err = db.Exec(`INSERT INTO downloads(id) VALUES (?)`, id); err != nil { + t.Fatal(err) + } + } +} + +// TestLegacyMigrationPreservesHistory is the regression test for the migration +// that silently wiped the dedupe state: rows were carried over with source=empty +// while IsDownloaded matched source exactly, so every previously downloaded +// track looked new and the whole library was re-downloaded. +func TestLegacyMigrationPreservesHistory(t *testing.T) { + ctx := context.Background() + dir := t.TempDir() + path := filepath.Join(dir, "downloads.db") + makeLegacyDB(t, path, "old-1", "old-2") + + s, err := NewSQLite(path) + if err != nil { + t.Fatalf("NewSQLite() error = %v", err) + } + defer func() { _ = s.Close() }() + + for _, id := range []string{"old-1", "old-2"} { + ok, err := s.IsDownloaded(ctx, "qobuz", id) + if err != nil { + t.Fatalf("IsDownloaded(%q) error = %v", id, err) + } + if !ok { + t.Errorf("pre-migration download %q reported as not downloaded", id) + } + } + + ok, err := s.IsDownloaded(ctx, "qobuz", "never-seen") + if err != nil { + t.Fatal(err) + } + if ok { + t.Error("unknown id reported as downloaded") + } +} + +// TestLegacyMigrationKeepsBackupAndLegacyTable asserts the migration is +// recoverable: the original file is copied aside and downloads_legacy survives. +func TestLegacyMigrationKeepsBackupAndLegacyTable(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "downloads.db") + makeLegacyDB(t, path, "old-1") + + s, err := NewSQLite(path) + if err != nil { + t.Fatalf("NewSQLite() error = %v", err) + } + defer func() { _ = s.Close() }() + + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + backups := 0 + for _, e := range entries { + if strings.Contains(e.Name(), ".bak-") { + backups++ + } + } + if backups != 1 { + t.Errorf("expected exactly 1 backup file, found %d in %v", backups, entries) + } + + var n int + if err = s.db.QueryRow(`SELECT COUNT(1) FROM downloads_legacy`).Scan(&n); err != nil { + t.Fatalf("downloads_legacy was dropped: %v", err) + } + if n != 1 { + t.Errorf("downloads_legacy row count = %d, want 1", n) + } + + if v, err := s.userVersion(); err != nil || v != schemaVersion { + t.Errorf("user_version = %d (err %v), want %d", v, err, schemaVersion) + } +} + +// TestLegacyRowsDetectedOnReopen covers a database already migrated by the +// earlier, lossy code path: the sourceless rows are on disk but nothing in +// this process migrated them. +func TestLegacyRowsDetectedOnReopen(t *testing.T) { + ctx := context.Background() + path := filepath.Join(t.TempDir(), "downloads.db") + makeLegacyDB(t, path, "old-1") + + s, err := NewSQLite(path) + if err != nil { + t.Fatal(err) + } + _ = s.Close() + + reopened, err := NewSQLite(path) + if err != nil { + t.Fatal(err) + } + defer func() { _ = reopened.Close() }() + + if !reopened.hasLegacyRows { + t.Fatal("legacy rows not detected on reopen") + } + ok, err := reopened.IsDownloaded(ctx, "tidal", "old-1") + if err != nil { + t.Fatal(err) + } + if !ok { + t.Error("legacy row not honored after reopen") + } +} + +// TestConcurrentAccess exercises the mutex and connection handling under load. +func TestConcurrentAccess(t *testing.T) { + ctx := context.Background() + path := filepath.Join(t.TempDir(), "downloads.db") + s, err := NewSQLite(path) + if err != nil { + t.Fatal(err) + } + defer func() { _ = s.Close() }() + + var wg sync.WaitGroup + for i := 0; i < 32; i++ { + wg.Add(1) + go func(n int) { + defer wg.Done() + id := fmt.Sprintf("t%d", n) + if err := s.MarkDownloaded(ctx, "qobuz", id); err != nil { + t.Errorf("MarkDownloaded: %v", err) + return + } + if _, err := s.IsDownloaded(ctx, "qobuz", id); err != nil { + t.Errorf("IsDownloaded: %v", err) + } + if err := s.MarkFailed(ctx, "qobuz", "track", id); err != nil { + t.Errorf("MarkFailed: %v", err) + } + }(i) + } + wg.Wait() +} + +func TestSQLiteDSNPragmasApplied(t *testing.T) { + // A path containing '?' must not be parsed as query parameters. + dir := t.TempDir() + path := filepath.Join(dir, "weird?name#x.db") + + s, err := NewSQLite(path) + if err != nil { + t.Fatalf("NewSQLite(%q) = %v", path, err) + } + defer func() { _ = s.Close() }() + + var mode string + if err := s.db.QueryRow(`PRAGMA journal_mode`).Scan(&mode); err != nil { + t.Fatal(err) + } + if mode != "wal" { + t.Errorf("journal_mode = %q, want wal", mode) + } + + var timeout int + if err := s.db.QueryRow(`PRAGMA busy_timeout`).Scan(&timeout); err != nil { + t.Fatal(err) + } + if timeout != 10000 { + t.Errorf("busy_timeout = %d, want 10000", timeout) + } + + if err := s.MarkDownloaded(context.Background(), "qobuz", "a"); err != nil { + t.Fatalf("write to odd path failed: %v", err) + } +}