Tried to write some unit tests, but unable due to dependency on DirectoryService.

Finally fixed the issue around memory mapped files preventing deleting. (Thought I did, but didn't) - Going to commit to try a full rewrite of the image work with NetVips instead.

API.Tests/AbstractFsTest.cs

@@ -1,6 +1,7 @@
 
 
 using System.IO;
+using System.IO.Abstractions;
 using System.IO.Abstractions.TestingHelpers;
 using API.Services.Tasks.Scanner.Parser;
 

API.Tests/Services/CoverDbServiceTests.cs

@@ -1,19 +1,36 @@
-using System.Threading.Tasks;
+using System.IO;
+using System.IO.Abstractions;
+using System.Reflection;
+using System.Threading.Tasks;
+using API.Constants;
+using API.Entities.Enums;
+using API.Extensions;
 using API.Services;
 using API.Services.Tasks.Metadata;
 using API.SignalR;
 using EasyCaching.Core;
+using Kavita.Common;
 using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.Logging;
 using NSubstitute;
+using Xunit;
 
 namespace API.Tests.Services;
 
 public class CoverDbServiceTests : AbstractDbTest
 {
-    private readonly IDirectoryService _directoryService;
+    private readonly DirectoryService _directoryService;
     private readonly IEasyCachingProviderFactory _cacheFactory = Substitute.For<IEasyCachingProviderFactory>();
     private readonly ICoverDbService _coverDbService;
+
+    private static readonly string FaviconPath = Path.Join(Directory.GetCurrentDirectory(),
+        "../../../Services/Test Data/CoverDbService/Favicons");
+    /// <summary>
+    /// Path to download files temp to. Should be empty after each test.
+    /// </summary>
+    private static readonly string TempPath = Path.Join(Directory.GetCurrentDirectory(),
+        "../../../Services/Test Data/CoverDbService/Temp");
+
     public CoverDbServiceTests()
     {
         _directoryService = new DirectoryService(Substitute.For<ILogger<DirectoryService>>(), CreateFileSystem());
@@ -27,4 +44,74 @@ public class CoverDbServiceTests : AbstractDbTest
     {
         throw new System.NotImplementedException();
     }
+
+
+    #region Download Favicon
+
+    /// <summary>
+    /// I cannot figure out how to test this code due to the reliance on the _directoryService.FaviconDirectory and not being
+    /// able to redirect it to the real filesystem.
+    /// </summary>
+    public async Task DownloadFaviconAsync_ShouldDownloadAndMatchExpectedFavicon()
+    {
+        // Arrange
+        var testUrl = "https://anilist.co/anime/6205/Kmpfer/";
+        var encodeFormat = EncodeFormat.WEBP;
+        var expectedFaviconPath = Path.Combine(FaviconPath, "anilist.co.webp");
+
+        // Ensure TempPath exists
+        _directoryService.ExistOrCreate(TempPath);
+
+        var baseUrl = "https://anilist.co";
+
+        // Ensure there is no cache result for this URL
+        var provider = Substitute.For<IEasyCachingProvider>();
+        provider.GetAsync<string>(baseUrl).Returns(new CacheValue<string>(null, false));
+        _cacheFactory.GetCachingProvider(EasyCacheProfiles.Favicon).Returns(provider);
+
+
+        // // Replace favicon directory with TempPath
+        // var directoryService = (DirectoryService)_directoryService;
+        // directoryService.FaviconDirectory = TempPath;
+
+        // Hack: Swap FaviconDirectory with TempPath for ability to download real files
+        typeof(DirectoryService)
+            .GetField("FaviconDirectory", BindingFlags.NonPublic | BindingFlags.Instance)
+            ?.SetValue(_directoryService, TempPath);
+
+
+        // Act
+        var resultFilename = await _coverDbService.DownloadFaviconAsync(testUrl, encodeFormat);
+        var actualFaviconPath = Path.Combine(TempPath, resultFilename);
+
+        // Assert file exists
+        Assert.True(File.Exists(actualFaviconPath), "Downloaded favicon does not exist in temp path");
+
+        // Load and compare similarity
+
+        var similarity = expectedFaviconPath.CalculateSimilarity(actualFaviconPath); // Assuming you have this extension
+        Assert.True(similarity > 0.9f, $"Image similarity too low: {similarity}");
+    }
+
+    [Fact]
+    public async Task DownloadFaviconAsync_ShouldThrowKavitaException_WhenPreviouslyFailedUrlExistsInCache()
+    {
+        // Arrange
+        var testUrl = "https://example.com";
+        var encodeFormat = EncodeFormat.WEBP;
+
+        var provider = Substitute.For<IEasyCachingProvider>();
+        provider.GetAsync<string>(Arg.Any<string>())
+            .Returns(new CacheValue<string>(string.Empty, true)); // Simulate previous failure
+
+        _cacheFactory.GetCachingProvider(EasyCacheProfiles.Favicon).Returns(provider);
+
+        // Act & Assert
+        await Assert.ThrowsAsync<KavitaException>(() =>
+            _coverDbService.DownloadFaviconAsync(testUrl, encodeFormat));
+    }
+
+    #endregion
+
+
 }

API/Extensions/ImageExtensions.cs

@@ -52,7 +52,7 @@ public static class ImageExtensions
         // Calculate mean squared error for pixel differences
         var mse = img1.GetMeanSquaredError(img2);
 
-        // Normalize MSE (65025 = 255² which is max possible squared difference per channel)
+        // Normalize MSE (65025 = 255², which is the max possible squared difference per channel)
         var normalizedMse = 1f - Math.Min(1f, mse / 65025f);
 
         // Final similarity score (weighted average of resolution difference and color difference)
@@ -121,8 +121,20 @@ public static class ImageExtensions
             return imagePath2;
 
         // Otherwise, we need to analyze the actual image data for both
-        var metrics1 = GetImageQualityMetrics(imagePath1);
+
-        var metrics2 = GetImageQualityMetrics(imagePath2);
+        // NOTE: We HAVE to use these scope blocks and load image here otherwise memory-mapped section exception will occur
+        ImageQualityMetrics metrics1;
+        using (var img1 = Image.Load<Rgba32>(imagePath1))
+        {
+            metrics1 = GetImageQualityMetrics(img1);
+        }
+
+        ImageQualityMetrics metrics2;
+        using (var img2 = Image.Load<Rgba32>(imagePath2))
+        {
+            metrics2 = GetImageQualityMetrics(img2);
+        }
+
 
         // If one is color, and one is grayscale, then we prefer color
         if (preferColor && metrics1.IsColor != metrics2.IsColor)
@@ -144,7 +156,7 @@ public static class ImageExtensions
     private static double CalculateOverallScore(ImageQualityMetrics metrics)
     {
         // Resolution factor (normalized to HD resolution)
-        var resolutionFactor = Math.Min(1.0, (metrics.Width * metrics.Height) / (double)(1920 * 1080));
+        var resolutionFactor = Math.Min(1.0, (metrics.Width * metrics.Height) / (double) (1920 * 1080));
 
         // Color factor
         var colorFactor = metrics.IsColor ? (0.5 + 0.5 * metrics.Colorfulness) : 0.3;
@@ -165,14 +177,11 @@ public static class ImageExtensions
     }
 
     /// <summary>
-    /// Gets quality metrics for an image using only ImageSharp
+    /// Gets quality metrics for an image
     /// </summary>
-    private static ImageQualityMetrics GetImageQualityMetrics(string imagePath)
+    private static ImageQualityMetrics GetImageQualityMetrics(Image<Rgba32> image)
     {
-        // Optimization: Use a smaller version for analysis
+        // Create a smaller version if the image is large to speed up analysis
-        using var image = Image.Load<Rgba32>(imagePath);
-
-        // Create a smaller version if image is large to speed up analysis
         Image<Rgba32> workingImage;
         if (image.Width > 512 || image.Height > 512)
         {
@@ -208,10 +217,7 @@ public static class ImageExtensions
         metrics.NoiseLevel = EstimateNoiseLevel(workingImage);
 
         // Clean up
-        if (workingImage != image)
+        workingImage.Dispose();
-        {
-            workingImage.Dispose();
-        }
 
         return metrics;
     }

API/Services/Tasks/Metadata/CoverDbService.cs

@@ -81,6 +81,22 @@ public class CoverDbService : ICoverDbService
         _eventHub = eventHub;
     }
 
+    /// <summary>
+    /// Downloads the favicon image from a given website URL, optionally falling back to a custom method if standard methods fail.
+    /// </summary>
+    /// <param name="url">The full URL of the website to extract the favicon from.</param>
+    /// <param name="encodeFormat">The desired image encoding format for saving the favicon (e.g., WebP, PNG).</param>
+    /// <returns>
+    /// A string representing the filename of the downloaded favicon image, saved to the configured favicon directory.
+    /// </returns>
+    /// <exception cref="KavitaException">
+    /// Thrown when favicon retrieval fails or if a previously failed domain is detected in cache.
+    /// </exception>
+    /// <remarks>
+    /// This method first checks for a cached failure to avoid re-requesting bad links.
+    /// It then attempts to parse HTML for `link` tags pointing to `.png` favicons and
+    /// falls back to an internal fallback method if needed. Valid results are saved to disk.
+    /// </remarks>
     public async Task<string> DownloadFaviconAsync(string url, EncodeFormat encodeFormat)
     {
         // Parse the URL to get the domain (including subdomain)
@@ -157,23 +173,10 @@ public class CoverDbService : ICoverDbService
             // Create the destination file path
             using var image = Image.PngloadStream(faviconStream);
             var filename = ImageService.GetWebLinkFormat(baseUrl, encodeFormat);
-            switch (encodeFormat)
-            {
-                case EncodeFormat.PNG:
-                    image.Pngsave(Path.Combine(_directoryService.FaviconDirectory, filename));
-                    break;
-                case EncodeFormat.WEBP:
-                    image.Webpsave(Path.Combine(_directoryService.FaviconDirectory, filename));
-                    break;
-                case EncodeFormat.AVIF:
-                    image.Heifsave(Path.Combine(_directoryService.FaviconDirectory, filename));
-                    break;
-                default:
-                    throw new ArgumentOutOfRangeException(nameof(encodeFormat), encodeFormat, null);
-            }
-
 
+            image.WriteToFile(Path.Combine(_directoryService.FaviconDirectory, filename));
             _logger.LogDebug("Favicon for {Domain} downloaded and saved successfully", domain);
+
             return filename;
         } catch (Exception ex)
         {
@@ -212,23 +215,10 @@ public class CoverDbService : ICoverDbService
             // Create the destination file path
             using var image = Image.NewFromStream(publisherStream);
             var filename = ImageService.GetPublisherFormat(publisherName, encodeFormat);
-            switch (encodeFormat)
-            {
-                case EncodeFormat.PNG:
-                    image.Pngsave(Path.Combine(_directoryService.PublisherDirectory, filename));
-                    break;
-                case EncodeFormat.WEBP:
-                    image.Webpsave(Path.Combine(_directoryService.PublisherDirectory, filename));
-                    break;
-                case EncodeFormat.AVIF:
-                    image.Heifsave(Path.Combine(_directoryService.PublisherDirectory, filename));
-                    break;
-                default:
-                    throw new ArgumentOutOfRangeException(nameof(encodeFormat), encodeFormat, null);
-            }
-
 
+            image.WriteToFile(Path.Combine(_directoryService.PublisherDirectory, filename));
             _logger.LogDebug("Publisher image for {PublisherName} downloaded and saved successfully", publisherName.Sanitize());
+
             return filename;
         } catch (Exception ex)
         {
@@ -305,40 +295,19 @@ public class CoverDbService : ICoverDbService
         var filename = filenameWithoutExtension + encodeFormat.GetExtension();
         var targetFile = Path.Combine(targetDirectory, filename);
 
-        // if (new FileInfo(targetFile).Exists)
-        // {
-        //     File.Delete(targetFile);
-        // }
-
         _logger.LogTrace("Fetching person image from {Url}", url.Sanitize());
         // Download the file using Flurl
         var imageStream = await url
             .AllowHttpStatus("2xx,304")
             .GetStreamAsync();
 
-
-
         using var image = Image.NewFromStream(imageStream);
         image.WriteToFile(targetFile);
-        // switch (encodeFormat)
-        // {
-        //     case EncodeFormat.PNG:
-        //         image.Pngsave(targetFile);
-        //         break;
-        //     case EncodeFormat.WEBP:
-        //         image.Webpsave(targetFile);
-        //         break;
-        //     case EncodeFormat.AVIF:
-        //         image.Heifsave(targetFile);
-        //         break;
-        //     default:
-        //         throw new ArgumentOutOfRangeException(nameof(encodeFormat), encodeFormat, null);
-        // }
 
         return filename;
     }
 
-    private async Task<string> GetCoverPersonImagePath(Person person)
+    private async Task<string?> GetCoverPersonImagePath(Person person)
     {
         var tempFile = Path.Join(_directoryService.LongTermCacheDirectory, "people.yml");
 
@@ -395,25 +364,22 @@ public class CoverDbService : ICoverDbService
         await CacheDataAsync(urlsFileName, allOverrides);
 
 
-        if (!string.IsNullOrEmpty(allOverrides))
+        if (string.IsNullOrEmpty(allOverrides)) return correctSizeLink;
+
+        var cleanedBaseUrl = baseUrl.Replace("https://", string.Empty);
+        var externalFile = allOverrides
+            .Split("\n")
+            .FirstOrDefault(url =>
+                cleanedBaseUrl.Equals(url.Replace(".png", string.Empty)) ||
+                cleanedBaseUrl.Replace("www.", string.Empty).Equals(url.Replace(".png", string.Empty)
+                ));
+
+        if (string.IsNullOrEmpty(externalFile))
         {
-            var cleanedBaseUrl = baseUrl.Replace("https://", string.Empty);
+            throw new KavitaException($"Could not grab favicon from {baseUrl.Sanitize()}");
-            var externalFile = allOverrides
-                .Split("\n")
-                .FirstOrDefault(url =>
-                    cleanedBaseUrl.Equals(url.Replace(".png", string.Empty)) ||
-                    cleanedBaseUrl.Replace("www.", string.Empty).Equals(url.Replace(".png", string.Empty)
-                    ));
-
-            if (string.IsNullOrEmpty(externalFile))
-            {
-                throw new KavitaException($"Could not grab favicon from {baseUrl.Sanitize()}");
-            }
-
-            correctSizeLink = $"{NewHost}favicons/" + externalFile;
         }
 
-        return correctSizeLink;
+        return $"{NewHost}favicons/{externalFile}";
     }
 
     private async Task<string> FallbackToKavitaReaderPublisher(string publisherName)
@@ -426,34 +392,30 @@ public class CoverDbService : ICoverDbService
         // Cache immediately
         await CacheDataAsync(publisherFileName, allOverrides);
 
+        if (string.IsNullOrEmpty(allOverrides)) return externalLink;
 
-        if (!string.IsNullOrEmpty(allOverrides))
+        var externalFile = allOverrides
-        {
+            .Split("\n")
-            var externalFile = allOverrides
+            .Select(publisherLine =>
-                .Split("\n")
-                .Select(publisherLine =>
-                {
-                    var tokens = publisherLine.Split("|");
-                    if (tokens.Length != 2) return null;
-                    var aliases = tokens[0];
-                    // Multiple publisher aliases are separated by #
-                    if (aliases.Split("#").Any(name => name.ToLowerInvariant().Trim().Equals(publisherName.ToLowerInvariant().Trim())))
-                    {
-                        return tokens[1];
-                    }
-                    return null;
-                })
-                .FirstOrDefault(url => !string.IsNullOrEmpty(url));
-
-            if (string.IsNullOrEmpty(externalFile))
             {
-                throw new KavitaException($"Could not grab publisher image for {publisherName}");
+                var tokens = publisherLine.Split("|");
-            }
+                if (tokens.Length != 2) return null;
+                var aliases = tokens[0];
+                // Multiple publisher aliases are separated by #
+                if (aliases.Split("#").Any(name => name.ToLowerInvariant().Trim().Equals(publisherName.ToLowerInvariant().Trim())))
+                {
+                    return tokens[1];
+                }
+                return null;
+            })
+            .FirstOrDefault(url => !string.IsNullOrEmpty(url));
 
-            externalLink = $"{NewHost}publishers/" + externalFile;
+        if (string.IsNullOrEmpty(externalFile))
+        {
+            throw new KavitaException($"Could not grab publisher image for {publisherName}");
         }
 
-        return externalLink;
+        return $"{NewHost}publishers/{externalLink}";
     }
 
     private async Task CacheDataAsync(string fileName, string? content)
@@ -681,7 +643,10 @@ public class CoverDbService : ICoverDbService
 
                         if (choseNewImage)
                         {
-                            _directoryService.DeleteFiles([existingPath]);
+
+                            File.Delete(existingPath);
+
+                            //_directoryService.DeleteFiles([existingPath]);
                             _directoryService.CopyFile(tempFullPath, finalFullPath);
                             _directoryService.DeleteFiles([tempFullPath]);
                         }