From 7c95bd6796208e914c7abe98aaf6c8ba3cf254cb Mon Sep 17 00:00:00 2001 From: maliming Date: Fri, 20 Mar 2026 21:01:03 +0800 Subject: [PATCH] Fix dynamic background job code review issues --- .../DefaultDynamicBackgroundJobManager.cs | 42 +++++++++++++++---- .../DynamicBackgroundJobArgs.cs | 2 +- .../BackgroundJobManager_Tests.cs | 23 ++++++---- .../DynamicJobExecutionTracker.cs | 4 +- 4 files changed, 51 insertions(+), 20 deletions(-) diff --git a/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DefaultDynamicBackgroundJobManager.cs b/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DefaultDynamicBackgroundJobManager.cs index 93f72788fe..121c9819cc 100644 --- a/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DefaultDynamicBackgroundJobManager.cs +++ b/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DefaultDynamicBackgroundJobManager.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Concurrent; using System.Linq; +using System.Linq.Expressions; using System.Reflection; using System.Threading.Tasks; using Microsoft.Extensions.Options; @@ -11,7 +12,7 @@ namespace Volo.Abp.BackgroundJobs; public class DefaultDynamicBackgroundJobManager : IDynamicBackgroundJobManager, ITransientDependency { - private static readonly ConcurrentDictionary EnqueueMethodCache = new(); + private static readonly ConcurrentDictionary>> EnqueueDelegateCache = new(); protected IBackgroundJobManager BackgroundJobManager { get; } protected IDynamicBackgroundJobHandlerRegistry HandlerRegistry { get; } @@ -83,9 +84,8 @@ public class DefaultDynamicBackgroundJobManager : IDynamicBackgroundJobManager, var json = JsonSerializer.Serialize(args); var typedArgs = JsonSerializer.Deserialize(argsType, json); - var enqueueMethod = GetOrCreateEnqueueMethod(argsType); - var task = (Task)enqueueMethod.Invoke(BackgroundJobManager, [typedArgs, priority, delay])!; - return await task; + var enqueueDelegate = GetOrCreateEnqueueDelegate(argsType); + return await enqueueDelegate(BackgroundJobManager, typedArgs, priority, delay); } protected virtual Task EnqueueDynamicHandlerJobAsync( @@ -99,15 +99,39 @@ public class DefaultDynamicBackgroundJobManager : IDynamicBackgroundJobManager, return BackgroundJobManager.EnqueueAsync(dynamicArgs, priority, delay); } - private static MethodInfo GetOrCreateEnqueueMethod(Type argsType) + private static Func> GetOrCreateEnqueueDelegate(Type argsType) { - return EnqueueMethodCache.GetOrAdd(argsType, static type => + return EnqueueDelegateCache.GetOrAdd(argsType, static type => { var method = typeof(IBackgroundJobManager) .GetMethods(BindingFlags.Public | BindingFlags.Instance) - .Single(m => m.Name == nameof(IBackgroundJobManager.EnqueueAsync) && m.IsGenericMethodDefinition); - - return method.MakeGenericMethod(type); + .FirstOrDefault(m => m.Name == nameof(IBackgroundJobManager.EnqueueAsync) + && m.IsGenericMethodDefinition + && m.GetParameters().Length == 3); + + if (method == null) + { + throw new AbpException( + $"Could not find the generic EnqueueAsync method on {nameof(IBackgroundJobManager)}."); + } + + var genericMethod = method.MakeGenericMethod(type); + + // Build: (manager, args, priority, delay) => manager.EnqueueAsync((TArgs)args, priority, delay) + var managerParam = Expression.Parameter(typeof(IBackgroundJobManager), "manager"); + var argsParam = Expression.Parameter(typeof(object), "args"); + var priorityParam = Expression.Parameter(typeof(BackgroundJobPriority), "priority"); + var delayParam = Expression.Parameter(typeof(TimeSpan?), "delay"); + + var call = Expression.Call( + managerParam, + genericMethod, + Expression.Convert(argsParam, type), + priorityParam, + delayParam); + + return Expression.Lambda>>( + call, managerParam, argsParam, priorityParam, delayParam).Compile(); }); } } diff --git a/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobArgs.cs b/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobArgs.cs index 9eaf2b00b2..e10f789889 100644 --- a/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobArgs.cs +++ b/framework/src/Volo.Abp.BackgroundJobs.Abstractions/Volo/Abp/BackgroundJobs/DynamicBackgroundJobArgs.cs @@ -12,6 +12,6 @@ public class DynamicBackgroundJobArgs public DynamicBackgroundJobArgs(string jobName, string jsonData) { JobName = Check.NotNullOrWhiteSpace(jobName, nameof(jobName)); - JsonData = Check.NotNull(jsonData, nameof(jsonData)); + JsonData = Check.NotNullOrWhiteSpace(jsonData, nameof(jsonData)); } } diff --git a/framework/test/Volo.Abp.BackgroundJobs.Tests/Volo/Abp/BackgroundJobs/BackgroundJobManager_Tests.cs b/framework/test/Volo.Abp.BackgroundJobs.Tests/Volo/Abp/BackgroundJobs/BackgroundJobManager_Tests.cs index 10981af5a4..8183d49c26 100644 --- a/framework/test/Volo.Abp.BackgroundJobs.Tests/Volo/Abp/BackgroundJobs/BackgroundJobManager_Tests.cs +++ b/framework/test/Volo.Abp.BackgroundJobs.Tests/Volo/Abp/BackgroundJobs/BackgroundJobManager_Tests.cs @@ -81,7 +81,7 @@ public class BackgroundJobManager_Tests : BackgroundJobsTestBase [Fact] public async Task Should_Execute_Dynamic_Handler_Job() { - _tracker.ExecutedJsonData.ShouldBeEmpty(); + _tracker.ExecutedJsonData.IsEmpty.ShouldBeTrue(); await _backgroundJobExecuter.ExecuteAsync( new JobExecutionContext( @@ -100,13 +100,20 @@ public class BackgroundJobManager_Tests : BackgroundJobsTestBase var typedJobName = BackgroundJobNameAttribute.GetName(); _dynamicBackgroundJobManager.RegisterHandler(typedJobName, (_, _) => Task.CompletedTask); - var jobIdAsString = await _dynamicBackgroundJobManager.EnqueueAsync(typedJobName, new { Value = "42" }); - jobIdAsString.ShouldNotBe(default); - - var jobInfo = await _backgroundJobStore.FindAsync(Guid.Parse(jobIdAsString)); - jobInfo.ShouldNotBeNull(); - jobInfo.JobName.ShouldBe(typedJobName); - jobInfo.JobName.ShouldNotBe(DynamicBackgroundJobArgs.JobNameConstant); + try + { + var jobIdAsString = await _dynamicBackgroundJobManager.EnqueueAsync(typedJobName, new { Value = "42" }); + jobIdAsString.ShouldNotBe(default); + + var jobInfo = await _backgroundJobStore.FindAsync(Guid.Parse(jobIdAsString)); + jobInfo.ShouldNotBeNull(); + jobInfo.JobName.ShouldBe(typedJobName); + jobInfo.JobName.ShouldNotBe(DynamicBackgroundJobArgs.JobNameConstant); + } + finally + { + _dynamicBackgroundJobManager.UnregisterHandler(typedJobName); + } } [Fact] diff --git a/framework/test/Volo.Abp.BackgroundJobs.Tests/Volo/Abp/BackgroundJobs/DynamicJobExecutionTracker.cs b/framework/test/Volo.Abp.BackgroundJobs.Tests/Volo/Abp/BackgroundJobs/DynamicJobExecutionTracker.cs index 1885d39c2d..3e496cdddd 100644 --- a/framework/test/Volo.Abp.BackgroundJobs.Tests/Volo/Abp/BackgroundJobs/DynamicJobExecutionTracker.cs +++ b/framework/test/Volo.Abp.BackgroundJobs.Tests/Volo/Abp/BackgroundJobs/DynamicJobExecutionTracker.cs @@ -1,8 +1,8 @@ -using System.Collections.Generic; +using System.Collections.Concurrent; namespace Volo.Abp.BackgroundJobs; public class DynamicJobExecutionTracker { - public List ExecutedJsonData { get; } = new(); + public ConcurrentBag ExecutedJsonData { get; } = new(); }