refactor: Service cache test fixes & improved testability

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
+    private string BuildServiceGuid()
-        .SubDirectory(_serviceCommand.Name.ToLower()).FullName;
+    {
+        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");
+    }
 }

src/TestLibrary/FluentAssertions/JsonEquivalencyStep.cs

@@ -1,4 +1,3 @@
-using FluentAssertions;
 using FluentAssertions.Equivalency;
 using FluentAssertions.Json;
 using Newtonsoft.Json.Linq;

src/TrashLib.Tests/Cache/ServiceCacheTest.cs

@@ -1,16 +1,12 @@
-using System.IO.Abstractions;
+using System.Collections.ObjectModel;
-using System.IO.Abstractions.Extensions;
 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.CustomFormat.Models.Cache;
-using TrashLib.Services.Radarr.Config;
 
 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<IFileSystem>();
-            StoragePath = Substitute.For<ICacheStoragePath>();
-
-            var config = new RadarrConfiguration {BaseUrl = "http://localhost:1234"};
-            Cache = new ServiceCache(Filesystem, StoragePath, config, Substitute.For<ILogger>());
-        }
-
-        public ServiceCache Cache { get; }
-        public ICacheStoragePath StoragePath { get; }
-        public IFileSystem Filesystem { get; }
-    }
-
     private class ObjectWithoutAttribute
     {
     }
@@ -51,90 +31,79 @@ public class ServiceCacheTest
     {
     }
 
-    [Test]
+    [Test, AutoMockData]
-    public void Load_returns_null_when_file_does_not_exist()
+    public void Load_returns_null_when_file_does_not_exist(
+        [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs,
+        ServiceCache sut)
     {
-        var ctx = new Context();
+        var result = sut.Load<ObjectWithAttribute>();
-        ctx.Filesystem.File.Exists(Arg.Any<string>()).Returns(false);
-
-        var result = ctx.Cache.Load<ObjectWithAttribute>();
         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");
+        const string testJsonPath = "cacheFile.json";
-        config.BaseUrl.Returns("http://localhost:1234");
-
-        var testJsonPath = fs.CurrentDirectory()
-            .SubDirectory("testpath")
-            .SubDirectory("be8fbc8f")
-            .File($"{ValidObjectName}.json").FullName;
-
         fs.AddFile(testJsonPath, new MockFileData(testJson));
 
+        storage.CalculatePath(default!).ReturnsForAnyArgs(fs.FileInfo.FromFileName(testJsonPath));
+
         var obj = sut.Load<ObjectWithAttribute>();
 
         obj.Should().NotBeNull();
         obj!.TestValue.Should().Be("Foo");
     }
 
-    [Test]
+    [Test, AutoMockData]
-    public void Loading_with_invalid_object_name_throws()
+    public void Loading_with_invalid_object_name_throws(ServiceCache sut)
     {
-        var ctx = new Context();
+        Action act = () => sut.Load<ObjectWithAttributeInvalidChars>();
-
-        Action act = () => ctx.Cache.Load<ObjectWithAttributeInvalidChars>();
 
         act.Should()
             .Throw<ArgumentException>()
             .WithMessage("*'invalid+name' has unacceptable characters*");
     }
 
-    [Test]
+    [Test, AutoMockData]
-    public void Loading_without_attribute_throws()
+    public void Loading_without_attribute_throws(ServiceCache sut)
     {
-        var ctx = new Context();
+        Action act = () => sut.Load<ObjectWithoutAttribute>();
-
-        Action act = () => ctx.Cache.Load<ObjectWithoutAttribute>();
 
         act.Should()
             .Throw<ArgumentException>()
             .WithMessage("CacheObjectNameAttribute is missing*");
     }
 
-    [Test]
+    [Test, AutoMockData]
-    public void Properties_are_saved_using_snake_case()
+    public void Properties_are_saved_using_snake_case(
+        [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs,
+        [Frozen] ICacheStoragePath storage,
+        ServiceCache sut)
     {
-        var ctx = new Context();
+        storage.CalculatePath(default!).ReturnsForAnyArgs(_ => fs.FileInfo.FromFileName($"{ValidObjectName}.json"));
-        ctx.StoragePath.Path.Returns("testpath");
+
-        ctx.Cache.Save(new ObjectWithAttribute {TestValue = "Foo"});
+        sut.Save(new ObjectWithAttribute {TestValue = "Foo"});
 
-        ctx.Filesystem.File.Received()
+        fs.AllFiles.Should().ContainMatch($"*{ValidObjectName}.json");
-            .WriteAllText(Arg.Any<string>(), Verify.That<string>(json => json.Should().Contain("\"test_value\"")));
+
+        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");
+        const string testJsonPath = "cacheFile.json";
-        config.BaseUrl.Returns("http://localhost:1234");
+        storage.CalculatePath(default!).ReturnsForAnyArgs(fs.FileInfo.FromFileName(testJsonPath));
-
-        var testJsonPath = fs.CurrentDirectory()
-            .SubDirectory("testpath")
-            .SubDirectory("be8fbc8f")
-            .File($"{ValidObjectName}.json").FullName;
 
         sut.Save(new ObjectWithAttribute {TestValue = "Foo"});
 
@@ -145,24 +114,20 @@ public class ServiceCacheTest
 }");
     }
 
-    [Test]
+    [Test, AutoMockData]
-    public void Saving_with_invalid_object_name_throws()
+    public void Saving_with_invalid_object_name_throws(ServiceCache sut)
     {
-        var ctx = new Context();
+        var act = () => sut.Save(new ObjectWithAttributeInvalidChars());
-
-        var act = () => ctx.Cache.Save(new ObjectWithAttributeInvalidChars());
 
         act.Should()
             .Throw<ArgumentException>()
             .WithMessage("*'invalid+name' has unacceptable characters*");
     }
 
-    [Test]
+    [Test, AutoMockData]
-    public void Saving_without_attribute_throws()
+    public void Saving_without_attribute_throws(ServiceCache sut)
     {
-        var ctx = new Context();
+        var act = () => sut.Save(new ObjectWithoutAttribute());
-
-        var act = () => ctx.Cache.Save(new ObjectWithoutAttribute());
 
         act.Should()
             .Throw<ArgumentException>()
@@ -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
+        storage.CalculatePath(default!).ReturnsForAnyArgs(fs.FileInfo.FromFileName("Bar.json"));
-        config.BaseUrl.Returns("http://localhost:5678");
-
         sut.Save(new ObjectWithAttribute {TestValue = "Bar"});
 
-        fs.AllFiles.Should().HaveCount(2)
+        var expectedFiles = new[] {"*Foo.json", "*Bar.json"};
-            .And.AllSatisfy(x => x.Should().EndWith("json"));
+        foreach (var expectedFile in expectedFiles)
+        {
+            fs.AllFiles.Should().ContainMatch(expectedFile);
+        }
     }
 
-    [Test]
+    [Test, AutoMockData]
-    public void When_cache_file_is_empty_do_not_throw()
+    public void When_cache_file_is_empty_do_not_throw(
+        [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs,
+        [Frozen] ICacheStoragePath storage,
+        ServiceCache sut)
     {
-        var ctx = new Context();
+        storage.CalculatePath(default!).ReturnsForAnyArgs(fs.FileInfo.FromFileName("cacheFile.json"));
-        ctx.Filesystem.File.Exists(Arg.Any<string>()).Returns(true);
+        fs.AddFile("cacheFile.json", new MockFileData(""));
-        ctx.Filesystem.File.ReadAllText(Arg.Any<string>())
-            .Returns(_ => "");
 
-        Action act = () => ctx.Cache.Load<ObjectWithAttribute>();
+        Action act = () => sut.Load<ObjectWithAttribute>();
 
         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<CustomFormatCache>();
+
+        result.Should().BeEquivalentTo(new CustomFormatCache
+        {
+            TrashIdMappings = new Collection<TrashIdMapping>
+            {
+                new("eca37840c13c6ef2dd0262b141a5482f", 4)
+            }
+        });
+    }
 }

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<ProcessedCustomFormatData>
-        {
-            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<List<JObject>>(@"[{
-  'name': 'updated',
-  'specifications': [{
-    'name': 'spec2',
-    'fields': {
-      'value': 'value2'
-    }
-  }]
-}, {
-  'name': 'no_change',
-  'specifications': [{
-    'name': 'spec4',
-    'negate': false,
-    'fields': {
-      'value': 'value1'
-    }
-  }]
-}]");
-
-        var radarrCfs = JsonConvert.DeserializeObject<List<JObject>>(radarrCfData);
-        var guideCfs = new List<ProcessedCustomFormatData>
-        {
-            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));
-    }
 }

src/TrashLib.Tests/Radarr/RadarrConfigurationTest.cs

@@ -25,15 +25,8 @@ public class RadarrConfigurationTest
         _container = builder.Build();
     }
 
-    private static readonly TestCaseData[] NameOrIdsTestData =
+    [Test]
-    {
+    public void Custom_format_is_valid_with_trash_id()
-        new(new Collection<string> {"name"}, new Collection<string>()),
-        new(new Collection<string>(), new Collection<string> {"trash_id"})
-    };
-
-    [TestCaseSource(nameof(NameOrIdsTestData))]
-    public void Custom_format_is_valid_with_trash_id(Collection<string> namesList,
-        Collection<string> trashIdsList)
     {
         var config = new RadarrConfiguration
         {
@@ -41,7 +34,7 @@ public class RadarrConfigurationTest
             BaseUrl = "required value",
             CustomFormats = new List<CustomFormatConfig>
             {
-                new() {TrashIds = trashIdsList}
+                new() {TrashIds = new Collection<string> {"trash_id"}}
             }
         };
 

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);
 }

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(
+    public ServiceCache(ICacheStoragePath storagePath, ILogger log)
-        IFileSystem fs,
-        ICacheStoragePath storagePath,
-        IServiceConfiguration config,
-        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<T>() where T : class
     {
         var path = PathFromAttribute<T>();
-        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>(T obj) where T : class
     {
         var path = PathFromAttribute<T>();
-        _fs.Directory.CreateDirectory(_fs.Path.GetDirectoryName(path));
+        path.Directory.Create();
-        _fs.File.WriteAllText(path, JsonConvert.SerializeObject(obj, _jsonSettings));
+
+        var serializer = JsonSerializer.Create(_jsonSettings);
+
+        using var stream = new JsonTextWriter(path.CreateText());
+        serializer.Serialize(stream, obj);
     }
 
     private static string GetCacheObjectNameAttribute<T>()
@@ -82,13 +74,7 @@ public class ServiceCache : IServiceCache
         return attribute.Name;
     }
 
-    private string BuildServiceGuid()
+    private IFileInfo PathFromAttribute<T>()
-    {
-        return _hash.ComputeHash(Encoding.ASCII.GetBytes(_config.BaseUrl))
-            .AsHexString();
-    }
-
-    private string PathFromAttribute<T>()
     {
         var objectName = GetCacheObjectNameAttribute<T>();
         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);
     }
 }

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;
 

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<int>("id"));
         }