More cleanup and DI

src/NzbDrone.Core.Test/ImportListTests/Discogs/DiscogsListsFixture.cs

@@ -2,6 +2,7 @@
 using System.Net;
 using FluentAssertions;
 using Moq;
+using NLog;
 using NUnit.Framework;
 using NzbDrone.Common.Http;
 using NzbDrone.Core.ImportLists;
@@ -18,15 +19,15 @@ public class DiscogsListsFixture
     private DiscogsListsParser _parser;
     private Mock<IHttpClient> _httpClient;
     private DiscogsListsSettings _settings;
+    private Logger _logger;
 
     [SetUp]
     public void SetUp()
     {
         _httpClient = new Mock<IHttpClient>();
-        _parser = new DiscogsListsParser();
         _settings = new DiscogsListsSettings { Token = "token", ListId = "123", BaseUrl = "https://api.discogs.com" };
-
+        _logger = LogManager.GetCurrentClassLogger();
-        _parser.SetContext(_httpClient.Object, _settings);
+        _parser = new DiscogsListsParser(_settings, _httpClient.Object, _logger);
     }
 
     [Test]

src/NzbDrone.Core.Test/ImportListTests/Discogs/DiscogsWantlistFixture.cs

@@ -23,10 +23,8 @@ public class DiscogsWantlistFixture
     public void SetUp()
     {
         _httpClient = new Mock<IHttpClient>();
-        _parser = new DiscogsWantlistParser();
         _settings = new DiscogsWantlistSettings { Token = "token", Username = "user", BaseUrl = "https://api.discogs.com" };
-
+        _parser = new DiscogsWantlistParser(_settings, _httpClient.Object);
-        _parser.SetContext(_httpClient.Object, _settings);
     }
 
     [Test]
@@ -133,5 +131,4 @@ private ImportListResponse BuildWantlistResponse(string content, HttpStatusCode
 
         return new ImportListResponse(importListRequest, httpResponse);
     }
-
 }

src/NzbDrone.Core/ImportLists/Discogs/DiscogsListsParser.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Linq;
 using NLog;
 using NzbDrone.Common.Extensions;
 using NzbDrone.Common.Http;
@@ -11,68 +10,61 @@ namespace NzbDrone.Core.ImportLists.Discogs;
 
 public class DiscogsListsParser : IParseImportListResponse
 {
-    private IHttpClient _httpClient;
+    private readonly DiscogsListsSettings _settings;
-    private DiscogsListsSettings _settings;
+    private readonly IHttpClient _httpClient;
-    private Logger _logger;
+    private readonly Logger _logger;
 
-    public DiscogsListsParser()
+    public DiscogsListsParser(DiscogsListsSettings settings, IHttpClient httpClient, Logger logger)
     {
-    }
-
-    public void SetContext(IHttpClient httpClient, DiscogsListsSettings settings, Logger logger = null)
-    {
-        _httpClient = httpClient;
         _settings = settings;
-        _logger = logger ?? LogManager.GetCurrentClassLogger();
+        _httpClient = httpClient;
+        _logger = logger;
     }
 
     public IList<ImportListItemInfo> ParseResponse(ImportListResponse importListResponse)
     {
-        DiscogsParserHelper.EnsureValidResponse(importListResponse,
+        var items = new List<ImportListItemInfo>();
-            "Discogs API responded with HTML content. List may be too large or API may be unavailable.");
+
+        if (!PreProcess(importListResponse))
+        {
+            return items;
+        }
 
         var jsonResponse = Json.Deserialize<DiscogsListResponse>(importListResponse.Content);
 
         if (jsonResponse?.Items == null)
         {
-            return new List<ImportListItemInfo>();
+            return items;
         }
 
-        var items = new List<ImportListItemInfo>();
+        foreach (var item in jsonResponse.Items)
-
-        foreach (var resourceUrl in jsonResponse.Items.Where(IsReleaseItem).Select(item => item.ResourceUrl))
         {
-            var releaseInfo = TryFetchRelease(resourceUrl);
+            if (item.Type == "release" && item.ResourceUrl.IsNotNullOrWhiteSpace())
-
-            if (releaseInfo != null)
             {
-                items.Add(releaseInfo);
+                try
+                {
+                    var releaseInfo = FetchReleaseDetails(item.ResourceUrl);
+                    items.AddIfNotNull(releaseInfo);
+                }
+                catch (Exception ex)
+                {
+                    _logger.Error(ex, "Discogs release details API call resulted in an unexpected exception");
+                }
             }
         }
 
         return items;
     }
 
-    private static bool IsReleaseItem(DiscogsListItem item)
+    private bool PreProcess(ImportListResponse importListResponse)
     {
-        return item?.Type == "release" && item.ResourceUrl.IsNotNullOrWhiteSpace();
+        DiscogsParserHelper.EnsureValidResponse(importListResponse,
+            "Discogs API responded with HTML content. List may be too large or API may be unavailable.");
+        return true;
     }
 
-    private ImportListItemInfo TryFetchRelease(string resourceUrl)
+    private ImportListItemInfo FetchReleaseDetails(string resourceUrl)
-    {
-        if (_httpClient == null || _settings == null)
-        {
-            return null;
-        }
-
-        try
     {
         return DiscogsParserHelper.FetchReleaseDetails(_httpClient, _settings.Token, resourceUrl);
     }
-        catch (Exception ex)
-        {
-            _logger?.Error(ex, "Failed to fetch release details from Discogs API for resource URL: {0}. Skipping this item.", resourceUrl);
-            return null;
-        }
-    }
 }

