From f5439bdcefe15b35ed5e3aab8189a0e7dd6fd125 Mon Sep 17 00:00:00 2001 From: kk Date: Wed, 15 Apr 2026 12:04:43 +0200 Subject: [PATCH 1/5] added check before deleting quartz job --- .../Handlers/QuartzDeleteJobHandler.cs | 19 +++++++++++++++++++ .../Jobs/ResumeWorkflowJob.cs | 3 ++- .../Jobs/RunWorkflowJob.cs | 5 +++-- 3 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 src/modules/scheduling/Elsa.Scheduling.Quartz/Handlers/QuartzDeleteJobHandler.cs diff --git a/src/modules/scheduling/Elsa.Scheduling.Quartz/Handlers/QuartzDeleteJobHandler.cs b/src/modules/scheduling/Elsa.Scheduling.Quartz/Handlers/QuartzDeleteJobHandler.cs new file mode 100644 index 00000000..7d9b6b19 --- /dev/null +++ b/src/modules/scheduling/Elsa.Scheduling.Quartz/Handlers/QuartzDeleteJobHandler.cs @@ -0,0 +1,19 @@ +using Elsa.Scheduling.Quartz.Jobs; +using Quartz; + +namespace Elsa.Scheduling.Quartz.Handlers; + +public static class QuartzDeleteJobHandler +{ + public static async Task DeleteJob(this IJobExecutionContext context, JobKey jobKey, CancellationToken cancellationToken = default) + { + if (IsJobAllowedToBeDeleted(jobKey.Name)) + await context.Scheduler.DeleteJob(jobKey, cancellationToken); + } + + private static bool IsJobAllowedToBeDeleted(string jobName) + { + return jobName != nameof(ResumeWorkflowJob) + && jobName != nameof(RunWorkflowJob); + } +} \ No newline at end of file diff --git a/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/ResumeWorkflowJob.cs b/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/ResumeWorkflowJob.cs index 139fabfd..57070d16 100644 --- a/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/ResumeWorkflowJob.cs +++ b/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/ResumeWorkflowJob.cs @@ -3,6 +3,7 @@ using Elsa.Extensions; using Elsa.Resilience; using Elsa.Scheduling.Quartz.Contracts; +using Elsa.Scheduling.Quartz.Handlers; using Elsa.Workflows.Models; using Elsa.Workflows.Runtime; using Elsa.Workflows.Runtime.Messages; @@ -57,7 +58,7 @@ public async Task Execute(IJobExecutionContext context) catch (Exception e) { logger.LogError(e, "An error occurred while resuming workflow instance {WorkflowInstanceId}", workflowInstanceId); - await context.Scheduler.DeleteJob(context.JobDetail.Key, cancellationToken); + await context.DeleteJob(context.JobDetail.Key, cancellationToken); } } } diff --git a/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/RunWorkflowJob.cs b/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/RunWorkflowJob.cs index 3db56cf2..ef2788b6 100644 --- a/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/RunWorkflowJob.cs +++ b/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/RunWorkflowJob.cs @@ -2,6 +2,7 @@ using Elsa.Extensions; using Elsa.Resilience; using Elsa.Scheduling.Quartz.Contracts; +using Elsa.Scheduling.Quartz.Handlers; using Elsa.Workflows.Models; using Elsa.Workflows.Runtime; using Elsa.Workflows.Runtime.Exceptions; @@ -58,7 +59,7 @@ public async Task Execute(IJobExecutionContext context) catch (WorkflowGraphNotFoundException e) { logger.LogWarning(e, "Could not find workflow graph for workflow definition handle {WorkflowDefinitionHandle}", startRequest.WorkflowDefinitionHandle); - await context.Scheduler.DeleteJob(context.JobDetail.Key, cancellationToken); + await context.DeleteJob(context.JobDetail.Key, cancellationToken); } catch (Exception e) when (transientExceptionDetector.IsTransient(e)) { @@ -68,7 +69,7 @@ public async Task Execute(IJobExecutionContext context) catch (Exception e) { logger.LogError(e, "An error occurred while starting workflow {WorkflowDefinitionHandle} with correlation ID {CorrelationId}", startRequest.WorkflowDefinitionHandle, startRequest.CorrelationId); - await context.Scheduler.DeleteJob(context.JobDetail.Key, cancellationToken); + await context.DeleteJob(context.JobDetail.Key, cancellationToken); } } } From ffc1bf427b5fc19f3fc4dfa5996df25d1d729b08 Mon Sep 17 00:00:00 2001 From: kk Date: Wed, 15 Apr 2026 12:35:42 +0200 Subject: [PATCH 2/5] added doc --- .../Handlers/QuartzDeleteJobHandler.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/modules/scheduling/Elsa.Scheduling.Quartz/Handlers/QuartzDeleteJobHandler.cs b/src/modules/scheduling/Elsa.Scheduling.Quartz/Handlers/QuartzDeleteJobHandler.cs index 7d9b6b19..84d2c631 100644 --- a/src/modules/scheduling/Elsa.Scheduling.Quartz/Handlers/QuartzDeleteJobHandler.cs +++ b/src/modules/scheduling/Elsa.Scheduling.Quartz/Handlers/QuartzDeleteJobHandler.cs @@ -3,14 +3,28 @@ namespace Elsa.Scheduling.Quartz.Handlers; +/// +/// A helper to delete Quartz jobs and prevent the required ones from doing so +/// public static class QuartzDeleteJobHandler { + /// + /// Executes delete job if allowed + /// + /// The Quartz job execution context. + /// The Quartz job key. + /// The cancellation token. public static async Task DeleteJob(this IJobExecutionContext context, JobKey jobKey, CancellationToken cancellationToken = default) { if (IsJobAllowedToBeDeleted(jobKey.Name)) await context.Scheduler.DeleteJob(jobKey, cancellationToken); } + /// + /// Checks if the job is allowed to be deleted by name + /// + /// Name of the job to check + /// False if the job is one of the required ones, otherwise true private static bool IsJobAllowedToBeDeleted(string jobName) { return jobName != nameof(ResumeWorkflowJob) From 889aaf41c5a67075b380997120d02c16fe092ac2 Mon Sep 17 00:00:00 2001 From: kk Date: Wed, 15 Apr 2026 13:04:29 +0200 Subject: [PATCH 3/5] refactored code, moved DeleteJob extension into JobExecutionExtension class and changed behaviour for WorklfowGraphNotFoundException --- .../Extensions/JobExecutionExtensions.cs | 24 ++++++++++++++ .../Handlers/QuartzDeleteJobHandler.cs | 33 ------------------- .../Jobs/ResumeWorkflowJob.cs | 1 - .../Jobs/RunWorkflowJob.cs | 3 +- 4 files changed, 25 insertions(+), 36 deletions(-) delete mode 100644 src/modules/scheduling/Elsa.Scheduling.Quartz/Handlers/QuartzDeleteJobHandler.cs diff --git a/src/modules/scheduling/Elsa.Scheduling.Quartz/Extensions/JobExecutionExtensions.cs b/src/modules/scheduling/Elsa.Scheduling.Quartz/Extensions/JobExecutionExtensions.cs index c0b3f099..4f9aa497 100644 --- a/src/modules/scheduling/Elsa.Scheduling.Quartz/Extensions/JobExecutionExtensions.cs +++ b/src/modules/scheduling/Elsa.Scheduling.Quartz/Extensions/JobExecutionExtensions.cs @@ -1,4 +1,5 @@ using Elsa.Common.Multitenancy; +using Elsa.Scheduling.Quartz.Jobs; using Quartz; namespace Elsa.Scheduling.Quartz; @@ -18,4 +19,27 @@ internal static class JobExecutionExtensions return await tenantFinder.FindByIdAsync(tenantId, context.CancellationToken); } + + /// + /// Executes delete job if allowed + /// + /// The Quartz job execution context. + /// The Quartz job key. + /// The cancellation token. + public static async Task DeleteJob(this IJobExecutionContext context, JobKey jobKey, CancellationToken cancellationToken = default) + { + if (IsJobAllowedToBeDeleted(jobKey.Name)) + await context.Scheduler.DeleteJob(jobKey, cancellationToken); + } + + /// + /// Checks if the job is allowed to be deleted by name + /// + /// Name of the job to check + /// False if the job is one of the required ones, otherwise true + private static bool IsJobAllowedToBeDeleted(string jobName) + { + return jobName != nameof(ResumeWorkflowJob) + && jobName != nameof(RunWorkflowJob); + } } \ No newline at end of file diff --git a/src/modules/scheduling/Elsa.Scheduling.Quartz/Handlers/QuartzDeleteJobHandler.cs b/src/modules/scheduling/Elsa.Scheduling.Quartz/Handlers/QuartzDeleteJobHandler.cs deleted file mode 100644 index 84d2c631..00000000 --- a/src/modules/scheduling/Elsa.Scheduling.Quartz/Handlers/QuartzDeleteJobHandler.cs +++ /dev/null @@ -1,33 +0,0 @@ -using Elsa.Scheduling.Quartz.Jobs; -using Quartz; - -namespace Elsa.Scheduling.Quartz.Handlers; - -/// -/// A helper to delete Quartz jobs and prevent the required ones from doing so -/// -public static class QuartzDeleteJobHandler -{ - /// - /// Executes delete job if allowed - /// - /// The Quartz job execution context. - /// The Quartz job key. - /// The cancellation token. - public static async Task DeleteJob(this IJobExecutionContext context, JobKey jobKey, CancellationToken cancellationToken = default) - { - if (IsJobAllowedToBeDeleted(jobKey.Name)) - await context.Scheduler.DeleteJob(jobKey, cancellationToken); - } - - /// - /// Checks if the job is allowed to be deleted by name - /// - /// Name of the job to check - /// False if the job is one of the required ones, otherwise true - private static bool IsJobAllowedToBeDeleted(string jobName) - { - return jobName != nameof(ResumeWorkflowJob) - && jobName != nameof(RunWorkflowJob); - } -} \ No newline at end of file diff --git a/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/ResumeWorkflowJob.cs b/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/ResumeWorkflowJob.cs index 57070d16..afb93bd2 100644 --- a/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/ResumeWorkflowJob.cs +++ b/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/ResumeWorkflowJob.cs @@ -3,7 +3,6 @@ using Elsa.Extensions; using Elsa.Resilience; using Elsa.Scheduling.Quartz.Contracts; -using Elsa.Scheduling.Quartz.Handlers; using Elsa.Workflows.Models; using Elsa.Workflows.Runtime; using Elsa.Workflows.Runtime.Messages; diff --git a/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/RunWorkflowJob.cs b/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/RunWorkflowJob.cs index ef2788b6..60ab3056 100644 --- a/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/RunWorkflowJob.cs +++ b/src/modules/scheduling/Elsa.Scheduling.Quartz/Jobs/RunWorkflowJob.cs @@ -2,7 +2,6 @@ using Elsa.Extensions; using Elsa.Resilience; using Elsa.Scheduling.Quartz.Contracts; -using Elsa.Scheduling.Quartz.Handlers; using Elsa.Workflows.Models; using Elsa.Workflows.Runtime; using Elsa.Workflows.Runtime.Exceptions; @@ -59,7 +58,7 @@ public async Task Execute(IJobExecutionContext context) catch (WorkflowGraphNotFoundException e) { logger.LogWarning(e, "Could not find workflow graph for workflow definition handle {WorkflowDefinitionHandle}", startRequest.WorkflowDefinitionHandle); - await context.DeleteJob(context.JobDetail.Key, cancellationToken); + await context.Scheduler.UnscheduleJob(context.Trigger.Key, cancellationToken); } catch (Exception e) when (transientExceptionDetector.IsTransient(e)) { From dadddbba43fc7bc554bc6bdd2d234426c25fa160 Mon Sep 17 00:00:00 2001 From: kk Date: Wed, 15 Apr 2026 15:20:01 +0200 Subject: [PATCH 4/5] adjusted unit tests --- .../Helpers/QuartzJobTestHelper.cs | 6 ++++++ .../Jobs/RunWorkflowJobTests.cs | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Helpers/QuartzJobTestHelper.cs b/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Helpers/QuartzJobTestHelper.cs index 346d42be..1c3141da 100644 --- a/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Helpers/QuartzJobTestHelper.cs +++ b/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Helpers/QuartzJobTestHelper.cs @@ -130,6 +130,12 @@ public void VerifyRescheduled() => /// public void VerifyDeleted() => scheduler.Verify(s => s.DeleteJob(It.IsAny(), It.IsAny()), Times.Once); + + /// + /// Verifies that the scheduler unscheduled a job exactly once. + /// + public void VerifyUnscheduled() => + scheduler.Verify(s => s.UnscheduleJob(It.IsAny(), It.IsAny()), Times.Once); } /// diff --git a/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/RunWorkflowJobTests.cs b/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/RunWorkflowJobTests.cs index 6ab6a553..05c6b1cd 100644 --- a/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/RunWorkflowJobTests.cs +++ b/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/RunWorkflowJobTests.cs @@ -65,7 +65,7 @@ public async Task Execute_WorkflowGraphNotFound_DeletesJob() await _job.Execute(context); - scheduler.VerifyDeleted(); + scheduler.VerifyUnscheduled(); } [Theory] From 763e2f3c701b94fd96c879c5adde69ce0acd6968 Mon Sep 17 00:00:00 2001 From: kk Date: Thu, 16 Apr 2026 07:59:26 +0200 Subject: [PATCH 5/5] added new test case, if job key name actually is RunWorkflowJob or ResumeWorkflowJob --- .../Helpers/QuartzJobTestHelper.cs | 11 ++++++++-- .../Jobs/ResumeWorkflowJobTests.cs | 20 ++++++++++++------- .../Jobs/RunWorkflowJobTests.cs | 19 +++++++++++------- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Helpers/QuartzJobTestHelper.cs b/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Helpers/QuartzJobTestHelper.cs index 1c3141da..7828315a 100644 --- a/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Helpers/QuartzJobTestHelper.cs +++ b/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Helpers/QuartzJobTestHelper.cs @@ -20,10 +20,11 @@ public static class QuartzJobTestHelper /// Creates a mock job execution context with the specified job data. /// public static (IJobExecutionContext Context, Mock Scheduler) CreateJobExecutionContext( - IDictionary jobData) + IDictionary jobData, + string? jobKeyName = null) { var jobDataMap = new JobDataMap(jobData); - var jobKey = new JobKey("test-job"); + var jobKey = new JobKey(jobKeyName ?? "test-job"); var triggerKey = new TriggerKey("test-trigger"); var jobDetail = new Mock(); @@ -131,6 +132,12 @@ public void VerifyRescheduled() => public void VerifyDeleted() => scheduler.Verify(s => s.DeleteJob(It.IsAny(), It.IsAny()), Times.Once); + /// + /// Verifies that the scheduler did not delete a job. + /// + public void VerifyNotDeleted() => + scheduler.Verify(s => s.DeleteJob(It.IsAny(), It.IsAny()), Times.Never); + /// /// Verifies that the scheduler unscheduled a job exactly once. /// diff --git a/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/ResumeWorkflowJobTests.cs b/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/ResumeWorkflowJobTests.cs index d28d93d1..490ed75e 100644 --- a/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/ResumeWorkflowJobTests.cs +++ b/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/ResumeWorkflowJobTests.cs @@ -65,17 +65,22 @@ public async Task Execute_TransientException_ReschedulesJob(Type exceptionType, } [Theory] - [InlineData(typeof(InvalidOperationException))] - [InlineData(typeof(ArgumentException))] - public async Task Execute_NonTransientException_DeletesJob(Type exceptionType) + [InlineData(typeof(InvalidOperationException), null)] + [InlineData(typeof(ArgumentException), null)] + [InlineData(typeof(InvalidOperationException), "ResumeWorkflowJob")] + [InlineData(typeof(ArgumentException), "ResumeWorkflowJob")] + public async Task Execute_NonTransientException_DeletesJob(Type exceptionType, string? jobKeyName) { - var (context, scheduler) = CreateJobExecutionContext(); + var (context, scheduler) = CreateJobExecutionContext(jobKeyName: jobKeyName); _transientDetector.SetupIsTransient(false); _workflowRuntime.SetupCreateClientThrows((Exception)Activator.CreateInstance(exceptionType)!); await _job.Execute(context); - scheduler.VerifyDeleted(); + if (string.IsNullOrEmpty(jobKeyName)) + scheduler.VerifyDeleted(); + else + scheduler.VerifyNotDeleted(); } [Fact] @@ -121,7 +126,8 @@ public async Task Execute_WithActivityHandle_DeserializesCorrectly() Assert.Equal("activity-123", capturedRequest.ActivityHandle?.ActivityId); } - private static (IJobExecutionContext, Mock) CreateJobExecutionContext(string? activityHandle = null) + private static (IJobExecutionContext, Mock) CreateJobExecutionContext(string? activityHandle = null, + string? jobKeyName = null) { var jobData = new Dictionary { @@ -132,6 +138,6 @@ private static (IJobExecutionContext, Mock) CreateJobExecutionC if (activityHandle != null) jobData.Add("ActivityHandle", activityHandle); - return QuartzJobTestHelper.CreateJobExecutionContext(jobData); + return QuartzJobTestHelper.CreateJobExecutionContext(jobData, jobKeyName); } } diff --git a/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/RunWorkflowJobTests.cs b/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/RunWorkflowJobTests.cs index 05c6b1cd..9c0562b3 100644 --- a/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/RunWorkflowJobTests.cs +++ b/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/RunWorkflowJobTests.cs @@ -83,17 +83,22 @@ public async Task Execute_TransientException_ReschedulesJob(Type exceptionType, } [Theory] - [InlineData(typeof(InvalidOperationException))] - [InlineData(typeof(ArgumentException))] - public async Task Execute_NonTransientException_DeletesJob(Type exceptionType) + [InlineData(typeof(InvalidOperationException), null)] + [InlineData(typeof(ArgumentException), null)] + [InlineData(typeof(InvalidOperationException), "RunWorkflowJob")] + [InlineData(typeof(ArgumentException), "RunWorkflowJob")] + public async Task Execute_NonTransientException_DeletesJob(Type exceptionType, string? jobKeyName) { - var (context, scheduler) = CreateJobExecutionContext(); + var (context, scheduler) = CreateJobExecutionContext(jobKeyName); _transientDetector.SetupIsTransient(false); _workflowStarter.SetupStartWorkflowThrows((Exception)Activator.CreateInstance(exceptionType)!); await _job.Execute(context); - scheduler.VerifyDeleted(); + if (string.IsNullOrEmpty(jobKeyName)) + scheduler.VerifyDeleted(); + else + scheduler.VerifyNotDeleted(); } [Fact] @@ -111,11 +116,11 @@ public async Task Execute_UsesCorrectWorkflowDefinitionHandle() Assert.Equal("workflow-def-123", capturedRequest.WorkflowDefinitionHandle.DefinitionVersionId); } - private static (IJobExecutionContext, Mock) CreateJobExecutionContext() => + private static (IJobExecutionContext, Mock) CreateJobExecutionContext(string? jobKeyName = null) => QuartzJobTestHelper.CreateJobExecutionContext(new Dictionary { { "DefinitionVersionId", "workflow-def-123" }, { "CorrelationId", "corr-123" }, { "TriggerActivityId", "trigger-123" } - }); + }, jobKeyName); }