diff --git a/CHANGELOG.md b/CHANGELOG.md index 8dbd1755..d6cb09f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Custom Formats: Smarter change detection logic for custom formats with language specifications, + which addresses the issue of some CFs constantly showing as updated during sync even if they + didn't change. + ## [7.2.3] - 2024-09-03 ### Changed diff --git a/src/Recyclarr.Cli/Pipelines/CustomFormat/PipelinePhases/CustomFormatTransactionPhase.cs b/src/Recyclarr.Cli/Pipelines/CustomFormat/PipelinePhases/CustomFormatTransactionPhase.cs index 70752909..97efe912 100644 --- a/src/Recyclarr.Cli/Pipelines/CustomFormat/PipelinePhases/CustomFormatTransactionPhase.cs +++ b/src/Recyclarr.Cli/Pipelines/CustomFormat/PipelinePhases/CustomFormatTransactionPhase.cs @@ -94,7 +94,7 @@ public class CustomFormatTransactionPhase(ILogger log, IServiceConfiguration con CustomFormatData serviceCf, CustomFormatTransactionData transactions) { - if (!CustomFormatData.Comparer.Equals(guideCf, serviceCf)) + if (guideCf != serviceCf) { transactions.UpdatedCustomFormats.Add(guideCf); } diff --git a/src/Recyclarr.Common/Extensions/HashCodeExtensions.cs b/src/Recyclarr.Common/Extensions/HashCodeExtensions.cs new file mode 100644 index 00000000..2e10bdf6 --- /dev/null +++ b/src/Recyclarr.Common/Extensions/HashCodeExtensions.cs @@ -0,0 +1,14 @@ +namespace Recyclarr.Common.Extensions; + +#pragma warning disable CS8851 +public static class HashCodeExtensions +{ + public static int CalcHashCode(this IEnumerable source) + { + return source.Aggregate(new HashCode(), (hash, item) => + { + hash.Add(item); + return hash; + }).ToHashCode(); + } +} diff --git a/src/Recyclarr.TrashGuide/CustomFormat/CustomFormatData.cs b/src/Recyclarr.TrashGuide/CustomFormat/CustomFormatData.cs index c75bc2d5..b61f7528 100644 --- a/src/Recyclarr.TrashGuide/CustomFormat/CustomFormatData.cs +++ b/src/Recyclarr.TrashGuide/CustomFormat/CustomFormatData.cs @@ -1,32 +1,15 @@ using System.Text.Json.Serialization; -using Recyclarr.Common.Extensions; using Recyclarr.Json; -namespace Recyclarr.TrashGuide.CustomFormat; - -public record CustomFormatFieldData -{ - public string Name { get; } = nameof(Value).ToCamelCase(); +// CA1065: Do not raise exceptions in unexpected locations +// Justification: Due to complex equivalency logic, hash codes are not possible. Additionally, these types are not +// intended to be used as keys in Dictionary, HashSet, etc. +#pragma warning disable CA1065 - [JsonConverter(typeof(NondeterministicValueConverter))] - public object? Value { get; init; } -} - -public record CustomFormatSpecificationData -{ - public string Name { get; init; } = ""; - public string Implementation { get; init; } = ""; - public bool Negate { get; init; } - public bool Required { get; init; } - - [JsonConverter(typeof(FieldsArrayJsonConverter))] - public IReadOnlyCollection Fields { get; init; } = Array.Empty(); -} +namespace Recyclarr.TrashGuide.CustomFormat; public record CustomFormatData { - public static CustomFormatDataEqualityComparer Comparer { get; } = new(); - [JsonIgnore] public string? Category { get; init; } @@ -46,4 +29,74 @@ public record CustomFormatData public bool IncludeCustomFormatWhenRenaming { get; init; } public IReadOnlyCollection Specifications { get; init; } = Array.Empty(); + + public virtual bool Equals(CustomFormatData? other) + { + if (other is null) + { + return false; + } + + if (ReferenceEquals(this, other)) + { + return true; + } + + var specsEqual = Specifications + .FullOuterHashJoin(other.Specifications, x => x.Name, x => x.Name, _ => false, _ => false, (x, y) => x == y) + .All(x => x); + + return + Id == other.Id && + Name == other.Name && + IncludeCustomFormatWhenRenaming == other.IncludeCustomFormatWhenRenaming && + specsEqual; + } + + public override int GetHashCode() => throw new NotImplementedException(); +} + +public record CustomFormatSpecificationData +{ + public string Name { get; init; } = ""; + public string Implementation { get; init; } = ""; + public bool Negate { get; init; } + public bool Required { get; init; } + + [JsonConverter(typeof(FieldsArrayJsonConverter))] + public IReadOnlyCollection Fields { get; init; } = Array.Empty(); + + public virtual bool Equals(CustomFormatSpecificationData? other) + { + if (other is null) + { + return false; + } + + if (ReferenceEquals(this, other)) + { + return true; + } + + var fieldsEqual = Fields + .InnerHashJoin(other.Fields, x => x.Name, x => x.Name, (x, y) => x == y) + .All(x => x); + + return + Name == other.Name && + Implementation == other.Implementation && + Negate == other.Negate && + Required == other.Required && + fieldsEqual; + } + + public override int GetHashCode() => throw new NotImplementedException(); +} + +public record CustomFormatFieldData +{ + public string Name { get; init; } = ""; + + [JsonConverter(typeof(NondeterministicValueConverter))] + public object? Value { get; init; } } diff --git a/src/Recyclarr.TrashGuide/CustomFormat/CustomFormatDataComparer.cs b/src/Recyclarr.TrashGuide/CustomFormat/CustomFormatDataComparer.cs deleted file mode 100644 index 89cf2065..00000000 --- a/src/Recyclarr.TrashGuide/CustomFormat/CustomFormatDataComparer.cs +++ /dev/null @@ -1,74 +0,0 @@ -namespace Recyclarr.TrashGuide.CustomFormat; - -public sealed class CustomFormatDataEqualityComparer : IEqualityComparer -{ - public bool Equals(CustomFormatData? x, CustomFormatData? y) - { - if (ReferenceEquals(x, y)) - { - return true; - } - - if (ReferenceEquals(x, null) || ReferenceEquals(y, null) || x.GetType() != y.GetType()) - { - return false; - } - - return x.Id.Equals(y.Id) && - x.Name.Equals(y.Name, StringComparison.Ordinal) && - x.IncludeCustomFormatWhenRenaming.Equals(y.IncludeCustomFormatWhenRenaming) && - AllSpecificationsEqual(x.Specifications, y.Specifications); - } - - private static bool AllSpecificationsEqual( - IReadOnlyCollection first, - IReadOnlyCollection second) - { - if (first.Count != second.Count) - { - return false; - } - - return first - .FullOuterHashJoin(second, x => x.Name, x => x.Name, _ => false, _ => false, SpecificationEqual) - .All(x => x); - } - - private static bool SpecificationEqual(CustomFormatSpecificationData a, CustomFormatSpecificationData b) - { - return a.Name.Equals(b.Name, StringComparison.Ordinal) && - a.Implementation.Equals(b.Implementation, StringComparison.Ordinal) && - a.Negate.Equals(b.Negate) && - a.Required.Equals(b.Required) && - AllFieldsEqual(a.Fields, b.Fields); - } - - private static bool AllFieldsEqual( - IReadOnlyCollection first, - IReadOnlyCollection second) - { - if (first.Count != second.Count) - { - return false; - } - - return first - .FullOuterHashJoin(second, x => x.Name, 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) - { - unchecked - { - var hashCode = obj.TrashId.GetHashCode(); - hashCode = (hashCode * 397) ^ obj.Id; - return hashCode; - } - } -} diff --git a/src/Recyclarr.TrashGuide/CustomFormat/FieldsArrayJsonConverter.cs b/src/Recyclarr.TrashGuide/CustomFormat/FieldsArrayJsonConverter.cs index 362d1dc9..5780913f 100644 --- a/src/Recyclarr.TrashGuide/CustomFormat/FieldsArrayJsonConverter.cs +++ b/src/Recyclarr.TrashGuide/CustomFormat/FieldsArrayJsonConverter.cs @@ -14,12 +14,27 @@ public class FieldsArrayJsonConverter : JsonConverter public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) { - if (reader.TokenType is JsonTokenType.StartObject) + return reader.TokenType switch { - return new[] {JsonSerializer.Deserialize(ref reader, options)!}; - } + JsonTokenType.StartObject => ConvertObjectToArray(ref reader, options), + JsonTokenType.StartArray => JsonSerializer.Deserialize(ref reader, options)!, + _ => throw new JsonException("Unexpected token type for CF fields") + }; + } - return JsonSerializer.Deserialize(ref reader, options)!; + private static CustomFormatFieldData[] ConvertObjectToArray( + ref Utf8JsonReader reader, + JsonSerializerOptions options) + { + var valueOptions = new JsonSerializerOptions(options); + valueOptions.Converters.Add(new NondeterministicValueConverter()); + return JsonSerializer.Deserialize>(ref reader, options)! + .Select(x => new CustomFormatFieldData + { + Name = x.Key, + Value = x.Value.Deserialize(valueOptions) + }) + .ToArray(); } public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options) diff --git a/tests/Recyclarr.Cli.Tests/Pipelines/CustomFormat/Models/CustomFormatDataComparerTest.cs b/tests/Recyclarr.Cli.Tests/Pipelines/CustomFormat/Models/CustomFormatDataComparerTest.cs index 5f83f2f6..bc320aef 100644 --- a/tests/Recyclarr.Cli.Tests/Pipelines/CustomFormat/Models/CustomFormatDataComparerTest.cs +++ b/tests/Recyclarr.Cli.Tests/Pipelines/CustomFormat/Models/CustomFormatDataComparerTest.cs @@ -1,226 +1,35 @@ +using System.Diagnostics.CodeAnalysis; using Recyclarr.TrashGuide.CustomFormat; namespace Recyclarr.Cli.Tests.Pipelines.CustomFormat.Models; -[TestFixture] +[Parallelizable(ParallelScope.All)] public class CustomFormatDataComparerTest { [Test] public void Custom_formats_equal() { - var a = new CustomFormatData - { - Name = "EVO (no WEBDL)", - IncludeCustomFormatWhenRenaming = false, - Specifications = - [ - new CustomFormatSpecificationData - { - Name = "EVO", - Implementation = "ReleaseTitleSpecification", - Negate = false, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = "\\bEVO(TGX)?\\b" - } - ] - }, - new CustomFormatSpecificationData - { - Name = "WEBDL", - Implementation = "SourceSpecification", - Negate = true, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = 7 - } - ] - }, - new CustomFormatSpecificationData - { - Name = "WEBRIP", - Implementation = "SourceSpecification", - Negate = true, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = 8 - } - ] - } - ] - }; + var a = CreateMockCustomFormatData(); - var b = new CustomFormatData - { - Name = "EVO (no WEBDL)", - IncludeCustomFormatWhenRenaming = false, - Specifications = - [ - new CustomFormatSpecificationData - { - Name = "EVO", - Implementation = "ReleaseTitleSpecification", - Negate = false, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = "\\bEVO(TGX)?\\b" - } - ] - }, - new CustomFormatSpecificationData - { - Name = "WEBDL", - Implementation = "SourceSpecification", - Negate = true, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = 7 - } - ] - }, - new CustomFormatSpecificationData - { - Name = "WEBRIP", - Implementation = "SourceSpecification", - Negate = true, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = 8 - } - ] - } - ] - }; + var b = CreateMockCustomFormatData(); - a.Should().BeEquivalentTo(b, o => o.Using(CustomFormatData.Comparer)); + a.Should().BeEquivalentTo(b, o => o.ComparingRecordsByValue()); } [Test] public void Custom_formats_not_equal_when_field_value_different() { - var a = new CustomFormatData - { - Name = "EVO (no WEBDL)", - IncludeCustomFormatWhenRenaming = false, - Specifications = - [ - new CustomFormatSpecificationData - { - Name = "EVO", - Implementation = "ReleaseTitleSpecification", - Negate = false, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = "\\bEVO(TGX)?\\b" - } - ] - }, - new CustomFormatSpecificationData - { - Name = "WEBDL", - Implementation = "SourceSpecification", - Negate = true, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = 7 - } - ] - }, - new CustomFormatSpecificationData - { - Name = "WEBRIP", - Implementation = "SourceSpecification", - Negate = true, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = 8 - } - ] - } - ] - }; + var a = CreateMockCustomFormatData(); - var b = new CustomFormatData + var b = CreateMockCustomFormatData() with { - Name = "EVO (no WEBDL)", - IncludeCustomFormatWhenRenaming = false, - Specifications = - [ - new CustomFormatSpecificationData - { - Name = "EVO", - Implementation = "ReleaseTitleSpecification", - Negate = false, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = "\\bEVO(TGX)?\\b" - } - ] - }, - new CustomFormatSpecificationData - { - Name = "WEBDL", - Implementation = "SourceSpecification", - Negate = true, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = 10 // this is different - } - ] - }, - new CustomFormatSpecificationData - { - Name = "WEBRIP", - Implementation = "SourceSpecification", - Negate = true, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = 8 - } - ] - } - ] + Specifications = a.Specifications.Select(spec => spec with + { + Name = spec.Name == "WEBRIP" ? "WEBRIP2" : spec.Name + }).ToList() }; - var result = CustomFormatData.Comparer.Equals(a, b); - - result.Should().BeFalse(); + a.Should().NotBeEquivalentTo(b, o => o.ComparingRecordsByValue()); } [Test] @@ -240,31 +49,25 @@ public class CustomFormatDataComparerTest Category = "two" }; - var result = CustomFormatData.Comparer.Equals(a, b); - - result.Should().BeTrue(); + a.Should().BeEquivalentTo(b, o => o.ComparingRecordsByValue()); } [Test] public void Not_equal_when_right_is_null() { var a = new CustomFormatData(); - var b = (CustomFormatData?) null; - - var result = CustomFormatData.Comparer.Equals(a, b); + CustomFormatData? b = null; - result.Should().BeFalse(); + a.Should().NotBeEquivalentTo(b, o => o.ComparingRecordsByValue()); } [Test] public void Not_equal_when_left_is_null() { - var a = (CustomFormatData?) null; + CustomFormatData? a = null; var b = new CustomFormatData(); - var result = CustomFormatData.Comparer.Equals(a, b); - - result.Should().BeFalse(); + a.Should().NotBeEquivalentTo(b, o => o.ComparingRecordsByValue()); } [Test] @@ -272,149 +75,167 @@ public class CustomFormatDataComparerTest { var a = new CustomFormatData(); - var result = CustomFormatData.Comparer.Equals(a, a); - - result.Should().BeTrue(); + a.Should().BeEquivalentTo(a, o => o.ComparingRecordsByValue()); } [Test] public void Not_equal_when_different_spec_count() { - var a = new CustomFormatData + var a = CreateMockCustomFormatData(); + + var b = a with { - Name = "EVO (no WEBDL)", - IncludeCustomFormatWhenRenaming = false, - Specifications = - [ - new CustomFormatSpecificationData(), - new CustomFormatSpecificationData() - ] + Specifications = a.Specifications.Concat([new CustomFormatSpecificationData()]).ToList() }; - var b = new CustomFormatData + a.Should().NotBeEquivalentTo(b, o => o.ComparingRecordsByValue()); + } + + [Test] + public void Not_equal_when_non_matching_spec_names() + { + var a = CreateMockCustomFormatData(); + + var b = a with { - Name = "EVO (no WEBDL)", - IncludeCustomFormatWhenRenaming = false, - Specifications = - [ - new CustomFormatSpecificationData(), - new CustomFormatSpecificationData(), - new CustomFormatSpecificationData() - ] + Specifications = a.Specifications.Select(spec => spec with + { + Name = spec.Name == "WEBRIP" ? "WEBRIP2" : spec.Name + }).ToList() }; - var result = CustomFormatData.Comparer.Equals(a, b); + a.Should().NotBeEquivalentTo(b, o => o.ComparingRecordsByValue()); + } - result.Should().BeFalse(); + [Test] + public void Not_equal_when_different_spec_names_and_values() + { + var a = CreateMockCustomFormatData(); + var b = a with + { + Specifications = a.Specifications.Select(spec => spec with + { + Name = spec.Name == "WEBRIP" ? "UNIQUE_NAME" : spec.Name, + Fields = spec.Fields.Select(field => field with + { + Value = field.Value is int ? 99 : "NEW_VALUE" + }).ToList() + }).ToList() + }; + + a.Should().NotBeEquivalentTo(b, o => o.ComparingRecordsByValue()); } [Test] - public void Not_equal_when_non_matching_spec_names() + public void Equal_when_different_field_counts_but_same_names_and_values() { - var a = new CustomFormatData + var a = CreateMockCustomFormatData(); + var b = a with { - Name = "EVO (no WEBDL)", - IncludeCustomFormatWhenRenaming = false, - Specifications = - [ - new CustomFormatSpecificationData - { - Name = "EVO", - Implementation = "ReleaseTitleSpecification", - Negate = false, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = "\\bEVO(TGX)?\\b" - } - ] - }, - new CustomFormatSpecificationData - { - Name = "WEBDL", - Implementation = "SourceSpecification", - Negate = true, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = 7 - } - ] - }, - new CustomFormatSpecificationData - { - Name = "WEBRIP", - Implementation = "SourceSpecification", - Negate = true, - Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = 8 - } - ] - } - ] + Specifications = a.Specifications.Select(spec => spec with + { + Fields = spec.Fields + .Concat([new CustomFormatFieldData {Name = "AdditionalField", Value = "ExtraValue"}]) + .ToList() + }).ToList() }; - var b = new CustomFormatData + a.Should().BeEquivalentTo(b, o => o.ComparingRecordsByValue()); + } + + [Test] + public void Equal_when_specifications_order_different() + { + var a = CreateMockCustomFormatData(); + + var b = a with + { + Specifications = a.Specifications.Reverse().ToList() + }; + + a.Should().BeEquivalentTo(b, o => o.ComparingRecordsByValue()); + } + + [Test] + public void Equal_when_fields_order_different_for_each_specification() + { + var a = CreateMockCustomFormatData(); + + var b = a with + { + Specifications = a.Specifications.Select(spec => spec with + { + Fields = spec.Fields.Reverse().ToList() + }).ToList() + }; + + a.Should().BeEquivalentTo(b, o => o.ComparingRecordsByValue()); + } + + [TestCase(typeof(CustomFormatData))] + [TestCase(typeof(CustomFormatSpecificationData))] + public void Throws_exception_when_used_as_key_in_dictionary(Type type) + { + var act = () => new Dictionary().Add(Activator.CreateInstance(type)!, null); + + act.Should().Throw(); + } + + [TestCase(typeof(CustomFormatData))] + [TestCase(typeof(CustomFormatSpecificationData))] + public void Throws_exception_when_used_as_key_in_hash_set(Type type) + { + var act = () => new HashSet().Add(Activator.CreateInstance(type)!); + + act.Should().Throw(); + } + + private static CustomFormatData CreateMockCustomFormatData() + { + return new CustomFormatData { Name = "EVO (no WEBDL)", IncludeCustomFormatWhenRenaming = false, - Specifications = - [ - new CustomFormatSpecificationData + Specifications = new List + { + new() { Name = "EVO", Implementation = "ReleaseTitleSpecification", Negate = false, Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = "\\bEVO(TGX)?\\b" - } - ] + Fields = new List + { + new() {Name = "value", Value = @"\bEVO(TGX)?\b"}, + new() {Name = "foo1", Value = "foo1"} + } }, - new CustomFormatSpecificationData + new() { Name = "WEBDL", Implementation = "SourceSpecification", Negate = true, Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = 7 - } - ] + Fields = new List + { + new() {Name = "value", Value = 7}, + new() {Name = "foo2", Value = "foo2"} + } }, - new CustomFormatSpecificationData + new() { - Name = "WEBRIP2", // This name is different - Implementation = "SourceSpecification", + Name = "WEBRIP", + Implementation = "LanguageSpecification", Negate = true, Required = true, - Fields = - [ - new CustomFormatFieldData - { - Value = 8 - } - ] + Fields = new List + { + new() {Name = "value", Value = 8}, + new() {Name = "exceptLanguage", Value = false}, + new() {Name = "foo3", Value = "foo3"} + } } - ] + } }; - - var result = CustomFormatData.Comparer.Equals(a, b); - - result.Should().BeFalse(); } } diff --git a/tests/Recyclarr.Cli.Tests/Pipelines/CustomFormat/Models/FieldsArrayJsonConverterTest.cs b/tests/Recyclarr.Cli.Tests/Pipelines/CustomFormat/Models/FieldsArrayJsonConverterTest.cs index 8e1ae8a3..25d98a71 100644 --- a/tests/Recyclarr.Cli.Tests/Pipelines/CustomFormat/Models/FieldsArrayJsonConverterTest.cs +++ b/tests/Recyclarr.Cli.Tests/Pipelines/CustomFormat/Models/FieldsArrayJsonConverterTest.cs @@ -8,7 +8,7 @@ namespace Recyclarr.Cli.Tests.Pipelines.CustomFormat.Models; public class FieldsArrayJsonConverterTest { [Test] - public void Read_multiple_as_array() + public void Read_array_as_is() { const string json = """ @@ -42,26 +42,20 @@ public class FieldsArrayJsonConverterTest JsonSerializer.Deserialize(json, GlobalJsonSerializerSettings.Services); result!.Fields.Should().BeEquivalentTo([ - new CustomFormatFieldData {Value = 25}, - new CustomFormatFieldData {Value = 40} + new CustomFormatFieldData {Name = "min", Value = 25}, + new CustomFormatFieldData {Name = "max", Value = 40} ]); } [Test] - public void Read_single_as_array() + public void Convert_key_value_pairs_to_array() { 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 + "value": 8, + "exceptLanguage": false } } """; @@ -69,7 +63,8 @@ public class FieldsArrayJsonConverterTest JsonSerializer.Deserialize(json, GlobalJsonSerializerSettings.Services); result!.Fields.Should().BeEquivalentTo([ - new CustomFormatFieldData {Value = "25"} + new CustomFormatFieldData {Name = "value", Value = 8}, + new CustomFormatFieldData {Name = "exceptLanguage", Value = false}, ]); }