diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Elsa.Scheduling.Quartz.csproj b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Elsa.Scheduling.Quartz.csproj index c0bf9abe73..acbcebf1f5 100644 --- a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Elsa.Scheduling.Quartz.csproj +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Elsa.Scheduling.Quartz.csproj @@ -30,4 +30,8 @@ + + + + diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Extensions/JobExecutionExtensions.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Extensions/JobExecutionExtensions.cs index 3849a52898..1520b086d4 100644 --- a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Extensions/JobExecutionExtensions.cs +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Extensions/JobExecutionExtensions.cs @@ -2,6 +2,7 @@ using Elsa.Common.Multitenancy; using Elsa.Scheduling.Quartz.Contracts; using Elsa.Scheduling.Quartz.Jobs; +using Microsoft.Extensions.Logging; using Quartz; namespace Elsa.Scheduling.Quartz; @@ -124,18 +125,63 @@ string stringValue when long.TryParse(stringValue, NumberStyles.Integer, Culture }; } - public static async Task GetTenantAsync(this IJobExecutionContext context, ITenantFinder tenantFinder) + /// + /// Fails the job when trigger or job data has a TenantId but the tenant in context + /// is missing or different. The job must never fall through to default-tenant stores. + /// + public static void EnsureScheduledTenant(this IJobExecutionContext context, ITenantAccessor tenantAccessor, ILogger logger) + { + var tenantId = ReadTenantId(context); + if (tenantId is null) + return; + + var currentTenantId = tenantAccessor.Tenant?.Id; + if (string.Equals(currentTenantId, tenantId, StringComparison.Ordinal)) + return; + + logger.LogError( + "Quartz job {JobKey} is scheduled for tenant {TenantId} but the tenant in context is {CurrentTenant}. TenantAwareJobFactory is not installed, or a custom IJobFactory did not resolve the job inside the trigger's tenant. The job will not fall through to the default tenant.", + context.JobDetail.Key, + tenantId, + currentTenantId ?? "null"); + + throw new InvalidOperationException( + $"Quartz job '{context.JobDetail.Key}' is scheduled for tenant '{tenantId}' but the tenant in context is '{currentTenantId ?? "null"}'. TenantAwareJobFactory is not installed, or a custom IJobFactory did not resolve the job inside the trigger's tenant."); + } + + private static string? ReadTenantId(IJobExecutionContext context) + { + if (TryGetTenantId(context.Trigger.JobDataMap, out var tenantId)) + return tenantId; + + if (TryGetTenantId(context.JobDetail.JobDataMap, out tenantId)) + return tenantId; + + if (TryGetTenantId(context.MergedJobDataMap, out tenantId)) + return tenantId; + + return null; + } + + private static bool TryGetTenantId(JobDataMap? map, out string tenantId) { - if(!context.MergedJobDataMap.ContainsKey("TenantId")) - return null; - - if(!context.MergedJobDataMap.TryGetString("TenantId", out var tenantId)) - return null; - - if (string.IsNullOrWhiteSpace(tenantId)) - return null; - - return await tenantFinder.FindByIdAsync(tenantId, context.CancellationToken); + tenantId = ""; + if (map is null) + return false; + + if (map.TryGetString("TenantId", out var value) && !string.IsNullOrWhiteSpace(value)) + { + tenantId = value; + return true; + } + + if (map.TryGetValue("TenantId", out var obj) && obj is string stringValue && !string.IsNullOrWhiteSpace(stringValue)) + { + tenantId = stringValue; + return true; + } + + return false; } /// diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Extensions/ModuleExtensions.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Extensions/ModuleExtensions.cs index 6344cb7cc2..1137e37cd8 100644 --- a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Extensions/ModuleExtensions.cs +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Extensions/ModuleExtensions.cs @@ -22,6 +22,10 @@ public static IModule UseQuartz(this IModule module, Action? conf /// /// Installs a Quartz.NET implementation for . + /// Registers a tenant-aware Quartz job factory when the container has no host-provided + /// IJobFactory. Quartz's own default MicrosoftDependencyInjectionJobFactory + /// is treated as replaceable. Hosts that register their own factory must resolve jobs + /// inside the trigger's tenant, or the job fails instead of using the default tenant. /// public static SchedulingFeature UseQuartzScheduler(this SchedulingFeature feature, Action? configure = null) { @@ -31,6 +35,8 @@ public static SchedulingFeature UseQuartzScheduler(this SchedulingFeature featur /// /// Installs a Quartz.NET implementation for . + /// Registers a tenant-aware Quartz job factory when the container has no host-provided + /// IJobFactory. Quartz's own default factory is treated as replaceable. /// public static WorkflowRuntimeFeature UseQuartzBackgroundActivityScheduler(this WorkflowRuntimeFeature feature, Action? configure = null) { diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Features/QuartzBackgroundActivitySchedulerFeature.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Features/QuartzBackgroundActivitySchedulerFeature.cs index fe5e6c625b..cb11ebe214 100644 --- a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Features/QuartzBackgroundActivitySchedulerFeature.cs +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Features/QuartzBackgroundActivitySchedulerFeature.cs @@ -34,5 +34,6 @@ public override void Configure() public override void Apply() { Services.AddSingleton(); + JobFactoryRegistration.TryUseTenantAwareJobFactory(Services); } } diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Features/QuartzFeature.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Features/QuartzFeature.cs index 32dcb7bfda..697b7fdc84 100644 --- a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Features/QuartzFeature.cs +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Features/QuartzFeature.cs @@ -1,5 +1,6 @@ using Elsa.Features.Abstractions; using Elsa.Features.Services; +using Elsa.Scheduling.Quartz.Services; using Microsoft.Extensions.DependencyInjection; using Quartz; @@ -124,6 +125,8 @@ public override void Apply() Services .AddQuartz(configure => { ConfigureQuartzInternal(configure, ConfigureQuartz); }); + + JobFactoryRegistration.TryUseTenantAwareJobFactory(Services); } private static void ConfigureQuartzInternal(IServiceCollectionQuartzConfigurator quartz, Action? configureQuartz) diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Features/QuartzSchedulerFeature.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Features/QuartzSchedulerFeature.cs index 3cb7b8657a..059a03d0c4 100644 --- a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Features/QuartzSchedulerFeature.cs +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Features/QuartzSchedulerFeature.cs @@ -62,6 +62,7 @@ public override void Apply() .AddStartupTask() .AddQuartz(); + JobFactoryRegistration.TryUseTenantAwareJobFactory(Services); Services.Configure(_configureQuartzJobOptions); } } diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Jobs/ExecuteBackgroundActivityJob.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Jobs/ExecuteBackgroundActivityJob.cs index 46cd34680a..33f3855319 100644 --- a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Jobs/ExecuteBackgroundActivityJob.cs +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Jobs/ExecuteBackgroundActivityJob.cs @@ -1,4 +1,5 @@ using Elsa.Common.Multitenancy; +using Elsa.Scheduling.Quartz; using Elsa.Workflows.Runtime; using JetBrains.Annotations; using Microsoft.Extensions.Logging; @@ -8,6 +9,9 @@ namespace Elsa.Scheduling.Quartz.Jobs; /// /// A job that executes a background activity. +/// Tenant context is established by the Quartz job factory. +/// verifies that the scheduled tenant is already in context; +/// is retained for binary compatibility (lookup happens in the factory). /// [UsedImplicitly] public class ExecuteBackgroundActivityJob( @@ -19,23 +23,22 @@ public class ExecuteBackgroundActivityJob( /// public async Task Execute(IJobExecutionContext context) { + _ = tenantFinder; var cancellationToken = context.CancellationToken; string? workflowInstanceId = null; try { - var tenant = await context.GetTenantAsync(tenantFinder); - using (tenantAccessor.PushContext(tenant)) - { - var map = context.MergedJobDataMap; - workflowInstanceId = (string?)map.Get(nameof(ScheduledBackgroundActivity.WorkflowInstanceId)); - var scheduledBackgroundActivity = new ScheduledBackgroundActivity( - workflowInstanceId!, - (string)map.Get(nameof(ScheduledBackgroundActivity.ActivityNodeId)), - (string)map.Get(nameof(ScheduledBackgroundActivity.BookmarkId))); + context.EnsureScheduledTenant(tenantAccessor, logger); - await backgroundActivityInvoker.ExecuteAsync(scheduledBackgroundActivity, cancellationToken); - } + var map = context.MergedJobDataMap; + workflowInstanceId = (string?)map.Get(nameof(ScheduledBackgroundActivity.WorkflowInstanceId)); + var scheduledBackgroundActivity = new ScheduledBackgroundActivity( + workflowInstanceId!, + (string)map.Get(nameof(ScheduledBackgroundActivity.ActivityNodeId)), + (string)map.Get(nameof(ScheduledBackgroundActivity.BookmarkId))); + + await backgroundActivityInvoker.ExecuteAsync(scheduledBackgroundActivity, cancellationToken); } catch (Exception e) { diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Jobs/ResumeWorkflowJob.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Jobs/ResumeWorkflowJob.cs index e91a8dc181..214f7d39a5 100644 --- a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Jobs/ResumeWorkflowJob.cs +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Jobs/ResumeWorkflowJob.cs @@ -1,6 +1,7 @@ using Elsa.Common; using Elsa.Common.Multitenancy; using Elsa.Extensions; +using Elsa.Scheduling.Quartz; using Elsa.Scheduling.Quartz.Contracts; using Elsa.Workflows.Models; using Elsa.Workflows.Runtime; @@ -13,6 +14,9 @@ namespace Elsa.Scheduling.Quartz.Jobs; /// /// A job that resumes a workflow. +/// Tenant context is established by the Quartz job factory. +/// verifies that the scheduled tenant is already in context; +/// is retained for binary compatibility (lookup happens in the factory). /// public class ResumeWorkflowJob( IWorkflowRuntime workflowRuntime, @@ -26,30 +30,29 @@ public class ResumeWorkflowJob( /// public async Task Execute(IJobExecutionContext context) { + _ = tenantFinder; + context.EnsureScheduledTenant(tenantAccessor, logger); + var cancellationToken = context.CancellationToken; string? workflowInstanceId = null; try { - var tenant = await context.GetTenantAsync(tenantFinder); - using (tenantAccessor.PushContext(tenant)) - { - var map = context.MergedJobDataMap; - var serializedActivityHandle = (string)map.Get(nameof(ScheduleExistingWorkflowInstanceRequest.ActivityHandle)); - var activityHandle = serializedActivityHandle != null! ? jsonSerializer.Deserialize(serializedActivityHandle) : null; - workflowInstanceId = (string)map.Get(nameof(ScheduleExistingWorkflowInstanceRequest.WorkflowInstanceId)); + var map = context.MergedJobDataMap; + var serializedActivityHandle = (string)map.Get(nameof(ScheduleExistingWorkflowInstanceRequest.ActivityHandle)); + var activityHandle = serializedActivityHandle != null! ? jsonSerializer.Deserialize(serializedActivityHandle) : null; + workflowInstanceId = (string)map.Get(nameof(ScheduleExistingWorkflowInstanceRequest.WorkflowInstanceId)); - var workflowClient = await workflowRuntime.CreateClientAsync(workflowInstanceId, cancellationToken); - var request = new RunWorkflowInstanceRequest - { - BookmarkId = (string)map.Get(nameof(ScheduleExistingWorkflowInstanceRequest.BookmarkId)), - ActivityHandle = activityHandle, - Input = map.GetDictionary(nameof(ScheduleExistingWorkflowInstanceRequest.Input)), - Properties = map.GetDictionary(nameof(ScheduleExistingWorkflowInstanceRequest.Properties)), - }; - await workflowClient.RunInstanceAsync(request, cancellationToken: cancellationToken); + var workflowClient = await workflowRuntime.CreateClientAsync(workflowInstanceId, cancellationToken); + var request = new RunWorkflowInstanceRequest + { + BookmarkId = (string)map.Get(nameof(ScheduleExistingWorkflowInstanceRequest.BookmarkId)), + ActivityHandle = activityHandle, + Input = map.GetDictionary(nameof(ScheduleExistingWorkflowInstanceRequest.Input)), + Properties = map.GetDictionary(nameof(ScheduleExistingWorkflowInstanceRequest.Properties)), + }; + await workflowClient.RunInstanceAsync(request, cancellationToken: cancellationToken); - logger.LogInformation("Resumed workflow instance {WorkflowInstanceId}", workflowInstanceId); - } + logger.LogInformation("Resumed workflow instance {WorkflowInstanceId}", workflowInstanceId); } catch (WorkflowGraphNotFoundException e) { diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Jobs/RunWorkflowJob.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Jobs/RunWorkflowJob.cs index 43334e45bf..19dec7bf93 100644 --- a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Jobs/RunWorkflowJob.cs +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Jobs/RunWorkflowJob.cs @@ -1,5 +1,6 @@ using Elsa.Common.Multitenancy; using Elsa.Extensions; +using Elsa.Scheduling.Quartz; using Elsa.Scheduling.Quartz.Contracts; using Elsa.Workflows.Models; using Elsa.Workflows.Runtime; @@ -12,6 +13,9 @@ namespace Elsa.Scheduling.Quartz.Jobs; /// /// A job that runs a workflow. +/// Tenant context is established by the Quartz job factory. +/// verifies that the scheduled tenant is already in context; +/// is retained for binary compatibility (lookup happens in the factory). /// [UsedImplicitly] public class RunWorkflowJob( @@ -25,36 +29,35 @@ public class RunWorkflowJob( /// public async Task Execute(IJobExecutionContext context) { + _ = tenantFinder; + context.EnsureScheduledTenant(tenantAccessor, logger); + var cancellationToken = context.CancellationToken; StartWorkflowRequest? startRequest = null; try { - var tenant = await context.GetTenantAsync(tenantFinder); - using (tenantAccessor.PushContext(tenant)) - { - var map = context.MergedJobDataMap; + var map = context.MergedJobDataMap; - startRequest = new StartWorkflowRequest - { - WorkflowDefinitionHandle = WorkflowDefinitionHandle.ByDefinitionVersionId((string)map.Get(nameof(ScheduleNewWorkflowInstanceRequest.WorkflowDefinitionHandle.DefinitionVersionId))), - CorrelationId = (string?)map.Get(nameof(ScheduleNewWorkflowInstanceRequest.CorrelationId)), - TriggerActivityId = (string?)map.Get(nameof(ScheduleNewWorkflowInstanceRequest.TriggerActivityId)), - Input = map.GetDictionary(nameof(ScheduleNewWorkflowInstanceRequest.Input)), - Variables = map.GetDictionary(nameof(ScheduleNewWorkflowInstanceRequest.Variables)), - Properties = map.GetDictionary(nameof(ScheduleNewWorkflowInstanceRequest.Properties)), - ParentId = (string?)map.Get(nameof(ScheduleNewWorkflowInstanceRequest.ParentId)) - }; - - var startResponse = await workflowStarter.StartWorkflowAsync(startRequest, cancellationToken); + startRequest = new StartWorkflowRequest + { + WorkflowDefinitionHandle = WorkflowDefinitionHandle.ByDefinitionVersionId((string)map.Get(nameof(ScheduleNewWorkflowInstanceRequest.WorkflowDefinitionHandle.DefinitionVersionId))), + CorrelationId = (string?)map.Get(nameof(ScheduleNewWorkflowInstanceRequest.CorrelationId)), + TriggerActivityId = (string?)map.Get(nameof(ScheduleNewWorkflowInstanceRequest.TriggerActivityId)), + Input = map.GetDictionary(nameof(ScheduleNewWorkflowInstanceRequest.Input)), + Variables = map.GetDictionary(nameof(ScheduleNewWorkflowInstanceRequest.Variables)), + Properties = map.GetDictionary(nameof(ScheduleNewWorkflowInstanceRequest.Properties)), + ParentId = (string?)map.Get(nameof(ScheduleNewWorkflowInstanceRequest.ParentId)) + }; - if (startResponse.CannotStart) - { - logger.LogWarning("Workflow activation strategy disallowed starting workflow {WorkflowDefinitionHandle} with correlation ID {CorrelationId}", startRequest.WorkflowDefinitionHandle, startRequest.CorrelationId); - return; - } + var startResponse = await workflowStarter.StartWorkflowAsync(startRequest, cancellationToken); - logger.LogInformation("Started workflow {WorkflowInstanceId} with correlation ID {CorrelationId}", startResponse.WorkflowInstanceId, startRequest.CorrelationId); + if (startResponse.CannotStart) + { + logger.LogWarning("Workflow activation strategy disallowed starting workflow {WorkflowDefinitionHandle} with correlation ID {CorrelationId}", startRequest.WorkflowDefinitionHandle, startRequest.CorrelationId); + return; } + + logger.LogInformation("Started workflow {WorkflowInstanceId} with correlation ID {CorrelationId}", startResponse.WorkflowInstanceId, startRequest.CorrelationId); } catch (WorkflowGraphNotFoundException e) { diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Services/JobFactoryRegistration.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Services/JobFactoryRegistration.cs new file mode 100644 index 0000000000..cda9e39e5d --- /dev/null +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Services/JobFactoryRegistration.cs @@ -0,0 +1,54 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Quartz.Simpl; +using Quartz.Spi; + +namespace Elsa.Scheduling.Quartz.Services; + +/// +/// Registers once per container when the host has not supplied its own +/// . Quartz 3.15.1's AddQuartz default +/// ( via TryAddSingleton) is treated as replaceable. +/// +internal static class JobFactoryRegistration +{ + /// + /// Replaces Quartz's default factory with unless a host custom + /// is already registered. No-ops when this factory is already present. + /// Safe to call from every Quartz feature, in any order relative to AddQuartz. + /// + public static void TryUseTenantAwareJobFactory(IServiceCollection services) + { + if (HasHostJobFactory(services)) + return; + + if (services.Any(IsTenantAwareJobFactory)) + return; + + services.Replace(ServiceDescriptor.Singleton(typeof(IJobFactory), typeof(TenantAwareJobFactory))); + } + + private static bool HasHostJobFactory(IServiceCollection services) => + services.Any(descriptor => descriptor.ServiceType == typeof(IJobFactory) && IsHostJobFactory(descriptor)); + + private static bool IsTenantAwareJobFactory(ServiceDescriptor descriptor) => + descriptor.ServiceType == typeof(IJobFactory) && GetImplementationType(descriptor) == typeof(TenantAwareJobFactory); + + /// + /// Quartz 3.15.1 registers its default as + /// TryAddSingleton(typeof(IJobFactory), typeof(MicrosoftDependencyInjectionJobFactory)) + /// (ImplementationType set, no factory delegate). UseJobFactory<T> uses the same shape with + /// Replace. Anything else is a host custom factory and must be kept. + /// + private static bool IsHostJobFactory(ServiceDescriptor descriptor) + { + var implementationType = GetImplementationType(descriptor); + if (implementationType == typeof(MicrosoftDependencyInjectionJobFactory) || implementationType == typeof(TenantAwareJobFactory)) + return false; + + return implementationType is not null || descriptor.ImplementationFactory is not null || descriptor.ImplementationInstance is not null; + } + + private static Type? GetImplementationType(ServiceDescriptor descriptor) => + descriptor.ImplementationType ?? descriptor.ImplementationInstance?.GetType(); +} diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/Services/TenantAwareJobFactory.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Services/TenantAwareJobFactory.cs new file mode 100644 index 0000000000..ef27900811 --- /dev/null +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/Services/TenantAwareJobFactory.cs @@ -0,0 +1,119 @@ +using System.Runtime.ExceptionServices; +using Elsa.Common.Multitenancy; +using Elsa.Scheduling.Quartz; +using Elsa.Scheduling.Quartz.Contracts; +using Elsa.Scheduling.Quartz.Jobs; +using JetBrains.Annotations; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Quartz; +using Quartz.Simpl; +using Quartz.Spi; +using QuartzScheduler = Quartz.IScheduler; + +namespace Elsa.Scheduling.Quartz.Services; + +/// +/// Resolves Quartz jobs inside a DI scope that already has the trigger's tenant pushed, +/// so constructor dependencies bind to that tenant instead of the default tenant. +/// +[UsedImplicitly] +internal sealed class TenantAwareJobFactory( + IServiceProvider serviceProvider, + IOptions options, + ILogger logger) + : MicrosoftDependencyInjectionJobFactory(serviceProvider, options) +{ + private const string TenantIdKey = "TenantId"; + + protected override IJob InstantiateJob(TriggerFiredBundle bundle, QuartzScheduler scheduler) + { + var tenantId = ReadTenantId(bundle); + if (tenantId is null) + return base.InstantiateJob(bundle, scheduler); + + Tenant? tenant; + try + { + tenant = serviceProvider.GetRequiredService().FindByIdAsync(tenantId).GetAwaiter().GetResult(); + } + catch (Exception exception) + { + logger.LogWarning(exception, "Could not find tenant {TenantId} while building Quartz job {JobKey}.", tenantId, bundle.JobDetail.Key); + return new TenantLookupFailedJob(exception, serviceProvider, logger, bundle.JobDetail.JobType); + } + + if (tenant is null) + { + logger.LogError("Could not find tenant {TenantId} while building Quartz job {JobKey}. Quartz will put this job's triggers in the ERROR state.", tenantId, bundle.JobDetail.Key); + throw new InvalidOperationException($"Could not find tenant '{tenantId}' while building Quartz job '{bundle.JobDetail.Key}'. Quartz will put this job's triggers in the ERROR state."); + } + + var accessor = serviceProvider.GetRequiredService(); + + using (accessor.PushContext(tenant)) + return new TenantJob(base.InstantiateJob(bundle, scheduler), accessor, tenant); + } + + private static string? ReadTenantId(TriggerFiredBundle bundle) + { + if (TryGetTenantId(bundle.Trigger.JobDataMap, out var tenantId)) + return tenantId; + + if (TryGetTenantId(bundle.JobDetail.JobDataMap, out tenantId)) + return tenantId; + + return null; + } + + private static bool TryGetTenantId(JobDataMap map, out string tenantId) + { + tenantId = ""; + + if (!map.TryGetString(TenantIdKey, out var value) || string.IsNullOrWhiteSpace(value)) + return false; + + tenantId = value; + return true; + } + + /// + /// Not on purpose: Quartz unwraps and async-disposes the base scope. + /// + private sealed class TenantJob(IJob inner, ITenantAccessor accessor, Tenant tenant) : IJob, IJobWrapper + { + public IJob Target => inner; + + public async Task Execute(IJobExecutionContext context) + { + using (accessor.PushContext(tenant)) + await inner.Execute(context); + } + } + + private sealed class TenantLookupFailedJob(Exception exception, IServiceProvider serviceProvider, ILogger logger, Type jobType) : IJob + { + public async Task Execute(IJobExecutionContext context) + { + if (jobType == typeof(ExecuteBackgroundActivityJob)) + { + logger.LogError(exception, "An error occurred while executing a background activity job {JobKey} after a tenant lookup failure.", context.JobDetail.Key); + await context.DeleteJob(context.JobDetail.Key, context.CancellationToken); + ExceptionDispatchInfo.Capture(exception).Throw(); + } + + var retryScheduler = serviceProvider.GetService(); + if (retryScheduler is not null) + { + if (retryScheduler.IsRetryable(exception) && await retryScheduler.ScheduleRetryAsync(context, exception, context.CancellationToken)) + return; + + logger.LogError(exception, "No retry was scheduled for job {JobKey} after a tenant lookup failure.", context.JobDetail.Key); + return; + } + + ExceptionDispatchInfo.Capture(exception).Throw(); + } + } +} diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzBackgroundActivitySchedulerShellFeature.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzBackgroundActivitySchedulerShellFeature.cs index ce64ebbb4a..949a4e9ba4 100644 --- a/extensions/src/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzBackgroundActivitySchedulerShellFeature.cs +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzBackgroundActivitySchedulerShellFeature.cs @@ -23,5 +23,6 @@ public void ConfigureServices(IServiceCollection services) services.AddSingleton(); services.RemoveAll(); services.AddSingleton(sp => sp.GetRequiredService()); + JobFactoryRegistration.TryUseTenantAwareJobFactory(services); } } diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzFeature.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzFeature.cs index 860812c15e..987bb7fd7f 100644 --- a/extensions/src/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzFeature.cs +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzFeature.cs @@ -1,6 +1,7 @@ using CShells.Features; using CShells.Lifecycle; using Elsa.Platform.PackageManifest.Generator.Hints; +using Elsa.Scheduling.Quartz.Services; using JetBrains.Annotations; using Microsoft.Extensions.DependencyInjection; using Quartz; @@ -66,6 +67,8 @@ public void ConfigureServices(IServiceCollection services) quartz.SchedulerName = SchedulerName; }); + JobFactoryRegistration.TryUseTenantAwareJobFactory(services); + // QuartzShellLifecycleHandler owns start/stop — no AddQuartzHostedService needed. // The Quartz hosted service is only invoked by the root IHost, which never sees // shell-scoped registrations, so registering it would have no effect. diff --git a/extensions/src/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzSchedulerFeature.cs b/extensions/src/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzSchedulerFeature.cs index 5e56d49d6c..99dcee9d98 100644 --- a/extensions/src/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzSchedulerFeature.cs +++ b/extensions/src/scheduling/Elsa.Scheduling.Quartz/ShellFeatures/QuartzSchedulerFeature.cs @@ -40,5 +40,7 @@ public void ConfigureServices(IServiceCollection services) .AddScoped() .AddSingleton() .AddQuartz(); + + JobFactoryRegistration.TryUseTenantAwareJobFactory(services); } } diff --git a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Features/QuartzBackgroundActivitySchedulerFeatureTests.cs b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Features/QuartzBackgroundActivitySchedulerFeatureTests.cs index a58a043a70..2bd2bacf8b 100644 --- a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Features/QuartzBackgroundActivitySchedulerFeatureTests.cs +++ b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Features/QuartzBackgroundActivitySchedulerFeatureTests.cs @@ -53,6 +53,9 @@ public void Apply_RegistersQuartzBackgroundActivityScheduler() Assert.Contains(services, d => d.ServiceType == typeof(QuartzBackgroundActivityScheduler) && d.Lifetime == ServiceLifetime.Singleton); + Assert.Contains(services, d => + d.ServiceType == typeof(global::Quartz.Spi.IJobFactory) && + d.ImplementationType == typeof(TenantAwareJobFactory)); } [Fact] diff --git a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Features/QuartzFeatureRegistrationTests.cs b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Features/QuartzFeatureRegistrationTests.cs new file mode 100644 index 0000000000..302ee5547c --- /dev/null +++ b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Features/QuartzFeatureRegistrationTests.cs @@ -0,0 +1,155 @@ +using Elsa.Extensions; +using Elsa.Features.Services; +using Elsa.Scheduling.Quartz.Features; +using Elsa.Scheduling.Quartz.Services; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Moq; +using Quartz; +using Quartz.Simpl; +using Quartz.Spi; + +namespace Elsa.Scheduling.Quartz.UnitTests.Features; + +public class QuartzFeatureRegistrationTests +{ + [Fact] + public void Apply_RegistersTenantAwareJobFactory() + { + var services = new ServiceCollection(); + var module = new Mock(); + module.Setup(m => m.Services).Returns(services); + + new QuartzFeature(module.Object).Apply(); + + AssertSingleTenantAwareFactory(services); + } + + [Fact] + public void SchedulerFeature_Apply_RegistersTenantAwareJobFactory() + { + var services = new ServiceCollection(); + var module = new Mock(); + module.Setup(m => m.Services).Returns(services); + + new QuartzSchedulerFeature(module.Object).Apply(); + + AssertSingleTenantAwareFactory(services); + } + + [Fact] + public void BackgroundActivitySchedulerFeature_Apply_RegistersTenantAwareJobFactory() + { + var services = new ServiceCollection(); + var module = new Mock(); + module.Setup(m => m.Services).Returns(services); + + new QuartzBackgroundActivitySchedulerFeature(module.Object).Apply(); + + AssertSingleTenantAwareFactory(services); + } + + [Fact] + public void Apply_QuartzThenScheduler_RegistersFactoryOnce() + { + var services = new ServiceCollection(); + var module = new Mock(); + module.Setup(m => m.Services).Returns(services); + + new QuartzFeature(module.Object).Apply(); + new QuartzSchedulerFeature(module.Object).Apply(); + new QuartzBackgroundActivitySchedulerFeature(module.Object).Apply(); + + AssertSingleTenantAwareFactory(services); + } + + [Fact] + public void TryUse_ReplacesQuartzDefaultFactory() + { + var services = new ServiceCollection(); + services.AddQuartz(); + + Assert.Contains(services, descriptor => + descriptor.ServiceType == typeof(IJobFactory) && + descriptor.ImplementationType == typeof(MicrosoftDependencyInjectionJobFactory)); + + JobFactoryRegistration.TryUseTenantAwareJobFactory(services); + + AssertSingleTenantAwareFactory(services); + } + + [Fact] + public void TryUse_KeepsHostTypedJobFactory() + { + var services = new ServiceCollection(); + services.AddSingleton(); + + JobFactoryRegistration.TryUseTenantAwareJobFactory(services); + new QuartzSchedulerFeature(MockModule(services)).Apply(); + + var factory = Assert.Single(services, descriptor => descriptor.ServiceType == typeof(IJobFactory)); + Assert.Equal(typeof(HostJobFactory), factory.ImplementationType); + } + + [Fact] + public void TryUse_KeepsHostFactoryRegisteredAfterAddQuartz() + { + var services = new ServiceCollection(); + services.AddQuartz(); + services.AddSingleton(); + + JobFactoryRegistration.TryUseTenantAwareJobFactory(services); + + Assert.Contains(services, descriptor => + descriptor.ServiceType == typeof(IJobFactory) && + descriptor.ImplementationType == typeof(HostJobFactory)); + Assert.DoesNotContain(services, descriptor => + descriptor.ServiceType == typeof(IJobFactory) && + descriptor.ImplementationType == typeof(TenantAwareJobFactory)); + } + + [Fact] + public void TryUse_KeepsHostFactoryDelegate() + { + var services = new ServiceCollection(); + services.AddSingleton(_ => new HostJobFactory()); + + JobFactoryRegistration.TryUseTenantAwareJobFactory(services); + + var factory = Assert.Single(services, descriptor => descriptor.ServiceType == typeof(IJobFactory)); + Assert.NotNull(factory.ImplementationFactory); + Assert.Null(factory.ImplementationType); + } + + [Fact] + public void TryUse_IsIdempotentAcrossFeatureOrder() + { + var services = new ServiceCollection(); + JobFactoryRegistration.TryUseTenantAwareJobFactory(services); + services.AddQuartz(); + JobFactoryRegistration.TryUseTenantAwareJobFactory(services); + + AssertSingleTenantAwareFactory(services); + } + + private static IModule MockModule(IServiceCollection services) + { + var module = new Mock(); + module.Setup(m => m.Services).Returns(services); + return module.Object; + } + + private static void AssertSingleTenantAwareFactory(IServiceCollection services) + { + var factory = Assert.Single(services, descriptor => descriptor.ServiceType == typeof(IJobFactory)); + Assert.Equal(typeof(TenantAwareJobFactory), factory.ImplementationType); + } + + private sealed class HostJobFactory : IJobFactory + { + public IJob NewJob(TriggerFiredBundle bundle, global::Quartz.IScheduler scheduler) => throw new NotSupportedException(); + public void ReturnJob(IJob job) + { + } + } +} diff --git a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Features/UseQuartzSchedulerTenantTests.cs b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Features/UseQuartzSchedulerTenantTests.cs new file mode 100644 index 0000000000..13d5615bde --- /dev/null +++ b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Features/UseQuartzSchedulerTenantTests.cs @@ -0,0 +1,144 @@ +using Elsa.Common.Multitenancy; +using Elsa.Extensions; +using Elsa.Scheduling.Quartz.Jobs; +using Elsa.Scheduling.Quartz.Services; +using Elsa.Scheduling.Quartz.UnitTests.Helpers; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Quartz; +using Quartz.Spi; + +namespace Elsa.Scheduling.Quartz.UnitTests.Features; + +public class UseQuartzSchedulerTenantTests +{ + [Fact] + public async Task UseQuartzScheduler_WithoutUseQuartz_TwoTenants_SeeOwnTenantAtConstructionAndExecute() + { + var probe = new TwoTenantProbe(); + var services = new ServiceCollection(); + services.AddLogging(); + services.AddSingleton(NullLoggerFactory.Instance); + services.AddSingleton(typeof(ILogger<>), typeof(NullLogger<>)); + services.AddSingleton(probe); + + var module = services.CreateModule(); + module.UseScheduling(scheduling => scheduling.UseQuartzScheduler()); + module.Apply(); + + services.RemoveAll(); + services.AddSingleton(); + + await using var provider = services.BuildServiceProvider(); + Assert.IsType(provider.GetRequiredService()); + + var scheduler = await provider.GetRequiredService().GetScheduler(); + await scheduler.Start(); + + try + { + await FireAndAssert(scheduler, probe, "tenant-a"); + probe.Reset(); + await FireAndAssert(scheduler, probe, "tenant-b"); + } + finally + { + await scheduler.Shutdown(waitForJobsToComplete: true); + } + } + + [Fact] + public async Task HostCustomJobFactory_IsKept_AndGuardThrowsWhenTenantMissingFromContext() + { + var services = new ServiceCollection(); + services.AddSingleton(); + var module = services.CreateModule(); + module.UseScheduling(scheduling => scheduling.UseQuartzScheduler()); + module.Apply(); + + var factory = Assert.Single(services, descriptor => descriptor.ServiceType == typeof(IJobFactory) && descriptor.ImplementationType == typeof(HostJobFactory)); + Assert.Equal(typeof(HostJobFactory), factory.ImplementationType); + Assert.DoesNotContain(services, descriptor => + descriptor.ServiceType == typeof(IJobFactory) && + descriptor.ImplementationType == typeof(TenantAwareJobFactory)); + + var starter = new Mock(); + var logger = new Mock>(); + var job = new RunWorkflowJob( + new DefaultTenantAccessor(), + new AnyIdTenantFinder(), + starter.Object, + Mock.Of(), + logger.Object); + var (context, _) = QuartzJobTestHelper.CreateJobExecutionContext( + new Dictionary { ["DefinitionVersionId"] = "def-1" }, + triggerData: new Dictionary { ["TenantId"] = "tenant-b" }); + + var exception = await Assert.ThrowsAsync(() => job.Execute(context)); + + Assert.Contains("tenant-b", exception.Message); + Assert.Contains(context.JobDetail.Key.ToString(), exception.Message); + starter.Verify(x => x.StartWorkflowAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + private static async Task FireAndAssert(global::Quartz.IScheduler scheduler, TwoTenantProbe probe, string tenantId) + { + var job = JobBuilder.Create().WithIdentity($"probe-{tenantId}").Build(); + var trigger = TriggerBuilder.Create() + .ForJob(job) + .UsingJobData("TenantId", tenantId) + .StartNow() + .Build(); + + await scheduler.ScheduleJob(job, trigger); + await probe.Done.Task.WaitAsync(TimeSpan.FromSeconds(20)); + + Assert.Equal(tenantId, probe.AtConstruction); + Assert.Equal(tenantId, probe.AtExecute); + } + + public sealed class TwoTenantProbe + { + public string? AtConstruction { get; set; } + public string? AtExecute { get; set; } + public TaskCompletionSource Done { get; private set; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public void Reset() + { + AtConstruction = null; + AtExecute = null; + Done = new(TaskCreationOptions.RunContinuationsAsynchronously); + } + } + + public sealed class TwoTenantProbeJob : IJob + { + public TwoTenantProbeJob(ITenantAccessor accessor, TwoTenantProbe probe) + { + probe.AtConstruction = accessor.Tenant?.Id; + _accessor = accessor; + _probe = probe; + } + + private readonly ITenantAccessor _accessor; + private readonly TwoTenantProbe _probe; + + public Task Execute(IJobExecutionContext context) + { + _probe.AtExecute = _accessor.Tenant?.Id; + _probe.Done.TrySetResult(); + return Task.CompletedTask; + } + } + + private sealed class HostJobFactory : IJobFactory + { + public IJob NewJob(TriggerFiredBundle bundle, global::Quartz.IScheduler scheduler) => throw new NotSupportedException(); + public void ReturnJob(IJob job) + { + } + } +} diff --git a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Helpers/QuartzJobTestHelper.cs b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Helpers/QuartzJobTestHelper.cs index 353c60ddad..abca081a2a 100644 --- a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Helpers/QuartzJobTestHelper.cs +++ b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Helpers/QuartzJobTestHelper.cs @@ -83,16 +83,6 @@ public static QuartzJobOptions CreateQuartzJobOptions(Action? /// public static IOptions AsOptions(this QuartzJobOptions options) => Microsoft.Extensions.Options.Options.Create(options); - /// - /// Creates a mock tenant accessor that allows context pushing. - /// - public static Mock CreateTenantAccessor() - { - var tenantAccessor = new Mock(); - tenantAccessor.Setup(t => t.PushContext(It.IsAny())).Returns((IDisposable)null!); - return tenantAccessor; - } - extension(Mock workflowStarter) { /// @@ -220,3 +210,12 @@ public void VerifyRetryScheduled(IJobExecutionContext context) => retryScheduler.Verify(x => x.ScheduleRetryAsync(context, It.IsAny(), It.IsAny()), Times.Once); } } + +/// +/// Returns a tenant whose id matches the requested id. +/// +public sealed class AnyIdTenantFinder : ITenantFinder +{ + public Task FindByIdAsync(string tenantId, CancellationToken cancellationToken = default) => + Task.FromResult(new Tenant { Id = tenantId, Name = tenantId }); +} diff --git a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/ExecuteBackgroundActivityJobTests.cs b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/ExecuteBackgroundActivityJobTests.cs index 0eb2023441..aca1481efa 100644 --- a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/ExecuteBackgroundActivityJobTests.cs +++ b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/ExecuteBackgroundActivityJobTests.cs @@ -11,14 +11,12 @@ namespace Elsa.Scheduling.Quartz.UnitTests.Jobs; public class ExecuteBackgroundActivityJobTests { private readonly Mock _invoker = new(); - private readonly Mock _tenantFinder = new(); - private readonly Mock _tenantAccessor = QuartzJobTestHelper.CreateTenantAccessor(); private readonly Mock> _logger = new(); private readonly ExecuteBackgroundActivityJob _job; public ExecuteBackgroundActivityJobTests() { - _job = new(_invoker.Object, _tenantFinder.Object, _tenantAccessor.Object, _logger.Object); + _job = new(_invoker.Object, Mock.Of(), Mock.Of(), _logger.Object); } [Fact] @@ -36,22 +34,34 @@ public async Task Execute_InvokesBackgroundActivityAndDeletesJob() Assert.Equal("instance-1", captured.WorkflowInstanceId); Assert.Equal("node-1", captured.ActivityNodeId); Assert.Equal("bookmark-1", captured.BookmarkId); - _tenantAccessor.Verify(t => t.PushContext(null), Times.Once); scheduler.VerifyDeleted(); } [Fact] - public async Task Execute_ResolvesTenantBeforeInvoking() + public async Task Execute_WhenTriggerTenantMissingFromContext_ThrowsNamingTenantIdAndJobKey() { - var tenant = new Tenant { Id = "tenant-123" }; - _tenantFinder.Setup(f => f.FindByIdAsync("tenant-123", It.IsAny())).ReturnsAsync(tenant); - var (context, _) = CreateJobExecutionContext(withTenantId: true); + var logger = new Mock>(); + var job = new ExecuteBackgroundActivityJob( + _invoker.Object, + new AnyIdTenantFinder(), + new DefaultTenantAccessor(), + logger.Object); + var (context, scheduler) = QuartzJobTestHelper.CreateJobExecutionContext( + new Dictionary + { + [nameof(ScheduledBackgroundActivity.WorkflowInstanceId)] = "instance-1", + [nameof(ScheduledBackgroundActivity.ActivityNodeId)] = "node-1", + [nameof(ScheduledBackgroundActivity.BookmarkId)] = "bookmark-1" + }, + triggerData: new Dictionary { ["TenantId"] = "tenant-b" }); - await _job.Execute(context); + var exception = await Assert.ThrowsAsync(() => job.Execute(context)); - _tenantFinder.Verify(f => f.FindByIdAsync("tenant-123", It.IsAny()), Times.Once); - _tenantAccessor.Verify(t => t.PushContext(tenant), Times.Once); - _invoker.Verify(i => i.ExecuteAsync(It.IsAny(), It.IsAny()), Times.Once); + Assert.Contains("tenant-b", exception.Message); + Assert.Contains(context.JobDetail.Key.ToString(), exception.Message); + _invoker.Verify(i => i.ExecuteAsync(It.IsAny(), It.IsAny()), Times.Never); + logger.VerifyLogged(LogLevel.Error, Times.AtLeastOnce()); + scheduler.VerifyDeleted(); } [Fact] @@ -67,7 +77,7 @@ public async Task Execute_WhenInvokerFails_LogsDeletesAndRethrows() scheduler.VerifyDeleted(); } - private static (IJobExecutionContext Context, Mock Scheduler) CreateJobExecutionContext(bool withTenantId = false) + private static (IJobExecutionContext Context, Mock Scheduler) CreateJobExecutionContext() { var jobData = new Dictionary { @@ -76,9 +86,6 @@ private static (IJobExecutionContext Context, Mock Sc [nameof(ScheduledBackgroundActivity.BookmarkId)] = "bookmark-1" }; - if (withTenantId) - jobData["TenantId"] = "tenant-123"; - return QuartzJobTestHelper.CreateJobExecutionContext(jobData); } } diff --git a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/ResumeWorkflowJobTests.cs b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/ResumeWorkflowJobTests.cs index 417f269c6f..0709e60e20 100644 --- a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/ResumeWorkflowJobTests.cs +++ b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/ResumeWorkflowJobTests.cs @@ -18,8 +18,6 @@ public class ResumeWorkflowJobTests { private readonly Mock _workflowRuntime = new(); private readonly Mock _jsonSerializer = new(); - private readonly Mock _tenantFinder = new(); - private readonly Mock _tenantAccessor = QuartzJobTestHelper.CreateTenantAccessor(); private readonly Mock _retryScheduler = new(); private readonly Mock> _logger = new(); private readonly ResumeWorkflowJob _job; @@ -29,8 +27,8 @@ public ResumeWorkflowJobTests() _job = new( _workflowRuntime.Object, _jsonSerializer.Object, - _tenantFinder.Object, - _tenantAccessor.Object, + Mock.Of(), + Mock.Of(), _retryScheduler.Object, _logger.Object); } @@ -196,19 +194,6 @@ public async Task Execute_NonTransientException_DeletesJob(Type exceptionType, s scheduler.VerifyNotDeleted(); } - [Fact] - public async Task Execute_TenantFinderThrowsTransientException_SchedulesRetryAndDoesNotPropagate() - { - var (context, _) = CreateJobExecutionContext(withTenantId: true); - _retryScheduler.SetupRetry(isRetryable: true); - _tenantFinder.Setup(f => f.FindByIdAsync("tenant-123", It.IsAny())).ThrowsAsync(new TimeoutException()); - - await _job.Execute(context); - - _retryScheduler.VerifyRetryScheduled(context); - _workflowRuntime.Verify(r => r.CreateClientAsync(It.IsAny(), It.IsAny()), Times.Never); - } - [Fact] public async Task Execute_UsesCorrectWorkflowInstanceId() { @@ -227,6 +212,29 @@ public async Task Execute_UsesCorrectWorkflowInstanceId() Assert.Equal("workflow-instance-123", capturedInstanceId); } + [Fact] + public async Task Execute_WhenTriggerTenantMissingFromContext_ThrowsNamingTenantIdAndJobKey() + { + var logger = new Mock>(); + var job = new ResumeWorkflowJob( + _workflowRuntime.Object, + _jsonSerializer.Object, + new AnyIdTenantFinder(), + new DefaultTenantAccessor(), + _retryScheduler.Object, + logger.Object); + var (context, _) = QuartzJobTestHelper.CreateJobExecutionContext( + new Dictionary { ["WorkflowInstanceId"] = "wf-1" }, + triggerData: new Dictionary { ["TenantId"] = "tenant-b" }); + + var exception = await Assert.ThrowsAsync(() => job.Execute(context)); + + Assert.Contains("tenant-b", exception.Message); + Assert.Contains(context.JobDetail.Key.ToString(), exception.Message); + _workflowRuntime.Verify(r => r.CreateClientAsync(It.IsAny(), It.IsAny()), Times.Never); + logger.VerifyLogged(LogLevel.Error, Times.Once()); + } + [Fact] public async Task Execute_WithActivityHandle_DeserializesCorrectly() { @@ -253,7 +261,7 @@ public async Task Execute_WithActivityHandle_DeserializesCorrectly() } private static (IJobExecutionContext, Mock) CreateJobExecutionContext(string? activityHandle = null, - string? jobKeyName = null, bool withTenantId = false) + string? jobKeyName = null) { var jobData = new Dictionary { @@ -264,9 +272,6 @@ private static (IJobExecutionContext, Mock) CreateJobExecutionC if (activityHandle != null) jobData.Add("ActivityHandle", activityHandle); - if (withTenantId) - jobData["TenantId"] = "tenant-123"; - return QuartzJobTestHelper.CreateJobExecutionContext(jobData, jobKeyName); } } diff --git a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/RunWorkflowJobTests.cs b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/RunWorkflowJobTests.cs index 7b5594ac84..2de4cf3c38 100644 --- a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/RunWorkflowJobTests.cs +++ b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Jobs/RunWorkflowJobTests.cs @@ -14,8 +14,6 @@ namespace Elsa.Scheduling.Quartz.UnitTests.Jobs; public class RunWorkflowJobTests { - private readonly Mock _tenantAccessor = QuartzJobTestHelper.CreateTenantAccessor(); - private readonly Mock _tenantFinder = new(); private readonly Mock _workflowStarter = new(); private readonly Mock _retryScheduler = new(); private readonly Mock> _logger = new(); @@ -24,8 +22,8 @@ public class RunWorkflowJobTests public RunWorkflowJobTests() { _job = new( - _tenantAccessor.Object, - _tenantFinder.Object, + Mock.Of(), + Mock.Of(), _workflowStarter.Object, _retryScheduler.Object, _logger.Object); @@ -235,16 +233,58 @@ public async Task Execute_NonTransientException_DeletesJob(Type exceptionType, s } [Fact] - public async Task Execute_TenantFinderThrowsTransientException_SchedulesRetryAndDoesNotPropagate() + public async Task Execute_WhenTriggerTenantMissingFromContext_ThrowsNamingTenantIdAndJobKey() { - var (context, _) = CreateJobExecutionContext(withTenantId: true); - _retryScheduler.SetupRetry(isRetryable: true); - _tenantFinder.Setup(f => f.FindByIdAsync("tenant-123", It.IsAny())).ThrowsAsync(new TimeoutException()); + var logger = new Mock>(); + var job = new RunWorkflowJob( + new DefaultTenantAccessor(), + new AnyIdTenantFinder(), + _workflowStarter.Object, + _retryScheduler.Object, + logger.Object); + var (context, _) = QuartzJobTestHelper.CreateJobExecutionContext( + new Dictionary { ["DefinitionVersionId"] = "def-1" }, + triggerData: new Dictionary { ["TenantId"] = "tenant-b" }); - await _job.Execute(context); + var exception = await Assert.ThrowsAsync(() => job.Execute(context)); - _retryScheduler.VerifyRetryScheduled(context); - _workflowStarter.Verify(w => w.StartWorkflowAsync(It.IsAny(), It.IsAny()), Times.Never); + Assert.Contains("tenant-b", exception.Message); + Assert.Contains(context.JobDetail.Key.ToString(), exception.Message); + _workflowStarter.Verify(x => x.StartWorkflowAsync(It.IsAny(), It.IsAny()), Times.Never); + logger.VerifyLogged(LogLevel.Error, Times.Once()); + } + + [Fact] + public async Task Execute_WhenTriggerTenantAlreadyInContext_StartsWorkflow() + { + var accessor = new DefaultTenantAccessor(); + using var _ = accessor.PushContext(new Tenant { Id = "tenant-b", Name = "tenant-b" }); + var job = new RunWorkflowJob(accessor, new AnyIdTenantFinder(), _workflowStarter.Object, _retryScheduler.Object, _logger.Object); + _workflowStarter.SetupStartWorkflow(new() { WorkflowInstanceId = "workflow-123" }); + var (context, _) = QuartzJobTestHelper.CreateJobExecutionContext( + new Dictionary { ["DefinitionVersionId"] = "workflow-def-123" }, + triggerData: new Dictionary { ["TenantId"] = "tenant-b" }); + + await job.Execute(context); + + _workflowStarter.VerifyStartWorkflowCalled(); + } + + [Fact] + public async Task Execute_WhenContextTenantDiffersFromTrigger_ThrowsNamingTenantIdAndJobKey() + { + var accessor = new DefaultTenantAccessor(); + using var _ = accessor.PushContext(new Tenant { Id = "tenant-a", Name = "tenant-a" }); + var job = new RunWorkflowJob(accessor, new AnyIdTenantFinder(), _workflowStarter.Object, _retryScheduler.Object, _logger.Object); + var (context, _) = QuartzJobTestHelper.CreateJobExecutionContext( + new Dictionary { ["DefinitionVersionId"] = "def-1" }, + triggerData: new Dictionary { ["TenantId"] = "tenant-b" }); + + var exception = await Assert.ThrowsAsync(() => job.Execute(context)); + + Assert.Contains("tenant-b", exception.Message); + Assert.Contains(context.JobDetail.Key.ToString(), exception.Message); + _workflowStarter.Verify(x => x.StartWorkflowAsync(It.IsAny(), It.IsAny()), Times.Never); } [Fact] @@ -262,7 +302,7 @@ public async Task Execute_UsesCorrectWorkflowDefinitionHandle() Assert.Equal("workflow-def-123", capturedRequest.WorkflowDefinitionHandle.DefinitionVersionId); } - private static (IJobExecutionContext, Mock) CreateJobExecutionContext(string? jobKeyName = null, bool withTenantId = false) + private static (IJobExecutionContext, Mock) CreateJobExecutionContext(string? jobKeyName = null) { var jobData = new Dictionary { @@ -271,9 +311,6 @@ private static (IJobExecutionContext, Mock) CreateJobExecutionC { "TriggerActivityId", "trigger-123" } }; - if (withTenantId) - jobData["TenantId"] = "tenant-123"; - return QuartzJobTestHelper.CreateJobExecutionContext(jobData, jobKeyName); } } diff --git a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Services/TenantAwareJobFactoryTests.cs b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Services/TenantAwareJobFactoryTests.cs new file mode 100644 index 0000000000..44c366a8b3 --- /dev/null +++ b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/Services/TenantAwareJobFactoryTests.cs @@ -0,0 +1,373 @@ +using Elsa.Common.Multitenancy; +using Elsa.Scheduling.Quartz.Contracts; +using Elsa.Scheduling.Quartz.Jobs; +using Elsa.Scheduling.Quartz.Services; +using Elsa.Scheduling.Quartz.UnitTests.Helpers; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Moq; +using Quartz; +using Quartz.Spi; +using QuartzScheduler = Quartz.IScheduler; + +namespace Elsa.Scheduling.Quartz.UnitTests.Services; + +public class TenantAwareJobFactoryTests +{ + [Fact] + public void NewJob_TenantIdOnTrigger_ResolvesJobInThatTenantScope() + { + // Arrange + var harness = Harness.Create(); + harness.Tenants.Add("tenant-a"); + var bundle = CreateBundle(triggerTenantId: "tenant-a"); + + // Act + var job = harness.Factory.NewJob(bundle, Scheduler()); + + // Assert + Assert.Equal("tenant-a", harness.Capture.TenantIdAtConstruction); + Assert.Equal("tenant-a", Assert.Single(harness.Tenants.RequestedIds)); + harness.Factory.ReturnJob(job); + } + + [Fact] + public void NewJob_TenantIdOnJob_WhenTriggerHasNone_ResolvesJobInThatTenantScope() + { + // Arrange + var harness = Harness.Create(); + harness.Tenants.Add("tenant-from-job"); + var bundle = CreateBundle(jobTenantId: "tenant-from-job"); + + // Act + var job = harness.Factory.NewJob(bundle, Scheduler()); + + // Assert + Assert.Equal("tenant-from-job", harness.Capture.TenantIdAtConstruction); + Assert.Equal("tenant-from-job", Assert.Single(harness.Tenants.RequestedIds)); + harness.Factory.ReturnJob(job); + } + + [Fact] + public void NewJob_TenantIdOnTrigger_TakesPrecedenceOverJob() + { + // Arrange + var harness = Harness.Create(); + harness.Tenants.Add("tenant-from-trigger"); + harness.Tenants.Add("tenant-from-job"); + var bundle = CreateBundle(triggerTenantId: "tenant-from-trigger", jobTenantId: "tenant-from-job"); + + // Act + var job = harness.Factory.NewJob(bundle, Scheduler()); + + // Assert + Assert.Equal("tenant-from-trigger", harness.Capture.TenantIdAtConstruction); + Assert.Equal("tenant-from-trigger", Assert.Single(harness.Tenants.RequestedIds)); + harness.Factory.ReturnJob(job); + } + + [Fact] + public void NewJob_WhenTenantIsMissing_ThrowsNamingTenantIdJobKeyAndErrorState() + { + // Arrange + var harness = Harness.Create(); + var bundle = CreateBundle(triggerTenantId: "missing-tenant", jobName: "run-workflow"); + + // Act + var exception = Assert.Throws(() => harness.Factory.NewJob(bundle, Scheduler())); + + // Assert + Assert.Contains("missing-tenant", exception.Message); + Assert.Contains(bundle.JobDetail.Key.ToString(), exception.Message); + Assert.Contains("ERROR", exception.Message); + Assert.Null(harness.Capture.TenantIdAtConstruction); + Assert.Contains(harness.Logger.Messages, message => message.Contains("ERROR")); + } + + [Fact] + public async Task NewJob_WhenTenantLookupThrows_DefersFailureToExecuteRetryPath() + { + // Arrange + var harness = Harness.Create(); + var lookupFailure = new TimeoutException("tenant store down"); + harness.Tenants.ThrowOn("tenant-b", lookupFailure); + harness.RetryScheduler.Setup(x => x.IsRetryable(lookupFailure)).Returns(true); + harness.RetryScheduler.Setup(x => x.ScheduleRetryAsync(It.IsAny(), lookupFailure, It.IsAny())).ReturnsAsync(true); + var (context, _) = QuartzJobTestHelper.CreateJobExecutionContext(new Dictionary()); + var bundle = CreateBundle(triggerTenantId: "tenant-b"); + + // Act + var job = harness.Factory.NewJob(bundle, Scheduler()); + await job.Execute(context); + + // Assert + Assert.Null(harness.Capture.TenantIdAtConstruction); + harness.RetryScheduler.Verify(x => x.ScheduleRetryAsync(context, lookupFailure, It.IsAny()), Times.Once); + harness.Factory.ReturnJob(job); + } + + [Fact] + public async Task NewJob_WhenTenantLookupThrows_ForBackgroundActivityJob_DeletesJobAndDoesNotRetry() + { + var harness = Harness.Create(); + var lookupFailure = new TimeoutException("tenant store down"); + harness.Tenants.ThrowOn("tenant-b", lookupFailure); + harness.RetryScheduler.Setup(x => x.IsRetryable(lookupFailure)).Returns(true); + harness.RetryScheduler.Setup(x => x.ScheduleRetryAsync(It.IsAny(), lookupFailure, It.IsAny())).ReturnsAsync(true); + var (context, scheduler) = QuartzJobTestHelper.CreateJobExecutionContext(new Dictionary()); + var bundle = CreateBundle(triggerTenantId: "tenant-b", jobType: typeof(ExecuteBackgroundActivityJob)); + + var job = harness.Factory.NewJob(bundle, Scheduler()); + var thrown = await Assert.ThrowsAsync(() => job.Execute(context)); + + Assert.Same(lookupFailure, thrown); + scheduler.VerifyDeleted(); + harness.RetryScheduler.Verify(x => x.ScheduleRetryAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + harness.Factory.ReturnJob(job); + } + + [Fact] + public void NewJob_WhenNoTenantId_UsesDefaultTenantBehavior() + { + // Arrange + var harness = Harness.Create(); + var bundle = CreateBundle(); + + // Act + var job = harness.Factory.NewJob(bundle, Scheduler()); + + // Assert + Assert.Null(harness.Capture.TenantIdAtConstruction); + Assert.Equal("default", harness.Capture.ServiceTenantId); + Assert.Empty(harness.Tenants.RequestedIds); + harness.Factory.ReturnJob(job); + } + + [Fact] + public void NewJob_ForTenantB_ResolvesTenantBScopedServices() + { + // Arrange + var harness = Harness.Create(); + harness.Tenants.Add("tenant-a"); + harness.Tenants.Add("tenant-b"); + var tenantABundle = CreateBundle(triggerTenantId: "tenant-a", jobName: "job-a"); + var tenantBBundle = CreateBundle(triggerTenantId: "tenant-b", jobName: "job-b"); + + // Act + var tenantAJob = harness.Factory.NewJob(tenantABundle, Scheduler()); + var tenantAServiceId = harness.Capture.ServiceTenantId; + harness.Factory.ReturnJob(tenantAJob); + + var tenantBJob = harness.Factory.NewJob(tenantBBundle, Scheduler()); + var tenantBServiceId = harness.Capture.ServiceTenantId; + harness.Factory.ReturnJob(tenantBJob); + + // Assert + Assert.Equal("tenant-a", tenantAServiceId); + Assert.Equal("tenant-b", tenantBServiceId); + } + + [Fact] + public async Task RealScheduler_TenantVisibleDuringConstructionAndExecute() + { + // Arrange + var probe = new SchedulerProbe(); + var services = new ServiceCollection(); + services.AddSingleton(NullLoggerFactory.Instance); + services.AddSingleton(typeof(ILogger<>), typeof(NullLogger<>)); + services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(probe); + services.AddQuartz(quartz => quartz.UseJobFactory()); + + await using var provider = services.BuildServiceProvider(); + var scheduler = await provider.GetRequiredService().GetScheduler(); + await scheduler.Start(); + + var job = JobBuilder.Create().WithIdentity("probe").Build(); + var trigger = TriggerBuilder.Create() + .ForJob(job) + .UsingJobData("TenantId", "tenant-b") + .StartNow() + .Build(); + + // Act + await scheduler.ScheduleJob(job, trigger); + await probe.Done.Task.WaitAsync(TimeSpan.FromSeconds(20)); + await scheduler.Shutdown(waitForJobsToComplete: true); + + // Assert + Assert.Equal("tenant-b", probe.AtConstruction); + Assert.Equal("tenant-b", probe.AtExecute); + } + + private static QuartzScheduler Scheduler() => Mock.Of(s => s.Context == new SchedulerContext()); + + private static TriggerFiredBundle CreateBundle(string? triggerTenantId = null, string? jobTenantId = null, string jobName = "test-job", Type? jobType = null) + { + var jobBuilder = JobBuilder.Create(jobType ?? typeof(ProbeJob)).WithIdentity(jobName); + if (jobTenantId is not null) + jobBuilder.UsingJobData("TenantId", jobTenantId); + var job = jobBuilder.Build(); + + var triggerBuilder = TriggerBuilder.Create().ForJob(job).WithIdentity($"{jobName}-trigger"); + if (triggerTenantId is not null) + triggerBuilder.UsingJobData("TenantId", triggerTenantId); + var trigger = (IOperableTrigger)triggerBuilder.Build(); + + return new TriggerFiredBundle(job, trigger, null, false, DateTimeOffset.UtcNow, DateTimeOffset.UtcNow, null, null); + } + + private sealed class Harness + { + private Harness( + TenantAwareJobFactory factory, + FakeTenantFinder tenants, + ProbeCapture capture, + CapturingLogger logger, + Mock retryScheduler) + { + Factory = factory; + Tenants = tenants; + Capture = capture; + Logger = logger; + RetryScheduler = retryScheduler; + } + + public TenantAwareJobFactory Factory { get; } + public FakeTenantFinder Tenants { get; } + public ProbeCapture Capture { get; } + public CapturingLogger Logger { get; } + public Mock RetryScheduler { get; } + + public static Harness Create() + { + var tenants = new FakeTenantFinder(); + var capture = new ProbeCapture(); + var logger = new CapturingLogger(); + var retryScheduler = new Mock(); + var options = Microsoft.Extensions.Options.Options.Create(new QuartzOptions()); + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddSingleton(tenants); + services.AddSingleton(capture); + services.AddSingleton>(logger); + services.AddSingleton(options); + services.AddSingleton(retryScheduler.Object); + services.AddTransient(); + services.AddScoped(sp => + { + var tenantId = sp.GetRequiredService().Tenant?.Id ?? "default"; + return new TenantBoundService(tenantId); + }); + + var provider = services.BuildServiceProvider(); + var factory = new TenantAwareJobFactory(provider, options, logger); + + return new Harness(factory, tenants, capture, logger, retryScheduler); + } + } + + private sealed class FakeTenantFinder : ITenantFinder + { + private readonly Dictionary _tenants = new(); + private readonly Dictionary _failures = new(); + + public List RequestedIds { get; } = []; + + public void Add(string tenantId) => _tenants[tenantId] = new Tenant { Id = tenantId, Name = tenantId }; + + public void ThrowOn(string tenantId, Exception exception) => _failures[tenantId] = exception; + + public Task FindByIdAsync(string tenantId, CancellationToken cancellationToken = default) + { + RequestedIds.Add(tenantId); + if (_failures.TryGetValue(tenantId, out var exception)) + throw exception; + + return Task.FromResult(_tenants.TryGetValue(tenantId, out var tenant) ? tenant : null); + } + } + + private sealed class AnyIdTenantFinder : ITenantFinder + { + public Task FindByIdAsync(string tenantId, CancellationToken cancellationToken = default) => + Task.FromResult(new Tenant { Id = tenantId, Name = tenantId }); + } + + private sealed class ProbeCapture + { + public string? TenantIdAtConstruction { get; set; } + public string? ServiceTenantId { get; set; } + } + + public sealed class SchedulerProbe + { + public string? AtConstruction { get; set; } + public string? AtExecute { get; set; } + public TaskCompletionSource Done { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + } + + public sealed class SchedulerProbeJob : IJob + { + private readonly ITenantAccessor _accessor; + private readonly SchedulerProbe _probe; + + public SchedulerProbeJob(ITenantAccessor accessor, SchedulerProbe probe) + { + _accessor = accessor; + _probe = probe; + probe.AtConstruction = accessor.Tenant?.Id; + } + + public Task Execute(IJobExecutionContext context) + { + _probe.AtExecute = _accessor.Tenant?.Id; + _probe.Done.TrySetResult(); + return Task.CompletedTask; + } + } + + private sealed class CapturingLogger : ILogger + { + public List Messages { get; } = []; + + public IDisposable BeginScope(TState state) where TState : notnull => NullScope.Instance; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) => + Messages.Add(formatter(state, exception)); + + private sealed class NullScope : IDisposable + { + public static readonly NullScope Instance = new(); + public void Dispose() + { + } + } + } + + private interface ITenantBoundService + { + string TenantId { get; } + } + + private sealed class TenantBoundService(string tenantId) : ITenantBoundService + { + public string TenantId { get; } = tenantId; + } + + private sealed class ProbeJob : IJob + { + public ProbeJob(ITenantAccessor tenantAccessor, ITenantBoundService tenantBoundService, ProbeCapture capture) + { + capture.TenantIdAtConstruction = tenantAccessor.Tenant?.Id; + capture.ServiceTenantId = tenantBoundService.TenantId; + } + + public Task Execute(IJobExecutionContext context) => Task.CompletedTask; + } +} diff --git a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/ShellFeatures/QuartzFeatureTests.cs b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/ShellFeatures/QuartzFeatureTests.cs index 51399a8e97..1f427b23c4 100644 --- a/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/ShellFeatures/QuartzFeatureTests.cs +++ b/extensions/test/modules/scheduling/Elsa.Scheduling.Quartz.UnitTests/ShellFeatures/QuartzFeatureTests.cs @@ -4,14 +4,28 @@ using Elsa.Scheduling.Quartz.EFCore.PostgreSql.ShellFeatures; using Elsa.Scheduling.Quartz.EFCore.SqlServer.ShellFeatures; using Elsa.Scheduling.Quartz.EFCore.Sqlite.ShellFeatures; +using Elsa.Scheduling.Quartz.Services; using Elsa.Scheduling.Quartz.ShellFeatures; using Microsoft.Extensions.DependencyInjection; +using Quartz.Spi; using System.Reflection; namespace Elsa.Scheduling.Quartz.UnitTests.ShellFeatures; public class QuartzFeatureTests { + [Fact] + public void ConfigureServices_RegistersTenantAwareJobFactory() + { + var services = new ServiceCollection(); + + new QuartzFeature().ConfigureServices(services); + + Assert.Contains(services, descriptor => + descriptor.ServiceType == typeof(IJobFactory) && + descriptor.ImplementationType == typeof(TenantAwareJobFactory)); + } + [Fact] public void SchedulerInitializer_IsRegisteredDuringPostConfigure() {