Remove old refresh code

db/migration/20210418232815_fix_album_comments.go

@@ -4,6 +4,7 @@ import (
 	"database/sql"
 	"strings"
 
+	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/log"
 	"github.com/pressly/goose"
 )
@@ -13,9 +14,9 @@ func init() {
 }
 
 func upFixAlbumComments(tx *sql.Tx) error {
-	const zwsp = string('\u200b')
+	//nolint:gosec
 	rows, err := tx.Query(`
-	SELECT album.id, group_concat(media_file.comment, '` + zwsp + `') FROM album, media_file WHERE media_file.album_id = album.id GROUP BY album.id;
+	SELECT album.id, group_concat(media_file.comment, '` + consts.Zwsp + `') FROM album, media_file WHERE media_file.album_id = album.id GROUP BY album.id;
 	   `)
 	if err != nil {
 		return err
@@ -37,7 +38,7 @@ func upFixAlbumComments(tx *sql.Tx) error {
 		if !comments.Valid {
 			continue
 		}
-		comment := getComment(comments.String, zwsp)
+		comment := getComment(comments.String, consts.Zwsp)
 		_, err = stmt.Exec(comment, id)
 
 		if err != nil {

model/mediafile_test.go

@@ -69,6 +69,7 @@ var _ = Describe("MediaFiles", func() {
 				Expect(album.CreatedAt).To(Equal(t("2022-12-19 08:30")))
 			})
 		})
+
 		When("we have multiple songs", func() {
 			BeforeEach(func() {
 				mfs = MediaFiles{
@@ -86,6 +87,18 @@ var _ = Describe("MediaFiles", func() {
 				Expect(album.UpdatedAt).To(Equal(t("2022-12-19 09:45")))
 				Expect(album.CreatedAt).To(Equal(t("2022-12-19 07:30")))
 			})
+			Context("MinYear", func() {
+				It("returns 0 when all values are 0", func() {
+					mfs = MediaFiles{{Year: 0}, {Year: 0}, {Year: 0}}
+					a := mfs.ToAlbum()
+					Expect(a.MinYear).To(Equal(0))
+				})
+				It("returns the smallest value from the list, not counting 0", func() {
+					mfs = MediaFiles{{Year: 2000}, {Year: 0}, {Year: 1999}}
+					a := mfs.ToAlbum()
+					Expect(a.MinYear).To(Equal(1999))
+				})
+			})
 		})
 	})
 	Context("Calculated attributes", func() {

persistence/album_repository.go

@@ -3,18 +3,12 @@ package persistence
 import (
 	"context"
 	"fmt"
-	"os"
-	"path/filepath"
-	"sort"
-	"strconv"
 	"strings"
-	"time"
 
 	. "github.com/Masterminds/squirrel"
 	"github.com/beego/beego/v2/client/orm"
 	"github.com/deluan/rest"
 	"github.com/navidrome/navidrome/conf"
-	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/utils"
@@ -151,7 +145,7 @@ func (r *albumRepository) GetAllWithoutGenres(options ...model.QueryOptions) (mo
 func (r *albumRepository) Refresh(ids ...string) error {
 	chunks := utils.BreakUpStringSlice(ids, 100)
 	for _, chunk := range chunks {
-		err := r.refresh2(chunk...)
+		err := r.refresh(chunk...)
 		if err != nil {
 			return err
 		}
@@ -159,7 +153,7 @@ func (r *albumRepository) Refresh(ids ...string) error {
 	return nil
 }
 
-func (r *albumRepository) refresh2(ids ...string) error {
+func (r *albumRepository) refresh(ids ...string) error {
 	mfRepo := NewMediaFileRepository(r.ctx, r.ormer)
 	mfs, err := mfRepo.GetAll(model.QueryOptions{Filters: Eq{"album_id": ids}})
 	if err != nil {
@@ -176,199 +170,10 @@ func (r *albumRepository) refresh2(ids ...string) error {
 		if err != nil {
 			return err
 		}
-
 	}
 	return nil
 }
 
-type refreshAlbum struct {
-	model.Album
-	CurrentId      string
-	SongArtists    string
-	SongArtistIds  string
-	AlbumArtistIds string
-	GenreIds       string
-	Years          string
-	DiscSubtitles  string
-	Comments       string
-	Path           string
-	MaxUpdatedAt   string
-	MaxCreatedAt   string
-}
-
-func (r *albumRepository) refresh(ids ...string) error {
-	stringListIds := "('" + strings.Join(ids, "','") + "')"
-	var albums []refreshAlbum
-	sel := Select(`f.album_id as id, f.album as name, f.artist, f.album_artist, f.artist_id, f.album_artist_id, 
-		f.sort_album_name, f.sort_artist_name, f.sort_album_artist_name, f.order_album_name, f.order_album_artist_name, 
-		f.path, f.mbz_album_artist_id, f.mbz_album_type, f.mbz_album_comment, f.catalog_num, f.compilation, f.genre, 
-		count(f.id) as song_count,  
-		sum(f.duration) as duration,
-		sum(f.size) as size,
-		max(f.year) as max_year, 
-		max(f.updated_at) as max_updated_at,
-		max(f.created_at) as max_created_at,
-		a.id as current_id,  
-		group_concat(f.comment, "`+consts.Zwsp+`") as comments,
-		group_concat(f.mbz_album_id, ' ') as mbz_album_id, 
-		group_concat(f.disc_subtitle, ' ') as disc_subtitles,
-		group_concat(f.artist, ' ') as song_artists, 
-		group_concat(f.artist_id, ' ') as song_artist_ids, 
-		group_concat(f.album_artist_id, ' ') as album_artist_ids, 
-		group_concat(f.year, ' ') as years`,
-		"cf.cover_art_id", "cf.cover_art_path",
-		"mfg.genre_ids").
-		From("media_file f").
-		LeftJoin("album a on f.album_id = a.id").
-		LeftJoin(`(select album_id, id as cover_art_id, path as cover_art_path from media_file 
-			where has_cover_art = true and album_id in ` + stringListIds + ` group by album_id) cf 
-			on cf.album_id = f.album_id`).
-		LeftJoin(`(select mf.album_id, group_concat(genre_id, ' ') as genre_ids from media_file_genres
-			left join media_file mf on mf.id = media_file_id where mf.album_id in ` +
-			stringListIds + ` group by mf.album_id) mfg on mfg.album_id = f.album_id`).
-		Where(Eq{"f.album_id": ids}).GroupBy("f.album_id")
-	err := r.queryAll(sel, &albums)
-	if err != nil {
-		return err
-	}
-	toInsert := 0
-	toUpdate := 0
-	for _, al := range albums {
-		if al.CoverArtPath == "" || !strings.HasPrefix(conf.Server.CoverArtPriority, "embedded") {
-			if path := getCoverFromPath(al.Path, al.CoverArtPath); path != "" {
-				al.CoverArtId = "al-" + al.ID
-				al.CoverArtPath = path
-			}
-		}
-
-		if al.CoverArtId != "" {
-			log.Trace(r.ctx, "Found album art", "id", al.ID, "name", al.Name, "coverArtPath", al.CoverArtPath, "coverArtId", al.CoverArtId)
-		} else {
-			log.Trace(r.ctx, "Could not find album art", "id", al.ID, "name", al.Name)
-		}
-
-		// Somehow, beego cannot parse the datetimes for the query above
-		if al.UpdatedAt, err = time.Parse(time.RFC3339Nano, al.MaxUpdatedAt); err != nil {
-			al.UpdatedAt = time.Now()
-		}
-		if al.CreatedAt, err = time.Parse(time.RFC3339Nano, al.MaxCreatedAt); err != nil {
-			al.CreatedAt = al.UpdatedAt
-		}
-
-		al.AlbumArtistID, al.AlbumArtist = getAlbumArtist(al)
-		al.MinYear = getMinYear(al.Years)
-		al.MbzAlbumID = getMostFrequentMbzID(r.ctx, al.MbzAlbumID, r.tableName, al.Name)
-		al.Comment = getComment(al.Comments, consts.Zwsp)
-		if al.CurrentId != "" {
-			toUpdate++
-		} else {
-			toInsert++
-		}
-		al.AllArtistIDs = utils.SanitizeStrings(al.SongArtistIds, al.AlbumArtistID, al.ArtistID)
-		al.FullText = getFullText(al.Name, al.Artist, al.AlbumArtist, al.SongArtists,
-			al.SortAlbumName, al.SortArtistName, al.SortAlbumArtistName, al.DiscSubtitles)
-		al.Genres = getGenres(al.GenreIds)
-		err := r.Put(&al.Album)
-		if err != nil {
-			return err
-		}
-	}
-	if toInsert > 0 {
-		log.Debug(r.ctx, "Inserted new albums", "totalInserted", toInsert)
-	}
-	if toUpdate > 0 {
-		log.Debug(r.ctx, "Updated albums", "totalUpdated", toUpdate)
-	}
-	return err
-}
-
-func getAlbumArtist(al refreshAlbum) (id, name string) {
-	if !al.Compilation {
-		if al.AlbumArtist != "" {
-			return al.AlbumArtistID, al.AlbumArtist
-		}
-		return al.ArtistID, al.Artist
-	}
-
-	ids := strings.Split(al.AlbumArtistIds, " ")
-	allSame := true
-	previous := al.AlbumArtistID
-	for _, id := range ids {
-		if id == previous {
-			continue
-		}
-		allSame = false
-		break
-	}
-	if allSame {
-		return al.AlbumArtistID, al.AlbumArtist
-	}
-	return consts.VariousArtistsID, consts.VariousArtists
-}
-
-func getComment(comments string, separator string) string {
-	cs := strings.Split(comments, separator)
-	if len(cs) == 0 {
-		return ""
-	}
-	first := cs[0]
-	for _, c := range cs[1:] {
-		if first != c {
-			return ""
-		}
-	}
-	return first
-}
-
-func getMinYear(years string) int {
-	ys := strings.Fields(years)
-	sort.Strings(ys)
-	for _, y := range ys {
-		if y != "0" {
-			r, _ := strconv.Atoi(y)
-			return r
-		}
-	}
-	return 0
-}
-
-// GetCoverFromPath accepts a path to a file, and returns a path to an eligible cover image from the
-// file's directory (as configured with CoverArtPriority). If no cover file is found, among
-// available choices, or an error occurs, an empty string is returned. If HasEmbeddedCover is true,
-// and 'embedded' is matched among eligible choices, GetCoverFromPath will return early with an
-// empty path.
-func getCoverFromPath(mediaPath string, embeddedPath string) string {
-	n, err := os.Open(filepath.Dir(mediaPath))
-	if err != nil {
-		return ""
-	}
-
-	defer n.Close()
-	names, err := n.Readdirnames(-1)
-	if err != nil {
-		return ""
-	}
-
-	for _, p := range strings.Split(conf.Server.CoverArtPriority, ",") {
-		pat := strings.ToLower(strings.TrimSpace(p))
-		if pat == "embedded" {
-			if embeddedPath != "" {
-				return ""
-			}
-			continue
-		}
-
-		for _, name := range names {
-			match, _ := filepath.Match(pat, strings.ToLower(name))
-			if match && utils.IsImageFile(name) {
-				return filepath.Join(filepath.Dir(mediaPath), name)
-			}
-		}
-	}
-
-	return ""
-}
-
 func (r *albumRepository) purgeEmpty() error {
 	del := Delete(r.tableName).Where("id not in (select distinct(album_id) from media_file)")
 	c, err := r.executeSQL(del)

persistence/album_repository_test.go

@@ -2,12 +2,8 @@ package persistence
 
 import (
 	"context"
-	"os"
-	"path/filepath"
 
 	"github.com/beego/beego/v2/client/orm"
-	"github.com/navidrome/navidrome/conf"
-	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/model/request"
@@ -60,132 +56,4 @@ var _ = Describe("AlbumRepository", func() {
 			}))
 		})
 	})
-
-	Describe("getMinYear", func() {
-		It("returns 0 when there's no valid year", func() {
-			Expect(getMinYear("a b c")).To(Equal(0))
-			Expect(getMinYear("")).To(Equal(0))
-		})
-		It("returns 0 when all values are 0", func() {
-			Expect(getMinYear("0 0 0 ")).To(Equal(0))
-		})
-		It("returns the smallest value from the list", func() {
-			Expect(getMinYear("2000 0 1800")).To(Equal(1800))
-		})
-	})
-
-	Describe("getComment", func() {
-		const zwsp = string('\u200b')
-		It("returns empty string if there are no comments", func() {
-			Expect(getComment("", "")).To(Equal(""))
-		})
-		It("returns empty string if comments are different", func() {
-			Expect(getComment("first"+zwsp+"second", zwsp)).To(Equal(""))
-		})
-		It("returns comment if all comments are the same", func() {
-			Expect(getComment("first"+zwsp+"first", zwsp)).To(Equal("first"))
-		})
-	})
-
-	Describe("getCoverFromPath", func() {
-		var testFolder, testPath, embeddedPath string
-		BeforeEach(func() {
-			testFolder, _ = os.MkdirTemp("", "album_persistence_tests")
-			if err := os.MkdirAll(testFolder, 0777); err != nil {
-				panic(err)
-			}
-			if _, err := os.Create(filepath.Join(testFolder, "Cover.jpeg")); err != nil {
-				panic(err)
-			}
-			if _, err := os.Create(filepath.Join(testFolder, "FRONT.PNG")); err != nil {
-				panic(err)
-			}
-			testPath = filepath.Join(testFolder, "somefile.test")
-			embeddedPath = filepath.Join(testFolder, "somefile.mp3")
-		})
-		AfterEach(func() {
-			_ = os.RemoveAll(testFolder)
-		})
-
-		It("returns audio file for embedded cover", func() {
-			conf.Server.CoverArtPriority = "embedded, cover.*, front.*"
-			Expect(getCoverFromPath(testPath, embeddedPath)).To(Equal(""))
-		})
-
-		It("returns external file when no embedded cover exists", func() {
-			conf.Server.CoverArtPriority = "embedded, cover.*, front.*"
-			Expect(getCoverFromPath(testPath, "")).To(Equal(filepath.Join(testFolder, "Cover.jpeg")))
-		})
-
-		It("returns embedded cover even if not first choice", func() {
-			conf.Server.CoverArtPriority = "something.png, embedded, cover.*, front.*"
-			Expect(getCoverFromPath(testPath, embeddedPath)).To(Equal(""))
-		})
-
-		It("returns first correct match case-insensitively", func() {
-			conf.Server.CoverArtPriority = "embedded, cover.jpg, front.svg, front.png"
-			Expect(getCoverFromPath(testPath, "")).To(Equal(filepath.Join(testFolder, "FRONT.PNG")))
-		})
-
-		It("returns match for embedded pattern", func() {
-			conf.Server.CoverArtPriority = "embedded, cover.jp?g, front.png"
-			Expect(getCoverFromPath(testPath, "")).To(Equal(filepath.Join(testFolder, "Cover.jpeg")))
-		})
-
-		It("returns empty string if no match was found", func() {
-			conf.Server.CoverArtPriority = "embedded, cover.jpg, front.apng"
-			Expect(getCoverFromPath(testPath, "")).To(Equal(""))
-		})
-
-		// Reset configuration to default.
-		conf.Server.CoverArtPriority = "embedded, cover.*, front.*"
-	})
-
-	Describe("getAlbumArtist", func() {
-		var al refreshAlbum
-		BeforeEach(func() {
-			al = refreshAlbum{}
-		})
-		Context("Non-Compilations", func() {
-			BeforeEach(func() {
-				al.Compilation = false
-				al.Artist = "Sparks"
-				al.ArtistID = "ar-123"
-			})
-			It("returns the track artist if no album artist is specified", func() {
-				id, name := getAlbumArtist(al)
-				Expect(id).To(Equal("ar-123"))
-				Expect(name).To(Equal("Sparks"))
-			})
-			It("returns the album artist if it is specified", func() {
-				al.AlbumArtist = "Sparks Brothers"
-				al.AlbumArtistID = "ar-345"
-				id, name := getAlbumArtist(al)
-				Expect(id).To(Equal("ar-345"))
-				Expect(name).To(Equal("Sparks Brothers"))
-			})
-		})
-		Context("Compilations", func() {
-			BeforeEach(func() {
-				al.Compilation = true
-				al.Name = "Sgt. Pepper Knew My Father"
-				al.AlbumArtistID = "ar-000"
-				al.AlbumArtist = "The Beatles"
-			})
-
-			It("returns VariousArtists if there's more than one album artist", func() {
-				al.AlbumArtistIds = `ar-123 ar-345`
-				id, name := getAlbumArtist(al)
-				Expect(id).To(Equal(consts.VariousArtistsID))
-				Expect(name).To(Equal(consts.VariousArtists))
-			})
-
-			It("returns the sole album artist if they are the same", func() {
-				al.AlbumArtistIds = `ar-000 ar-000`
-				id, name := getAlbumArtist(al)
-				Expect(id).To(Equal("ar-000"))
-				Expect(name).To(Equal("The Beatles"))
-			})
-		})
-	})
 })