fix source-aware download tracking and filter/path regressions

Make download dedupe source-specific to prevent cross-provider ID collisions. Also correct non-remaster filtering, avoid FLAC tagging on non-FLAC files, and use album IDs for singles folder templating.
This commit is contained in:
2026-04-19 21:25:04 +02:00
parent 97e8b758b3
commit d4643d877e
6 changed files with 157 additions and 21 deletions

View File

@@ -403,7 +403,7 @@ func main() {
for _, idx := range selection { for _, idx := range selection {
item := results[idx] item := results[idx]
if !sopts.ignoreDB { if !sopts.ignoreDB {
already, checkErr := mainApp.Store.IsDownloaded(ctx, item.ID) already, checkErr := mainApp.Store.IsDownloaded(ctx, source, item.ID)
if checkErr == nil && already { if checkErr == nil && already {
skippedDownloaded++ skippedDownloaded++
fmt.Printf("skip (already downloaded): id=%s | %s\n", item.ID, item.Title) fmt.Printf("skip (already downloaded): id=%s | %s\n", item.ID, item.Title)
@@ -457,7 +457,7 @@ func main() {
for _, idx := range selection { for _, idx := range selection {
item := results[idx] item := results[idx]
if !sopts.ignoreDB { if !sopts.ignoreDB {
already, checkErr := mainApp.Store.IsDownloaded(ctx, item.ID) already, checkErr := mainApp.Store.IsDownloaded(ctx, source, item.ID)
if checkErr == nil && already { if checkErr == nil && already {
skippedDownloaded++ skippedDownloaded++
fmt.Printf("skip (already downloaded): id=%s | %s\n", item.ID, item.Title) fmt.Printf("skip (already downloaded): id=%s | %s\n", item.ID, item.Title)

View File

@@ -258,7 +258,7 @@ func applyQobuzArtistFilters(artistName string, albums []collectionAlbum, filt c
if filt.NonRemaster { if filt.NonRemaster {
tmp := out[:0] tmp := out[:0]
for _, a := range out { for _, a := range out {
if qobuzRemasterRe.MatchString(a.Title) { if !qobuzRemasterRe.MatchString(a.Title) {
tmp = append(tmp, a) tmp = append(tmp, a)
} }
} }
@@ -592,7 +592,7 @@ func (m *Main) ripPlaylist(ctx context.Context, p provider.Client, source, playl
} }
func (m *Main) ripTrack(ctx context.Context, p provider.Client, source, id, fallbackTitle string, opts ripTrackOptions) error { func (m *Main) ripTrack(ctx context.Context, p provider.Client, source, id, fallbackTitle string, opts ripTrackOptions) error {
alreadyDownloaded, err := m.Store.IsDownloaded(ctx, id) alreadyDownloaded, err := m.Store.IsDownloaded(ctx, source, id)
if err == nil && alreadyDownloaded { if err == nil && alreadyDownloaded {
if m.IgnoreDB { if m.IgnoreDB {
alreadyDownloaded = false alreadyDownloaded = false
@@ -680,8 +680,10 @@ func (m *Main) ripTrack(ctx context.Context, p provider.Client, source, id, fall
coverPath = tag.CoverPathForTrack(outPath, opts.albumFolder) coverPath = tag.CoverPathForTrack(outPath, opts.albumFolder)
} }
} }
if err = m.Tagger.TagFLAC(outPath, tagMeta, coverPath); err != nil { if strings.EqualFold(filepath.Ext(outPath), ".flac") {
m.logf("warning: tag failed for %s: %v\n", filepath.Base(outPath), err) if err = m.Tagger.TagFLAC(outPath, tagMeta, coverPath); err != nil {
m.logf("warning: tag failed for %s: %v\n", filepath.Base(outPath), err)
}
} }
if m.Config.Session.Conversion.Enabled { if m.Config.Session.Conversion.Enabled {
@@ -693,7 +695,7 @@ func (m *Main) ripTrack(ctx context.Context, p provider.Client, source, id, fall
outPath = convertedPath outPath = convertedPath
} }
return m.Store.MarkDownloaded(ctx, id) return m.Store.MarkDownloaded(ctx, source, id)
} }
func (m *Main) qualityForSource(source string) int { func (m *Main) qualityForSource(source string) int {
@@ -772,6 +774,10 @@ func (m *Main) trackOutputPath(source, id, title, ext string, trackMeta map[stri
if albumFolder == "" && m.Config.Session.Filepaths.AddSinglesToFolder { if albumFolder == "" && m.Config.Session.Filepaths.AddSinglesToFolder {
albumTitle := nestedString(trackMeta, "album", "title") albumTitle := nestedString(trackMeta, "album", "title")
albumID := nestedString(trackMeta, "album", "id")
if albumID == "" {
albumID = id
}
albumArtist := nestedString(trackMeta, "album", "artist", "name") albumArtist := nestedString(trackMeta, "album", "artist", "name")
if albumArtist == "" { if albumArtist == "" {
albumArtist = nestedString(trackMeta, "performer", "name") albumArtist = nestedString(trackMeta, "performer", "name")
@@ -780,7 +786,7 @@ func (m *Main) trackOutputPath(source, id, title, ext string, trackMeta map[stri
if albumYear == "Unknown" { if albumYear == "Unknown" {
albumYear = naming.YearFromDate(stringFromAny(trackMeta["release_date"])) albumYear = naming.YearFromDate(stringFromAny(trackMeta["release_date"]))
} }
albumFolder = m.albumFolderPath(source, id, albumTitle, albumArtist, albumYear, intFromAny(trackMeta["maximum_bit_depth"]), stringFromAny(trackMeta["maximum_sampling_rate"])) albumFolder = m.albumFolderPath(source, albumID, albumTitle, albumArtist, albumYear, intFromAny(trackMeta["maximum_bit_depth"]), stringFromAny(trackMeta["maximum_sampling_rate"]))
} }
if albumFolder != "" { if albumFolder != "" {
base = albumFolder base = albumFolder

View File

@@ -189,7 +189,7 @@ func TestTrackRipPipeline(t *testing.T) {
t.Fatalf("expected downloaded file: %v", err) t.Fatalf("expected downloaded file: %v", err)
} }
ok, err := sqlite.IsDownloaded(ctx, "19512574") ok, err := sqlite.IsDownloaded(ctx, "qobuz", "19512574")
if err != nil { if err != nil {
t.Fatalf("IsDownloaded() error = %v", err) t.Fatalf("IsDownloaded() error = %v", err)
} }
@@ -316,3 +316,44 @@ func TestApplyQobuzArtistFiltersRepeats(t *testing.T) {
t.Fatalf("unexpected winners: %+v", ids) t.Fatalf("unexpected winners: %+v", ids)
} }
} }
func TestApplyQobuzArtistFiltersNonRemaster(t *testing.T) {
albums := []collectionAlbum{
{ID: "rm", Title: "Album X (Remastered)"},
{ID: "orig", Title: "Album X"},
}
filtered := applyQobuzArtistFilters("artist", albums, config.QobuzDiscographyFilterConfig{NonRemaster: true})
if len(filtered) != 1 {
t.Fatalf("len(filtered)=%d want 1", len(filtered))
}
if filtered[0].ID != "orig" {
t.Fatalf("unexpected album kept: %+v", filtered[0])
}
}
func TestTrackOutputPathSinglesUsesAlbumID(t *testing.T) {
tmp := t.TempDir()
d := config.DefaultConfigData()
d.Downloads.Folder = tmp
d.Downloads.SourceSubdirectories = false
d.Filepaths.AddSinglesToFolder = true
d.Filepaths.FolderFormat = "{id}"
d.Filepaths.TrackFormat = "{title}"
d.Filepaths.RestrictCharacters = false
m := &Main{Config: &config.Config{File: d, Session: d}}
meta := map[string]any{
"album": map[string]any{
"id": "album-123",
"title": "Album",
"artist": map[string]any{"name": "Artist"},
},
"performer": map[string]any{"name": "Artist"},
}
out := m.trackOutputPath("qobuz", "track-999", "Song", "flac", meta, "", 0)
if got, want := filepath.Dir(out), filepath.Join(tmp, "album-123"); got != want {
t.Fatalf("trackOutputPath() dir=%q want %q", got, want)
}
}

View File

@@ -28,8 +28,11 @@ func NewSQLite(path string) (*SQLite, error) {
} }
func (s *SQLite) init() error { func (s *SQLite) init() error {
if err := s.ensureDownloadsSchema(); err != nil {
return err
}
queries := []string{ queries := []string{
`CREATE TABLE IF NOT EXISTS downloads (id TEXT PRIMARY KEY)`,
`CREATE TABLE IF NOT EXISTS failed_downloads ( `CREATE TABLE IF NOT EXISTS failed_downloads (
source TEXT NOT NULL, source TEXT NOT NULL,
media_type TEXT NOT NULL, media_type TEXT NOT NULL,
@@ -47,23 +50,101 @@ func (s *SQLite) init() error {
return nil return nil
} }
func (s *SQLite) IsDownloaded(ctx context.Context, id string) (bool, error) { func (s *SQLite) ensureDownloadsSchema() error {
rows, err := s.db.Query(`PRAGMA table_info(downloads)`)
if err != nil {
return err
}
defer rows.Close()
hasTable := false
hasSource := false
for rows.Next() {
hasTable = true
var (
cid int
name string
colType string
notNull int
defaultV sql.NullString
pkOrdinal int
)
if err = rows.Scan(&cid, &name, &colType, &notNull, &defaultV, &pkOrdinal); err != nil {
return err
}
if name == "source" {
hasSource = true
}
}
if err = rows.Err(); err != nil {
return err
}
if !hasTable {
_, err = s.db.Exec(`CREATE TABLE downloads (
source TEXT NOT NULL,
id TEXT NOT NULL,
PRIMARY KEY (source, id)
)`)
return err
}
if hasSource {
_, err = s.db.Exec(`CREATE UNIQUE INDEX IF NOT EXISTS idx_downloads_source_id ON downloads(source, id)`)
return err
}
tx, err := s.db.Begin()
if err != nil {
return err
}
defer func() {
if err != nil {
_ = tx.Rollback()
}
}()
if _, err = tx.Exec(`DROP TABLE IF EXISTS downloads_legacy`); err != nil {
return err
}
if _, err = tx.Exec(`ALTER TABLE downloads RENAME TO downloads_legacy`); err != nil {
return err
}
if _, err = tx.Exec(`CREATE TABLE downloads (
source TEXT NOT NULL,
id TEXT NOT NULL,
PRIMARY KEY (source, id)
)`); err != nil {
return err
}
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 {
return err
}
err = tx.Commit()
return err
}
func (s *SQLite) IsDownloaded(ctx context.Context, source, id string) (bool, error) {
s.mu.Lock() s.mu.Lock()
defer s.mu.Unlock() defer s.mu.Unlock()
var count int var count int
err := s.db.QueryRowContext(ctx, `SELECT COUNT(1) FROM downloads WHERE id = ?`, id).Scan(&count) err := s.db.QueryRowContext(ctx, `SELECT COUNT(1) FROM downloads WHERE source = ? AND id = ?`, source, id).Scan(&count)
if err != nil { if err != nil {
return false, err return false, err
} }
return count > 0, nil return count > 0, nil
} }
func (s *SQLite) MarkDownloaded(ctx context.Context, id string) error { func (s *SQLite) MarkDownloaded(ctx context.Context, source, id string) error {
s.mu.Lock() s.mu.Lock()
defer s.mu.Unlock() defer s.mu.Unlock()
_, err := s.db.ExecContext(ctx, `INSERT OR IGNORE INTO downloads(id) VALUES (?)`, id) _, err := s.db.ExecContext(ctx, `INSERT OR IGNORE INTO downloads(source, id) VALUES (?, ?)`, source, id)
return err return err
} }

View File

@@ -16,7 +16,7 @@ func TestSQLiteStore(t *testing.T) {
} }
defer func() { _ = s.Close() }() defer func() { _ = s.Close() }()
ok, err := s.IsDownloaded(ctx, "a") ok, err := s.IsDownloaded(ctx, "qobuz", "a")
if err != nil { if err != nil {
t.Fatalf("IsDownloaded() error = %v", err) t.Fatalf("IsDownloaded() error = %v", err)
} }
@@ -24,11 +24,11 @@ func TestSQLiteStore(t *testing.T) {
t.Fatalf("expected not downloaded") t.Fatalf("expected not downloaded")
} }
if err = s.MarkDownloaded(ctx, "a"); err != nil { if err = s.MarkDownloaded(ctx, "qobuz", "a"); err != nil {
t.Fatalf("MarkDownloaded() error = %v", err) t.Fatalf("MarkDownloaded() error = %v", err)
} }
ok, err = s.IsDownloaded(ctx, "a") ok, err = s.IsDownloaded(ctx, "qobuz", "a")
if err != nil { if err != nil {
t.Fatalf("IsDownloaded() error = %v", err) t.Fatalf("IsDownloaded() error = %v", err)
} }
@@ -36,6 +36,14 @@ func TestSQLiteStore(t *testing.T) {
t.Fatalf("expected downloaded") t.Fatalf("expected downloaded")
} }
ok, err = s.IsDownloaded(ctx, "tidal", "a")
if err != nil {
t.Fatalf("IsDownloaded() error = %v", err)
}
if ok {
t.Fatalf("expected source-specific download tracking")
}
if err = s.MarkFailed(ctx, "qobuz", "track", "1"); err != nil { if err = s.MarkFailed(ctx, "qobuz", "track", "1"); err != nil {
t.Fatalf("MarkFailed() error = %v", err) t.Fatalf("MarkFailed() error = %v", err)
} }

View File

@@ -3,8 +3,8 @@ package store
import "context" import "context"
type Database interface { type Database interface {
IsDownloaded(ctx context.Context, id string) (bool, error) IsDownloaded(ctx context.Context, source, id string) (bool, error)
MarkDownloaded(ctx context.Context, id string) error MarkDownloaded(ctx context.Context, source, id string) error
MarkFailed(ctx context.Context, source, mediaType, id string) error MarkFailed(ctx context.Context, source, mediaType, id string) error
Close() error Close() error
} }
@@ -15,11 +15,11 @@ func NewDummy() *Dummy {
return &Dummy{} return &Dummy{}
} }
func (d *Dummy) IsDownloaded(context.Context, string) (bool, error) { func (d *Dummy) IsDownloaded(context.Context, string, string) (bool, error) {
return false, nil return false, nil
} }
func (d *Dummy) MarkDownloaded(context.Context, string) error { func (d *Dummy) MarkDownloaded(context.Context, string, string) error {
return nil return nil
} }