From 71580acc406214ba41dd93d6dc485caa8ef68df6 Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Thu, 13 Oct 2022 11:39:13 -0500 Subject: [PATCH] refactor: Service cache test fixes & improved testability --- .../Command/Helpers/CacheStoragePath.cs | 26 ++- .../FluentAssertions/JsonEquivalencyStep.cs | 1 - src/TrashLib.Tests/Cache/ServiceCacheTest.cs | 173 +++++++++--------- .../JsonTransactionStepTest.cs | 117 ------------ .../Radarr/RadarrConfigurationTest.cs | 13 +- src/TrashLib/Cache/ICacheStoragePath.cs | 4 +- src/TrashLib/Cache/ServiceCache.cs | 38 ++-- .../Services/CustomFormat/CachePersister.cs | 2 +- .../PersistenceSteps/JsonTransactionStep.cs | 2 +- 9 files changed, 129 insertions(+), 247 deletions(-) diff --git a/src/Recyclarr/Command/Helpers/CacheStoragePath.cs b/src/Recyclarr/Command/Helpers/CacheStoragePath.cs index 54dba0e0..f665e862 100644 --- a/src/Recyclarr/Command/Helpers/CacheStoragePath.cs +++ b/src/Recyclarr/Command/Helpers/CacheStoragePath.cs @@ -1,5 +1,8 @@ +using System.Data.HashFunction.FNV; using System.IO.Abstractions; +using System.Text; using TrashLib.Cache; +using TrashLib.Config.Services; using TrashLib.Startup; namespace Recyclarr.Command.Helpers; @@ -8,13 +11,30 @@ public class CacheStoragePath : ICacheStoragePath { private readonly IAppPaths _paths; private readonly IServiceCommand _serviceCommand; + private readonly IServiceConfiguration _config; + private readonly IFNV1a _hash; - public CacheStoragePath(IAppPaths paths, IServiceCommand serviceCommand) + public CacheStoragePath( + IAppPaths paths, + IServiceCommand serviceCommand, + IServiceConfiguration config) { _paths = paths; _serviceCommand = serviceCommand; + _config = config; + _hash = FNV1aFactory.Instance.Create(FNVConfig.GetPredefinedConfig(32)); } - public string Path => _paths.CacheDirectory - .SubDirectory(_serviceCommand.Name.ToLower()).FullName; + private string BuildServiceGuid() + { + return _hash.ComputeHash(Encoding.ASCII.GetBytes(_config.BaseUrl)).AsHexString(); + } + + public IFileInfo CalculatePath(string cacheObjectName) + { + return _paths.CacheDirectory + .SubDirectory(_serviceCommand.Name.ToLower()) + .SubDirectory(BuildServiceGuid()) + .File(cacheObjectName + ".json"); + } } diff --git a/src/TestLibrary/FluentAssertions/JsonEquivalencyStep.cs b/src/TestLibrary/FluentAssertions/JsonEquivalencyStep.cs index 0b0adf12..022fc44a 100644 --- a/src/TestLibrary/FluentAssertions/JsonEquivalencyStep.cs +++ b/src/TestLibrary/FluentAssertions/JsonEquivalencyStep.cs @@ -1,4 +1,3 @@ -using FluentAssertions; using FluentAssertions.Equivalency; using FluentAssertions.Json; using Newtonsoft.Json.Linq; diff --git a/src/TrashLib.Tests/Cache/ServiceCacheTest.cs b/src/TrashLib.Tests/Cache/ServiceCacheTest.cs index 5cef7b84..a9adbbb4 100644 --- a/src/TrashLib.Tests/Cache/ServiceCacheTest.cs +++ b/src/TrashLib.Tests/Cache/ServiceCacheTest.cs @@ -1,16 +1,12 @@ -using System.IO.Abstractions; -using System.IO.Abstractions.Extensions; +using System.Collections.ObjectModel; using System.IO.Abstractions.TestingHelpers; using AutoFixture.NUnit3; using FluentAssertions; using NSubstitute; using NUnit.Framework; -using Serilog; using TestLibrary.AutoFixture; -using TestLibrary.NSubstitute; using TrashLib.Cache; -using TrashLib.Config.Services; -using TrashLib.Services.Radarr.Config; +using TrashLib.Services.CustomFormat.Models.Cache; namespace TrashLib.Tests.Cache; @@ -18,22 +14,6 @@ namespace TrashLib.Tests.Cache; [Parallelizable(ParallelScope.All)] public class ServiceCacheTest { - private class Context - { - public Context(IFileSystem? fs = null) - { - Filesystem = fs ?? Substitute.For(); - StoragePath = Substitute.For(); - - var config = new RadarrConfiguration {BaseUrl = "http://localhost:1234"}; - Cache = new ServiceCache(Filesystem, StoragePath, config, Substitute.For()); - } - - public ServiceCache Cache { get; } - public ICacheStoragePath StoragePath { get; } - public IFileSystem Filesystem { get; } - } - private class ObjectWithoutAttribute { } @@ -51,90 +31,79 @@ public class ServiceCacheTest { } - [Test] - public void Load_returns_null_when_file_does_not_exist() + [Test, AutoMockData] + public void Load_returns_null_when_file_does_not_exist( + [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, + ServiceCache sut) { - var ctx = new Context(); - ctx.Filesystem.File.Exists(Arg.Any()).Returns(false); - - var result = ctx.Cache.Load(); + var result = sut.Load(); result.Should().BeNull(); } [Test, AutoMockData] public void Loading_with_attribute_parses_correctly( [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, - [Frozen] IServiceConfiguration config, [Frozen] ICacheStoragePath storage, ServiceCache sut) { const string testJson = @"{'test_value': 'Foo'}"; - storage.Path.Returns("testpath"); - config.BaseUrl.Returns("http://localhost:1234"); - - var testJsonPath = fs.CurrentDirectory() - .SubDirectory("testpath") - .SubDirectory("be8fbc8f") - .File($"{ValidObjectName}.json").FullName; - + const string testJsonPath = "cacheFile.json"; fs.AddFile(testJsonPath, new MockFileData(testJson)); + storage.CalculatePath(default!).ReturnsForAnyArgs(fs.FileInfo.FromFileName(testJsonPath)); + var obj = sut.Load(); obj.Should().NotBeNull(); obj!.TestValue.Should().Be("Foo"); } - [Test] - public void Loading_with_invalid_object_name_throws() + [Test, AutoMockData] + public void Loading_with_invalid_object_name_throws(ServiceCache sut) { - var ctx = new Context(); - - Action act = () => ctx.Cache.Load(); + Action act = () => sut.Load(); act.Should() .Throw() .WithMessage("*'invalid+name' has unacceptable characters*"); } - [Test] - public void Loading_without_attribute_throws() + [Test, AutoMockData] + public void Loading_without_attribute_throws(ServiceCache sut) { - var ctx = new Context(); - - Action act = () => ctx.Cache.Load(); + Action act = () => sut.Load(); act.Should() .Throw() .WithMessage("CacheObjectNameAttribute is missing*"); } - [Test] - public void Properties_are_saved_using_snake_case() + [Test, AutoMockData] + public void Properties_are_saved_using_snake_case( + [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, + [Frozen] ICacheStoragePath storage, + ServiceCache sut) { - var ctx = new Context(); - ctx.StoragePath.Path.Returns("testpath"); - ctx.Cache.Save(new ObjectWithAttribute {TestValue = "Foo"}); + storage.CalculatePath(default!).ReturnsForAnyArgs(_ => fs.FileInfo.FromFileName($"{ValidObjectName}.json")); + + sut.Save(new ObjectWithAttribute {TestValue = "Foo"}); - ctx.Filesystem.File.Received() - .WriteAllText(Arg.Any(), Verify.That(json => json.Should().Contain("\"test_value\""))); + fs.AllFiles.Should().ContainMatch($"*{ValidObjectName}.json"); + + var file = fs.GetFile(storage.CalculatePath("").FullName); + file.Should().NotBeNull(); + file.TextContents.Should().Contain("\"test_value\""); } [Test, AutoMockData] public void Saving_with_attribute_parses_correctly( [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, - [Frozen] IServiceConfiguration config, [Frozen] ICacheStoragePath storage, ServiceCache sut) { - storage.Path.Returns("testpath"); - config.BaseUrl.Returns("http://localhost:1234"); - - var testJsonPath = fs.CurrentDirectory() - .SubDirectory("testpath") - .SubDirectory("be8fbc8f") - .File($"{ValidObjectName}.json").FullName; + const string testJsonPath = "cacheFile.json"; + storage.CalculatePath(default!).ReturnsForAnyArgs(fs.FileInfo.FromFileName(testJsonPath)); sut.Save(new ObjectWithAttribute {TestValue = "Foo"}); @@ -145,24 +114,20 @@ public class ServiceCacheTest }"); } - [Test] - public void Saving_with_invalid_object_name_throws() + [Test, AutoMockData] + public void Saving_with_invalid_object_name_throws(ServiceCache sut) { - var ctx = new Context(); - - var act = () => ctx.Cache.Save(new ObjectWithAttributeInvalidChars()); + var act = () => sut.Save(new ObjectWithAttributeInvalidChars()); act.Should() .Throw() .WithMessage("*'invalid+name' has unacceptable characters*"); } - [Test] - public void Saving_without_attribute_throws() + [Test, AutoMockData] + public void Saving_without_attribute_throws(ServiceCache sut) { - var ctx = new Context(); - - var act = () => ctx.Cache.Save(new ObjectWithoutAttribute()); + var act = () => sut.Save(new ObjectWithoutAttribute()); act.Should() .Throw() @@ -172,32 +137,66 @@ public class ServiceCacheTest [Test, AutoMockData] public void Switching_config_and_base_url_should_yield_different_cache_paths( [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, - [Frozen] IServiceConfiguration config, + [Frozen] ICacheStoragePath storage, ServiceCache sut) { - config.BaseUrl.Returns("http://localhost:1234"); - + storage.CalculatePath(default!).ReturnsForAnyArgs(fs.FileInfo.FromFileName("Foo.json")); sut.Save(new ObjectWithAttribute {TestValue = "Foo"}); - // Change the active config & base URL so we get a different path - config.BaseUrl.Returns("http://localhost:5678"); - + storage.CalculatePath(default!).ReturnsForAnyArgs(fs.FileInfo.FromFileName("Bar.json")); sut.Save(new ObjectWithAttribute {TestValue = "Bar"}); - fs.AllFiles.Should().HaveCount(2) - .And.AllSatisfy(x => x.Should().EndWith("json")); + var expectedFiles = new[] {"*Foo.json", "*Bar.json"}; + foreach (var expectedFile in expectedFiles) + { + fs.AllFiles.Should().ContainMatch(expectedFile); + } } - [Test] - public void When_cache_file_is_empty_do_not_throw() + [Test, AutoMockData] + public void When_cache_file_is_empty_do_not_throw( + [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, + [Frozen] ICacheStoragePath storage, + ServiceCache sut) { - var ctx = new Context(); - ctx.Filesystem.File.Exists(Arg.Any()).Returns(true); - ctx.Filesystem.File.ReadAllText(Arg.Any()) - .Returns(_ => ""); + storage.CalculatePath(default!).ReturnsForAnyArgs(fs.FileInfo.FromFileName("cacheFile.json")); + fs.AddFile("cacheFile.json", new MockFileData("")); - Action act = () => ctx.Cache.Load(); + Action act = () => sut.Load(); act.Should().NotThrow(); } + + [Test, AutoMockData] + public void Name_properties_get_truncated_on_load( + [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, + [Frozen] ICacheStoragePath storage, + ServiceCache sut) + { + const string cacheJson = @" +{ + 'version': 1, + 'trash_id_mappings': [ + { + 'custom_format_name': '4K Remaster', + 'trash_id': 'eca37840c13c6ef2dd0262b141a5482f', + 'custom_format_id': 4 + } + ] +} +"; + + fs.AddFile("cacheFile.json", new MockFileData(cacheJson)); + storage.CalculatePath(default!).ReturnsForAnyArgs(fs.FileInfo.FromFileName("cacheFile.json")); + + var result = sut.Load(); + + result.Should().BeEquivalentTo(new CustomFormatCache + { + TrashIdMappings = new Collection + { + new("eca37840c13c6ef2dd0262b141a5482f", 4) + } + }); + } } diff --git a/src/TrashLib.Tests/CustomFormat/Processors/PersistenceSteps/JsonTransactionStepTest.cs b/src/TrashLib.Tests/CustomFormat/Processors/PersistenceSteps/JsonTransactionStepTest.cs index e69efff3..13c8e8a1 100644 --- a/src/TrashLib.Tests/CustomFormat/Processors/PersistenceSteps/JsonTransactionStepTest.cs +++ b/src/TrashLib.Tests/CustomFormat/Processors/PersistenceSteps/JsonTransactionStepTest.cs @@ -42,62 +42,6 @@ namespace TrashLib.Tests.CustomFormat.Processors.PersistenceSteps; [Parallelizable(ParallelScope.All)] public class JsonTransactionStepTest { - [TestCase(1, "cf2")] - [TestCase(2, "cf1")] - [TestCase(null, "cf1")] - public void Updates_using_combination_of_id_and_name(int? id, string guideCfName) - { - const string radarrCfData = @"{ - 'id': 1, - 'name': 'cf1', - 'specifications': [{ - 'name': 'spec1', - 'fields': [{ - 'name': 'value', - 'value': 'value1' - }] - }] -}"; - var guideCfData = JObject.Parse(@"{ - 'name': 'cf1', - 'specifications': [{ - 'name': 'spec1', - 'new': 'valuenew', - 'fields': { - 'value': 'value2' - } - }] -}"); - var cacheEntry = id != null ? new TrashIdMapping("") {CustomFormatId = id.Value} : null; - - var guideCfs = new List - { - NewCf.Processed(guideCfName, "", guideCfData, cacheEntry) - }; - - var processor = new JsonTransactionStep(); - processor.Process(guideCfs, new[] {JObject.Parse(radarrCfData)}); - - var expectedTransactions = new CustomFormatTransactionData(); - expectedTransactions.UpdatedCustomFormats.Add(guideCfs[0]); - processor.Transactions.Should().BeEquivalentTo(expectedTransactions); - - const string expectedJsonData = @"{ - 'id': 1, - 'name': 'cf1', - 'specifications': [{ - 'name': 'spec1', - 'new': 'valuenew', - 'fields': [{ - 'name': 'value', - 'value': 'value2' - }] - }] -}"; - processor.Transactions.UpdatedCustomFormats.First().Json.Should() - .BeEquivalentTo(JObject.Parse(expectedJsonData), op => op.Using(new JsonEquivalencyStep())); - } - [Test] public void Combination_of_create_update_and_unchanged_and_verify_proper_json_merging() { @@ -359,65 +303,4 @@ public class JsonTransactionStepTest expectedTransactions.DeletedCustomFormatIds.Add(new TrashIdMapping("testtrashid", 2)); processor.Transactions.Should().BeEquivalentTo(expectedTransactions); } - - [Test] - public void Updated_and_unchanged_custom_formats_have_cache_entry_set_when_there_is_no_cache() - { - const string radarrCfData = @"[{ - 'id': 1, - 'name': 'updated', - 'specifications': [{ - 'name': 'spec2', - 'fields': [{ - 'name': 'value', - 'value': 'value1' - }] - }] -}, { - 'id': 2, - 'name': 'no_change', - 'specifications': [{ - 'name': 'spec4', - 'negate': false, - 'fields': [{ - 'name': 'value', - 'value': 'value1' - }] - }] -}]"; - var guideCfData = JsonConvert.DeserializeObject>(@"[{ - 'name': 'updated', - 'specifications': [{ - 'name': 'spec2', - 'fields': { - 'value': 'value2' - } - }] -}, { - 'name': 'no_change', - 'specifications': [{ - 'name': 'spec4', - 'negate': false, - 'fields': { - 'value': 'value1' - } - }] -}]"); - - var radarrCfs = JsonConvert.DeserializeObject>(radarrCfData); - var guideCfs = new List - { - NewCf.Processed("updated", "", guideCfData![0]), - NewCf.Processed("no_change", "", guideCfData[1]) - }; - - var processor = new JsonTransactionStep(); - processor.Process(guideCfs, radarrCfs!); - - processor.Transactions.UpdatedCustomFormats.First().CacheEntry.Should() - .BeEquivalentTo(new TrashIdMapping("", 1)); - - processor.Transactions.UnchangedCustomFormats.First().CacheEntry.Should() - .BeEquivalentTo(new TrashIdMapping("", 2)); - } } diff --git a/src/TrashLib.Tests/Radarr/RadarrConfigurationTest.cs b/src/TrashLib.Tests/Radarr/RadarrConfigurationTest.cs index 58b8d814..93786495 100644 --- a/src/TrashLib.Tests/Radarr/RadarrConfigurationTest.cs +++ b/src/TrashLib.Tests/Radarr/RadarrConfigurationTest.cs @@ -25,15 +25,8 @@ public class RadarrConfigurationTest _container = builder.Build(); } - private static readonly TestCaseData[] NameOrIdsTestData = - { - new(new Collection {"name"}, new Collection()), - new(new Collection(), new Collection {"trash_id"}) - }; - - [TestCaseSource(nameof(NameOrIdsTestData))] - public void Custom_format_is_valid_with_trash_id(Collection namesList, - Collection trashIdsList) + [Test] + public void Custom_format_is_valid_with_trash_id() { var config = new RadarrConfiguration { @@ -41,7 +34,7 @@ public class RadarrConfigurationTest BaseUrl = "required value", CustomFormats = new List { - new() {TrashIds = trashIdsList} + new() {TrashIds = new Collection {"trash_id"}} } }; diff --git a/src/TrashLib/Cache/ICacheStoragePath.cs b/src/TrashLib/Cache/ICacheStoragePath.cs index 68c7d6a2..14fbf0b7 100644 --- a/src/TrashLib/Cache/ICacheStoragePath.cs +++ b/src/TrashLib/Cache/ICacheStoragePath.cs @@ -1,6 +1,8 @@ +using System.IO.Abstractions; + namespace TrashLib.Cache; public interface ICacheStoragePath { - string Path { get; } + IFileInfo CalculatePath(string cacheObjectName); } diff --git a/src/TrashLib/Cache/ServiceCache.cs b/src/TrashLib/Cache/ServiceCache.cs index a6cba5d6..7313fb62 100644 --- a/src/TrashLib/Cache/ServiceCache.cs +++ b/src/TrashLib/Cache/ServiceCache.cs @@ -1,35 +1,22 @@ -using System.Data.HashFunction.FNV; using System.IO.Abstractions; using System.Reflection; -using System.Text; using System.Text.RegularExpressions; using Newtonsoft.Json; using Newtonsoft.Json.Serialization; using Serilog; -using TrashLib.Config.Services; namespace TrashLib.Cache; public class ServiceCache : IServiceCache { private static readonly Regex AllowedObjectNameCharacters = new(@"^[\w-]+$", RegexOptions.Compiled); - private readonly IServiceConfiguration _config; - private readonly IFileSystem _fs; - private readonly IFNV1a _hash; private readonly ICacheStoragePath _storagePath; private readonly JsonSerializerSettings _jsonSettings; - public ServiceCache( - IFileSystem fs, - ICacheStoragePath storagePath, - IServiceConfiguration config, - ILogger log) + public ServiceCache(ICacheStoragePath storagePath, ILogger log) { - _fs = fs; _storagePath = storagePath; - _config = config; Log = log; - _hash = FNV1aFactory.Instance.Create(FNVConfig.GetPredefinedConfig(32)); _jsonSettings = new JsonSerializerSettings { Formatting = Formatting.Indented, @@ -45,12 +32,13 @@ public class ServiceCache : IServiceCache public T? Load() where T : class { var path = PathFromAttribute(); - if (!_fs.File.Exists(path)) + if (!path.Exists) { return null; } - var json = _fs.File.ReadAllText(path); + using var stream = path.OpenText(); + var json = stream.ReadToEnd(); try { @@ -67,8 +55,12 @@ public class ServiceCache : IServiceCache public void Save(T obj) where T : class { var path = PathFromAttribute(); - _fs.Directory.CreateDirectory(_fs.Path.GetDirectoryName(path)); - _fs.File.WriteAllText(path, JsonConvert.SerializeObject(obj, _jsonSettings)); + path.Directory.Create(); + + var serializer = JsonSerializer.Create(_jsonSettings); + + using var stream = new JsonTextWriter(path.CreateText()); + serializer.Serialize(stream, obj); } private static string GetCacheObjectNameAttribute() @@ -82,13 +74,7 @@ public class ServiceCache : IServiceCache return attribute.Name; } - private string BuildServiceGuid() - { - return _hash.ComputeHash(Encoding.ASCII.GetBytes(_config.BaseUrl)) - .AsHexString(); - } - - private string PathFromAttribute() + private IFileInfo PathFromAttribute() { var objectName = GetCacheObjectNameAttribute(); if (!AllowedObjectNameCharacters.IsMatch(objectName)) @@ -96,6 +82,6 @@ public class ServiceCache : IServiceCache throw new ArgumentException($"Object name '{objectName}' has unacceptable characters"); } - return _fs.Path.Combine(_storagePath.Path, BuildServiceGuid(), objectName + ".json"); + return _storagePath.CalculatePath(objectName); } } diff --git a/src/TrashLib/Services/CustomFormat/CachePersister.cs b/src/TrashLib/Services/CustomFormat/CachePersister.cs index 1bec73dc..028823df 100644 --- a/src/TrashLib/Services/CustomFormat/CachePersister.cs +++ b/src/TrashLib/Services/CustomFormat/CachePersister.cs @@ -6,7 +6,7 @@ using TrashLib.Services.CustomFormat.Models.Cache; namespace TrashLib.Services.CustomFormat; -internal class CachePersister : ICachePersister +public class CachePersister : ICachePersister { private readonly IServiceCache _cache; diff --git a/src/TrashLib/Services/CustomFormat/Processors/PersistenceSteps/JsonTransactionStep.cs b/src/TrashLib/Services/CustomFormat/Processors/PersistenceSteps/JsonTransactionStep.cs index d818da79..cd3b35c9 100644 --- a/src/TrashLib/Services/CustomFormat/Processors/PersistenceSteps/JsonTransactionStep.cs +++ b/src/TrashLib/Services/CustomFormat/Processors/PersistenceSteps/JsonTransactionStep.cs @@ -80,7 +80,7 @@ internal class JsonTransactionStep : IJsonTransactionStep JObject? match = null; // Try to find match in cache first - if (cfId != null) + if (cfId is not null) { match = serviceCfs.FirstOrDefault(rcf => cfId == rcf.Value("id")); }