Merge pull request #18 from skuill/bugfix/17_instrumental_track

#17 Error when searching for lyrics of an instrumental track
skuill · Dec 18, 2023 · f24603c · f24603c
2 parents 0c71753 + db48c40
commit f24603c
Show file tree

Hide file tree

Showing 47 changed files with 2,613 additions and 103 deletions.
diff --git a/LyricsScraperNET.Client/Program.cs b/LyricsScraperNET.Client/Program.cs
@@ -12,6 +12,7 @@
 using LyricsScraperNET.Models.Responses;
 using System.Threading.Tasks;
 using System;
+using Microsoft.Extensions.Logging;
 
 class Program
 {
@@ -21,18 +22,33 @@ static async Task Main()
         string artistToSearch = "Parkway Drive";
         string songToSearch = "Idols And Anchors";
 
+        //// The case when a song contains only an instrumental, without vocals.
+        //string artistToSearch = "Rush";
+        //string songToSearch = "YYZ";
+
         //// How to configure for ASP.NET applications:
         var result = ExampleWithHostConfiguration(artistToSearch, songToSearch);
 
         //// How to configure for a certain external provider:
         //var result = ExampleWithCertainProvider(artistToSearch, songToSearch);
 
-        //// Checking if something was found. 
-        //// If not, search errors can be found in the logs. 
+        //// Checking that something was found. The response can be empty in two cases:
+        //// 1) A search error occurred. Detailed information can be found in the logs or in response fields like 'ResponseStatusCode' and 'ResponseMessage'.
+        //// 2) The requested song contains only the instrumental, no lyrics. In this case the flag 'Instrumental' will be true.
         if (result.IsEmpty())
         {
-            Console.ForegroundColor = ConsoleColor.Red;
-            Console.WriteLine($"Can't find lyrics for: {artistToSearch} - {songToSearch}");
+            if (result.Instrumental)
+            {
+                Console.ForegroundColor = ConsoleColor.Gray;
+                Console.WriteLine($"This song [{artistToSearch} - {songToSearch}] is instrumental.\r\nIt does not contain any lyrics");
+            }
+            else
+            {
+                Console.ForegroundColor = ConsoleColor.Red;
+                Console.WriteLine($"Can't find lyrics for: [{artistToSearch} - {songToSearch}]. " +
+                    $"Status code: [{result.ResponseStatusCode}]. " +
+                    $"Response message: [{result.ResponseMessage}].");
+            }
             Console.ResetColor();
 
             Console.ReadLine();
@@ -41,13 +57,13 @@ static async Task Main()
 
         //// Output result to console
         Console.ForegroundColor = ConsoleColor.Yellow;
-        Console.WriteLine($"{artistToSearch} - {songToSearch}\r\n");
+        Console.WriteLine($"[{artistToSearch} - {songToSearch}]\r\n");
         Console.ResetColor();
 
         Console.WriteLine(result.LyricText);
 
         Console.ForegroundColor = ConsoleColor.Magenta;
-        Console.WriteLine($"\r\nThis text was found by {result.ExternalProviderType}\r\n");
+        Console.WriteLine($"\r\nThis lyric was found by [{result.ExternalProviderType}]\r\n");
         Console.ResetColor();
 
         Console.ReadLine();
@@ -105,6 +121,16 @@ ILyricsScraperClient lyricsScraperClient
                 .WithSongLyrics()
                 .WithLyricFind();
 
+        // To enable library logging, the LoggerFactory must be configured and passed to the client.
+        var loggerFactory = LoggerFactory.Create(builder =>
+        {
+            builder.AddFilter("Microsoft", LogLevel.Warning)
+                   .AddFilter("System", LogLevel.Warning)
+                   .AddFilter("LyricsScraperNET", LogLevel.Trace)
+                   .AddConsole();
+        });
+        lyricsScraperClient.WithLogger(loggerFactory);
+
         //// Another way to configure:
         //// 1. First create instance of LyricScraperClient.
         // ILyricsScraperClient lyricsScraperClient = new LyricsScraperClient();

diff --git a/LyricsScraperNET.TestShared/TestModel/LyricsTestData.cs b/LyricsScraperNET.TestShared/TestModel/LyricsTestData.cs
@@ -10,8 +10,11 @@ public class LyricsTestData
         public string SongName { get; set; }
         public string SongUri { get; set; }
 
-        public string LyricPageData => File.ReadAllText(LyricPagePath);
+        public string LyricPageData => ReadFileData(LyricPagePath);
 
-        public string LyricResultData => File.ReadAllText(LyricResultPath);
+        public string LyricResultData => ReadFileData(LyricResultPath);
+
+        private string ReadFileData(string path) =>
+            !string.IsNullOrEmpty(path) ? File.ReadAllText(path) : string.Empty;
     }
 }
