diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 485f1e7..811e247 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -20,7 +20,7 @@ iteration. Both tests now require a second dispatch, which is what actually prov the first. The worker has no equivalent of `ToHttpResult()`. Its category-to-disposition mapping — retry, dead -letter, log once — lives in `RunOnceAsync`, alongside the scope it resolves and the handler it +letter, log once — lives in the pass, alongside the scope it resolves and the handler it dispatches to; `ExecuteAsync` above it only schedules and keeps the loop alive. If a second worker project ever wants the same mapping, that is the moment to consider a package for it, not before. diff --git a/docs/agents/10-worker.md b/docs/agents/10-worker.md index 60ae18a..35bfb4a 100644 --- a/docs/agents/10-worker.md +++ b/docs/agents/10-worker.md @@ -8,11 +8,25 @@ Applies to `src/MyApp.Worker/`. - **`BackgroundService` + `PeriodicTimer`.** No scheduling framework by default — most workers are "every N minutes" or "drain this," and the BCL does both with no dependencies. - **Escalate to Quartz.NET only for a stated need:** cron expressions, clustering, or persistent job state. Adding it speculatively buys a database table and a configuration surface you do not want. - **Hangfire is out.** `Hangfire.Core` is LGPL v3, which is a licence to adopt deliberately rather than inherit, and its Pro tier is paid. Its dashboard was compensating for missing observability, which Aspire already provides. -- **`ExecuteAsync` contains no business logic.** It ticks, runs one pass, and refuses to let a failure end the loop. Resolving a scope and dispatching to a handler happens a level down in `RunOnceAsync` — that is the position an endpoint occupies in the API archetype, and keeping the two apart is what makes either testable without the other. +- **`ExecuteAsync` contains no business logic.** It ticks, runs one pass, and refuses to let a failure end the loop. Resolving a scope and dispatching to a handler belongs to a **pass** of its own — that is the position an endpoint occupies in the API archetype, and giving it a separate type is what lets either be tested without the other. ```csharp +// One pass: a scope, a dispatch, and what the outcome means. Everything here is decided by the +// result, so it can be exercised with no clock and no timer. +internal sealed class ReconcileOrdersPass(IServiceScopeFactory scopes) +{ + public async Task RunAsync(CancellationToken ct) + { + await using var scope = scopes.CreateAsyncScope(); + var handler = scope.ServiceProvider.GetRequiredService>(); + var result = await handler.HandleAsync(new Request(), ct); + // map result category to retry / dead-letter / log — never throw to signal it + } +} + +// The schedule, and one guarantee: a failed pass must not end the loop. internal sealed partial class ReconcileOrdersWorker( - IServiceScopeFactory scopes, + ReconcileOrdersPass pass, TimeProvider clock, ILogger logger) : BackgroundService @@ -32,7 +46,7 @@ internal sealed partial class ReconcileOrdersWorker( { try { - await RunOnceAsync(ct); + await pass.RunAsync(ct); } catch (Exception exception) when (exception is not OperationCanceledException || !ct.IsCancellationRequested) @@ -42,15 +56,6 @@ internal sealed partial class ReconcileOrdersWorker( } } - // ExecuteAsync schedules and keeps the loop alive; one pass lives here. - internal async Task RunOnceAsync(CancellationToken ct) - { - await using var scope = scopes.CreateAsyncScope(); - var handler = scope.ServiceProvider.GetRequiredService>(); - var result = await handler.HandleAsync(new Request(), ct); - // map result category to retry / dead-letter / log — never throw to signal it - } - [LoggerMessage(Level = LogLevel.Error, Message = "A pass threw and was swallowed to keep the worker alive.")] private static partial void Failed(ILogger logger, Exception exception); } @@ -69,5 +74,5 @@ internal sealed partial class ReconcileOrdersWorker( ### Testing - **Handlers are tested directly**, against Testcontainers Postgres, with Respawn between tests. There is no transport to go through, so the handler *is* the entry point. -- **Separate the pass from the schedule, and test the pass.** `RunOnceAsync` holds everything decided by a result — the scope, the dispatch, retry against dead-letter — so it can be exercised against a stub handler with no clock and no timer. Make the retry backoff a setting so a test can set it to zero. +- **Separate the pass from the schedule, and test the pass.** The pass holds everything decided by a result — the scope, the dispatch, retry against dead-letter — so it can be exercised against a stub handler with no clock and no timer. Make the retry backoff a setting so a test can set it to zero. - **Test one guarded iteration, not the timer.** Extract the `try`/`catch` into an `internal` method — with `` on the host project — so a test can call it directly and assert which exceptions survive it. That a failing pass does not end the loop is your logic; that `PeriodicTimer` fires is the BCL's. Driving a real `BackgroundService` from a fake clock needs the scheduler to hand off between advancing the clock and the loop resuming — a test that does it is flaky unless it sleeps, and then it is a slow test of someone else's code. diff --git a/src/Loom.Templates/templates/loom-api/src/MyApp.Api/Program.cs b/src/Loom.Templates/templates/loom-api/src/MyApp.Api/Program.cs index 3018c0b..ddbf367 100644 --- a/src/Loom.Templates/templates/loom-api/src/MyApp.Api/Program.cs +++ b/src/Loom.Templates/templates/loom-api/src/MyApp.Api/Program.cs @@ -42,7 +42,7 @@ // includeInternalTypes matters: slice validators are internal, and without it none are registered. // The validating decorator treats a missing validator as nothing to validate, so the omission would -// be silent — which is why ValidatorRegistrationTests asserts every validator is resolvable. +// be silent — which is why RegistrationTests asserts every validator is resolvable. builder.Services.AddValidatorsFromAssemblyContaining(ServiceLifetime.Scoped, includeInternalTypes: true); builder.Services.AddProblemDetails(); diff --git a/src/Loom.Templates/templates/loom-worker/AGENTS.md b/src/Loom.Templates/templates/loom-worker/AGENTS.md index 98e846f..c88d7c3 100644 --- a/src/Loom.Templates/templates/loom-worker/AGENTS.md +++ b/src/Loom.Templates/templates/loom-worker/AGENTS.md @@ -196,11 +196,25 @@ Applies to `src/MyApp.Worker/`. - **`BackgroundService` + `PeriodicTimer`.** No scheduling framework by default — most workers are "every N minutes" or "drain this," and the BCL does both with no dependencies. - **Escalate to Quartz.NET only for a stated need:** cron expressions, clustering, or persistent job state. Adding it speculatively buys a database table and a configuration surface you do not want. - **Hangfire is out.** `Hangfire.Core` is LGPL v3, which is a licence to adopt deliberately rather than inherit, and its Pro tier is paid. Its dashboard was compensating for missing observability, which Aspire already provides. -- **`ExecuteAsync` contains no business logic.** It ticks, runs one pass, and refuses to let a failure end the loop. Resolving a scope and dispatching to a handler happens a level down in `RunOnceAsync` — that is the position an endpoint occupies in the API archetype, and keeping the two apart is what makes either testable without the other. +- **`ExecuteAsync` contains no business logic.** It ticks, runs one pass, and refuses to let a failure end the loop. Resolving a scope and dispatching to a handler belongs to a **pass** of its own — that is the position an endpoint occupies in the API archetype, and giving it a separate type is what lets either be tested without the other. ```csharp +// One pass: a scope, a dispatch, and what the outcome means. Everything here is decided by the +// result, so it can be exercised with no clock and no timer. +internal sealed class ReconcileOrdersPass(IServiceScopeFactory scopes) +{ + public async Task RunAsync(CancellationToken ct) + { + await using var scope = scopes.CreateAsyncScope(); + var handler = scope.ServiceProvider.GetRequiredService>(); + var result = await handler.HandleAsync(new Request(), ct); + // map result category to retry / dead-letter / log — never throw to signal it + } +} + +// The schedule, and one guarantee: a failed pass must not end the loop. internal sealed partial class ReconcileOrdersWorker( - IServiceScopeFactory scopes, + ReconcileOrdersPass pass, TimeProvider clock, ILogger logger) : BackgroundService @@ -220,7 +234,7 @@ internal sealed partial class ReconcileOrdersWorker( { try { - await RunOnceAsync(ct); + await pass.RunAsync(ct); } catch (Exception exception) when (exception is not OperationCanceledException || !ct.IsCancellationRequested) @@ -230,15 +244,6 @@ internal sealed partial class ReconcileOrdersWorker( } } - // ExecuteAsync schedules and keeps the loop alive; one pass lives here. - internal async Task RunOnceAsync(CancellationToken ct) - { - await using var scope = scopes.CreateAsyncScope(); - var handler = scope.ServiceProvider.GetRequiredService>(); - var result = await handler.HandleAsync(new Request(), ct); - // map result category to retry / dead-letter / log — never throw to signal it - } - [LoggerMessage(Level = LogLevel.Error, Message = "A pass threw and was swallowed to keep the worker alive.")] private static partial void Failed(ILogger logger, Exception exception); } @@ -257,5 +262,5 @@ internal sealed partial class ReconcileOrdersWorker( ### Testing - **Handlers are tested directly**, against Testcontainers Postgres, with Respawn between tests. There is no transport to go through, so the handler *is* the entry point. -- **Separate the pass from the schedule, and test the pass.** `RunOnceAsync` holds everything decided by a result — the scope, the dispatch, retry against dead-letter — so it can be exercised against a stub handler with no clock and no timer. Make the retry backoff a setting so a test can set it to zero. +- **Separate the pass from the schedule, and test the pass.** The pass holds everything decided by a result — the scope, the dispatch, retry against dead-letter — so it can be exercised against a stub handler with no clock and no timer. Make the retry backoff a setting so a test can set it to zero. - **Test one guarded iteration, not the timer.** Extract the `try`/`catch` into an `internal` method — with `` on the host project — so a test can call it directly and assert which exceptions survive it. That a failing pass does not end the loop is your logic; that `PeriodicTimer` fires is the BCL's. Driving a real `BackgroundService` from a fake clock needs the scheduler to hand off between advancing the clock and the loop resuming — a test that does it is flaky unless it sleeps, and then it is a slow test of someone else's code. diff --git a/src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/RegistrationTests.cs b/src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/RegistrationTests.cs new file mode 100644 index 0000000..d2bf649 --- /dev/null +++ b/src/Loom.Templates/templates/loom-worker/tests/MyApp.Worker.Tests/RegistrationTests.cs @@ -0,0 +1,69 @@ +using FluentValidation; +using Loom.Handlers; +using Microsoft.Extensions.DependencyInjection; + +namespace MyApp.Worker.Tests; + +/// +/// Every handler and every validator resolves. +/// +/// +/// A validator that is not registered is silently ignored — the decorator treats a missing one as +/// nothing to validate, which is right for a slice with no rules and disastrous for one with them. +/// Assembly scanning excludes internal types unless asked, and slice validators are internal. +/// +/// Both sets are discovered rather than listed. Naming one handler would only ever prove that handler +/// is registered, so a new slice whose AddHandler call was forgotten would still pass — and in a +/// worker the first sign of that is a tick failing in a deployment, with no request to fail instead. +/// +/// +[NotInParallel] +public sealed class RegistrationTests +{ + [Test] + public async Task Every_Validator_In_The_Application_Resolves() + { + using IServiceScope scope = WorkerFixture.Host.CreateScope(); + + Type[] requestTypes = + [ + .. typeof(WorkerServices).Assembly.GetTypes() + .Where(type => type is { IsClass: true, IsAbstract: false } + && type.BaseType is { IsGenericType: true } baseType + && baseType.GetGenericTypeDefinition() == typeof(AbstractValidator<>)) + .Select(type => type.BaseType!.GetGenericArguments()[0]), + ]; + + await Assert.That(requestTypes).IsNotEmpty(); + + foreach (Type requestType in requestTypes) + { + Type validatorType = typeof(IValidator<>).MakeGenericType(requestType); + + await Assert.That(scope.ServiceProvider.GetService(validatorType)).IsNotNull(); + } + } + + [Test] + public async Task Every_Handler_Resolves_Through_Its_Decorator_Chain() + { + using IServiceScope scope = WorkerFixture.Host.CreateScope(); + + Type[] handlerInterfaces = + [ + .. typeof(WorkerServices).Assembly.GetTypes() + .Where(type => type is { IsClass: true, IsAbstract: false }) + .SelectMany(type => type.GetInterfaces()) + .Where(contract => contract.IsGenericType + && contract.GetGenericTypeDefinition() == typeof(IHandler<,>)) + .Distinct(), + ]; + + await Assert.That(handlerInterfaces).IsNotEmpty(); + + foreach (Type contract in handlerInterfaces) + { + await Assert.That(scope.ServiceProvider.GetService(contract)).IsNotNull(); + } + } +}