From 703cb2d3924c3b9f46319206efefba93bc255037 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 19 May 2026 02:07:29 -0400 Subject: [PATCH] feat(notification-outbox): add AddNotificationOutbox DI registration --- .../NotificationOutboxActor.cs | 36 ++++-- .../ServiceCollectionExtensions.cs | 49 ++++++++ .../NotificationOutboxActorDispatchTests.cs | 53 ++++---- .../NotificationOutboxActorIngestTests.cs | 3 +- .../NotificationOutboxActorPurgeTests.cs | 3 +- .../NotificationOutboxActorQueryTests.cs | 3 +- .../ScadaLink.NotificationOutbox.Tests.csproj | 3 + .../ServiceRegistrationTests.cs | 113 ++++++++++++++++++ 8 files changed, 218 insertions(+), 45 deletions(-) create mode 100644 src/ScadaLink.NotificationOutbox/ServiceCollectionExtensions.cs create mode 100644 tests/ScadaLink.NotificationOutbox.Tests/ServiceRegistrationTests.cs diff --git a/src/ScadaLink.NotificationOutbox/NotificationOutboxActor.cs b/src/ScadaLink.NotificationOutbox/NotificationOutboxActor.cs index 947a2cc..9874968 100644 --- a/src/ScadaLink.NotificationOutbox/NotificationOutboxActor.cs +++ b/src/ScadaLink.NotificationOutbox/NotificationOutboxActor.cs @@ -31,7 +31,6 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers private readonly IServiceProvider _serviceProvider; private readonly NotificationOutboxOptions _options; private readonly ILogger _logger; - private readonly IReadOnlyDictionary _adapters; /// /// In-flight guard for the dispatch loop. Set true at the start of a sweep and cleared @@ -46,13 +45,11 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers public NotificationOutboxActor( IServiceProvider serviceProvider, NotificationOutboxOptions options, - ILogger logger, - IReadOnlyDictionary adapters) + ILogger logger) { _serviceProvider = serviceProvider; _options = options; _logger = logger; - _adapters = adapters; Receive(HandleSubmit); Receive(HandleIngestPersisted); @@ -174,6 +171,11 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers /// in a try/catch so the returned task never faults — scope creation, service resolution, /// and retry-policy resolution can all throw, and a faulted task would otherwise leave /// the dispatcher's in-flight guard stuck and wedge the loop permanently. + /// + /// The channel delivery adapters are resolved from the per-sweep scope, not held in a + /// field: takes a scoped + /// directly, so a long-lived adapter reference on + /// this singleton actor would be a captive dependency over a disposed DbContext. /// private async Task RunDispatchPass(DateTimeOffset now) { @@ -182,6 +184,7 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers using var scope = _serviceProvider.CreateScope(); var outboxRepository = scope.ServiceProvider.GetRequiredService(); var notificationRepository = scope.ServiceProvider.GetRequiredService(); + var adapters = ResolveAdapters(scope.ServiceProvider); IReadOnlyList due; try @@ -205,7 +208,7 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers { try { - await DeliverOneAsync(notification, now, maxRetries, retryDelay, outboxRepository); + await DeliverOneAsync(notification, now, maxRetries, retryDelay, outboxRepository, adapters); } catch (Exception ex) { @@ -238,6 +241,24 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers : (configuration.MaxRetries, configuration.RetryDelay); } + /// + /// Builds the → adapter lookup for a dispatch sweep from + /// the registered services in the supplied + /// scope. The last adapter registered for a given type wins, mirroring DI's last-wins + /// resolution semantics. + /// + private static IReadOnlyDictionary ResolveAdapters( + IServiceProvider scopedServices) + { + var adapters = new Dictionary(); + foreach (var adapter in scopedServices.GetServices()) + { + adapters[adapter.Type] = adapter; + } + + return adapters; + } + /// /// Delivers a single notification through its channel adapter and applies the resulting /// status transition. A missing adapter parks the notification; otherwise the @@ -248,9 +269,10 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers DateTimeOffset now, int maxRetries, TimeSpan retryDelay, - INotificationOutboxRepository outboxRepository) + INotificationOutboxRepository outboxRepository, + IReadOnlyDictionary adapters) { - if (!_adapters.TryGetValue(notification.Type, out var adapter)) + if (!adapters.TryGetValue(notification.Type, out var adapter)) { notification.Status = NotificationStatus.Parked; notification.LastError = $"no delivery adapter for type {notification.Type}"; diff --git a/src/ScadaLink.NotificationOutbox/ServiceCollectionExtensions.cs b/src/ScadaLink.NotificationOutbox/ServiceCollectionExtensions.cs new file mode 100644 index 0000000..69ecf84 --- /dev/null +++ b/src/ScadaLink.NotificationOutbox/ServiceCollectionExtensions.cs @@ -0,0 +1,49 @@ +using Microsoft.Extensions.DependencyInjection; +using ScadaLink.NotificationOutbox.Delivery; + +namespace ScadaLink.NotificationOutbox; + +/// +/// DI registration for the Notification Outbox component: binds +/// and registers the channel delivery adapters. +/// +public static class ServiceCollectionExtensions +{ + /// Configuration section bound to . + public const string OptionsSection = "ScadaLink:NotificationOutbox"; + + /// + /// Registers the Notification Outbox services: the + /// binding and the channel delivery adapters. + /// + /// This extension covers only the outbox-specific registrations. The + /// reuses the + /// SMTP machinery — + /// Func<ISmtpClientWrapper>, OAuth2TokenService and + /// NotificationOptions — so the caller (the Host on the central node) must also + /// call AddNotificationService(). Re-registering those services here would + /// duplicate them; relying on AddNotificationService keeps a single source of truth. + /// + /// is registered scoped because it + /// takes a scoped + /// directly. The resolves the adapters from a fresh + /// scope per dispatch sweep rather than holding them, so no scoped adapter is captured by + /// the singleton actor. + /// + public static IServiceCollection AddNotificationOutbox(this IServiceCollection services) + { + ArgumentNullException.ThrowIfNull(services); + + services.AddOptions() + .BindConfiguration(OptionsSection); + + // Scoped: the adapter holds a scoped INotificationRepository. Registered both under + // the interface (so the dispatch sweep can enumerate every channel adapter) and as + // the concrete type (so callers and tests can resolve it directly). + services.AddScoped(); + services.AddScoped( + sp => sp.GetRequiredService()); + + return services; + } +} diff --git a/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorDispatchTests.cs b/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorDispatchTests.cs index 0c73dd5..fafdf3d 100644 --- a/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorDispatchTests.cs +++ b/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorDispatchTests.cs @@ -26,11 +26,19 @@ public class NotificationOutboxActorDispatchTests : TestKit private readonly INotificationRepository _notificationRepository = Substitute.For(); - private IServiceProvider BuildServiceProvider() + private IServiceProvider BuildServiceProvider( + IEnumerable adapters) { var services = new ServiceCollection(); services.AddScoped(_ => _outboxRepository); services.AddScoped(_ => _notificationRepository); + // The actor resolves the channel adapters from its per-sweep DI scope; register + // each stub adapter under the INotificationDeliveryAdapter service. + foreach (var adapter in adapters) + { + services.AddScoped(_ => adapter); + } + return services.BuildServiceProvider(); } @@ -67,14 +75,13 @@ public class NotificationOutboxActorDispatchTests : TestKit } private IActorRef CreateActor( - IReadOnlyDictionary adapters, + IEnumerable adapters, NotificationOutboxOptions? options = null) { return Sys.ActorOf(Props.Create(() => new NotificationOutboxActor( - BuildServiceProvider(), + BuildServiceProvider(adapters), options ?? new NotificationOutboxOptions { DispatchInterval = TimeSpan.FromHours(1) }, - NullLogger.Instance, - adapters))); + NullLogger.Instance))); } private static Notification MakeNotification( @@ -107,10 +114,7 @@ public class NotificationOutboxActorDispatchTests : TestKit _outboxRepository.GetDueAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(new[] { notification }); var adapter = new StubAdapter(() => DeliveryOutcome.Success("ops@example.com")); - var actor = CreateActor(new Dictionary - { - [NotificationType.Email] = adapter, - }); + var actor = CreateActor([adapter]); actor.Tell(InternalMessages.DispatchTick.Instance); @@ -130,10 +134,7 @@ public class NotificationOutboxActorDispatchTests : TestKit _outboxRepository.GetDueAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(new[] { notification }); var adapter = new StubAdapter(() => DeliveryOutcome.Success("ops@example.com")); - var actor = CreateActor(new Dictionary - { - [NotificationType.Email] = adapter, - }); + var actor = CreateActor([adapter]); actor.Tell(InternalMessages.DispatchTick.Instance); @@ -158,10 +159,7 @@ public class NotificationOutboxActorDispatchTests : TestKit _outboxRepository.GetDueAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(new[] { notification }); var adapter = new StubAdapter(() => DeliveryOutcome.Transient("smtp timeout")); - var actor = CreateActor(new Dictionary - { - [NotificationType.Email] = adapter, - }); + var actor = CreateActor([adapter]); actor.Tell(InternalMessages.DispatchTick.Instance); @@ -187,10 +185,7 @@ public class NotificationOutboxActorDispatchTests : TestKit _outboxRepository.GetDueAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(new[] { notification }); var adapter = new StubAdapter(() => DeliveryOutcome.Transient("smtp timeout")); - var actor = CreateActor(new Dictionary - { - [NotificationType.Email] = adapter, - }); + var actor = CreateActor([adapter]); actor.Tell(InternalMessages.DispatchTick.Instance); @@ -212,10 +207,7 @@ public class NotificationOutboxActorDispatchTests : TestKit _outboxRepository.GetDueAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(new[] { notification }); var adapter = new StubAdapter(() => DeliveryOutcome.Permanent("invalid recipient address")); - var actor = CreateActor(new Dictionary - { - [NotificationType.Email] = adapter, - }); + var actor = CreateActor([adapter]); actor.Tell(InternalMessages.DispatchTick.Instance); @@ -237,8 +229,8 @@ public class NotificationOutboxActorDispatchTests : TestKit var notification = MakeNotification(); _outboxRepository.GetDueAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(new[] { notification }); - // Empty adapter dictionary: no adapter resolves for the notification's type. - var actor = CreateActor(new Dictionary()); + // No adapters registered: none resolves for the notification's type. + var actor = CreateActor([]); actor.Tell(InternalMessages.DispatchTick.Instance); @@ -262,7 +254,7 @@ public class NotificationOutboxActorDispatchTests : TestKit // failure were not handled, which would leave _dispatching stuck true forever. _outboxRepository.GetDueAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns>(_ => throw new InvalidOperationException("db down")); - var actor = CreateActor(new Dictionary()); + var actor = CreateActor([]); // First tick: the pass faults internally but must still clear the in-flight guard. actor.Tell(InternalMessages.DispatchTick.Instance); @@ -287,10 +279,7 @@ public class NotificationOutboxActorDispatchTests : TestKit var adapter = new StubAdapter( () => DeliveryOutcome.Success("ops@example.com"), delay: TimeSpan.FromMilliseconds(800)); - var actor = CreateActor(new Dictionary - { - [NotificationType.Email] = adapter, - }); + var actor = CreateActor([adapter]); actor.Tell(InternalMessages.DispatchTick.Instance); actor.Tell(InternalMessages.DispatchTick.Instance); diff --git a/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorIngestTests.cs b/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorIngestTests.cs index b712e88..4c62943 100644 --- a/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorIngestTests.cs +++ b/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorIngestTests.cs @@ -34,8 +34,7 @@ public class NotificationOutboxActorIngestTests : TestKit return Sys.ActorOf(Props.Create(() => new NotificationOutboxActor( BuildServiceProvider(), new NotificationOutboxOptions(), - NullLogger.Instance, - new Dictionary()))); + NullLogger.Instance))); } private static NotificationSubmit MakeSubmit(string? notificationId = null) diff --git a/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorPurgeTests.cs b/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorPurgeTests.cs index ecc1159..bcd749e 100644 --- a/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorPurgeTests.cs +++ b/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorPurgeTests.cs @@ -46,8 +46,7 @@ public class NotificationOutboxActorPurgeTests : TestKit DispatchInterval = TimeSpan.FromHours(1), PurgeInterval = TimeSpan.FromHours(1), }, - NullLogger.Instance, - new Dictionary()))); + NullLogger.Instance))); } [Fact] diff --git a/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorQueryTests.cs b/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorQueryTests.cs index f5aa410..36da5e3 100644 --- a/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorQueryTests.cs +++ b/tests/ScadaLink.NotificationOutbox.Tests/NotificationOutboxActorQueryTests.cs @@ -36,8 +36,7 @@ public class NotificationOutboxActorQueryTests : TestKit BuildServiceProvider(), // A long dispatch interval keeps the dispatch loop from interfering with these tests. options ?? new NotificationOutboxOptions { DispatchInterval = TimeSpan.FromHours(1) }, - NullLogger.Instance, - new Dictionary()))); + NullLogger.Instance))); } private static Notification MakeNotification( diff --git a/tests/ScadaLink.NotificationOutbox.Tests/ScadaLink.NotificationOutbox.Tests.csproj b/tests/ScadaLink.NotificationOutbox.Tests/ScadaLink.NotificationOutbox.Tests.csproj index ec6f557..790e6a3 100644 --- a/tests/ScadaLink.NotificationOutbox.Tests/ScadaLink.NotificationOutbox.Tests.csproj +++ b/tests/ScadaLink.NotificationOutbox.Tests/ScadaLink.NotificationOutbox.Tests.csproj @@ -11,6 +11,9 @@ + + + diff --git a/tests/ScadaLink.NotificationOutbox.Tests/ServiceRegistrationTests.cs b/tests/ScadaLink.NotificationOutbox.Tests/ServiceRegistrationTests.cs new file mode 100644 index 0000000..fe1ee79 --- /dev/null +++ b/tests/ScadaLink.NotificationOutbox.Tests/ServiceRegistrationTests.cs @@ -0,0 +1,113 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using NSubstitute; +using ScadaLink.Commons.Interfaces.Repositories; +using ScadaLink.Commons.Types.Enums; +using ScadaLink.NotificationOutbox.Delivery; +using ScadaLink.NotificationService; + +namespace ScadaLink.NotificationOutbox.Tests; + +/// +/// Task 17: Tests for — the +/// DI registration extension for the Notification Outbox component. The extension binds +/// from the ScadaLink:NotificationOutbox +/// configuration section and registers the channel delivery adapter(s). +/// +/// The Host wires both AddNotificationService and AddNotificationOutbox on the +/// central node; these tests do the same so the +/// adapter's SMTP dependencies (Func<ISmtpClientWrapper>, OAuth2TokenService, +/// NotificationOptions) are satisfied. — which the +/// takes directly and is registered scoped by the +/// Configuration Database component — is supplied here by a lightweight stub. +/// +public class ServiceRegistrationTests +{ + private static ServiceProvider BuildProvider() + { + var services = new ServiceCollection(); + + // Empty configuration: the options binding must still succeed and yield the + // documented NotificationOutboxOptions defaults. + var configuration = new ConfigurationBuilder().Build(); + services.AddSingleton(configuration); + services.AddLogging(); + + // INotificationRepository is registered scoped by the Configuration Database + // component in production; a no-op stub stands in for it here. + services.AddScoped(_ => Substitute.For()); + + services.AddNotificationService(); + services.AddNotificationOutbox(); + + return services.BuildServiceProvider(); + } + + [Fact] + public void AddNotificationOutbox_RegistersNotificationOutboxOptions_WithDefaults() + { + using var provider = BuildProvider(); + + var options = provider.GetRequiredService>().Value; + + Assert.NotNull(options); + Assert.Equal(TimeSpan.FromSeconds(10), options.DispatchInterval); + Assert.Equal(100, options.DispatchBatchSize); + } + + [Fact] + public void AddNotificationOutbox_OptionsSection_IsTheNotificationOutboxConfigPath() + { + Assert.Equal( + "ScadaLink:NotificationOutbox", + NotificationOutbox.ServiceCollectionExtensions.OptionsSection); + } + + [Fact] + public void AddNotificationOutbox_BindsNotificationOutboxOptions_FromConfiguration() + { + var services = new ServiceCollection(); + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["ScadaLink:NotificationOutbox:DispatchBatchSize"] = "250", + }) + .Build(); + services.AddSingleton(configuration); + services.AddLogging(); + services.AddScoped(_ => Substitute.For()); + services.AddNotificationService(); + services.AddNotificationOutbox(); + + using var provider = services.BuildServiceProvider(); + var options = provider.GetRequiredService>().Value; + + Assert.Equal(250, options.DispatchBatchSize); + } + + [Fact] + public void AddNotificationOutbox_RegistersEmailDeliveryAdapter() + { + using var provider = BuildProvider(); + using var scope = provider.CreateScope(); + + var adapter = scope.ServiceProvider.GetRequiredService(); + + Assert.NotNull(adapter); + Assert.Equal(NotificationType.Email, adapter.Type); + } + + [Fact] + public void AddNotificationOutbox_RegistersEmailAdapter_AsINotificationDeliveryAdapter() + { + using var provider = BuildProvider(); + using var scope = provider.CreateScope(); + + var adapters = scope.ServiceProvider.GetServices().ToList(); + + var email = Assert.Single(adapters); + Assert.IsType(email); + Assert.Equal(NotificationType.Email, email.Type); + } +}