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.
This commit is contained in:
Robbie Davis 2026-01-13 22:30:42 -05:00
parent 58ca5ee886
commit 95d0da9ab8
4 changed files with 116 additions and 54 deletions

View file

@ -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 (
<EditApplicationModalConnector
isOpen={true}
item={selectedApplication}
onModalClose={onModalClose}
/>
);
}
export default Listenarr;

View file

@ -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<int> { NewznabStandardCategory.Movies.Id }
SyncCategories = new List<int> { NewznabStandardCategory.AudioAudiobook.Id }
}
};
@ -223,18 +223,14 @@ public void Test_should_fail_when_schema_missing()
// Arrange
Mocker.GetMock<IListenarrV1Proxy>().Setup(c => c.GetIndexerSchema(It.IsAny<ListenarrSettings>())).Returns(new List<ListenarrIndexer>());
// 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<System.Reflection.TargetInvocationException>(() => method.Invoke(Subject, new object[] { indexerDef, indexerDef.Capabilities, DownloadProtocol.Usenet, 0 }));
Assert.IsInstanceOf<ApplicationException>(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<IIndexer>();
mockIndexer.Setup(i => i.GetCapabilities()).Returns(indexerDefinition.Capabilities);
@ -331,5 +327,75 @@ public void AddIndexer_should_insert_app_indexer_mapping_on_success()
Mocker.GetMock<IListenarrV1Proxy>().Verify(m => m.AddIndexer(It.IsAny<ListenarrIndexer>(), It.IsAny<ListenarrSettings>()), Times.Once());
Mocker.GetMock<IAppIndexerMapService>().Verify(m => m.Insert(It.Is<AppIndexerMap>(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<AppSyncProfile>(new AppSyncProfile { EnableRss = true, EnableAutomaticSearch = true, EnableInteractiveSearch = true })
};
indexerDefinition.Capabilities.Categories.AddCategoryMapping(1, NewznabStandardCategory.AudioAudiobook);
var mockIndexer = new Mock<IIndexer>();
mockIndexer.Setup(i => i.GetCapabilities()).Returns(indexerDefinition.Capabilities);
Mocker.GetMock<IIndexerFactory>().Setup(m => m.GetInstance(It.IsAny<IndexerDefinition>())).Returns(mockIndexer.Object);
var schema = new List<ListenarrIndexer>
{
new ListenarrIndexer
{
Implementation = "Newznab",
Fields = new List<ListenarrField>
{
new ListenarrField { Name = "baseUrl", Value = "" },
new ListenarrField { Name = "apiPath", Value = "" },
new ListenarrField { Name = "apiKey", Value = "" },
new ListenarrField { Name = "categories", Value = new List<int>() }
}
}
};
// Existing remote indexer with matching baseUrl
var existing = new ListenarrIndexer
{
Id = 501,
Implementation = "Newznab",
Fields = new List<ListenarrField>
{
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<int>() }
}
};
Mocker.GetMock<IListenarrV1Proxy>().Setup(c => c.GetIndexerSchema(It.IsAny<ListenarrSettings>())).Returns(schema);
Mocker.GetMock<IListenarrV1Proxy>().Setup(c => c.GetIndexers(It.IsAny<ListenarrSettings>())).Returns(new List<ListenarrIndexer> { existing });
// Ensure the private schema cache will execute the factory so it invokes our mocked proxy
var cachedForTest = new Mock<ICached<System.Collections.Generic.List<ListenarrIndexer>>>();
cachedForTest.Setup(c => c.Get(It.IsAny<string>(), It.IsAny<Func<System.Collections.Generic.List<ListenarrIndexer>>>(), It.IsAny<TimeSpan>()))
.Returns<string, Func<System.Collections.Generic.List<ListenarrIndexer>>, 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<IListenarrV1Proxy>().Verify(m => m.AddIndexer(It.IsAny<ListenarrIndexer>(), It.IsAny<ListenarrSettings>()), Times.Never());
Mocker.GetMock<IAppIndexerMapService>().Verify(m => m.Insert(It.Is<AppIndexerMap>(a => a.IndexerId == 12 && a.RemoteIndexerId == 501)), Times.Once());
}
}
}

View file

@ -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<string> { "earlyReleaseLimit", "additionalParameters" });
syncFields.AddRange(new List<string> { "additionalParameters" });
}
var newznab = schemas.FirstOrDefault(i => i.Implementation == "Newznab");

View file

@ -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<ListenarrIndexer>(request);
var fallback = BuildRequest(settings, $"{AppApiRoute}/indexers/{indexerId}", HttpMethod.Get);
return Execute<ListenarrIndexer>(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<ListenarrIndexer>(fallback);
}
catch (HttpException)
{
return null;
}
return null;
}
}
@ -293,6 +280,35 @@ public List<ListenarrIndexer> 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());