From 0a63921dab7ac066fde9789c681f0c39817d3266 Mon Sep 17 00:00:00 2001
From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com>
Date: Sun, 24 Mar 2024 15:04:40 -0400
Subject: [PATCH] style: fix logger analyzer warnings (#262)

---
 .editorconfig                                         |  9 +++++++++
 .../TestThemerrController.cs                          |  6 ++++--
 Jellyfin.Plugin.Themerr/Api/ThemerrController.cs      | 10 ++++++----
 .../ScheduledTasks/ThemerrTasks.cs                    | 10 +++++++---
 Jellyfin.Plugin.Themerr/ThemerrManager.cs             | 11 ++++-------
 docs/source/conf.py                                   |  1 +
 6 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/.editorconfig b/.editorconfig
index 569f539..29f4a12 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -116,6 +116,15 @@ dotnet_diagnostic.s2094.severity = None
 # Fix this implementation of 'IDisposable' to conform to the dispose pattern.
 dotnet_diagnostic.s3881.severity = None
 
+# Logging in a catch clause should pass the caught exception as a parameter.
+dotnet_diagnostic.s6667.severity = error
+
+# Logging arguments should be passed to the correct parameter.
+dotnet_diagnostic.s6668.severity = error
+
+# Update this logger to use its enclosing type.
+dotnet_diagnostic.s6672.severity = error
+
 
 ## StyleCop.Analyzers
 
diff --git a/Jellyfin.Plugin.Themerr.Tests/TestThemerrController.cs b/Jellyfin.Plugin.Themerr.Tests/TestThemerrController.cs
index bbdb05c..bb4c119 100644
--- a/Jellyfin.Plugin.Themerr.Tests/TestThemerrController.cs
+++ b/Jellyfin.Plugin.Themerr.Tests/TestThemerrController.cs
@@ -25,8 +25,9 @@ public TestThemerrController(ITestOutputHelper output)
         TestLogger.Initialize(output);
 
         Mock<ILibraryManager> mockLibraryManager = new();
-        Mock<ILogger<ThemerrManager>> mockLogger = new();
+        Mock<ILogger<ThemerrController>> mockLogger = new();
         Mock<IServerConfigurationManager> mockServerConfigurationManager = new();
+        Mock<ILoggerFactory> mockLoggerFactory = new();
 
         // Create a TestableServerConfiguration with UICulture set to "en-US"
         var testableServerConfiguration = new TestableServerConfiguration("en-US");
@@ -37,7 +38,8 @@ public TestThemerrController(ITestOutputHelper output)
         _controller = new ThemerrController(
             mockLibraryManager.Object,
             mockLogger.Object,
-            mockServerConfigurationManager.Object);
+            mockServerConfigurationManager.Object,
+            mockLoggerFactory.Object);
     }
 
     /// <summary>
diff --git a/Jellyfin.Plugin.Themerr/Api/ThemerrController.cs b/Jellyfin.Plugin.Themerr/Api/ThemerrController.cs
index dee3554..858c2f3 100644
--- a/Jellyfin.Plugin.Themerr/Api/ThemerrController.cs
+++ b/Jellyfin.Plugin.Themerr/Api/ThemerrController.cs
@@ -27,7 +27,7 @@ namespace Jellyfin.Plugin.Themerr.Api
     public class ThemerrController : ControllerBase
     {
         private readonly ThemerrManager _themerrManager;
-        private readonly ILogger<ThemerrManager> _logger;
+        private readonly ILogger<ThemerrController> _logger;
         private readonly IServerConfigurationManager _configurationManager;
 
         /// <summary>
@@ -36,12 +36,14 @@ public class ThemerrController : ControllerBase
         /// <param name="libraryManager">The library manager.</param>
         /// <param name="logger">The logger.</param>
         /// <param name="configurationManager">The configuration manager.</param>
+        /// <param name="loggerFactory">The logger factory.</param>
         public ThemerrController(
             ILibraryManager libraryManager,
-            ILogger<ThemerrManager> logger,
-            IServerConfigurationManager configurationManager)
+            ILogger<ThemerrController> logger,
+            IServerConfigurationManager configurationManager,
+            ILoggerFactory loggerFactory)
         {
-            _themerrManager = new ThemerrManager(libraryManager,  logger);
+            _themerrManager = new ThemerrManager(libraryManager,  loggerFactory.CreateLogger<ThemerrManager>());
             _logger = logger;
             _configurationManager = configurationManager;
         }
