Skip to content

Commit

Permalink
feat: Use GetRequiredService instead of GetService to resolve jobs (N…
Browse files Browse the repository at this point in the history 
…CronJob-Dev#24)

* Use GetRequiredService instead of GetService and added tests

* Removed redundant test

* Added entry in changelog

* Remove empty line in NCronJobIntegrationTests.cs

* Update CHANGELOG.md

Co-authored-by: Steven Giesel <stgiesel35@gmail.com>

---------

Co-authored-by: Steven Giesel <stgiesel35@gmail.com>
  • Loading branch information
@skarum @linkdotnet @falvarez1
2 people authored and falvarez1 committed Apr 24, 2024
1 parent 470f1a6 commit 34884f4
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 51 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ All notable changes to **NCronJob** will be documented in this file. The project
### Changed

- Implementation of the scheduling. Better performance and closed some memory leaks
- Throw exception if job cannot be resolved with dependencies from the DI container. Reported and implemented by [@skarum](https://github.com/skarum) in [#23)(https://github.com/linkdotnet/NCronJob/issues/23)

## [2.0.4] - 2024-04-16

Expand Down
6 changes: 1 addition & 5 deletions src/LinkDotNet.NCronJob/Execution/JobExecutor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,7 @@ public async Task RunJob(RegistryEntry run, CancellationToken stoppingToken)
}

var scope = serviceProvider.CreateScope();
if (scope.ServiceProvider.GetService(run.Type) is not IJob job)
{
LogJobNotRegistered(run.Type);
return;
}
var job = (IJob)scope.ServiceProvider.GetRequiredService(run.Type);

var jobExecutionInstance = new JobExecutionContext(run.Type, run.Output);
await ExecuteJob(jobExecutionInstance, job, scope, stopToken);
Expand Down
82 changes: 36 additions & 46 deletions tests/NCronJob.Tests/NCronJobIntegrationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
using LinkDotNet.NCronJob;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Time.Testing;
using Microsoft.Extensions.Logging.Abstractions;
using Shouldly;
using TimeProviderExtensions;

namespace NCronJob.Tests;

Expand All @@ -13,16 +14,12 @@ public sealed class NCronJobIntegrationTests : JobIntegrationBase
[Fact]
public async Task CronJobThatIsScheduledEveryMinuteShouldBeExecuted()
{
var fakeTimer = new FakeTimeProvider();
var fakeTimer = TimeProviderFactory.GetTimeProvider();
ServiceCollection.AddSingleton<TimeProvider>(fakeTimer);
ServiceCollection.AddNCronJob(n => n.AddJob<SimpleJob>(p => p.WithCronExpression("* * * * *")));
var provider = CreateServiceProvider();

// there may be more than one IHostedService, we want to specifically start the CronScheduler
await provider.GetServices<IHostedService>()
.OfType<CronScheduler>()
.Single()
.StartAsync(CancellationToken);
await provider.GetRequiredService<IHostedService>().StartAsync(CancellationToken);

var jobFinished = await WaitForJobsOrTimeout(1);
jobFinished.ShouldBeTrue();
Expand All @@ -31,7 +28,7 @@ await provider.GetServices<IHostedService>()
[Fact]
public async Task AdvancingTheWholeTimeShouldHaveTenEntries()
{
var fakeTimer = new FakeTimeProvider();
var fakeTimer = TimeProviderFactory.GetTimeProvider();
ServiceCollection.AddSingleton<TimeProvider>(fakeTimer);
ServiceCollection.AddNCronJob(n => n.AddJob<SimpleJob>(p => p.WithCronExpression("* * * * *")));
var provider = CreateServiceProvider();
Expand All @@ -42,24 +39,10 @@ public async Task AdvancingTheWholeTimeShouldHaveTenEntries()
jobFinished.ShouldBeTrue();
}

[Fact]
public async Task JobsShouldCancelOnCancellation()
{
var fakeTimer = new FakeTimeProvider();
ServiceCollection.AddSingleton<TimeProvider>(fakeTimer);
ServiceCollection.AddNCronJob(n => n.AddJob<SimpleJob>(p => p.WithCronExpression("* * * * *")));
var provider = CreateServiceProvider();

await provider.GetRequiredService<IHostedService>().StartAsync(CancellationToken);

var jobFinished = await DoNotWaitJustCancel(10);
jobFinished.ShouldBeFalse();
}

[Fact]
public async Task EachJobRunHasItsOwnScope()
{
var fakeTimer = new FakeTimeProvider();
var fakeTimer = new ManualTimeProvider();
var storage = new Storage();
ServiceCollection.AddSingleton<TimeProvider>(fakeTimer);
ServiceCollection.AddSingleton(storage);
Expand All @@ -80,7 +63,7 @@ public async Task EachJobRunHasItsOwnScope()
[Fact]
public async Task ExecuteAnInstantJob()
{
var fakeTimer = new FakeTimeProvider();
var fakeTimer = TimeProviderFactory.GetTimeProvider();
ServiceCollection.AddSingleton<TimeProvider>(fakeTimer);
ServiceCollection.AddNCronJob(n => n.AddJob<SimpleJob>());
var provider = CreateServiceProvider();
Expand All @@ -95,23 +78,21 @@ public async Task ExecuteAnInstantJob()
[Fact]
public async Task CronJobShouldPassDownParameter()
{
var fakeTimer = new FakeTimeProvider();
var fakeTimer = TimeProviderFactory.GetTimeProvider();
ServiceCollection.AddSingleton<TimeProvider>(fakeTimer);
ServiceCollection.AddNCronJob(n => n.AddJob<ParameterJob>(p => p.WithCronExpression("* * * * *").WithParameter("Hello World")));
var provider = CreateServiceProvider();

await provider.GetRequiredService<IHostedService>().StartAsync(CancellationToken);

fakeTimer.Advance(TimeSpan.FromMinutes(1));

var content = await CommunicationChannel.Reader.ReadAsync(CancellationToken);
content.ShouldBe("Hello World");
}

[Fact]
public async Task InstantJobShouldGetParameter()
{
var fakeTimer = new FakeTimeProvider();
var fakeTimer = TimeProviderFactory.GetTimeProvider();
ServiceCollection.AddSingleton<TimeProvider>(fakeTimer);
ServiceCollection.AddNCronJob(n => n.AddJob<ParameterJob>());
var provider = CreateServiceProvider();
Expand All @@ -126,23 +107,21 @@ public async Task InstantJobShouldGetParameter()
[Fact]
public async Task CronJobThatIsScheduledEverySecondShouldBeExecuted()
{
var fakeTimer = new FakeTimeProvider();
fakeTimer.Advance(TimeSpan.FromSeconds(1));
var fakeTimer = TimeProviderFactory.GetTimeProvider(TimeSpan.FromSeconds(1));
ServiceCollection.AddSingleton<TimeProvider>(fakeTimer);
ServiceCollection.AddNCronJob(n => n.AddJob<SimpleJob>(p => p.WithCronExpression("* * * * * *", true)));
var provider = CreateServiceProvider();

await provider.GetRequiredService<IHostedService>().StartAsync(CancellationToken);
fakeTimer.Advance(TimeSpan.FromMinutes(2));

var jobFinished = await WaitForJobsOrTimeout(2);
jobFinished.ShouldBeTrue();
}

[Fact]
public async Task CanRunSecondPrecisionAndMinutePrecisionJobs()
{
var fakeTimer = new FakeTimeProvider();
fakeTimer.Advance(TimeSpan.FromSeconds(1));
var fakeTimer = TimeProviderFactory.GetTimeProvider(TimeSpan.FromSeconds(1));
ServiceCollection.AddSingleton<TimeProvider>(fakeTimer);
ServiceCollection.AddNCronJob(n => n.AddJob<SimpleJob>(
p => p.WithCronExpression("* * * * * *", true).And.WithCronExpression("* * * * *")));
Expand All @@ -157,7 +136,7 @@ public async Task CanRunSecondPrecisionAndMinutePrecisionJobs()
[Fact]
public async Task LongRunningJobShouldNotBlockScheduler()
{
var fakeTimer = new FakeTimeProvider();
var fakeTimer = TimeProviderFactory.GetTimeProvider();
ServiceCollection.AddSingleton<TimeProvider>(fakeTimer);
ServiceCollection.AddNCronJob(n => n
.AddJob<LongRunningJob>(p => p.WithCronExpression("* * * * *"))
Expand All @@ -173,9 +152,10 @@ public async Task LongRunningJobShouldNotBlockScheduler()
[Fact]
public async Task NotRegisteredJobShouldNotAbortOtherRuns()
{
var fakeTimer = new FakeTimeProvider();
var fakeTimer = TimeProviderFactory.GetTimeProvider();
ServiceCollection.AddSingleton<TimeProvider>(fakeTimer);
ServiceCollection.AddNCronJob(n => n.AddJob<SimpleJob>(p => p.WithCronExpression("* * * * *")));
ServiceCollection.AddTransient<ParameterJob>();
var provider = CreateServiceProvider();
provider.GetRequiredService<IInstantJobRegistry>().RunInstantJob<ParameterJob>();

Expand All @@ -185,6 +165,20 @@ public async Task NotRegisteredJobShouldNotAbortOtherRuns()
jobFinished.ShouldBeTrue();
}

[Fact]
public void ThrowIfJobWithDependenciesIsNotRegistered()
{
ServiceCollection
.AddNCronJob(n => n.AddJob<JobWithDependency>(p => p.WithCronExpression("* * * * *")));
var provider = CreateServiceProvider();

Assert.Throws<InvalidOperationException>(() =>
{
using var executor = new JobExecutor(provider, NullLogger<JobExecutor>.Instance);
executor.RunJob(new RegistryEntry(typeof(JobWithDependency), new JobExecutionContext(null), null), CancellationToken.None);
});
}

private sealed class GuidGenerator
{
public Guid NewGuid { get; } = Guid.NewGuid();
Expand All @@ -198,17 +192,7 @@ private sealed class Storage
private sealed class SimpleJob(ChannelWriter<object> writer) : IJob
{
public async Task RunAsync(JobExecutionContext context, CancellationToken token)
{
try
{
context.Output = "Job Completed";
await writer.WriteAsync(context.Output, token);
}
catch (Exception ex)
{
await writer.WriteAsync(ex, token);
}
}
=> await writer.WriteAsync(true, token);
}

private sealed class LongRunningJob : IJob
Expand All @@ -234,4 +218,10 @@ private sealed class ParameterJob(ChannelWriter<object> writer) : IJob
public async Task RunAsync(JobExecutionContext context, CancellationToken token)
=> await writer.WriteAsync(context.Parameter!, token);
}

private sealed class JobWithDependency(ChannelWriter<object> writer, GuidGenerator guidGenerator) : IJob
{
public async Task RunAsync(JobExecutionContext context, CancellationToken token)
=> await writer.WriteAsync(guidGenerator.NewGuid, token);
}
}

0 comments on commit 34884f4

Please sign in to comment.