Skip to content

Commit f15bb7d

Browse files
committed
Code review fixes
1 parent aabb6cb commit f15bb7d

File tree

9 files changed

+36
-38
lines changed

9 files changed

+36
-38
lines changed

README.md

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -280,38 +280,30 @@ There are two scheduled jobs:
280280

281281
# Scheduled jobs
282282

283-
Scheduled jobs powered by Coravel are included in the package, allowing jobs to run at set intervals.
284-
285-
**Important**
286-
Use this only if the package is not intended for an Optimizely site. Optimizely has built-in scheduled jobs mechanism.
287-
288-
To enable scheduled jobs, you need to configure the following settings:
283+
Scheduled job - process that runs in background
284+
- Suggestions cleanup job - shipped with the package, contains process that cleans up suggestions table.
285+
This job is configured by default to remove records older than 14 days. You can adjust the retention period or timeout as needed.
289286
```
290287
services.AddNotFoundHandler(o =>
291288
{
292-
...
293-
o.ScheduledJobs = true;
289+
o.SuggestionsCleanupOptions.DaysToKeep = 30;
290+
o.SuggestionsCleanupOptions.Timeout = 30 * 60;
294291
});
295292
```
296293

297-
## Suggestions cleanup job
298-
Practice shows that the suggestions table grows quickly in production, so a suggestions cleanup job was added to control its growth.
299-
300-
This job is configured by default to run daily at midnight, removing records older than 14 days.
301-
You can adjust the retention period as needed.
302-
294+
Scheduler - mechanism that triggers scheduled jobs in a recurrent manner
295+
- InternalScheduler - default scheduler, included in the core package, a scheduler that uses [Coravel](https://docs.coravel.net/).
296+
To enable the scheduler, you need to enable UseInternalScheduler flag. Additionally, you can adjust the scheduler run interval:
303297
```
304298
services.AddNotFoundHandler(o =>
305299
{
306-
o.ScheduledJobs = true;
307-
o.SuggestionsCleanupOptions.CronInterval = "0 0 * * 0" // weekly on Sunday midnight
308-
o.SuggestionsCleanupOptions.DaysToKeep = 30;
300+
...
301+
o.UseInternalScheduler = true;
302+
o.InternalSchedulerCronInterval = "0 0 * * *" // by default it's configured to run daily at midnight
309303
});
310304
```
311-
**Note**
312-
For Optimizely was added job that is powered by built-in scheduled jobs mechanism.
313-
314-
[Geta NotFoundHandler] Suggestions cleanup job
305+
- OptimizelyScheduler - uses Optimizely to schedule job runs.
306+
An Optimizely scheduled job was added - <code>[Geta NotFoundHandler] Suggestions cleanup job</code>.
315307

316308
# Troubleshooting
317309

src/Geta.NotFoundHandler.Optimizely/Core/Suggestions/Jobs/SuggestionsCleanupJob.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ public SuggestionsCleanupJob(ISuggestionsCleanupService suggestionsCleanupServic
2323

2424
public override string Execute()
2525
{
26-
return _suggestionsCleanupService.Cleanup() ? "": "Unable to cleanup suggestions; please refer to the logs.";
26+
_suggestionsCleanupService.Cleanup();
27+
28+
return string.Empty;
2729
}
2830
}

src/Geta.NotFoundHandler/Core/ScheduledJobs/ApplicationBuilderExtensions.cs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Geta.NotFoundHandler.Infrastructure.Configuration;
77
using Microsoft.AspNetCore.Builder;
88
using Microsoft.Extensions.DependencyInjection;
9+
using Microsoft.Extensions.Logging;
910
using Microsoft.Extensions.Options;
1011

1112
namespace Geta.NotFoundHandler.Core.ScheduledJobs;
@@ -17,14 +18,19 @@ public static IApplicationBuilder UseScheduler(this IApplicationBuilder app)
1718
var services = app.ApplicationServices;
1819

1920
var options = services.GetRequiredService<IOptions<NotFoundHandlerOptions>>().Value;
20-
21+
var logger = services.GetRequiredService<ILogger>();
22+
2123
services.UseScheduler(scheduler =>
22-
{
23-
scheduler
24-
.Schedule<SuggestionsCleanupJob>()
25-
.Cron(options.SuggestionsCleanupOptions.CronInterval)
26-
.PreventOverlapping(nameof(SuggestionsCleanupJob));
27-
});
24+
{
25+
scheduler
26+
.Schedule<SuggestionsCleanupJob>()
27+
.Cron(options.InternalSchedulerCronInterval)
28+
.PreventOverlapping(nameof(SuggestionsCleanupJob));
29+
})
30+
.OnError(x =>
31+
{
32+
logger.LogError(x, "Something went wrong, scheduled job fails with exception");
33+
});
2834