src/NzbDrone.Core/ImportLists/Discogs/DiscogsWantlist.cs

@@ -30,9 +30,7 @@ public override IImportListRequestGenerator GetRequestGenerator()
 
         public override IParseImportListResponse GetParser()
         {
-            var parser = new DiscogsWantlistParser();
+            return new DiscogsWantlistParser(Settings, _httpClient);
-            parser.SetContext(_httpClient, Settings);
-            return parser;
         }
     }
 }

src/NzbDrone.Core/ImportLists/Discogs/DiscogsWantlistParser.cs

@@ -9,33 +9,31 @@ namespace NzbDrone.Core.ImportLists.Discogs;
 
 public class DiscogsWantlistParser : IParseImportListResponse
 {
-    private IHttpClient _httpClient;
+    private readonly DiscogsWantlistSettings _settings;
-    private DiscogsWantlistSettings _settings;
+    private readonly IHttpClient _httpClient;
 
-    public DiscogsWantlistParser()
+    public DiscogsWantlistParser(DiscogsWantlistSettings settings, IHttpClient httpClient)
     {
-    }
-
-    public void SetContext(IHttpClient httpClient, DiscogsWantlistSettings settings)
-    {
-        _httpClient = httpClient;
         _settings = settings;
+        _httpClient = httpClient;
     }
 
     public IList<ImportListItemInfo> ParseResponse(ImportListResponse importListResponse)
     {
-        DiscogsParserHelper.EnsureValidResponse(importListResponse,
+        var items = new List<ImportListItemInfo>();
-            "Discogs API responded with HTML content. Wantlist may be too large or API may be unavailable.");
+
+        if (!PreProcess(importListResponse))
+        {
+            return items;
+        }
 
         var jsonResponse = Json.Deserialize<DiscogsWantlistResponse>(importListResponse.Content);
 
         if (jsonResponse?.Wants == null)
         {
-            return new List<ImportListItemInfo>();
+            return items;
         }
 
-        var items = new List<ImportListItemInfo>();
-
         foreach (var want in jsonResponse.Wants)
         {
             var basicInfo = want?.BasicInformation;
@@ -52,7 +50,7 @@ public IList<ImportListItemInfo> ParseResponse(ImportListResponse importListResp
                 continue;
             }
 
-            items.Add(new ImportListItemInfo
+            items.AddIfNotNull(new ImportListItemInfo
             {
                 Artist = basicInfo.Artists.First().Name,
                 Album = basicInfo.Title
@@ -61,4 +59,11 @@ public IList<ImportListItemInfo> ParseResponse(ImportListResponse importListResp
 
         return items;
     }
+
+    private bool PreProcess(ImportListResponse importListResponse)
+    {
+        DiscogsParserHelper.EnsureValidResponse(importListResponse,
+            "Discogs API responded with HTML content. Wantlist may be too large or API may be unavailable.");
+        return true;
+    }
 }