mirror of
https://github.com/navidrome/navidrome.git
synced 2025-12-23 23:18:05 -05:00
feat: make Unicode handling in external API calls configurable (#4277)
* feat: make Unicode handling in external API calls configurable - Add DevPreserveUnicodeInExternalCalls config option (default: false) - Refactor external provider to use NameForExternal() method on auxArtist - Remove redundant Name field from auxArtist struct - Update all external API calls (image, URL, biography, similar, top songs, MBID) to use configurable Unicode handling - Add comprehensive tests for both Unicode-preserving and normalized behaviors - Refactor tests to use constants and improved structure with BeforeEach blocks Fixes issue where Spotify integration failed to find artist images for artists with Unicode characters (e.g., en dash) in their names. Signed-off-by: Deluan <deluan@navidrome.org> * address comments Signed-off-by: Deluan <deluan@navidrome.org> * avoid calling str.Clean multiple times Signed-off-by: Deluan <deluan@navidrome.org> * refactor: apply Unicode handling pattern to auxAlbum Extended the configurable Unicode handling to album names, matching the pattern already implemented for artist names. This ensures consistent behavior when DevPreserveUnicodeInExternalCalls is enabled for both artist and album external API calls. Changes: - Removed Name field from auxAlbum struct, added Name() method with Unicode logic - Updated getAlbum, UpdateAlbumInfo, populateAlbumInfo, and AlbumImage functions - Added comprehensive tests for album Unicode handling (preserve and normalize) - Fixed typo in artist image test description --------- Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
@@ -106,32 +106,33 @@ type configOptions struct {
|
||||
Agents string
|
||||
|
||||
// DevFlags. These are used to enable/disable debugging and incomplete features
|
||||
DevLogLevels map[string]string `json:",omitempty"`
|
||||
DevLogSourceLine bool
|
||||
DevEnableProfiler bool
|
||||
DevAutoCreateAdminPassword string
|
||||
DevAutoLoginUsername string
|
||||
DevActivityPanel bool
|
||||
DevActivityPanelUpdateRate time.Duration
|
||||
DevSidebarPlaylists bool
|
||||
DevShowArtistPage bool
|
||||
DevUIShowConfig bool
|
||||
DevNewEventStream bool
|
||||
DevOffsetOptimize int
|
||||
DevArtworkMaxRequests int
|
||||
DevArtworkThrottleBacklogLimit int
|
||||
DevArtworkThrottleBacklogTimeout time.Duration
|
||||
DevArtistInfoTimeToLive time.Duration
|
||||
DevAlbumInfoTimeToLive time.Duration
|
||||
DevExternalScanner bool
|
||||
DevScannerThreads uint
|
||||
DevSelectiveWatcher bool
|
||||
DevInsightsInitialDelay time.Duration
|
||||
DevEnablePlayerInsights bool
|
||||
DevEnablePluginsInsights bool
|
||||
DevPluginCompilationTimeout time.Duration
|
||||
DevExternalArtistFetchMultiplier float64
|
||||
DevOptimizeDB bool
|
||||
DevLogLevels map[string]string `json:",omitempty"`
|
||||
DevLogSourceLine bool
|
||||
DevEnableProfiler bool
|
||||
DevAutoCreateAdminPassword string
|
||||
DevAutoLoginUsername string
|
||||
DevActivityPanel bool
|
||||
DevActivityPanelUpdateRate time.Duration
|
||||
DevSidebarPlaylists bool
|
||||
DevShowArtistPage bool
|
||||
DevUIShowConfig bool
|
||||
DevNewEventStream bool
|
||||
DevOffsetOptimize int
|
||||
DevArtworkMaxRequests int
|
||||
DevArtworkThrottleBacklogLimit int
|
||||
DevArtworkThrottleBacklogTimeout time.Duration
|
||||
DevArtistInfoTimeToLive time.Duration
|
||||
DevAlbumInfoTimeToLive time.Duration
|
||||
DevExternalScanner bool
|
||||
DevScannerThreads uint
|
||||
DevSelectiveWatcher bool
|
||||
DevInsightsInitialDelay time.Duration
|
||||
DevEnablePlayerInsights bool
|
||||
DevEnablePluginsInsights bool
|
||||
DevPluginCompilationTimeout time.Duration
|
||||
DevExternalArtistFetchMultiplier float64
|
||||
DevOptimizeDB bool
|
||||
DevPreserveUnicodeInExternalCalls bool
|
||||
}
|
||||
|
||||
type scannerOptions struct {
|
||||
@@ -630,6 +631,7 @@ func setViperDefaults() {
|
||||
viper.SetDefault("devplugincompilationtimeout", time.Minute)
|
||||
viper.SetDefault("devexternalartistfetchmultiplier", 1.5)
|
||||
viper.SetDefault("devoptimizedb", true)
|
||||
viper.SetDefault("devpreserveunicodeinexternalcalls", false)
|
||||
}
|
||||
|
||||
func init() {
|
||||
|
||||
89
core/external/provider.go
vendored
89
core/external/provider.go
vendored
@@ -51,12 +51,28 @@ type provider struct {
|
||||
|
||||
type auxAlbum struct {
|
||||
model.Album
|
||||
Name string
|
||||
}
|
||||
|
||||
// Name returns the appropriate album name for external API calls
|
||||
// based on the DevPreserveUnicodeInExternalCalls configuration option
|
||||
func (a *auxAlbum) Name() string {
|
||||
if conf.Server.DevPreserveUnicodeInExternalCalls {
|
||||
return a.Album.Name
|
||||
}
|
||||
return str.Clear(a.Album.Name)
|
||||
}
|
||||
|
||||
type auxArtist struct {
|
||||
model.Artist
|
||||
Name string
|
||||
}
|
||||
|
||||
// Name returns the appropriate artist name for external API calls
|
||||
// based on the DevPreserveUnicodeInExternalCalls configuration option
|
||||
func (a *auxArtist) Name() string {
|
||||
if conf.Server.DevPreserveUnicodeInExternalCalls {
|
||||
return a.Artist.Name
|
||||
}
|
||||
return str.Clear(a.Artist.Name)
|
||||
}
|
||||
|
||||
type Agents interface {
|
||||
@@ -88,7 +104,6 @@ func (e *provider) getAlbum(ctx context.Context, id string) (auxAlbum, error) {
|
||||
switch v := entity.(type) {
|
||||
case *model.Album:
|
||||
album.Album = *v
|
||||
album.Name = str.Clear(v.Name)
|
||||
case *model.MediaFile:
|
||||
return e.getAlbum(ctx, v.AlbumID)
|
||||
default:
|
||||
@@ -106,8 +121,9 @@ func (e *provider) UpdateAlbumInfo(ctx context.Context, id string) (*model.Album
|
||||
}
|
||||
|
||||
updatedAt := V(album.ExternalInfoUpdatedAt)
|
||||
albumName := album.Name()
|
||||
if updatedAt.IsZero() {
|
||||
log.Debug(ctx, "AlbumInfo not cached. Retrieving it now", "updatedAt", updatedAt, "id", id, "name", album.Name)
|
||||
log.Debug(ctx, "AlbumInfo not cached. Retrieving it now", "updatedAt", updatedAt, "id", id, "name", albumName)
|
||||
album, err = e.populateAlbumInfo(ctx, album)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -116,7 +132,7 @@ func (e *provider) UpdateAlbumInfo(ctx context.Context, id string) (*model.Album
|
||||
|
||||
// If info is expired, trigger a populateAlbumInfo in the background
|
||||
if time.Since(updatedAt) > conf.Server.DevAlbumInfoTimeToLive {
|
||||
log.Debug("Found expired cached AlbumInfo, refreshing in the background", "updatedAt", album.ExternalInfoUpdatedAt, "name", album.Name)
|
||||
log.Debug("Found expired cached AlbumInfo, refreshing in the background", "updatedAt", album.ExternalInfoUpdatedAt, "name", albumName)
|
||||
e.albumQueue.enqueue(&album)
|
||||
}
|
||||
|
||||
@@ -125,12 +141,13 @@ func (e *provider) UpdateAlbumInfo(ctx context.Context, id string) (*model.Album
|
||||
|
||||
func (e *provider) populateAlbumInfo(ctx context.Context, album auxAlbum) (auxAlbum, error) {
|
||||
start := time.Now()
|
||||
info, err := e.ag.GetAlbumInfo(ctx, album.Name, album.AlbumArtist, album.MbzAlbumID)
|
||||
albumName := album.Name()
|
||||
info, err := e.ag.GetAlbumInfo(ctx, albumName, album.AlbumArtist, album.MbzAlbumID)
|
||||
if errors.Is(err, agents.ErrNotFound) {
|
||||
return album, nil
|
||||
}
|
||||
if err != nil {
|
||||
log.Error("Error refreshing AlbumInfo", "id", album.ID, "name", album.Name, "artist", album.AlbumArtist,
|
||||
log.Error("Error refreshing AlbumInfo", "id", album.ID, "name", albumName, "artist", album.AlbumArtist,
|
||||
"elapsed", time.Since(start), err)
|
||||
return album, err
|
||||
}
|
||||
@@ -142,7 +159,7 @@ func (e *provider) populateAlbumInfo(ctx context.Context, album auxAlbum) (auxAl
|
||||
album.Description = info.Description
|
||||
}
|
||||
|
||||
images, err := e.ag.GetAlbumImages(ctx, album.Name, album.AlbumArtist, album.MbzAlbumID)
|
||||
images, err := e.ag.GetAlbumImages(ctx, albumName, album.AlbumArtist, album.MbzAlbumID)
|
||||
if err == nil && len(images) > 0 {
|
||||
sort.Slice(images, func(i, j int) bool {
|
||||
return images[i].Size > images[j].Size
|
||||
@@ -161,7 +178,7 @@ func (e *provider) populateAlbumInfo(ctx context.Context, album auxAlbum) (auxAl
|
||||
|
||||
err = e.ds.Album(ctx).UpdateExternalInfo(&album.Album)
|
||||
if err != nil {
|
||||
log.Error(ctx, "Error trying to update album external information", "id", album.ID, "name", album.Name,
|
||||
log.Error(ctx, "Error trying to update album external information", "id", album.ID, "name", albumName,
|
||||
"elapsed", time.Since(start), err)
|
||||
} else {
|
||||
log.Trace(ctx, "AlbumInfo collected", "album", album, "elapsed", time.Since(start))
|
||||
@@ -181,7 +198,6 @@ func (e *provider) getArtist(ctx context.Context, id string) (auxArtist, error)
|
||||
switch v := entity.(type) {
|
||||
case *model.Artist:
|
||||
artist.Artist = *v
|
||||
artist.Name = str.Clear(v.Name)
|
||||
case *model.MediaFile:
|
||||
return e.getArtist(ctx, v.ArtistID)
|
||||
case *model.Album:
|
||||
@@ -210,8 +226,9 @@ func (e *provider) refreshArtistInfo(ctx context.Context, id string) (auxArtist,
|
||||
|
||||
// If we don't have any info, retrieves it now
|
||||
updatedAt := V(artist.ExternalInfoUpdatedAt)
|
||||
artistName := artist.Name()
|
||||
if updatedAt.IsZero() {
|
||||
log.Debug(ctx, "ArtistInfo not cached. Retrieving it now", "updatedAt", updatedAt, "id", id, "name", artist.Name)
|
||||
log.Debug(ctx, "ArtistInfo not cached. Retrieving it now", "updatedAt", updatedAt, "id", id, "name", artistName)
|
||||
artist, err = e.populateArtistInfo(ctx, artist)
|
||||
if err != nil {
|
||||
return auxArtist{}, err
|
||||
@@ -220,7 +237,7 @@ func (e *provider) refreshArtistInfo(ctx context.Context, id string) (auxArtist,
|
||||
|
||||
// If info is expired, trigger a populateArtistInfo in the background
|
||||
if time.Since(updatedAt) > conf.Server.DevArtistInfoTimeToLive {
|
||||
log.Debug("Found expired cached ArtistInfo, refreshing in the background", "updatedAt", updatedAt, "name", artist.Name)
|
||||
log.Debug("Found expired cached ArtistInfo, refreshing in the background", "updatedAt", updatedAt, "name", artistName)
|
||||
e.artistQueue.enqueue(&artist)
|
||||
}
|
||||
return artist, nil
|
||||
@@ -229,8 +246,9 @@ func (e *provider) refreshArtistInfo(ctx context.Context, id string) (auxArtist,
|
||||
func (e *provider) populateArtistInfo(ctx context.Context, artist auxArtist) (auxArtist, error) {
|
||||
start := time.Now()
|
||||
// Get MBID first, if it is not yet available
|
||||
artistName := artist.Name()
|
||||
if artist.MbzArtistID == "" {
|
||||
mbid, err := e.ag.GetArtistMBID(ctx, artist.ID, artist.Name)
|
||||
mbid, err := e.ag.GetArtistMBID(ctx, artist.ID, artistName)
|
||||
if mbid != "" && err == nil {
|
||||
artist.MbzArtistID = mbid
|
||||
}
|
||||
@@ -246,14 +264,14 @@ func (e *provider) populateArtistInfo(ctx context.Context, artist auxArtist) (au
|
||||
_ = g.Wait()
|
||||
|
||||
if utils.IsCtxDone(ctx) {
|
||||
log.Warn(ctx, "ArtistInfo update canceled", "elapsed", "id", artist.ID, "name", artist.Name, time.Since(start), ctx.Err())
|
||||
log.Warn(ctx, "ArtistInfo update canceled", "id", artist.ID, "name", artistName, "elapsed", time.Since(start), ctx.Err())
|
||||
return artist, ctx.Err()
|
||||
}
|
||||
|
||||
artist.ExternalInfoUpdatedAt = P(time.Now())
|
||||
err := e.ds.Artist(ctx).UpdateExternalInfo(&artist.Artist)
|
||||
if err != nil {
|
||||
log.Error(ctx, "Error trying to update artist external information", "id", artist.ID, "name", artist.Name,
|
||||
log.Error(ctx, "Error trying to update artist external information", "id", artist.ID, "name", artistName,
|
||||
"elapsed", time.Since(start), err)
|
||||
} else {
|
||||
log.Trace(ctx, "ArtistInfo collected", "artist", artist, "elapsed", time.Since(start))
|
||||
@@ -281,7 +299,7 @@ func (e *provider) ArtistRadio(ctx context.Context, id string, count int) (model
|
||||
}
|
||||
|
||||
topCount := max(count, 20)
|
||||
topSongs, err := e.getMatchingTopSongs(ctx, e.ag, &auxArtist{Name: a.Name, Artist: a}, topCount)
|
||||
topSongs, err := e.getMatchingTopSongs(ctx, e.ag, &auxArtist{Artist: a}, topCount)
|
||||
if err != nil {
|
||||
log.Warn(ctx, "Error getting artist's top songs", "artist", a.Name, err)
|
||||
return nil
|
||||
@@ -344,22 +362,23 @@ func (e *provider) AlbumImage(ctx context.Context, id string) (*url.URL, error)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
images, err := e.ag.GetAlbumImages(ctx, album.Name, album.AlbumArtist, album.MbzAlbumID)
|
||||
albumName := album.Name()
|
||||
images, err := e.ag.GetAlbumImages(ctx, albumName, album.AlbumArtist, album.MbzAlbumID)
|
||||
if err != nil {
|
||||
switch {
|
||||
case errors.Is(err, agents.ErrNotFound):
|
||||
log.Trace(ctx, "Album not found in agent", "albumID", id, "name", album.Name, "artist", album.AlbumArtist)
|
||||
log.Trace(ctx, "Album not found in agent", "albumID", id, "name", albumName, "artist", album.AlbumArtist)
|
||||
return nil, model.ErrNotFound
|
||||
case errors.Is(err, context.Canceled):
|
||||
log.Debug(ctx, "GetAlbumImages call canceled", err)
|
||||
default:
|
||||
log.Warn(ctx, "Error getting album images from agent", "albumID", id, "name", album.Name, "artist", album.AlbumArtist, err)
|
||||
log.Warn(ctx, "Error getting album images from agent", "albumID", id, "name", albumName, "artist", album.AlbumArtist, err)
|
||||
}
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if len(images) == 0 {
|
||||
log.Warn(ctx, "Agent returned no images without error", "albumID", id, "name", album.Name, "artist", album.AlbumArtist)
|
||||
log.Warn(ctx, "Agent returned no images without error", "albumID", id, "name", albumName, "artist", album.AlbumArtist)
|
||||
return nil, model.ErrNotFound
|
||||
}
|
||||
|
||||
@@ -401,9 +420,10 @@ func (e *provider) TopSongs(ctx context.Context, artistName string, count int) (
|
||||
}
|
||||
|
||||
func (e *provider) getMatchingTopSongs(ctx context.Context, agent agents.ArtistTopSongsRetriever, artist *auxArtist, count int) (model.MediaFiles, error) {
|
||||
songs, err := agent.GetArtistTopSongs(ctx, artist.ID, artist.Name, artist.MbzArtistID, count)
|
||||
artistName := artist.Name()
|
||||
songs, err := agent.GetArtistTopSongs(ctx, artist.ID, artistName, artist.MbzArtistID, count)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to get top songs for artist %s: %w", artist.Name, err)
|
||||
return nil, fmt.Errorf("failed to get top songs for artist %s: %w", artistName, err)
|
||||
}
|
||||
|
||||
mbidMatches, err := e.loadTracksByMBID(ctx, songs)
|
||||
@@ -415,13 +435,13 @@ func (e *provider) getMatchingTopSongs(ctx context.Context, agent agents.ArtistT
|
||||
return nil, fmt.Errorf("failed to load tracks by title: %w", err)
|
||||
}
|
||||
|
||||
log.Trace(ctx, "Top Songs loaded", "name", artist.Name, "numSongs", len(songs), "numMBIDMatches", len(mbidMatches), "numTitleMatches", len(titleMatches))
|
||||
log.Trace(ctx, "Top Songs loaded", "name", artistName, "numSongs", len(songs), "numMBIDMatches", len(mbidMatches), "numTitleMatches", len(titleMatches))
|
||||
mfs := e.selectTopSongs(songs, mbidMatches, titleMatches, count)
|
||||
|
||||
if len(mfs) == 0 {
|
||||
log.Debug(ctx, "No matching top songs found", "name", artist.Name)
|
||||
log.Debug(ctx, "No matching top songs found", "name", artistName)
|
||||
} else {
|
||||
log.Debug(ctx, "Found matching top songs", "name", artist.Name, "numSongs", len(mfs))
|
||||
log.Debug(ctx, "Found matching top songs", "name", artistName, "numSongs", len(mfs))
|
||||
}
|
||||
|
||||
return mfs, nil
|
||||
@@ -518,7 +538,7 @@ func (e *provider) selectTopSongs(songs []agents.Song, byMBID, byTitle map[strin
|
||||
}
|
||||
|
||||
func (e *provider) callGetURL(ctx context.Context, agent agents.ArtistURLRetriever, artist *auxArtist) {
|
||||
artisURL, err := agent.GetArtistURL(ctx, artist.ID, artist.Name, artist.MbzArtistID)
|
||||
artisURL, err := agent.GetArtistURL(ctx, artist.ID, artist.Name(), artist.MbzArtistID)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
@@ -526,7 +546,7 @@ func (e *provider) callGetURL(ctx context.Context, agent agents.ArtistURLRetriev
|
||||
}
|
||||
|
||||
func (e *provider) callGetBiography(ctx context.Context, agent agents.ArtistBiographyRetriever, artist *auxArtist) {
|
||||
bio, err := agent.GetArtistBiography(ctx, artist.ID, str.Clear(artist.Name), artist.MbzArtistID)
|
||||
bio, err := agent.GetArtistBiography(ctx, artist.ID, artist.Name(), artist.MbzArtistID)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
@@ -536,7 +556,7 @@ func (e *provider) callGetBiography(ctx context.Context, agent agents.ArtistBiog
|
||||
}
|
||||
|
||||
func (e *provider) callGetImage(ctx context.Context, agent agents.ArtistImageRetriever, artist *auxArtist) {
|
||||
images, err := agent.GetArtistImages(ctx, artist.ID, artist.Name, artist.MbzArtistID)
|
||||
images, err := agent.GetArtistImages(ctx, artist.ID, artist.Name(), artist.MbzArtistID)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
@@ -555,13 +575,14 @@ func (e *provider) callGetImage(ctx context.Context, agent agents.ArtistImageRet
|
||||
|
||||
func (e *provider) callGetSimilar(ctx context.Context, agent agents.ArtistSimilarRetriever, artist *auxArtist,
|
||||
limit int, includeNotPresent bool) {
|
||||
similar, err := agent.GetSimilarArtists(ctx, artist.ID, artist.Name, artist.MbzArtistID, limit)
|
||||
artistName := artist.Name()
|
||||
similar, err := agent.GetSimilarArtists(ctx, artist.ID, artistName, artist.MbzArtistID, limit)
|
||||
if len(similar) == 0 || err != nil {
|
||||
return
|
||||
}
|
||||
start := time.Now()
|
||||
sa, err := e.mapSimilarArtists(ctx, similar, limit, includeNotPresent)
|
||||
log.Debug(ctx, "Mapped Similar Artists", "artist", artist.Name, "numSimilar", len(sa), "elapsed", time.Since(start))
|
||||
log.Debug(ctx, "Mapped Similar Artists", "artist", artistName, "numSimilar", len(sa), "elapsed", time.Since(start))
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
@@ -635,11 +656,7 @@ func (e *provider) findArtistByName(ctx context.Context, artistName string) (*au
|
||||
if len(artists) == 0 {
|
||||
return nil, model.ErrNotFound
|
||||
}
|
||||
artist := &auxArtist{
|
||||
Artist: artists[0],
|
||||
Name: str.Clear(artists[0].Name),
|
||||
}
|
||||
return artist, nil
|
||||
return &auxArtist{Artist: artists[0]}, nil
|
||||
}
|
||||
|
||||
func (e *provider) loadSimilar(ctx context.Context, artist *auxArtist, count int, includeNotPresent bool) error {
|
||||
@@ -655,7 +672,7 @@ func (e *provider) loadSimilar(ctx context.Context, artist *auxArtist, count int
|
||||
Filters: squirrel.Eq{"artist.id": ids},
|
||||
})
|
||||
if err != nil {
|
||||
log.Error("Error loading similar artists", "id", artist.ID, "name", artist.Name, err)
|
||||
log.Error("Error loading similar artists", "id", artist.ID, "name", artist.Name(), err)
|
||||
return err
|
||||
}
|
||||
|
||||
|
||||
63
core/external/provider_albumimage_test.go
vendored
63
core/external/provider_albumimage_test.go
vendored
@@ -260,6 +260,69 @@ var _ = Describe("Provider - AlbumImage", func() {
|
||||
mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "not-found")
|
||||
mockAlbumAgent.AssertNotCalled(GinkgoT(), "GetAlbumImages", mock.Anything, mock.Anything, mock.Anything)
|
||||
})
|
||||
|
||||
Context("Unicode handling in album names", func() {
|
||||
var albumWithEnDash *model.Album
|
||||
var expectedURL *url.URL
|
||||
|
||||
const (
|
||||
originalAlbumName = "Raising Hell–Deluxe" // Album name with en dash
|
||||
normalizedAlbumName = "Raising Hell-Deluxe" // Normalized version with hyphen
|
||||
)
|
||||
|
||||
BeforeEach(func() {
|
||||
// Test with en dash (–) in album name
|
||||
albumWithEnDash = &model.Album{ID: "album-endash", Name: originalAlbumName, AlbumArtistID: "artist-1"}
|
||||
mockArtistRepo.Mock = mock.Mock{} // Reset default expectations
|
||||
mockAlbumRepo.Mock = mock.Mock{} // Reset default expectations
|
||||
mockArtistRepo.On("Get", "album-endash").Return(nil, model.ErrNotFound).Once()
|
||||
mockAlbumRepo.On("Get", "album-endash").Return(albumWithEnDash, nil).Once()
|
||||
|
||||
expectedURL, _ = url.Parse("http://example.com/album.jpg")
|
||||
|
||||
// Mock the album agent to return an image for the album
|
||||
mockAlbumAgent.On("GetAlbumImages", ctx, mock.AnythingOfType("string"), "", "").
|
||||
Return([]agents.ExternalImage{
|
||||
{URL: "http://example.com/album.jpg", Size: 1000},
|
||||
}, nil).Once()
|
||||
})
|
||||
|
||||
When("DevPreserveUnicodeInExternalCalls is true", func() {
|
||||
BeforeEach(func() {
|
||||
conf.Server.DevPreserveUnicodeInExternalCalls = true
|
||||
})
|
||||
|
||||
It("preserves Unicode characters in album names", func() {
|
||||
// Act
|
||||
imgURL, err := provider.AlbumImage(ctx, "album-endash")
|
||||
|
||||
// Assert
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(imgURL).To(Equal(expectedURL))
|
||||
mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-endash")
|
||||
// This is the key assertion: ensure the original Unicode name is used
|
||||
mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumImages", ctx, originalAlbumName, "", "")
|
||||
})
|
||||
})
|
||||
|
||||
When("DevPreserveUnicodeInExternalCalls is false", func() {
|
||||
BeforeEach(func() {
|
||||
conf.Server.DevPreserveUnicodeInExternalCalls = false
|
||||
})
|
||||
|
||||
It("normalizes Unicode characters", func() {
|
||||
// Act
|
||||
imgURL, err := provider.AlbumImage(ctx, "album-endash")
|
||||
|
||||
// Assert
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(imgURL).To(Equal(expectedURL))
|
||||
mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-endash")
|
||||
// This assertion ensures the normalized name is used (en dash → hyphen)
|
||||
mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumImages", ctx, normalizedAlbumName, "", "")
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
// mockAlbumInfoAgent implementation
|
||||
|
||||
61
core/external/provider_artistimage_test.go
vendored
61
core/external/provider_artistimage_test.go
vendored
@@ -265,6 +265,67 @@ var _ = Describe("Provider - ArtistImage", func() {
|
||||
mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1")
|
||||
mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "")
|
||||
})
|
||||
|
||||
Context("Unicode handling in artist names", func() {
|
||||
var artistWithEnDash *model.Artist
|
||||
var expectedURL *url.URL
|
||||
|
||||
const (
|
||||
originalArtistName = "Run–D.M.C." // Artist name with en dash
|
||||
normalizedArtistName = "Run-D.M.C." // Normalized version with hyphen
|
||||
)
|
||||
|
||||
BeforeEach(func() {
|
||||
// Test with en dash (–) in artist name like "Run–D.M.C."
|
||||
artistWithEnDash = &model.Artist{ID: "artist-endash", Name: originalArtistName}
|
||||
mockArtistRepo.Mock = mock.Mock{} // Reset default expectations
|
||||
mockArtistRepo.On("Get", "artist-endash").Return(artistWithEnDash, nil).Once()
|
||||
|
||||
expectedURL, _ = url.Parse("http://example.com/rundmc.jpg")
|
||||
|
||||
// Mock the image agent to return an image for the artist
|
||||
mockImageAgent.On("GetArtistImages", ctx, "artist-endash", mock.AnythingOfType("string"), "").
|
||||
Return([]agents.ExternalImage{
|
||||
{URL: "http://example.com/rundmc.jpg", Size: 1000},
|
||||
}, nil).Once()
|
||||
|
||||
})
|
||||
|
||||
When("DevPreserveUnicodeInExternalCalls is true", func() {
|
||||
BeforeEach(func() {
|
||||
conf.Server.DevPreserveUnicodeInExternalCalls = true
|
||||
})
|
||||
It("preserves Unicode characters in artist names", func() {
|
||||
// Act
|
||||
imgURL, err := provider.ArtistImage(ctx, "artist-endash")
|
||||
|
||||
// Assert
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(imgURL).To(Equal(expectedURL))
|
||||
mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-endash")
|
||||
// This is the key assertion: ensure the original Unicode name is used
|
||||
mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-endash", originalArtistName, "")
|
||||
})
|
||||
})
|
||||
|
||||
When("DevPreserveUnicodeInExternalCalls is false", func() {
|
||||
BeforeEach(func() {
|
||||
conf.Server.DevPreserveUnicodeInExternalCalls = false
|
||||
})
|
||||
|
||||
It("normalizes Unicode characters", func() {
|
||||
// Act
|
||||
imgURL, err := provider.ArtistImage(ctx, "artist-endash")
|
||||
|
||||
// Assert
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(imgURL).To(Equal(expectedURL))
|
||||
mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-endash")
|
||||
// This assertion ensures the normalized name is used (en dash → hyphen)
|
||||
mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-endash", normalizedArtistName, "")
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
// mockArtistImageAgent implementation using testify/mock
|
||||
|
||||
Reference in New Issue
Block a user