From 34121e27809e893db6d9929a439e7372221377ad Mon Sep 17 00:00:00 2001 From: jibedoubleve Date: Tue, 30 Jul 2024 15:05:11 +0200 Subject: [PATCH] (#521) Improve FavIcon download facility --- GitVersion.yml | 2 +- src/Lanceur.Infra/Constants/Paths.cs | 1 - src/Lanceur.Infra/Managers/FavIconManager.cs | 36 ++++++++---- .../Mixins/FavIconHelpers.cs} | 9 ++- src/Lanceur.SharedKernel/Mixins/UriMixin.cs | 7 ++- src/Lanceur.SharedKernel/Utils/Conditional.cs | 2 +- .../Web/FavIconDownloader.cs | 32 +++++++---- .../Web/IFavIconDownloader.cs | 4 +- src/Lanceur/Bootstrapper.cs | 10 +++- .../Functional/FavIconManagerShould.cs | 55 +++++++++++++++++++ 10 files changed, 120 insertions(+), 38 deletions(-) rename src/{Lanceur.Infra.Win32/Thumbnails/ImageRepositoryMixin.cs => Lanceur.SharedKernel/Mixins/FavIconHelpers.cs} (79%) create mode 100644 src/Tests/Lanceur.Tests/Functional/FavIconManagerShould.cs diff --git a/GitVersion.yml b/GitVersion.yml index 5b426e69..b158ac65 100644 --- a/GitVersion.yml +++ b/GitVersion.yml @@ -1,5 +1,5 @@ assembly-versioning-scheme: MajorMinorPatchTag -next-version: 2.7.0 +next-version: 2.7.1 branches: master: is-release-branch: true diff --git a/src/Lanceur.Infra/Constants/Paths.cs b/src/Lanceur.Infra/Constants/Paths.cs index 4bcbced1..71bc00f1 100644 --- a/src/Lanceur.Infra/Constants/Paths.cs +++ b/src/Lanceur.Infra/Constants/Paths.cs @@ -8,7 +8,6 @@ public static class Paths #region Fields private static readonly Conditional LogUrlValue = new("http://localhost:5341", "http://ec2-15-237-113-93.eu-west-3.compute.amazonaws.com:5341"); - public const string FaviconPrefix = "favicon_"; #endregion Fields diff --git a/src/Lanceur.Infra/Managers/FavIconManager.cs b/src/Lanceur.Infra/Managers/FavIconManager.cs index 7e76a750..86c5f22e 100644 --- a/src/Lanceur.Infra/Managers/FavIconManager.cs +++ b/src/Lanceur.Infra/Managers/FavIconManager.cs @@ -5,6 +5,8 @@ using Microsoft.Extensions.Logging; using System.Text.RegularExpressions; using Lanceur.SharedKernel.Mixins; +using Splat; +using ILogger = Microsoft.Extensions.Logging.ILogger; namespace Lanceur.Infra.Managers { @@ -19,38 +21,50 @@ public class FavIconManager : IFavIconManager private static readonly Regex IsMacroRegex = new("@.*@"); private readonly IFavIconDownloader _favIconDownloader; - private readonly ILogger _logger; + private readonly ILogger _logger; + private readonly string _imageRepository; #endregion Fields #region Constructors - public FavIconManager(IPackagedAppSearchService searchService, IFavIconDownloader favIconDownloader, ILoggerFactory appLoggerFactory) + /// + /// This is used for unit tests. Keep default value unless you're testing + /// + public FavIconManager( + IPackagedAppSearchService searchService, + IFavIconDownloader favIconDownloader, + ILoggerFactory loggerFactory, + string imageRepository = null) { + _imageRepository = imageRepository ?? Paths.ImageRepository; ArgumentNullException.ThrowIfNull(searchService); ArgumentNullException.ThrowIfNull(favIconDownloader); - ArgumentNullException.ThrowIfNull(appLoggerFactory); + ArgumentNullException.ThrowIfNull(loggerFactory); _favIconDownloader = favIconDownloader; - _logger = appLoggerFactory.CreateLogger(); + _logger = loggerFactory.CreateLogger(); } #endregion Constructors #region Methods - public async Task RetrieveFaviconAsync(string fileName) + public async Task RetrieveFaviconAsync(string url) { - if (fileName is null) return; - if (IsMacroRegex.Match(fileName).Success) return; - if (!Uri.TryCreate(fileName, UriKind.Absolute, out var uri)) return; + if (url is null) return; + if (IsMacroRegex.Match(url).Success) return; + if (!Uri.TryCreate(url, UriKind.Absolute, out var uri)) return; - var output = Path.Combine(Paths.ImageRepository, $"{Paths.FaviconPrefix}{uri.Host}.png"); + var output = Path.Combine(_imageRepository, $"{FavIconHelpers.FilePrefix}{uri.Host}.png"); if (File.Exists(output)) return; + + var uriAuthority = uri.GetAuthority(); + _logger.LogInformation("Getting favicon for {Uri}", uriAuthority); - if (!await _favIconDownloader.CheckExistsAsync(new($"{uri.Scheme}://{uri.Host}"))) return; + if (!await _favIconDownloader.CheckExistsAsync(uriAuthority)) return; - await _favIconDownloader.SaveToFileAsync(uri, output); + await _favIconDownloader.SaveToFileAsync(uriAuthority, output); } #endregion Methods diff --git a/src/Lanceur.Infra.Win32/Thumbnails/ImageRepositoryMixin.cs b/src/Lanceur.SharedKernel/Mixins/FavIconHelpers.cs similarity index 79% rename from src/Lanceur.Infra.Win32/Thumbnails/ImageRepositoryMixin.cs rename to src/Lanceur.SharedKernel/Mixins/FavIconHelpers.cs index c8acfa17..d20c8dd8 100644 --- a/src/Lanceur.Infra.Win32/Thumbnails/ImageRepositoryMixin.cs +++ b/src/Lanceur.SharedKernel/Mixins/FavIconHelpers.cs @@ -1,12 +1,11 @@ -using Lanceur.Infra.Constants; - -namespace Lanceur.Infra.Win32.Thumbnails +namespace Lanceur.SharedKernel.Mixins { - internal static class ImageRepositoryMixin + public static class FavIconHelpers { #region Fields private static readonly string[] SupportedSchemes = { "http", "https" }; + public const string FilePrefix = "favicon_"; #endregion Fields @@ -16,7 +15,7 @@ public static string GetKeyForFavIcon(this string address) { ArgumentNullException.ThrowIfNull(address); return Uri.TryCreate(address, new UriCreationOptions(), out _) - ? $"{Paths.FaviconPrefix}{new Uri(address).Host}" + ? $"{FilePrefix}{new Uri(address).Host}" : string.Empty; } diff --git a/src/Lanceur.SharedKernel/Mixins/UriMixin.cs b/src/Lanceur.SharedKernel/Mixins/UriMixin.cs index 71c8946c..b1f08213 100644 --- a/src/Lanceur.SharedKernel/Mixins/UriMixin.cs +++ b/src/Lanceur.SharedKernel/Mixins/UriMixin.cs @@ -4,14 +4,15 @@ public static class UriMixin { #region Methods - private static Uri GetAuthority(this Uri baseUri) => new(baseUri.GetLeftPart(UriPartial.Authority)); + public static Uri GetAuthority(this Uri baseUri) => new(baseUri.GetLeftPart(UriPartial.Authority)); private static Uri ToUri(this string path, UriKind kind) => new(path, kind); - public static Uri GetFavicon(this Uri baseUri) + public static IEnumerable GetFavicons(this Uri baseUri) { var uri = baseUri.GetAuthority(); - return new(uri, "favicon.ico"); + yield return new(uri, "favicon.ico"); + yield return new(uri, "favicon.png"); } public static Uri ToUriRelative(this string path) => path.ToUri(UriKind.Relative); diff --git a/src/Lanceur.SharedKernel/Utils/Conditional.cs b/src/Lanceur.SharedKernel/Utils/Conditional.cs index f918eaf9..c5ef8157 100644 --- a/src/Lanceur.SharedKernel/Utils/Conditional.cs +++ b/src/Lanceur.SharedKernel/Utils/Conditional.cs @@ -23,7 +23,7 @@ public class Conditional /// /// Create an instance of this object /// - /// If #if DEBUG is set then return this value + /// If DEBUG constant is defined then return this value /// Otherwise return this value. public Conditional(T onDebug, T onRelease) { diff --git a/src/Lanceur.SharedKernel/Web/FavIconDownloader.cs b/src/Lanceur.SharedKernel/Web/FavIconDownloader.cs index 2f5ce66a..a592d22e 100644 --- a/src/Lanceur.SharedKernel/Web/FavIconDownloader.cs +++ b/src/Lanceur.SharedKernel/Web/FavIconDownloader.cs @@ -16,6 +16,7 @@ public FavIconDownloader(ILogger logger) } #region Methods + /// public async Task CheckExistsAsync(Uri url) { try @@ -28,29 +29,36 @@ public async Task CheckExistsAsync(Uri url) } /// - public async Task SaveToFileAsync(Uri url, string path) + public async Task SaveToFileAsync(Uri url, string outputPath) { try { ArgumentNullException.ThrowIfNull(url); - ArgumentNullException.ThrowIfNull(path); + ArgumentNullException.ThrowIfNull(outputPath); - if (_failedPaths.Contains(path)) return false; - if (File.Exists(path)) return true; - - var uri = url.GetFavicon(); + if (_failedPaths.Contains(outputPath)) return false; + if (File.Exists(outputPath)) return true; + var uris = url.GetFavicons(); var httpClient = new HttpClient(); - var bytes = await httpClient.GetByteArrayAsync(uri); - if (bytes.Length == 0) return true; - await File.WriteAllBytesAsync(path, bytes); - return true; + foreach (var uri in uris) + { + _logger.LogTrace("Checking favicon url {Url}", uri); + if (! await CheckExistsAsync(uri)) continue; + + var bytes = await httpClient.GetByteArrayAsync(uri); + if (bytes.Length == 0) continue; + + await File.WriteAllBytesAsync(outputPath, bytes); + return true; + } + return false; } catch (Exception ex) { - _failedPaths.Add(path); - _logger.LogInformation(ex, "An error occured while saving FavIcon {Path}", path); + _failedPaths.Add(outputPath); + _logger.LogInformation(ex, "An error occured while saving FavIcon {Path}", outputPath); return false; } } diff --git a/src/Lanceur.SharedKernel/Web/IFavIconDownloader.cs b/src/Lanceur.SharedKernel/Web/IFavIconDownloader.cs index 0840f230..c3037d01 100644 --- a/src/Lanceur.SharedKernel/Web/IFavIconDownloader.cs +++ b/src/Lanceur.SharedKernel/Web/IFavIconDownloader.cs @@ -20,9 +20,9 @@ public interface IFavIconDownloader /// favicon, nothing is saved /// /// The url of the website - /// The path of the file to create. + /// The path of the file to create. /// True if favicon was found at the specified address; otherwise False - Task SaveToFileAsync(Uri url, string path); + Task SaveToFileAsync(Uri url, string outputPath); #endregion Methods } diff --git a/src/Lanceur/Bootstrapper.cs b/src/Lanceur/Bootstrapper.cs index 0667b85e..e35f46a6 100644 --- a/src/Lanceur/Bootstrapper.cs +++ b/src/Lanceur/Bootstrapper.cs @@ -46,7 +46,9 @@ using Lanceur.Infra.SQLite.DataAccess; using Lanceur.SharedKernel.Utils; using Serilog.Core; +using Serilog.Events; using Serilog.Filters; +using LogLevel = Microsoft.Extensions.Logging.LogLevel; namespace Lanceur; @@ -108,7 +110,9 @@ private static void RegisterServices() { var l = Locator.CurrentMutable; - l.RegisterConstant(new LoggingLevelSwitch()); + var conditional = new Conditional(LogEventLevel.Verbose, LogEventLevel.Information); + l.RegisterConstant(new LoggingLevelSwitch(conditional)); + l.RegisterLazySingleton(() => new Mapper(GetAutoMapperCfg())); l.RegisterLazySingleton(() => new UserNotification()); l.RegisterLazySingleton(() => new RoutingState()); @@ -150,7 +154,9 @@ private static void RegisterServices() l.RegisterLazySingleton(() => new PackagedAppManager()); l.Register(() => new PackagedAppSearchService(Get())); l.RegisterLazySingleton(() => new FavIconDownloader(Get().GetLogger())); - l.Register(() => new FavIconManager(Get(), Get(), Get())); + l.Register(() => new FavIconManager(Get(), + Get(), + Get())); // Formatters l.Register(() => new DefaultStringFormatter()); diff --git a/src/Tests/Lanceur.Tests/Functional/FavIconManagerShould.cs b/src/Tests/Lanceur.Tests/Functional/FavIconManagerShould.cs new file mode 100644 index 00000000..50012b99 --- /dev/null +++ b/src/Tests/Lanceur.Tests/Functional/FavIconManagerShould.cs @@ -0,0 +1,55 @@ +using FluentAssertions; +using Lanceur.Core.Services; +using Lanceur.Infra.Managers; +using Lanceur.SharedKernel.Mixins; +using Lanceur.SharedKernel.Web; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Xunit; + +namespace Lanceur.Tests.Functional; + +public class FavIconManagerShould +{ + [Theory] + [InlineData("http://www.google.com", "http://www.google.com")] + [InlineData("http://www.google.com:4001", "http://www.google.com:4001")] + [InlineData("http://www.google.com:80", "http://www.google.com:80")] + [InlineData("https://www.google.com", "https://www.google.com")] + [InlineData("https://www.google.com:4001", "https://www.google.com:4001")] + [InlineData("https://www.google.com:80", "https://www.google.com:80")] + [InlineData("https://www.google.com/some/index.html", "https://www.google.com")] + [InlineData("https://www.google.com:4001/some/index.html", "https://www.google.com:4001")] + [InlineData("https://www.google.com:80/some/index.html", "https://www.google.com:80")] + public async Task RetrieveExpectedUrl(string url, string asExpected) + { + // ARRANGE + var searchService = Substitute.For(); + var favIconDownloader = Substitute.For(); + var logger = Substitute.For(); + var repository = Path.GetTempPath(); + + // ACT + var manager = new FavIconManager(searchService, favIconDownloader, logger, repository); + await manager.RetrieveFaviconAsync(url); + + // ASSERT + await favIconDownloader.Received() + .CheckExistsAsync(new(asExpected)); + + } + + [Theory] + [InlineData("http://www.google.com", "http://www.google.com/favicon.ico")] + [InlineData("http://www.google.com:4001", "http://www.google.com:4001/favicon.ico")] + [InlineData("http://www.google.com:80", "http://www.google.com:80/favicon.ico")] + [InlineData("https://www.google.com", "https://www.google.com/favicon.ico")] + [InlineData("https://www.google.com:4001", "https://www.google.com:4001/favicon.ico")] + [InlineData("https://www.google.com:80", "https://www.google.com:80/favicon.ico")] + public void CreateFaviconUri(string url, string expected) + { + var thisUri = new Uri(expected); + new Uri(url).GetFavicons() + .Should().Contain(thisUri); + } +} \ No newline at end of file