feat: store and expose individual track artists

a
sentriz · Oct 28, 2023 · 31f6da9 · 31f6da9
1 parent a34ac03
commit 31f6da9
Show file tree

Hide file tree

Showing 24 changed files with 176 additions and 64 deletions.
diff --git a/README.md b/README.md
@@ -77,6 +77,7 @@ password can then be changed from the web interface
 | `GONIC_PODCAST_PURGE_AGE`        | `-podcast-purge-age`        | **optional** age (in days) to purge podcast episodes if not accessed                                                                                                                                                                                                              |
 | `GONIC_EXCLUDE_PATTERN`          | `-exclude-pattern`          | **optional** files matching this regex pattern will not be imported                                                                                                                                                                                                               |
 | `GONIC_MULTI_VALUE_GENRE`        | `-multi-value-genre`        | **optional** setting for multi-valued genre tags when scanning ([see more](#multi-valued-tags))                                                                                                                                                                                   |
+| `GONIC_MULTI_VALUE_ARTIST`       | `-multi-value-artist`       | **optional** setting for multi-valued artist tags when scanning ([see more](#multi-valued-tags))                                                                                                                                                                                  |
 | `GONIC_MULTI_VALUE_ALBUM_ARTIST` | `-multi-value-album-artist` | **optional** setting for multi-valued album artist tags when scanning ([see more](#multi-valued-tags))                                                                                                                                                                            |
 | `GONIC_EXPVAR`                   | `-expvar`                   | **optional** enable the /debug/vars endpoint (exposes useful debugging attributes as well as database stats)                                                                                                                                                                      |
 

diff --git a/cmd/gonic/gonic.go b/cmd/gonic/gonic.go
@@ -81,8 +81,9 @@ func main() {
 
 	confExcludePatterns := set.String("exclude-pattern", "", "regex pattern to exclude files from scan (optional)")
 
-	var confMultiValueGenre, confMultiValueAlbumArtist multiValueSetting
+	var confMultiValueGenre, confMultiValueArtist, confMultiValueAlbumArtist multiValueSetting
 	set.Var(&confMultiValueGenre, "multi-value-genre", "setting for mutli-valued genre scanning (optional)")
+	set.Var(&confMultiValueArtist, "multi-value-artist", "setting for mutli-valued track artist scanning (optional)")
 	set.Var(&confMultiValueAlbumArtist, "multi-value-album-artist", "setting for mutli-valued album artist scanning (optional)")
 
 	confExpvar := set.Bool("expvar", false, "enable the /debug/vars endpoint (optional)")
@@ -184,6 +185,7 @@ func main() {
 		dbc,
 		map[scanner.Tag]scanner.MultiValueSetting{
 			scanner.Genre:       scanner.MultiValueSetting(confMultiValueGenre),
+			scanner.Artist:      scanner.MultiValueSetting(confMultiValueArtist),
 			scanner.AlbumArtist: scanner.MultiValueSetting(confMultiValueAlbumArtist),
 		},
 		tagReader,

diff --git a/db/db.go b/db/db.go
@@ -201,17 +201,18 @@ type Track struct {
 	Filename       string `gorm:"not null; unique_index:idx_folder_filename" sql:"default: null"`
 	FilenameUDec   string `sql:"default: null"`
 	Album          *Album
-	AlbumID        int      `gorm:"not null; unique_index:idx_folder_filename" sql:"default: null; type:int REFERENCES albums(id) ON DELETE CASCADE"`
-	Genres         []*Genre `gorm:"many2many:track_genres"`
-	Size           int      `sql:"default: null"`
-	Length         int      `sql:"default: null"`
-	Bitrate        int      `sql:"default: null"`
-	TagTitle       string   `sql:"default: null"`
-	TagTitleUDec   string   `sql:"default: null"`
-	TagTrackArtist string   `sql:"default: null"`
-	TagTrackNumber int      `sql:"default: null"`
-	TagDiscNumber  int      `sql:"default: null"`
-	TagBrainzID    string   `sql:"default: null"`
+	AlbumID        int       `gorm:"not null; unique_index:idx_folder_filename" sql:"default: null; type:int REFERENCES albums(id) ON DELETE CASCADE"`
+	Artists        []*Artist `gorm:"many2many:track_artists"`
+	Genres         []*Genre  `gorm:"many2many:track_genres"`
+	Size           int       `sql:"default: null"`
+	Length         int       `sql:"default: null"`
+	Bitrate        int       `sql:"default: null"`
+	TagTitle       string    `sql:"default: null"`
+	TagTitleUDec   string    `sql:"default: null"`
+	TagTrackArtist string    `sql:"default: null"`
+	TagTrackNumber int       `sql:"default: null"`
+	TagDiscNumber  int       `sql:"default: null"`
+	TagBrainzID    string    `sql:"default: null"`
 	TrackStar      *TrackStar
 	TrackRating    *TrackRating
 	AverageRating  float64 `sql:"default: null"`
@@ -372,6 +373,13 @@ type AlbumArtist struct {
 	ArtistID int `gorm:"not null; unique_index:idx_album_id_artist_id" sql:"default: null; type:int REFERENCES artists(id) ON DELETE CASCADE"`
 }
 
+type TrackArtist struct {
+	Track    *Track
+	TrackID  int `gorm:"not null; unique_index:idx_track_id_artist_id" sql:"default: null; type:int REFERENCES tracks(id) ON DELETE CASCADE"`
+	Artist   *Artist
+	ArtistID int `gorm:"not null; unique_index:idx_track_id_artist_id" sql:"default: null; type:int REFERENCES artists(id) ON DELETE CASCADE"`
+}
+
 type TrackGenre struct {
 	Track   *Track
 	TrackID int `gorm:"not null; unique_index:idx_track_id_genre_id" sql:"default: null; type:int REFERENCES tracks(id) ON DELETE CASCADE"`

diff --git a/db/migrations.go b/db/migrations.go
@@ -67,6 +67,7 @@ func (db *DB) Migrate(ctx MigrationContext) error {
 		construct(ctx, "202309131743", migrateArtistInfo),
 		construct(ctx, "202309161411", migratePlaylistsPaths),
 		construct(ctx, "202310252205", migrateAlbumTagArtistString),
+		construct(ctx, "202310281803", migrateTrackArtists),
 	}
 
 	return gormigrate.
@@ -734,3 +735,12 @@ func backupDBPre016(tx *gorm.DB, ctx MigrationContext) error {
 func migrateAlbumTagArtistString(tx *gorm.DB, _ MigrationContext) error {
 	return tx.AutoMigrate(Album{}).Error
 }
+
+func migrateTrackArtists(tx *gorm.DB, _ MigrationContext) error {
+	// gorms seems to want to create the table automatically without ON DELETE rules
+	step := tx.DropTableIfExists(TrackArtist{})
+	if err := step.Error; err != nil {
+		return fmt.Errorf("step drop prev: %w", err)
+	}
+	return tx.AutoMigrate(TrackArtist{}).Error
+}
diff --git a/mockfs/mockfs.go b/mockfs/mockfs.go
@@ -339,6 +339,7 @@ func (m *tagReader) Read(absPath string) (tagcommon.Info, error) {
 type TagInfo struct {
 	RawTitle        string
 	RawArtist       string
+	RawArtists      []string
 	RawAlbum        string
 	RawAlbumArtist  string
 	RawAlbumArtists []string
@@ -351,6 +352,7 @@ type TagInfo struct {
 func (i *TagInfo) Title() string          { return i.RawTitle }
 func (i *TagInfo) BrainzID() string       { return "" }
 func (i *TagInfo) Artist() string         { return i.RawArtist }
+func (i *TagInfo) Artists() []string      { return i.RawArtists }
 func (i *TagInfo) Album() string          { return i.RawAlbum }
 func (i *TagInfo) AlbumArtist() string    { return i.RawAlbumArtist }
 func (i *TagInfo) AlbumArtists() []string { return i.RawAlbumArtists }

diff --git a/scanner/scanner.go b/scanner/scanner.go
@@ -296,15 +296,15 @@ func (s *Scanner) scanDir(tx *db.DB, c *Context, absPath string) error {
 	sort.Strings(tracks)
 	for i, basename := range tracks {
 		absPath := filepath.Join(musicDir, relPath, basename)
-		if err := s.populateTrackAndAlbumArtists(tx, c, i, &album, basename, absPath); err != nil {
+		if err := s.populateTrackAndArtists(tx, c, i, &album, basename, absPath); err != nil {
 			return fmt.Errorf("populate track %q: %w", basename, err)
 		}
 	}
 
 	return nil
 }
 
-func (s *Scanner) populateTrackAndAlbumArtists(tx *db.DB, c *Context, i int, album *db.Album, basename string, absPath string) error {
+func (s *Scanner) populateTrackAndArtists(tx *db.DB, c *Context, i int, album *db.Album, basename string, absPath string) error {
 	stat, err := os.Stat(absPath)
 	if err != nil {
 		return fmt.Errorf("stating %q: %w", basename, err)
@@ -362,6 +362,19 @@ func (s *Scanner) populateTrackAndAlbumArtists(tx *db.DB, c *Context, i int, alb
 		return fmt.Errorf("populate track genres: %w", err)
 	}
 
+	trackArtistNames := parseMulti(trags, s.multiValueSettings[Artist], tagcommon.MustArtists, tagcommon.MustArtist)
+	var trackArtistIDs []int
+	for _, trackArtistName := range trackArtistNames {
+		trackArtist, err := populateArtist(tx, trackArtistName)
+		if err != nil {
+			return fmt.Errorf("populate track artist: %w", err)
+		}
+		trackArtistIDs = append(trackArtistIDs, trackArtist.ID)
+	}
+	if err := populateTrackArtists(tx, &track, trackArtistIDs); err != nil {
+		return fmt.Errorf("populate track artists: %w", err)
+	}
+
 	c.seenTracks[track.ID] = struct{}{}
 	c.seenTracksNew++
 
@@ -498,6 +511,17 @@ func populateAlbumArtists(tx *db.DB, album *db.Album, albumArtistIDs []int) erro
 	return nil
 }
 
+func populateTrackArtists(tx *db.DB, track *db.Track, trackArtistIDs []int) error {
+	if err := tx.Where("track_id=?", track.ID).Delete(db.TrackArtist{}).Error; err != nil {
+		return fmt.Errorf("delete old track artists: %w", err)
+	}
+
+	if err := tx.InsertBulkLeftMany("track_artists", []string{"track_id", "artist_id"}, track.ID, trackArtistIDs); err != nil {
+		return fmt.Errorf("insert bulk track artists: %w", err)
+	}
+	return nil
+}
+
 func (s *Scanner) cleanTracks(c *Context) error {
 	start := time.Now()
 	defer func() { log.Printf("finished clean tracks in %s, %d removed", durSince(start), c.TracksMissing()) }()
@@ -546,15 +570,15 @@ func (s *Scanner) cleanArtists(c *Context) error {
 	start := time.Now()
 	defer func() { log.Printf("finished clean artists in %s, %d removed", durSince(start), c.ArtistsMissing()) }()
 
-	sub := s.db.
-		Select("artists.id").
-		Model(&db.Artist{}).
-		Joins("LEFT JOIN album_artists ON album_artists.artist_id=artists.id").
-		Where("album_artists.artist_id IS NULL").
-		SubQuery()
-	q := s.db.
-		Where("artists.id IN ?", sub).
-		Delete(&db.Artist{})
+	// gorm doesn't seem to support subqueries without parens for UNION
+	q := s.db.Exec(`
+		DELETE FROM artists
+		WHERE id NOT IN (
+			SELECT artist_id FROM track_artists
+			UNION
+			SELECT artist_id FROM album_artists
+		)
+    `)
 	if err := q.Error; err != nil {
 		return err
 	}
@@ -654,6 +678,7 @@ type Tag uint8
 
 const (
 	Genre Tag = iota
+	Artist
 	AlbumArtist
 )
 

diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go
@@ -562,7 +562,7 @@ func TestCompilationAlbumWithoutAlbumArtist(t *testing.T) {
 	assert.Equal(t, 5, trackCount)
 
 	var artists []*db.Artist
-	assert.NoError(t, m.DB().Find(&artists).Error)
+	assert.NoError(t, m.DB().Joins("JOIN album_artists ON album_artists.artist_id=artists.id").Group("artists.id").Find(&artists).Error)
 	assert.Equal(t, 1, len(artists))             // we only have one album artist
 	assert.Equal(t, "artist 0", artists[0].Name) // it came from the first track's fallback to artist tag
 
@@ -656,7 +656,7 @@ func TestMultiArtistSupport(t *testing.T) {
 	m.ScanAndClean()
 
 	var artists []*db.Artist
-	assert.NoError(t, m.DB().Find(&artists).Error)
+	assert.NoError(t, m.DB().Joins("JOIN album_artists ON album_artists.artist_id=artists.id").Group("artists.id").Find(&artists).Error)
 	assert.Len(t, artists, 3) // alan, liz, mercury
 
 	var albumArtists []*db.AlbumArtist
@@ -695,7 +695,7 @@ func TestMultiArtistSupport(t *testing.T) {
 
 	m.ScanAndClean()
 
-	assert.NoError(t, m.DB().Find(&artists).Error)
+	assert.NoError(t, m.DB().Joins("JOIN album_artists ON album_artists.artist_id=artists.id").Group("artists.id").Find(&artists).Error)
 	assert.Len(t, artists, 2) // alan, liz
 
 	assert.NoError(t, m.DB().Find(&albumArtists).Error)
@@ -745,7 +745,7 @@ func TestMultiArtistPreload(t *testing.T) {
 	}
 
 	var artists []*db.Artist
-	assert.NoError(t, m.DB().Preload("Albums").Find(&artists).Error)
+	assert.NoError(t, m.DB().Preload("Albums").Joins("JOIN album_artists ON album_artists.artist_id=artists.id").Group("artists.id").Find(&artists).Error)
 	assert.Equal(t, 3, len(artists))
 
 	for _, artist := range artists {

diff --git a/...estdata/fuzz/FuzzScanner/22023bdee30f39396809ddfa903114a127d16fbf11dbd52256541a17c9f60b07 b/...estdata/fuzz/FuzzScanner/22023bdee30f39396809ddfa903114a127d16fbf11dbd52256541a17c9f60b07
diff --git a/...estdata/fuzz/FuzzScanner/50640a7a67794863f262b749f288aff8610f2cac65b7cf65b8708eb0ea104519 b/...estdata/fuzz/FuzzScanner/50640a7a67794863f262b749f288aff8610f2cac65b7cf65b8708eb0ea104519
diff --git a/...estdata/fuzz/FuzzScanner/b1f8e3fd8087149215418297f28d6e1e447100179409d69a827c6651370d96cc b/...estdata/fuzz/FuzzScanner/b1f8e3fd8087149215418297f28d6e1e447100179409d69a827c6651370d96cc
diff --git a/...estdata/fuzz/FuzzScanner/caf81e9797b19c76c1fc4dbf537d4d81f389524539f402d13aa01f93a65ac7e9 b/...estdata/fuzz/FuzzScanner/caf81e9797b19c76c1fc4dbf537d4d81f389524539f402d13aa01f93a65ac7e9