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
2023-09-07 00:19:04 +01:00
parent 3ac77823c3
commit 657fb221db
16 changed files with 198 additions and 156 deletions
--- 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
+}
--- 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"`
--- 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)
--- 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)
--- 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,24 +333,16 @@ 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
-			}
-		}
+	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)
 	inclNotPresent := params.GetOrBool("includeNotPresent", false)
--- 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 {
--- 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
--- 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,
--- 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
        }
      ]
    }
--- 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": [
        {
--- 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": [
        {
--- 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": [
        {
--- 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
+            }
          ]
        }
      ]
--- 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
+            }
          ]
        }
      ]
--- 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
+            }
          ]
        }
      ]
--- 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
+        }
      ]
    }
  }