diff --git a/LyricsScraperNET/Extensions/SearchResultExtensions.cs b/LyricsScraperNET/Extensions/SearchResultExtensions.cs
@@ -29,5 +29,12 @@ internal static SearchResult AppendResponseMessage(this SearchResult searchResul
                 : responseMessage;
             return searchResult;
         }
+
+        internal static SearchResult AddInstrumental(this SearchResult searchResult, bool instrumental)
+        {
+            searchResult.ResponseStatusCode = ResponseStatusCode.Success;
+            searchResult.Instrumental = instrumental;
+            return searchResult;
+        }
     }
 }
diff --git a/LyricsScraperNET/ILyricsScraperClient.cs b/LyricsScraperNET/ILyricsScraperClient.cs
@@ -2,6 +2,7 @@
 using LyricsScraperNET.Models.Responses;
 using LyricsScraperNET.Providers.Abstract;
 using LyricsScraperNET.Providers.Models;
+using Microsoft.Extensions.Logging;
 using System.Threading.Tasks;
 
 namespace LyricsScraperNET
@@ -49,5 +50,11 @@ public interface ILyricsScraperClient
         /// Calling the lyrics search method will return an empty result.
         /// </summary>
         void Disable();
+
+        /// <summary>
+        /// Creates a new ILogger instance from <paramref name="loggerFactory"/>.
+        /// Can be useful for error analysis if the lyric text is not found.
+        /// </summary>
+        void WithLogger(ILoggerFactory loggerFactory);
     }
 }
diff --git a/LyricsScraperNET/LyricsScraperClient.cs b/LyricsScraperNET/LyricsScraperClient.cs
@@ -16,8 +16,8 @@ namespace LyricsScraperNET
 {
     public sealed class LyricsScraperClient : ILyricsScraperClient
     {
-
-        private readonly ILogger<LyricsScraperClient> _logger;
+        private ILoggerFactory _loggerFactory;
+        private ILogger<LyricsScraperClient> _logger;
 
         private List<IExternalProvider> _externalProviders;
         private readonly ILyricScraperClientConfig _lyricScraperClientConfig;
@@ -70,15 +70,15 @@ public SearchResult SearchLyric(SearchRequest searchRequest)
             foreach (var externalProvider in GetAvailableProvidersForSearchRequest(searchRequest))
             {
                 var providerSearchResult = externalProvider.SearchLyric(searchRequest);
-                if (!providerSearchResult.IsEmpty())
+                if (!providerSearchResult.IsEmpty() || providerSearchResult.Instrumental)
                 {
                     return providerSearchResult;
                 }
-                _logger?.LogWarning($"Can't find lyric by provider: {externalProvider}.");
+                _logger?.LogWarning($"Can't find lyric by provider: [{externalProvider.Options?.ExternalProviderType}].");
             }
 
             searchResult.AddNoDataFoundMessage(Constants.ResponseMessages.NotFoundLyric);
-            _logger?.LogError($"Can't find lyrics for searchRequest: {searchRequest}.");
+            _logger?.LogError($"Can't find lyrics for searchRequest: [{searchRequest}].");
 
             return searchResult;
         }
@@ -102,15 +102,15 @@ public async Task<SearchResult> SearchLyricAsync(SearchRequest searchRequest)
             foreach (var externalProvider in GetAvailableProvidersForSearchRequest(searchRequest))
             {
                 var providerSearchResult = await externalProvider.SearchLyricAsync(searchRequest);
-                if (!providerSearchResult.IsEmpty())
+                if (!providerSearchResult.IsEmpty() || providerSearchResult.Instrumental)
                 {
                     return providerSearchResult;
                 }
-                _logger?.LogWarning($"Can't find lyric by provider: {externalProvider}.");
+                _logger?.LogWarning($"Can't find lyric by provider: [{externalProvider.Options?.ExternalProviderType}].");
             }
 
             searchResult.AddNoDataFoundMessage(Constants.ResponseMessages.NotFoundLyric);
-            _logger?.LogError($"Can't find lyrics for searchRequest: {searchRequest}.");
+            _logger?.LogError($"Can't find lyrics for searchRequest: [{searchRequest}].");
 
             return searchResult;
         }
