From 2a69071e23c21699c7822511a28b58096a804a54 Mon Sep 17 00:00:00 2001 From: skuill Date: Thu, 8 Feb 2024 17:47:21 +0400 Subject: [PATCH] #21. Fixed Musixmatch token regeneration logic. MusixmatchProvider refactoring. --- .../ServiceCollectionExtensions.cs | 2 + LyricsScraperNET/LyricsScraperNET.csproj | 4 +- .../Musixmatch/IMusixmatchClientWrapper.cs | 15 ++ .../Musixmatch/IMusixmatchTokenCache.cs | 15 ++ .../Musixmatch/MusixmatchClientWrapper.cs | 104 ++++++++++++++ .../Musixmatch/MusixmatchProvider.cs | 134 +++--------------- .../Musixmatch/MusixmatchTokenCache.cs | 71 ++++++++++ .../Musixmatch/MusixmatchProviderTest.cs | 43 ++++++ 8 files changed, 270 insertions(+), 118 deletions(-) create mode 100644 LyricsScraperNET/Providers/Musixmatch/IMusixmatchClientWrapper.cs create mode 100644 LyricsScraperNET/Providers/Musixmatch/IMusixmatchTokenCache.cs create mode 100644 LyricsScraperNET/Providers/Musixmatch/MusixmatchClientWrapper.cs create mode 100644 LyricsScraperNET/Providers/Musixmatch/MusixmatchTokenCache.cs create mode 100644 Tests/LyricsScraperNET.UnitTest/Providers/Musixmatch/MusixmatchProviderTest.cs diff --git a/LyricsScraperNET/Configuration/ServiceCollectionExtensions.cs b/LyricsScraperNET/Configuration/ServiceCollectionExtensions.cs index bd33898..b010ba8 100644 --- a/LyricsScraperNET/Configuration/ServiceCollectionExtensions.cs +++ b/LyricsScraperNET/Configuration/ServiceCollectionExtensions.cs @@ -73,6 +73,8 @@ public static IServiceCollection AddMusixmatchService( { services.Configure(configurationSection); + services.AddSingleton(); + services.AddScoped(); services.AddScoped(typeof(IExternalProvider), typeof(MusixmatchProvider)); } diff --git a/LyricsScraperNET/LyricsScraperNET.csproj b/LyricsScraperNET/LyricsScraperNET.csproj index 55f05d7..e3a49cc 100644 --- a/LyricsScraperNET/LyricsScraperNET.csproj +++ b/LyricsScraperNET/LyricsScraperNET.csproj @@ -28,14 +28,14 @@ - + - + diff --git a/LyricsScraperNET/Providers/Musixmatch/IMusixmatchClientWrapper.cs b/LyricsScraperNET/Providers/Musixmatch/IMusixmatchClientWrapper.cs new file mode 100644 index 0000000..4736ff4 --- /dev/null +++ b/LyricsScraperNET/Providers/Musixmatch/IMusixmatchClientWrapper.cs @@ -0,0 +1,15 @@ +using LyricsScraperNET.Models.Responses; +using System.Threading.Tasks; + +namespace LyricsScraperNET.Providers.Musixmatch +{ + /// + /// Decorator for the + /// + public interface IMusixmatchClientWrapper + { + SearchResult SearchLyric(string artist, string song, bool regenerateToken = false); + + Task SearchLyricAsync(string artist, string song, bool regenerateToken = false); + } +} diff --git a/LyricsScraperNET/Providers/Musixmatch/IMusixmatchTokenCache.cs b/LyricsScraperNET/Providers/Musixmatch/IMusixmatchTokenCache.cs new file mode 100644 index 0000000..7507fb5 --- /dev/null +++ b/LyricsScraperNET/Providers/Musixmatch/IMusixmatchTokenCache.cs @@ -0,0 +1,15 @@ +namespace LyricsScraperNET.Providers.Musixmatch +{ + /// + /// Token cache provider + /// + public interface IMusixmatchTokenCache + { + /// + /// Get or create token from cache. + /// + /// If true, then the token will be created again. + /// + string GetOrCreateToken(bool regenerate = false); + } +} diff --git a/LyricsScraperNET/Providers/Musixmatch/MusixmatchClientWrapper.cs b/LyricsScraperNET/Providers/Musixmatch/MusixmatchClientWrapper.cs new file mode 100644 index 0000000..9873c5d --- /dev/null +++ b/LyricsScraperNET/Providers/Musixmatch/MusixmatchClientWrapper.cs @@ -0,0 +1,104 @@ +using LyricsScraperNET.Extensions; +using LyricsScraperNET.Models.Responses; +using Microsoft.Extensions.Logging; +using MusixmatchClientLib; +using MusixmatchClientLib.API.Model.Types; +using MusixmatchClientLib.Types; +using System.Linq; +using System.Threading.Tasks; + +namespace LyricsScraperNET.Providers.Musixmatch +{ + public sealed class MusixmatchClientWrapper : IMusixmatchClientWrapper + { + private ILogger _logger; + private IMusixmatchTokenCache _tokenCache; + + public MusixmatchClientWrapper() + { + } + + public MusixmatchClientWrapper(IMusixmatchTokenCache tokenCache) + { + _tokenCache = tokenCache; + } + + public MusixmatchClientWrapper(ILogger logger, IMusixmatchTokenCache tokenCache) + : this(tokenCache) + { + _logger = logger; + } + + public SearchResult SearchLyric(string artist, string song, bool regenerateToken = false) + { + var client = GetMusixmatchClient(regenerateToken); + var trackSearchParameters = GetTrackSearchParameters(artist, song); + + var trackId = client.SongSearch(trackSearchParameters)?.FirstOrDefault()?.TrackId; + if (trackId != null) + { + Lyrics lyrics = client.GetTrackLyrics(trackId.Value); + + // lyrics.LyricsBody is null when the track is instrumental + if (lyrics.Instrumental != 1) + return new SearchResult(lyrics.LyricsBody, Models.ExternalProviderType.Musixmatch); + + // Instrumental music without lyric + return new SearchResult(Models.ExternalProviderType.Musixmatch) + .AddInstrumental(true); + } + else + { + _logger?.LogWarning($"Musixmatch. Can't find any information about artist {artist} and song {song}"); + return new SearchResult(Models.ExternalProviderType.Musixmatch); + } + } + + public async Task SearchLyricAsync(string artist, string song, bool regenerateToken = false) + { + var client = GetMusixmatchClient(regenerateToken); + var trackSearchParameters = GetTrackSearchParameters(artist, song); + + var songSearchTask = await client.SongSearchAsync(trackSearchParameters); + + var trackId = songSearchTask?.FirstOrDefault()?.TrackId; + if (trackId != null) + { + Lyrics lyrics = await client.GetTrackLyricsAsync(trackId.Value); + + // lyrics.LyricsBody is null when the track is instrumental + if (lyrics.Instrumental != 1) + return new SearchResult(lyrics.LyricsBody, Models.ExternalProviderType.Musixmatch); + + // Instrumental music without lyric + return new SearchResult(Models.ExternalProviderType.Musixmatch) + .AddInstrumental(true); + } + else + { + _logger?.LogWarning($"Musixmatch. Can't find any information about artist {artist} and song {song}"); + return new SearchResult(Models.ExternalProviderType.Musixmatch); + } + } + + private MusixmatchClient GetMusixmatchClient(bool regenerateToken = false) + { + var musixmatchToken = _tokenCache.GetOrCreateToken(regenerateToken); + return new MusixmatchClient(musixmatchToken); + } + + private TrackSearchParameters GetTrackSearchParameters(string artist, string song) + { + return new TrackSearchParameters + { + Artist = artist, + Title = song, // Track name + //Query = $"{artist} - {song}", // Search query, covers all the search parameters above + //HasLyrics = false, // Only search for tracks with lyrics + //HasSubtitles = false, // Only search for tracks with synced lyrics + //Language = "", // Only search for tracks with lyrics in specified language + Sort = TrackSearchParameters.SortStrategy.TrackRatingDesc // List sorting strategy + }; + } + } +} diff --git a/LyricsScraperNET/Providers/Musixmatch/MusixmatchProvider.cs b/LyricsScraperNET/Providers/Musixmatch/MusixmatchProvider.cs index 8a5b288..91cd734 100644 --- a/LyricsScraperNET/Providers/Musixmatch/MusixmatchProvider.cs +++ b/LyricsScraperNET/Providers/Musixmatch/MusixmatchProvider.cs @@ -1,17 +1,11 @@ -using LyricsScraperNET.Extensions; -using LyricsScraperNET.Helpers; +using LyricsScraperNET.Helpers; using LyricsScraperNET.Models.Responses; using LyricsScraperNET.Providers.Abstract; -using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using MusixmatchClientLib; using MusixmatchClientLib.API.Model.Exceptions; using MusixmatchClientLib.API.Model.Types; -using MusixmatchClientLib.Auth; -using MusixmatchClientLib.Types; using System; -using System.Linq; using System.Threading.Tasks; namespace LyricsScraperNET.Providers.Musixmatch @@ -19,55 +13,43 @@ namespace LyricsScraperNET.Providers.Musixmatch public sealed class MusixmatchProvider : ExternalProviderBase { private ILogger _logger; - - // Musixmatch Token memory cache - private static readonly IMemoryCache _memoryCache; - private static readonly MemoryCacheEntryOptions _memoryCacheEntryOptions; - - private static readonly string MusixmatchTokenKey = "MusixmatchToken"; + private IMusixmatchClientWrapper _clientWrapper; private readonly int _searchRetryAmount = 2; #region Constructors - static MusixmatchProvider() - { - _memoryCache = new MemoryCache(new MemoryCacheOptions() - { - SizeLimit = 1024, - }); - _memoryCacheEntryOptions = new MemoryCacheEntryOptions() - { - Size = 1 - }; - } - public MusixmatchProvider() { Options = new MusixmatchOptions() { Enabled = true }; + + var tokenCache = new MusixmatchTokenCache(); + _clientWrapper = new MusixmatchClientWrapper(tokenCache); } - public MusixmatchProvider(ILogger logger, MusixmatchOptions options) + public MusixmatchProvider(ILogger logger, MusixmatchOptions options, IMusixmatchClientWrapper clientWrapper) { _logger = logger; Ensure.ArgumentNotNull(options, nameof(options)); Options = options; + Ensure.ArgumentNotNull(clientWrapper, nameof(clientWrapper)); + _clientWrapper = clientWrapper; } - public MusixmatchProvider(ILogger logger, IOptionsSnapshot options) - : this(logger, options.Value) + public MusixmatchProvider(ILogger logger, IOptionsSnapshot options, IMusixmatchClientWrapper clientWrapper) + : this(logger, options.Value, clientWrapper) { Ensure.ArgumentNotNull(options, nameof(options)); } - public MusixmatchProvider(MusixmatchOptions options) - : this(null, options) + public MusixmatchProvider(MusixmatchOptions options, IMusixmatchClientWrapper clientWrapper) + : this(null, options, clientWrapper) { Ensure.ArgumentNotNull(options, nameof(options)); } - public MusixmatchProvider(IOptionsSnapshot options) - : this(null, options.Value) + public MusixmatchProvider(IOptionsSnapshot options, IMusixmatchClientWrapper clientWrapper) + : this(null, options.Value, clientWrapper) { Ensure.ArgumentNotNull(options, nameof(options)); } @@ -86,32 +68,13 @@ protected override SearchResult SearchLyric(Uri uri) protected override SearchResult SearchLyric(string artist, string song) { - int? trackId; bool regenerateToken = false; for (int i = 1; i <= _searchRetryAmount; i++) { - var client = GetMusixmatchClient(regenerateToken); - var trackSearchParameters = GetTrackSearchParameters(artist, song); try { - trackId = client.SongSearch(trackSearchParameters)?.FirstOrDefault()?.TrackId; - if (trackId != null) - { - Lyrics lyrics = client.GetTrackLyrics(trackId.Value); - - // lyrics.LyricsBody is null when the track is instrumental - if (lyrics.Instrumental != 1) - return new SearchResult(lyrics.LyricsBody, Models.ExternalProviderType.Musixmatch); - - // Instrumental music without lyric - return new SearchResult(Models.ExternalProviderType.Musixmatch) - .AddInstrumental(true); - } - else - { - _logger?.LogWarning($"Musixmatch. Can't find any information about artist {artist} and song {song}"); - return new SearchResult(Models.ExternalProviderType.Musixmatch); - } + var result = _clientWrapper.SearchLyric(artist, song, regenerateToken); + return result; } catch (MusixmatchRequestException requestException) when (requestException.StatusCode == StatusCode.AuthFailed) { @@ -137,30 +100,10 @@ protected override async Task SearchLyricAsync(string artist, stri bool regenerateToken = false; for (int i = 1; i <= _searchRetryAmount; i++) { - var client = GetMusixmatchClient(regenerateToken); - var trackSearchParameters = GetTrackSearchParameters(artist, song); try { - var songSearchTask = await client.SongSearchAsync(trackSearchParameters); - - var trackId = songSearchTask?.FirstOrDefault()?.TrackId; - if (trackId != null) - { - Lyrics lyrics = await client.GetTrackLyricsAsync(trackId.Value); - - // lyrics.LyricsBody is null when the track is instrumental - if (lyrics.Instrumental != 1) - return new SearchResult(lyrics.LyricsBody, Models.ExternalProviderType.Musixmatch); - - // Instrumental music without lyric - return new SearchResult(Models.ExternalProviderType.Musixmatch) - .AddInstrumental(true); - } - else - { - _logger?.LogWarning($"Musixmatch. Can't find any information about artist {artist} and song {song}"); - return new SearchResult(Models.ExternalProviderType.Musixmatch); - } + var result = await _clientWrapper.SearchLyricAsync(artist, song, regenerateToken); + return result; } catch (MusixmatchRequestException requestException) when (requestException.StatusCode == StatusCode.AuthFailed) { @@ -177,46 +120,5 @@ public override void WithLogger(ILoggerFactory loggerFactory) { _logger = loggerFactory.CreateLogger(); } - - private MusixmatchClient GetMusixmatchClient(bool regenerateToken = false) - { - // TODO: uncomment after the fix of https://github.com/Eimaen/MusixmatchClientLib/issues/21 - //if (Options.TryGetApiKeyFromOptions(out var apiKey)) - //{ - // _logger.LogInformation("Use MusixmatchToken from options."); - // return new MusixmatchClient(apiKey); - //} - //else - //{ - if (regenerateToken) - _memoryCache.Remove(MusixmatchTokenKey); - - _logger?.LogDebug("Musixmatch. Use default MusixmatchToken."); - string musixmatchTokenValue; - if (!_memoryCache.TryGetValue(MusixmatchTokenKey, out musixmatchTokenValue)) - { - _logger?.LogDebug("Musixmatch. Generate new token."); - var musixmatchToken = new MusixmatchToken(); - musixmatchTokenValue = musixmatchToken.Token; - _memoryCache.Set(MusixmatchTokenKey, musixmatchTokenValue, _memoryCacheEntryOptions); - } - (Options as IExternalProviderOptionsWithApiKey).ApiKey = musixmatchTokenValue; - return new MusixmatchClient(musixmatchTokenValue); - //} - } - - private TrackSearchParameters GetTrackSearchParameters(string artist, string song) - { - return new TrackSearchParameters - { - Artist = artist, - Title = song, // Track name - //Query = $"{artist} - {song}", // Search query, covers all the search parameters above - //HasLyrics = false, // Only search for tracks with lyrics - //HasSubtitles = false, // Only search for tracks with synced lyrics - //Language = "", // Only search for tracks with lyrics in specified language - Sort = TrackSearchParameters.SortStrategy.TrackRatingDesc // List sorting strategy - }; - } } } diff --git a/LyricsScraperNET/Providers/Musixmatch/MusixmatchTokenCache.cs b/LyricsScraperNET/Providers/Musixmatch/MusixmatchTokenCache.cs new file mode 100644 index 0000000..013b4d7 --- /dev/null +++ b/LyricsScraperNET/Providers/Musixmatch/MusixmatchTokenCache.cs @@ -0,0 +1,71 @@ +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Logging; +using MusixmatchClientLib.Auth; + +namespace LyricsScraperNET.Providers.Musixmatch +{ + public sealed class MusixmatchTokenCache : IMusixmatchTokenCache + { + private ILogger _logger; + + // Musixmatch Token memory cache + private static IMemoryCache _memoryCache; + private static MemoryCacheEntryOptions _memoryCacheEntryOptions; + + private static readonly object _syncLock = new object(); + + private readonly string MusixmatchTokenKey = "MusixmatchToken"; + + public MusixmatchTokenCache() + { + if (_memoryCache == null) + { + lock (_syncLock) + { + if (_memoryCache == null) + { + _memoryCache = new MemoryCache(new MemoryCacheOptions() + { + SizeLimit = 1024, + }); + } + } + } + if (_memoryCacheEntryOptions == null) + { + lock (_syncLock) + { + if (_memoryCacheEntryOptions == null) + { + _memoryCacheEntryOptions = new MemoryCacheEntryOptions() + { + Size = 1 + }; + } + } + } + } + + public MusixmatchTokenCache(ILogger logger) : this() + { + _logger = logger; + } + + public string GetOrCreateToken(bool regenerate = false) + { + if (regenerate) + _memoryCache.Remove(MusixmatchTokenKey); + + _logger?.LogDebug("Musixmatch. Use default MusixmatchToken."); + string musixmatchTokenValue; + if (!_memoryCache.TryGetValue(MusixmatchTokenKey, out musixmatchTokenValue)) + { + _logger?.LogDebug("Musixmatch. Generate new token."); + var musixmatchToken = new MusixmatchToken(); + musixmatchTokenValue = musixmatchToken.Token; + _memoryCache.Set(MusixmatchTokenKey, musixmatchTokenValue, _memoryCacheEntryOptions); + } + return musixmatchTokenValue; + } + } +} diff --git a/Tests/LyricsScraperNET.UnitTest/Providers/Musixmatch/MusixmatchProviderTest.cs b/Tests/LyricsScraperNET.UnitTest/Providers/Musixmatch/MusixmatchProviderTest.cs new file mode 100644 index 0000000..5dbe109 --- /dev/null +++ b/Tests/LyricsScraperNET.UnitTest/Providers/Musixmatch/MusixmatchProviderTest.cs @@ -0,0 +1,43 @@ +using LyricsScraperNET.Models.Requests; +using LyricsScraperNET.Models.Responses; +using LyricsScraperNET.Providers.Models; +using LyricsScraperNET.Providers.Musixmatch; +using LyricsScraperNET.TestShared.Providers; +using Moq; +using MusixmatchClientLib.API.Model.Exceptions; +using Xunit; + +namespace LyricsScraperNET.UnitTest.Providers.Musixmatch +{ + public class MusixmatchProviderTest : ProviderTestBase + { + [Fact] + public void SearchLyric_ThrowAuthFailedException_ShouldRegenerateToken() + { + // Arrange + string expectedLyric = "Жидкий кот, У ворот, мур-мур-мур, Он поёт."; + var searchRequest = new ArtistAndSongSearchRequest("Дымка", "Мау"); + + var clientWrapperMock = new Mock(); + + // First call. Throw an exception if the token is invalid or expired. + clientWrapperMock.Setup(c => c.SearchLyric(It.IsAny(), It.IsAny(), It.Is(r => r == false))) + .Returns(() => throw new MusixmatchRequestException(MusixmatchClientLib.API.Model.Types.StatusCode.AuthFailed)); + + // Second call. A repeated call with token regeneration is expected. + clientWrapperMock.Setup(c => c.SearchLyric(It.IsAny(), It.IsAny(), It.Is(r => r == true))) + .Returns(new SearchResult(expectedLyric, ExternalProviderType.Musixmatch)); + + var options = new MusixmatchOptions() { Enabled = true }; + var lyricsClient = new MusixmatchProvider(null, options, clientWrapperMock.Object); + + var result = lyricsClient.SearchLyric(searchRequest); + + // Assert + Assert.NotNull(result); + Assert.Equal(expectedLyric, result.LyricText); + clientWrapperMock.Verify(c => c.SearchLyric(It.IsAny(), It.IsAny(), It.Is(r => r == false)), Times.Once); + clientWrapperMock.Verify(c => c.SearchLyric(It.IsAny(), It.IsAny(), It.Is(r => r == true)), Times.Once); + } + } +}