2935
return app;
3036
}

src/Geta.NotFoundHandler/Core/ScheduledJobs/ServiceCollectionExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public static IServiceCollection EnableScheduler(this IServiceCollection service
1616
using var serviceProvider = services.BuildServiceProvider();
1717
var options = serviceProvider.GetRequiredService<IOptions<NotFoundHandlerOptions>>().Value;
1818

19-
if (options.UseScheduler)
19+
if (options.UseInternalScheduler)
2020
{
2121
services.AddScheduler();
2222

src/Geta.NotFoundHandler/Core/Suggestions/ISuggestionsCleanupService.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ namespace Geta.NotFoundHandler.Core.Suggestions;
55

66
public interface ISuggestionsCleanupService
77
{
8-
bool Cleanup();
8+
void Cleanup();
99
}

src/Geta.NotFoundHandler/Core/Suggestions/SuggestionsCleanupOptions.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,4 @@ public class SuggestionsCleanupOptions
77
{
88
public int DaysToKeep { get; set; } = 14;
99
public int Timeout { get; set; } = 30 * 60;
10-
public string CronInterval { get; set; } = "0 0 * * *";
1110
}

src/Geta.NotFoundHandler/Core/Suggestions/SuggestionsCleanupService.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ IF OBJECT_ID('[NotFoundHandler.Suggestions]', 'U') IS NOT NULL
3838
";
3939

4040

41-
public bool Cleanup()
41+
public void Cleanup()
4242
{
4343
try
4444
{
@@ -49,14 +49,12 @@ public bool Cleanup()
4949
command.CommandTimeout = _options.Value.SuggestionsCleanupOptions.Timeout;
5050
command.Connection.Open();
5151
command.ExecuteNonQuery();
52-
53-
return true;
5452
}
5553
catch (Exception ex)
5654
{
5755
_logger.LogError(ex, "There was a problem while performing cleanup on connection");
5856

59-
return false;
57+
throw;
6058
}
6159
}
6260
}

src/Geta.NotFoundHandler/Infrastructure/Configuration/NotFoundHandlerOptions.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ public class NotFoundHandlerOptions
1717
public int BufferSize { get; set; } = 30;
1818
public int ThreshHold { get; set; } = 5;
1919
public SuggestionsCleanupOptions SuggestionsCleanupOptions { get; set; } = new();
20-
public bool UseScheduler { get; set; }
20+
public bool UseInternalScheduler { get; set; }
21+
public string InternalSchedulerCronInterval { get; set; } = "0 0 * * *";
2122
public FileNotFoundMode HandlerMode { get; set; } = FileNotFoundMode.On;
2223
public TimeSpan RegexTimeout { get; set; } = TimeSpan.FromMilliseconds(100);
2324

src/Geta.NotFoundHandler/Infrastructure/Initialization/ApplicationBuilderExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public static IApplicationBuilder UseNotFoundHandler(this IApplicationBuilder ap
2626

2727
var options = services.GetRequiredService<IOptions<NotFoundHandlerOptions>>().Value;
2828

29-
if (options.UseScheduler)
29+
if (options.UseInternalScheduler)
3030
{
3131
app.UseScheduler();
3232
}

0 commit comments

Comments
 (0)