From c5714f2bed466a83fcfae4d2668bb74c84d1a09d Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Thu, 1 Sep 2022 09:56:49 -0500 Subject: [PATCH] feat: Add setting to specify max log files to keep A new `log_janitor` setting added to `settings.yml` to allow the user to specify the maximum number of files to keep when doing log cleanup. Additionally, the way startup tasks occur has been cleaned up / refactored to make it easier to integration test. --- CHANGELOG.md | 6 ++ .../BaseCommandSetupIntegrationTest.cs | 92 +++++++++++++++++++ .../Command/BaseCommandTest.cs | 51 ---------- src/Recyclarr/Command/BaseCommand.cs | 28 +++--- .../Command/Setup/AppPathSetupTask.cs | 30 ++++++ .../Command/Setup/IBaseCommandSetupTask.cs | 7 ++ .../Command/Setup/JanitorCleanupTask.cs | 30 ++++++ src/Recyclarr/CompositionRoot.cs | 7 ++ .../Config/Settings/SettingsValues.cs | 6 ++ 9 files changed, 190 insertions(+), 67 deletions(-) create mode 100644 src/Recyclarr.Tests/BaseCommandSetupIntegrationTest.cs delete mode 100644 src/Recyclarr.Tests/Command/BaseCommandTest.cs create mode 100644 src/Recyclarr/Command/Setup/AppPathSetupTask.cs create mode 100644 src/Recyclarr/Command/Setup/IBaseCommandSetupTask.cs create mode 100644 src/Recyclarr/Command/Setup/JanitorCleanupTask.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 992d3841..3b80ba9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Settings: New `log_janitor` setting that allows you to specify how many log files are kept when + cleaning up (deleting) old log files. See the [Settings Reference] wiki page for more details. + ### Fixed - Docker: Fix `/config` permissions when not using bind-mount for the volume. (#111) @@ -23,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [CVE-2019-0980]: https://avd.aquasec.com/nvd/cve-2019-0981 [CVE-2019-0820]: https://avd.aquasec.com/nvd/cve-2019-0820 [CVE-2019-0657]: https://avd.aquasec.com/nvd/cve-2019-0657 +[Settings Reference]: https://github.com/recyclarr/recyclarr/wiki/Settings-Reference ## [2.4.1] - 2022-08-26 diff --git a/src/Recyclarr.Tests/BaseCommandSetupIntegrationTest.cs b/src/Recyclarr.Tests/BaseCommandSetupIntegrationTest.cs new file mode 100644 index 00000000..35f4967b --- /dev/null +++ b/src/Recyclarr.Tests/BaseCommandSetupIntegrationTest.cs @@ -0,0 +1,92 @@ +using System.IO.Abstractions; +using System.IO.Abstractions.TestingHelpers; +using FluentAssertions; +using NUnit.Framework; +using Recyclarr.Command.Setup; +using Recyclarr.TestLibrary; +using TrashLib.Config.Settings; +using TrashLib.Startup; + +namespace Recyclarr.Tests; + +[TestFixture] +[Parallelizable(ParallelScope.All)] +public class BaseCommandSetupIntegrationTest : IntegrationFixture +{ + [Test] + public void Base_command_startup_tasks_are_registered() + { + var registrations = Resolve>(); + registrations.Select(x => x.GetType()).Should().BeEquivalentTo(new[] + { + typeof(JanitorCleanupTask), + typeof(AppPathSetupTask) + }); + } + + [Test] + public void Log_janitor_cleans_up_user_specified_max_files() + { + var paths = Resolve(); + const int maxFiles = 25; + + Fs.AddFile(paths.SettingsPath.FullName, new MockFileData($@" +log_janitor: + max_files: {maxFiles} +")); + + for (var i = 0; i < maxFiles+20; ++i) + { + Fs.AddFile(paths.LogDirectory.File($"logfile-{i}.log").FullName, new MockFileData("")); + } + + var sut = Resolve(); + sut.OnFinish(); + + Fs.AllFiles.Where(x => x.StartsWith(paths.LogDirectory.FullName)) + .Should().HaveCount(maxFiles); + } + + [Test] + public void Log_janitor_cleans_up_default_max_files() + { + var paths = Resolve(); + var settingsProvider = Resolve(); + var maxFiles = settingsProvider.Settings.LogJanitor.MaxFiles; + + for (var i = 0; i < maxFiles+20; ++i) + { + Fs.AddFile(paths.LogDirectory.File($"logfile-{i}.log").FullName, new MockFileData("")); + } + + var sut = Resolve(); + sut.OnFinish(); + + maxFiles.Should().BeGreaterThan(0); + Fs.AllFiles.Where(x => x.StartsWith(paths.LogDirectory.FullName)) + .Should().HaveCount(maxFiles); + } + + [Test] + public void App_paths_setup_creates_initial_directories() + { + var paths = Resolve(); + + for (var i = 0; i < 50; ++i) + { + Fs.AddFile(paths.LogDirectory.File($"logfile-{i}.log").FullName, new MockFileData("")); + } + + var sut = Resolve(); + sut.OnStart(); + + var expectedDirs = new[] + { + paths.LogDirectory.FullName, + paths.RepoDirectory.FullName, + paths.CacheDirectory.FullName + }; + + expectedDirs.Should().IntersectWith(Fs.AllDirectories); + } +} diff --git a/src/Recyclarr.Tests/Command/BaseCommandTest.cs b/src/Recyclarr.Tests/Command/BaseCommandTest.cs deleted file mode 100644 index ba389af6..00000000 --- a/src/Recyclarr.Tests/Command/BaseCommandTest.cs +++ /dev/null @@ -1,51 +0,0 @@ -using System.IO.Abstractions.TestingHelpers; -using AutoFixture.NUnit3; -using CliFx.Attributes; -using CliFx.Infrastructure; -using FluentAssertions; -using NUnit.Framework; -using Recyclarr.Command; -using TestLibrary.AutoFixture; -using TrashLib.TestLibrary; - -namespace Recyclarr.Tests.Command; - -[Command] -public class StubBaseCommand : BaseCommand -{ - public override string? AppDataDirectory { get; set; } - - public StubBaseCommand(ICompositionRoot compositionRoot) - { - CompositionRoot = compositionRoot; - } - - public override Task Process(IServiceLocatorProxy container) - { - return Task.CompletedTask; - } -} - -[TestFixture] -// Cannot be parallelized due to static CompositionRoot property -public class BaseCommandTest -{ - [Test, AutoMockData] - public async Task All_directories_are_created( - [Frozen(Matching.ImplementedInterfaces)] MockFileSystem fs, - [Frozen(Matching.ImplementedInterfaces)] TestAppPaths paths, - IConsole console, - StubBaseCommand sut) - { - await sut.ExecuteAsync(console); - - var expectedDirs = new[] - { - paths.LogDirectory.FullName, - paths.RepoDirectory.FullName, - paths.CacheDirectory.FullName - }; - - expectedDirs.Should().IntersectWith(fs.AllDirectories); - } -} diff --git a/src/Recyclarr/Command/BaseCommand.cs b/src/Recyclarr/Command/BaseCommand.cs index ac7dcc36..1df0cc84 100644 --- a/src/Recyclarr/Command/BaseCommand.cs +++ b/src/Recyclarr/Command/BaseCommand.cs @@ -3,10 +3,9 @@ using CliFx.Attributes; using CliFx.Exceptions; using CliFx.Infrastructure; using JetBrains.Annotations; -using Recyclarr.Logging; -using Serilog; +using MoreLinq.Extensions; +using Recyclarr.Command.Setup; using Serilog.Events; -using TrashLib.Startup; namespace Recyclarr.Command; @@ -34,20 +33,17 @@ public abstract class BaseCommand : ICommand using var container = CompositionRoot.Setup(AppDataDirectory, console, logLevel); - var paths = container.Resolve(); - var janitor = container.Resolve(); - var log = container.Resolve(); + var tasks = container.Resolve>().ToArray(); + tasks.ForEach(x => x.OnStart()); - log.Debug("App Data Dir: {AppData}", paths.AppDataDirectory); - - // Initialize other directories used throughout the application - paths.RepoDirectory.Create(); - paths.CacheDirectory.Create(); - paths.LogDirectory.Create(); - - await Process(container); - - janitor.DeleteOldestLogFiles(20); + try + { + await Process(container); + } + finally + { + tasks.Reverse().ForEach(x => x.OnFinish()); + } } public abstract Task Process(IServiceLocatorProxy container); diff --git a/src/Recyclarr/Command/Setup/AppPathSetupTask.cs b/src/Recyclarr/Command/Setup/AppPathSetupTask.cs new file mode 100644 index 00000000..433a894c --- /dev/null +++ b/src/Recyclarr/Command/Setup/AppPathSetupTask.cs @@ -0,0 +1,30 @@ +using Serilog; +using TrashLib.Startup; + +namespace Recyclarr.Command.Setup; + +public class AppPathSetupTask : IBaseCommandSetupTask +{ + private readonly ILogger _log; + private readonly IAppPaths _paths; + + public AppPathSetupTask(ILogger log, IAppPaths paths) + { + _log = log; + _paths = paths; + } + + public void OnStart() + { + _log.Debug("App Data Dir: {AppData}", _paths.AppDataDirectory); + + // Initialize other directories used throughout the application + _paths.RepoDirectory.Create(); + _paths.CacheDirectory.Create(); + _paths.LogDirectory.Create(); + } + + public void OnFinish() + { + } +} diff --git a/src/Recyclarr/Command/Setup/IBaseCommandSetupTask.cs b/src/Recyclarr/Command/Setup/IBaseCommandSetupTask.cs new file mode 100644 index 00000000..8738b4b0 --- /dev/null +++ b/src/Recyclarr/Command/Setup/IBaseCommandSetupTask.cs @@ -0,0 +1,7 @@ +namespace Recyclarr.Command.Setup; + +public interface IBaseCommandSetupTask +{ + void OnStart(); + void OnFinish(); +} diff --git a/src/Recyclarr/Command/Setup/JanitorCleanupTask.cs b/src/Recyclarr/Command/Setup/JanitorCleanupTask.cs new file mode 100644 index 00000000..4a413bd8 --- /dev/null +++ b/src/Recyclarr/Command/Setup/JanitorCleanupTask.cs @@ -0,0 +1,30 @@ +using Recyclarr.Logging; +using Serilog; +using TrashLib.Config.Settings; + +namespace Recyclarr.Command.Setup; + +public class JanitorCleanupTask : IBaseCommandSetupTask +{ + private readonly ILogJanitor _janitor; + private readonly ILogger _log; + private readonly ISettingsProvider _settingsProvider; + + public JanitorCleanupTask(ILogJanitor janitor, ILogger log, ISettingsProvider settingsProvider) + { + _janitor = janitor; + _log = log; + _settingsProvider = settingsProvider; + } + + public void OnStart() + { + } + + public void OnFinish() + { + var maxFiles = _settingsProvider.Settings.LogJanitor.MaxFiles; + _log.Debug("Cleaning up logs using max files of {MaxFiles}", maxFiles); + _janitor.DeleteOldestLogFiles(maxFiles); + } +} diff --git a/src/Recyclarr/CompositionRoot.cs b/src/Recyclarr/CompositionRoot.cs index 7eebd7a9..3c694bf0 100644 --- a/src/Recyclarr/CompositionRoot.cs +++ b/src/Recyclarr/CompositionRoot.cs @@ -7,6 +7,7 @@ using CliFx; using CliFx.Infrastructure; using Common; using Recyclarr.Command.Helpers; +using Recyclarr.Command.Setup; using Recyclarr.Config; using Recyclarr.Logging; using Recyclarr.Migration; @@ -94,6 +95,12 @@ public class CompositionRoot : ICompositionRoot private static void CommandRegistrations(ContainerBuilder builder) { + builder.RegisterTypes( + typeof(AppPathSetupTask), + typeof(JanitorCleanupTask)) + .As() + .OrderByRegistration(); + // Register all types deriving from CliFx's ICommand. These are all of our supported subcommands. builder.RegisterAssemblyTypes(Assembly.GetExecutingAssembly()) .AssignableTo(); diff --git a/src/TrashLib/Config/Settings/SettingsValues.cs b/src/TrashLib/Config/Settings/SettingsValues.cs index 5409e44e..f2c2b50f 100644 --- a/src/TrashLib/Config/Settings/SettingsValues.cs +++ b/src/TrashLib/Config/Settings/SettingsValues.cs @@ -7,8 +7,14 @@ public record TrashRepository public string? Sha1 { get; init; } } +public record LogJanitorSettings +{ + public int MaxFiles { get; init; } = 20; +} + public record SettingsValues { public TrashRepository Repository { get; init; } = new(); public bool EnableSslCertificateValidation { get; init; } = true; + public LogJanitorSettings LogJanitor { get; init; } = new(); }