Skip to content

fix(di): share store singletons and preserve registrations #622

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.

Sign up for GitHub

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

Open
nmummau wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

fix(di): share store singletons and preserve registrations #622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 46 additions & 38 deletions src/Extensions/src/Eventuous.Extensions.DependencyInjection/Registrations/Stores.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,148 +13,156 @@ public static partial class ServiceCollectionExtensions {
/// <param name="services"></param>
extension(IServiceCollection services) {
/// <summary>
/// Registers the event reader
/// Registers a singleton event reader, preserving existing registrations.
/// </summary>
/// <typeparam name="T">Implementation of <see cref="IEventReader"/></typeparam>
/// <returns></returns>
public IServiceCollection AddEventReader<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>() where T : class, IEventReader {
services.TryAddSingleton<T>();

if (EventuousDiagnostics.Enabled) {
services.TryAddSingleton<T>();
services.TryAddSingleton(sp => TracedEventReader.Trace(sp.GetRequiredService<T>()));
}
else { services.TryAddSingleton<IEventReader, T>(); }
else { services.TryAddSingleton<IEventReader>(sp => sp.GetRequiredService<T>()); }

return services;
}

/// <summary>
/// Registers the event reader
/// Registers a singleton event reader, preserving existing registrations.
/// </summary>
/// <param name="getService">Function to create an instance of <see cref="IEventReader"/></param>
/// <typeparam name="T">Implementation of <see cref="IEventReader"/></typeparam>
/// <returns></returns>
public IServiceCollection AddEventReader<T>(Func<IServiceProvider, T> getService) where T : class, IEventReader {
services.TryAddSingleton(getService);

if (EventuousDiagnostics.Enabled) {
services.TryAddSingleton(getService);
services.TryAddSingleton(sp => TracedEventReader.Trace(sp.GetRequiredService<T>()));
}
else { services.TryAddSingleton<IEventReader>(getService); }
else { services.TryAddSingleton<IEventReader>(sp => sp.GetRequiredService<T>()); }

return services;
}

/// <summary>
/// Registers the event writer
/// Registers a singleton event writer, preserving existing registrations.
/// </summary>
/// <typeparam name="T">Implementation of <see cref="IEventWriter"/></typeparam>
/// <returns></returns>
public IServiceCollection AddEventWriter<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>() where T : class, IEventWriter {
services.TryAddSingleton<T>();

if (EventuousDiagnostics.Enabled) {
services.TryAddSingleton<T>();
services.TryAddSingleton(sp => TracedEventWriter.Trace(sp.GetRequiredService<T>()));
}
else { services.TryAddSingleton<IEventWriter, T>(); }
else { services.TryAddSingleton<IEventWriter>(sp => sp.GetRequiredService<T>()); }

return services;
}

/// <summary>
/// Registers the event writer
/// Registers a singleton event writer, preserving existing registrations.
/// </summary>
/// <param name="getService">Function to create an instance of <see cref="IEventWriter"/></param>
/// <typeparam name="T">Implementation of <see cref="IEventWriter"/></typeparam>
/// <returns></returns>
public IServiceCollection AddEventWriter<T>(Func<IServiceProvider, T> getService) where T : class, IEventWriter {
services.TryAddSingleton(getService);

if (EventuousDiagnostics.Enabled) {
services.TryAddSingleton(getService);
services.TryAddSingleton(sp => TracedEventWriter.Trace(sp.GetRequiredService<T>()));
}
else { services.TryAddSingleton<IEventWriter>(getService); }
else { services.TryAddSingleton<IEventWriter>(sp => sp.GetRequiredService<T>()); }

return services;
}

/// <summary>
/// Registers the event reader and writer
/// Registers a shared singleton event reader and writer, preserving existing registrations.
/// </summary>
/// <typeparam name="T">Implementation of <see cref="IEventWriter"/> and <see cref="IEventReader"/></typeparam>
/// <returns></returns>
public IServiceCollection AddEventReaderWriter<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>() where T : class, IEventWriter, IEventReader {
services.TryAddSingleton<T>();

if (EventuousDiagnostics.Enabled) {
services.TryAddSingleton<T>();
services.TryAddSingleton(sp => TracedEventReader.Trace(sp.GetRequiredService<T>()));
services.TryAddSingleton(sp => TracedEventWriter.Trace(sp.GetRequiredService<T>()));
}
else {
services.TryAddSingleton<IEventReader, T>();
services.TryAddSingleton<IEventWriter, T>();
services.TryAddSingleton<IEventReader>(sp => sp.GetRequiredService<T>());
services.TryAddSingleton<IEventWriter>(sp => sp.GetRequiredService<T>());
}

return services;
}

/// <summary>
/// Registers the event reader and writer implemented by one class
/// Registers a shared singleton event reader and writer, preserving existing registrations.
/// </summary>
/// <param name="getService">Function to create an instance of the class,
/// which implements both <see cref="IEventReader"/> and <see cref="IEventWriter"/></param>
/// <typeparam name="T">Implementation of <see cref="IEventWriter"/></typeparam>
/// <returns></returns>
public IServiceCollection AddEventReaderWriter<T>(Func<IServiceProvider, T> getService)
where T : class, IEventWriter, IEventReader {
services.TryAddSingleton(getService);

if (EventuousDiagnostics.Enabled) {
services.TryAddSingleton(getService);
services.TryAddSingleton(sp => TracedEventReader.Trace(sp.GetRequiredService<T>()));
services.TryAddSingleton(sp => TracedEventWriter.Trace(sp.GetRequiredService<T>()));
}
else {
services.TryAddSingleton<IEventWriter>(getService);
services.TryAddSingleton<IEventReader>(getService);
services.TryAddSingleton<IEventWriter>(sp => sp.GetRequiredService<T>());
services.TryAddSingleton<IEventReader>(sp => sp.GetRequiredService<T>());
}

return services;
}

/// <summary>
/// Registers an event store service as reader, writer, and event store
/// Registers one singleton as reader, writer, and event store, preserving existing registrations.
/// </summary>
/// <typeparam name="T">Implementation of <see cref="IEventStore"/> </typeparam>
/// <returns></returns>
public IServiceCollection AddEventStore<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] T>() where T : class, IEventStore {
services.TryAddSingleton<T>();

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Oct 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

3. Unused store types can prevent host builds 🐞 Bug ≡ Correctness

The generic AddEventStore<T> overload now registers T even when existing reader, writer, and
store registrations cause every conditional interface registration to be skipped. With tracing
disabled and build-time service validation enabled, an unused T whose constructor has an
unresolved dependency can therefore fail host construction, although the existing interfaces need no
replacement.
Agent Prompt 
## Issue description
An existing registration for every store interface leaves the new concrete registration unused, but build-time validation can still reject its constructor.
## Fix Focus Areas
- src/Extensions/src/Eventuous.Extensions.DependencyInjection/Registrations/Stores.cs[129-141]
## Recommended Fix
When no interface or wrapper will need the concrete store, avoid adding its descriptor. Add a regression test with all three interfaces pre-registered, an unconstructable replacement type, tracing disabled, and build-time validation enabled.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


if (EventuousDiagnostics.Enabled) {
services.TryAddSingleton<T>();
services.AddSingleton(sp => new TracedEventStore(sp.GetRequiredService<T>()));
services.AddSingleton<IEventStore>(sp => sp.GetRequiredService<TracedEventStore>());
services.AddSingleton<IEventReader>(sp => sp.GetRequiredService<TracedEventStore>());
services.AddSingleton<IEventWriter>(sp => sp.GetRequiredService<TracedEventStore>());
services.TryAddSingleton(sp => new TracedEventStore(sp.GetRequiredService<T>()));
services.TryAddSingleton<IEventStore>(sp => sp.GetRequiredService<TracedEventStore>());
services.TryAddSingleton<IEventReader>(sp => sp.GetRequiredService<TracedEventStore>());
services.TryAddSingleton<IEventWriter>(sp => sp.GetRequiredService<TracedEventStore>());
}
else {
services.TryAddSingleton<IEventReader, T>();
services.TryAddSingleton<IEventWriter, T>();
services.TryAddSingleton<IEventStore, T>();
services.TryAddSingleton<IEventReader>(sp => sp.GetRequiredService<T>());
services.TryAddSingleton<IEventWriter>(sp => sp.GetRequiredService<T>());
services.TryAddSingleton<IEventStore>(sp => sp.GetRequiredService<T>());
Comment on lines +139 to +141

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Oct 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Disposable stores can be disposed repeatedly 🐞 Bug ☼ Reliability

When tracing is disabled, AddEventStore<T> registers T and three singleton interface factories
that each return that same instance. If a custom store implements IDisposable or
IAsyncDisposable and the interfaces are resolved, the container tracks the factory results
separately, so host shutdown can dispose the store multiple times; the aliases can also dispose an
externally supplied instance.
Agent Prompt 
## Issue description
Factory-based interface aliases can make the container track one disposable store several times, including a store supplied as an externally owned instance.
## Fix Focus Areas
- src/Extensions/src/Eventuous.Extensions.DependencyInjection/Registrations/Stores.cs[129-141]
- src/Extensions/src/Eventuous.Extensions.DependencyInjection/Registrations/Stores.cs[153-165]
## Recommended Fix
Revise the alias and ownership strategy so resolving every interface shares the backing store without registering multiple disposable factory results for that object. Cover both container-created disposable stores and pre-registered instances with shutdown tests.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines +139 to +141

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Oct 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Scoped stores can break singleton resolution 🐞 Bug ☼ Reliability

TryAddSingleton<T>() leaves an existing scoped registration of T in place, but the new singleton
interface factories resolve that scoped T. When a host with scope validation resolves an
interface, resolution throws; without validation, the singleton can retain a scoped instance beyond
its intended lifetime.
Agent Prompt 
## Issue description
A pre-registered scoped concrete store can become a dependency of the newly added singleton interface aliases.
## Fix Focus Areas
- src/Extensions/src/Eventuous.Extensions.DependencyInjection/Registrations/Stores.cs[129-141]
- src/Extensions/src/Eventuous.Extensions.DependencyInjection/Registrations/Stores.cs[153-165]
## Recommended Fix
Before adding singleton aliases to an existing concrete registration, detect a non-singleton lifetime and reject it with a clear registration-time error, or choose an explicit lifetime policy that does not capture scoped services. Add a scope-validation regression test.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

return services;
}

/// <summary>
/// Registers an event store service as reader, writer, and event store
/// Registers one singleton as reader, writer, and event store, preserving existing registrations.
/// </summary>
/// <param name="getService">Function to create an instance of the class, which implements <see cref="IEventStore"/></param>
/// <typeparam name="T">Implementation of <see cref="IEventStore"/></typeparam>
/// <returns></returns>
public IServiceCollection AddEventStore<T>(Func<IServiceProvider, T> getService) where T : class, IEventStore {
services.TryAddSingleton(getService);

if (EventuousDiagnostics.Enabled) {
services.TryAddSingleton(getService);
services.AddSingleton(sp => new TracedEventStore(sp.GetRequiredService<T>()));
services.AddSingleton<IEventStore>(sp => sp.GetRequiredService<TracedEventStore>());
services.AddSingleton<IEventReader>(sp => sp.GetRequiredService<TracedEventStore>());
services.AddSingleton<IEventWriter>(sp => sp.GetRequiredService<TracedEventStore>());
services.TryAddSingleton(sp => new TracedEventStore(sp.GetRequiredService<T>()));
services.TryAddSingleton<IEventStore>(sp => sp.GetRequiredService<TracedEventStore>());
services.TryAddSingleton<IEventReader>(sp => sp.GetRequiredService<TracedEventStore>());
services.TryAddSingleton<IEventWriter>(sp => sp.GetRequiredService<TracedEventStore>());
}
else {
services.TryAddSingleton<IEventWriter>(getService);
services.TryAddSingleton<IEventReader>(getService);
services.TryAddSingleton<IEventStore>(getService);
services.TryAddSingleton<IEventWriter>(sp => sp.GetRequiredService<T>());
services.TryAddSingleton<IEventReader>(sp => sp.GetRequiredService<T>());
services.TryAddSingleton<IEventStore>(sp => sp.GetRequiredService<T>());
}

return services;
Expand Down
1 change: 1 addition & 0 deletions ...sions/test/Eventuous.Tests.DependencyInjection/Eventuous.Tests.DependencyInjection.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

<ItemGroup>
<ProjectReference Include="$(LocalRoot)\Eventuous.Extensions.DependencyInjection\Eventuous.Extensions.DependencyInjection.csproj" />
<ProjectReference Include="$(SrcRoot)\Testing\src\Eventuous.Testing\Eventuous.Testing.csproj" />
</ItemGroup>

<ItemGroup>
Expand Down
180 changes: 180 additions & 0 deletions src/Extensions/test/Eventuous.Tests.DependencyInjection/StoreRegistrationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
using Eventuous.Diagnostics;
using Eventuous.Testing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Shouldly;

namespace Eventuous.Tests.DependencyInjection;

// Diagnostics is process-wide. No other tests may observe the temporary setting.
[NotInParallel]
public class StoreRegistrationTests {
[Test]
[Arguments(false, false, Registration.Store)]
[Arguments(false, true, Registration.Store)]
[Arguments(true, false, Registration.Store)]
[Arguments(true, true, Registration.Store)]
[Arguments(false, false, Registration.ReaderWriter)]
[Arguments(false, true, Registration.ReaderWriter)]
[Arguments(true, false, Registration.ReaderWriter)]
[Arguments(true, true, Registration.ReaderWriter)]
[Arguments(false, false, Registration.Separate)]
[Arguments(false, true, Registration.Separate)]
[Arguments(true, false, Registration.Separate)]
[Arguments(true, true, Registration.Separate)]
public async Task WrittenEventsAreVisibleThroughEveryRegisteredInterface(bool tracing, bool factory, Registration registration) {
using var diagnostics = new DiagnosticsSetting(tracing);
var factoryCalls = 0;
using var host = new HostBuilder().ConfigureServices(services => {
Register(services, registration, factory, _ => { factoryCalls++; return new(); });
}).Build();
await host.StartAsync();

var writer = host.Services.GetRequiredService<IEventWriter>();
var reader = host.Services.GetRequiredService<IEventReader>();
var stream = new StreamName("shared-store");
await Append(writer, stream, "first");
(await Read(reader, stream)).ShouldBe(new[] { "first" });
if (registration == Registration.Store) {
var store = host.Services.GetRequiredService<IEventStore>();
(await store.StreamExists(stream)).ShouldBeTrue();
await Append(store, stream, "second", new(0));
(await Read(reader, stream)).ShouldBe(new[] { "first", "second" });
}

using (var scope = host.Services.CreateScope()) {
scope.ServiceProvider.GetRequiredService<IEventReader>().ShouldBeSameAs(reader);
scope.ServiceProvider.GetRequiredService<IEventWriter>().ShouldBeSameAs(writer);
}
if (factory) factoryCalls.ShouldBe(1);
await host.StopAsync();
}

[Test]
[Arguments(false, false, true)]
[Arguments(false, true, true)]
[Arguments(true, false, true)]
[Arguments(true, true, true)]
[Arguments(false, false, false)]
[Arguments(false, true, false)]
[Arguments(true, false, false)]
[Arguments(true, true, false)]
public async Task StandaloneRegistrationUsesTheExistingBackingStore(bool tracing, bool factory, bool readerOnly) {
using var diagnostics = new DiagnosticsSetting(tracing);
var backing = new InMemoryEventStore();
var stream = new StreamName("standalone-store");
var factoryCalls = 0;
using var host = new HostBuilder().ConfigureServices(services => {
services.AddSingleton(backing);
InMemoryEventStore Create(IServiceProvider _) { factoryCalls++; return new(); }
if (readerOnly) {
if (factory) services.AddEventReader<InMemoryEventStore>(Create);
else services.AddEventReader<InMemoryEventStore>();
}
else {
if (factory) services.AddEventWriter<InMemoryEventStore>(Create);
else services.AddEventWriter<InMemoryEventStore>();
}
}).Build();
await host.StartAsync();

if (readerOnly) {
await Append(backing, stream, "seeded");
(await Read(host.Services.GetRequiredService<IEventReader>(), stream)).ShouldBe(new[] { "seeded" });
}
else {
await Append(host.Services.GetRequiredService<IEventWriter>(), stream, "written");
(await Read(backing, stream)).ShouldBe(new[] { "written" });
}
factoryCalls.ShouldBe(0);
await host.StopAsync();
}

[Test]
[Arguments(false, false)]
[Arguments(false, true)]
[Arguments(true, false)]
[Arguments(true, true)]
public async Task RepeatedRegistrationKeepsOneServiceAndOneFactory(bool tracing, bool factory) {
using var diagnostics = new DiagnosticsSetting(tracing);
var firstCalls = 0;
var secondCalls = 0;
using var host = new HostBuilder().ConfigureServices(services => {
Register(services, Registration.Store, factory, _ => { firstCalls++; return new(); });
Register(services, Registration.Store, factory, _ => { secondCalls++; return new(); });
}).Build();
await host.StartAsync();

var writer = host.Services.GetServices<IEventWriter>().ShouldHaveSingleItem();
var reader = host.Services.GetServices<IEventReader>().ShouldHaveSingleItem();
var store = host.Services.GetServices<IEventStore>().ShouldHaveSingleItem();
var stream = new StreamName("repeated-store");
await Append(writer, stream, "once");
(await Read(reader, stream)).ShouldBe(new[] { "once" });
(await store.StreamExists(stream)).ShouldBeTrue();
firstCalls.ShouldBe(factory ? 1 : 0);
secondCalls.ShouldBe(0);
await host.StopAsync();
}

[Test]
[Arguments(false, false)]
[Arguments(false, true)]
[Arguments(true, false)]
[Arguments(true, true)]
public async Task RegistrationPreservesExplicitInterfaceOverrides(bool tracing, bool factory) {
using var diagnostics = new DiagnosticsSetting(tracing);
var existing = new InMemoryEventStore();
var factoryCalls = 0;
using var host = new HostBuilder().ConfigureServices(services => {
services.AddSingleton<IEventStore>(existing);
services.AddSingleton<IEventReader>(existing);
services.AddSingleton<IEventWriter>(existing);
Register(services, Registration.Store, factory, _ => { factoryCalls++; return new(); });
}).Build();
await host.StartAsync();

var stream = new StreamName("explicit-store");
await Append(host.Services.GetRequiredService<IEventWriter>(), stream, "preserved");
(await Read(existing, stream)).ShouldBe(new[] { "preserved" });
host.Services.GetRequiredService<IEventReader>().ShouldBeSameAs(existing);
host.Services.GetRequiredService<IEventStore>().ShouldBeSameAs(existing);
factoryCalls.ShouldBe(0);
await host.StopAsync();
}

static void Register(IServiceCollection services, Registration registration, bool factory, Func<IServiceProvider, InMemoryEventStore> create) {
switch (registration) {
case Registration.Store:
if (factory) services.AddEventStore(create);
else services.AddEventStore<InMemoryEventStore>();
break;
case Registration.ReaderWriter:
if (factory) services.AddEventReaderWriter(create);
else services.AddEventReaderWriter<InMemoryEventStore>();
break;
case Registration.Separate:
if (factory) services.AddEventReader(create).AddEventWriter(create);
else services.AddEventReader<InMemoryEventStore>().AddEventWriter<InMemoryEventStore>();
break;
}
}

static Task<AppendEventsResult> Append(IEventWriter writer, StreamName stream, string value, ExpectedStreamVersion? version = null)
=> writer.AppendEvents(stream, version ?? ExpectedStreamVersion.NoStream, new[] { new NewStreamEvent(Guid.NewGuid(), value, new()) }, default);

static async Task<string[]> Read(IEventReader reader, StreamName stream)
=> (await reader.ReadEvents(stream, StreamReadPosition.Start, 10, true, default)).Select(x => (string)x.Payload!).ToArray();

public enum Registration { Store, ReaderWriter, Separate }

sealed class DiagnosticsSetting : IDisposable {
readonly bool _previous = EventuousDiagnostics.Enabled;
public DiagnosticsSetting(bool enabled) => Set(enabled);
public void Dispose() => Set(_previous);
static void Set(bool enabled) {
if (enabled) EventuousDiagnostics.Enable();
else EventuousDiagnostics.Disable();
}
}
}
Loading