From 566ae939506ac698bc1ef9f0c9d6738d4a17f4dd Mon Sep 17 00:00:00 2001 From: Deluan Date: Mon, 19 Dec 2022 11:23:57 -0500 Subject: [PATCH] Remove old refresh code --- .../20210418232815_fix_album_comments.go | 7 +- model/mediafile_test.go | 13 ++ persistence/album_repository.go | 199 +----------------- persistence/album_repository_test.go | 132 ------------ 4 files changed, 19 insertions(+), 332 deletions(-) diff --git a/db/migration/20210418232815_fix_album_comments.go b/db/migration/20210418232815_fix_album_comments.go index 44a231f58..e8603f697 100644 --- a/db/migration/20210418232815_fix_album_comments.go +++ b/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 { diff --git a/model/mediafile_test.go b/model/mediafile_test.go index 96f7536a4..b816b082e 100644 --- a/model/mediafile_test.go +++ b/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() { diff --git a/persistence/album_repository.go b/persistence/album_repository.go index 38f79c2f1..8a47a2834 100644 --- a/persistence/album_repository.go +++ b/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) diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index d635d1b9a..e7b9fafc6 100644 --- a/persistence/album_repository_test.go +++ b/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")) - }) - }) - }) })