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

Multiple AddNCronJob only registers ones #138

Closed
skarum opened this issue Nov 15, 2024 · 7 comments
Closed

Multiple AddNCronJob only registers ones #138

skarum opened this issue Nov 15, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@skarum
Copy link
Contributor

skarum commented Nov 15, 2024

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

  • NCronJobVersion: 3.3.3-5
  • .NET runtime version: 8 and 9

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
Copy link
Member

Hey @skarum

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

@linkdotnet linkdotnet added the bug Something isn't working label Nov 15, 2024
@skarum
Copy link
Contributor Author

skarum commented Nov 15, 2024

Ohh - can see I forgot to link to the commit: 0fe7e66#diff-997ac4e971e8a1108f07128317dfa5a16164555c9220704a45a2d7f26ff01eb0

@linkdotnet
Copy link
Member

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

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

@linkdotnet
Copy link
Member

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

@skarum
Copy link
Contributor Author

skarum commented Nov 15, 2024

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. :-)

@linkdotnet
Copy link
Member

I think I have a fix - which I will push as a preview in a few minutes

@linkdotnet
Copy link
Member

A new 3.3.7-preview package will be available in a few minutes. Thanks for the report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants