From 1d61d1057ea17f144f2ec9601cd7c77e748f93fc Mon Sep 17 00:00:00 2001
From: Joseph Milazzo <joseph.v.milazzo@gmail.com>
Date: Tue, 26 Jan 2021 10:45:44 -0600
Subject: [PATCH] Refactored and cleaned up GetCoverImage code.

---
 API.Tests/Services/ArchiveServiceTests.cs | 16 +++++-
 API/Services/ArchiveService.cs            | 62 ++++++++++-------------
 2 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/API.Tests/Services/ArchiveServiceTests.cs b/API.Tests/Services/ArchiveServiceTests.cs
index b3c4ac877..86f186b95 100644
--- a/API.Tests/Services/ArchiveServiceTests.cs
+++ b/API.Tests/Services/ArchiveServiceTests.cs
@@ -32,7 +32,7 @@ namespace API.Tests.Services
         }
 
         [Theory]
-        [InlineData("non existant file.zip", false)]
+        [InlineData("non existent file.zip", false)]
         [InlineData("wrong extension.rar", false)]
         [InlineData("empty.zip", false)]
         [InlineData("flat file.zip", true)]
@@ -45,6 +45,20 @@ namespace API.Tests.Services
             Assert.Equal(expected, _archiveService.IsValidArchive(Path.Join(testDirectory, archivePath)));
         }
         
+        [Theory]
+        [InlineData("non existent file.zip", 0)]
+        [InlineData("wrong extension.rar", 0)]
+        [InlineData("empty.zip", 0)]
+        [InlineData("flat file.zip", 1)]
+        [InlineData("file in folder in folder.zip", 1)]
+        [InlineData("file in folder.zip", 1)]
+        [InlineData("file in folder_alt.zip", 1)]
+        public void GetNumberOfPagesFromArchiveTest(string archivePath, int expected)
+        {
+            var testDirectory = Path.Join(Directory.GetCurrentDirectory(), "../../../Services/Test Data/ArchiveService/Archives");
+            Assert.Equal(expected, _archiveService.GetNumberOfPagesFromArchive(Path.Join(testDirectory, archivePath)));
+        }
+        
         [Theory]
         [InlineData("v10.cbz", "v10.expected.jpg")]
         [InlineData("v10 - with folder.cbz", "v10 - with folder.expected.jpg")]
diff --git a/API/Services/ArchiveService.cs b/API/Services/ArchiveService.cs
index b1615c705..41d20a8e7 100644
--- a/API/Services/ArchiveService.cs
+++ b/API/Services/ArchiveService.cs
@@ -11,11 +11,12 @@ using NetVips;
 namespace API.Services
 {
     /// <summary>
-    /// Responsible for manipulating Archive files. Used by <see cref="CacheService"/> almost exclusively.
+    /// Responsible for manipulating Archive files. Used by <see cref="CacheService"/> and <see cref="ScannerService"/>
     /// </summary>
     public class ArchiveService : IArchiveService
     {
         private readonly ILogger<ArchiveService> _logger;
+        private const int ThumbnailWidth = 320;
 
         public ArchiveService(ILogger<ArchiveService> logger)
         {
@@ -25,7 +26,6 @@ namespace API.Services
         public int GetNumberOfPagesFromArchive(string archivePath)
         {
             if (!IsValidArchive(archivePath)) return 0;
-
             _logger.LogDebug($"Getting Page numbers from  {archivePath}");
 
             try
@@ -53,53 +53,43 @@ namespace API.Services
             try
             {
                 if (!IsValidArchive(filepath)) return Array.Empty<byte>();
-                //if (string.IsNullOrEmpty(filepath) || !File.Exists(filepath) || !Parser.Parser.IsArchive(filepath)) return Array.Empty<byte>();
-
                 _logger.LogDebug($"Extracting Cover image from {filepath}");
+                
                 using ZipArchive archive = ZipFile.OpenRead(filepath);
                 if (!archive.HasFiles()) return Array.Empty<byte>();
 
                 var folder = archive.Entries.SingleOrDefault(x => Path.GetFileNameWithoutExtension(x.Name).ToLower() == "folder");
                 var entries = archive.Entries.Where(x => Path.HasExtension(x.FullName) && Parser.Parser.IsImage(x.FullName)).OrderBy(x => x.FullName).ToList();
-                ZipArchiveEntry entry;
-                
-                if (folder != null)
-                {
-                    entry = folder;
-                } else if (!entries.Any())
-                {
-                    return Array.Empty<byte>();
-                }
-                else
-                {
-                    entry = entries[0];
-                }
+                var entry = folder ?? entries[0];
 
-
-                if (createThumbnail)
-                {
-                    try
-                    {
-                        using var stream = entry.Open();
-                        var thumbnail = Image.ThumbnailStream(stream, 320);
-                        return thumbnail.WriteToBuffer(".jpg");
-                    }
-                    catch (Exception ex)
-                    {
-                        _logger.LogError(ex, "There was a critical error and prevented thumbnail generation.");
-                    }
-                }
-                
-                return ExtractEntryToImage(entry);
+                return createThumbnail ? CreateThumbnail(entry) : ConvertEntryToByteArray(entry);
             }
             catch (Exception ex)
             {
                 _logger.LogError(ex, "There was an exception when reading archive stream.");
-                return Array.Empty<byte>();
             }
+            
+            return Array.Empty<byte>();
         }
-        
-        private static byte[] ExtractEntryToImage(ZipArchiveEntry entry)
+
+        private byte[] CreateThumbnail(ZipArchiveEntry entry)
+        {
+            var coverImage = Array.Empty<byte>();
+            try
+            {
+                using var stream = entry.Open();
+                using var thumbnail = Image.ThumbnailStream(stream, ThumbnailWidth);
+                coverImage = thumbnail.WriteToBuffer(".jpg");
+            }
+            catch (Exception ex)
+            {
+                _logger.LogError(ex, "There was a critical error and prevented thumbnail generation. Defaulting to no cover image.");
+            }
+
+            return coverImage;
+        }
+
+        private static byte[] ConvertEntryToByteArray(ZipArchiveEntry entry)
         {
             using var stream = entry.Open();
             using var ms = new MemoryStream();