diff --git a/CHANGELOG.md b/CHANGELOG.md index ed9d5dc..bae6470 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ All notable changes to **NCronJob** will be documented in this file. The project ## [Unreleased] +### Changed + +- Identical Job Definitions will not lead to multiple instances of the job running concurrently. By [@linkdotnet](https://github.com/linkdotnet). + ## [2.8.2] - 2024-06-18 ### Changed diff --git a/Directory.Build.props b/Directory.Build.props index a8f2dc6..b7592d3 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -1,7 +1,7 @@ - + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/docs/features/define-and-schedule-jobs.md b/docs/features/define-and-schedule-jobs.md index c6ee150..e409d10 100644 --- a/docs/features/define-and-schedule-jobs.md +++ b/docs/features/define-and-schedule-jobs.md @@ -60,6 +60,24 @@ Services.AddNCronJob(options => }); ``` +!!! info + + Defining multiple identifical schedules for the same job will not lead to multiple instances of the job running concurrently. NCronJob will ensure that only one instance of the job is running at any given time. One can define different custom names for each schedule to differentiate between them. + +The following example illustrates how to define multiple schedules that are identical and will only lead to one instance of the job running at any given time: + +```csharp +Services.AddNCronJob(options => +{ + options.AddJob(j => + { + j.WithCronExpression("0 20 * * *") + .And + .WithCronExpression("0 20 * * *"); + }); +}); +``` + ## Scheduling Jobs With Time Zones The library offers you the ability to schedule jobs using time zones. diff --git a/mkdocs.yml b/mkdocs.yml index 182e10a..c55f1e6 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -13,6 +13,7 @@ theme: features: - navigation.instant - content.action.edit + - content.code.copy markdown_extensions: - smarty - sane_lists diff --git a/src/NCronJob/Configuration/ConcurrencySettings.cs b/src/NCronJob/Configuration/ConcurrencySettings.cs index 65dc23c..96682c0 100644 --- a/src/NCronJob/Configuration/ConcurrencySettings.cs +++ b/src/NCronJob/Configuration/ConcurrencySettings.cs @@ -23,6 +23,6 @@ internal class ConcurrencySettings /// currently uses the ThreadPool.UnsafeQueueUserWorkItem method to execute tasks, which may have security implications, /// and is not currently recommended for production use. /// - public static bool UseDeterministicTaskScheduler => true; + public static bool UseDeterministicTaskScheduler => false; } diff --git a/src/NCronJob/Registry/JobRegistry.cs b/src/NCronJob/Registry/JobRegistry.cs index 8683555..f5ab9a0 100644 --- a/src/NCronJob/Registry/JobRegistry.cs +++ b/src/NCronJob/Registry/JobRegistry.cs @@ -4,38 +4,39 @@ namespace NCronJob; internal sealed class JobRegistry { - private readonly List allJobs; - private readonly ImmutableArray oneTimeJobs; - private readonly List cronJobs; + private readonly HashSet allJobs; + private readonly ImmutableHashSet oneTimeJobs; + private readonly HashSet cronJobs; public JobRegistry(IEnumerable jobs) { var jobDefinitions = jobs as JobDefinition[] ?? jobs.ToArray(); - allJobs = [..jobDefinitions]; - cronJobs = [..jobDefinitions.Where(c => c.CronExpression is not null)]; + allJobs = new HashSet(jobDefinitions, JobDefinitionEqualityComparer.Instance); + cronJobs = new HashSet(jobDefinitions.Where(c => c.CronExpression is not null), JobDefinitionEqualityComparer.Instance); oneTimeJobs = [..jobDefinitions.Where(c => c.IsStartupJob)]; AssertNoDuplicateJobNames(); } public IReadOnlyCollection GetAllCronJobs() => cronJobs; + public IReadOnlyCollection GetAllOneTimeJobs() => oneTimeJobs; - public bool IsJobRegistered() => allJobs.Exists(j => j.Type == typeof(T)); + public bool IsJobRegistered() => allJobs.Any(j => j.Type == typeof(T)); public JobDefinition GetJobDefinition() => allJobs.First(j => j.Type == typeof(T)); public JobDefinition? FindJobDefinition(Type type) - => allJobs.Find(j => j.Type == type); + => allJobs.FirstOrDefault(j => j.Type == type); public JobDefinition? FindJobDefinition(string jobName) - => allJobs.Find(j => j.CustomName == jobName); + => allJobs.FirstOrDefault(j => j.CustomName == jobName); public void Add(JobDefinition jobDefinition) { AssertNoDuplicateJobNames(jobDefinition.CustomName); - var isTypeUpdate = allJobs.Exists(j => j.JobFullName == jobDefinition.JobFullName); + var isTypeUpdate = allJobs.Any(j => j.JobFullName == jobDefinition.JobFullName); if (isTypeUpdate) { Remove(jobDefinition); @@ -48,13 +49,13 @@ public void Add(JobDefinition jobDefinition) } } public int GetJobTypeConcurrencyLimit(string jobTypeName) - => allJobs.Find(j => j.JobFullName == jobTypeName) + => allJobs.FirstOrDefault(j => j.JobFullName == jobTypeName) ?.ConcurrencyPolicy ?.MaxDegreeOfParallelism ?? 1; - public void RemoveByName(string jobName) => Remove(allJobs.Find(j => j.CustomName == jobName)); + public void RemoveByName(string jobName) => Remove(allJobs.FirstOrDefault(j => j.CustomName == jobName)); - public void RemoveByType(Type type) => Remove(allJobs.Find(j => j.Type == type)); + public void RemoveByType(Type type) => Remove(allJobs.FirstOrDefault(j => j.Type == type)); private void Remove(JobDefinition? jobDefinition) { @@ -84,4 +85,25 @@ private void AssertNoDuplicateJobNames(string? additionalJobName = null) throw new InvalidOperationException($"Duplicate job names found: {string.Join(", ", duplicateJobName)}"); } } + + private sealed class JobDefinitionEqualityComparer : IEqualityComparer + { + public static readonly JobDefinitionEqualityComparer Instance = new(); + + public bool Equals(JobDefinition? x, JobDefinition? y) => + (x is null && y is null) || (x is not null && y is not null && x.Type == y.Type + && x.Parameter == y.Parameter + && x.CronExpression == y.CronExpression + && x.TimeZone == y.TimeZone + && x.CustomName == y.CustomName + && x.IsStartupJob == y.IsStartupJob); + + public int GetHashCode(JobDefinition obj) => HashCode.Combine( + obj.Type, + obj.Parameter, + obj.CronExpression, + obj.TimeZone, + obj.CustomName, + obj.IsStartupJob); + } } diff --git a/src/NCronJob/Scheduler/TaskScheduler/TaskFactoryProvider.cs b/src/NCronJob/Scheduler/TaskScheduler/TaskFactoryProvider.cs index a186566..3229616 100644 --- a/src/NCronJob/Scheduler/TaskScheduler/TaskFactoryProvider.cs +++ b/src/NCronJob/Scheduler/TaskScheduler/TaskFactoryProvider.cs @@ -2,7 +2,10 @@ namespace NCronJob; internal static class TaskFactoryProvider { - private static TaskScheduler TaskScheduler => ConcurrencySettings.UseDeterministicTaskScheduler ? new DeterministicTaskScheduler() : TaskScheduler.Default; + private static TaskScheduler TaskScheduler => ConcurrencySettings.UseDeterministicTaskScheduler + ? new DeterministicTaskScheduler() + : TaskScheduler.Default; + private static readonly TaskFactory TaskFactory = new( CancellationToken.None, TaskCreationOptions.LongRunning | TaskCreationOptions.PreferFairness, diff --git a/tests/NCronJob.Tests/NCronJobIntegrationTests.cs b/tests/NCronJob.Tests/NCronJobIntegrationTests.cs index ddd946a..0496216 100644 --- a/tests/NCronJob.Tests/NCronJobIntegrationTests.cs +++ b/tests/NCronJob.Tests/NCronJobIntegrationTests.cs @@ -54,7 +54,9 @@ public async Task EachJobRunHasItsOwnScope() ServiceCollection.AddSingleton(storage); ServiceCollection.AddScoped(); ServiceCollection.AddNCronJob(n => n.AddJob( - p => p.WithCronExpression("* * * * *").And.WithCronExpression("* * * * *"))); + p => p.WithCronExpression("* * * * *").WithParameter("null") + .And + .WithCronExpression("* * * * *"))); var provider = CreateServiceProvider(); await provider.GetRequiredService().StartAsync(CancellationToken); @@ -214,10 +216,6 @@ public async Task WhileAwaitingJobTriggeringInstantJobShouldAnywayTriggerCronJob [Fact] public async Task MinimalJobApiCanBeUsedForTriggeringCronJobs() { - ServiceCollection.AddNCronJob(async (ChannelWriter writer) => - { - await writer.WriteAsync(null); - }, "* * * * *"); ServiceCollection.AddNCronJob(async (ChannelWriter writer) => { await writer.WriteAsync(null); @@ -227,7 +225,7 @@ public async Task MinimalJobApiCanBeUsedForTriggeringCronJobs() await provider.GetRequiredService().StartAsync(CancellationToken); FakeTimer.Advance(TimeSpan.FromMinutes(1)); - var jobFinished = await WaitForJobsOrTimeout(2); + var jobFinished = await WaitForJobsOrTimeout(1); jobFinished.ShouldBeTrue(); } @@ -235,10 +233,10 @@ public async Task MinimalJobApiCanBeUsedForTriggeringCronJobs() public async Task ConcurrentJobConfigurationShouldBeRespected() { ServiceCollection.AddNCronJob(n => n.AddJob(p => p - .WithCronExpression("* * * * *") - .And.WithCronExpression("* * * * *") - .And.WithCronExpression("* * * * *") - .And.WithCronExpression("* * * * *"))); + .WithCronExpression("* * * * *").WithName("Job 1") + .And.WithCronExpression("* * * * *").WithName("Job 2") + .And.WithCronExpression("* * * * *").WithName("Job 3") + .And.WithCronExpression("* * * * *").WithName("Job 4"))); var provider = CreateServiceProvider(); await provider.GetRequiredService().StartAsync(CancellationToken); @@ -297,7 +295,6 @@ public async Task ExecuteAnInstantJobDelegate() [Fact] public async Task AnonymousJobsCanBeExecutedMultipleTimes() { - ServiceCollection.AddNCronJob(n => n.AddJob(async (ChannelWriter writer, CancellationToken ct) => { await Task.Delay(10, ct); @@ -334,7 +331,7 @@ public void AddingJobsWithTheSameCustomNameLeadsToException() { ServiceCollection.AddNCronJob( n => n.AddJob(() => { }, "* * * * *", jobName: "Job1") - .AddJob(() => { }, "* * * * *", jobName: "Job1")); + .AddJob(() => { }, "0 * * * *", jobName: "Job1")); var provider = CreateServiceProvider(); Action act = () => provider.GetRequiredService(); @@ -354,6 +351,38 @@ public void AddJobsDynamicallyWhenNameIsDuplicatedLeadsToException() act.ShouldThrow(); } + [Fact] + public async Task TwoJobsWithSameDefinitionLeadToOneExecution() + { + ServiceCollection.AddNCronJob(n => n.AddJob(p => p.WithCronExpression("* * * * *").And.WithCronExpression("* * * * *"))); + var provider = CreateServiceProvider(); + await provider.GetRequiredService().StartAsync(CancellationToken); + + var countJobs = provider.GetRequiredService().GetAllCronJobs().Count; + countJobs.ShouldBe(1); + + FakeTimer.Advance(TimeSpan.FromMinutes(1)); + var jobFinished = await WaitForJobsOrTimeout(2, TimeSpan.FromMilliseconds(250)); + jobFinished.ShouldBeFalse(); + } + + [Fact] + public async Task TwoJobsWithDifferentDefinitionLeadToTwoExecutions() + { + ServiceCollection.AddNCronJob(n => n + .AddJob(p => p.WithCronExpression("* * * * *").WithParameter("1")) + .AddJob(p => p.WithCronExpression("* * * * *").WithParameter("2"))); + var provider = CreateServiceProvider(); + await provider.GetRequiredService().StartAsync(CancellationToken); + + var countJobs = provider.GetRequiredService().GetAllCronJobs().Count; + countJobs.ShouldBe(2); + + FakeTimer.Advance(TimeSpan.FromMinutes(1)); + var jobFinished = await WaitForJobsOrTimeout(2, TimeSpan.FromMilliseconds(250)); + jobFinished.ShouldBeTrue(); + } + private static class JobMethods { public static async Task WriteTrueStaticAsync(ChannelWriter writer, CancellationToken ct)