diff --git a/Jellyfin.Plugin.Themerr/ScheduledTasks/ThemerrTasks.cs b/Jellyfin.Plugin.Themerr/ScheduledTasks/ThemerrTasks.cs
index 2f267e7..7e7dd25 100644
--- a/Jellyfin.Plugin.Themerr/ScheduledTasks/ThemerrTasks.cs
+++ b/Jellyfin.Plugin.Themerr/ScheduledTasks/ThemerrTasks.cs
@@ -13,7 +13,7 @@ namespace Jellyfin.Plugin.Themerr.ScheduledTasks
     /// </summary>
     public class ThemerrTasks : IScheduledTask
     {
-        private readonly ILogger<ThemerrManager> _logger;
+        private readonly ILogger<ThemerrTasks> _logger;
         private readonly ThemerrManager _themerrManager;
 
         /// <summary>
@@ -21,10 +21,14 @@ public class ThemerrTasks : IScheduledTask
         /// </summary>
         /// <param name="libraryManager">The library manager.</param>
         /// <param name="logger">The logger.</param>
-        public ThemerrTasks(ILibraryManager libraryManager, ILogger<ThemerrManager> logger)
+        /// <param name="loggerFactory">The logger factory.</param>
+        public ThemerrTasks(
+            ILibraryManager libraryManager,
+            ILogger<ThemerrTasks> logger,
+            ILoggerFactory loggerFactory)
         {
             _logger = logger;
-            _themerrManager = new ThemerrManager(libraryManager,  logger);
+            _themerrManager = new ThemerrManager(libraryManager,  loggerFactory.CreateLogger<ThemerrManager>());
         }
 
         /// <summary>
diff --git a/Jellyfin.Plugin.Themerr/ThemerrManager.cs b/Jellyfin.Plugin.Themerr/ThemerrManager.cs
index 3d8d6bc..495050d 100644
--- a/Jellyfin.Plugin.Themerr/ThemerrManager.cs
+++ b/Jellyfin.Plugin.Themerr/ThemerrManager.cs
@@ -84,7 +84,7 @@ public bool SaveMp3(string destination, string videoUrl)
             }
             catch (Exception e)
             {
-                _logger.LogError("Unable to download {VideoUrl} to {Destination}: {Error}", videoUrl, destination, e);
+                _logger.LogError(e, "Unable to download {VideoUrl} to {Destination}", videoUrl, destination);
                 return false;
             }
 
@@ -331,12 +331,9 @@ public string GetYoutubeThemeUrl(string themerrDbUrl, BaseItem item)
                 dynamic jsonData = JsonConvert.DeserializeObject(jsonString);
                 return jsonData?.youtube_theme_url;
             }
-            catch (Exception)
+            catch (Exception e)
             {
-                _logger.LogWarning(
-                    "Missing from ThemerrDB: {ItemTitle}, contribute:\n  {IssueUrl}",
-                    item.Name,
-                    GetIssueUrl(item));
+                _logger.LogWarning(e, "Missing from ThemerrDB: {ItemTitle}, contribute:\n  {IssueUrl}", item.Name, GetIssueUrl(item));
                 return string.Empty;
             }
         }
@@ -405,7 +402,7 @@ public bool SaveThemerrData(string themePath, string themerrDataPath, string you
             }
             catch (Exception e)
             {
-                _logger.LogError("Unable to save themerr data to {ThemerrDataPath}: {Error}", themerrDataPath, e);
+                _logger.LogError(e, "Unable to save themerr data to {ThemerrDataPath}", themerrDataPath);
             }
 
             return success && WaitForFile(themerrDataPath, 10000);
diff --git a/docs/source/conf.py b/docs/source/conf.py
index 9675ab9..616be17 100644
--- a/docs/source/conf.py
+++ b/docs/source/conf.py
@@ -164,6 +164,7 @@
         ],
         'Extensions.Logging': [
             'ILogger',
+            'ILoggerFactory',
         ],
     },
     'Jellyfin.Controller.MediaBrowser.Common.Configuration': {