From a0d021f1e26a68c0ac682021f429b0242a5f6a81 Mon Sep 17 00:00:00 2001 From: Josh Date: Sat, 17 Jun 2023 14:48:24 -0500 Subject: [PATCH] Refactor RepeatingTaskHandler (#796) * Refactor RepeatingTaskHandler into an ASP.NET service * Add unit tests for RepeatingTaskService * Make repeating task unit tests work independent of time * Fix weird behavior when task is canceled --- .../Startup/GameServerStartup.cs | 4 + .../Startup/WebsiteStartup.cs | 4 + .../Unit/RepeatingTaskTests.cs | 120 ++++++++++++++++++ .../Administration/RepeatingTaskHandler.cs | 65 ---------- ProjectLighthouse/Database/DatabaseContext.cs | 4 + .../Services/RepeatingTaskService.cs | 81 ++++++++++++ ProjectLighthouse/StartupTasks.cs | 4 - 7 files changed, 213 insertions(+), 69 deletions(-) create mode 100644 ProjectLighthouse.Tests/Unit/RepeatingTaskTests.cs delete mode 100644 ProjectLighthouse/Administration/RepeatingTaskHandler.cs create mode 100644 ProjectLighthouse/Services/RepeatingTaskService.cs diff --git a/ProjectLighthouse.Servers.GameServer/Startup/GameServerStartup.cs b/ProjectLighthouse.Servers.GameServer/Startup/GameServerStartup.cs index 2b73bdfc..6b414b9f 100644 --- a/ProjectLighthouse.Servers.GameServer/Startup/GameServerStartup.cs +++ b/ProjectLighthouse.Servers.GameServer/Startup/GameServerStartup.cs @@ -1,4 +1,5 @@ using System.Net; +using LBPUnion.ProjectLighthouse.Administration.Maintenance; using LBPUnion.ProjectLighthouse.Configuration; using LBPUnion.ProjectLighthouse.Database; using LBPUnion.ProjectLighthouse.Logging; @@ -6,6 +7,7 @@ using LBPUnion.ProjectLighthouse.Mail; using LBPUnion.ProjectLighthouse.Middlewares; using LBPUnion.ProjectLighthouse.Serialization; using LBPUnion.ProjectLighthouse.Servers.GameServer.Middlewares; +using LBPUnion.ProjectLighthouse.Services; using LBPUnion.ProjectLighthouse.Types.Logging; using LBPUnion.ProjectLighthouse.Types.Mail; using Microsoft.AspNetCore.Authorization; @@ -63,6 +65,8 @@ public class GameServerStartup : new NullMailService(); services.AddSingleton(mailService); + services.AddHostedService(provider => new RepeatingTaskService(provider, MaintenanceHelper.RepeatingTasks)); + services.Configure ( options => diff --git a/ProjectLighthouse.Servers.Website/Startup/WebsiteStartup.cs b/ProjectLighthouse.Servers.Website/Startup/WebsiteStartup.cs index 69daff13..42f83e85 100644 --- a/ProjectLighthouse.Servers.Website/Startup/WebsiteStartup.cs +++ b/ProjectLighthouse.Servers.Website/Startup/WebsiteStartup.cs @@ -1,5 +1,6 @@ using System.Globalization; using System.Net; +using LBPUnion.ProjectLighthouse.Administration.Maintenance; using LBPUnion.ProjectLighthouse.Configuration; using LBPUnion.ProjectLighthouse.Configuration.ConfigurationCategories; using LBPUnion.ProjectLighthouse.Database; @@ -8,6 +9,7 @@ using LBPUnion.ProjectLighthouse.Mail; using LBPUnion.ProjectLighthouse.Middlewares; using LBPUnion.ProjectLighthouse.Servers.Website.Captcha; using LBPUnion.ProjectLighthouse.Servers.Website.Middlewares; +using LBPUnion.ProjectLighthouse.Services; using LBPUnion.ProjectLighthouse.Types.Mail; using Microsoft.AspNetCore.HttpOverrides; using Microsoft.AspNetCore.Localization; @@ -59,6 +61,8 @@ public class WebsiteStartup : new NullMailService(); services.AddSingleton(mailService); + services.AddHostedService(provider => new RepeatingTaskService(provider, MaintenanceHelper.RepeatingTasks)); + services.AddHttpClient("CaptchaAPI", client => { diff --git a/ProjectLighthouse.Tests/Unit/RepeatingTaskTests.cs b/ProjectLighthouse.Tests/Unit/RepeatingTaskTests.cs new file mode 100644 index 00000000..678f434c --- /dev/null +++ b/ProjectLighthouse.Tests/Unit/RepeatingTaskTests.cs @@ -0,0 +1,120 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using LBPUnion.ProjectLighthouse.Database; +using LBPUnion.ProjectLighthouse.Services; +using LBPUnion.ProjectLighthouse.Types.Maintenance; +using Microsoft.Extensions.DependencyInjection; +using Moq; +using Xunit; + +namespace LBPUnion.ProjectLighthouse.Tests.Unit; + +[Trait("Category", "Unit")] +public class RepeatingTaskTests +{ + private class TestTask : IRepeatingTask + { + public string Name { get; init; } = ""; + public TimeSpan RepeatInterval => TimeSpan.FromSeconds(5); + public DateTime LastRan { get; set; } + public Task Run(DatabaseContext database) => Task.CompletedTask; + } + + [Fact] + public void GetNextTask_ShouldReturnNull_WhenTaskListEmpty() + { + List tasks = new(); + IServiceProvider provider = new DefaultServiceProviderFactory().CreateServiceProvider(new ServiceCollection()); + RepeatingTaskService service = new(provider, tasks); + + bool gotTask = service.TryGetNextTask(out IRepeatingTask? outTask); + Assert.False(gotTask); + Assert.Null(outTask); + } + + [Fact] + public void GetNextTask_ShouldReturnTask_WhenTaskListContainsOne() + { + List tasks = new() + { + new TestTask(), + }; + IServiceProvider provider = new DefaultServiceProviderFactory().CreateServiceProvider(new ServiceCollection()); + RepeatingTaskService service = new(provider, tasks); + + bool gotTask = service.TryGetNextTask(out IRepeatingTask? outTask); + Assert.True(gotTask); + Assert.NotNull(outTask); + } + + [Fact] + public void GetNextTask_ShouldReturnShortestTask_WhenTaskListContainsMultiple() + { + List tasks = new() + { + new TestTask + { + Name = "Task 1", + LastRan = DateTime.UtcNow, + }, + new TestTask + { + Name = "Task 2", + LastRan = DateTime.UtcNow.AddMinutes(1), + }, + new TestTask + { + Name = "Task 3", + LastRan = DateTime.UtcNow.AddMinutes(-1), + }, + }; + IServiceProvider provider = new DefaultServiceProviderFactory().CreateServiceProvider(new ServiceCollection()); + RepeatingTaskService service = new(provider, tasks); + + bool gotTask = service.TryGetNextTask(out IRepeatingTask? outTask); + Assert.True(gotTask); + Assert.NotNull(outTask); + Assert.Equal("Task 3", outTask.Name); + } + + [Fact] + public async Task BackgroundService_ShouldExecuteTask_AndUpdateTime() + { + Mock taskMock = new(); + taskMock.Setup(t => t.Run(It.IsAny())).Returns(Task.CompletedTask); + taskMock.SetupGet(t => t.RepeatInterval).Returns(TimeSpan.FromSeconds(10)); + TaskCompletionSource taskCompletion = new(); + DateTime lastRan = default; + taskMock.SetupSet(t => t.LastRan = It.IsAny()) + .Callback(time => + { + lastRan = time; + taskCompletion.TrySetResult(); + }); + taskMock.SetupGet(t => t.LastRan).Returns(() => lastRan); + List tasks = new() + { + taskMock.Object, + }; + ServiceCollection serviceCollection = new(); + serviceCollection.AddScoped(_ => new Mock().Object); + IServiceProvider provider = new DefaultServiceProviderFactory().CreateServiceProvider(serviceCollection); + + RepeatingTaskService service = new(provider, tasks); + + CancellationTokenSource stoppingToken = new(); + + await service.StartAsync(stoppingToken.Token); + + await taskCompletion.Task; + stoppingToken.Cancel(); + + Assert.NotNull(service.ExecuteTask); + await service.ExecuteTask; + + taskMock.Verify(x => x.Run(It.IsAny()), Times.Once); + taskMock.VerifySet(x => x.LastRan = It.IsAny(), Times.Once()); + } +} \ No newline at end of file diff --git a/ProjectLighthouse/Administration/RepeatingTaskHandler.cs b/ProjectLighthouse/Administration/RepeatingTaskHandler.cs deleted file mode 100644 index 531cc206..00000000 --- a/ProjectLighthouse/Administration/RepeatingTaskHandler.cs +++ /dev/null @@ -1,65 +0,0 @@ -#nullable enable -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.Threading; -using System.Threading.Tasks; -using LBPUnion.ProjectLighthouse.Administration.Maintenance; -using LBPUnion.ProjectLighthouse.Database; -using LBPUnion.ProjectLighthouse.Logging; -using LBPUnion.ProjectLighthouse.Types.Logging; -using LBPUnion.ProjectLighthouse.Types.Maintenance; - -namespace LBPUnion.ProjectLighthouse.Administration; - -public static class RepeatingTaskHandler -{ - private static bool initialized = false; - - public static void Initialize() - { - if (initialized) throw new InvalidOperationException("RepeatingTaskHandler was initialized twice"); - - initialized = true; - Task.Factory.StartNew(taskLoop); - } - - [SuppressMessage("ReSharper", "FunctionNeverReturns")] - private static async Task taskLoop() - { - Queue taskQueue = new(); - foreach (IRepeatingTask task in MaintenanceHelper.RepeatingTasks) taskQueue.Enqueue(task); - - DatabaseContext database = DatabaseContext.CreateNewInstance(); - - while (true) - { - try - { - if (!taskQueue.TryDequeue(out IRepeatingTask? task)) - { - Thread.Sleep(100); - continue; - } - - Debug.Assert(task != null); - - if ((task.LastRan + task.RepeatInterval) <= DateTime.Now) - { - await task.Run(database); - task.LastRan = DateTime.Now; - - Logger.Debug($"Ran task \"{task.Name}\"", LogArea.Maintenance); - } - - taskQueue.Enqueue(task); - Thread.Sleep(500); // Doesn't need to be that precise. - } - catch(Exception e) - { - Logger.Warn($"Error occured while processing repeating tasks: \n{e}", LogArea.Maintenance); - } - } - } -} \ No newline at end of file diff --git a/ProjectLighthouse/Database/DatabaseContext.cs b/ProjectLighthouse/Database/DatabaseContext.cs index 30f33066..830ee726 100644 --- a/ProjectLighthouse/Database/DatabaseContext.cs +++ b/ProjectLighthouse/Database/DatabaseContext.cs @@ -64,6 +64,10 @@ public partial class DatabaseContext : DbContext #endregion + // Used for mocking DbContext + protected internal DatabaseContext() + { } + public DatabaseContext(DbContextOptions options) : base(options) { } diff --git a/ProjectLighthouse/Services/RepeatingTaskService.cs b/ProjectLighthouse/Services/RepeatingTaskService.cs new file mode 100644 index 00000000..715c7feb --- /dev/null +++ b/ProjectLighthouse/Services/RepeatingTaskService.cs @@ -0,0 +1,81 @@ +#nullable enable +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using LBPUnion.ProjectLighthouse.Database; +using LBPUnion.ProjectLighthouse.Logging; +using LBPUnion.ProjectLighthouse.Types.Logging; +using LBPUnion.ProjectLighthouse.Types.Maintenance; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; + +namespace LBPUnion.ProjectLighthouse.Services; + +public class RepeatingTaskService : BackgroundService +{ + private readonly IServiceProvider provider; + private readonly List taskList; + + public RepeatingTaskService(IServiceProvider provider, List tasks) + { + this.provider = provider; + this.taskList = tasks; + + Logger.Info("Initializing repeating tasks service", LogArea.Startup); + } + + public bool TryGetNextTask(out IRepeatingTask? outTask) + { + TimeSpan smallestSpan = TimeSpan.MaxValue; + IRepeatingTask? smallestTask = null; + foreach (IRepeatingTask task in this.taskList) + { + TimeSpan smallestTimeRemaining = task.RepeatInterval.Subtract(DateTime.UtcNow.Subtract(task.LastRan)); + if (smallestTimeRemaining >= smallestSpan) continue; + + smallestSpan = smallestTimeRemaining; + smallestTask = task; + } + outTask = smallestTask; + + return outTask != null; + } + + protected override async Task ExecuteAsync(CancellationToken stoppingToken) + { + await Task.Yield(); + while (!stoppingToken.IsCancellationRequested) + { + if (!this.TryGetNextTask(out IRepeatingTask? task) || task == null) + { + // If we fail to fetch the next task then something has gone wrong and the service should halt + Logger.Debug("Failed to fetch next smallest task", LogArea.Maintenance); + return; + } + + TimeSpan timeElapsedSinceRun = DateTime.UtcNow.Subtract(task.LastRan); + + // If the task's repeat interval hasn't elapsed + if (timeElapsedSinceRun < task.RepeatInterval) + { + TimeSpan timeToWait = task.RepeatInterval.Subtract(timeElapsedSinceRun); + Logger.Debug($"Waiting {timeToWait} for {task.Name}", LogArea.Maintenance); + try + { + await Task.Delay(timeToWait, stoppingToken); + } + catch (TaskCanceledException) + { + break; + } + } + + using IServiceScope scope = this.provider.CreateScope(); + DatabaseContext database = scope.ServiceProvider.GetRequiredService(); + await task.Run(database); + Logger.Debug($"Successfully ran task \"{task.Name}\"", LogArea.Maintenance); + task.LastRan = DateTime.UtcNow; + } + } +} \ No newline at end of file diff --git a/ProjectLighthouse/StartupTasks.cs b/ProjectLighthouse/StartupTasks.cs index 85cdcd7d..9080806f 100644 --- a/ProjectLighthouse/StartupTasks.cs +++ b/ProjectLighthouse/StartupTasks.cs @@ -5,7 +5,6 @@ using System.Linq; using System.Reflection; using System.Threading; using System.Threading.Tasks; -using LBPUnion.ProjectLighthouse.Administration; using LBPUnion.ProjectLighthouse.Administration.Maintenance; using LBPUnion.ProjectLighthouse.Configuration; using LBPUnion.ProjectLighthouse.Database; @@ -93,9 +92,6 @@ public static class StartupTasks Logger.Info("Initializing Redis...", LogArea.Startup); RedisDatabase.Initialize().Wait(); - Logger.Info("Initializing repeating tasks...", LogArea.Startup); - RepeatingTaskHandler.Initialize(); - // Create admin user if no users exist if (serverType == ServerType.Website && database.Users.CountAsync().Result == 0) {