From f15bb7d79ac4bc0b26226c27784a7f0733566041 Mon Sep 17 00:00:00 2001 From: Jevgenij Puchko Date: Wed, 6 Nov 2024 16:00:48 +0200 Subject: [PATCH] Code review fixes --- README.md | 34 +++++++------------ .../Suggestions/Jobs/SuggestionsCleanupJob.cs | 4 ++- .../ApplicationBuilderExtensions.cs | 20 +++++++---- .../ServiceCollectionExtensions.cs | 2 +- .../Suggestions/ISuggestionsCleanupService.cs | 2 +- .../Suggestions/SuggestionsCleanupOptions.cs | 1 - .../Suggestions/SuggestionsCleanupService.cs | 6 ++-- .../Configuration/NotFoundHandlerOptions.cs | 3 +- .../ApplicationBuilderExtensions.cs | 2 +- 9 files changed, 36 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index c659d28..0fd8724 100644 --- a/README.md +++ b/README.md @@ -280,38 +280,30 @@ There are two scheduled jobs: # Scheduled jobs -Scheduled jobs powered by Coravel are included in the package, allowing jobs to run at set intervals. - -**Important** -Use this only if the package is not intended for an Optimizely site. Optimizely has built-in scheduled jobs mechanism. - -To enable scheduled jobs, you need to configure the following settings: +Scheduled job - process that runs in background +- Suggestions cleanup job - shipped with the package, contains process that cleans up suggestions table. +This job is configured by default to remove records older than 14 days. You can adjust the retention period or timeout as needed. ``` services.AddNotFoundHandler(o => { - ... - o.ScheduledJobs = true; + o.SuggestionsCleanupOptions.DaysToKeep = 30; + o.SuggestionsCleanupOptions.Timeout = 30 * 60; }); ``` -## Suggestions cleanup job -Practice shows that the suggestions table grows quickly in production, so a suggestions cleanup job was added to control its growth. - -This job is configured by default to run daily at midnight, removing records older than 14 days. -You can adjust the retention period as needed. - +Scheduler - mechanism that triggers scheduled jobs in a recurrent manner +- InternalScheduler - default scheduler, included in the core package, a scheduler that uses [Coravel](https://docs.coravel.net/). + To enable the scheduler, you need to enable UseInternalScheduler flag. Additionally, you can adjust the scheduler run interval: ``` services.AddNotFoundHandler(o => { - o.ScheduledJobs = true; - o.SuggestionsCleanupOptions.CronInterval = "0 0 * * 0" // weekly on Sunday midnight - o.SuggestionsCleanupOptions.DaysToKeep = 30; + ... + o.UseInternalScheduler = true; + o.InternalSchedulerCronInterval = "0 0 * * *" // by default it's configured to run daily at midnight }); ``` -**Note** -For Optimizely was added job that is powered by built-in scheduled jobs mechanism. - -[Geta NotFoundHandler] Suggestions cleanup job +- OptimizelyScheduler - uses Optimizely to schedule job runs. +An Optimizely scheduled job was added - [Geta NotFoundHandler] Suggestions cleanup job. # Troubleshooting diff --git a/src/Geta.NotFoundHandler.Optimizely/Core/Suggestions/Jobs/SuggestionsCleanupJob.cs b/src/Geta.NotFoundHandler.Optimizely/Core/Suggestions/Jobs/SuggestionsCleanupJob.cs index 1fbda1e..e40de31 100644 --- a/src/Geta.NotFoundHandler.Optimizely/Core/Suggestions/Jobs/SuggestionsCleanupJob.cs +++ b/src/Geta.NotFoundHandler.Optimizely/Core/Suggestions/Jobs/SuggestionsCleanupJob.cs @@ -23,6 +23,8 @@ public SuggestionsCleanupJob(ISuggestionsCleanupService suggestionsCleanupServic public override string Execute() { - return _suggestionsCleanupService.Cleanup() ? "": "Unable to cleanup suggestions; please refer to the logs."; + _suggestionsCleanupService.Cleanup(); + + return string.Empty; } } diff --git a/src/Geta.NotFoundHandler/Core/ScheduledJobs/ApplicationBuilderExtensions.cs b/src/Geta.NotFoundHandler/Core/ScheduledJobs/ApplicationBuilderExtensions.cs index c86acf8..2bfae1e 100644 --- a/src/Geta.NotFoundHandler/Core/ScheduledJobs/ApplicationBuilderExtensions.cs +++ b/src/Geta.NotFoundHandler/Core/ScheduledJobs/ApplicationBuilderExtensions.cs @@ -6,6 +6,7 @@ using Geta.NotFoundHandler.Infrastructure.Configuration; using Microsoft.AspNetCore.Builder; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace Geta.NotFoundHandler.Core.ScheduledJobs; @@ -17,14 +18,19 @@ public static IApplicationBuilder UseScheduler(this IApplicationBuilder app) var services = app.ApplicationServices; var options = services.GetRequiredService>().Value; - + var logger = services.GetRequiredService(); + services.UseScheduler(scheduler => - { - scheduler - .Schedule() - .Cron(options.SuggestionsCleanupOptions.CronInterval) - .PreventOverlapping(nameof(SuggestionsCleanupJob)); - }); + { + scheduler + .Schedule() + .Cron(options.InternalSchedulerCronInterval) + .PreventOverlapping(nameof(SuggestionsCleanupJob)); + }) + .OnError(x => + { + logger.LogError(x, "Something went wrong, scheduled job fails with exception"); + }); return app; } diff --git a/src/Geta.NotFoundHandler/Core/ScheduledJobs/ServiceCollectionExtensions.cs b/src/Geta.NotFoundHandler/Core/ScheduledJobs/ServiceCollectionExtensions.cs index 9429501..a698cf7 100644 --- a/src/Geta.NotFoundHandler/Core/ScheduledJobs/ServiceCollectionExtensions.cs +++ b/src/Geta.NotFoundHandler/Core/ScheduledJobs/ServiceCollectionExtensions.cs @@ -16,7 +16,7 @@ public static IServiceCollection EnableScheduler(this IServiceCollection service using var serviceProvider = services.BuildServiceProvider(); var options = serviceProvider.GetRequiredService>().Value; - if (options.UseScheduler) + if (options.UseInternalScheduler) { services.AddScheduler(); diff --git a/src/Geta.NotFoundHandler/Core/Suggestions/ISuggestionsCleanupService.cs b/src/Geta.NotFoundHandler/Core/Suggestions/ISuggestionsCleanupService.cs index 79cd13a..8365c9e 100644 --- a/src/Geta.NotFoundHandler/Core/Suggestions/ISuggestionsCleanupService.cs +++ b/src/Geta.NotFoundHandler/Core/Suggestions/ISuggestionsCleanupService.cs @@ -5,5 +5,5 @@ namespace Geta.NotFoundHandler.Core.Suggestions; public interface ISuggestionsCleanupService { - bool Cleanup(); + void Cleanup(); } diff --git a/src/Geta.NotFoundHandler/Core/Suggestions/SuggestionsCleanupOptions.cs b/src/Geta.NotFoundHandler/Core/Suggestions/SuggestionsCleanupOptions.cs index 5c5c1c2..13dc587 100644 --- a/src/Geta.NotFoundHandler/Core/Suggestions/SuggestionsCleanupOptions.cs +++ b/src/Geta.NotFoundHandler/Core/Suggestions/SuggestionsCleanupOptions.cs @@ -7,5 +7,4 @@ public class SuggestionsCleanupOptions { public int DaysToKeep { get; set; } = 14; public int Timeout { get; set; } = 30 * 60; - public string CronInterval { get; set; } = "0 0 * * *"; } diff --git a/src/Geta.NotFoundHandler/Core/Suggestions/SuggestionsCleanupService.cs b/src/Geta.NotFoundHandler/Core/Suggestions/SuggestionsCleanupService.cs index 4476396..ffb2844 100644 --- a/src/Geta.NotFoundHandler/Core/Suggestions/SuggestionsCleanupService.cs +++ b/src/Geta.NotFoundHandler/Core/Suggestions/SuggestionsCleanupService.cs @@ -38,7 +38,7 @@ IF OBJECT_ID('[NotFoundHandler.Suggestions]', 'U') IS NOT NULL "; - public bool Cleanup() + public void Cleanup() { try { @@ -49,14 +49,12 @@ public bool Cleanup() command.CommandTimeout = _options.Value.SuggestionsCleanupOptions.Timeout; command.Connection.Open(); command.ExecuteNonQuery(); - - return true; } catch (Exception ex) { _logger.LogError(ex, "There was a problem while performing cleanup on connection"); - return false; + throw; } } } diff --git a/src/Geta.NotFoundHandler/Infrastructure/Configuration/NotFoundHandlerOptions.cs b/src/Geta.NotFoundHandler/Infrastructure/Configuration/NotFoundHandlerOptions.cs index 4705ad7..c8b1a80 100644 --- a/src/Geta.NotFoundHandler/Infrastructure/Configuration/NotFoundHandlerOptions.cs +++ b/src/Geta.NotFoundHandler/Infrastructure/Configuration/NotFoundHandlerOptions.cs @@ -17,7 +17,8 @@ public class NotFoundHandlerOptions public int BufferSize { get; set; } = 30; public int ThreshHold { get; set; } = 5; public SuggestionsCleanupOptions SuggestionsCleanupOptions { get; set; } = new(); - public bool UseScheduler { get; set; } + public bool UseInternalScheduler { get; set; } + public string InternalSchedulerCronInterval { get; set; } = "0 0 * * *"; public FileNotFoundMode HandlerMode { get; set; } = FileNotFoundMode.On; public TimeSpan RegexTimeout { get; set; } = TimeSpan.FromMilliseconds(100); diff --git a/src/Geta.NotFoundHandler/Infrastructure/Initialization/ApplicationBuilderExtensions.cs b/src/Geta.NotFoundHandler/Infrastructure/Initialization/ApplicationBuilderExtensions.cs index b52afac..07dc72b 100644 --- a/src/Geta.NotFoundHandler/Infrastructure/Initialization/ApplicationBuilderExtensions.cs +++ b/src/Geta.NotFoundHandler/Infrastructure/Initialization/ApplicationBuilderExtensions.cs @@ -26,7 +26,7 @@ public static IApplicationBuilder UseNotFoundHandler(this IApplicationBuilder ap var options = services.GetRequiredService>().Value; - if (options.UseScheduler) + if (options.UseInternalScheduler) { app.UseScheduler(); }