@@ -200,7 +200,11 @@ public void AddProvider(IExternalProvider provider)
             if (IsEmptyProvidersList())
                 _externalProviders = new List<IExternalProvider>();
             if (!_externalProviders.Contains(provider))
+            {
+                if (_loggerFactory != null)
+                    provider.WithLogger(_loggerFactory);
                 _externalProviders.Add(provider);
+            }
             else
                 _logger?.LogWarning($"External provider {provider} already added");
         }
@@ -219,9 +223,7 @@ public void Enable()
                 return;
 
             foreach (var provider in _externalProviders)
-            {
                 provider.Enable();
-            }
         }
 
         public void Disable()
@@ -230,9 +232,19 @@ public void Disable()
                 return;
 
             foreach (var provider in _externalProviders)
-            {
                 provider.Disable();
-            }
+        }
+
+        public void WithLogger(ILoggerFactory loggerFactory)
+        {
+            _loggerFactory = loggerFactory;
+            _logger = loggerFactory.CreateLogger<LyricsScraperClient>();
+
+            if (IsEmptyProvidersList())
+                return;
+
+            foreach (var provider in _externalProviders)
+                provider.WithLogger(loggerFactory);
         }
 
         private bool IsEmptyProvidersList() => _externalProviders == null || !_externalProviders.Any();

diff --git a/LyricsScraperNET/Models/Requests/ArtistAndSongSearchRequest.cs b/LyricsScraperNET/Models/Requests/ArtistAndSongSearchRequest.cs
@@ -34,5 +34,10 @@ public ArtistAndSongSearchRequest(string artist, string song, ExternalProviderTy
         {
             Provider = provider;
         }
+
+        public override string ToString()
+        {
+            return $"Artist: [{Artist}]. Song: [{Song}]. Provider: [{Provider}]";
+        }
     }
 }
diff --git a/LyricsScraperNET/Models/Requests/UriSearchRequest.cs b/LyricsScraperNET/Models/Requests/UriSearchRequest.cs
@@ -35,5 +35,10 @@ public UriSearchRequest(string uri) : this(new Uri(uri))
 
         public UriSearchRequest(string uri, ExternalProviderType provider) : this(new Uri(uri), provider)
         { }
+
+        public override string ToString()
+        {
+            return $"Uri: [{Uri}]. Provider: [{Provider}]";
+        }
     }
 }
