From 95d0da9ab84e70ca757a48baf9e603961fecb8aa Mon Sep 17 00:00:00 2001 From: Robbie Davis Date: Tue, 13 Jan 2026 22:30:42 -0500 Subject: [PATCH] Fixing review comments Adjusted ListenarrFixture tests to use AudioAudiobook category and updated test logic for schema validation. Simplified ListenarrV1Proxy.GetIndexer to use a single API route and return null on failure, removing plural form fallback. --- .../Applications/Listenarr/Listenarr.tsx | 20 ---- .../Listenarr/ListenarrFixture.cs | 96 ++++++++++++++++--- .../Applications/Listenarr/Listenarr.cs | 2 +- .../Listenarr/ListenarrV1Proxy.cs | 52 ++++++---- 4 files changed, 116 insertions(+), 54 deletions(-) delete mode 100644 frontend/src/Settings/Applications/Listenarr/Listenarr.tsx diff --git a/frontend/src/Settings/Applications/Listenarr/Listenarr.tsx b/frontend/src/Settings/Applications/Listenarr/Listenarr.tsx deleted file mode 100644 index 62d3075c3..000000000 --- a/frontend/src/Settings/Applications/Listenarr/Listenarr.tsx +++ /dev/null @@ -1,20 +0,0 @@ -import React from 'react'; -import EditApplicationModalConnector from 'Settings/Applications/Applications/EditApplicationModalConnector'; -import Application from 'typings/Application'; - -interface ListenarrProps { - selectedApplication: Application; - onModalClose: () => void; -} - -function Listenarr({ selectedApplication, onModalClose }: ListenarrProps) { - return ( - - ); -} - -export default Listenarr; diff --git a/src/NzbDrone.Core.Test/Applications/Listenarr/ListenarrFixture.cs b/src/NzbDrone.Core.Test/Applications/Listenarr/ListenarrFixture.cs index dade03d40..5f5f0fccd 100644 --- a/src/NzbDrone.Core.Test/Applications/Listenarr/ListenarrFixture.cs +++ b/src/NzbDrone.Core.Test/Applications/Listenarr/ListenarrFixture.cs @@ -27,9 +27,9 @@ public void Setup() Settings = new ListenarrSettings { ProwlarrUrl = "http://localhost:9696", - BaseUrl = "http://localhost:5000", + BaseUrl = "http://localhost:4545", ApiKey = "abc", - SyncCategories = new List { NewznabStandardCategory.Movies.Id } + SyncCategories = new List { NewznabStandardCategory.AudioAudiobook.Id } } }; @@ -223,18 +223,14 @@ public void Test_should_fail_when_schema_missing() // Arrange Mocker.GetMock().Setup(c => c.GetIndexerSchema(It.IsAny())).Returns(new List()); - // Act & Assert - try - { - var result = Subject.Test(); - result.IsValid.Should().BeFalse(); - result.Errors.Should().ContainSingle(e => e.ErrorMessage.Contains("indexer schema")); - } - finally - { - // Consume expected warnings even if Subject.Test throws or an assert fails so teardown does not fail - ExceptionVerification.IgnoreWarns(); - } + // Act & Assert - call private BuildListenarrIndexer to assert it throws + var method = typeof(NzbDrone.Core.Applications.Listenarr.Listenarr).GetMethod("BuildListenarrIndexer", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + var indexerDef = new IndexerDefinition { Id = 1, Name = "Test", Protocol = DownloadProtocol.Usenet, Capabilities = new IndexerCapabilities() }; + + var ex = Assert.Throws(() => method.Invoke(Subject, new object[] { indexerDef, indexerDef.Capabilities, DownloadProtocol.Usenet, 0 })); + Assert.IsInstanceOf(ex.InnerException); + Assert.That(ex.InnerException.Message, Does.Contain("indexer schemas")); + ExceptionVerification.ExpectedWarns(1); } [Test] @@ -290,7 +286,7 @@ public void AddIndexer_should_insert_app_indexer_mapping_on_success() }; // Add a category that matches Settings.SyncCategories - indexerDefinition.Capabilities.Categories.AddCategoryMapping(1, NewznabStandardCategory.Movies); + indexerDefinition.Capabilities.Categories.AddCategoryMapping(1, NewznabStandardCategory.AudioAudiobook); var mockIndexer = new Mock(); mockIndexer.Setup(i => i.GetCapabilities()).Returns(indexerDefinition.Capabilities); @@ -331,5 +327,75 @@ public void AddIndexer_should_insert_app_indexer_mapping_on_success() Mocker.GetMock().Verify(m => m.AddIndexer(It.IsAny(), It.IsAny()), Times.Once()); Mocker.GetMock().Verify(m => m.Insert(It.Is(a => a.IndexerId == 12 && a.RemoteIndexerId == 501)), Times.Once()); } + + [Test] + public void AddIndexer_should_use_existing_remote_indexer_if_baseUrl_matches() + { + // Arrange + var indexerDefinition = new IndexerDefinition + { + Id = 12, + Name = "TestIndexer", + Protocol = DownloadProtocol.Usenet, + Capabilities = new IndexerCapabilities(), + Enable = true, + AppProfile = new LazyLoaded(new AppSyncProfile { EnableRss = true, EnableAutomaticSearch = true, EnableInteractiveSearch = true }) + }; + + indexerDefinition.Capabilities.Categories.AddCategoryMapping(1, NewznabStandardCategory.AudioAudiobook); + + var mockIndexer = new Mock(); + mockIndexer.Setup(i => i.GetCapabilities()).Returns(indexerDefinition.Capabilities); + + Mocker.GetMock().Setup(m => m.GetInstance(It.IsAny())).Returns(mockIndexer.Object); + + var schema = new List + { + new ListenarrIndexer + { + Implementation = "Newznab", + Fields = new List + { + new ListenarrField { Name = "baseUrl", Value = "" }, + new ListenarrField { Name = "apiPath", Value = "" }, + new ListenarrField { Name = "apiKey", Value = "" }, + new ListenarrField { Name = "categories", Value = new List() } + } + } + }; + + // Existing remote indexer with matching baseUrl + var existing = new ListenarrIndexer + { + Id = 501, + Implementation = "Newznab", + Fields = new List + { + new ListenarrField { Name = "baseUrl", Value = $"{((ListenarrSettings)Subject.Definition.Settings).ProwlarrUrl.TrimEnd('/')}/12/" }, + new ListenarrField { Name = "apiPath", Value = "/api" }, + new ListenarrField { Name = "apiKey", Value = "abc" }, + new ListenarrField { Name = "categories", Value = new List() } + } + }; + + Mocker.GetMock().Setup(c => c.GetIndexerSchema(It.IsAny())).Returns(schema); + Mocker.GetMock().Setup(c => c.GetIndexers(It.IsAny())).Returns(new List { existing }); + + // Ensure the private schema cache will execute the factory so it invokes our mocked proxy + var cachedForTest = new Mock>>(); + cachedForTest.Setup(c => c.Get(It.IsAny(), It.IsAny>>(), It.IsAny())) + .Returns>, TimeSpan>((k, f, t) => f()); + typeof(NzbDrone.Core.Applications.Listenarr.Listenarr).GetField("_schemaCache", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic).SetValue(Subject, cachedForTest.Object); + + // pre-check + indexerDefinition.Capabilities.Categories.SupportedCategories(((ListenarrSettings)Subject.Definition.Settings).SyncCategories.ToArray()).Should().NotBeEmpty(); + + // Act + Subject.AddIndexer(indexerDefinition); + + // Assert - AddIndexer should not be called because remote already exists + Mocker.GetMock().Verify(m => m.AddIndexer(It.IsAny(), It.IsAny()), Times.Never()); + Mocker.GetMock().Verify(m => m.Insert(It.Is(a => a.IndexerId == 12 && a.RemoteIndexerId == 501)), Times.Once()); + } } } diff --git a/src/NzbDrone.Core/Applications/Listenarr/Listenarr.cs b/src/NzbDrone.Core/Applications/Listenarr/Listenarr.cs index 485e31175..a10417e61 100644 --- a/src/NzbDrone.Core/Applications/Listenarr/Listenarr.cs +++ b/src/NzbDrone.Core/Applications/Listenarr/Listenarr.cs @@ -391,7 +391,7 @@ private ListenarrIndexer BuildListenarrIndexer(IndexerDefinition indexer, Indexe if (id == 0) { // Ensuring backward compatibility with older versions on first sync - syncFields.AddRange(new List { "earlyReleaseLimit", "additionalParameters" }); + syncFields.AddRange(new List { "additionalParameters" }); } var newznab = schemas.FirstOrDefault(i => i.Implementation == "Newznab"); diff --git a/src/NzbDrone.Core/Applications/Listenarr/ListenarrV1Proxy.cs b/src/NzbDrone.Core/Applications/Listenarr/ListenarrV1Proxy.cs index c5499cb43..ca7f9da76 100644 --- a/src/NzbDrone.Core/Applications/Listenarr/ListenarrV1Proxy.cs +++ b/src/NzbDrone.Core/Applications/Listenarr/ListenarrV1Proxy.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Net; using System.Net.Http; using FluentValidation.Results; @@ -64,26 +65,12 @@ public ListenarrIndexer GetIndexer(int indexerId, ListenarrSettings settings) { try { - var request = BuildRequest(settings, $"{AppIndexerApiRoute}/{indexerId}", HttpMethod.Get); - return Execute(request); + var fallback = BuildRequest(settings, $"{AppApiRoute}/indexers/{indexerId}", HttpMethod.Get); + return Execute(fallback); } - catch (HttpException ex) + catch (HttpException) { - if (ex.Response.StatusCode != HttpStatusCode.NotFound) - { - throw; - } - - // Try plural form as a fallback - try - { - var fallback = BuildRequest(settings, $"{AppApiRoute}/indexers/{indexerId}", HttpMethod.Get); - return Execute(fallback); - } - catch (HttpException) - { - return null; - } + return null; } } @@ -293,6 +280,35 @@ public List GetIndexerSchema(ListenarrSettings settings) public ListenarrIndexer AddIndexer(ListenarrIndexer indexer, ListenarrSettings settings) { + // Defensive check: avoid creating duplicates if an indexer with the same baseUrl already exists on the remote app. + try + { + var incomingBaseUrl = indexer?.Fields?.FirstOrDefault(f => f.Name == "baseUrl")?.Value as string; + if (!string.IsNullOrWhiteSpace(incomingBaseUrl)) + { + var existing = GetIndexers(settings); + if (existing != null) + { + var match = existing.FirstOrDefault(e => + string.Equals( + (e.Fields?.FirstOrDefault(f => f.Name == "baseUrl")?.Value as string)?.TrimEnd('/'), + incomingBaseUrl.TrimEnd('/'), + StringComparison.InvariantCultureIgnoreCase)); + + if (match != null) + { + _logger.Debug("Found existing remote indexer matching baseUrl; skipping add and returning existing id {0}", match.Id); + return match; + } + } + } + } + catch (Exception ex) + { + // If the existence check fails for any reason, proceed with the add flow and let any resulting errors bubble up. + _logger.Debug(ex, "Failed to run pre-flight existence check before AddIndexer; proceeding to create"); + } + var request = BuildRequest(settings, $"{AppIndexerApiRoute}", HttpMethod.Post); request.SetContent(indexer.ToJson());