Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bug] TokenCleanupBackgroundWorker will fail when enable Volo.Abp.BackgroundJobs.Quartz #4442

Closed
yinchang0626 opened this issue Jun 21, 2020 · 2 comments · Fixed by #4505
Closed
Assignees
Milestone

Comments

@yinchang0626
Copy link
Contributor

@liangshiw Hi:
Thank you for Volo.Abp.BackgroundWorkers.Quartz module to integrate Quartz,
but I think QuartzBackgroundWorkerManager lost some things :

[Dependency(ReplaceServices = true)]
public class QuartzBackgroundWorkerManager : IBackgroundWorkerManager
{
        public virtual void Add(IBackgroundWorker worker)
        {
            AsyncHelper.RunSync(() => ReScheduleJobAsync(worker));
        }

        protected virtual async Task ReScheduleJobAsync(IBackgroundWorker worker)
        {
            if (worker is IQuartzBackgroundWorker quartzWork)
            {
                Check.NotNull(quartzWork.Trigger, nameof(quartzWork.Trigger));
                Check.NotNull(quartzWork.JobDetail, nameof(quartzWork.JobDetail));

                if (quartzWork.ScheduleJob != null)
                {
                    await quartzWork.ScheduleJob.Invoke(_scheduler);
                }
                else
                {
                    await DefaultScheduleJobAsync(quartzWork);
                }
            }
        }
}

When I enable this module, it take over the only one IBackgroundWorkerManager in app,
so if other modules has workers that is not a IQuartzBackgroundWorker(example:TokenCleanupBackgroundWorker)
will be ignored.

namespace Volo.Abp.IdentityServer.Tokens
{
    public class TokenCleanupBackgroundWorker : AsyncPeriodicBackgroundWorkerBase
    {
        protected TokenCleanupOptions Options { get; }
        protected override async Task DoWorkAsync(PeriodicBackgroundWorkerContext workerContext)
        {
             ....
        }
    }
}

I should to create a new QuartzTokenCleanupBackgroundWorker class that implement IQuartzBackgroundWorker to fix it.

And also, BackgroundJobWorker of Volo.Abp.BackgroundJobs is the same,
If I just use Volo.Abp.BackgroundWorkers.Quartz but not Volo.Abp.BackgroundJobs.Quartz,
the default BackgroundJobWorker will doesn't work.

I think IBackgroundWorkerManager should ensure All IBackgroundJobWorker(not only IQuartzBackgroundWorker) in application works fine

Do you have any idea ?
Please let me know if I have misunderstood about design of this module,Thanks.

@yinchang0626
Copy link
Contributor Author

Temporary,maybe this is the simplest way to resolve it now.
But I think will cause other issue when QuartzBackgroundJobManager is enabled

    [Dependency(ReplaceServices = true)]
    public class FixQuartzBackgroundWorkerManager : IBackgroundWorkerManager,ISingletonDependency
    {
        private readonly QuartzBackgroundWorkerManager quartzBackgroundWorkerManager;
        private readonly BackgroundWorkerManager backgroundWorkerManager;

        public FixQuartzBackgroundWorkerManager(
            QuartzBackgroundWorkerManager quartzBackgroundWorkerManager,
            BackgroundWorkerManager backgroundWorkerManager
            )
        {
            this.quartzBackgroundWorkerManager = quartzBackgroundWorkerManager;
            this.backgroundWorkerManager = backgroundWorkerManager;
        }
        public void Add(IBackgroundWorker worker)
        {
            quartzBackgroundWorkerManager.Add(worker);
            backgroundWorkerManager.Add(worker);
        }

        public async Task StartAsync(CancellationToken cancellationToken = default)
        {
            await quartzBackgroundWorkerManager.StartAsync(cancellationToken);
            await backgroundWorkerManager.StartAsync(cancellationToken);
        }

        public async Task StopAsync(CancellationToken cancellationToken = default)
        {
            await quartzBackgroundWorkerManager.StopAsync(cancellationToken);
            await backgroundWorkerManager.StopAsync(cancellationToken);
        }
    }

@maliming maliming added this to the 3.0 milestone Jun 21, 2020
@realLiangshiwei
Copy link
Member

We can create an adapter to provide quartz support instead of using the default implementation, I will try implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants