diff --git a/conf/configuration.go b/conf/configuration.go index 77e9c94a..9d7b1a3d 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -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() { diff --git a/core/external/provider.go b/core/external/provider.go index 8e9a458c..413c7e0c 100644 --- a/core/external/provider.go +++ b/core/external/provider.go @@ -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 } diff --git a/core/external/provider_albumimage_test.go b/core/external/provider_albumimage_test.go index 9b682462..8a81b4f4 100644 --- a/core/external/provider_albumimage_test.go +++ b/core/external/provider_albumimage_test.go @@ -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 diff --git a/core/external/provider_artistimage_test.go b/core/external/provider_artistimage_test.go index 96341836..11290bb6 100644 --- a/core/external/provider_artistimage_test.go +++ b/core/external/provider_artistimage_test.go @@ -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