diff --git a/LyricsScraperNET/Models/Responses/SearchResult.cs b/LyricsScraperNET/Models/Responses/SearchResult.cs
@@ -47,6 +47,11 @@ internal SearchResult(string lyricText, ExternalProviderType externalProviderTyp
         /// </summary>
         public string ResponseMessage { get; internal set; } = string.Empty;
 
+        /// <summary>
+        /// The flag indicates that the search results are for music only, without text.
+        /// </summary>
+        public bool Instrumental { get; internal set; } = false;
+
         /// <summary>
         /// Returns true if the field <seealso cref="LyricText"/> is empty.
         /// </summary>

diff --git a/LyricsScraperNET/Providers/AZLyrics/AZLyricsProvider.cs b/LyricsScraperNET/Providers/AZLyrics/AZLyricsProvider.cs
@@ -11,7 +11,7 @@ namespace LyricsScraperNET.Providers.AZLyrics
 {
     public sealed class AZLyricsProvider : ExternalProviderBase
     {
-        private readonly ILogger<AZLyricsProvider> _logger;
+        private ILogger<AZLyricsProvider> _logger;
         private readonly IExternalUriConverter _uriConverter;
 
         private const string _lyricStart = "<!-- Usage of azlyrics.com content by any third-party lyrics provider is prohibited by our licensing agreement. Sorry about that. -->";
@@ -68,7 +68,7 @@ protected override SearchResult SearchLyric(Uri uri)
             if (WebClient == null || Parser == null)
             {
                 _logger?.LogWarning($"AZLyrics. Please set up WebClient and Parser first");
-                return new SearchResult();
+                return new SearchResult(Models.ExternalProviderType.AZLyrics);
             }
             var text = WebClient.Load(uri);
             return PostProcessLyric(uri, text);
@@ -88,28 +88,33 @@ protected override async Task<SearchResult> SearchLyricAsync(Uri uri)
             if (WebClient == null || Parser == null)
             {
                 _logger?.LogWarning($"AZLyrics. Please set up WebClient and Parser first");
-                return new SearchResult();
+                return new SearchResult(Models.ExternalProviderType.AZLyrics);
             }
             var text = await WebClient.LoadAsync(uri);
             return PostProcessLyric(uri, text);
         }
 
         #endregion
 
+        public override void WithLogger(ILoggerFactory loggerFactory)
+        {
+            _logger = loggerFactory.CreateLogger<AZLyricsProvider>();
+        }
+
         private SearchResult PostProcessLyric(Uri uri, string text)
         {
             if (string.IsNullOrEmpty(text))
             {
-                _logger?.LogWarning($"AZLyrics. Text is empty for {uri}");
-                return new SearchResult();
+                _logger?.LogWarning($"AZLyrics. Text is empty for Uri: [{uri}]");
+                return new SearchResult(Models.ExternalProviderType.AZLyrics);
             }
 
             var startIndex = text.IndexOf(_lyricStart);
             var endIndex = text.IndexOf(_lyricEnd);
             if (startIndex <= 0 || endIndex <= 0)
             {
-                _logger?.LogWarning($"AZLyrics. Can't find lyrics for {uri}");
-                return new SearchResult();
+                _logger?.LogWarning($"AZLyrics. Can't find lyrics for Uri: [{uri}]");
+                return new SearchResult(Models.ExternalProviderType.AZLyrics);
             }
 
             string result = Parser.Parse(text.Substring(startIndex, endIndex - startIndex));

diff --git a/LyricsScraperNET/Providers/Abstract/ExternalProviderBase.cs b/LyricsScraperNET/Providers/Abstract/ExternalProviderBase.cs
@@ -1,6 +1,7 @@
 using LyricsScraperNET.Models.Requests;
 using LyricsScraperNET.Models.Responses;
 using LyricsScraperNET.Network.Abstract;
+using Microsoft.Extensions.Logging;
 using System;
 using System.Threading.Tasks;
 
@@ -95,5 +96,8 @@ public void Disable()
             if (Options != null)
                 Options.Enabled = false;
         }
+
+        public virtual void WithLogger(ILoggerFactory loggerFactory)
+            => throw new NotImplementedException();
     }
 }
diff --git a/LyricsScraperNET/Providers/Abstract/IExternalProvider.cs b/LyricsScraperNET/Providers/Abstract/IExternalProvider.cs
@@ -1,6 +1,7 @@
 using LyricsScraperNET.Models.Requests;
 using LyricsScraperNET.Models.Responses;
 using LyricsScraperNET.Network.Abstract;
+using Microsoft.Extensions.Logging;
 using System.Threading.Tasks;
 
 namespace LyricsScraperNET.Providers.Abstract
@@ -27,5 +28,7 @@ public interface IExternalProvider
         void Enable();
 
         void Disable();
+
+        void WithLogger(ILoggerFactory loggerFactory);
     }
 }