From fb324e7576a72ca573984a6a21f12eab84bd8148 Mon Sep 17 00:00:00 2001 From: sam Date: Wed, 4 Sep 2024 01:46:39 +0200 Subject: [PATCH 1/2] refactor: replace periodic tasks loop with background service --- .../Extensions/WebApplicationExtensions.cs | 57 ++++++++++++------- Foxnouns.Backend/Program.cs | 37 ++---------- Foxnouns.Backend/Services/KeyCacheService.cs | 4 +- .../Services/PeriodicTasksService.cs | 26 +++++++++ 4 files changed, 67 insertions(+), 57 deletions(-) create mode 100644 Foxnouns.Backend/Services/PeriodicTasksService.cs diff --git a/Foxnouns.Backend/Extensions/WebApplicationExtensions.cs b/Foxnouns.Backend/Extensions/WebApplicationExtensions.cs index a76928c..bf6eda2 100644 --- a/Foxnouns.Backend/Extensions/WebApplicationExtensions.cs +++ b/Foxnouns.Backend/Extensions/WebApplicationExtensions.cs @@ -5,6 +5,7 @@ using Foxnouns.Backend.Jobs; using Foxnouns.Backend.Middleware; using Foxnouns.Backend.Services; using Microsoft.EntityFrameworkCore; +using Minio; using NodaTime; using Prometheus; using Serilog; @@ -57,16 +58,6 @@ public static class WebApplicationExtensions return config; } - public static WebApplicationBuilder AddMetrics(this WebApplicationBuilder builder, Config config) - { - builder.Services.AddMetricServer(o => o.Port = config.Logging.MetricsPort) - .AddSingleton(); - if (!config.Logging.EnableMetrics) - builder.Services.AddHostedService(); - - return builder; - } - public static IConfigurationBuilder AddConfiguration(this IConfigurationBuilder builder) { var file = Environment.GetEnvironmentVariable("FOXNOUNS_CONFIG_FILE") ?? "config.ini"; @@ -78,18 +69,40 @@ public static class WebApplicationExtensions .AddEnvironmentVariables(); } - public static IServiceCollection AddCustomServices(this IServiceCollection services) => services - .AddSingleton(SystemClock.Instance) - .AddSnowflakeGenerator() - .AddScoped() - .AddScoped() - .AddScoped() - .AddScoped() - .AddScoped() - .AddScoped() - // Transient jobs - .AddTransient() - .AddTransient(); + /// + /// Adds required services to the IServiceCollection. + /// This should only add services that are not ASP.NET-related (i.e. no middleware). + /// + public static IServiceCollection AddServices(this IServiceCollection services, Config config) + { + services + .AddQueue() + .AddDbContext() + .AddMetricServer(o => o.Port = config.Logging.MetricsPort) + .AddMinio(c => + c.WithEndpoint(config.Storage.Endpoint) + .WithCredentials(config.Storage.AccessKey, config.Storage.SecretKey) + .Build()) + .AddSingleton() + .AddSingleton(SystemClock.Instance) + .AddSnowflakeGenerator() + .AddScoped() + .AddScoped() + .AddScoped() + .AddScoped() + .AddScoped() + .AddScoped() + // Background services + .AddHostedService() + // Transient jobs + .AddTransient() + .AddTransient(); + + if (!config.Logging.EnableMetrics) + services.AddHostedService(); + + return services; + } public static IServiceCollection AddCustomMiddleware(this IServiceCollection services) => services .AddScoped() diff --git a/Foxnouns.Backend/Program.cs b/Foxnouns.Backend/Program.cs index e849bd1..911c895 100644 --- a/Foxnouns.Backend/Program.cs +++ b/Foxnouns.Backend/Program.cs @@ -1,12 +1,9 @@ -using Coravel; using Foxnouns.Backend; -using Foxnouns.Backend.Database; using Serilog; using Foxnouns.Backend.Extensions; using Foxnouns.Backend.Services; using Foxnouns.Backend.Utils; using Microsoft.AspNetCore.Mvc; -using Minio; using Newtonsoft.Json; using Newtonsoft.Json.Serialization; using Prometheus; @@ -19,9 +16,7 @@ var builder = WebApplication.CreateBuilder(args); var config = builder.AddConfiguration(); -builder - .AddSerilog() - .AddMetrics(config); +builder.AddSerilog(); builder.WebHost .UseSentry(opts => @@ -64,16 +59,10 @@ JsonConvert.DefaultSettings = () => new JsonSerializerSettings }; builder.Services - .AddQueue() - .AddDbContext() - .AddCustomServices() + .AddServices(config) .AddCustomMiddleware() .AddEndpointsApiExplorer() - .AddSwaggerGen() - .AddMinio(c => - c.WithEndpoint(config.Storage.Endpoint) - .WithCredentials(config.Storage.AccessKey, config.Storage.SecretKey) - .Build()); + .AddSwaggerGen(); var app = builder.Build(); @@ -97,23 +86,5 @@ app.Urls.Add(config.Address); Metrics.DefaultRegistry.AddBeforeCollectCallback(async ct => await app.Services.GetRequiredService().CollectMetricsAsync(ct)); -// Fire off the periodic tasks loop in the background -_ = new Timer(_ => -{ - var __ = RunPeriodicTasksAsync(); -}, null, TimeSpan.FromSeconds(30), TimeSpan.FromMinutes(1)); - app.Run(); -Log.CloseAndFlush(); - -return; - -async Task RunPeriodicTasksAsync() -{ - await using var scope = app.Services.CreateAsyncScope(); - var logger = scope.ServiceProvider.GetRequiredService(); - logger.Debug("Running periodic tasks"); - - var keyCacheSvc = scope.ServiceProvider.GetRequiredService(); - await keyCacheSvc.DeleteExpiredKeysAsync(); -} \ No newline at end of file +Log.CloseAndFlush(); \ No newline at end of file diff --git a/Foxnouns.Backend/Services/KeyCacheService.cs b/Foxnouns.Backend/Services/KeyCacheService.cs index 4523d16..108199c 100644 --- a/Foxnouns.Backend/Services/KeyCacheService.cs +++ b/Foxnouns.Backend/Services/KeyCacheService.cs @@ -37,9 +37,9 @@ public class KeyCacheService(DatabaseContext db, IClock clock, ILogger logger) return value.Value; } - public async Task DeleteExpiredKeysAsync() + public async Task DeleteExpiredKeysAsync(CancellationToken ct) { - var count = await db.TemporaryKeys.Where(k => k.Expires < clock.GetCurrentInstant()).ExecuteDeleteAsync(); + var count = await db.TemporaryKeys.Where(k => k.Expires < clock.GetCurrentInstant()).ExecuteDeleteAsync(ct); if (count != 0) logger.Information("Removed {Count} expired keys from the database", count); } diff --git a/Foxnouns.Backend/Services/PeriodicTasksService.cs b/Foxnouns.Backend/Services/PeriodicTasksService.cs new file mode 100644 index 0000000..d043317 --- /dev/null +++ b/Foxnouns.Backend/Services/PeriodicTasksService.cs @@ -0,0 +1,26 @@ +namespace Foxnouns.Backend.Services; + +public class PeriodicTasksService(ILogger logger, IServiceProvider services) : BackgroundService +{ + private readonly ILogger _logger = logger.ForContext(); + + protected override async Task ExecuteAsync(CancellationToken ct) + { + using var timer = new PeriodicTimer(TimeSpan.FromMinutes(1)); + while (await timer.WaitForNextTickAsync(ct)) + { + _logger.Debug("Collecting metrics"); + await RunPeriodicTasksAsync(ct); + } + } + + private async Task RunPeriodicTasksAsync(CancellationToken ct) + { + _logger.Debug("Running periodic tasks"); + + await using var scope = services.CreateAsyncScope(); + + var keyCacheSvc = scope.ServiceProvider.GetRequiredService(); + await keyCacheSvc.DeleteExpiredKeysAsync(ct); + } +} \ No newline at end of file From 6c9d1c328b5b856f9450b3fe405be1af5398d659 Mon Sep 17 00:00:00 2001 From: sam Date: Wed, 4 Sep 2024 14:25:44 +0200 Subject: [PATCH 2/2] fix: add class context to all loggers, format --- .../Authentication/AuthController.cs | 4 ++- .../Authentication/DiscordAuthController.cs | 10 ++++--- .../Authentication/EmailAuthController.cs | 6 ++-- .../Controllers/DebugController.cs | 4 ++- .../Database/DatabaseQueryExtensions.cs | 3 +- Foxnouns.Backend/Database/Models/User.cs | 2 +- .../Extensions/WebApplicationExtensions.cs | 2 +- Foxnouns.Backend/Foxnouns.Backend.csproj | 30 +++++++++---------- Foxnouns.Backend/FoxnounsMetrics.cs | 2 +- Foxnouns.Backend/GlobalUsing.cs | 2 +- .../Jobs/UserAvatarUpdateInvocable.cs | 4 +-- .../Middleware/ErrorHandlerMiddleware.cs | 1 + Foxnouns.Backend/Services/KeyCacheService.cs | 4 ++- .../Services/ObjectStorageService.cs | 8 +++-- .../Services/PeriodicTasksService.cs | 2 +- Foxnouns.Backend/Utils/ValidationUtils.cs | 7 +++-- 16 files changed, 54 insertions(+), 37 deletions(-) diff --git a/Foxnouns.Backend/Controllers/Authentication/AuthController.cs b/Foxnouns.Backend/Controllers/Authentication/AuthController.cs index 6565fba..db2e21f 100644 --- a/Foxnouns.Backend/Controllers/Authentication/AuthController.cs +++ b/Foxnouns.Backend/Controllers/Authentication/AuthController.cs @@ -9,11 +9,13 @@ namespace Foxnouns.Backend.Controllers.Authentication; [Route("/api/v2/auth")] public class AuthController(Config config, KeyCacheService keyCacheSvc, ILogger logger) : ApiControllerBase { + private readonly ILogger _logger = logger.ForContext(); + [HttpPost("urls")] [ProducesResponseType(StatusCodes.Status200OK)] public async Task UrlsAsync() { - logger.Debug("Generating auth URLs for Discord: {Discord}, Google: {Google}, Tumblr: {Tumblr}", + _logger.Debug("Generating auth URLs for Discord: {Discord}, Google: {Google}, Tumblr: {Tumblr}", config.DiscordAuth.Enabled, config.GoogleAuth.Enabled, config.TumblrAuth.Enabled); diff --git a/Foxnouns.Backend/Controllers/Authentication/DiscordAuthController.cs b/Foxnouns.Backend/Controllers/Authentication/DiscordAuthController.cs index 577231c..d717aba 100644 --- a/Foxnouns.Backend/Controllers/Authentication/DiscordAuthController.cs +++ b/Foxnouns.Backend/Controllers/Authentication/DiscordAuthController.cs @@ -20,6 +20,8 @@ public class DiscordAuthController( RemoteAuthService remoteAuthSvc, UserRendererService userRendererSvc) : ApiControllerBase { + private readonly ILogger _logger = logger.ForContext(); + [HttpPost("callback")] // TODO: duplicating attribute doesn't work, find another way to mark both as possible response // leaving it here for documentation purposes @@ -34,7 +36,7 @@ public class DiscordAuthController( var user = await authSvc.AuthenticateUserAsync(AuthType.Discord, remoteUser.Id); if (user != null) return Ok(await GenerateUserTokenAsync(user)); - logger.Debug("Discord user {Username} ({Id}) authenticated with no local account", remoteUser.Username, + _logger.Debug("Discord user {Username} ({Id}) authenticated with no local account", remoteUser.Username, remoteUser.Id); var ticket = AuthUtils.RandomToken(); @@ -51,7 +53,7 @@ public class DiscordAuthController( if (remoteUser == null) throw new ApiError.BadRequest("Invalid ticket", "ticket", req.Ticket); if (await db.AuthMethods.AnyAsync(a => a.AuthType == AuthType.Discord && a.RemoteId == remoteUser.Id)) { - logger.Error("Discord user {Id} has valid ticket but is already linked to an existing account", + _logger.Error("Discord user {Id} has valid ticket but is already linked to an existing account", remoteUser.Id); throw new FoxnounsError("Discord ticket was issued for user with existing link"); } @@ -65,13 +67,13 @@ public class DiscordAuthController( private async Task GenerateUserTokenAsync(User user) { var frontendApp = await db.GetFrontendApplicationAsync(); - logger.Debug("Logging user {Id} in with Discord", user.Id); + _logger.Debug("Logging user {Id} in with Discord", user.Id); var (tokenStr, token) = authSvc.GenerateToken(user, frontendApp, ["*"], clock.GetCurrentInstant() + Duration.FromDays(365)); db.Add(token); - logger.Debug("Generated token {TokenId} for {UserId}", user.Id, token.Id); + _logger.Debug("Generated token {TokenId} for {UserId}", user.Id, token.Id); await db.SaveChangesAsync(); diff --git a/Foxnouns.Backend/Controllers/Authentication/EmailAuthController.cs b/Foxnouns.Backend/Controllers/Authentication/EmailAuthController.cs index e1146c5..19dfc2f 100644 --- a/Foxnouns.Backend/Controllers/Authentication/EmailAuthController.cs +++ b/Foxnouns.Backend/Controllers/Authentication/EmailAuthController.cs @@ -13,6 +13,8 @@ public class EmailAuthController( IClock clock, ILogger logger) : ApiControllerBase { + private readonly ILogger _logger = logger.ForContext(); + [HttpPost("login")] [ProducesResponseType(StatusCodes.Status200OK)] public async Task LoginAsync([FromBody] LoginRequest req) @@ -23,13 +25,13 @@ public class EmailAuthController( var frontendApp = await db.GetFrontendApplicationAsync(); - logger.Debug("Logging user {Id} in with email and password", user.Id); + _logger.Debug("Logging user {Id} in with email and password", user.Id); var (tokenStr, token) = authSvc.GenerateToken(user, frontendApp, ["*"], clock.GetCurrentInstant() + Duration.FromDays(365)); db.Add(token); - logger.Debug("Generated token {TokenId} for {UserId}", user.Id, token.Id); + _logger.Debug("Generated token {TokenId} for {UserId}", token.Id, user.Id); await db.SaveChangesAsync(); diff --git a/Foxnouns.Backend/Controllers/DebugController.cs b/Foxnouns.Backend/Controllers/DebugController.cs index 94a0ff2..a8d3ab2 100644 --- a/Foxnouns.Backend/Controllers/DebugController.cs +++ b/Foxnouns.Backend/Controllers/DebugController.cs @@ -14,11 +14,13 @@ public class DebugController( IClock clock, ILogger logger) : ApiControllerBase { + private readonly ILogger _logger = logger.ForContext(); + [HttpPost("users")] [ProducesResponseType(StatusCodes.Status200OK)] public async Task CreateUserAsync([FromBody] CreateUserRequest req) { - logger.Debug("Creating user with username {Username} and email {Email}", req.Username, req.Email); + _logger.Debug("Creating user with username {Username} and email {Email}", req.Username, req.Email); var user = await authSvc.CreateUserWithPasswordAsync(req.Username, req.Email, req.Password); var frontendApp = await db.GetFrontendApplicationAsync(); diff --git a/Foxnouns.Backend/Database/DatabaseQueryExtensions.cs b/Foxnouns.Backend/Database/DatabaseQueryExtensions.cs index b8f10e9..1d4e851 100644 --- a/Foxnouns.Backend/Database/DatabaseQueryExtensions.cs +++ b/Foxnouns.Backend/Database/DatabaseQueryExtensions.cs @@ -49,7 +49,8 @@ public static class DatabaseQueryExtensions throw new ApiError.NotFound("No member with that ID found.", code: ErrorCode.MemberNotFound); } - public static async Task ResolveMemberAsync(this DatabaseContext context, string userRef, string memberRef, Token? token) + public static async Task ResolveMemberAsync(this DatabaseContext context, string userRef, string memberRef, + Token? token) { var user = await context.ResolveUserAsync(userRef, token); return await context.ResolveMemberAsync(user.Id, memberRef); diff --git a/Foxnouns.Backend/Database/Models/User.cs b/Foxnouns.Backend/Database/Models/User.cs index 305bd46..c152e65 100644 --- a/Foxnouns.Backend/Database/Models/User.cs +++ b/Foxnouns.Backend/Database/Models/User.cs @@ -39,7 +39,7 @@ public class User : BaseModel public required string Tooltip { get; set; } public bool Muted { get; set; } public bool Favourite { get; set; } - + // This type is generally serialized directly, so the converter is applied here. [JsonConverter(typeof(ScreamingSnakeCaseEnumConverter))] public PreferenceSize Size { get; set; } diff --git a/Foxnouns.Backend/Extensions/WebApplicationExtensions.cs b/Foxnouns.Backend/Extensions/WebApplicationExtensions.cs index bf6eda2..1f1ee31 100644 --- a/Foxnouns.Backend/Extensions/WebApplicationExtensions.cs +++ b/Foxnouns.Backend/Extensions/WebApplicationExtensions.cs @@ -97,7 +97,7 @@ public static class WebApplicationExtensions // Transient jobs .AddTransient() .AddTransient(); - + if (!config.Logging.EnableMetrics) services.AddHostedService(); diff --git a/Foxnouns.Backend/Foxnouns.Backend.csproj b/Foxnouns.Backend/Foxnouns.Backend.csproj index 82ccf80..c22fdf4 100644 --- a/Foxnouns.Backend/Foxnouns.Backend.csproj +++ b/Foxnouns.Backend/Foxnouns.Backend.csproj @@ -6,31 +6,31 @@ - + - - - - + + + + runtime; build; native; contentfiles; analyzers; buildtransitive all - + - - - - - + + + + + - - - - + + + + diff --git a/Foxnouns.Backend/FoxnounsMetrics.cs b/Foxnouns.Backend/FoxnounsMetrics.cs index b5cc1ac..648b97a 100644 --- a/Foxnouns.Backend/FoxnounsMetrics.cs +++ b/Foxnouns.Backend/FoxnounsMetrics.cs @@ -15,7 +15,7 @@ public static class FoxnounsMetrics public static readonly Gauge UsersActiveDayCount = Metrics.CreateGauge("foxnouns_user_count_active_day", "Number of users active in the last day"); - + public static readonly Gauge MemberCount = Metrics.CreateGauge("foxnouns_member_count", "Number of total members"); diff --git a/Foxnouns.Backend/GlobalUsing.cs b/Foxnouns.Backend/GlobalUsing.cs index 5275c8d..8c73595 100644 --- a/Foxnouns.Backend/GlobalUsing.cs +++ b/Foxnouns.Backend/GlobalUsing.cs @@ -1,2 +1,2 @@ global using ILogger = Serilog.ILogger; -global using Log = Serilog.Log; +global using Log = Serilog.Log; \ No newline at end of file diff --git a/Foxnouns.Backend/Jobs/UserAvatarUpdateInvocable.cs b/Foxnouns.Backend/Jobs/UserAvatarUpdateInvocable.cs index cbec277..f0b04b2 100644 --- a/Foxnouns.Backend/Jobs/UserAvatarUpdateInvocable.cs +++ b/Foxnouns.Backend/Jobs/UserAvatarUpdateInvocable.cs @@ -21,7 +21,7 @@ public class UserAvatarUpdateInvocable(DatabaseContext db, ObjectStorageService private async Task UpdateUserAvatarAsync(Snowflake id, string newAvatar) { _logger.Debug("Updating avatar for user {MemberId}", id); - + var user = await db.Users.FindAsync(id); if (user == null) { @@ -55,7 +55,7 @@ public class UserAvatarUpdateInvocable(DatabaseContext db, ObjectStorageService private async Task ClearUserAvatarAsync(Snowflake id) { _logger.Debug("Clearing avatar for user {MemberId}", id); - + var user = await db.Users.FindAsync(id); if (user == null) { diff --git a/Foxnouns.Backend/Middleware/ErrorHandlerMiddleware.cs b/Foxnouns.Backend/Middleware/ErrorHandlerMiddleware.cs index 39dfd85..c52c3f0 100644 --- a/Foxnouns.Backend/Middleware/ErrorHandlerMiddleware.cs +++ b/Foxnouns.Backend/Middleware/ErrorHandlerMiddleware.cs @@ -110,6 +110,7 @@ public record HttpApiError [JsonConverter(typeof(ScreamingSnakeCaseEnumConverter))] public required ErrorCode Code { get; init; } + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public string? ErrorId { get; init; } diff --git a/Foxnouns.Backend/Services/KeyCacheService.cs b/Foxnouns.Backend/Services/KeyCacheService.cs index 108199c..bd8a862 100644 --- a/Foxnouns.Backend/Services/KeyCacheService.cs +++ b/Foxnouns.Backend/Services/KeyCacheService.cs @@ -9,6 +9,8 @@ namespace Foxnouns.Backend.Services; public class KeyCacheService(DatabaseContext db, IClock clock, ILogger logger) { + private readonly ILogger _logger = logger.ForContext(); + public Task SetKeyAsync(string key, string value, Duration expireAfter) => SetKeyAsync(key, value, clock.GetCurrentInstant() + expireAfter); @@ -40,7 +42,7 @@ public class KeyCacheService(DatabaseContext db, IClock clock, ILogger logger) public async Task DeleteExpiredKeysAsync(CancellationToken ct) { var count = await db.TemporaryKeys.Where(k => k.Expires < clock.GetCurrentInstant()).ExecuteDeleteAsync(ct); - if (count != 0) logger.Information("Removed {Count} expired keys from the database", count); + if (count != 0) _logger.Information("Removed {Count} expired keys from the database", count); } public Task SetKeyAsync(string key, T obj, Duration expiresAt) where T : class => diff --git a/Foxnouns.Backend/Services/ObjectStorageService.cs b/Foxnouns.Backend/Services/ObjectStorageService.cs index 2180b90..de60074 100644 --- a/Foxnouns.Backend/Services/ObjectStorageService.cs +++ b/Foxnouns.Backend/Services/ObjectStorageService.cs @@ -7,21 +7,25 @@ namespace Foxnouns.Backend.Services; public class ObjectStorageService(ILogger logger, Config config, IMinioClient minio) { private readonly ILogger _logger = logger.ForContext(); - + public async Task RemoveObjectAsync(string path) { - logger.Debug("Deleting object at path {Path}", path); + _logger.Debug("Deleting object at path {Path}", path); try { await minio.RemoveObjectAsync(new RemoveObjectArgs().WithBucket(config.Storage.Bucket).WithObject(path)); } catch (InvalidObjectNameException) { + // ignore non-existent objects } } public async Task PutObjectAsync(string path, Stream data, string contentType) { + _logger.Debug("Putting object at path {Path} with length {Length} and content type {ContentType}", path, + data.Length, contentType); + await minio.PutObjectAsync(new PutObjectArgs() .WithBucket(config.Storage.Bucket) .WithObject(path) diff --git a/Foxnouns.Backend/Services/PeriodicTasksService.cs b/Foxnouns.Backend/Services/PeriodicTasksService.cs index d043317..b799abe 100644 --- a/Foxnouns.Backend/Services/PeriodicTasksService.cs +++ b/Foxnouns.Backend/Services/PeriodicTasksService.cs @@ -17,7 +17,7 @@ public class PeriodicTasksService(ILogger logger, IServiceProvider services) : B private async Task RunPeriodicTasksAsync(CancellationToken ct) { _logger.Debug("Running periodic tasks"); - + await using var scope = services.CreateAsyncScope(); var keyCacheSvc = scope.ServiceProvider.GetRequiredService(); diff --git a/Foxnouns.Backend/Utils/ValidationUtils.cs b/Foxnouns.Backend/Utils/ValidationUtils.cs index 5c3c591..8100f4f 100644 --- a/Foxnouns.Backend/Utils/ValidationUtils.cs +++ b/Foxnouns.Backend/Utils/ValidationUtils.cs @@ -120,7 +120,6 @@ public static class ValidationUtils } - private static readonly string[] DefaultStatusOptions = [ "favourite", @@ -147,11 +146,13 @@ public static class ValidationUtils { case > Limits.FieldNameLimit: errors.Add(($"fields.{index}.name", - ValidationError.LengthError("Field name is too long", 1, Limits.FieldNameLimit, field.Name.Length))); + ValidationError.LengthError("Field name is too long", 1, Limits.FieldNameLimit, + field.Name.Length))); break; case < 1: errors.Add(($"fields.{index}.name", - ValidationError.LengthError("Field name is too short", 1, Limits.FieldNameLimit, field.Name.Length))); + ValidationError.LengthError("Field name is too short", 1, Limits.FieldNameLimit, + field.Name.Length))); break; }