From 9d53fd015235060fd2b530cad1333d9cdc342127 Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Sat, 8 Apr 2023 14:18:05 -0500 Subject: [PATCH] fix: Do not assume only one field element in CF specifications For most CF specifications, there is only one element in the `fields` array, which has a `value` property inside of each of its objects. One particular specification, however, deviates from this assumption. The "SizeSpecification" has been observed with *two* field objects. Logic for parsing custom format CFs no longer assumes that the fields property may only have one element in it. Fixes #178 --- CHANGELOG.md | 4 + .../Api/CustomFormatServiceTest.cs | 27 + .../CustomFormat/Api/Data/issue_178.json | 9661 +++++++++++++++++ .../Guide/CustomFormatParserTest.cs | 21 +- .../Models/CustomFormatDataComparerTest.cs | 130 +- .../Models/FieldsArrayJsonConverterTest.cs | 101 + .../CustomFormat/Api/CustomFormatService.cs | 2 +- .../CustomFormat/Models/CustomFormatData.cs | 2 +- .../Models/CustomFormatDataComparer.cs | 23 +- .../Models/FieldsArrayJsonConverter.cs | 30 +- 10 files changed, 9928 insertions(+), 73 deletions(-) create mode 100644 src/Recyclarr.TrashLib.Tests/Pipelines/CustomFormat/Api/CustomFormatServiceTest.cs create mode 100644 src/Recyclarr.TrashLib.Tests/Pipelines/CustomFormat/Api/Data/issue_178.json create mode 100644 src/Recyclarr.TrashLib.Tests/Pipelines/CustomFormat/Models/FieldsArrayJsonConverterTest.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fae9027..10b9eedf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Fixed JSON parsing issue that sometimes occurs when pulling custom formats from Radarr (#178) + ## [4.4.0] - 2023-04-06 ### Added diff --git a/src/Recyclarr.TrashLib.Tests/Pipelines/CustomFormat/Api/CustomFormatServiceTest.cs b/src/Recyclarr.TrashLib.Tests/Pipelines/CustomFormat/Api/CustomFormatServiceTest.cs new file mode 100644 index 00000000..e98742b2 --- /dev/null +++ b/src/Recyclarr.TrashLib.Tests/Pipelines/CustomFormat/Api/CustomFormatServiceTest.cs @@ -0,0 +1,27 @@ +using Flurl.Http.Testing; +using Recyclarr.Cli.TestLibrary; +using Recyclarr.Common; +using Recyclarr.TrashLib.Config.Services; +using Recyclarr.TrashLib.Pipelines.CustomFormat.Api; + +namespace Recyclarr.TrashLib.Tests.Pipelines.CustomFormat.Api; + +[TestFixture] +[Parallelizable(ParallelScope.All)] +public class CustomFormatServiceTest : IntegrationFixture +{ + [Test, AutoMockData] + public async Task Get_can_parse_json(IServiceConfiguration config) + { + var resourceData = new ResourceDataReader(typeof(CustomFormatServiceTest), "Data"); + var jsonBody = resourceData.ReadData("issue_178.json"); + + using var http = new HttpTest(); + http.RespondWith(jsonBody); + + var sut = Resolve(); + var result = await sut.GetCustomFormats(config); + + result.Should().HaveCountGreaterThan(5); + } +} diff --git a/src/Recyclarr.TrashLib.Tests/Pipelines/CustomFormat/Api/Data/issue_178.json b/src/Recyclarr.TrashLib.Tests/Pipelines/CustomFormat/Api/Data/issue_178.json new file mode 100644 index 00000000..36b7aaf8 --- /dev/null +++ b/src/Recyclarr.TrashLib.Tests/Pipelines/CustomFormat/Api/Data/issue_178.json @@ -0,0 +1,9661 @@ +[ + { + "id": 1, + "name": "kickoff and buyin", + "includeCustomFormatWhenRenaming": false, + "specifications": [ + { + "name": "Preferred Words", + "implementation": "ReleaseTitleSpecification", + "implementationName": "Release Title", + "infoLink": "https://wiki.servarr.com/radarr/settings#custom-formats-2", + "negate": false, + "required": false, + "fields": [ + { + "order": 0, + "name": "value", + "label": "Regular Expression", + "helpText": "Custom Format RegEx is Case Insensitive", + "value": "(?i)\\b(KICKOFF|kick-off|Buyin|Buy-in|buy.in|buy\\sin)\\b", + "type": "textbox", + "advanced": false + } + ] + } + ] + }, + { + "id": 3, + "name": "BR-DISK", + "includeCustomFormatWhenRenaming": false, + "specifications": [ + { + "name": "BR-DISK", + "implementation": "ReleaseTitleSpecification", + "implementationName": "Release Title", + "infoLink": "https://wiki.servarr.com/radarr/settings#custom-formats-2", + "negate": false, + "required": true, + "fields": [ + { + "order": 0, + "name": "value", + "label": "Regular Expression", + "helpText": "Custom Format RegEx is Case Insensitive", + "value": "^(?!.*\\b((? o.Using(CustomFormatData.Comparer)); } [Test] @@ -113,9 +129,12 @@ public class CustomFormatDataComparerTest Implementation = "ReleaseTitleSpecification", Negate = false, Required = true, - Fields = new CustomFormatFieldData + Fields = new[] { - Value = "\\bEVO(TGX)?\\b" + new CustomFormatFieldData + { + Value = "\\bEVO(TGX)?\\b" + } } }, new CustomFormatSpecificationData @@ -124,9 +143,12 @@ public class CustomFormatDataComparerTest Implementation = "SourceSpecification", Negate = true, Required = true, - Fields = new CustomFormatFieldData + Fields = new[] { - Value = 7 + new CustomFormatFieldData + { + Value = 7 + } } }, new CustomFormatSpecificationData @@ -135,9 +157,12 @@ public class CustomFormatDataComparerTest Implementation = "SourceSpecification", Negate = true, Required = true, - Fields = new CustomFormatFieldData + Fields = new[] { - Value = 8 + new CustomFormatFieldData + { + Value = 8 + } } } } @@ -155,9 +180,12 @@ public class CustomFormatDataComparerTest Implementation = "ReleaseTitleSpecification", Negate = false, Required = true, - Fields = new CustomFormatFieldData + Fields = new[] { - Value = "\\bEVO(TGX)?\\b" + new CustomFormatFieldData + { + Value = "\\bEVO(TGX)?\\b" + } } }, new CustomFormatSpecificationData @@ -166,9 +194,12 @@ public class CustomFormatDataComparerTest Implementation = "SourceSpecification", Negate = true, Required = true, - Fields = new CustomFormatFieldData + Fields = new[] { - Value = 10 // this is different + new CustomFormatFieldData + { + Value = 10 // this is different + } } }, new CustomFormatSpecificationData @@ -177,9 +208,12 @@ public class CustomFormatDataComparerTest Implementation = "SourceSpecification", Negate = true, Required = true, - Fields = new CustomFormatFieldData + Fields = new[] { - Value = 8 + new CustomFormatFieldData + { + Value = 8 + } } } } @@ -292,9 +326,12 @@ public class CustomFormatDataComparerTest Implementation = "ReleaseTitleSpecification", Negate = false, Required = true, - Fields = new CustomFormatFieldData + Fields = new[] { - Value = "\\bEVO(TGX)?\\b" + new CustomFormatFieldData + { + Value = "\\bEVO(TGX)?\\b" + } } }, new CustomFormatSpecificationData @@ -303,9 +340,12 @@ public class CustomFormatDataComparerTest Implementation = "SourceSpecification", Negate = true, Required = true, - Fields = new CustomFormatFieldData + Fields = new[] { - Value = 7 + new CustomFormatFieldData + { + Value = 7 + } } }, new CustomFormatSpecificationData @@ -314,9 +354,12 @@ public class CustomFormatDataComparerTest Implementation = "SourceSpecification", Negate = true, Required = true, - Fields = new CustomFormatFieldData + Fields = new[] { - Value = 8 + new CustomFormatFieldData + { + Value = 8 + } } } } @@ -334,9 +377,12 @@ public class CustomFormatDataComparerTest Implementation = "ReleaseTitleSpecification", Negate = false, Required = true, - Fields = new CustomFormatFieldData + Fields = new[] { - Value = "\\bEVO(TGX)?\\b" + new CustomFormatFieldData + { + Value = "\\bEVO(TGX)?\\b" + } } }, new CustomFormatSpecificationData @@ -345,9 +391,12 @@ public class CustomFormatDataComparerTest Implementation = "SourceSpecification", Negate = true, Required = true, - Fields = new CustomFormatFieldData + Fields = new[] { - Value = 7 + new CustomFormatFieldData + { + Value = 7 + } } }, new CustomFormatSpecificationData @@ -356,9 +405,12 @@ public class CustomFormatDataComparerTest Implementation = "SourceSpecification", Negate = true, Required = true, - Fields = new CustomFormatFieldData + Fields = new[] { - Value = 8 + new CustomFormatFieldData + { + Value = 8 + } } } } diff --git a/src/Recyclarr.TrashLib.Tests/Pipelines/CustomFormat/Models/FieldsArrayJsonConverterTest.cs b/src/Recyclarr.TrashLib.Tests/Pipelines/CustomFormat/Models/FieldsArrayJsonConverterTest.cs new file mode 100644 index 00000000..327da970 --- /dev/null +++ b/src/Recyclarr.TrashLib.Tests/Pipelines/CustomFormat/Models/FieldsArrayJsonConverterTest.cs @@ -0,0 +1,101 @@ +using Flurl.Http.Configuration; +using Recyclarr.TrashLib.Json; +using Recyclarr.TrashLib.Pipelines.CustomFormat.Models; + +namespace Recyclarr.TrashLib.Tests.Pipelines.CustomFormat.Models; + +[TestFixture] +[Parallelizable(ParallelScope.All)] +public class FieldsArrayJsonConverterTest +{ + [Test] + public void Read_multiple_as_array() + { + var serializer = new NewtonsoftJsonSerializer(ServiceJsonSerializerFactory.Settings); + + const string json = @" +{ + 'fields': [ + { + 'order': 0, + 'name': 'min', + 'label': 'Minimum Size', + 'unit': 'GB', + 'helpText': 'Release must be greater than this size', + 'value': 25, + 'type': 'number', + 'advanced': false + }, + { + 'order': 1, + 'name': 'max', + 'label': 'Maximum Size', + 'unit': 'GB', + 'helpText': 'Release must be less than or equal to this size', + 'value': 40, + 'type': 'number', + 'advanced': false + } + ] +} +"; + var result = serializer.Deserialize(json); + + result.Fields.Should().BeEquivalentTo(new[] + { + new CustomFormatFieldData + { + Value = 25 + }, + new CustomFormatFieldData + { + Value = 40 + } + }); + } + + [Test] + public void Read_single_as_array() + { + var serializer = new NewtonsoftJsonSerializer(ServiceJsonSerializerFactory.Settings); + + const string json = @" +{ + 'fields': { + 'order': 0, + 'name': 'min', + 'label': 'Minimum Size', + 'unit': 'GB', + 'helpText': 'Release must be greater than this size', + 'value': 25, + 'type': 'number', + 'advanced': false + } +} +"; + var result = serializer.Deserialize(json); + + result.Fields.Should().BeEquivalentTo(new[] + { + new CustomFormatFieldData + { + Value = 25 + } + }); + } + + [Test] + public void Read_throws_on_unsupported_token_type() + { + var serializer = new NewtonsoftJsonSerializer(ServiceJsonSerializerFactory.Settings); + + const string json = @" +{ + 'fields': 0 +} +"; + var act = () => serializer.Deserialize(json); + + act.Should().Throw(); + } +} diff --git a/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Api/CustomFormatService.cs b/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Api/CustomFormatService.cs index e0238e83..65e9f7b6 100644 --- a/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Api/CustomFormatService.cs +++ b/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Api/CustomFormatService.cs @@ -5,7 +5,7 @@ using Recyclarr.TrashLib.Pipelines.CustomFormat.Models; namespace Recyclarr.TrashLib.Pipelines.CustomFormat.Api; -internal class CustomFormatService : ICustomFormatService +public class CustomFormatService : ICustomFormatService { private readonly IServiceRequestBuilder _service; diff --git a/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Models/CustomFormatData.cs b/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Models/CustomFormatData.cs index d376bfc4..1a012843 100644 --- a/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Models/CustomFormatData.cs +++ b/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Models/CustomFormatData.cs @@ -19,7 +19,7 @@ public record CustomFormatSpecificationData public bool Required { get; init; } [JsonConverter(typeof(FieldsArrayJsonConverter))] - public CustomFormatFieldData Fields { get; init; } = new(); + public IReadOnlyCollection Fields { get; init; } = Array.Empty(); } public record CustomFormatData diff --git a/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Models/CustomFormatDataComparer.cs b/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Models/CustomFormatDataComparer.cs index c8905517..ba0371b4 100644 --- a/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Models/CustomFormatDataComparer.cs +++ b/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Models/CustomFormatDataComparer.cs @@ -39,10 +39,29 @@ public sealed class CustomFormatDataEqualityComparer : IEqualityComparer first, + IReadOnlyCollection second) + { + if (first.Count != second.Count) + { + return false; + } + + return first + .FullJoin(second, x => x.Name, _ => false, _ => false, FieldEqual) + .All(x => x); + } + + private static bool FieldEqual(CustomFormatFieldData a, CustomFormatFieldData b) + { + return a.Value?.Equals(b.Value) ?? false; } public int GetHashCode(CustomFormatData obj) diff --git a/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Models/FieldsArrayJsonConverter.cs b/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Models/FieldsArrayJsonConverter.cs index f7a76617..d004739d 100644 --- a/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Models/FieldsArrayJsonConverter.cs +++ b/src/Recyclarr.TrashLib/Pipelines/CustomFormat/Models/FieldsArrayJsonConverter.cs @@ -7,22 +7,7 @@ public class FieldsArrayJsonConverter : JsonConverter { public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer) { - if (value is not CustomFormatFieldData) - { - serializer.Serialize(writer, value); - return; - } - - var token = JToken.FromObject(value); - if (token.Type == JTokenType.Object) - { - var array = new JArray(token); - serializer.Serialize(writer, array); - } - else - { - serializer.Serialize(writer, token); - } + serializer.Serialize(writer, value); } public override object? ReadJson( @@ -31,17 +16,14 @@ public class FieldsArrayJsonConverter : JsonConverter object? existingValue, JsonSerializer serializer) { - if (existingValue is not CustomFormatFieldData) - { - return new CustomFormatFieldData(); - } - var token = JToken.Load(reader); + + // ReSharper disable once SwitchExpressionHandlesSomeKnownEnumValuesWithExceptionInDefault return token.Type switch { - JTokenType.Object => token.ToObject(), - JTokenType.Array => token.ToObject()?.SingleOrDefault(), - _ => null + JTokenType.Object => new[] {token.ToObject()}, + JTokenType.Array => token.ToObject(), + _ => throw new InvalidOperationException("Unsupported token type for CustomFormatFieldData") }; }