Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#17 Error when searching for lyrics of an instrumental track #18

Merged
merged 5 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions LyricsScraperNET.Client/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using LyricsScraperNET.Models.Responses;
using System.Threading.Tasks;
using System;
using Microsoft.Extensions.Logging;

class Program
{
Expand All @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
7 changes: 5 additions & 2 deletions LyricsScraperNET.TestShared/TestModel/LyricsTestData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
7 changes: 7 additions & 0 deletions LyricsScraperNET/Extensions/SearchResultExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
7 changes: 7 additions & 0 deletions LyricsScraperNET/ILyricsScraperClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
36 changes: 24 additions & 12 deletions LyricsScraperNET/LyricsScraperClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
{
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;
Expand All @@ -26,14 +26,14 @@

public IExternalProvider this[ExternalProviderType providerType]
{
get => IsProviderAvailable(providerType)

Check warning on line 29 in LyricsScraperNET/LyricsScraperClient.cs

View workflow job for this annotation

GitHub Actions / lyrics_scraper_net-cicd

Possible null reference return.

Check warning on line 29 in LyricsScraperNET/LyricsScraperClient.cs

View workflow job for this annotation

GitHub Actions / lyrics_scraper_net-cicd

Possible null reference return.
? _externalProviders.First(p => p.Options.ExternalProviderType == providerType)
: null;
}

public LyricsScraperClient() { }

Check warning on line 34 in LyricsScraperNET/LyricsScraperClient.cs

View workflow job for this annotation

GitHub Actions / lyrics_scraper_net-cicd

Non-nullable field '_loggerFactory' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.

Check warning on line 34 in LyricsScraperNET/LyricsScraperClient.cs

View workflow job for this annotation

GitHub Actions / lyrics_scraper_net-cicd

Non-nullable field '_logger' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.

Check warning on line 34 in LyricsScraperNET/LyricsScraperClient.cs

View workflow job for this annotation

GitHub Actions / lyrics_scraper_net-cicd

Non-nullable field '_loggerFactory' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.

Check warning on line 34 in LyricsScraperNET/LyricsScraperClient.cs

View workflow job for this annotation

GitHub Actions / lyrics_scraper_net-cicd

Non-nullable field '_logger' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.

Check warning on line 34 in LyricsScraperNET/LyricsScraperClient.cs

View workflow job for this annotation

GitHub Actions / lyrics_scraper_net-cicd

Non-nullable field '_externalProviders' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.

Check warning on line 34 in LyricsScraperNET/LyricsScraperClient.cs

View workflow job for this annotation

GitHub Actions / lyrics_scraper_net-cicd

Non-nullable field '_lyricScraperClientConfig' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.

public LyricsScraperClient(ILyricScraperClientConfig lyricScraperClientConfig,

Check warning on line 36 in LyricsScraperNET/LyricsScraperClient.cs

View workflow job for this annotation

GitHub Actions / lyrics_scraper_net-cicd

Non-nullable field '_loggerFactory' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.
IEnumerable<IExternalProvider> externalProviders)
{
Ensure.ArgumentNotNull(lyricScraperClientConfig, nameof(lyricScraperClientConfig));
Expand Down Expand Up @@ -70,15 +70,15 @@
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;
}
Expand All @@ -102,15 +102,15 @@
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;
}
Expand Down Expand Up @@ -200,7 +200,11 @@
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");
}
Expand All @@ -219,9 +223,7 @@
return;

foreach (var provider in _externalProviders)
{
provider.Enable();
}
}

public void Disable()
Expand All @@ -230,9 +232,19 @@
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,10 @@ public ArtistAndSongSearchRequest(string artist, string song, ExternalProviderTy
{
Provider = provider;
}

public override string ToString()
{
return $"Artist: [{Artist}]. Song: [{Song}]. Provider: [{Provider}]";
}
}
}
5 changes: 5 additions & 0 deletions LyricsScraperNET/Models/Requests/UriSearchRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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}]";
}
}
}
5 changes: 5 additions & 0 deletions LyricsScraperNET/Models/Responses/SearchResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
19 changes: 12 additions & 7 deletions LyricsScraperNET/Providers/AZLyrics/AZLyricsProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
{
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. -->";
Expand All @@ -19,7 +19,7 @@

#region Constructors

public AZLyricsProvider()

Check warning on line 22 in LyricsScraperNET/Providers/AZLyrics/AZLyricsProvider.cs

View workflow job for this annotation

GitHub Actions / lyrics_scraper_net-cicd

Non-nullable field '_logger' must contain a non-null value when exiting constructor. Consider declaring the field as nullable.
{
Parser = new AZLyricsParser();
WebClient = new HtmlAgilityWebClient();
Expand All @@ -41,7 +41,7 @@
}

public AZLyricsProvider(AZLyricsOptions options)
: this(null, options)

Check warning on line 44 in LyricsScraperNET/Providers/AZLyrics/AZLyricsProvider.cs

View workflow job for this annotation

GitHub Actions / lyrics_scraper_net-cicd

Cannot convert null literal to non-nullable reference type.
{
Ensure.ArgumentNotNull(options, nameof(options));
}
Expand All @@ -68,7 +68,7 @@
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);
Expand All @@ -88,28 +88,33 @@
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));
Expand Down
4 changes: 4 additions & 0 deletions LyricsScraperNET/Providers/Abstract/ExternalProviderBase.cs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -95,5 +96,8 @@ public void Disable()
if (Options != null)
Options.Enabled = false;
}

public virtual void WithLogger(ILoggerFactory loggerFactory)
=> throw new NotImplementedException();
}
}
3 changes: 3 additions & 0 deletions LyricsScraperNET/Providers/Abstract/IExternalProvider.cs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -27,5 +28,7 @@ public interface IExternalProvider
void Enable();

void Disable();

void WithLogger(ILoggerFactory loggerFactory);
}
}
Loading
Loading