NCronJob-Dev / NCronJob

A Job Scheduler sitting on top of IHostedService in dotnet.
https://docs.ncronjob.dev/
MIT License
157 stars 11 forks source link

Multiple AddNCronJob only registers ones #138

Open skarum opened 2 hours ago

skarum commented 2 hours ago

Describe the bug Prior to version 3.3.3 you could register multiple jobs by calling AddNCronJob multiple times.

If I had a setup like this:

services.AddNCronJob(
    builder => builder
        .AddJob<TestJob1>(o => o.WithCronExpression("0/1 * * * * *"))
);

services.AddNCronJob(
    builder => builder
        .AddJob<TestJob2>(o => o.WithCronExpression("0/1 * * * * *"))
);

Current behavior Only TestJob1 is executed

Expected behavior Both TestJob1 and TestJob2 should be executed

Version information

I think the change from builder.RegisterJobs(); to services.TryAddSingleton(jobRegistry); in this commit is the cause of this behavior, but I'm not sure if it's intentional or a bug?

If this is intentional I think the second call to .AddNCronJob(... should throw an exception.

linkdotnet commented 2 hours ago

Hey @skarum

No that is absolutely not intentional (at least until now :D) and should be fixed! Thanks for the report!

skarum commented 2 hours ago

Ohh - can see I forgot to link to the commit: https://github.com/NCronJob-Dev/NCronJob/commit/0fe7e6666d074b2c83ce0fccc679b349a1a7f41d#diff-997ac4e971e8a1108f07128317dfa5a16164555c9220704a45a2d7f26ff01eb0

linkdotnet commented 1 hour ago

As a workaround, you can "just" have one call:

services.AddNCronJob(
    builder => builder
        .AddJob<TestJob1>(o => o.WithCronExpression("0/1 * * * * *"))
        .AddJob<TestJob2>(...)
);
linkdotnet commented 1 hour ago

@nulltoken we might wanna change that in the future to remove ambiguity (and make our code simpler)

skarum commented 1 hour ago

As a workaround, you can "just" have one call:

services.AddNCronJob(
    builder => builder
        .AddJob<TestJob1>(o => o.WithCronExpression("0/1 * * * * *"))
        .AddJob<TestJob2>(...)
);

Thanks, that could work.

Unfortunately I have decentralized my setup, where different parts of my code can just add stuff to the IOC container, including registering a job.

So, If you are going to fix it so it'll work in old way I'll just wait for the next release. If not I'll adjust my code. :-)