From d499537f91e7da4c3ddc9081b41bf11b30a5cd74 Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Sat, 28 May 2022 15:17:03 -0500 Subject: [PATCH] fix: Improve app data migration logic Smarter migration logic that does a directory merge instead of a straight move. This is designed to fail less in cases like the `recyclarr` directory already existing. --- CHANGELOG.md | 3 ++ .../Init/InitializeAppDataPathTest.cs | 3 ++ .../MigrateTrashUpdaterAppDataDirTest.cs | 14 ++++-- .../Migration/Steps/TestAppPaths.cs | 15 ++++++ src/Recyclarr/AppPaths.cs | 1 + .../Initialization/DefaultAppDataSetup.cs | 32 +++++++++++++ .../Initialization/IDefaultAppDataSetup.cs | 6 +++ .../Init/InitializeAppDataPath.cs | 14 +++--- .../InitializationAutofacModule.cs | 5 +- src/Recyclarr/Command/MigrateCommand.cs | 6 ++- .../Migration/MigrationAutofacModule.cs | 8 +++- .../Steps/MigrateTrashUpdaterAppDataDir.cs | 48 +++++++++---------- src/TrashLib/IAppPaths.cs | 1 + 13 files changed, 116 insertions(+), 40 deletions(-) create mode 100644 src/Recyclarr.Tests/Migration/Steps/TestAppPaths.cs create mode 100644 src/Recyclarr/Command/Initialization/DefaultAppDataSetup.cs create mode 100644 src/Recyclarr/Command/Initialization/IDefaultAppDataSetup.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index ea51fbd0..0e5cbc6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Version information in help output has been fixed. - If a HOME directory is not available, throw an error to the user (use `--app-data` instead). - Create `$HOME/.config` (on Linux) if it does not exist. +- Smarter migration logic in the `trash-updater` migration step that does a directory merge instead + of a straight move. This is designed to fail less in cases such as `recyclarr` directory already + existing. [appdata]: https://github.com/recyclarr/recyclarr/wiki/File-Structure diff --git a/src/Recyclarr.Tests/Command/Initialization/Init/InitializeAppDataPathTest.cs b/src/Recyclarr.Tests/Command/Initialization/Init/InitializeAppDataPathTest.cs index 870a3c8e..9c0dcfe9 100644 --- a/src/Recyclarr.Tests/Command/Initialization/Init/InitializeAppDataPathTest.cs +++ b/src/Recyclarr.Tests/Command/Initialization/Init/InitializeAppDataPathTest.cs @@ -5,6 +5,7 @@ using FluentAssertions; using NSubstitute; using NUnit.Framework; using Recyclarr.Command; +using Recyclarr.Command.Initialization; using Recyclarr.Command.Initialization.Init; using TestLibrary.AutoFixture; using TrashLib; @@ -20,9 +21,11 @@ public class InitializeAppDataPathTest [Frozen] IEnvironment env, [Frozen] IAppPaths paths, [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, + [Frozen(Matching.ImplementedInterfaces)] DefaultAppDataSetup appDataSetup, SonarrCommand cmd, InitializeAppDataPath sut) { + paths.DefaultAppDataDirectoryName.Returns("recyclarr"); env.GetFolderPath(Arg.Any(), Arg.Any()) .Returns("app_data"); diff --git a/src/Recyclarr.Tests/Migration/Steps/MigrateTrashUpdaterAppDataDirTest.cs b/src/Recyclarr.Tests/Migration/Steps/MigrateTrashUpdaterAppDataDirTest.cs index cf22110e..80b1c5b5 100644 --- a/src/Recyclarr.Tests/Migration/Steps/MigrateTrashUpdaterAppDataDirTest.cs +++ b/src/Recyclarr.Tests/Migration/Steps/MigrateTrashUpdaterAppDataDirTest.cs @@ -12,14 +12,15 @@ namespace Recyclarr.Tests.Migration.Steps; [Parallelizable(ParallelScope.All)] public class MigrateTrashUpdaterAppDataDirTest { - private static readonly string BasePath = Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData); + private const string BasePath = "base_path"; [Test, AutoMockData] public void Migration_check_returns_true_if_trash_updater_dir_exists( [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, + [Frozen(Matching.ImplementedInterfaces)] TestAppPaths paths, MigrateTrashUpdaterAppDataDir sut) { - fs.AddDirectory(Path.Combine(BasePath, "trash-updater")); + fs.AddDirectory(fs.Path.Combine(paths.BasePath, "trash-updater")); sut.CheckIfNeeded().Should().BeTrue(); } @@ -39,7 +40,7 @@ public class MigrateTrashUpdaterAppDataDirTest fs.AddDirectory(Path.Combine(BasePath, "trash-updater")); fs.AddDirectory(Path.Combine(BasePath, "recyclarr")); - var act = () => sut.Execute(); + var act = sut.Execute; act.Should().Throw(); } @@ -47,12 +48,15 @@ public class MigrateTrashUpdaterAppDataDirTest [Test, AutoMockData] public void Migration_success( [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, + [Frozen(Matching.ImplementedInterfaces)] TestAppPaths paths, MigrateTrashUpdaterAppDataDir sut) { - fs.AddDirectory(Path.Combine(BasePath, "trash-updater")); + // Add file instead of directory since the migration step only operates on files + fs.AddFile(fs.Path.Combine(paths.BasePath, "trash-updater", "1", "2", "test.txt"), new MockFileData("")); sut.Execute(); - fs.AllDirectories.Should().ContainSingle(x => Regex.IsMatch(x, @"[/\\]recyclarr$")); + fs.AllDirectories.Should().NotContain(x => x.Contains("trash-updater")); + fs.AllFiles.Should().Contain(x => Regex.IsMatch(x, @"[/\\]recyclarr[/\\]1[/\\]2[/\\]test.txt$")); } } diff --git a/src/Recyclarr.Tests/Migration/Steps/TestAppPaths.cs b/src/Recyclarr.Tests/Migration/Steps/TestAppPaths.cs new file mode 100644 index 00000000..1570528f --- /dev/null +++ b/src/Recyclarr.Tests/Migration/Steps/TestAppPaths.cs @@ -0,0 +1,15 @@ +using System.IO.Abstractions; + +namespace Recyclarr.Tests.Migration.Steps; + +public class TestAppPaths : AppPaths +{ + public string BasePath { get; } + + public TestAppPaths(IFileSystem fs) + : base(fs) + { + BasePath = fs.Path.Combine("base", "path"); + SetAppDataPath(fs.Path.Combine(BasePath, DefaultAppDataDirectoryName)); + } +} diff --git a/src/Recyclarr/AppPaths.cs b/src/Recyclarr/AppPaths.cs index 3ec97bc9..f24c8da3 100644 --- a/src/Recyclarr/AppPaths.cs +++ b/src/Recyclarr/AppPaths.cs @@ -15,6 +15,7 @@ public class AppPaths : IAppPaths } public string DefaultConfigFilename => "recyclarr.yml"; + public string DefaultAppDataDirectoryName => "recyclarr"; public bool IsAppDataPathValid => _appDataPath is not null; public void SetAppDataPath(string path) => _appDataPath = path; diff --git a/src/Recyclarr/Command/Initialization/DefaultAppDataSetup.cs b/src/Recyclarr/Command/Initialization/DefaultAppDataSetup.cs new file mode 100644 index 00000000..a582baf2 --- /dev/null +++ b/src/Recyclarr/Command/Initialization/DefaultAppDataSetup.cs @@ -0,0 +1,32 @@ +using System.IO.Abstractions; +using Common; +using TrashLib; + +namespace Recyclarr.Command.Initialization; + +public class DefaultAppDataSetup : IDefaultAppDataSetup +{ + private readonly IEnvironment _env; + private readonly IAppPaths _paths; + private readonly IFileSystem _fs; + + public DefaultAppDataSetup(IEnvironment env, IAppPaths paths, IFileSystem fs) + { + _env = env; + _paths = paths; + _fs = fs; + } + + public void SetupDefaultPath(bool forceCreate = false) + { + var appData = _env.GetFolderPath(Environment.SpecialFolder.ApplicationData, + forceCreate ? Environment.SpecialFolderOption.Create : Environment.SpecialFolderOption.None); + + if (string.IsNullOrEmpty(appData)) + { + throw new DirectoryNotFoundException("Unable to find the default app data directory"); + } + + _paths.SetAppDataPath(_fs.Path.Combine(appData, _paths.DefaultAppDataDirectoryName)); + } +} diff --git a/src/Recyclarr/Command/Initialization/IDefaultAppDataSetup.cs b/src/Recyclarr/Command/Initialization/IDefaultAppDataSetup.cs new file mode 100644 index 00000000..f545e904 --- /dev/null +++ b/src/Recyclarr/Command/Initialization/IDefaultAppDataSetup.cs @@ -0,0 +1,6 @@ +namespace Recyclarr.Command.Initialization; + +public interface IDefaultAppDataSetup +{ + void SetupDefaultPath(bool forceCreate = false); +} diff --git a/src/Recyclarr/Command/Initialization/Init/InitializeAppDataPath.cs b/src/Recyclarr/Command/Initialization/Init/InitializeAppDataPath.cs index 83418c06..5a965942 100644 --- a/src/Recyclarr/Command/Initialization/Init/InitializeAppDataPath.cs +++ b/src/Recyclarr/Command/Initialization/Init/InitializeAppDataPath.cs @@ -10,12 +10,18 @@ public class InitializeAppDataPath : IServiceInitializer private readonly IFileSystem _fs; private readonly IAppPaths _paths; private readonly IEnvironment _env; + private readonly IDefaultAppDataSetup _appDataSetup; - public InitializeAppDataPath(IFileSystem fs, IAppPaths paths, IEnvironment env) + public InitializeAppDataPath( + IFileSystem fs, + IAppPaths paths, + IEnvironment env, + IDefaultAppDataSetup appDataSetup) { _fs = fs; _paths = paths; _env = env; + _appDataSetup = appDataSetup; } public void Initialize(ServiceCommand cmd) @@ -36,11 +42,7 @@ public class InitializeAppDataPath : IServiceInitializer // Set app data path to application directory value (e.g. `$HOME/.config` on Linux) and ensure it is // created. - var appData = _env.GetFolderPath( - Environment.SpecialFolder.ApplicationData, - Environment.SpecialFolderOption.Create); - - _paths.SetAppDataPath(_fs.Path.Combine(appData, "recyclarr")); + _appDataSetup.SetupDefaultPath(true); } else { diff --git a/src/Recyclarr/Command/Initialization/InitializationAutofacModule.cs b/src/Recyclarr/Command/Initialization/InitializationAutofacModule.cs index f72bd274..6f6bf87b 100644 --- a/src/Recyclarr/Command/Initialization/InitializationAutofacModule.cs +++ b/src/Recyclarr/Command/Initialization/InitializationAutofacModule.cs @@ -11,12 +11,13 @@ public class InitializationAutofacModule : Module { base.Load(builder); builder.RegisterType().As(); + builder.RegisterType().As(); // Initialization Services builder.RegisterTypes( typeof(InitializeAppDataPath), - typeof(ServiceInitializer), - typeof(CheckMigrationNeeded)) + typeof(CheckMigrationNeeded), + typeof(ServiceInitializer)) .As() .OrderByRegistration(); diff --git a/src/Recyclarr/Command/MigrateCommand.cs b/src/Recyclarr/Command/MigrateCommand.cs index 653def72..d0e2a9eb 100644 --- a/src/Recyclarr/Command/MigrateCommand.cs +++ b/src/Recyclarr/Command/MigrateCommand.cs @@ -4,6 +4,7 @@ using CliFx.Attributes; using CliFx.Exceptions; using CliFx.Infrastructure; using JetBrains.Annotations; +using Recyclarr.Command.Initialization; using Recyclarr.Migration; namespace Recyclarr.Command; @@ -13,14 +14,17 @@ namespace Recyclarr.Command; public class MigrateCommand : ICommand { private readonly IMigrationExecutor _migration; + private readonly IDefaultAppDataSetup _appDataSetup; - public MigrateCommand(IMigrationExecutor migration) + public MigrateCommand(IMigrationExecutor migration, IDefaultAppDataSetup appDataSetup) { _migration = migration; + _appDataSetup = appDataSetup; } public ValueTask ExecuteAsync(IConsole console) { + _appDataSetup.SetupDefaultPath(); PerformMigrations(); return ValueTask.CompletedTask; } diff --git a/src/Recyclarr/Migration/MigrationAutofacModule.cs b/src/Recyclarr/Migration/MigrationAutofacModule.cs index 7259a5d0..6be8bf6e 100644 --- a/src/Recyclarr/Migration/MigrationAutofacModule.cs +++ b/src/Recyclarr/Migration/MigrationAutofacModule.cs @@ -11,7 +11,11 @@ public class MigrationAutofacModule : Module builder.RegisterType().As(); // Migration Steps - builder.RegisterType().As(); - builder.RegisterType().As(); + builder.RegisterTypes + ( + typeof(MigrateTrashYml), + typeof(MigrateTrashUpdaterAppDataDir) + ) + .As(); } } diff --git a/src/Recyclarr/Migration/Steps/MigrateTrashUpdaterAppDataDir.cs b/src/Recyclarr/Migration/Steps/MigrateTrashUpdaterAppDataDir.cs index cb2fe6af..59a697a2 100644 --- a/src/Recyclarr/Migration/Steps/MigrateTrashUpdaterAppDataDir.cs +++ b/src/Recyclarr/Migration/Steps/MigrateTrashUpdaterAppDataDir.cs @@ -1,5 +1,7 @@ using System.IO.Abstractions; +using Common.Extensions; using JetBrains.Annotations; +using TrashLib; namespace Recyclarr.Migration.Steps; @@ -9,37 +11,35 @@ namespace Recyclarr.Migration.Steps; [UsedImplicitly] public class MigrateTrashUpdaterAppDataDir : IMigrationStep { - private readonly IFileSystem _fileSystem; - - private readonly string _oldPath = - Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), "trash-updater"); - - // Do not use AppPaths class here since that may change yet again in the future and break this migration step. - private readonly string _newPath = - Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), "recyclarr"); + private readonly IFileSystem _fs; + private readonly Lazy _newPath, _oldPath; public int Order => 20; - public string Description { get; } - public IReadOnlyCollection Remediation { get; } public bool Required => true; - public MigrateTrashUpdaterAppDataDir(IFileSystem fileSystem) + public string Description + => $"Merge files from old app data directory `{GetOldPath()}` into `{GetNewPath()}` and delete old directory"; + + public IReadOnlyCollection Remediation => new[] { - _fileSystem = fileSystem; - Remediation = new[] - { - $"Check if `{_newPath}` already exists. If so, manually copy settings you want and then delete `{_oldPath}` to fix the error.", - $"Ensure Recyclarr has permission to recursively delete {_oldPath}", - $"Ensure Recyclarr has permission to create {_newPath}" - }; - - Description = $"Rename app data directory from `{_oldPath}` to `{_newPath}`"; - } + $"Check if `{GetNewPath()}` already exists. If so, manually copy all files from `{GetOldPath()}` and delete it to fix the error.", + $"Ensure Recyclarr has permission to recursively delete {GetOldPath()}", + $"Ensure Recyclarr has permission to create and move files into {GetNewPath()}" + }; - public bool CheckIfNeeded() => _fileSystem.Directory.Exists(_oldPath); + private string GetNewPath() => _newPath.Value; + private string GetOldPath() => _oldPath.Value; - public void Execute() + public MigrateTrashUpdaterAppDataDir(IFileSystem fs, IAppPaths paths) { - _fileSystem.Directory.Move(_oldPath, _newPath); + _fs = fs; + + // Will be something like `/home/user/.config/recyclarr`. + _newPath = new Lazy(paths.GetAppDataPath); + _oldPath = new Lazy(() => _fs.Path.Combine(_fs.Path.GetDirectoryName(GetNewPath()), "trash-updater")); } + + public bool CheckIfNeeded() => _fs.Directory.Exists(GetOldPath()); + + public void Execute() => _fs.MergeDirectory(GetOldPath(), GetNewPath()); } diff --git a/src/TrashLib/IAppPaths.cs b/src/TrashLib/IAppPaths.cs index ed627b01..12d73ef6 100644 --- a/src/TrashLib/IAppPaths.cs +++ b/src/TrashLib/IAppPaths.cs @@ -11,4 +11,5 @@ public interface IAppPaths string CacheDirectory { get; } string DefaultConfigFilename { get; } bool IsAppDataPathValid { get; } + string DefaultAppDataDirectoryName { get; } }