Defer producer configuration building by nabisobhi · Pull Request #115 · dfds/dafda

nabisobhi · 2026-03-03T16:55:05Z

What changed

Added new AddProducerFor overloads that allow ProducerOptions to be created using the built IServiceProvider.

Previously, producers could only be configured at registration time:

services.AddProducerFor<MyProducer>(options =>
{
    options.WithBootstrapServers("...");
});

This required configuration to be defined before the DI container was available.

Now producers can also be configured using a factory:

services.AddProducerFor<MyProducer>(provider =>
{
    var configuration = provider.GetRequiredService<IConfiguration>();

    var options = new ProducerOptions();
    options.WithBootstrapServers(configuration["Kafka:BootstrapServers"]);

    return options;
});

Implementation

The producer factory is now registered using a DI-aware singleton:

services.AddSingleton(provider =>
{
    var options = optionsFactory(provider);
    return new ProducerFactory<TImplementation>(options);
});

This ensures that:

ProducerOptions can be constructed using services from the DI container
The ProducerFactory is created once per producer type
All producers resolved through DI share the same factory instance

ProducerFactory remains singleton because it holds configuration and infrastructure required to construct producer instances, while the producer services themselves are still registered as transient.

Why

The previous Action<ProducerOptions> approach built configuration too early and could not access services from the DI container.

This created limitations:

Producer configuration could not depend on other services (e.g. IConfiguration)
Integration tests could not override dependencies cleanly
Configuration values were effectively fixed at registration time

By allowing ProducerOptions to be created using IServiceProvider:

The full DI container is available when building configuration
Overrides (especially in integration tests) are respected
Configuration can depend on runtime services

Net Effect

Producers can now be configured using runtime dependencies
Improved integration test flexibility
Better alignment with dependency injection patterns
Clear separation between registration-time and runtime configuration

DFDS-Snyk · 2026-03-03T16:55:22Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

This reverts commit 32e716a.

toizo

it looks good to me, except the one mistake in the assertion

Copilot

Pull request overview

This PR updates Dafda’s producer registration APIs to support deferring ProducerOptions creation until the IServiceProvider is available, enabling DI-dependent configuration (e.g., IConfiguration) while keeping producer services transient and the underlying factory infrastructure singleton.

Changes:

Add AddProducerFor overloads that accept Func<IServiceProvider, ProducerOptions> for DI-aware producer configuration.
Refactor producer infrastructure: replace the old shared ProducerFactory implementation with a typed singleton factory per producer implementation type.
Update Kafka producer factory plumbing to accept IServiceProvider and adjust tests accordingly.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/Dafda/Producing/ProducerFactoryException.cs	Update XML docs to reference all `AddProducerFor` overloads that can throw duplicate-registration exceptions.
src/Dafda/Producing/ProducerFactory.cs	Remove legacy producer factory implementation.
src/Dafda/Producing/Producer.cs	Expose internal `KafkaProducer` for test verification and update usage accordingly.
src/Dafda/Configuration/ProducerServiceCollectionExtensions.cs	Add DI-aware overloads and introduce typed singleton `ProducerFactory<TImplementation>` used by transient producer services.
src/Dafda/Configuration/ProducerOptions.cs	Make `ProducerOptions` instantiable for factory-based configuration; update internal wiring for builder/registry.
src/Dafda/Configuration/ProducerConfigurationBuilder.cs	Change Kafka producer factory delegate to `Func<IServiceProvider, KafkaProducer>` and construct default producer using DI-resolved `ILoggerFactory`.
src/Dafda/Configuration/ProducerConfiguration.cs	Update stored Kafka producer factory delegate type to accept `IServiceProvider`.
src/Dafda/Configuration/OutboxServiceCollectionExtensions.cs	Update outbox producer creation to invoke `KafkaProducerFactory` with `IServiceProvider`.
src/Dafda/Configuration/OutboxProducerOptions.cs	Update internal Kafka producer factory delegate type to accept `IServiceProvider`.
src/Dafda.Tests/Producing/TestProducerFactory.cs	Remove tests for legacy `ProducerFactory` implementation.
src/Dafda.Tests/Configuration/TestServiceCollectionExtensions.cs	Add coverage for DI-aware producer options factories and singleton Kafka producer reuse; update name assertions.
src/Dafda.Tests/Builders/ProducerFactoryBuilder.cs	Remove legacy test builder tied to deleted factory.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…zation

toizo

Looks good to me now

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T13:06:47Z

+        public Producer CreateProducerInstance(IServiceProvider provider)
        {
-            var outgoingMessageRegistry = new OutgoingMessageRegistry();
-            var configurationBuilder = new ProducerConfigurationBuilder();
-            var consumerOptions = new ProducerOptions(configurationBuilder, outgoingMessageRegistry);
-            options?.Invoke(consumerOptions);
+            // Use LazyInitializer to ensure thread-safe, once-only construction while
+            // capturing `provider` in the factory delegate.
+            LazyInitializer.EnsureInitialized(
+                ref _kafkaProducer,
+                () => new Lazy<KafkaProducer>(() => _configuration.KafkaProducerFactory(provider), isThreadSafe: true));



ProducerFactory<TService> is registered as a singleton, but the lazy initialization captures the provider passed to CreateProducerInstance. If the first resolution happens from a scoped provider (e.g., inside an ASP.NET request), that scoped IServiceProvider (and potentially scoped services) will be retained by the singleton for the lifetime of the app, causing a scope leak and potentially resolving scoped dependencies into a singleton KafkaProducer. Consider capturing the root provider at singleton construction time (e.g., pass it into ProducerFactory<TService> via the AddSingleton(provider => ...) delegate) and use that stored root provider when creating the KafkaProducer instead of the per-resolution provider.

Very complex and edge case, but agree 😆

Copilot · 2026-03-24T13:06:47Z

+            if (services.Any(d => d.ServiceType == typeof(TService)))
+            {
+                throw new ProducerFactoryException(
+                    $"A producer has already been registered for service type \"{typeof(TService).FullName}\". Each producer must use a unique service type.");


The duplicate-registration guard checks whether any service descriptor already exists for TService, not whether an existing producer registration exists. In that case the exception message ("A producer has already been registered...") can be misleading because the existing registration might be unrelated. Consider rewording to indicate the service type is already registered (or tightening the check to detect Dafda producer registrations specifically).

Suggested change

$"A producer has already been registered for service type \"{typeof(TService).FullName}\". Each producer must use a unique service type.");

$"Service type \"{typeof(TService).FullName}\" is already registered in the dependency injection container. Each Dafda producer must use a unique service type.");

I think it makes more sense, change it.

Copilot · 2026-03-24T13:06:47Z

+            ThrowIfProducerServiceAlreadyRegisteredFor<TService>(services);
+


ThrowIfProducerServiceAlreadyRegisteredFor introduces new behavior (throwing ProducerFactoryException on duplicates), but the updated test suite no longer has coverage asserting that registering the same producer service type twice fails. Consider adding a test that calls AddProducerFor<T> twice for the same TService and asserts the expected ProducerFactoryException is thrown.

Added some tests

Copilot · 2026-03-24T13:06:48Z

+    /// Exception thrown when multiple identical producers are added via any overload of
+    /// <see cref="ProducerServiceCollectionExtensions.AddProducerFor{TClient}(Microsoft.Extensions.DependencyInjection.IServiceCollection,System.Action{Dafda.Configuration.ProducerOptions})"/>,
+    /// <see cref="ProducerServiceCollectionExtensions.AddProducerFor{TService,TImplementation}(Microsoft.Extensions.DependencyInjection.IServiceCollection,System.Action{Dafda.Configuration.ProducerOptions})"/>,
+    /// <see cref="ProducerServiceCollectionExtensions.AddProducerFor{TClient}(Microsoft.Extensions.DependencyInjection.IServiceCollection,System.Func{System.IServiceProvider,Dafda.Configuration.ProducerOptions})"/>, or
+    /// <see cref="ProducerServiceCollectionExtensions.AddProducerFor{TService,TImplementation}(Microsoft.Extensions.DependencyInjection.IServiceCollection,System.Func{System.IServiceProvider,Dafda.Configuration.ProducerOptions})"/>.


The exception is now thrown when AddProducerFor detects that TService is already present in the IServiceCollection, even if that existing registration is not a Dafda producer registration. The summary currently says "multiple identical producers" were added, which may be inaccurate/misleading in that scenario. Consider updating the XML doc to describe the actual condition (duplicate service type registration for a producer).

