From 657fb221db002ffebbf2a8d603566191acb40d17 Mon Sep 17 00:00:00 2001 From: sentriz Date: Thu, 7 Sep 2023 00:19:04 +0100 Subject: [PATCH] feat(subsonic)!: drop support for guessed artist covers in filesystem it doesn't make sense with multi-artist support anymore gonic will use lastfm or an album cover instead Release-As: 0.16.0 --- db/migrations.go | 16 +++ db/model.go | 1 - scanner/scanner.go | 11 +- scanner/scanner_test.go | 18 --- server/ctrlsubsonic/handlers_by_tags.go | 31 ++--- server/ctrlsubsonic/handlers_raw.go | 6 +- server/ctrlsubsonic/spec/construct_by_tags.go | 4 +- .../testdata/test_get_album_list_random | 60 ++++----- .../testdata/test_get_album_list_two_random | 120 +++++++++--------- .../testdata/test_get_artist_id_one | 1 + .../testdata/test_get_artist_id_three | 1 + .../testdata/test_get_artist_id_two | 1 + .../testdata/test_get_artists_no_args | 21 ++- .../test_get_artists_with_music_folder_1 | 21 ++- .../test_get_artists_with_music_folder_2 | 21 ++- .../testdata/test_search_three_q_art | 21 ++- 16 files changed, 198 insertions(+), 156 deletions(-) diff --git a/db/migrations.go b/db/migrations.go index c5732e7..212cfd6 100644 --- a/db/migrations.go +++ b/db/migrations.go @@ -57,6 +57,7 @@ func (db *DB) Migrate(ctx MigrationContext) error { construct(ctx, "202304221528", migratePlaylistsToM3U), construct(ctx, "202305301718", migratePlayCountToLength), construct(ctx, "202307281628", migrateAlbumArtistsMany2Many), + construct(ctx, "202309070009", migrateDeleteArtistCoverField), } return gormigrate. @@ -589,3 +590,18 @@ func migrateAlbumArtistsMany2Many(tx *gorm.DB, _ MigrationContext) error { return nil } + +func migrateDeleteArtistCoverField(tx *gorm.DB, _ MigrationContext) error { + if !tx.Dialect().HasColumn("artists", "cover") { + return nil + } + + step := tx.Exec(` + ALTER TABLE artists DROP COLUMN cover; + `) + if err := step.Error; err != nil { + return fmt.Errorf("step drop: %w", err) + } + + return nil +} diff --git a/db/model.go b/db/model.go index 1a275f8..90b7048 100644 --- a/db/model.go +++ b/db/model.go @@ -49,7 +49,6 @@ type Artist struct { NameUDec string `sql:"default: null"` Albums []*Album `gorm:"many2many:album_artists"` AlbumCount int `sql:"-"` - Cover string `sql:"default: null"` ArtistStar *ArtistStar ArtistRating *ArtistRating AverageRating float64 `sql:"default: null"` diff --git a/scanner/scanner.go b/scanner/scanner.go index 6d748dc..ede0d64 100644 --- a/scanner/scanner.go +++ b/scanner/scanner.go @@ -324,7 +324,7 @@ func (s *Scanner) scanDir(tx *db.DB, c *Context, musicDir string, absPath string sort.Strings(tracks) for i, basename := range tracks { absPath := filepath.Join(musicDir, relPath, basename) - if err := s.populateTrackAndAlbumArtists(tx, c, i, &parent, &album, basename, absPath); err != nil { + if err := s.populateTrackAndAlbumArtists(tx, c, i, &album, basename, absPath); err != nil { return fmt.Errorf("populate track %q: %w", basename, err) } } @@ -332,7 +332,7 @@ func (s *Scanner) scanDir(tx *db.DB, c *Context, musicDir string, absPath string return nil } -func (s *Scanner) populateTrackAndAlbumArtists(tx *db.DB, c *Context, i int, parent, album *db.Album, basename string, absPath string) error { +func (s *Scanner) populateTrackAndAlbumArtists(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) @@ -364,7 +364,7 @@ func (s *Scanner) populateTrackAndAlbumArtists(tx *db.DB, c *Context, i int, par albumArtists := tags.MustAlbumArtists(trags) var albumArtistIDs []int for _, albumArtistName := range albumArtists { - albumArtist, err := populateArtist(tx, parent, albumArtistName) + albumArtist, err := populateArtist(tx, albumArtistName) if err != nil { return fmt.Errorf("populate album artist: %w", err) } @@ -461,13 +461,10 @@ func populateTrack(tx *db.DB, album *db.Album, track *db.Track, trags tags.Parse return nil } -func populateArtist(tx *db.DB, parent *db.Album, artistName string) (*db.Artist, error) { +func populateArtist(tx *db.DB, artistName string) (*db.Artist, error) { var update db.Artist update.Name = artistName update.NameUDec = decoded(artistName) - if parent.Cover != "" { - update.Cover = parent.Cover - } var artist db.Artist if err := tx.Where("name=?", artistName).Assign(update).FirstOrCreate(&artist).Error; err != nil { return nil, fmt.Errorf("find or create artist: %w", err) diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go index baaf5ef..a05a1bc 100644 --- a/scanner/scanner_test.go +++ b/scanner/scanner_test.go @@ -509,24 +509,6 @@ func TestSymlinkedSubdiscs(t *testing.T) { require.NotZero(info.ModTime()) // track resolves } -func TestArtistHasCover(t *testing.T) { - t.Parallel() - require := require.New(t) - m := mockfs.New(t) - - m.AddItemsWithCovers() - m.AddCover("artist-2/artist.png") - m.ScanAndClean() - - var artistWith db.Artist - require.NoError(m.DB().Where("name=?", "artist-2").First(&artistWith).Error) - require.Equal("artist.png", artistWith.Cover) - - var artistWithout db.Artist - require.NoError(m.DB().Where("name=?", "artist-0").First(&artistWithout).Error) - require.Equal("", artistWithout.Cover) -} - func TestTagErrors(t *testing.T) { t.Parallel() require := require.New(t) diff --git a/server/ctrlsubsonic/handlers_by_tags.go b/server/ctrlsubsonic/handlers_by_tags.go index 5f96d8a..d739575 100644 --- a/server/ctrlsubsonic/handlers_by_tags.go +++ b/server/ctrlsubsonic/handlers_by_tags.go @@ -319,11 +319,6 @@ func (c *Controller) ServeGetArtistInfoTwo(r *http.Request) *spec.Response { sub := spec.NewResponse() sub.ArtistInfoTwo = &spec.ArtistInfo{} - if artist.Cover != "" { - sub.ArtistInfoTwo.SmallImageURL = c.genArtistCoverURL(r, &artist, 64) - sub.ArtistInfoTwo.MediumImageURL = c.genArtistCoverURL(r, &artist, 126) - sub.ArtistInfoTwo.LargeImageURL = c.genArtistCoverURL(r, &artist, 256) - } apiKey, _ := c.DB.GetSetting("lastfm_api_key") if apiKey == "" { @@ -338,23 +333,15 @@ func (c *Controller) ServeGetArtistInfoTwo(r *http.Request) *spec.Response { sub.ArtistInfoTwo.MusicBrainzID = info.MBID sub.ArtistInfoTwo.LastFMURL = info.URL - if artist.Cover == "" { - for _, image := range info.Image { - switch image.Size { - case "small": - sub.ArtistInfoTwo.SmallImageURL = image.Text - case "medium": - sub.ArtistInfoTwo.MediumImageURL = image.Text - case "large": - sub.ArtistInfoTwo.LargeImageURL = image.Text - } - } - if url, _ := c.LastFMClient.StealArtistImage(info.URL); url != "" { - sub.ArtistInfoTwo.SmallImageURL = url - sub.ArtistInfoTwo.MediumImageURL = url - sub.ArtistInfoTwo.LargeImageURL = url - sub.ArtistInfoTwo.ArtistImageURL = url - } + sub.ArtistInfoTwo.SmallImageURL = c.genArtistCoverURL(r, &artist, 64) + sub.ArtistInfoTwo.MediumImageURL = c.genArtistCoverURL(r, &artist, 126) + sub.ArtistInfoTwo.LargeImageURL = c.genArtistCoverURL(r, &artist, 256) + + if url, _ := c.LastFMClient.StealArtistImage(info.URL); url != "" { + sub.ArtistInfoTwo.SmallImageURL = url + sub.ArtistInfoTwo.MediumImageURL = url + sub.ArtistInfoTwo.LargeImageURL = url + sub.ArtistInfoTwo.ArtistImageURL = url } count := params.GetOrInt("count", 20) diff --git a/server/ctrlsubsonic/handlers_raw.go b/server/ctrlsubsonic/handlers_raw.go index 58e13e5..1cf6364 100644 --- a/server/ctrlsubsonic/handlers_raw.go +++ b/server/ctrlsubsonic/handlers_raw.go @@ -149,10 +149,10 @@ func coverGetPathAlbum(dbc *db.DB, id int) (string, error) { func coverGetPathArtist(dbc *db.DB, id int) (string, error) { folder := &db.Album{} err := dbc.DB. - Select("parent.id, parent.root_dir, parent.left_path, parent.right_path, parent.cover"). - Joins("JOIN album_artists ON album_artists.album_id"). + Select("albums.id, albums.root_dir, albums.left_path, albums.right_path, albums.cover"). + Joins("JOIN album_artists ON album_artists.album_id=albums.id"). Where("album_artists.artist_id=?", id). - Joins("JOIN albums parent ON parent.id=albums.parent_id"). + Group("albums.id"). Find(folder). Error if err != nil { diff --git a/server/ctrlsubsonic/spec/construct_by_tags.go b/server/ctrlsubsonic/spec/construct_by_tags.go index d4ef075..1fe0f04 100644 --- a/server/ctrlsubsonic/spec/construct_by_tags.go +++ b/server/ctrlsubsonic/spec/construct_by_tags.go @@ -102,9 +102,7 @@ func NewArtistByTags(a *db.Artist) *Artist { Name: a.Name, AlbumCount: a.AlbumCount, AverageRating: formatRating(a.AverageRating), - } - if a.Cover != "" { - r.CoverID = a.SID() + CoverID: a.SID(), } if a.ArtistStar != nil { r.Starred = &a.ArtistStar.StarDate diff --git a/server/ctrlsubsonic/testdata/test_get_album_list_random b/server/ctrlsubsonic/testdata/test_get_album_list_random index e464835..aff162c 100644 --- a/server/ctrlsubsonic/testdata/test_get_album_list_random +++ b/server/ctrlsubsonic/testdata/test_get_album_list_random @@ -6,6 +6,19 @@ "serverVersion": "", "albumList": { "album": [ + { + "id": "al-8", + "coverArt": "al-8", + "artist": "artist-1", + "created": "2019-11-30T00:00:00Z", + "title": "album-1", + "album": "", + "parent": "al-6", + "isDir": true, + "name": "", + "songCount": 3, + "duration": 300 + }, { "id": "al-7", "coverArt": "al-7", @@ -19,6 +32,19 @@ "songCount": 3, "duration": 300 }, + { + "id": "al-9", + "coverArt": "al-9", + "artist": "artist-1", + "created": "2019-11-30T00:00:00Z", + "title": "album-2", + "album": "", + "parent": "al-6", + "isDir": true, + "name": "", + "songCount": 3, + "duration": 300 + }, { "id": "al-4", "coverArt": "al-4", @@ -58,19 +84,6 @@ "songCount": 3, "duration": 300 }, - { - "id": "al-13", - "coverArt": "al-13", - "artist": "artist-2", - "created": "2019-11-30T00:00:00Z", - "title": "album-2", - "album": "", - "parent": "al-10", - "isDir": true, - "name": "", - "songCount": 3, - "duration": 300 - }, { "id": "al-5", "coverArt": "al-5", @@ -85,26 +98,13 @@ "duration": 300 }, { - "id": "al-9", - "coverArt": "al-9", - "artist": "artist-1", + "id": "al-13", + "coverArt": "al-13", + "artist": "artist-2", "created": "2019-11-30T00:00:00Z", "title": "album-2", "album": "", - "parent": "al-6", - "isDir": true, - "name": "", - "songCount": 3, - "duration": 300 - }, - { - "id": "al-8", - "coverArt": "al-8", - "artist": "artist-1", - "created": "2019-11-30T00:00:00Z", - "title": "album-1", - "album": "", - "parent": "al-6", + "parent": "al-10", "isDir": true, "name": "", "songCount": 3, diff --git a/server/ctrlsubsonic/testdata/test_get_album_list_two_random b/server/ctrlsubsonic/testdata/test_get_album_list_two_random index a52a076..d36a5bd 100644 --- a/server/ctrlsubsonic/testdata/test_get_album_list_two_random +++ b/server/ctrlsubsonic/testdata/test_get_album_list_two_random @@ -6,51 +6,6 @@ "serverVersion": "", "albumList2": { "album": [ - { - "id": "al-4", - "coverArt": "al-4", - "artistId": "ar-1", - "artist": "artist-0", - "artistIds": ["ar-1"], - "artists": ["artist-0"], - "created": "2019-11-30T00:00:00Z", - "title": "", - "album": "", - "name": "album-1", - "songCount": 3, - "duration": 300, - "year": 2021 - }, - { - "id": "al-13", - "coverArt": "al-13", - "artistId": "ar-3", - "artist": "artist-2", - "artistIds": ["ar-3"], - "artists": ["artist-2"], - "created": "2019-11-30T00:00:00Z", - "title": "", - "album": "", - "name": "album-2", - "songCount": 3, - "duration": 300, - "year": 2021 - }, - { - "id": "al-3", - "coverArt": "al-3", - "artistId": "ar-1", - "artist": "artist-0", - "artistIds": ["ar-1"], - "artists": ["artist-0"], - "created": "2019-11-30T00:00:00Z", - "title": "", - "album": "", - "name": "album-0", - "songCount": 3, - "duration": 300, - "year": 2021 - }, { "id": "al-11", "coverArt": "al-11", @@ -66,21 +21,6 @@ "duration": 300, "year": 2021 }, - { - "id": "al-9", - "coverArt": "al-9", - "artistId": "ar-2", - "artist": "artist-1", - "artistIds": ["ar-2"], - "artists": ["artist-1"], - "created": "2019-11-30T00:00:00Z", - "title": "", - "album": "", - "name": "album-2", - "songCount": 3, - "duration": 300, - "year": 2021 - }, { "id": "al-8", "coverArt": "al-8", @@ -96,6 +36,36 @@ "duration": 300, "year": 2021 }, + { + "id": "al-3", + "coverArt": "al-3", + "artistId": "ar-1", + "artist": "artist-0", + "artistIds": ["ar-1"], + "artists": ["artist-0"], + "created": "2019-11-30T00:00:00Z", + "title": "", + "album": "", + "name": "album-0", + "songCount": 3, + "duration": 300, + "year": 2021 + }, + { + "id": "al-13", + "coverArt": "al-13", + "artistId": "ar-3", + "artist": "artist-2", + "artistIds": ["ar-3"], + "artists": ["artist-2"], + "created": "2019-11-30T00:00:00Z", + "title": "", + "album": "", + "name": "album-2", + "songCount": 3, + "duration": 300, + "year": 2021 + }, { "id": "al-12", "coverArt": "al-12", @@ -111,6 +81,21 @@ "duration": 300, "year": 2021 }, + { + "id": "al-9", + "coverArt": "al-9", + "artistId": "ar-2", + "artist": "artist-1", + "artistIds": ["ar-2"], + "artists": ["artist-1"], + "created": "2019-11-30T00:00:00Z", + "title": "", + "album": "", + "name": "album-2", + "songCount": 3, + "duration": 300, + "year": 2021 + }, { "id": "al-5", "coverArt": "al-5", @@ -140,6 +125,21 @@ "songCount": 3, "duration": 300, "year": 2021 + }, + { + "id": "al-4", + "coverArt": "al-4", + "artistId": "ar-1", + "artist": "artist-0", + "artistIds": ["ar-1"], + "artists": ["artist-0"], + "created": "2019-11-30T00:00:00Z", + "title": "", + "album": "", + "name": "album-1", + "songCount": 3, + "duration": 300, + "year": 2021 } ] } diff --git a/server/ctrlsubsonic/testdata/test_get_artist_id_one b/server/ctrlsubsonic/testdata/test_get_artist_id_one index 5cc1306..7135120 100644 --- a/server/ctrlsubsonic/testdata/test_get_artist_id_one +++ b/server/ctrlsubsonic/testdata/test_get_artist_id_one @@ -7,6 +7,7 @@ "artist": { "id": "ar-1", "name": "artist-0", + "coverArt": "ar-1", "albumCount": 3, "album": [ { diff --git a/server/ctrlsubsonic/testdata/test_get_artist_id_three b/server/ctrlsubsonic/testdata/test_get_artist_id_three index 5bf678c..860fa36 100644 --- a/server/ctrlsubsonic/testdata/test_get_artist_id_three +++ b/server/ctrlsubsonic/testdata/test_get_artist_id_three @@ -7,6 +7,7 @@ "artist": { "id": "ar-3", "name": "artist-2", + "coverArt": "ar-3", "albumCount": 3, "album": [ { diff --git a/server/ctrlsubsonic/testdata/test_get_artist_id_two b/server/ctrlsubsonic/testdata/test_get_artist_id_two index 46c48fe..e3de495 100644 --- a/server/ctrlsubsonic/testdata/test_get_artist_id_two +++ b/server/ctrlsubsonic/testdata/test_get_artist_id_two @@ -7,6 +7,7 @@ "artist": { "id": "ar-2", "name": "artist-1", + "coverArt": "ar-2", "albumCount": 3, "album": [ { diff --git a/server/ctrlsubsonic/testdata/test_get_artists_no_args b/server/ctrlsubsonic/testdata/test_get_artists_no_args index 88cfdfa..565e5fd 100644 --- a/server/ctrlsubsonic/testdata/test_get_artists_no_args +++ b/server/ctrlsubsonic/testdata/test_get_artists_no_args @@ -10,9 +10,24 @@ { "name": "a", "artist": [ - { "id": "ar-1", "name": "artist-0", "albumCount": 6 }, - { "id": "ar-2", "name": "artist-1", "albumCount": 6 }, - { "id": "ar-3", "name": "artist-2", "albumCount": 6 } + { + "id": "ar-1", + "name": "artist-0", + "coverArt": "ar-1", + "albumCount": 6 + }, + { + "id": "ar-2", + "name": "artist-1", + "coverArt": "ar-2", + "albumCount": 6 + }, + { + "id": "ar-3", + "name": "artist-2", + "coverArt": "ar-3", + "albumCount": 6 + } ] } ] diff --git a/server/ctrlsubsonic/testdata/test_get_artists_with_music_folder_1 b/server/ctrlsubsonic/testdata/test_get_artists_with_music_folder_1 index 0ee73fa..1cbbe07 100644 --- a/server/ctrlsubsonic/testdata/test_get_artists_with_music_folder_1 +++ b/server/ctrlsubsonic/testdata/test_get_artists_with_music_folder_1 @@ -10,9 +10,24 @@ { "name": "a", "artist": [ - { "id": "ar-1", "name": "artist-0", "albumCount": 3 }, - { "id": "ar-2", "name": "artist-1", "albumCount": 3 }, - { "id": "ar-3", "name": "artist-2", "albumCount": 3 } + { + "id": "ar-1", + "name": "artist-0", + "coverArt": "ar-1", + "albumCount": 3 + }, + { + "id": "ar-2", + "name": "artist-1", + "coverArt": "ar-2", + "albumCount": 3 + }, + { + "id": "ar-3", + "name": "artist-2", + "coverArt": "ar-3", + "albumCount": 3 + } ] } ] diff --git a/server/ctrlsubsonic/testdata/test_get_artists_with_music_folder_2 b/server/ctrlsubsonic/testdata/test_get_artists_with_music_folder_2 index 0ee73fa..1cbbe07 100644 --- a/server/ctrlsubsonic/testdata/test_get_artists_with_music_folder_2 +++ b/server/ctrlsubsonic/testdata/test_get_artists_with_music_folder_2 @@ -10,9 +10,24 @@ { "name": "a", "artist": [ - { "id": "ar-1", "name": "artist-0", "albumCount": 3 }, - { "id": "ar-2", "name": "artist-1", "albumCount": 3 }, - { "id": "ar-3", "name": "artist-2", "albumCount": 3 } + { + "id": "ar-1", + "name": "artist-0", + "coverArt": "ar-1", + "albumCount": 3 + }, + { + "id": "ar-2", + "name": "artist-1", + "coverArt": "ar-2", + "albumCount": 3 + }, + { + "id": "ar-3", + "name": "artist-2", + "coverArt": "ar-3", + "albumCount": 3 + } ] } ] diff --git a/server/ctrlsubsonic/testdata/test_search_three_q_art b/server/ctrlsubsonic/testdata/test_search_three_q_art index 33a55f6..c4940ce 100644 --- a/server/ctrlsubsonic/testdata/test_search_three_q_art +++ b/server/ctrlsubsonic/testdata/test_search_three_q_art @@ -6,9 +6,24 @@ "serverVersion": "", "searchResult3": { "artist": [ - { "id": "ar-1", "name": "artist-0", "albumCount": 3 }, - { "id": "ar-2", "name": "artist-1", "albumCount": 3 }, - { "id": "ar-3", "name": "artist-2", "albumCount": 3 } + { + "id": "ar-1", + "name": "artist-0", + "coverArt": "ar-1", + "albumCount": 3 + }, + { + "id": "ar-2", + "name": "artist-1", + "coverArt": "ar-2", + "albumCount": 3 + }, + { + "id": "ar-3", + "name": "artist-2", + "coverArt": "ar-3", + "albumCount": 3 + } ] } }