From d4818640355a53cf64038b62cd3e8b1a699aea16 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 22 Aug 2021 11:46:52 -0400 Subject: [PATCH] Some small refactorings --- persistence/user_repository_test.go | 2 +- scanner/mapping.go | 20 +++---- scanner/mapping_test.go | 7 +-- scanner/metadata/metadata.go | 88 ++++++++++++++--------------- tests/mock_genre_repo.go | 38 +++++++++++++ tests/mock_persistence.go | 6 +- tests/mock_property_repo.go | 20 +++---- tests/mock_share_repo.go | 12 +++- tests/mock_user_props_repo.go | 20 +++---- tests/mock_user_repo.go | 18 +++--- 10 files changed, 137 insertions(+), 94 deletions(-) create mode 100644 tests/mock_genre_repo.go diff --git a/persistence/user_repository_test.go b/persistence/user_repository_test.go index 9a2c3c91..6792bde3 100644 --- a/persistence/user_repository_test.go +++ b/persistence/user_repository_test.go @@ -190,7 +190,7 @@ var _ = Describe("UserRepository", func() { Expect(err.(*rest.ValidationError).Errors).To(HaveKeyWithValue("userName", "ra.validation.unique")) }) It("returns generic error if repository call fails", func() { - repo.Err = errors.New("fake error") + repo.Error = errors.New("fake error") var newUser = &model.User{ID: "2", UserName: "newuser"} err := validateUsernameUnique(repo, newUser) diff --git a/scanner/mapping.go b/scanner/mapping.go index 17ef17b2..d551360f 100644 --- a/scanner/mapping.go +++ b/scanner/mapping.go @@ -31,7 +31,7 @@ func newMediaFileMapper(rootFolder string, genres model.GenreRepository) *mediaF } } -func (s *mediaFileMapper) toMediaFile(md *metadata.Tags) model.MediaFile { +func (s mediaFileMapper) toMediaFile(md metadata.Tags) model.MediaFile { mf := &model.MediaFile{} mf.ID = s.trackID(md) mf.Title = s.mapTrackTitle(md) @@ -82,7 +82,7 @@ func sanitizeFieldForSorting(originalValue string) string { return utils.NoArticle(v) } -func (s *mediaFileMapper) mapTrackTitle(md *metadata.Tags) string { +func (s mediaFileMapper) mapTrackTitle(md metadata.Tags) string { if md.Title() == "" { s := strings.TrimPrefix(md.FilePath(), s.rootFolder+string(os.PathSeparator)) e := filepath.Ext(s) @@ -91,7 +91,7 @@ func (s *mediaFileMapper) mapTrackTitle(md *metadata.Tags) string { return md.Title() } -func (s *mediaFileMapper) mapAlbumArtistName(md *metadata.Tags) string { +func (s mediaFileMapper) mapAlbumArtistName(md metadata.Tags) string { switch { case md.AlbumArtist() != "": return md.AlbumArtist() @@ -104,14 +104,14 @@ func (s *mediaFileMapper) mapAlbumArtistName(md *metadata.Tags) string { } } -func (s *mediaFileMapper) mapArtistName(md *metadata.Tags) string { +func (s mediaFileMapper) mapArtistName(md metadata.Tags) string { if md.Artist() != "" { return md.Artist() } return consts.UnknownArtist } -func (s *mediaFileMapper) mapAlbumName(md *metadata.Tags) string { +func (s mediaFileMapper) mapAlbumName(md metadata.Tags) string { name := md.Album() if name == "" { return "[Unknown Album]" @@ -119,24 +119,24 @@ func (s *mediaFileMapper) mapAlbumName(md *metadata.Tags) string { return name } -func (s *mediaFileMapper) trackID(md *metadata.Tags) string { +func (s mediaFileMapper) trackID(md metadata.Tags) string { return fmt.Sprintf("%x", md5.Sum([]byte(md.FilePath()))) } -func (s *mediaFileMapper) albumID(md *metadata.Tags) string { +func (s mediaFileMapper) albumID(md metadata.Tags) string { albumPath := strings.ToLower(fmt.Sprintf("%s\\%s", s.mapAlbumArtistName(md), s.mapAlbumName(md))) return fmt.Sprintf("%x", md5.Sum([]byte(albumPath))) } -func (s *mediaFileMapper) artistID(md *metadata.Tags) string { +func (s mediaFileMapper) artistID(md metadata.Tags) string { return fmt.Sprintf("%x", md5.Sum([]byte(strings.ToLower(s.mapArtistName(md))))) } -func (s *mediaFileMapper) albumArtistID(md *metadata.Tags) string { +func (s mediaFileMapper) albumArtistID(md metadata.Tags) string { return fmt.Sprintf("%x", md5.Sum([]byte(strings.ToLower(s.mapAlbumArtistName(md))))) } -func (s *mediaFileMapper) mapGenres(genres []string) (string, model.Genres) { +func (s mediaFileMapper) mapGenres(genres []string) (string, model.Genres) { var result model.Genres unique := map[string]struct{}{} var all []string diff --git a/scanner/mapping_test.go b/scanner/mapping_test.go index 01c2cf78..10265a25 100644 --- a/scanner/mapping_test.go +++ b/scanner/mapping_test.go @@ -3,10 +3,9 @@ package scanner import ( "context" - "github.com/astaxie/beego/orm" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/persistence" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) @@ -33,8 +32,8 @@ var _ = Describe("mapping", func() { var ctx context.Context BeforeEach(func() { ctx = context.Background() - o := orm.NewOrm() - gr = persistence.NewGenreRepository(ctx, o) + ds := &tests.MockDataStore{} + gr = ds.Genre(ctx) gr = newCachedGenreRepository(ctx, gr) mapper = newMediaFileMapper("/", gr) }) diff --git a/scanner/metadata/metadata.go b/scanner/metadata/metadata.go index 9b2a8734..bac50b7d 100644 --- a/scanner/metadata/metadata.go +++ b/scanner/metadata/metadata.go @@ -28,7 +28,7 @@ var parsers = map[string]Parser{ "taglib": &taglib.Parser{}, } -func Extract(files ...string) (map[string]*Tags, error) { +func Extract(files ...string) (map[string]Tags, error) { p, ok := parsers[conf.Server.Scanner.Extractor] if !ok { log.Warn("Invalid 'Scanner.Extractor' option. Using default", "requested", conf.Server.Scanner.Extractor, @@ -41,7 +41,7 @@ func Extract(files ...string) (map[string]*Tags, error) { return nil, err } - result := map[string]*Tags{} + result := map[string]Tags{} for filePath, tags := range extractedTags { fileInfo, err := os.Stat(filePath) if err != nil { @@ -49,7 +49,7 @@ func Extract(files ...string) (map[string]*Tags, error) { continue } - result[filePath] = &Tags{ + result[filePath] = Tags{ filePath: filePath, fileInfo: fileInfo, tags: tags, @@ -67,57 +67,57 @@ type Tags struct { // Common tags -func (t *Tags) Title() string { return t.getFirstTagValue("title", "sort_name", "titlesort") } -func (t *Tags) Album() string { return t.getFirstTagValue("album", "sort_album", "albumsort") } -func (t *Tags) Artist() string { return t.getFirstTagValue("artist", "sort_artist", "artistsort") } -func (t *Tags) AlbumArtist() string { +func (t Tags) Title() string { return t.getFirstTagValue("title", "sort_name", "titlesort") } +func (t Tags) Album() string { return t.getFirstTagValue("album", "sort_album", "albumsort") } +func (t Tags) Artist() string { return t.getFirstTagValue("artist", "sort_artist", "artistsort") } +func (t Tags) AlbumArtist() string { return t.getFirstTagValue("album_artist", "album artist", "albumartist") } -func (t *Tags) SortTitle() string { return t.getSortTag("", "title", "name") } -func (t *Tags) SortAlbum() string { return t.getSortTag("", "album") } -func (t *Tags) SortArtist() string { return t.getSortTag("", "artist") } -func (t *Tags) SortAlbumArtist() string { return t.getSortTag("tso2", "albumartist", "album_artist") } -func (t *Tags) Genres() []string { return t.getAllTagValues("genre") } -func (t *Tags) Year() int { return t.getYear("date") } -func (t *Tags) Comment() string { return t.getFirstTagValue("comment") } -func (t *Tags) Lyrics() string { return t.getFirstTagValue("lyrics", "lyrics-eng") } -func (t *Tags) Compilation() bool { return t.getBool("tcmp", "compilation") } -func (t *Tags) TrackNumber() (int, int) { return t.getTuple("track", "tracknumber") } -func (t *Tags) DiscNumber() (int, int) { return t.getTuple("disc", "discnumber") } -func (t *Tags) DiscSubtitle() string { +func (t Tags) SortTitle() string { return t.getSortTag("", "title", "name") } +func (t Tags) SortAlbum() string { return t.getSortTag("", "album") } +func (t Tags) SortArtist() string { return t.getSortTag("", "artist") } +func (t Tags) SortAlbumArtist() string { return t.getSortTag("tso2", "albumartist", "album_artist") } +func (t Tags) Genres() []string { return t.getAllTagValues("genre") } +func (t Tags) Year() int { return t.getYear("date") } +func (t Tags) Comment() string { return t.getFirstTagValue("comment") } +func (t Tags) Lyrics() string { return t.getFirstTagValue("lyrics", "lyrics-eng") } +func (t Tags) Compilation() bool { return t.getBool("tcmp", "compilation") } +func (t Tags) TrackNumber() (int, int) { return t.getTuple("track", "tracknumber") } +func (t Tags) DiscNumber() (int, int) { return t.getTuple("disc", "discnumber") } +func (t Tags) DiscSubtitle() string { return t.getFirstTagValue("tsst", "discsubtitle", "setsubtitle") } -func (t *Tags) CatalogNum() string { return t.getFirstTagValue("catalognumber") } -func (t *Tags) Bpm() int { return (int)(math.Round(t.getFloat("tbpm", "bpm", "fbpm"))) } -func (t *Tags) HasPicture() bool { return t.getFirstTagValue("has_picture") != "" } +func (t Tags) CatalogNum() string { return t.getFirstTagValue("catalognumber") } +func (t Tags) Bpm() int { return (int)(math.Round(t.getFloat("tbpm", "bpm", "fbpm"))) } +func (t Tags) HasPicture() bool { return t.getFirstTagValue("has_picture") != "" } // MusicBrainz Identifiers -func (t *Tags) MbzTrackID() string { return t.getMbzID("musicbrainz_trackid", "musicbrainz track id") } -func (t *Tags) MbzAlbumID() string { return t.getMbzID("musicbrainz_albumid", "musicbrainz album id") } -func (t *Tags) MbzArtistID() string { +func (t Tags) MbzTrackID() string { return t.getMbzID("musicbrainz_trackid", "musicbrainz track id") } +func (t Tags) MbzAlbumID() string { return t.getMbzID("musicbrainz_albumid", "musicbrainz album id") } +func (t Tags) MbzArtistID() string { return t.getMbzID("musicbrainz_artistid", "musicbrainz artist id") } -func (t *Tags) MbzAlbumArtistID() string { +func (t Tags) MbzAlbumArtistID() string { return t.getMbzID("musicbrainz_albumartistid", "musicbrainz album artist id") } -func (t *Tags) MbzAlbumType() string { +func (t Tags) MbzAlbumType() string { return t.getFirstTagValue("musicbrainz_albumtype", "musicbrainz album type") } -func (t *Tags) MbzAlbumComment() string { +func (t Tags) MbzAlbumComment() string { return t.getFirstTagValue("musicbrainz_albumcomment", "musicbrainz album comment") } // File properties -func (t *Tags) Duration() float32 { return float32(t.getFloat("duration")) } -func (t *Tags) BitRate() int { return t.getInt("bitrate") } -func (t *Tags) ModificationTime() time.Time { return t.fileInfo.ModTime() } -func (t *Tags) Size() int64 { return t.fileInfo.Size() } -func (t *Tags) FilePath() string { return t.filePath } -func (t *Tags) Suffix() string { return strings.ToLower(strings.TrimPrefix(path.Ext(t.filePath), ".")) } +func (t Tags) Duration() float32 { return float32(t.getFloat("duration")) } +func (t Tags) BitRate() int { return t.getInt("bitrate") } +func (t Tags) ModificationTime() time.Time { return t.fileInfo.ModTime() } +func (t Tags) Size() int64 { return t.fileInfo.Size() } +func (t Tags) FilePath() string { return t.filePath } +func (t Tags) Suffix() string { return strings.ToLower(strings.TrimPrefix(path.Ext(t.filePath), ".")) } -func (t *Tags) getTags(tagNames ...string) []string { +func (t Tags) getTags(tagNames ...string) []string { for _, tag := range tagNames { if v, ok := t.tags[tag]; ok { return v @@ -126,7 +126,7 @@ func (t *Tags) getTags(tagNames ...string) []string { return nil } -func (t *Tags) getFirstTagValue(tagNames ...string) string { +func (t Tags) getFirstTagValue(tagNames ...string) string { ts := t.getTags(tagNames...) if len(ts) > 0 { return ts[0] @@ -134,7 +134,7 @@ func (t *Tags) getFirstTagValue(tagNames ...string) string { return "" } -func (t *Tags) getAllTagValues(tagNames ...string) []string { +func (t Tags) getAllTagValues(tagNames ...string) []string { var values []string for _, tag := range tagNames { if v, ok := t.tags[tag]; ok { @@ -144,7 +144,7 @@ func (t *Tags) getAllTagValues(tagNames ...string) []string { return values } -func (t *Tags) getSortTag(originalTag string, tagNamess ...string) string { +func (t Tags) getSortTag(originalTag string, tagNamess ...string) string { formats := []string{"sort%s", "sort_%s", "sort-%s", "%ssort", "%s_sort", "%s-sort"} all := []string{originalTag} for _, tag := range tagNamess { @@ -158,7 +158,7 @@ func (t *Tags) getSortTag(originalTag string, tagNamess ...string) string { var dateRegex = regexp.MustCompile(`([12]\d\d\d)`) -func (t *Tags) getYear(tagNames ...string) int { +func (t Tags) getYear(tagNames ...string) int { tag := t.getFirstTagValue(tagNames...) if tag == "" { return 0 @@ -172,7 +172,7 @@ func (t *Tags) getYear(tagNames ...string) int { return year } -func (t *Tags) getBool(tagNames ...string) bool { +func (t Tags) getBool(tagNames ...string) bool { tag := t.getFirstTagValue(tagNames...) if tag == "" { return false @@ -181,7 +181,7 @@ func (t *Tags) getBool(tagNames ...string) bool { return i == 1 } -func (t *Tags) getTuple(tagNames ...string) (int, int) { +func (t Tags) getTuple(tagNames ...string) (int, int) { tag := t.getFirstTagValue(tagNames...) if tag == "" { return 0, 0 @@ -198,7 +198,7 @@ func (t *Tags) getTuple(tagNames ...string) (int, int) { return t1, t2 } -func (t *Tags) getMbzID(tagNames ...string) string { +func (t Tags) getMbzID(tagNames ...string) string { tag := t.getFirstTagValue(tagNames...) if _, err := uuid.Parse(tag); err != nil { return "" @@ -206,13 +206,13 @@ func (t *Tags) getMbzID(tagNames ...string) string { return tag } -func (t *Tags) getInt(tagNames ...string) int { +func (t Tags) getInt(tagNames ...string) int { tag := t.getFirstTagValue(tagNames...) i, _ := strconv.Atoi(tag) return i } -func (t *Tags) getFloat(tagNames ...string) float64 { +func (t Tags) getFloat(tagNames ...string) float64 { var tag = t.getFirstTagValue(tagNames...) var value, err = strconv.ParseFloat(tag, 64) if err != nil { diff --git a/tests/mock_genre_repo.go b/tests/mock_genre_repo.go new file mode 100644 index 00000000..a24f2b0c --- /dev/null +++ b/tests/mock_genre_repo.go @@ -0,0 +1,38 @@ +package tests + +import ( + "github.com/navidrome/navidrome/model" +) + +type MockedGenreRepo struct { + Error error + data map[string]model.Genre +} + +func (r *MockedGenreRepo) init() { + if r.data == nil { + r.data = make(map[string]model.Genre) + } +} + +func (r *MockedGenreRepo) GetAll(...model.QueryOptions) (model.Genres, error) { + if r.Error != nil { + return nil, r.Error + } + r.init() + + var all model.Genres + for _, g := range r.data { + all = append(all, g) + } + return all, nil +} + +func (r *MockedGenreRepo) Put(g *model.Genre) error { + if r.Error != nil { + return r.Error + } + r.init() + r.data[g.ID] = *g + return nil +} diff --git a/tests/mock_persistence.go b/tests/mock_persistence.go index e4517602..42c3077b 100644 --- a/tests/mock_persistence.go +++ b/tests/mock_persistence.go @@ -46,10 +46,10 @@ func (db *MockDataStore) MediaFolder(context.Context) model.MediaFolderRepositor } func (db *MockDataStore) Genre(context.Context) model.GenreRepository { - if db.MockedGenre != nil { - return db.MockedGenre + if db.MockedGenre == nil { + db.MockedGenre = &MockedGenreRepo{} } - return struct{ model.GenreRepository }{} + return db.MockedGenre } func (db *MockDataStore) Playlist(context.Context) model.PlaylistRepository { diff --git a/tests/mock_property_repo.go b/tests/mock_property_repo.go index 2dd789ec..be88efc1 100644 --- a/tests/mock_property_repo.go +++ b/tests/mock_property_repo.go @@ -4,8 +4,8 @@ import "github.com/navidrome/navidrome/model" type MockedPropertyRepo struct { model.PropertyRepository - data map[string]string - err error + Error error + data map[string]string } func (p *MockedPropertyRepo) init() { @@ -15,8 +15,8 @@ func (p *MockedPropertyRepo) init() { } func (p *MockedPropertyRepo) Put(id string, value string) error { - if p.err != nil { - return p.err + if p.Error != nil { + return p.Error } p.init() p.data[id] = value @@ -24,8 +24,8 @@ func (p *MockedPropertyRepo) Put(id string, value string) error { } func (p *MockedPropertyRepo) Get(id string) (string, error) { - if p.err != nil { - return "", p.err + if p.Error != nil { + return "", p.Error } p.init() if v, ok := p.data[id]; ok { @@ -35,8 +35,8 @@ func (p *MockedPropertyRepo) Get(id string) (string, error) { } func (p *MockedPropertyRepo) Delete(id string) error { - if p.err != nil { - return p.err + if p.Error != nil { + return p.Error } p.init() if _, ok := p.data[id]; ok { @@ -47,8 +47,8 @@ func (p *MockedPropertyRepo) Delete(id string) error { } func (p *MockedPropertyRepo) DefaultGet(id string, defaultValue string) (string, error) { - if p.err != nil { - return "", p.err + if p.Error != nil { + return "", p.Error } p.init() v, err := p.Get(id) diff --git a/tests/mock_share_repo.go b/tests/mock_share_repo.go index 3cf52bf4..e78150f7 100644 --- a/tests/mock_share_repo.go +++ b/tests/mock_share_repo.go @@ -12,16 +12,22 @@ type MockShareRepo struct { Entity interface{} Cols []string - Err error + Error error } func (m *MockShareRepo) Save(entity interface{}) (string, error) { + if m.Error != nil { + return "", m.Error + } m.Entity = entity - return "id", m.Err + return "id", nil } func (m *MockShareRepo) Update(entity interface{}, cols ...string) error { + if m.Error != nil { + return m.Error + } m.Entity = entity m.Cols = cols - return m.Err + return nil } diff --git a/tests/mock_user_props_repo.go b/tests/mock_user_props_repo.go index 7fa581bb..2f8554d7 100644 --- a/tests/mock_user_props_repo.go +++ b/tests/mock_user_props_repo.go @@ -4,8 +4,8 @@ import "github.com/navidrome/navidrome/model" type MockedUserPropsRepo struct { model.UserPropsRepository - data map[string]string - err error + Error error + data map[string]string } func (p *MockedUserPropsRepo) init() { @@ -15,8 +15,8 @@ func (p *MockedUserPropsRepo) init() { } func (p *MockedUserPropsRepo) Put(userId, key string, value string) error { - if p.err != nil { - return p.err + if p.Error != nil { + return p.Error } p.init() p.data[userId+key] = value @@ -24,8 +24,8 @@ func (p *MockedUserPropsRepo) Put(userId, key string, value string) error { } func (p *MockedUserPropsRepo) Get(userId, key string) (string, error) { - if p.err != nil { - return "", p.err + if p.Error != nil { + return "", p.Error } p.init() if v, ok := p.data[userId+key]; ok { @@ -35,8 +35,8 @@ func (p *MockedUserPropsRepo) Get(userId, key string) (string, error) { } func (p *MockedUserPropsRepo) Delete(userId, key string) error { - if p.err != nil { - return p.err + if p.Error != nil { + return p.Error } p.init() if _, ok := p.data[userId+key]; ok { @@ -47,8 +47,8 @@ func (p *MockedUserPropsRepo) Delete(userId, key string) error { } func (p *MockedUserPropsRepo) DefaultGet(userId, key string, defaultValue string) (string, error) { - if p.err != nil { - return "", p.err + if p.Error != nil { + return "", p.Error } p.init() v, err := p.Get(userId, key) diff --git a/tests/mock_user_repo.go b/tests/mock_user_repo.go index 1a4025ad..31be2c8b 100644 --- a/tests/mock_user_repo.go +++ b/tests/mock_user_repo.go @@ -15,20 +15,20 @@ func CreateMockUserRepo() *MockedUserRepo { type MockedUserRepo struct { model.UserRepository - Err error - Data map[string]*model.User + Error error + Data map[string]*model.User } func (u *MockedUserRepo) CountAll(qo ...model.QueryOptions) (int64, error) { - if u.Err != nil { - return 0, u.Err + if u.Error != nil { + return 0, u.Error } return int64(len(u.Data)), nil } func (u *MockedUserRepo) Put(usr *model.User) error { - if u.Err != nil { - return u.Err + if u.Error != nil { + return u.Error } if usr.ID == "" { usr.ID = base64.StdEncoding.EncodeToString([]byte(usr.UserName)) @@ -39,8 +39,8 @@ func (u *MockedUserRepo) Put(usr *model.User) error { } func (u *MockedUserRepo) FindByUsername(username string) (*model.User, error) { - if u.Err != nil { - return nil, u.Err + if u.Error != nil { + return nil, u.Error } usr, ok := u.Data[strings.ToLower(username)] if !ok { @@ -54,5 +54,5 @@ func (u *MockedUserRepo) FindByUsernameWithPassword(username string) (*model.Use } func (u *MockedUserRepo) UpdateLastLoginAt(id string) error { - return u.Err + return u.Error }