(#521) Improve FavIcon download facility

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  

src/Lanceur.Infra/Constants/Paths.cs

@@ -8,7 +8,6 @@ public static class Paths
     #region Fields
 
     private static readonly Conditional<string> 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
 

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<IFavIconManager> _logger;
+        private readonly ILogger _logger;
+        private readonly string _imageRepository;
 
         #endregion Fields
 
         #region Constructors
 
-        public FavIconManager(IPackagedAppSearchService searchService, IFavIconDownloader favIconDownloader, ILoggerFactory appLoggerFactory)
+        /// <param name="imageRepository">
+        /// This is used for unit tests. Keep default value unless you're testing
+        /// </param>
+        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<FavIconManager>();
+            _logger = loggerFactory.CreateLogger<FavIconDownloader>();
         }
 
         #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

src/Lanceur.Infra.Win32/Thumbnails/ImageRepositoryMixin.cs → 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;
         }
 

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<Uri> 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);

src/Lanceur.SharedKernel/Utils/Conditional.cs

@@ -23,7 +23,7 @@ public class Conditional<T>
     /// <summary>
     /// Create an instance of this object
     /// </summary>
-    /// <param name="onDebug">If <c>#if DEBUG</c> is set then return this value</param>
+    /// <param name="onDebug">If <c>DEBUG</c> constant is defined then return this value</param>
     /// <param name="onRelease">Otherwise return this value.</param>
     public Conditional(T onDebug, T onRelease)
     {

src/Lanceur.SharedKernel/Web/FavIconDownloader.cs

@@ -16,6 +16,7 @@ public FavIconDownloader(ILogger logger)
         }
         #region Methods
 
+        ///<inheritdoc />
         public async Task<bool> CheckExistsAsync(Uri url)
         {
             try
@@ -28,29 +29,36 @@ public async Task<bool> CheckExistsAsync(Uri url)
         }
 
         ///<inheritdoc />
-        public async Task<bool> SaveToFileAsync(Uri url, string path)
+        public async Task<bool> 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;
             }
         }

src/Lanceur.SharedKernel/Web/IFavIconDownloader.cs

@@ -20,9 +20,9 @@ public interface IFavIconDownloader
         /// favicon, nothing is saved
         /// </summary>
         /// <param name="url">The url of the website</param>
-        /// <param name="path">The path of the file to create.</param>
+        /// <param name="outputPath">The path of the file to create.</param>
         /// <returns><c>True</c> if favicon was found at the specified address; otherwise <c>False</c></returns>
-        Task<bool> SaveToFileAsync(Uri url, string path);
+        Task<bool> SaveToFileAsync(Uri url, string outputPath);
 
         #endregion Methods
     }

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>(LogEventLevel.Verbose, LogEventLevel.Information);
+        l.RegisterConstant(new LoggingLevelSwitch(conditional));
+
         l.RegisterLazySingleton<IMapper>(() => new Mapper(GetAutoMapperCfg()));
         l.RegisterLazySingleton<IUserNotification>(() => new UserNotification());
         l.RegisterLazySingleton(() => new RoutingState());
@@ -150,7 +154,9 @@ private static void RegisterServices()
         l.RegisterLazySingleton<IPackagedAppManager>(() => new PackagedAppManager());
         l.Register<IPackagedAppSearchService>(() => new PackagedAppSearchService(Get<ILoggerFactory>()));
         l.RegisterLazySingleton<IFavIconDownloader>(() => new FavIconDownloader(Get<ILoggerFactory>().GetLogger<IFavIconDownloader>()));
-        l.Register<IFavIconManager>(() => new FavIconManager(Get<IPackagedAppSearchService>(), Get<IFavIconDownloader>(), Get<ILoggerFactory>()));
+        l.Register<IFavIconManager>(() => new FavIconManager(Get<IPackagedAppSearchService>(),
+                                                             Get<IFavIconDownloader>(), 
+                                                             Get<ILoggerFactory>()));
 
         // Formatters
         l.Register<IStringFormatter>(() => new DefaultStringFormatter());

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<IPackagedAppSearchService>();
+        var favIconDownloader = Substitute.For<IFavIconDownloader>();
+        var logger = Substitute.For<ILoggerFactory>();
+        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);
+    }
+}