mirror of
https://github.com/navidrome/navidrome.git
synced 2025-04-03 20:47:35 +03:00
refactor
Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan
parent
018b139a79
commit
61240375db
1 changed files with 13 additions and 89 deletions
|
|
@ -4,7 +4,6 @@ import (
|
|||
"context"
|
||||
"errors"
|
||||
|
||||
"github.com/Masterminds/squirrel"
|
||||
"github.com/navidrome/navidrome/conf"
|
||||
"github.com/navidrome/navidrome/core/agents"
|
||||
_ "github.com/navidrome/navidrome/core/agents/lastfm"
|
||||
|
|
@ -57,7 +56,7 @@ var _ = Describe("Provider - TopSongs", func() {
|
|||
It("returns top songs for a known artist", func() {
|
||||
// Mock finding the artist
|
||||
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
||||
artistRepo.On("GetAll", mock.MatchedBy(matchArtistByNameFilter)).Return(model.Artists{artist1}, nil).Once()
|
||||
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
||||
|
||||
// Mock agent response
|
||||
agentSongs := []agents.Song{
|
||||
|
|
@ -69,8 +68,8 @@ var _ = Describe("Provider - TopSongs", func() {
|
|||
// Mock finding matching tracks
|
||||
song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"}
|
||||
song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-song-2"}
|
||||
mediaFileRepo.On("GetAll", mock.MatchedBy(matchMediaFileByMBIDFilter("mbid-song-1"))).Return(model.MediaFiles{song1}, nil).Once()
|
||||
mediaFileRepo.On("GetAll", mock.MatchedBy(matchMediaFileByMBIDFilter("mbid-song-2"))).Return(model.MediaFiles{song2}, nil).Once()
|
||||
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once()
|
||||
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song2}, nil).Once()
|
||||
|
||||
songs, err := p.TopSongs(ctx, "Artist One", 2)
|
||||
|
||||
|
|
@ -85,7 +84,7 @@ var _ = Describe("Provider - TopSongs", func() {
|
|||
|
||||
It("returns nil for an unknown artist", func() {
|
||||
// Mock artist not found
|
||||
artistRepo.On("GetAll", mock.MatchedBy(matchArtistByNameFilter)).Return(model.Artists{}, nil).Once()
|
||||
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{}, nil).Once()
|
||||
|
||||
songs, err := p.TopSongs(ctx, "Unknown Artist", 5)
|
||||
|
||||
|
|
@ -98,7 +97,7 @@ var _ = Describe("Provider - TopSongs", func() {
|
|||
It("returns error when the agent returns an error", func() {
|
||||
// Mock finding the artist
|
||||
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
||||
artistRepo.On("GetAll", mock.MatchedBy(matchArtistByNameFilter)).Return(model.Artists{artist1}, nil).Once()
|
||||
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
||||
|
||||
// Mock agent error
|
||||
agentErr := errors.New("agent error")
|
||||
|
|
@ -115,7 +114,7 @@ var _ = Describe("Provider - TopSongs", func() {
|
|||
It("returns ErrNotFound when the agent returns ErrNotFound", func() {
|
||||
// Mock finding the artist
|
||||
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
||||
artistRepo.On("GetAll", mock.MatchedBy(matchArtistByNameFilter)).Return(model.Artists{artist1}, nil).Once()
|
||||
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
||||
|
||||
// Mock agent ErrNotFound
|
||||
ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, agents.ErrNotFound).Once()
|
||||
|
|
@ -131,7 +130,7 @@ var _ = Describe("Provider - TopSongs", func() {
|
|||
It("returns fewer songs if count is less than available top songs", func() {
|
||||
// Mock finding the artist
|
||||
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
||||
artistRepo.On("GetAll", mock.MatchedBy(matchArtistByNameFilter)).Return(model.Artists{artist1}, nil).Once()
|
||||
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
||||
|
||||
// Mock agent response (only need 1 for the test)
|
||||
agentSongs := []agents.Song{{Name: "Song One", MBID: "mbid-song-1"}}
|
||||
|
|
@ -139,7 +138,7 @@ var _ = Describe("Provider - TopSongs", func() {
|
|||
|
||||
// Mock finding matching track
|
||||
song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"}
|
||||
mediaFileRepo.On("GetAll", mock.MatchedBy(matchMediaFileByMBIDFilter("mbid-song-1"))).Return(model.MediaFiles{song1}, nil).Once()
|
||||
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once()
|
||||
|
||||
songs, err := p.TopSongs(ctx, "Artist One", 1)
|
||||
|
||||
|
|
@ -154,7 +153,7 @@ var _ = Describe("Provider - TopSongs", func() {
|
|||
It("returns fewer songs if fewer matching tracks are found", func() {
|
||||
// Mock finding the artist
|
||||
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
||||
artistRepo.On("GetAll", mock.MatchedBy(matchArtistByNameFilter)).Return(model.Artists{artist1}, nil).Once()
|
||||
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
||||
|
||||
// Mock agent response
|
||||
agentSongs := []agents.Song{
|
||||
|
|
@ -165,10 +164,9 @@ var _ = Describe("Provider - TopSongs", func() {
|
|||
|
||||
// Mock finding matching tracks (only find song 1)
|
||||
song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"}
|
||||
mediaFileRepo.On("GetAll", mock.MatchedBy(matchMediaFileByMBIDFilter("mbid-song-1"))).Return(model.MediaFiles{song1}, nil).Once()
|
||||
mediaFileRepo.On("GetAll", mock.MatchedBy(matchMediaFileByMBIDFilter("mbid-song-2"))).Return(model.MediaFiles{}, nil).Once() // MBID lookup fails
|
||||
// Mock fallback lookup by title (also fails)
|
||||
mediaFileRepo.On("GetAll", mock.MatchedBy(matchMediaFileByTitleFilter("artist-1", "Song Two"))).Return(model.MediaFiles{}, nil).Once()
|
||||
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once()
|
||||
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once() // For mbid-song-2 (fails)
|
||||
mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once() // For title fallback (fails)
|
||||
|
||||
songs, err := p.TopSongs(ctx, "Artist One", 2)
|
||||
|
||||
|
|
@ -183,7 +181,7 @@ var _ = Describe("Provider - TopSongs", func() {
|
|||
It("returns error when context is canceled during agent call", func() {
|
||||
// Mock finding the artist
|
||||
artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"}
|
||||
artistRepo.On("GetAll", mock.MatchedBy(matchArtistByNameFilter)).Return(model.Artists{artist1}, nil).Once()
|
||||
artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once()
|
||||
|
||||
// Setup context that will be canceled
|
||||
canceledCtx, cancel := context.WithCancel(ctx)
|
||||
|
|
@ -201,77 +199,3 @@ var _ = Describe("Provider - TopSongs", func() {
|
|||
})
|
||||
})
|
||||
})
|
||||
|
||||
// Helper functions to match squirrel filters for testify/mock
|
||||
func matchArtistByNameFilter(opt model.QueryOptions) bool {
|
||||
if opt.Max != 1 || opt.Filters == nil {
|
||||
return false
|
||||
}
|
||||
_, ok := opt.Filters.(squirrel.Like)
|
||||
// Could add more specific checks on the Like clause if needed
|
||||
return ok
|
||||
}
|
||||
|
||||
func matchMediaFileByMBIDFilter(expectedMBID string) func(opt model.QueryOptions) bool {
|
||||
return func(opt model.QueryOptions) bool {
|
||||
if opt.Filters == nil {
|
||||
return false
|
||||
}
|
||||
andClause, ok := opt.Filters.(squirrel.And)
|
||||
if !ok || len(andClause) < 2 {
|
||||
return false
|
||||
}
|
||||
foundMBID := false
|
||||
foundMissing := false
|
||||
for _, condition := range andClause {
|
||||
if eqClause, ok := condition.(squirrel.Eq); ok {
|
||||
if mbid, exists := eqClause["mbz_recording_id"]; exists && mbid == expectedMBID {
|
||||
foundMBID = true
|
||||
}
|
||||
if missing, exists := eqClause["missing"]; exists && missing == false {
|
||||
foundMissing = true
|
||||
}
|
||||
}
|
||||
}
|
||||
return foundMBID && foundMissing
|
||||
}
|
||||
}
|
||||
|
||||
func matchMediaFileByTitleFilter(expectedArtistID, expectedTitle string) func(opt model.QueryOptions) bool {
|
||||
return func(opt model.QueryOptions) bool {
|
||||
if opt.Filters == nil || opt.Max != 1 {
|
||||
return false
|
||||
}
|
||||
andClause, ok := opt.Filters.(squirrel.And)
|
||||
if !ok || len(andClause) < 3 {
|
||||
return false
|
||||
}
|
||||
foundArtist := false
|
||||
foundTitle := false
|
||||
foundMissing := false
|
||||
for _, condition := range andClause {
|
||||
if orClause, ok := condition.(squirrel.Or); ok {
|
||||
for _, orCond := range orClause {
|
||||
if eq, ok := orCond.(squirrel.Eq); ok {
|
||||
if id, exists := eq["artist_id"]; exists && id == expectedArtistID {
|
||||
foundArtist = true
|
||||
}
|
||||
if id, exists := eq["album_artist_id"]; exists && id == expectedArtistID {
|
||||
foundArtist = true
|
||||
}
|
||||
}
|
||||
}
|
||||
} else if likeClause, ok := condition.(squirrel.Like); ok {
|
||||
if _, exists := likeClause["order_title"]; exists {
|
||||
// We assume the title matches here for simplicity; could compare sanitized title if needed
|
||||
foundTitle = true
|
||||
}
|
||||
} else if eqClause, ok := condition.(squirrel.Eq); ok {
|
||||
if missing, exists := eqClause["missing"]; exists && missing == false {
|
||||
foundMissing = true
|
||||
}
|
||||
}
|
||||
}
|
||||
return foundArtist && foundTitle && foundMissing
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue