From 7c5da06e57ee816bda00f739a14210bd9c451926 Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Fri, 30 Jun 2023 21:43:45 -0500 Subject: [PATCH] refactor: Remove instance name from cache storage path This step is necessary to support merging of instance sections based on a shared URL. --- src/Recyclarr.Cli/Cache/CachePersister.cs | 1 + src/Recyclarr.Cli/Cache/CustomFormatCache.cs | 1 + src/Recyclarr.Cli/Cache/ServiceCache.cs | 10 ++- .../Console/Helpers/CacheStoragePath.cs | 65 ++++++++++++++-- .../CustomFormat/CustomFormatSyncPipeline.cs | 5 +- .../Interfaces/ICacheStoragePath.cs | 1 + .../Console/Helpers/CacheStoragePathTest.cs | 74 +++++++++++++++++- .../FluentAssertionsExtensions.cs | 77 +++++++++++++++++++ 8 files changed, 221 insertions(+), 13 deletions(-) create mode 100644 src/tests/Recyclarr.TestLibrary/FluentAssertions/FluentAssertionsExtensions.cs diff --git a/src/Recyclarr.Cli/Cache/CachePersister.cs b/src/Recyclarr.Cli/Cache/CachePersister.cs index 59979c98..2caf873f 100644 --- a/src/Recyclarr.Cli/Cache/CachePersister.cs +++ b/src/Recyclarr.Cli/Cache/CachePersister.cs @@ -38,6 +38,7 @@ public class CachePersister : ICachePersister public void Save(IServiceConfiguration config, CustomFormatCache cache) { _log.Debug("Saving Cache with {Mappings}", JsonConvert.SerializeObject(cache.TrashIdMappings)); + _cache.Save(cache, config); } } diff --git a/src/Recyclarr.Cli/Cache/CustomFormatCache.cs b/src/Recyclarr.Cli/Cache/CustomFormatCache.cs index f90a29da..969d49f0 100644 --- a/src/Recyclarr.Cli/Cache/CustomFormatCache.cs +++ b/src/Recyclarr.Cli/Cache/CustomFormatCache.cs @@ -9,6 +9,7 @@ public record CustomFormatCache public const int LatestVersion = 1; public int Version { get; init; } = LatestVersion; + public string? InstanceName { get; init; } public IReadOnlyList TrashIdMappings { get; init; } = new List(); public CustomFormatCache Update(CustomFormatTransactionData transactions) diff --git a/src/Recyclarr.Cli/Cache/ServiceCache.cs b/src/Recyclarr.Cli/Cache/ServiceCache.cs index af70113b..299b44ee 100644 --- a/src/Recyclarr.Cli/Cache/ServiceCache.cs +++ b/src/Recyclarr.Cli/Cache/ServiceCache.cs @@ -31,7 +31,7 @@ public partial class ServiceCache : IServiceCache public T? Load(IServiceConfiguration config) where T : class { - var path = PathFromAttribute(config); + var path = PathFromAttribute(config, true); _log.Debug("Loading cache from path: {Path}", path.FullName); if (!path.Exists) { @@ -77,7 +77,7 @@ public partial class ServiceCache : IServiceCache return attribute.Name; } - private IFileInfo PathFromAttribute(IServiceConfiguration config) + private IFileInfo PathFromAttribute(IServiceConfiguration config, bool migratePath = false) { var objectName = GetCacheObjectNameAttribute(); if (!AllowedObjectNameCharactersRegex().IsMatch(objectName)) @@ -85,6 +85,12 @@ public partial class ServiceCache : IServiceCache throw new ArgumentException($"Object name '{objectName}' has unacceptable characters"); } + if (migratePath) + { + // Only do this while loading the cache. Saving should always use the direct (latest) path. + _storagePath.MigrateOldPath(config, objectName); + } + return _storagePath.CalculatePath(config, objectName); } diff --git a/src/Recyclarr.Cli/Console/Helpers/CacheStoragePath.cs b/src/Recyclarr.Cli/Console/Helpers/CacheStoragePath.cs index 9e68060b..90b03e68 100644 --- a/src/Recyclarr.Cli/Console/Helpers/CacheStoragePath.cs +++ b/src/Recyclarr.Cli/Console/Helpers/CacheStoragePath.cs @@ -10,28 +10,79 @@ namespace Recyclarr.Cli.Console.Helpers; public class CacheStoragePath : ICacheStoragePath { + private readonly ILogger _log; private readonly IAppPaths _paths; + private readonly IFNV1a _hashOld; private readonly IFNV1a _hash; - public CacheStoragePath( - IAppPaths paths) + public CacheStoragePath(ILogger log, IAppPaths paths) { + _log = log; _paths = paths; - _hash = FNV1aFactory.Instance.Create(FNVConfig.GetPredefinedConfig(32)); + _hashOld = FNV1aFactory.Instance.Create(FNVConfig.GetPredefinedConfig(32)); + _hash = FNV1aFactory.Instance.Create(FNVConfig.GetPredefinedConfig(64)); } private string BuildUniqueServiceDir(IServiceConfiguration config) { var url = config.BaseUrl.OriginalString; - var guid = _hash.ComputeHash(Encoding.ASCII.GetBytes(url)).AsHexString(); - return $"{config.InstanceName}_{guid}"; + return _hash.ComputeHash(Encoding.ASCII.GetBytes(url)).AsHexString(); } - public IFileInfo CalculatePath(IServiceConfiguration config, string cacheObjectName) + // TODO: Remove backward compatibility for cache dir names later + private string BuildOldUniqueServiceDir(IServiceConfiguration config) + { + var url = config.BaseUrl.OriginalString; + var hash = _hashOld.ComputeHash(Encoding.ASCII.GetBytes(url)).AsHexString(); + return $"{config.InstanceName}_{hash}"; + } + + private IFileInfo CalculatePathInternal(IServiceConfiguration config, string cacheObjectName, string serviceDir) { return _paths.CacheDirectory .SubDirectory(config.ServiceType.ToString().ToLower(CultureInfo.CurrentCulture)) - .SubDirectory(BuildUniqueServiceDir(config)) + .SubDirectory(serviceDir) .File(cacheObjectName + ".json"); } + + public IFileInfo CalculatePath(IServiceConfiguration config, string cacheObjectName) + { + return CalculatePathInternal(config, cacheObjectName, BuildUniqueServiceDir(config)); + } + + public IFileInfo CalculateOldPath(IServiceConfiguration config, string cacheObjectName) + { + return CalculatePathInternal(config, cacheObjectName, BuildOldUniqueServiceDir(config)); + } + + public void MigrateOldPath(IServiceConfiguration config, string cacheObjectName) + { + var oldServiceDir = CalculateOldPath(config, cacheObjectName).Directory; + var newServiceDir = CalculatePath(config, cacheObjectName).Directory; + + if (oldServiceDir is null || newServiceDir is null) + { + _log.Debug("Cache Migration: Unable to migrate cache dir due to null value for either old or new path"); + return; + } + + if (!oldServiceDir.Exists) + { + _log.Debug("Cache Migration: Old path doesn't exist; skipping"); + return; + } + + if (newServiceDir.Exists) + { + // New dir already exists, so we can't move. Delete it. + _log.Debug("Cache Migration: Deleting {OldDir}", oldServiceDir); + oldServiceDir.Delete(true); + } + else + { + // New dir doesn't exist yet; so rename old to new. + _log.Debug("Cache Migration: Moving from {OldDir} to {NewDir}", oldServiceDir, newServiceDir); + oldServiceDir.MoveTo(newServiceDir.FullName); + } + } } diff --git a/src/Recyclarr.Cli/Pipelines/CustomFormat/CustomFormatSyncPipeline.cs b/src/Recyclarr.Cli/Pipelines/CustomFormat/CustomFormatSyncPipeline.cs index 77eb3db9..352be4ac 100644 --- a/src/Recyclarr.Cli/Pipelines/CustomFormat/CustomFormatSyncPipeline.cs +++ b/src/Recyclarr.Cli/Pipelines/CustomFormat/CustomFormatSyncPipeline.cs @@ -69,6 +69,9 @@ public class CustomFormatSyncPipeline : ISyncPipeline await _phases.ApiPersistencePhase.Execute(config, transactions); - _cachePersister.Save(config, cache.Update(transactions)); + _cachePersister.Save(config, cache.Update(transactions) with + { + InstanceName = config.InstanceName + }); } } diff --git a/src/Recyclarr.TrashLib/Interfaces/ICacheStoragePath.cs b/src/Recyclarr.TrashLib/Interfaces/ICacheStoragePath.cs index 0546f4e8..91a7e527 100644 --- a/src/Recyclarr.TrashLib/Interfaces/ICacheStoragePath.cs +++ b/src/Recyclarr.TrashLib/Interfaces/ICacheStoragePath.cs @@ -6,4 +6,5 @@ namespace Recyclarr.TrashLib.Interfaces; public interface ICacheStoragePath { IFileInfo CalculatePath(IServiceConfiguration config, string cacheObjectName); + void MigrateOldPath(IServiceConfiguration config, string cacheObjectName); } diff --git a/src/tests/Recyclarr.Cli.Tests/Console/Helpers/CacheStoragePathTest.cs b/src/tests/Recyclarr.Cli.Tests/Console/Helpers/CacheStoragePathTest.cs index e0539abd..c5cdb0b8 100644 --- a/src/tests/Recyclarr.Cli.Tests/Console/Helpers/CacheStoragePathTest.cs +++ b/src/tests/Recyclarr.Cli.Tests/Console/Helpers/CacheStoragePathTest.cs @@ -8,16 +8,84 @@ namespace Recyclarr.Cli.Tests.Console.Helpers; public class CacheStoragePathTest { [Test, AutoMockData] - public void Use_instance_name_in_path(CacheStoragePath sut) + public void Use_correct_name_in_path(CacheStoragePath sut) { var config = new SonarrConfiguration { - BaseUrl = new Uri("http://something"), + BaseUrl = new Uri("http://something/foo/bar"), InstanceName = "thename" }; var result = sut.CalculatePath(config, "obj"); - result.FullName.Should().MatchRegex(@".*[/\\]thename_[a-f0-9]+[/\\]obj\.json$"); + result.FullName.Should().MatchRegex(@".*[/\\][a-f0-9]+[/\\]obj\.json$"); + } + + [Test, AutoMockData] + public void Migration_old_path_moved_to_new_path( + [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, + CacheStoragePath sut) + { + var config = new SonarrConfiguration + { + BaseUrl = new Uri("http://something"), + InstanceName = "thename" + }; + + var oldPath = sut.CalculateOldPath(config, "obj"); + var newPath = sut.CalculatePath(config, "obj"); + + fs.AddEmptyFile(oldPath); + + sut.MigrateOldPath(config, "obj"); + + fs.AllFiles.Should().Contain(newPath.FullName); + fs.AllFiles.Should().NotContain(oldPath.FullName); + } + + [Test, AutoMockData] + public void Migration_old_path_deleted_when_new_path_already_exists( + [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, + CacheStoragePath sut) + { + var config = new SonarrConfiguration + { + BaseUrl = new Uri("http://something"), + InstanceName = "thename" + }; + + var oldPath = sut.CalculateOldPath(config, "obj"); + var newPath = sut.CalculatePath(config, "obj"); + + fs.AddEmptyFile(oldPath); + fs.AddFile(newPath, new MockFileData("something")); + + sut.MigrateOldPath(config, "obj"); + + fs.AllFiles.Should().NotContain(oldPath.FullName); + + var file = fs.GetFile(newPath); + file.Should().NotBeNull(); + file.TextContents.Should().Be("something"); + } + + [Test, AutoMockData] + public void Migration_nothing_moved_if_old_path_not_exist( + [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, + CacheStoragePath sut) + { + var config = new SonarrConfiguration + { + BaseUrl = new Uri("http://something"), + InstanceName = "thename" + }; + + var oldPath = sut.CalculateOldPath(config, "obj"); + var newPath = sut.CalculatePath(config, "obj"); + + sut.MigrateOldPath(config, "obj"); + + fs.AllFiles.Should().NotContain(oldPath.FullName); + fs.AllFiles.Should().NotContain(newPath.FullName); } } diff --git a/src/tests/Recyclarr.TestLibrary/FluentAssertions/FluentAssertionsExtensions.cs b/src/tests/Recyclarr.TestLibrary/FluentAssertions/FluentAssertionsExtensions.cs new file mode 100644 index 00000000..3141ed21 --- /dev/null +++ b/src/tests/Recyclarr.TestLibrary/FluentAssertions/FluentAssertionsExtensions.cs @@ -0,0 +1,77 @@ +using FluentAssertions.Collections; +using FluentAssertions.Execution; + +namespace Recyclarr.TestLibrary.FluentAssertions; + +public static class FluentAssertionsExtensions +{ + public static AndWhichConstraint ContainRegexMatch( + this StringCollectionAssertions assert, + string regexPattern, + string because = "", + params object[] becauseArgs + ) + where TCollection : IEnumerable + where TAssertions : StringCollectionAssertions + { + bool ContainsRegexMatch() + { + return assert.Subject.Any(item => + { + using var scope = new AssertionScope(); + item.Should().MatchRegex(regexPattern); + return !scope.Discard().Any(); + }); + } + + Execute.Assertion + .BecauseOf(because, becauseArgs) + .ForCondition(ContainsRegexMatch()) + .FailWith("Expected {context:collection} {0} to contain a regex match of {1}{reason}.", assert.Subject, + regexPattern); + + var matched = assert.Subject.Where(item => + { + using var scope = new AssertionScope(); + item.Should().MatchRegex(regexPattern); + return !scope.Discard().Any(); + }); + + return new AndWhichConstraint((TAssertions) assert, matched); + } + + public static AndWhichConstraint NotContainRegexMatch( + this StringCollectionAssertions assert, + string regexPattern, + string because = "", + params object[] becauseArgs + ) + where TCollection : IEnumerable + where TAssertions : StringCollectionAssertions + { + bool NotContainsRegexMatch() + { + return assert.Subject.Any(item => + { + using var scope = new AssertionScope(); + item.Should().NotMatchRegex(regexPattern); + return !scope.Discard().Any(); + }); + } + + Execute.Assertion + .BecauseOf(because, becauseArgs) + .ForCondition(NotContainsRegexMatch()) + .FailWith("Expected {context:collection} {0} to not contain a regex match of {1}{reason}.", assert.Subject, + regexPattern); + + var matched = assert.Subject.Where(item => + { + using var scope = new AssertionScope(); + item.Should().NotMatchRegex(regexPattern); + return !scope.Discard().Any(); + }); + + return new AndWhichConstraint((TAssertions) assert, matched); + } +}