From ff3dd8dc11b1add9cec172fcf687bc8a948c958c Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 1 Jan 2025 11:48:16 +0100 Subject: [PATCH 1/7] Drop references to non longer existing v4 branch --- .github/workflows/dotnet.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/dotnet.yml b/.github/workflows/dotnet.yml index e9fa6fbd..bde9deec 100644 --- a/.github/workflows/dotnet.yml +++ b/.github/workflows/dotnet.yml @@ -2,9 +2,9 @@ name: .NET on: push: - branches: [ main, v4 ] + branches: [ main ] pull_request: - branches: [ main, v4 ] + branches: [ main ] env: VSTEST_CONNECTION_TIMEOUT: 180 From 3c5c664635a8d33c7ddbf320201c68381cc6c60d Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 1 Jan 2025 11:51:25 +0100 Subject: [PATCH 2/7] Minor cleanup to IRuntimeJobRegistry documentation --- src/NCronJob/Registry/RuntimeJobRegistry.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/NCronJob/Registry/RuntimeJobRegistry.cs b/src/NCronJob/Registry/RuntimeJobRegistry.cs index 901e96f9..a55d1e48 100644 --- a/src/NCronJob/Registry/RuntimeJobRegistry.cs +++ b/src/NCronJob/Registry/RuntimeJobRegistry.cs @@ -34,11 +34,13 @@ public interface IRuntimeJobRegistry /// /// Removes all jobs of the given type. /// + /// If the given job is not found, no exception is thrown. void RemoveJob() where TJob : IJob; /// /// Removes all jobs of the given type. /// + /// If the given job is not found, no exception is thrown. void RemoveJob(Type type); /// @@ -80,7 +82,7 @@ public interface IRuntimeJobRegistry IReadOnlyCollection GetAllRecurringJobs(); /// - /// This will enable a job that was previously disabled. + /// Enables a job that was previously disabled. /// /// The unique job name that identifies this job. /// @@ -90,7 +92,7 @@ public interface IRuntimeJobRegistry void EnableJob(string jobName); /// - /// This will disable a job that was previously enabled. + /// Disables a job that was previously enabled. /// /// The unique job name that identifies this job. /// From 1578e334e4d75ff18c7ca2b0db4eec08ea7bcf80 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 1 Jan 2025 12:24:15 +0100 Subject: [PATCH 3/7] Make RemoveJob() behave according to its documentation --- CHANGELOG.md | 4 ++++ src/NCronJob/Registry/IInstantJobRegistry.cs | 2 +- src/NCronJob/Registry/JobRegistry.cs | 15 +++++++++++++-- src/NCronJob/Scheduler/JobWorker.cs | 2 +- .../NCronJob.Tests/NCronJobIntegrationTests.cs | 2 +- .../NCronJob.Tests/RuntimeJobRegistryTests.cs | 18 ++++++++++++++++++ 6 files changed, 38 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c20de28e..e4dee511 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] +### Fixed + +- Make `RemoveJob()` and `RemoveJob(Type)` remove all jobs of the given type. Fixed in [#151](https://github.com/NCronJob-Dev/NCronJob/issues/151), by [@nulltoken](https://github.com/nulltoken). + ## [v4.0.2] - 2024-12-28 New `v4` release with some new features and improvements. Check the [`v4` migration guide](https://docs.ncronjob.dev/migration/v4/) for more information. diff --git a/src/NCronJob/Registry/IInstantJobRegistry.cs b/src/NCronJob/Registry/IInstantJobRegistry.cs index e9803a66..40b2a0b9 100644 --- a/src/NCronJob/Registry/IInstantJobRegistry.cs +++ b/src/NCronJob/Registry/IInstantJobRegistry.cs @@ -237,7 +237,7 @@ private void RunJob(DateTimeOffset startDate, object? parameter = null, bo { using (logger.BeginScope("Triggering RunScheduledJob:")) { - var jobDefinition = jobRegistry.FindJobDefinition(typeof(TJob)); + var jobDefinition = jobRegistry.FindFirstJobDefinition(typeof(TJob)); if (jobDefinition is null) { diff --git a/src/NCronJob/Registry/JobRegistry.cs b/src/NCronJob/Registry/JobRegistry.cs index b81098ce..2577efe2 100644 --- a/src/NCronJob/Registry/JobRegistry.cs +++ b/src/NCronJob/Registry/JobRegistry.cs @@ -16,7 +16,10 @@ private readonly Dictionary> depe public IReadOnlyCollection GetAllOneTimeJobs() => allJobs.Where(c => c.IsStartupJob).ToList(); - public JobDefinition? FindJobDefinition(Type type) + public IReadOnlyCollection FindAllJobDefinition(Type type) + => allJobs.Where(j => j.Type == type).ToList(); + + public JobDefinition? FindFirstJobDefinition(Type type) => allJobs.FirstOrDefault(j => j.Type == type); public JobDefinition? FindJobDefinition(string jobName) @@ -43,7 +46,15 @@ public int GetJobTypeConcurrencyLimit(string jobTypeName) public void RemoveByName(string jobName) => Remove(allJobs.FirstOrDefault(j => j.CustomName == jobName)); - public void RemoveByType(Type type) => Remove(allJobs.FirstOrDefault(j => j.Type == type)); + public void RemoveByType(Type type) + { + var jobDefinitions = FindAllJobDefinition(type); + + foreach (var jobDefinition in jobDefinitions) + { + Remove(jobDefinition); + } + } public JobDefinition AddDynamicJob( Delegate jobDelegate, diff --git a/src/NCronJob/Scheduler/JobWorker.cs b/src/NCronJob/Scheduler/JobWorker.cs index 3d1efad2..37767f53 100644 --- a/src/NCronJob/Scheduler/JobWorker.cs +++ b/src/NCronJob/Scheduler/JobWorker.cs @@ -203,7 +203,7 @@ public void RemoveJobByName(string jobName) public void RemoveJobType(Type type) { - var fullName = registry.FindJobDefinition(type)?.JobFullName; + var fullName = registry.FindFirstJobDefinition(type)?.JobFullName; if (fullName is null) { return; diff --git a/tests/NCronJob.Tests/NCronJobIntegrationTests.cs b/tests/NCronJob.Tests/NCronJobIntegrationTests.cs index e90a9eb2..206ae8f6 100644 --- a/tests/NCronJob.Tests/NCronJobIntegrationTests.cs +++ b/tests/NCronJob.Tests/NCronJobIntegrationTests.cs @@ -83,7 +83,7 @@ public async Task ExecuteAnInstantJobWithoutPreviousRegistration() jobFinished.ShouldBeTrue(); var jobRegistry = provider.GetRequiredService(); - jobRegistry.FindJobDefinition(typeof(SimpleJob)).ShouldBeNull(); + jobRegistry.FindFirstJobDefinition(typeof(SimpleJob)).ShouldBeNull(); } [Fact] diff --git a/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs b/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs index b1638e9b..6334e20e 100644 --- a/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs +++ b/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs @@ -69,6 +69,24 @@ public async Task CanRemoveByJobType() jobFinished.ShouldBeFalse(); } + [Fact] + public async Task RemovingByJobTypeAccountsForAllJobs() + { + ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression("1 * * * *"))); + ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression("2 * * * *"))); + + var provider = CreateServiceProvider(); + await provider.GetRequiredService().StartAsync(CancellationToken); + var registry = provider.GetRequiredService(); + + var jobRegistry = provider.GetRequiredService(); + Assert.Equal(2, jobRegistry.FindAllJobDefinition(typeof(SimpleJob)).Count); + + registry.RemoveJob(); + + Assert.Empty(jobRegistry.FindAllJobDefinition(typeof(SimpleJob))); + } + [Fact] public async Task CanUpdateScheduleOfAJob() { From 47b8f5ca4dfc12f38acbfa6c38db141420d5a259 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 1 Jan 2025 12:53:43 +0100 Subject: [PATCH 4/7] Expose typed version of DisableJob() --- CHANGELOG.md | 4 + docs/advanced/dynamic-job-control.md | 16 +++- src/NCronJob/Registry/RuntimeJobRegistry.cs | 83 ++++++++++++++++--- src/NCronJob/Scheduler/JobWorker.cs | 9 ++ .../NCronJob.Tests/RuntimeJobRegistryTests.cs | 34 ++++++++ 5 files changed, 134 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4dee511..cdf17935 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] +### Added + +- Expose typed version of `DisableJob()`. Added in [#151](https://github.com/NCronJob-Dev/NCronJob/issues/151), by [@nulltoken](https://github.com/nulltoken). + ### Fixed - Make `RemoveJob()` and `RemoveJob(Type)` remove all jobs of the given type. Fixed in [#151](https://github.com/NCronJob-Dev/NCronJob/issues/151), by [@nulltoken](https://github.com/nulltoken). diff --git a/docs/advanced/dynamic-job-control.md b/docs/advanced/dynamic-job-control.md index 32f4ebbf..7a8cfd18 100644 --- a/docs/advanced/dynamic-job-control.md +++ b/docs/advanced/dynamic-job-control.md @@ -97,7 +97,9 @@ var found = registry.TryGetSchedule("MyName", out string? cronExpression, out Ti The cron expression and time zone can be `null` even if the job was found. This indicates that the job has no schedule (like dependent jobs). ## Disabling and enabling jobs -To disable a job, use the `DisableJob` method: +There are two ways to disable a job from the scheduler. By name or by type. + +To disable a job by name: ```csharp app.MapPut("/disable-job", (IRuntimeJobRegistry registry) => @@ -107,6 +109,18 @@ app.MapPut("/disable-job", (IRuntimeJobRegistry registry) => }); ``` +That will prevent one job named `MyName` from being scheduled. + +In contrast disabling by type will disable all jobs of the given type (so zero to many jobs): + +```csharp +app.MapPut("/disable-job", (IRuntimeJobRegistry registry) => +{ + registry.DisableJob(); // Alternatively DisableJob(typeof(SampleJob)) + return TypedResults.Ok(); +}); +``` + If a job is disabled, it will not be scheduled anymore. Any planned job will be cancelled and the job will be removed from the scheduler. To enable a job, use the `EnableJob` method: diff --git a/src/NCronJob/Registry/RuntimeJobRegistry.cs b/src/NCronJob/Registry/RuntimeJobRegistry.cs index a55d1e48..39881b2c 100644 --- a/src/NCronJob/Registry/RuntimeJobRegistry.cs +++ b/src/NCronJob/Registry/RuntimeJobRegistry.cs @@ -100,6 +100,24 @@ public interface IRuntimeJobRegistry /// If the job is not found, an exception is thrown. /// void DisableJob(string jobName); + + /// + /// Disables all jobs of the given type. + /// + /// + /// If the job is already disabled, this method does nothing. + /// If the job is not found, an exception is thrown. + /// + void DisableJob() where TJob : IJob; + + /// + /// Disables all jobs of the given type. + /// + /// + /// If the job is already disabled, this method does nothing. + /// If the job is not found, an exception is thrown. + /// + void DisableJob(Type type); } /// @@ -113,6 +131,9 @@ public sealed record RecurringJobSchedule(string? JobName, string CronExpression /// internal sealed class RuntimeJobRegistry : IRuntimeJobRegistry { + // https://crontab.guru/#*_*_31_2_* + internal static readonly CronExpression TheThirtyFirstOfFebruary = CronExpression.Parse("* * 31 2 *"); + private readonly IServiceCollection services; private readonly JobRegistry jobRegistry; private readonly JobWorker jobWorker; @@ -237,12 +258,7 @@ public void EnableJob(string jobName) var job = jobRegistry.FindJobDefinition(jobName) ?? throw new InvalidOperationException($"Job with name '{jobName}' not found."); - if (job.CronExpression is not null) - { - job.CronExpression = CronExpression.Parse(job.UserDefinedCronExpression); - } - - jobWorker.RescheduleJobWithJobName(job); + EnableJob(job, jobName); } /// @@ -251,13 +267,58 @@ public void DisableJob(string jobName) var job = jobRegistry.FindJobDefinition(jobName) ?? throw new InvalidOperationException($"Job with name '{jobName}' not found."); - if (job.CronExpression is not null) + DisableJob(job, jobName); + } + + /// + public void DisableJob() where TJob : IJob => DisableJob(typeof(TJob)); + + /// + public void DisableJob(Type type) + { + ProcessAllJobDefinitionsOfType(type, j=> DisableJob(j)); + } + + private void ProcessAllJobDefinitionsOfType(Type type, Action processor) + { + var jobDefinitions = jobRegistry.FindAllJobDefinition(type); + + foreach (var jobDefinition in jobDefinitions) { - // https://crontab.guru/#*_*_31_2_* - // Scheduling on Feb, 31st is a sure way to never get it to run - job.CronExpression = CronExpression.Parse("* * 31 2 *"); + processor(jobDefinition); } + } - jobWorker.RescheduleJobWithJobName(job); + private void EnableJob(JobDefinition job, string? customName = null) + { + if (job.UserDefinedCronExpression is not null) + { + job.CronExpression = CronExpression.Parse(job.UserDefinedCronExpression); + } + else + { + job.CronExpression = null; + } + + RescheduleJob(job, customName); + } + + private void DisableJob(JobDefinition job, string? customName = null) + { + // Scheduling on Feb, 31st is a sure way to never get it to run + job.CronExpression = TheThirtyFirstOfFebruary; + + RescheduleJob(job, customName); + } + + private void RescheduleJob(JobDefinition job, string? customName = null) + { + if (customName is not null) + { + jobWorker.RescheduleJobWithJobName(job); + return; + } + + jobWorker.RescheduleJobByType(job); } } diff --git a/src/NCronJob/Scheduler/JobWorker.cs b/src/NCronJob/Scheduler/JobWorker.cs index 37767f53..4b7bf99f 100644 --- a/src/NCronJob/Scheduler/JobWorker.cs +++ b/src/NCronJob/Scheduler/JobWorker.cs @@ -223,6 +223,15 @@ public void RescheduleJobWithJobName(JobDefinition jobDefinition) jobQueueManager.SignalJobQueue(jobDefinition.JobFullName); } + public void RescheduleJobByType(JobDefinition jobDefinition) + { + ArgumentNullException.ThrowIfNull(jobDefinition); + + jobQueueManager.RemoveQueue(jobDefinition.JobFullName); + ScheduleJob(jobDefinition); + jobQueueManager.SignalJobQueue(jobDefinition.JobFullName); + } + private void RemoveJobRunByName(string jobName) { var jobType = registry.FindJobDefinition(jobName); diff --git a/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs b/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs index 6334e20e..622cdeb7 100644 --- a/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs +++ b/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs @@ -232,6 +232,40 @@ public async Task ShouldDisableJob() jobFinished.ShouldBeFalse(); } + [Fact] + public async Task DisablingByJobTypeAccountsForAllJobs() + { + ServiceCollection.AddNCronJob(s => s.AddJob()); + ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression("2 * * * *"))); + + var provider = CreateServiceProvider(); + await provider.GetRequiredService().StartAsync(CancellationToken); + var registry = provider.GetRequiredService(); + + var jobRegistry = provider.GetRequiredService(); + var jobs = jobRegistry.FindAllJobDefinition(typeof(SimpleJob)); + Assert.Equal(2, jobs.Count); + + Assert.All(jobs, j => + { + if (j.CronExpression is null) + { + return; + } + + Assert.NotEqual(RuntimeJobRegistry.TheThirtyFirstOfFebruary, j.CronExpression); + }); + + registry.DisableJob(); + + jobs = jobRegistry.FindAllJobDefinition(typeof(SimpleJob)); + + Assert.All(jobs, j => + { + Assert.Equal(RuntimeJobRegistry.TheThirtyFirstOfFebruary, j.CronExpression); + }); + } + [Fact] public async Task ShouldThrowAnExceptionWhenJobIsNotFoundAndTryingToDisable() { From 0ffa2934430a7fc23fda97a3ddf17fcd99a0b48e Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 1 Jan 2025 15:09:22 +0100 Subject: [PATCH 5/7] Expose typed version of EnableJob() --- CHANGELOG.md | 1 + docs/advanced/dynamic-job-control.md | 14 +++++++++- src/NCronJob/Registry/RuntimeJobRegistry.cs | 27 +++++++++++++++++++ .../NCronJob.Tests/RuntimeJobRegistryTests.cs | 17 +++++++++--- 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cdf17935..da2543e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ All notable changes to **NCronJob** will be documented in this file. The project ### Added - Expose typed version of `DisableJob()`. Added in [#151](https://github.com/NCronJob-Dev/NCronJob/issues/151), by [@nulltoken](https://github.com/nulltoken). +- Expose typed version of `EnableJob()`. Added in [#151](https://github.com/NCronJob-Dev/NCronJob/issues/151), by [@nulltoken](https://github.com/nulltoken). ### Fixed diff --git a/docs/advanced/dynamic-job-control.md b/docs/advanced/dynamic-job-control.md index 7a8cfd18..78609a13 100644 --- a/docs/advanced/dynamic-job-control.md +++ b/docs/advanced/dynamic-job-control.md @@ -123,7 +123,9 @@ app.MapPut("/disable-job", (IRuntimeJobRegistry registry) => If a job is disabled, it will not be scheduled anymore. Any planned job will be cancelled and the job will be removed from the scheduler. -To enable a job, use the `EnableJob` method: +Of course, it's also possible to enable back previously disabled jobs. + +To enable a job by name: ```csharp app.MapPut("/enable-job", (IRuntimeJobRegistry registry) => @@ -132,3 +134,13 @@ app.MapPut("/enable-job", (IRuntimeJobRegistry registry) => return TypedResults.Ok(); }); ``` + +And similarly, to enable all jobs of the given type (so zero to many jobs): + +```csharp +app.MapPut("/enable-job", (IRuntimeJobRegistry registry) => +{ + registry.EnableJob(); // Alternatively EnableJob(typeof(SampleJob)) + return TypedResults.Ok(); +}); +``` diff --git a/src/NCronJob/Registry/RuntimeJobRegistry.cs b/src/NCronJob/Registry/RuntimeJobRegistry.cs index 39881b2c..cfea6631 100644 --- a/src/NCronJob/Registry/RuntimeJobRegistry.cs +++ b/src/NCronJob/Registry/RuntimeJobRegistry.cs @@ -91,6 +91,24 @@ public interface IRuntimeJobRegistry /// void EnableJob(string jobName); + /// + /// Enables all jobs of the given type that were previously disabled. + /// + /// + /// If the job is already enabled, this method does nothing. + /// If the job is not found, an exception is thrown. + /// + void EnableJob() where TJob : IJob; + + /// + /// Enables all jobs of the given type that were previously disabled. + /// + /// + /// If the job is already enabled, this method does nothing. + /// If the job is not found, an exception is thrown. + /// + void EnableJob(Type type); + /// /// Disables a job that was previously enabled. /// @@ -261,6 +279,15 @@ public void EnableJob(string jobName) EnableJob(job, jobName); } + /// + public void EnableJob() where TJob : IJob => EnableJob(typeof(TJob)); + + /// + public void EnableJob(Type type) + { + ProcessAllJobDefinitionsOfType(type, j => EnableJob(j)); + } + /// public void DisableJob(string jobName) { diff --git a/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs b/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs index 622cdeb7..d79cb012 100644 --- a/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs +++ b/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs @@ -7,6 +7,8 @@ namespace NCronJob.Tests; public class RuntimeJobRegistryTests : JobIntegrationBase { + private const string AtMinute2 = "2 * * * *"; + [Fact] public async Task DynamicallyAddedJobIsExecuted() { @@ -73,7 +75,7 @@ public async Task CanRemoveByJobType() public async Task RemovingByJobTypeAccountsForAllJobs() { ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression("1 * * * *"))); - ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression("2 * * * *"))); + ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression(AtMinute2))); var provider = CreateServiceProvider(); await provider.GetRequiredService().StartAsync(CancellationToken); @@ -233,10 +235,10 @@ public async Task ShouldDisableJob() } [Fact] - public async Task DisablingByJobTypeAccountsForAllJobs() + public async Task DisablingAndEnablingByJobTypeAccountsForAllJobs() { ServiceCollection.AddNCronJob(s => s.AddJob()); - ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression("2 * * * *"))); + ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression(AtMinute2))); var provider = CreateServiceProvider(); await provider.GetRequiredService().StartAsync(CancellationToken); @@ -259,11 +261,20 @@ public async Task DisablingByJobTypeAccountsForAllJobs() registry.DisableJob(); jobs = jobRegistry.FindAllJobDefinition(typeof(SimpleJob)); + Assert.Equal(2, jobs.Count); Assert.All(jobs, j => { Assert.Equal(RuntimeJobRegistry.TheThirtyFirstOfFebruary, j.CronExpression); }); + + registry.EnableJob(); + + jobs = jobRegistry.FindAllJobDefinition(typeof(SimpleJob)); + Assert.Equal(2, jobs.Count); + + Assert.Single(jobs, j => j.CronExpression is null); + Assert.Single(jobs, j => j.CronExpression is not null && j.CronExpression.ToString() == AtMinute2); } [Fact] From f1c8999be3eeed7f8305d553c05afe0e92dad066 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 1 Jan 2025 15:25:26 +0100 Subject: [PATCH 6/7] Ensure UpdateSchedule() behavior is consistent --- CHANGELOG.md | 1 + src/NCronJob/Registry/RuntimeJobRegistry.cs | 1 + .../NCronJob.Tests/RuntimeJobRegistryTests.cs | 48 +++++++++++-------- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da2543e8..ed23b8d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ All notable changes to **NCronJob** will be documented in this file. The project ### Fixed - Make `RemoveJob()` and `RemoveJob(Type)` remove all jobs of the given type. Fixed in [#151](https://github.com/NCronJob-Dev/NCronJob/issues/151), by [@nulltoken](https://github.com/nulltoken). +- Ensure `UpdateSchedule()` behavior is consistent. Fixed in [#151](https://github.com/NCronJob-Dev/NCronJob/issues/151), by [@nulltoken](https://github.com/nulltoken). ## [v4.0.2] - 2024-12-28 diff --git a/src/NCronJob/Registry/RuntimeJobRegistry.cs b/src/NCronJob/Registry/RuntimeJobRegistry.cs index cfea6631..b943da00 100644 --- a/src/NCronJob/Registry/RuntimeJobRegistry.cs +++ b/src/NCronJob/Registry/RuntimeJobRegistry.cs @@ -226,6 +226,7 @@ public void UpdateSchedule(string jobName, string cronExpression, TimeZoneInfo? var cron = NCronJobOptionBuilder.GetCronExpression(cronExpression); job.CronExpression = cron; + job.UserDefinedCronExpression = cronExpression; job.TimeZone = timeZoneInfo ?? TimeZoneInfo.Utc; jobWorker.RescheduleJobWithJobName(job); diff --git a/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs b/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs index d79cb012..215667f1 100644 --- a/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs +++ b/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs @@ -7,6 +7,7 @@ namespace NCronJob.Tests; public class RuntimeJobRegistryTests : JobIntegrationBase { + private const string AtEveryMinute = "* * * * *"; private const string AtMinute2 = "2 * * * *"; [Fact] @@ -17,7 +18,7 @@ public async Task DynamicallyAddedJobIsExecuted() await provider.GetRequiredService().StartAsync(CancellationToken); var registry = provider.GetRequiredService(); - registry.TryRegister(s => s.AddJob(async (ChannelWriter writer) => await writer.WriteAsync(true), "* * * * *"), out _); + registry.TryRegister(s => s.AddJob(async (ChannelWriter writer) => await writer.WriteAsync(true), AtEveryMinute), out _); FakeTimer.Advance(TimeSpan.FromMinutes(1)); var jobFinished = await WaitForJobsOrTimeout(1); @@ -32,8 +33,8 @@ public async Task MultipleDynamicallyAddedJobsAreExecuted() await provider.GetRequiredService().StartAsync(CancellationToken); var registry = provider.GetRequiredService(); - registry.TryRegister(s => s.AddJob(async (ChannelWriter writer) => await writer.WriteAsync(true), "* * * * *")); - registry.TryRegister(s => s.AddJob(async (ChannelWriter writer) => await writer.WriteAsync(true), "* * * * *")); + registry.TryRegister(s => s.AddJob(async (ChannelWriter writer) => await writer.WriteAsync(true), AtEveryMinute)); + registry.TryRegister(s => s.AddJob(async (ChannelWriter writer) => await writer.WriteAsync(true), AtEveryMinute)); FakeTimer.Advance(TimeSpan.FromMinutes(1)); var jobFinished = await WaitForJobsOrTimeout(2); @@ -44,7 +45,7 @@ public async Task MultipleDynamicallyAddedJobsAreExecuted() public async Task CanRemoveJobByName() { ServiceCollection.AddNCronJob( - s => s.AddJob(async (ChannelWriter writer) => await writer.WriteAsync(true), "* * * * *", jobName: "Job")); + s => s.AddJob(async (ChannelWriter writer) => await writer.WriteAsync(true), AtEveryMinute, jobName: "Job")); var provider = CreateServiceProvider(); await provider.GetRequiredService().StartAsync(CancellationToken); var registry = provider.GetRequiredService(); @@ -59,7 +60,7 @@ public async Task CanRemoveJobByName() [Fact] public async Task CanRemoveByJobType() { - ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression("* * * * *"))); + ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression(AtEveryMinute))); var provider = CreateServiceProvider(); await provider.GetRequiredService().StartAsync(CancellationToken); var registry = provider.GetRequiredService(); @@ -97,7 +98,12 @@ public async Task CanUpdateScheduleOfAJob() await provider.GetRequiredService().StartAsync(CancellationToken); var registry = provider.GetRequiredService(); - registry.UpdateSchedule("JobName", "* * * * *"); + registry.UpdateSchedule("JobName", AtEveryMinute); + + var jobRegistry = provider.GetRequiredService(); + var jobDefinition = jobRegistry.GetAllJobs().Single(); + + Assert.Equal(AtEveryMinute, jobDefinition.UserDefinedCronExpression); FakeTimer.Advance(TimeSpan.FromMinutes(1)); var jobFinished = await WaitForJobsOrTimeout(1); @@ -112,7 +118,7 @@ public async Task ShouldThrowAnExceptionWhenJobIsNotFoundAndTryingToUpdateSchedu await provider.GetRequiredService().StartAsync(CancellationToken); var registry = provider.GetRequiredService(); - Should.Throw(() => registry.UpdateSchedule("JobName", "* * * * *")); + Should.Throw(() => registry.UpdateSchedule("JobName", AtEveryMinute)); } [Fact] @@ -150,7 +156,7 @@ public async Task ShouldFindDependentJobsWithAGivenName() { ServiceCollection.AddNCronJob(s => { - s.AddJob(p => p.WithCronExpression("* * * * *").WithName("Job1")) + s.AddJob(p => p.WithCronExpression(AtEveryMinute).WithName("Job1")) .ExecuteWhen(r => r.RunJob(() => { }, "Job2")); }); var provider = CreateServiceProvider(); @@ -167,7 +173,7 @@ public async Task ShouldFindDependentJobsWithAGivenName() public async Task UpdatingParameterHasImmediateEffect() { ServiceCollection.AddNCronJob(s => s.AddJob(p => p - .WithCronExpression("* * * * *") + .WithCronExpression(AtEveryMinute) .WithParameter("foo") .WithName("JobName"))); var provider = CreateServiceProvider(); @@ -190,7 +196,7 @@ public void ShouldRetrieveAllSchedules() .WithName("JobName"))); var provider = CreateServiceProvider(); var registry = provider.GetRequiredService(); - registry.TryRegister(s => s.AddJob(() => { }, "* * * * *", jobName: "JobName2"), out _); + registry.TryRegister(s => s.AddJob(() => { }, AtEveryMinute, jobName: "JobName2"), out _); var allSchedules = registry.GetAllRecurringJobs(); @@ -199,7 +205,7 @@ public void ShouldRetrieveAllSchedules() && s.CronExpression == "*/2 * * * *" && s.TimeZone == timeZone); allSchedules.ShouldContain(s => s.JobName == "JobName2" - && s.CronExpression == "* * * * *" + && s.CronExpression == AtEveryMinute && s.TimeZone == TimeZoneInfo.Utc); } @@ -209,20 +215,20 @@ public void AddingJobDuringRuntimeIsRetrieved() ServiceCollection.AddNCronJob(p => p.AddJob()); var provider = CreateServiceProvider(); var registry = provider.GetRequiredService(); - registry.TryRegister(n => n.AddJob(p => p.WithCronExpression("* * * * *").WithName("JobName"))); + registry.TryRegister(n => n.AddJob(p => p.WithCronExpression(AtEveryMinute).WithName("JobName"))); var allSchedules = registry.GetAllRecurringJobs(); allSchedules.Count.ShouldBe(1); allSchedules.ShouldContain(s => s.JobName == "JobName" - && s.CronExpression == "* * * * *" + && s.CronExpression == AtEveryMinute && s.TimeZone == TimeZoneInfo.Utc); } [Fact] public async Task ShouldDisableJob() { - ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression("* * * * *").WithName("JobName"))); + ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression(AtEveryMinute).WithName("JobName"))); var provider = CreateServiceProvider(); await provider.GetRequiredService().StartAsync(CancellationToken); var registry = provider.GetRequiredService(); @@ -291,7 +297,7 @@ public async Task ShouldThrowAnExceptionWhenJobIsNotFoundAndTryingToDisable() [Fact] public async Task ShouldEnableJob() { - ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression("* * * * *").WithName("JobName"))); + ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression(AtEveryMinute).WithName("JobName"))); var provider = CreateServiceProvider(); await provider.GetRequiredService().StartAsync(CancellationToken); var registry = provider.GetRequiredService(); @@ -307,7 +313,7 @@ public async Task ShouldEnableJob() [Fact] public void ShouldThrowWhenDuplicateJobNamesDuringRegistration() { - var act = () => ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression("* * * * *").WithName("JobName") + var act = () => ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression(AtEveryMinute).WithName("JobName") .And.WithCronExpression("*/2 * * * *").WithName("JobName"))); act.ShouldThrow(); @@ -316,12 +322,12 @@ public void ShouldThrowWhenDuplicateJobNamesDuringRegistration() [Fact] public void ShouldThrowRuntimeExceptionWithDuplicateJob() { - ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression("* * * * *").WithName("JobName"))); + ServiceCollection.AddNCronJob(s => s.AddJob(p => p.WithCronExpression(AtEveryMinute).WithName("JobName"))); var runtimeJobRegistry = CreateServiceProvider().GetRequiredService(); var successful = runtimeJobRegistry.TryRegister(s => s.AddJob(() => { - }, "* * * * *", jobName: "JobName"), out var exception); + }, AtEveryMinute, jobName: "JobName"), out var exception); successful.ShouldBeFalse(); exception.ShouldNotBeNull(); @@ -333,9 +339,9 @@ public void TryRegisteringShouldIndicateFailureWithAGivenException() { ServiceCollection.AddNCronJob(); var runtimeJobRegistry = CreateServiceProvider().GetRequiredService(); - runtimeJobRegistry.TryRegister(s => s.AddJob(p => p.WithCronExpression("* * * * *"))); + runtimeJobRegistry.TryRegister(s => s.AddJob(p => p.WithCronExpression(AtEveryMinute))); - var successful = runtimeJobRegistry.TryRegister(s => s.AddJob(p => p.WithCronExpression("* * * * *")), out var exception); + var successful = runtimeJobRegistry.TryRegister(s => s.AddJob(p => p.WithCronExpression(AtEveryMinute)), out var exception); successful.ShouldBeFalse(); exception.ShouldNotBeNull(); @@ -347,7 +353,7 @@ public void TryRegisterShouldIndicateSuccess() ServiceCollection.AddNCronJob(); var runtimeJobRegistry = CreateServiceProvider().GetRequiredService(); - var successful = runtimeJobRegistry.TryRegister(s => s.AddJob(p => p.WithCronExpression("* * * * *")), out var exception); + var successful = runtimeJobRegistry.TryRegister(s => s.AddJob(p => p.WithCronExpression(AtEveryMinute)), out var exception); successful.ShouldBeTrue(); exception.ShouldBeNull(); From bbd8eee99dffcef7f00fc3c2dffe2df2fa373d4e Mon Sep 17 00:00:00 2001 From: nulltoken Date: Wed, 1 Jan 2025 15:58:50 +0100 Subject: [PATCH 7/7] Slightly increase test coverage --- tests/NCronJob.Tests/RuntimeJobRegistryTests.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs b/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs index 215667f1..b49795ce 100644 --- a/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs +++ b/tests/NCronJob.Tests/RuntimeJobRegistryTests.cs @@ -57,6 +57,22 @@ public async Task CanRemoveJobByName() jobFinished.ShouldBeFalse(); } + [Fact] + public async Task DoesNotCringeWhenRemovingNonExistingJobs() + { + ServiceCollection.AddNCronJob(); + + var provider = CreateServiceProvider(); + await provider.GetRequiredService().StartAsync(CancellationToken); + var registry = provider.GetRequiredService(); + + var jobRegistry = provider.GetRequiredService(); + Assert.Empty(jobRegistry.GetAllJobs()); + + registry.RemoveJob("Nope"); + registry.RemoveJob(); + } + [Fact] public async Task CanRemoveByJobType() {