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

@@ -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
 

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<IEnumerable<IBaseCommandSetupTask>>();
+        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<IAppPaths>();
+        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<JanitorCleanupTask>();
+        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<IAppPaths>();
+        var settingsProvider = Resolve<ISettingsProvider>();
+        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<JanitorCleanupTask>();
+        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<IAppPaths>();
+
+        for (var i = 0; i < 50; ++i)
+        {
+            Fs.AddFile(paths.LogDirectory.File($"logfile-{i}.log").FullName, new MockFileData(""));
+        }
+
+        var sut = Resolve<AppPathSetupTask>();
+        sut.OnStart();
+
+        var expectedDirs = new[]
+        {
+            paths.LogDirectory.FullName,
+            paths.RepoDirectory.FullName,
+            paths.CacheDirectory.FullName
+        };
+
+        expectedDirs.Should().IntersectWith(Fs.AllDirectories);
+    }
+}

src/Recyclarr.Tests/Command/BaseCommandTest.cs

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

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<IAppPaths>();
-        var janitor = container.Resolve<ILogJanitor>();
-        var log = container.Resolve<ILogger>();
+        var tasks = container.Resolve<IOrderedEnumerable<IBaseCommandSetupTask>>().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);

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()
+    {
+    }
+}

src/Recyclarr/Command/Setup/IBaseCommandSetupTask.cs

@@ -0,0 +1,7 @@
+namespace Recyclarr.Command.Setup;
+
+public interface IBaseCommandSetupTask
+{
+    void OnStart();
+    void OnFinish();
+}

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

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<IBaseCommandSetupTask>()
+            .OrderByRegistration();
+
         // Register all types deriving from CliFx's ICommand. These are all of our supported subcommands.
         builder.RegisterAssemblyTypes(Assembly.GetExecutingAssembly())
             .AssignableTo<ICommand>();

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