It helps better debugging it for the user, I believe. Updated it 👍🏻

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T13:42:02Z

    internal class ProducerConfiguration
    {
-        public ProducerConfiguration(IDictionary<string, string> configuration, MessageIdGenerator messageIdGenerator, Func<ILoggerFactory, KafkaProducer> kafkaProducerFactory)
+        public ProducerConfiguration(IDictionary<string, string> configuration, MessageIdGenerator messageIdGenerator, Func<IServiceProvider, KafkaProducer> kafkaProducerFactory)


ProducerConfiguration no longer references ILoggerFactory, but the file still imports Microsoft.Extensions.Logging. With TreatWarningsAsErrors=true, this will cause a build failure (CS8019). Remove the unused using directive.

zxfv

b3cfaca

nabisobhi changed the title ~~Defer producer configuration building~~ WIP: Defer producer configuration building Mar 3, 2026

nabisobhi added 10 commits March 6, 2026 12:09

rdet

54dc98a

drg

75bb722

Merge branch 'master' into nabi/new-add-producers

04970a2

dfg

cb40e0d

dfrg

87dea16

sd

bb01d3a

fdg

454d335

sdfg

4fc4cd0

sf

6a0542d

sdf

1251966

nabisobhi changed the title ~~WIP: Defer producer configuration building~~ WIP: Add IServiceProvider-based configuration for producers Mar 10, 2026

nabisobhi changed the title ~~WIP: Add IServiceProvider-based configuration for producers~~ WIP: Defer consumer configuration building Mar 10, 2026

nabisobhi changed the title ~~WIP: Defer consumer configuration building~~ WIP: Defer producer configuration building Mar 10, 2026

nabisobhi added 2 commits March 12, 2026 14:18

sad

7beac1c

sd

d0bc2ea

nabisobhi changed the title ~~WIP: Defer producer configuration building~~ Defer producer configuration building Mar 16, 2026

nabisobhi marked this pull request as ready for review March 16, 2026 09:51

toizo reviewed Mar 16, 2026

View reviewed changes

Comment thread src/Dafda/Configuration/ProducerServiceCollectionExtensions.cs Outdated

nabisobhi added 5 commits March 17, 2026 08:24

revert namesapce styling change

f8dad6c

revert

6e34e8e

revert unwanted

32e716a

Revert "revert unwanted"

5093e17

This reverts commit 32e716a.

fix doc

7a88fda

toizo reviewed Mar 24, 2026

View reviewed changes

Comment thread src/Dafda.Tests/Configuration/TestServiceCollectionExtensions.cs Outdated

toizo requested a review from Copilot March 24, 2026 10:22

Copilot started reviewing on behalf of toizo March 24, 2026 10:22 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

nabisobhi added 3 commits March 24, 2026 13:08

fix test

8b0eea1

ProducerFactory<TService>

5823274

add test for disposing

c732d27

nabisobhi requested a review from Copilot March 24, 2026 12:26

Copilot started reviewing on behalf of nabisobhi March 24, 2026 12:27 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Comment thread src/Dafda/Configuration/ProducerServiceCollectionExtensions.cs Outdated

refactor Producer to use Lazy<KafkaProducer> for thread-safe initiali…

80e5227

…zation

toizo requested a review from Copilot March 24, 2026 13:00

Copilot started reviewing on behalf of toizo March 24, 2026 13:01 View session

toizo approved these changes Mar 24, 2026

View reviewed changes

Copilot AI reviewed Mar 24, 2026

View reviewed changes

copilot

9697f76

nabisobhi requested a review from Copilot March 24, 2026 13:36

Copilot started reviewing on behalf of nabisobhi March 24, 2026 13:36 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

nabisobhi added 2 commits March 24, 2026 14:43

copilot

f41389e

build again

c1c0992

toizo merged commit 7ad2069 into dfds:master Mar 24, 2026
4 checks passed

nabisobhi deleted the nabi/new-add-producers branch March 24, 2026 14:04

	$"A producer has already been registered for service type \"{typeof(TService).FullName}\". Each producer must use a unique service type.");
	$"Service type \"{typeof(TService).FullName}\" is already registered in the dependency injection container. Each Dafda producer must use a unique service type.");

		ThrowIfProducerServiceAlreadyRegisteredFor<TService>(services);

Conversation

nabisobhi commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Implementation

Why

Net Effect

Uh oh!

DFDS-Snyk commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

Uh oh!

toizo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

toizo left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

nabisobhi Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

nabisobhi Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

nabisobhi Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

nabisobhi Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nabisobhi commented Mar 3, 2026 •

edited

Loading

DFDS-Snyk commented Mar 3, 2026 •

edited

Loading