From 3ee8749d3e1181153f52ac0ab094308557fe1a3f Mon Sep 17 00:00:00 2001 From: Sadeq Abu-Hattem Date: Mon, 2 Nov 2026 21:04:19 +0200 Subject: [PATCH 1/3] Give SettingWrite the settings class it was made to SettingWrite named the group by GroupName and Route but not by type, so an observer that needed the class's attributes had to rebuild the map itself: scan an assembly, call SettingGroupAttribute.ResolveName on every [SettingGroup] class, and throw for a group outside the scanned assembly. Auditing needs the type to apply [AuditIgnore] and [AuditRedact], and the sample's audit observer did exactly that scan. GroupType is an init property rather than a new positional parameter. A positional one changes the constructor and Deconstruct, which breaks compiled callers even with a default value. As a result it is nullable: the library sets it on every write it raises, and it is null only on a SettingWrite built elsewhere, such as in a test. The manager's TypeMap now carries the Type, so NotifyWrittenAsync sets it once and all three write paths (group set, single-property set, clear) get it. The docs say to key a group on GroupName, not on the type's name. Rows are found by the [SettingGroup] Name, or the class's simple name, so moving a class to another namespace changes its FullName but not what it reads; a record keyed on the type would split one group's history in two. Verified: the solution builds with 0 warnings, and the unit tests pass (189 x2, net8.0 and net10.0), including a new one that checks GroupType on all three write paths. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +++ docs/write-notifications.md | 5 +++++ .../Interfaces/ISettingWriteObserver.cs | 18 +++++++++++++++- .../Services/SettingManager.cs | 9 +++++--- .../SettingWriteObserverTests.cs | 21 +++++++++++++++++++ 5 files changed, 52 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28acb94..43f17f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ This project follows [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Added +- `SettingWrite.GroupType`: the settings class a write was made to. An observer that needs the group's attributes no longer has to scan assemblies for a class whose resolved name matches `GroupName`. It is an init property, so code that constructs a `SettingWrite` keeps compiling. + ### Changed - Aspire moved from 13.5.2 to 13.6.4, and Microsoft.Extensions.* and EF Core packages from 10.0.11 to 10.0.12 — the floor Aspire 13.6 requires. - Microsoft.Data.SqlClient (integration tests and sample only) moved from 6.1.6 to 6.1.8, which takes System.Security.Cryptography.Pkcs 9.0.21 and clears CVE-2026-70162 and CVE-2026-84008. diff --git a/docs/write-notifications.md b/docs/write-notifications.md index c268810..0e34bfb 100644 --- a/docs/write-notifications.md +++ b/docs/write-notifications.md @@ -43,6 +43,11 @@ Several observers can be registered; all of them run, in registration order. | `Actor` | Who made the change, or `null` for "not captured" | | `CorrelationId` | One value per write, so the several changes of one save are identifiable as one edit | | `OccurredAt` | UTC timestamp | +| `GroupType` | The settings class, for reading its attributes | + +Identify a group by `GroupName`, not by `GroupType`'s name. Moving the class to another +namespace changes its full name but not the rows it reads, so anything keyed on the type name +would split one group's history in two. Each `SettingChange` is `(PropertyName, OldValue, NewValue, IsRedacted)`. `OldValue` is `null` when the property had never been stored. diff --git a/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteObserver.cs b/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteObserver.cs index 05efceb..3971388 100644 --- a/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteObserver.cs +++ b/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteObserver.cs @@ -67,7 +67,23 @@ public sealed record SettingWrite( IReadOnlyList Changes, string? Actor, string CorrelationId, - DateTime OccurredAt); + DateTime OccurredAt) +{ + /// + /// The settings class the group is defined by, so an observer can read its attributes + /// without rebuilding the name-to-type map by scanning assemblies. + /// + /// + /// Always set on a write the library raises. null only on a + /// constructed elsewhere, such as in a test. + /// + /// Identify the group by , not by this type's name: moving the class + /// to another namespace changes but not the rows it reads, so a + /// record keyed on the type would split one group's history in two. + /// + /// + public Type? GroupType { get; init; } +} /// /// Observes every completed write to every settings group. This is the hook an auditing diff --git a/src/DotNetBoost.Settings.Core/Services/SettingManager.cs b/src/DotNetBoost.Settings.Core/Services/SettingManager.cs index cea2ff9..4d32a32 100644 --- a/src/DotNetBoost.Settings.Core/Services/SettingManager.cs +++ b/src/DotNetBoost.Settings.Core/Services/SettingManager.cs @@ -464,7 +464,10 @@ private async Task NotifyWrittenAsync( // One id for the whole write. The ambient Activity when there is one, so the edit // joins the surrounding trace rather than being correlatable only with itself. CorrelationId: Activity.Current?.Id ?? Guid.NewGuid().ToString("n"), - OccurredAt: DateTime.UtcNow); + OccurredAt: DateTime.UtcNow) + { + GroupType = map.Type + }; foreach (var observer in observers) { @@ -607,7 +610,7 @@ private static TypeMap GetTypeMap(Type type) var route = t.GetCustomAttribute()?.Route; var name = SettingGroupAttribute.ResolveName(t); - return new TypeMap(name, string.IsNullOrWhiteSpace(route) ? name : route, props); + return new TypeMap(t, name, string.IsNullOrWhiteSpace(route) ? name : route, props); }); private static PropertyMap BuildPropertyMap(PropertyInfo p) @@ -717,7 +720,7 @@ private bool TryRowsFromCache(string key, out Setting[]? rows) /// What this instance's cached copy of a group was loaded under, and when it was last re-checked. private sealed record CacheProbe(string? Version, DateTimeOffset CheckedAt); - private sealed record TypeMap(string GroupName, string Route, IReadOnlyList Properties); + private sealed record TypeMap(Type Type, string GroupName, string Route, IReadOnlyList Properties); private sealed record PropertyMap( string Name, diff --git a/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs b/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs index 658f63c..658c43d 100644 --- a/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs +++ b/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs @@ -196,6 +196,27 @@ public async Task Clear_RaisesAClearedNotification_WithNoChanges() Assert.Equal(nameof(ObservedSettings), write.GroupName); } + /// + /// What an observer needs to read the group's attributes without scanning assemblies for + /// a class whose resolved name matches. Checked on all three write paths, since each + /// builds its notification separately. + /// + [Fact] + public async Task Write_CarriesTheGroupType_OnEveryWritePath() + { + var (mgr, _, observer) = Build(); + var accessor = mgr.For(); + + await accessor.SetAsync(new ObservedSettings { Host = "a" }); + await accessor.SetAsync(x => x.Port, 587); + await accessor.ClearAsync(); + + Assert.Equal( + [SettingWriteKind.Updated, SettingWriteKind.Updated, SettingWriteKind.Cleared], + observer.Writes.Select(w => w.Kind)); + Assert.All(observer.Writes, w => Assert.Equal(typeof(ObservedSettings), w.GroupType)); + } + /// /// An observer runs after the store write committed and the cache entry was evicted, so /// failing the call would report a write that in fact succeeded. From 72e6b95ea6cd00c3c266e79b5205b480bd84e0b6 Mon Sep 17 00:00:00 2001 From: Sadeq Abu-Hattem Date: Mon, 2 Nov 2026 21:08:27 +0200 Subject: [PATCH 2/3] Count observer and change-handler failures on a meter Write observers and change handlers run after the write commits, so their exceptions are logged and swallowed (events 1007 and 1002). That stays. But the log line was the only trace: nothing counted them, so a steadily failing observer showed up only for someone reading logs. This adds the library's first meter, DotNetBoost.Settings, with two counters beside those log lines: dotnetboost.settings.write_observer.failures and dotnetboost.settings.change_handler.failures. Each is tagged with the implementation's class name (as in the log line), the group, and error.type. Every tag is bounded by the application's own types, never by a value. The meter comes from the container's IMeterFactory when there is one, which every generic host registers, so each host, a test host included, gets its own meter, disposed with it. Without a factory (a bare ServiceCollection, or a manager constructed by hand) the counters fall back to one process-wide meter. SettingMetrics is internal and is resolved from the manager's service provider only when something fails, the same way observers and handlers are resolved, so SettingManager's public constructor is unchanged. No OnObserverFailure(Throw) option, though the gap notes first proposed one. ChangeSignalPublisher is itself an observer, so it would surface change-signal failures the read path deliberately tolerates; it would skip the change handlers that run after observers; and reporting a committed write as failed invites a retry. Tests can assert the counter instead. The sample exports the meter, and the write-notifications doc lists both instruments and their tags. Verified: the solution builds with 0 warnings; unit (193 x2), provider (54 x2) and API (51 x2) tests pass. Both counter tests fail with the two counting calls removed. Integration tests were not run; nothing they cover changed. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + docs/change-notifications.md | 3 +- docs/write-notifications.md | 20 +++ samples/SampleApp/Program.cs | 4 +- .../Services/SettingManager.cs | 8 + .../Services/SettingMetrics.cs | 79 ++++++++++ .../SettingServiceCollectionExtensions.cs | 4 + .../SettingWriteObserverTests.cs | 146 ++++++++++++++++++ 8 files changed, 263 insertions(+), 2 deletions(-) create mode 100644 src/DotNetBoost.Settings.Core/Services/SettingMetrics.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 43f17f7..0686032 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ This project follows [Semantic Versioning](https://semver.org/). ### Added - `SettingWrite.GroupType`: the settings class a write was made to. An observer that needs the group's attributes no longer has to scan assemblies for a class whose resolved name matches `GroupName`. It is an init property, so code that constructs a `SettingWrite` keeps compiling. +- The library's first metrics, on a `DotNetBoost.Settings` meter: `dotnetboost.settings.write_observer.failures` and `dotnetboost.settings.change_handler.failures`, each tagged with the implementation's class name, the group and `error.type`. Observer and handler exceptions are still logged and swallowed; until now the log line was the only trace of them. The meter is created through the container's `IMeterFactory` when there is one. ### Changed - Aspire moved from 13.5.2 to 13.6.4, and Microsoft.Extensions.* and EF Core packages from 10.0.11 to 10.0.12 — the floor Aspire 13.6 requires. diff --git a/docs/change-notifications.md b/docs/change-notifications.md index 43726f4..61e2e59 100644 --- a/docs/change-notifications.md +++ b/docs/change-notifications.md @@ -39,6 +39,7 @@ the wrong one is mostly a matter of fighting the data you are handed. Both are **in-process only**, and both have the same failure policy: **an exception is logged and swallowed**, so neither can fail a write that has already committed, and one failing handler -never skips the ones after it. +never skips the ones after it. Each failure is also counted on the `DotNetBoost.Settings` meter; +see [Write notifications](write-notifications.md#failure-policy). Registering both is normal — they are not alternatives. diff --git a/docs/write-notifications.md b/docs/write-notifications.md index 0e34bfb..917ae36 100644 --- a/docs/write-notifications.md +++ b/docs/write-notifications.md @@ -91,6 +91,26 @@ deliberately not the one for [`ISettingProjector`](rest-api.md#reshaping-what which *can* fail a request — a failed notification must not break a committed write, while a failed projection would otherwise return a wrong response. +So an observer is the wrong place for anything that must not be lost. Its failures are counted +rather than surfaced: each one also adds 1 to a counter on the `DotNetBoost.Settings` meter, so +a rate of them shows up on a dashboard instead of only in the logs. + +| Instrument | Counts | Tags | +|---|---|---| +| `dotnetboost.settings.write_observer.failures` | Write observers that threw | `dotnetboost.settings.observer`, `dotnetboost.settings.group`, `error.type` | +| `dotnetboost.settings.change_handler.failures` | [Change handlers](change-notifications.md) that threw | `dotnetboost.settings.handler`, `dotnetboost.settings.group`, `error.type` | + +The observer and handler tags are the implementation's class name, as in the log line. Every +tag is bounded by your own types, never by a value. To export them with OpenTelemetry: + +```csharp +builder.Services.AddOpenTelemetry() + .WithMetrics(metrics => metrics.AddMeter("DotNetBoost.Settings")); +``` + +The meter is created through the container's `IMeterFactory`, which every generic host +registers, so each host — a test host included — gets its own. + ## The stopgap: logging writes `LoggingSettingWriteObserver` ships in Core so the hook is not an empty port: diff --git a/samples/SampleApp/Program.cs b/samples/SampleApp/Program.cs index 7d5ce8c..c356110 100644 --- a/samples/SampleApp/Program.cs +++ b/samples/SampleApp/Program.cs @@ -48,12 +48,14 @@ // telemetry — Npgsql's ActivitySource and Meter, so a trace on the dashboard shows the // SQL a request ran instead of stopping at the HTTP span, and the Metrics // page carries the connection-pool counters. +// The DotNetBoost.Settings meter is this library's own, not Npgsql's: it counts write observers +// and change handlers that threw, which are otherwise only a log line. builder.Services.AddHealthChecks() .AddDbContextCheck(); builder.Services.AddOpenTelemetry() .WithTracing(tracing => tracing.AddSource("Npgsql")) - .WithMetrics(metrics => metrics.AddMeter("Npgsql")); + .WithMetrics(metrics => metrics.AddMeter("Npgsql", "DotNetBoost.Settings")); // ───────────────────────────────────────────────────────────────────────────── // Storage provider — ALTERNATIVE: SQL Server (Entity Framework Core) diff --git a/src/DotNetBoost.Settings.Core/Services/SettingManager.cs b/src/DotNetBoost.Settings.Core/Services/SettingManager.cs index 4d32a32..bc53505 100644 --- a/src/DotNetBoost.Settings.Core/Services/SettingManager.cs +++ b/src/DotNetBoost.Settings.Core/Services/SettingManager.cs @@ -54,6 +54,12 @@ private static readonly ConditionalWeakTable Probes => ProbesByCache.GetOrCreateValue(_cache); + /// + /// Resolved when something fails rather than in the constructor, the way observers and + /// handlers are, so the public constructor stays as it is and the success path pays nothing. + /// + private SettingMetrics Metrics => _sp.GetService() ?? SettingMetrics.Shared; + /// /// Creates a manager. , , /// and are optional: omitting them @@ -478,6 +484,7 @@ private async Task NotifyWrittenAsync( catch (Exception ex) { LogWriteObserverFailed(_logger, observer.GetType().Name, map.GroupName, ex); + Metrics.WriteObserverFailed(observer.GetType(), map.GroupName, ex); } } } @@ -541,6 +548,7 @@ private async Task FireChangedHandlersAsync(T previous, T current, string gro catch (Exception ex) { LogChangeHandlerFailed(_logger, handler.GetType().Name, groupName, ex); + Metrics.ChangeHandlerFailed(handler.GetType(), groupName, ex); } } } diff --git a/src/DotNetBoost.Settings.Core/Services/SettingMetrics.cs b/src/DotNetBoost.Settings.Core/Services/SettingMetrics.cs new file mode 100644 index 0000000..3c2ffd0 --- /dev/null +++ b/src/DotNetBoost.Settings.Core/Services/SettingMetrics.cs @@ -0,0 +1,79 @@ +using System.Diagnostics.Metrics; + +namespace DotNetBoost.Settings.Core.Services; + +/// +/// The library's instruments, on the DotNetBoost.Settings meter. Registered by +/// AddSettings() as a singleton. +/// +/// +/// +/// The meter comes from when the container has one, which every +/// generic host does: a meter created that way belongs to the container, so a test host's +/// measurements stay apart from another's and the meter is disposed with it. Without a +/// factory, a plain ServiceCollection or a manager constructed by hand, the instruments +/// fall back to one meter shared by the process. +/// +/// +/// Each failure counter sits beside the log line for the same failure: a swallowed exception +/// is still logged, and the counter is what makes a rate of them visible without anyone +/// reading logs. Every tag is bounded by the application's own types — the implementation, +/// the group, the exception type — never by a value. +/// +/// +internal sealed class SettingMetrics +{ + public const string MeterName = "DotNetBoost.Settings"; + + public const string WriteObserverFailuresName = "dotnetboost.settings.write_observer.failures"; + public const string ChangeHandlerFailuresName = "dotnetboost.settings.change_handler.failures"; + + public const string GroupTag = "dotnetboost.settings.group"; + public const string ObserverTag = "dotnetboost.settings.observer"; + public const string HandlerTag = "dotnetboost.settings.handler"; + public const string ErrorTag = "error.type"; + + /// The instruments on the process-wide meter, for a manager with no container meter. + public static SettingMetrics Shared { get; } = new(new Meter(MeterName)); + + private readonly Counter _writeObserverFailures; + private readonly Counter _changeHandlerFailures; + + /// Creates the instruments on a meter from , or on the shared one. + public SettingMetrics(IMeterFactory? meterFactory = null) + : this(meterFactory?.Create(MeterName)) + { + } + + private SettingMetrics(Meter? meter) + { + // Instruments are created once per meter, so the fallback reuses Shared's rather than + // adding a second set of the same names to the process-wide meter. + if (meter is null) + { + _writeObserverFailures = Shared._writeObserverFailures; + _changeHandlerFailures = Shared._changeHandlerFailures; + return; + } + + _writeObserverFailures = meter.CreateCounter( + WriteObserverFailuresName, unit: "{failure}", + description: "Write observers that threw after a settings write committed. The write itself succeeded."); + + _changeHandlerFailures = meter.CreateCounter( + ChangeHandlerFailuresName, unit: "{failure}", + description: "Change handlers that threw after a settings write committed. The write itself succeeded."); + } + + public void WriteObserverFailed(Type observer, string group, Exception ex) + => _writeObserverFailures.Add(1, + new(ObserverTag, observer.Name), + new(GroupTag, group), + new(ErrorTag, ex.GetType().FullName)); + + public void ChangeHandlerFailed(Type handler, string group, Exception ex) + => _changeHandlerFailures.Add(1, + new(HandlerTag, handler.Name), + new(GroupTag, group), + new(ErrorTag, ex.GetType().FullName)); +} diff --git a/src/DotNetBoost.Settings.Core/SettingServiceCollectionExtensions.cs b/src/DotNetBoost.Settings.Core/SettingServiceCollectionExtensions.cs index cab21cf..50831cd 100644 --- a/src/DotNetBoost.Settings.Core/SettingServiceCollectionExtensions.cs +++ b/src/DotNetBoost.Settings.Core/SettingServiceCollectionExtensions.cs @@ -28,6 +28,10 @@ public static SettingBuilder AddSettings(this IServiceCollection services) // needs no configuration and behaves exactly as it did before signalling existed. services.TryAddSingleton(); + // Takes the container's IMeterFactory when there is one, which every generic host + // registers; without one the instruments use a meter shared by the process. + services.TryAddSingleton(); + return new SettingBuilder(services); } } diff --git a/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs b/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs index 658c43d..64e6595 100644 --- a/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs +++ b/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using System.Diagnostics.Metrics; using System.Security.Cryptography; namespace DotNetBoost.Settings.UnitTests; @@ -246,6 +247,86 @@ public async Task Observer_ThatThrows_DoesNotFailTheWrite_AndLaterObserversStill Assert.Equal("a", (await store.GetAsync(nameof(ObservedSettings), nameof(ObservedSettings.Host)))!.Value); } + /// + /// A swallowed exception is still logged; the counter is what makes a rate of them visible + /// without anyone reading logs. + /// + [Fact] + public async Task Observer_ThatThrows_IsCounted_WithItsTypeGroupAndError() + { + using var metrics = new MetricsCapture(); + var services = new ServiceCollection(); + services.AddScoped(); + var mgr = Build(services, metrics); + + await mgr.For().SetAsync(new ObservedSettings { Host = "a" }); + + var m = Assert.Single(metrics.Measurements); + Assert.Equal(SettingMetrics.WriteObserverFailuresName, m.Instrument); + Assert.Equal(1, m.Value); + Assert.Equal(nameof(ThrowingObserver), m.Tags[SettingMetrics.ObserverTag]); + Assert.Equal(nameof(ObservedSettings), m.Tags[SettingMetrics.GroupTag]); + Assert.Equal(typeof(InvalidOperationException).FullName, m.Tags[SettingMetrics.ErrorTag]); + } + + [Fact] + public async Task ChangeHandler_ThatThrows_IsCounted_WithItsTypeGroupAndError() + { + using var metrics = new MetricsCapture(); + var services = new ServiceCollection(); + services.AddScoped, ThrowingChangeHandler>(); + var mgr = Build(services, metrics); + + var ex = await Record.ExceptionAsync( + () => mgr.For().SetAsync(new ObservedSettings { Host = "a" })); + + Assert.Null(ex); + var m = Assert.Single(metrics.Measurements); + Assert.Equal(SettingMetrics.ChangeHandlerFailuresName, m.Instrument); + Assert.Equal(nameof(ThrowingChangeHandler), m.Tags[SettingMetrics.HandlerTag]); + Assert.Equal(nameof(ObservedSettings), m.Tags[SettingMetrics.GroupTag]); + Assert.Equal(typeof(InvalidOperationException).FullName, m.Tags[SettingMetrics.ErrorTag]); + } + + [Fact] + public async Task Write_WhoseObserversAndHandlersSucceed_CountsNothing() + { + using var metrics = new MetricsCapture(); + var services = new ServiceCollection(); + services.AddScoped(); + var mgr = Build(services, metrics); + + await mgr.For().SetAsync(new ObservedSettings { Host = "a" }); + await mgr.For().ClearAsync(); + + Assert.Empty(metrics.Measurements); + } + + /// + /// AddSettings() takes the container's meter factory when there is one, so a host's + /// measurements stay on its own meter, and still resolves when there is none. + /// + [Fact] + public void AddSettings_CreatesTheMeter_FromTheContainersFactory_WhenThereIsOne() + { + using var metrics = new MetricsCapture(); + + var withFactory = new ServiceCollection() + .AddSingleton(metrics.Factory); + withFactory.AddSettings(); + using var sp = withFactory.BuildServiceProvider(); + + sp.GetRequiredService() + .WriteObserverFailed(typeof(ThrowingObserver), "g", new InvalidOperationException()); + Assert.Single(metrics.Measurements); + + var withoutFactory = new ServiceCollection(); + withoutFactory.AddSettings(); + using var bare = withoutFactory.BuildServiceProvider(); + + Assert.NotNull(bare.GetRequiredService()); + } + [Fact] public async Task Actor_IsNull_WhenNoAccessorIsRegistered() { @@ -308,6 +389,13 @@ private static (ISettingManager, FakeStore, RecordingObserver) Build( return (mgr, store, observer); } + private static SettingManager Build(ServiceCollection services, MetricsCapture metrics) + { + services.AddSingleton(new SettingMetrics(metrics.Factory)); + return new SettingManager(new FakeStore(), new NoCache(), services.BuildServiceProvider(), + NullLogger.Instance); + } + [SettingGroup("observed")] public class ObservedSettings { @@ -338,6 +426,64 @@ public Task OnWrittenAsync(SettingWrite write, CancellationToken ct = default) } } + private sealed class ThrowingChangeHandler : ISettingChangedHandler + { + public Task OnChangedAsync(ObservedSettings previous, ObservedSettings current, CancellationToken ct = default) + => throw new InvalidOperationException("handler is down"); + } + + /// + /// Collects measurements from meters made by its own factory only. Listening by meter name + /// would also catch measurements from tests running in parallel. + /// + private sealed class MetricsCapture : IDisposable + { + private readonly MeterListener _listener = new(); + + public TestMeterFactory Factory { get; } = new(); + public List<(string Instrument, long Value, Dictionary Tags)> Measurements { get; } = []; + + public MetricsCapture() + { + _listener.InstrumentPublished = (instrument, listener) => + { + if (ReferenceEquals(instrument.Meter.Scope, Factory)) + listener.EnableMeasurementEvents(instrument); + }; + _listener.SetMeasurementEventCallback((instrument, value, tags, _) => + { + var bag = new Dictionary(StringComparer.Ordinal); + foreach (var tag in tags) bag[tag.Key] = tag.Value; + lock (Measurements) Measurements.Add((instrument.Name, value, bag)); + }); + _listener.Start(); + } + + public void Dispose() + { + _listener.Dispose(); + Factory.Dispose(); + } + } + + private sealed class TestMeterFactory : IMeterFactory + { + private readonly List _meters = []; + + public Meter Create(MeterOptions options) + { + options.Scope = this; + var meter = new Meter(options); + _meters.Add(meter); + return meter; + } + + public void Dispose() + { + foreach (var meter in _meters) meter.Dispose(); + } + } + private sealed class StubActorAccessor(string? actor) : ISettingActorAccessor { public string? GetActor() => actor; From ec59bd529605c3fdbce854fabc18afdb9d73190a Mon Sep 17 00:00:00 2001 From: Sadeq Abu-Hattem Date: Mon, 2 Nov 2026 21:21:08 +0200 Subject: [PATCH 3/3] Let a participant commit its work with the settings write Observers run after the store has committed, and their exceptions are logged and swallowed. That is right for notifications, and wrong for an audit entry: a failure to record leaves the change standing with no entry, and the observer has to open a scope and save a second time, outside the write's transaction. That is the flaw the built-in audit trail was removed for in preview.2, and the sample's audit observer had it again. ISettingWriteParticipant runs inside every write, before the store is called. It enlists entities through EfCoreSettingWriteScope.Add, which wraps the very context instance the EF Core store saves, so the store's own SaveChangesAsync commits them with the rows. An entity on any other context would never be saved; one the settings context does not map throws on Add and aborts the write. A participant's exception aborts the write: the store is not called, the cache keeps its entry, observers and change handlers do not run, and what earlier participants enlisted is discarded. If the store fails after them (a concurrency conflict), the enlisted work is discarded too. The manager builds one SettingWrite per write, before the store call, and hands the same instance to participants and observers, so an audit entry and a log line share a correlation id. Only a store that implements IEnlistingSettingStore accepts participants, and a provider declares it through a new EnsureProviderNotConfigured overload. EF Core does. Dapper opens its own transaction per write and MongoDB uses none, so Build() refuses a participant for either, and a write refuses one at run time for a store that cannot enlist. Running them anyway would look like an audit trail and not be one, as the old Dapper SettingAudits table did. A clear now reports what it deleted: one SettingChange per row with the stored value as OldValue, redacted for [Sensitive] properties and any encrypted row. The group is read first only when a participant or observer is registered. In the EF store, a clear with something enlisted runs ExecuteDelete and SaveChanges in one transaction: the caller's when there is one, otherwise its own inside the execution strategy, since a transaction opened outside a retrying strategy throws. Also fixed in passing: a failed EF write left its modified and added Setting entities tracked on the scoped context, so the application's next unrelated SaveChangesAsync wrote the losing value over the winner's. They are now detached when the save fails. Verified: the solution builds with 0 warnings; unit (210 x2), provider (66 x2), API (51 x2) and integration (158, Testcontainers Postgres, SQL Server and MongoDB) tests pass. The EF participant suite runs on SQLite, Postgres, and SQL Server under EnableRetryOnFailure. Removing the store's detach, the manager's discard, or the save in the clear's transaction each fails the tests that cover it. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 7 + README.md | 5 +- docs/README.md | 2 +- docs/architecture.md | 2 + docs/configuration-reference.md | 1 + docs/write-notifications.md | 118 ++++- .../Interfaces/ISettingWriteObserver.cs | 32 +- .../Interfaces/ISettingWriteParticipant.cs | 66 +++ .../Services/SettingManager.cs | 166 ++++++- .../Services/SettingWriteObservers.cs | 3 +- .../SettingInfrastructure.cs | 72 ++- .../SettingServiceCollectionExtensions.cs | 19 + .../EfCore.cs | 154 +++++-- .../EfCoreServiceCollectionExtensions.cs | 15 +- .../EfCoreSettingWriteScope.cs | 61 +++ .../EfCoreWriteParticipantIntegrationTests.cs | 35 ++ .../Stores/EfCoreWriteParticipantTests.cs | 396 ++++++++++++++++ .../SettingWriteObserverTests.cs | 39 +- .../SettingWriteParticipantTests.cs | 428 ++++++++++++++++++ 19 files changed, 1536 insertions(+), 85 deletions(-) create mode 100644 src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteParticipant.cs create mode 100644 src/DotNetBoost.Settings.EntityFrameworkCore/EfCoreSettingWriteScope.cs create mode 100644 tests/DotNetBoost.Settings.IntegrationTests/Stores/EfCoreWriteParticipantIntegrationTests.cs create mode 100644 tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreWriteParticipantTests.cs create mode 100644 tests/DotNetBoost.Settings.UnitTests/SettingWriteParticipantTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 0686032..57fbe65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,18 @@ This project follows [Semantic Versioning](https://semver.org/). ### Added - `SettingWrite.GroupType`: the settings class a write was made to. An observer that needs the group's attributes no longer has to scan assemblies for a class whose resolved name matches `GroupName`. It is an init property, so code that constructs a `SettingWrite` keeps compiling. - The library's first metrics, on a `DotNetBoost.Settings` meter: `dotnetboost.settings.write_observer.failures` and `dotnetboost.settings.change_handler.failures`, each tagged with the implementation's class name, the group and `error.type`. Observer and handler exceptions are still logged and swallowed; until now the log line was the only trace of them. The meter is created through the container's `IMeterFactory` when there is one. +- `ISettingWriteParticipant` and `.UseWriteParticipant()`: a hook that runs inside every write, before the store saves, for work that must commit with the write or not at all — an audit entry. A participant enlists entities through `EfCoreSettingWriteScope.Add`, and the Entity Framework Core store's own save commits them with the rows. An exception aborts the write: the store is not called, the cache keeps its entry, and what was enlisted is discarded. Participants and observers receive the same `SettingWrite` instance, so they share a correlation id. See [Write notifications](docs/write-notifications.md#isettingwriteparticipant). +- `IEnlistingSettingStore`, the store capability participants require, and a `SettingBuilderGuard.EnsureProviderNotConfigured` overload through which a provider declares it. Only Entity Framework Core does. `Build()` refuses a participant for Dapper or MongoDB, and a write refuses one at run time for a store that cannot enlist, rather than run it with no atomicity. ### Changed +- **A clear now reports what it deleted.** A `Cleared` `SettingWrite` used to carry an empty `Changes`; it now lists one `SettingChange` per deleted row, with the stored value as `OldValue` and a null `NewValue`, redacted for `[Sensitive]` properties and for any encrypted row. The manager reads the group before deleting to do it, but only when a participant or observer is registered. An observer that relied on `Changes` being empty for a clear should check `Kind` instead. +- `EfCoreSettingStore.DeleteGroupAsync`, when a participant has enlisted something, runs the delete and a `SaveChangesAsync` in one transaction: the caller's if there is one, otherwise its own inside the context's execution strategy, so it works under `EnableRetryOnFailure`. With nothing enlisted it is the single `ExecuteDeleteAsync` it was. - Aspire moved from 13.5.2 to 13.6.4, and Microsoft.Extensions.* and EF Core packages from 10.0.11 to 10.0.12 — the floor Aspire 13.6 requires. - Microsoft.Data.SqlClient (integration tests and sample only) moved from 6.1.6 to 6.1.8, which takes System.Security.Cryptography.Pkcs 9.0.21 and clears CVE-2026-70162 and CVE-2026-84008. +### Fixed +- A failed Entity Framework Core write no longer leaves its `Setting` entities tracked. After a `SettingConcurrencyException` the rows the store had modified or added stayed in the change tracker of the scoped, usually shared, context, so the application's next unrelated `SaveChangesAsync` wrote the losing value over the winner's. They are now detached when the save fails. + ## [1.0.0-preview.2] — 2026-10-10 The second prerelease. **One breaking change from `1.0.0-preview.1`:** the built-in audit trail diff --git a/README.md b/README.md index 851860c..e126d79 100644 --- a/README.md +++ b/README.md @@ -183,7 +183,8 @@ builder.Services.AddSettings() .UseAesEncryption(key) // encrypt [Sensitive] properties .UseFluentValidation(typeof(Program).Assembly) // reject invalid values .OnChanged() // react when a value changes - .UseWriteObserver() // record or forward every write + .UseWriteParticipant() // commit an audit entry with every write + .UseWriteObserver() // forward every committed write .Build(); ``` @@ -198,7 +199,7 @@ builder.Services.AddSettings() | Run on several servers and keep their caches in step | [Caching](https://github.com/dotnetboost/settings/blob/main/docs/caching.md#running-on-more-than-one-instance) | | Run code when a setting changes | [Change notifications](https://github.com/dotnetboost/settings/blob/main/docs/change-notifications.md) | | Stop two people overwriting each other's edits | [Reading & writing](https://github.com/dotnetboost/settings/blob/main/docs/reading-and-writing.md#concurrent-writes) | -| Record or forward every write (and who made it) | [Write notifications](https://github.com/dotnetboost/settings/blob/main/docs/write-notifications.md) | +| Audit or forward every write (and who made it) | [Write notifications](https://github.com/dotnetboost/settings/blob/main/docs/write-notifications.md) | | Generate a form from the class (list groups, read the schema) | [Discovery endpoints](https://github.com/dotnetboost/settings/blob/main/docs/discovery-endpoints.md) | | Give admins a web UI to edit settings | [Dashboard](https://github.com/dotnetboost/settings/blob/main/docs/dashboard.md) | | See every builder option | [Configuration reference](https://github.com/dotnetboost/settings/blob/main/docs/configuration-reference.md) | diff --git a/docs/README.md b/docs/README.md index b742e0b..df14ae3 100644 --- a/docs/README.md +++ b/docs/README.md @@ -11,7 +11,7 @@ New here? Start with the [5-minute quick start](../README.md#get-started-in-5-mi - [Encrypting sensitive values](encryption.md): `[Sensitive]`, AES-256-GCM, key rotation - [Validation](validation.md): Data Annotations and FluentValidation - [Change notifications](change-notifications.md): run code when a setting changes -- [Write notifications](write-notifications.md): observe every write — what changed, by whom +- [Write notifications](write-notifications.md): audit every write with it, or observe it after — what changed, by whom - [Caching](caching.md): cache duration, Redis for multi-server deployments - [REST API endpoints](rest-api.md): generated endpoints and **how to secure them** - [Discovery endpoints](discovery-endpoints.md): list the groups, describe one's properties and constraints diff --git a/docs/architecture.md b/docs/architecture.md index 5669108..5369229 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -14,6 +14,8 @@ or Redis) Dapper / or custom) or FluentValidation) MongoDB) │ + ISettingWriteParticipant (inside the write: enlists work that + │ commits with it, or aborts it) ┌───────────┴────────────┐ ISettingChangedHandler ISettingWriteObserver (typed, one group, (untyped, every group: the diff --git a/docs/configuration-reference.md b/docs/configuration-reference.md index 1570e68..a565c30 100644 --- a/docs/configuration-reference.md +++ b/docs/configuration-reference.md @@ -15,6 +15,7 @@ | `.UseAesEncryption(key, retiredKeys...)` | Core | Enable built-in AES-256-GCM encryption; retired keys decrypt only | | `.IgnoreDecryptionFailures()` | Core | Fall back to defaults instead of throwing when a value will not decrypt | | `.UseCustomEncryption()` | Core | Plug in a custom encryptor | +| `.UseWriteParticipant()` | Core | Enlist work that commits with every write, or aborts it; Entity Framework Core only | | `.UseWriteObserver()` | Core | Observe every completed write; several may be registered | | `.UseLoggingWriteObserver()` | Core | Log one line per write — property names, never values | | `.UseActorAccessor()` | Core | Supply who is making the change | diff --git a/docs/write-notifications.md b/docs/write-notifications.md index 917ae36..6f2d446 100644 --- a/docs/write-notifications.md +++ b/docs/write-notifications.md @@ -6,11 +6,109 @@ the write's own transaction, could fail a write that had already committed, expo history endpoint, and existed only for Entity Framework Core — the Dapper bootstrap created a `SettingAudits` table that nothing ever wrote to. -What the library does instead is report every write, and let you decide what to do with it. -A general `DotNetBoost.Auditing` package will implement recording over this hook, the same way -`DotNetBoost.Settings.FluentValidation` implements `ISettingValidator` — port here, +What the library does instead is report every write, through two hooks, and let you decide +what to do with it. A general `DotNetBoost.Auditing` package will implement recording over them, +the same way `DotNetBoost.Settings.FluentValidation` implements `ISettingValidator` — port here, implementation in a package of its own. +| | `ISettingWriteParticipant` | `ISettingWriteObserver` | +|---|---|---| +| Runs | Inside the write, before the store saves | After the write has committed | +| An exception | Aborts the write | Is logged, counted and swallowed | +| Commits with the write | Yes, what it enlists | No — it runs after | +| Providers | Entity Framework Core | All | +| Good for | An audit trail: anything that must not be lost | Cache signals, webhooks, indexing, logging | + +Both receive the same `SettingWrite` for a given write, so an audit entry and a log line share a +correlation id. + +## `ISettingWriteParticipant` + +Called once inside every write to any settings group, before the store saves it. What it +**enlists** is committed by the store's own save, in the same transaction as the rows, so the +two commit together or not at all. + +```csharp +public sealed class SettingAuditParticipant(EfCoreSettingWriteScope scope) : ISettingWriteParticipant +{ + public Task OnWritingAsync(SettingWrite write, CancellationToken ct = default) + { + scope.Add(new SettingAuditEntry + { + Group = write.GroupName, + Kind = write.Kind.ToString(), + Actor = write.Actor, + CorrelationId = write.CorrelationId, + Changes = JsonSerializer.Serialize(write.Changes), + OccurredAt = write.OccurredAt + }); + return Task.CompletedTask; + } +} +``` + +```csharp +builder.Services.AddSettings() + .UseEntityFrameworkCore() + .UseWriteParticipant() + .Build(); +``` + +`SettingAuditEntry` here is your own entity, mapped on `AppDbContext`, the same context the +settings are on. + +### Enlist, never save + +- **Add through `EfCoreSettingWriteScope`**, not through a context of your own. The scope wraps + the exact context instance the settings store saves, so what you add goes out in its save. An + entity added to any other context would never be saved, and nothing would tell you. An entity + whose type the settings context does not map throws on `Add`, which aborts the write. +- **Do not call `SaveChangesAsync`**, and do nothing with an effect outside the context — no + messages, no HTTP calls. The write can still fail after every participant has run, on a + concurrency conflict for one, and only what was enlisted is undone. Work that should happen + only once the write has committed belongs in an observer. +- **Enlist standalone entities.** If the write does not commit, each entity you added is + detached again, so the application's next `SaveChangesAsync` on the same context cannot save + it. Related entities reached only through a navigation are not detached with it. + +### When a participant fails + +**A participant's exception aborts the write**, and reaches the caller unchanged. The store is +not called, the cached copy of the group stays, no observer or change handler runs, and what +earlier participants enlisted is discarded. Participants run in registration order, and the +first to throw stops the rest. + +The same happens when the store itself fails after the participants ran: a +`SettingConcurrencyException` discards what they enlisted, and the store takes back its own +rows, so nothing from the failed write is left tracked on the context. + +### What the store does with it + +A settings write calls `SaveChangesAsync` on the shared context, which saves everything tracked +there — your application's own pending changes included. That has always been true of the +Entity Framework Core store; a participant is what makes it useful. + +A clear is the one write that does not normally go through the change tracker: it is a single +`ExecuteDeleteAsync`. When something was enlisted, the store runs the delete and a +`SaveChangesAsync` in one transaction instead: + +- inside **your transaction**, when the context has one, which you then commit or roll back; +- otherwise in **its own**, opened inside the context's execution strategy, so it works under + `EnableRetryOnFailure`. + +Inside your own transaction, participants and observers have already run by the time you roll +back: an observer can be told about a write your rollback then undid. What a participant +enlisted is undone with everything else. + +### Which providers accept participants + +Only Entity Framework Core. Dapper opens a transaction of its own inside each write, and MongoDB +uses none, so neither can commit something a participant enlists together with the rows. +`Build()` refuses a participant for either, rather than run it with no atomicity — the mistake +the old Dapper audit table made. A custom store opts in by implementing `IEnlistingSettingStore` +and declaring it through `SettingBuilderGuard.EnsureProviderNotConfigured(builder, name, +supportsWriteParticipants: true)`. + ## `ISettingWriteObserver` Called once after every completed write to any settings group. @@ -39,10 +137,10 @@ Several observers can be registered; all of them run, in registration order. | `GroupName` | The persistence key the rows are stored under | | `Route` | The group's API route segment | | `Kind` | `Updated`, or `Cleared` for `ClearAsync` | -| `Changes` | One `SettingChange` per property that actually changed; empty when `Cleared` | +| `Changes` | One `SettingChange` per property that actually changed, or per row a clear deleted | | `Actor` | Who made the change, or `null` for "not captured" | | `CorrelationId` | One value per write, so the several changes of one save are identifiable as one edit | -| `OccurredAt` | UTC timestamp | +| `OccurredAt` | UTC timestamp, taken just before the store is called | | `GroupType` | The settings class, for reading its attributes | Identify a group by `GroupName`, not by `GroupType`'s name. Moving the class to another @@ -52,6 +150,10 @@ would split one group's history in two. Each `SettingChange` is `(PropertyName, OldValue, NewValue, IsRedacted)`. `OldValue` is `null` when the property had never been stored. +A clear lists what it deleted: one entry per stored row, with the value it held as `OldValue` and +`NewValue` `null`. It reads the group first to do that, but only when a participant or observer +is registered. A row written between that read and the delete is deleted without being listed. + A write that changes nothing raises no notification at all. Change detection compares `[Sensitive]` values as plaintext rather than as stored ciphertext: AES-GCM draws a fresh nonce on every call, so an unchanged secret re-encrypts to different bytes each save and a ciphertext @@ -91,7 +193,8 @@ deliberately not the one for [`ISettingProjector`](rest-api.md#reshaping-what which *can* fail a request — a failed notification must not break a committed write, while a failed projection would otherwise return a wrong response. -So an observer is the wrong place for anything that must not be lost. Its failures are counted +So an observer is the wrong place for anything that must not be lost; that is what +[a participant](#isettingwriteparticipant) is for. An observer's failures are counted rather than surfaced: each one also adds 1 to a counter on the `DotNetBoost.Settings` meter, so a rate of them shows up on a dashboard instead of only in the logs. @@ -125,7 +228,8 @@ is exactly the kind of thing that should not reach a log aggregator because a de on. It is off unless you ask for it. This is a stopgap, not a trail: there is nothing to query, and nothing enforces retention. For a -real history, implement `ISettingWriteObserver` over your own store. +real history, implement an [`ISettingWriteParticipant`](#isettingwriteparticipant) that records +into your own table. ## Upgrading from `1.0.0-preview.1` diff --git a/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteObserver.cs b/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteObserver.cs index 3971388..dfc0a77 100644 --- a/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteObserver.cs +++ b/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteObserver.cs @@ -8,8 +8,9 @@ public enum SettingWriteKind /// /// Every row in the group was deleted by ClearAsync. - /// is empty — the operation is group-wide, and nothing - /// is read back to enumerate what was there. + /// lists one entry per deleted row, with the value it + /// held as and a null + /// . Empty when the group had nothing stored. /// Cleared } @@ -21,7 +22,8 @@ public enum SettingWriteKind /// Always null when is set. /// /// -/// The value as written. Always null when is set. +/// The value as written. Always null when is set, and for +/// a write, which writes nothing. /// /// /// Whether the property is [Sensitive]. Its values are withheld rather than reported: @@ -35,7 +37,11 @@ public sealed record SettingChange( string? NewValue, bool IsRedacted); -/// A completed write to one settings group. +/// +/// One write to one settings group. An is handed it +/// before the store commits, and every the same instance +/// after. +/// /// /// The persistence key the group's rows are stored under — the Name on /// [SettingGroup], or the class name when none is set. @@ -46,9 +52,9 @@ public sealed record SettingChange( /// /// Whether properties were written or the whole group was cleared. /// -/// One entry per property that actually changed; empty when is -/// . A write that changed nothing raises no -/// at all. +/// One entry per property that actually changed, or, when is +/// , one per row deleted. A set that changed nothing +/// raises no at all. /// /// /// Who made the change, from the registered . null @@ -59,7 +65,9 @@ public sealed record SettingChange( /// identifiable as one edit. The ambient Activity id when there is one, otherwise a /// fresh value. /// -/// UTC timestamp of the write. +/// +/// UTC timestamp of the write, taken just before the store is called. +/// public sealed record SettingWrite( string GroupName, string Route, @@ -86,8 +94,8 @@ public sealed record SettingWrite( } /// -/// Observes every completed write to every settings group. This is the hook an auditing -/// package, a webhook publisher, a search indexer or a distributed cache signal implements. +/// Observes every completed write to every settings group. This is the hook a webhook +/// publisher, a search indexer or a distributed cache signal implements. /// /// /// @@ -104,6 +112,10 @@ public sealed record SettingWrite( /// order. /// /// +/// For work that must commit together with the write, or not at all — an audit entry — +/// implement instead. +/// +/// /// This is the untyped half of the notification pair. Use /// instead when you want the typed before/after models /// of one group — "if MailSettings.Host changed, reconnect" — rather than a diff --git a/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteParticipant.cs b/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteParticipant.cs new file mode 100644 index 0000000..1b90fe3 --- /dev/null +++ b/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteParticipant.cs @@ -0,0 +1,66 @@ +namespace DotNetBoost.Settings.Core.Interfaces; + +/// +/// Takes part in every write to every settings group, before the store saves it. This is the +/// hook for work that must commit together with the change, or not at all: an audit entry is +/// the reason it exists. +/// +/// +/// +/// Failure policy: an exception aborts the write. The store is not called, the cache +/// keeps its entry, and no or +/// runs. The exception reaches the caller unchanged. +/// This is the opposite of an observer's policy, and the reason both hooks exist: an observer +/// runs after the commit and cannot fail it, a participant runs before it and can. +/// +/// +/// Enlist, never save. A participant adds what it wants committed to the store's own +/// unit of work, and the store saves it with the rows. For Entity Framework Core that is +/// EfCoreSettingWriteScope.Add. Calling SaveChangesAsync yourself, or doing +/// anything with an effect outside that unit of work — sending a message, calling a service — +/// defeats the point: the write can still fail after the participant has run, on a +/// concurrency conflict for example, and only what was enlisted is undone. +/// +/// +/// Only a store that implements accepts participants. +/// Build() refuses a participant for a provider that has not declared support, and a +/// write refuses one at run time if the store cannot enlist. Of the shipped providers, Entity +/// Framework Core does; Dapper and MongoDB do not. +/// +/// +/// Several may be registered; all run, in registration order, and the first to throw stops +/// the rest. A write that changes nothing calls none of them. +/// +/// +public interface ISettingWriteParticipant +{ + /// + /// Called once per write, before the store saves it. Throwing aborts the write. + /// + /// + /// The write about to be committed. The same instance is handed to every + /// once it has been, so the two share a + /// . + /// + /// Cancellation token. + Task OnWritingAsync(SettingWrite write, CancellationToken ct = default); +} + +/// +/// A store that commits what s enlist together with its +/// own write. Implement it only when that is true: the manager refuses participants for a store +/// that does not implement it, rather than run them with no atomicity. +/// +/// +/// How a participant enlists is the store's own API, since only the store knows its unit of +/// work. What the manager needs from every such store is the one call here. +/// +public interface IEnlistingSettingStore : ISettingStore +{ + /// + /// Drops everything participants enlisted for the current write, without saving it. Called + /// when the write will not be committed: a participant threw, or the store's write did. + /// Must be safe to call when nothing was enlisted. + /// + void DiscardEnlisted(); +} diff --git a/src/DotNetBoost.Settings.Core/Services/SettingManager.cs b/src/DotNetBoost.Settings.Core/Services/SettingManager.cs index bc53505..9613a4f 100644 --- a/src/DotNetBoost.Settings.Core/Services/SettingManager.cs +++ b/src/DotNetBoost.Settings.Core/Services/SettingManager.cs @@ -258,14 +258,17 @@ internal async Task GetAsync( return; } - await _store.UpsertManyAsync(rows, ct).ConfigureAwait(false); + // One notification for the whole write, not one per row: the N properties of a single + // save are one edit, and the shared correlation id is what says so. + var listeners = ResolveListeners(); + var write = listeners.Any ? NewWrite(map, SettingWriteKind.Updated, BuildChanges(map, rows, prevMap)) : null; + + await RunParticipantsAsync(listeners, write, ct).ConfigureAwait(false); + await CommitAsync(() => _store.UpsertManyAsync(rows, ct)).ConfigureAwait(false); EvictLocally(); - // One notification for the whole write, not one per row: the N properties of a single - // save are one edit, and the shared correlation id is what says so. - await NotifyWrittenAsync( - map, SettingWriteKind.Updated, BuildChanges(map, rows, prevMap), ct).ConfigureAwait(false); + await NotifyWrittenAsync(listeners, write, map, ct).ConfigureAwait(false); await FireChangedHandlersAsync(previous, model, map.GroupName, ct).ConfigureAwait(false); @@ -331,12 +334,15 @@ internal async Task SetAsync( return; } - await _store.UpsertManyAsync([row], ct).ConfigureAwait(false); + var listeners = ResolveListeners(); + var write = listeners.Any ? NewWrite(map, SettingWriteKind.Updated, BuildChanges(map, [row], prevMap)) : null; + + await RunParticipantsAsync(listeners, write, ct).ConfigureAwait(false); + await CommitAsync(() => _store.UpsertManyAsync([row], ct)).ConfigureAwait(false); EvictLocally(); - await NotifyWrittenAsync( - map, SettingWriteKind.Updated, BuildChanges(map, [row], prevMap), ct).ConfigureAwait(false); + await NotifyWrittenAsync(listeners, write, map, ct).ConfigureAwait(false); await FireChangedHandlersAsync(previous, candidate, map.GroupName, ct).ConfigureAwait(false); @@ -401,13 +407,25 @@ await NotifyWrittenAsync( internal async Task ClearAsync(CancellationToken ct) where T : new() { var map = GetTypeMap(typeof(T)); - await _store.DeleteGroupAsync(map.GroupName, ct).ConfigureAwait(false); + + // The most destructive operation in the library, and the one most worth reporting — + // including what it destroyed, or an audit of a clear could record only that it + // happened. The rows are read only when someone is listening, so a clear with no + // participant or observer costs what it always did. + var listeners = ResolveListeners(); + SettingWrite? write = null; + if (listeners.Any) + { + var deleted = await _store.GetGroupAsync(map.GroupName, ct).ConfigureAwait(false); + write = NewWrite(map, SettingWriteKind.Cleared, BuildClearedChanges(map, deleted)); + } + + await RunParticipantsAsync(listeners, write, ct).ConfigureAwait(false); + await CommitAsync(() => _store.DeleteGroupAsync(map.GroupName, ct)).ConfigureAwait(false); + EvictLocally(); - // The most destructive operation in the library, and the one most worth reporting. - // Changes is empty: the deletion is group-wide, and nothing is read back to enumerate - // what was there — SettingWriteKind.Cleared is what carries the meaning. - await NotifyWrittenAsync(map, SettingWriteKind.Cleared, [], ct).ConfigureAwait(false); + await NotifyWrittenAsync(listeners, write, map, ct).ConfigureAwait(false); LogGroupCleared(_logger, map.GroupName); } @@ -444,23 +462,42 @@ private static List BuildChanges( } /// - /// Reports a completed write to every registered . + /// The diff for a clear: one entry per row about to be deleted, with the stored value as + /// and no new one. /// - /// Each observer is wrapped on its own: the write has already committed and the cache - /// entry is already gone, so an observer's exception must neither fail the caller nor - /// skip the observers after it. Same policy as - /// , and the two are documented as a pair. + /// A row for a property the class no longer declares is listed too, since it is deleted + /// too, and is redacted if it was stored encrypted: with no property there is no + /// [Sensitive] to consult, and ciphertext is not a value worth reporting. /// /// - private async Task NotifyWrittenAsync( - TypeMap map, SettingWriteKind kind, IReadOnlyList changes, CancellationToken ct) + private static List BuildClearedChanges(TypeMap map, IReadOnlyList deleted) { - var observers = _sp.GetServices() as IList - ?? [.. _sp.GetServices()]; + var sensitive = map.Properties + .Where(p => p.IsSensitive) + .Select(p => p.Name) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + + return deleted + .OrderBy(r => r.Key, StringComparer.Ordinal) + .Select(r => sensitive.Contains(r.Key) || r.IsEncrypted + ? new SettingChange(r.Key, null, null, IsRedacted: true) + : new SettingChange(r.Key, r.Value, null, IsRedacted: false)) + .ToList(); + } - if (observers.Count == 0) return; + /// + /// The participants and observers for one write, resolved once so a transient + /// registration is not constructed twice and both phases see the same instances. + /// + private WriteListeners ResolveListeners() + => new([.. _sp.GetServices()], [.. _sp.GetServices()]); - var write = new SettingWrite( + /// + /// The one for a write, built before the store is called so the + /// participants and the observers are handed the same instance. + /// + private SettingWrite NewWrite(TypeMap map, SettingWriteKind kind, IReadOnlyList changes) + => new( GroupName: map.GroupName, Route: map.Route, Kind: kind, @@ -475,7 +512,79 @@ private async Task NotifyWrittenAsync( GroupType = map.Type }; - foreach (var observer in observers) + /// + /// Runs every before the store is called. + /// + /// Not wrapped, unlike observers: a participant's exception is the write's exception, so + /// the store is never called and the cache keeps its entry. What earlier participants + /// enlisted is discarded first, or it would stay in the store's unit of work and be saved by + /// whatever saves it next. + /// + /// + /// + /// A participant is registered and the store cannot commit what it enlists. Checked here as + /// well as in Build(), which does not see registrations made outside the builder. + /// + private async Task RunParticipantsAsync(WriteListeners listeners, SettingWrite? write, CancellationToken ct) + { + if (write is null || listeners.Participants.Count == 0) return; + + if (_store is not IEnlistingSettingStore) + { + throw new InvalidOperationException( + $"A write participant ({listeners.Participants[0].GetType().Name}) is registered, but the " + + $"settings store ({_store.GetType().Name}) cannot commit what it enlists together with the " + + "write. Nothing was written."); + } + + try + { + foreach (var participant in listeners.Participants) + await participant.OnWritingAsync(write, ct).ConfigureAwait(false); + } + catch + { + DiscardEnlisted(); + throw; + } + } + + /// + /// Calls the store, discarding what participants enlisted if it fails — on a concurrency + /// conflict, for one. The store undoes its own rows; the enlisted work is the manager's to + /// drop, because the manager is what ran the participants. + /// + private async Task CommitAsync(Func storeWrite) + { + try + { + // Invoked inside the try, so a store that throws before returning its task is + // handled the same as one whose task faults. + await storeWrite().ConfigureAwait(false); + } + catch + { + DiscardEnlisted(); + throw; + } + } + + private void DiscardEnlisted() => (_store as IEnlistingSettingStore)?.DiscardEnlisted(); + + /// + /// Reports a committed write to every registered . + /// + /// Each observer is wrapped on its own: the write has already committed and the cache + /// entry is already gone, so an observer's exception must neither fail the caller nor + /// skip the observers after it. Same policy as + /// , and the two are documented as a pair. + /// + /// + private async Task NotifyWrittenAsync(WriteListeners listeners, SettingWrite? write, TypeMap map, CancellationToken ct) + { + if (write is null) return; + + foreach (var observer in listeners.Observers) { try { @@ -725,6 +834,13 @@ private bool TryRowsFromCache(string key, out Setting[]? rows) Message = "Cannot map the stored state of settings group '{group}'; comparing against defaults instead.")] private static partial void LogPreviousModelFailed(ILogger logger, string group, Exception ex); + private sealed record WriteListeners( + IReadOnlyList Participants, + IReadOnlyList Observers) + { + public bool Any => Participants.Count > 0 || Observers.Count > 0; + } + /// What this instance's cached copy of a group was loaded under, and when it was last re-checked. private sealed record CacheProbe(string? Version, DateTimeOffset CheckedAt); diff --git a/src/DotNetBoost.Settings.Core/Services/SettingWriteObservers.cs b/src/DotNetBoost.Settings.Core/Services/SettingWriteObservers.cs index 7c67c0e..788ed04 100644 --- a/src/DotNetBoost.Settings.Core/Services/SettingWriteObservers.cs +++ b/src/DotNetBoost.Settings.Core/Services/SettingWriteObservers.cs @@ -25,7 +25,8 @@ internal sealed class NullSettingActorAccessor : ISettingActorAccessor /// also the part that is safe to keep. /// /// This exists so the write hook is not an empty port. For a real trail — queryable, with -/// retention — implement over your own store. +/// retention, and committed with the write — implement +/// over your own table. /// /// public sealed partial class LoggingSettingWriteObserver : ISettingWriteObserver diff --git a/src/DotNetBoost.Settings.Core/SettingInfrastructure.cs b/src/DotNetBoost.Settings.Core/SettingInfrastructure.cs index b38952a..fa148de 100644 --- a/src/DotNetBoost.Settings.Core/SettingInfrastructure.cs +++ b/src/DotNetBoost.Settings.Core/SettingInfrastructure.cs @@ -15,8 +15,9 @@ public sealed class SettingBuilder /// The service collection being configured. Providers add their registrations here. public IServiceCollection Services { get; } - internal bool ProviderConfigured { get; set; } - internal string? ProviderName { get; set; } + internal bool ProviderConfigured { get; set; } + internal string? ProviderName { get; set; } + internal bool SupportsWriteParticipants { get; set; } /// /// The options instance handed to SettingManager through DI. Builder methods mutate @@ -52,8 +53,9 @@ public SettingBuilder(IServiceCollection services) /// seam open. /// /// - /// No provider is configured, a [SettingGroup] class breaks a naming rule, or a - /// settings group has two different projectors registered. + /// No provider is configured, a [SettingGroup] class breaks a naming rule, a + /// settings group has two different projectors registered, or a write participant is + /// registered for a provider that cannot commit what it enlists. /// public IServiceCollection Build() { @@ -66,6 +68,7 @@ public IServiceCollection Build() SettingBuilderValidator.Validate(AppDomain.CurrentDomain.GetAssemblies()); SettingBuilderValidator.ValidateProjectors(Services); + SettingBuilderValidator.ValidateWriteParticipants(Services, ProviderName, SupportsWriteParticipants); return Services; } } @@ -74,10 +77,26 @@ public IServiceCollection Build() public static class SettingBuilderGuard { /// - /// Records as the builder's provider. + /// Records as the builder's provider, one that does not + /// accept write participants. /// /// A provider is already configured. public static void EnsureProviderNotConfigured(SettingBuilder builder, string providerName) + => EnsureProviderNotConfigured(builder, providerName, supportsWriteParticipants: false); + + /// + /// Records as the builder's provider. + /// + /// The builder being configured. + /// Named in error messages. + /// + /// Whether the provider's store implements , so an + /// can enlist in its writes. + /// refuses a participant when this is false. + /// + /// A provider is already configured. + public static void EnsureProviderNotConfigured( + SettingBuilder builder, string providerName, bool supportsWriteParticipants) { ArgumentNullException.ThrowIfNull(builder); ArgumentException.ThrowIfNullOrWhiteSpace(providerName); @@ -89,14 +108,15 @@ public static void EnsureProviderNotConfigured(SettingBuilder builder, string pr $"Cannot also configure '{providerName}'."); } - builder.ProviderConfigured = true; - builder.ProviderName = providerName; + builder.ProviderConfigured = true; + builder.ProviderName = providerName; + builder.SupportsWriteParticipants = supportsWriteParticipants; } } /// /// Startup validation for [SettingGroup] classes — blank routes, blank explicit names, -/// duplicate group names or routes — and for projector registrations. Run by +/// duplicate group names or routes — and for projector and write-participant registrations. Run by /// . /// public static class SettingBuilderValidator @@ -171,6 +191,42 @@ public static void ValidateProjectors(IServiceCollection services) throw new InvalidOperationException(sb.ToString()); } + /// + /// Refuses a write participant for a provider that has not declared it can commit what + /// participants enlist. Running them anyway would look like an audit trail and not be one — + /// the same trap as the SettingAudits table the Dapper bootstrap used to create and + /// nothing wrote to. + /// + /// The registrations to check. + /// The configured provider, named in the message. + /// + /// What the provider declared through . + /// + /// + /// A participant is registered and the provider does not support them; the message names + /// the participants. + /// + public static void ValidateWriteParticipants( + IServiceCollection services, string? providerName, bool providerSupportsParticipants) + { + ArgumentNullException.ThrowIfNull(services); + + if (providerSupportsParticipants) return; + + var participants = services + .Where(d => d.ServiceType == typeof(ISettingWriteParticipant)) + .Select(d => d.ImplementationType?.Name ?? "(factory registration)") + .ToList(); + + if (participants.Count == 0) return; + + throw new InvalidOperationException( + $"The '{providerName}' settings provider cannot commit what a write participant enlists " + + $"together with the write, so it does not accept participants. Registered: " + + $"{string.Join(", ", participants)}. Use the Entity Framework Core provider, or an " + + "ISettingWriteObserver if the work does not have to commit with the write."); + } + /// /// The distinct implementations registered for one group. /// diff --git a/src/DotNetBoost.Settings.Core/SettingServiceCollectionExtensions.cs b/src/DotNetBoost.Settings.Core/SettingServiceCollectionExtensions.cs index 50831cd..541c731 100644 --- a/src/DotNetBoost.Settings.Core/SettingServiceCollectionExtensions.cs +++ b/src/DotNetBoost.Settings.Core/SettingServiceCollectionExtensions.cs @@ -129,6 +129,25 @@ public static SettingBuilder UseWriteObserver(this SettingBuilder bui return builder; } + /// + /// Registers a participant called once inside every write to any settings group, + /// before the store saves it, to enlist work that must commit with the write. + /// + /// Several may be registered; all run, in registration order. An exception aborts the + /// write — see . Only a provider whose store can + /// commit what participants enlist accepts them; Build() refuses one otherwise. + /// + /// + public static SettingBuilder UseWriteParticipant(this SettingBuilder builder) + where TParticipant : class, ISettingWriteParticipant + { + ArgumentNullException.ThrowIfNull(builder); + + // Add, not TryAdd, for the same reason as observers: participants compose. + builder.Services.AddScoped(); + return builder; + } + /// /// Registers , which logs one line per write /// naming the changed properties — never their values. Off unless you ask for it. diff --git a/src/DotNetBoost.Settings.EntityFrameworkCore/EfCore.cs b/src/DotNetBoost.Settings.EntityFrameworkCore/EfCore.cs index 647e2ba..05efcc6 100644 --- a/src/DotNetBoost.Settings.EntityFrameworkCore/EfCore.cs +++ b/src/DotNetBoost.Settings.EntityFrameworkCore/EfCore.cs @@ -160,13 +160,49 @@ public static ModelBuilder ApplySettingsConfiguration(this ModelBuilder mb, Data /// EF Core-backed . /// +/// /// Takes a plain and reaches its entities through /// , so a consuming context needs no interface and no /// DbSet properties — only ApplySettings in OnModelCreating. +/// +/// +/// The context is shared. A write calls SaveChangesAsync on it, which saves +/// everything tracked on it, the application's own pending changes included. That is what lets +/// an commit with the write: it enlists through +/// , and the store's save carries it. A clear saves only +/// when something was enlisted, as part of the same transaction as the delete. +/// /// -/// The context the settings entities are mapped on. -public sealed class EfCoreSettingStore(DbContext db) : ISettingStore +public sealed class EfCoreSettingStore : IEnlistingSettingStore { + private readonly DbContext db; + private readonly EfCoreSettingWriteScope? writeScope; + + /// A store with no write scope, so no participant can enlist in its writes. + /// The context the settings entities are mapped on. + public EfCoreSettingStore(DbContext db) : this(db, null) { } + + /// A store whose writes commit what participants enlist in . + /// The context the settings entities are mapped on. + /// Where participants enlist. Must wrap . + /// + /// wraps a different context, whose entities this store's save + /// would never reach. + /// + public EfCoreSettingStore(DbContext db, EfCoreSettingWriteScope? writeScope) + { + this.db = db ?? throw new ArgumentNullException(nameof(db)); + + if (writeScope is not null && !ReferenceEquals(writeScope.Context, db)) + { + throw new ArgumentException( + "The write scope wraps a different DbContext instance from the store's, so nothing " + + "enlisted in it would be saved with the settings.", nameof(writeScope)); + } + + this.writeScope = writeScope; + } + private DbSet Settings => db.Set(); /// @@ -203,41 +239,67 @@ public async Task UpsertManyAsync(IEnumerable settings, CancellationTok var existing = await Settings.Where(x => x.Group == group && keys.Contains(x.Key)).ToListAsync(ct).ConfigureAwait(false); var byKey = existing.ToDictionary(x => x.Key, StringComparer.Ordinal); - foreach (var item in list) + // Every entity this write leaves tracked, so a failed write can take them all back. + var touched = new List(existing); + + try { - if (byKey.TryGetValue(item.Key, out var e)) + foreach (var item in list) { - // OriginalValue is what EF puts in the UPDATE's WHERE clause. Setting it to the - // token the caller read — rather than the one just loaded — is what makes the - // write conditional on nothing having changed in between. - if (item.RowVersion is not null) - db.Entry(e).Property(x => x.RowVersion).OriginalValue = item.RowVersion; - - e.Value = item.Value; - e.Type = item.Type; - e.IsEncrypted = item.IsEncrypted; - e.UpdatedAt = item.UpdatedAt; - e.UpdatedBy = item.UpdatedBy; - e.RowVersion = Setting.NewRowVersion(); - } - else - { - item.RowVersion = Setting.NewRowVersion(); - await Settings.AddAsync(item, ct).ConfigureAwait(false); + if (byKey.TryGetValue(item.Key, out var e)) + { + // OriginalValue is what EF puts in the UPDATE's WHERE clause. Setting it to the + // token the caller read — rather than the one just loaded — is what makes the + // write conditional on nothing having changed in between. + if (item.RowVersion is not null) + db.Entry(e).Property(x => x.RowVersion).OriginalValue = item.RowVersion; + + e.Value = item.Value; + e.Type = item.Type; + e.IsEncrypted = item.IsEncrypted; + e.UpdatedAt = item.UpdatedAt; + e.UpdatedBy = item.UpdatedBy; + e.RowVersion = Setting.NewRowVersion(); + } + else + { + item.RowVersion = Setting.NewRowVersion(); + await Settings.AddAsync(item, ct).ConfigureAwait(false); + touched.Add(item); + } } - } - try - { await db.SaveChangesAsync(ct).ConfigureAwait(false); } catch (DbUpdateConcurrencyException ex) { + Detach(touched); var clash = ex.Entries.Select(e => e.Entity).OfType().FirstOrDefault(); throw new SettingConcurrencyException(clash?.Group ?? group, clash?.Key ?? "?", ex); } + catch + { + Detach(touched); + throw; + } + + writeScope?.Committed(); + } + + /// + /// Takes back what a failed write left tracked. The context is scoped and usually the + /// application's own, so a row left modified or added would otherwise be saved by its next + /// unrelated SaveChangesAsync — writing the very value whose write just failed. + /// + private void Detach(List touched) + { + foreach (var entity in touched) + db.Entry(entity).State = EntityState.Detached; } + /// + public void DiscardEnlisted() => writeScope?.Discard(); + /// public async Task DeleteAsync(string group, string key, CancellationToken ct = default) { @@ -250,10 +312,50 @@ public async Task DeleteAsync(string group, string key, CancellationToken ct = d } /// - public Task DeleteGroupAsync(string group, CancellationToken ct = default) + /// + /// + /// With nothing enlisted this is one ExecuteDeleteAsync, which bypasses the change + /// tracker and saves nothing else. + /// + /// + /// With something enlisted, the delete and a SaveChangesAsync for it run in one + /// transaction. The caller's transaction when there is one, which the caller then commits + /// or rolls back. Otherwise one of its own, inside the context's execution strategy: under + /// a retrying strategy (EnableRetryOnFailure) a transaction opened outside it throws. + /// The save does not accept its changes until the transaction commits, so a retry saves + /// them again rather than skipping them. + /// + /// + public async Task DeleteGroupAsync(string group, CancellationToken ct = default) { ArgumentException.ThrowIfNullOrWhiteSpace(group); - return Settings.Where(x => x.Group == group).ExecuteDeleteAsync(ct); + + if (writeScope is not { HasEnlisted: true }) + { + await Settings.Where(x => x.Group == group).ExecuteDeleteAsync(ct).ConfigureAwait(false); + return; + } + + if (db.Database.CurrentTransaction is not null) + { + await Settings.Where(x => x.Group == group).ExecuteDeleteAsync(ct).ConfigureAwait(false); + await db.SaveChangesAsync(ct).ConfigureAwait(false); + } + else + { + var strategy = db.Database.CreateExecutionStrategy(); + await strategy.ExecuteAsync(async () => + { + await using var tx = await db.Database.BeginTransactionAsync(ct).ConfigureAwait(false); + await Settings.Where(x => x.Group == group).ExecuteDeleteAsync(ct).ConfigureAwait(false); + await db.SaveChangesAsync(acceptAllChangesOnSuccess: false, ct).ConfigureAwait(false); + await tx.CommitAsync(ct).ConfigureAwait(false); + }).ConfigureAwait(false); + + db.ChangeTracker.AcceptAllChanges(); + } + + writeScope.Committed(); } diff --git a/src/DotNetBoost.Settings.EntityFrameworkCore/EfCoreServiceCollectionExtensions.cs b/src/DotNetBoost.Settings.EntityFrameworkCore/EfCoreServiceCollectionExtensions.cs index dd46910..714d341 100644 --- a/src/DotNetBoost.Settings.EntityFrameworkCore/EfCoreServiceCollectionExtensions.cs +++ b/src/DotNetBoost.Settings.EntityFrameworkCore/EfCoreServiceCollectionExtensions.cs @@ -18,18 +18,29 @@ public static class EntityFrameworkBuilderExtensions /// /// /// + /// Accepts s: it registers + /// over the same context, for them to enlist in. + /// /// Registers nothing against itself. The store is constructed /// from explicitly, so the library never claims a /// framework type it does not own — which in an application with two contexts would /// silently hand a bare DbContext dependency whichever one Settings was /// pointed at. + /// /// public static SettingBuilder UseEntityFrameworkCore(this SettingBuilder builder) where TContext : DbContext { - SettingBuilderGuard.EnsureProviderNotConfigured(builder, "EntityFrameworkCore"); + SettingBuilderGuard.EnsureProviderNotConfigured( + builder, "EntityFrameworkCore", supportsWriteParticipants: true); + + // The scope and the store wrap the same scoped TContext instance, which is the + // whole guarantee a participant relies on: what it enlists is saved by the store. + builder.Services.AddScoped( + sp => new EfCoreSettingWriteScope(sp.GetRequiredService())); builder.Services.AddScoped( - sp => new EfCoreSettingStore(sp.GetRequiredService())); + sp => new EfCoreSettingStore( + sp.GetRequiredService(), sp.GetRequiredService())); return builder; } } diff --git a/src/DotNetBoost.Settings.EntityFrameworkCore/EfCoreSettingWriteScope.cs b/src/DotNetBoost.Settings.EntityFrameworkCore/EfCoreSettingWriteScope.cs new file mode 100644 index 0000000..1347816 --- /dev/null +++ b/src/DotNetBoost.Settings.EntityFrameworkCore/EfCoreSettingWriteScope.cs @@ -0,0 +1,61 @@ +using DotNetBoost.Settings.Core.Interfaces; +using Microsoft.EntityFrameworkCore; + +namespace DotNetBoost.Settings.EntityFrameworkCore; + +/// +/// Where an enlists entities in a settings write on the +/// Entity Framework Core store. Take it as a dependency; UseEntityFrameworkCore() +/// registers it scoped. +/// +/// +/// +/// It wraps the very context instance the store saves, so an entity added here goes out in the +/// store's own SaveChangesAsync and commits with the rows, or not at all. That is why a +/// participant adds through this rather than through a context of its own: an entity tracked on +/// any other context would never be saved, and nothing would say so. An entity type that is not +/// mapped on the settings context throws here instead, which aborts the write. +/// +/// +/// Add, never save. If the write does not commit, everything added here is detached again, so +/// it cannot be saved later by the application's next SaveChangesAsync on the same +/// context. Only the entity passed in is detached, so enlist standalone entities rather than +/// graphs with new related entities. +/// +/// +/// The context the settings store saves. +public sealed class EfCoreSettingWriteScope(DbContext context) +{ + private readonly List _enlisted = []; + + /// The context the settings store saves. + internal DbContext Context { get; } = context ?? throw new ArgumentNullException(nameof(context)); + + internal bool HasEnlisted => _enlisted.Count > 0; + + /// + /// Tracks as added on the settings store's context, to be saved + /// with the current write. + /// + /// + /// The entity's type is not mapped on the settings context. + /// + public void Add(object entity) + { + ArgumentNullException.ThrowIfNull(entity); + Context.Add(entity); + _enlisted.Add(entity); + } + + /// The write committed what was enlisted; forget it without detaching. + internal void Committed() => _enlisted.Clear(); + + /// The write will not commit; detach what was enlisted. + internal void Discard() + { + foreach (var entity in _enlisted) + Context.Entry(entity).State = EntityState.Detached; + + _enlisted.Clear(); + } +} diff --git a/tests/DotNetBoost.Settings.IntegrationTests/Stores/EfCoreWriteParticipantIntegrationTests.cs b/tests/DotNetBoost.Settings.IntegrationTests/Stores/EfCoreWriteParticipantIntegrationTests.cs new file mode 100644 index 0000000..8457849 --- /dev/null +++ b/tests/DotNetBoost.Settings.IntegrationTests/Stores/EfCoreWriteParticipantIntegrationTests.cs @@ -0,0 +1,35 @@ +using DotNetBoost.Settings.ProviderTests.Stores; +using Microsoft.EntityFrameworkCore; + +namespace DotNetBoost.Settings.IntegrationTests.Stores; + +/// +/// The participant suite on a real PostgreSQL server, where a transaction and a concurrency +/// conflict are the server's, not SQLite's. +/// +[Collection(PostgreSqlCollection.Name)] +[Trait("Category", "Integration")] +public sealed class EfCorePostgresWriteParticipantTests(PostgreSqlFixture fixture) : EfCoreWriteParticipantTests +{ + protected override async Task> NewDatabaseAsync() + { + var connectionString = await fixture.CreateDatabaseAsync(); + return o => o.UseNpgsql(connectionString); + } +} + +/// +/// The participant suite on a real SQL Server, under a retrying execution strategy. A clear +/// that opened its transaction outside the strategy would throw here on every call, which is +/// why the store opens it inside. +/// +[Collection(SqlServerCollection.Name)] +[Trait("Category", "Integration")] +public sealed class EfCoreSqlServerWriteParticipantTests(SqlServerFixture fixture) : EfCoreWriteParticipantTests +{ + protected override async Task> NewDatabaseAsync() + { + var connectionString = await fixture.CreateDatabaseAsync(); + return o => o.UseSqlServer(connectionString, sql => sql.EnableRetryOnFailure()); + } +} diff --git a/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreWriteParticipantTests.cs b/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreWriteParticipantTests.cs new file mode 100644 index 0000000..2be8605 --- /dev/null +++ b/tests/DotNetBoost.Settings.ProviderTests/Stores/EfCoreWriteParticipantTests.cs @@ -0,0 +1,396 @@ +using DotNetBoost.Settings.Core; +using DotNetBoost.Settings.Core.Attributes; +using DotNetBoost.Settings.Core.Interfaces; +using DotNetBoost.Settings.Core.Models; +using DotNetBoost.Settings.EntityFrameworkCore; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; + +namespace DotNetBoost.Settings.ProviderTests.Stores; + +/// A context mapping the settings beside an application entity, as a real one does. +public sealed class ParticipantDbContext(DbContextOptions options) : DbContext(options) +{ + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.ApplySettings(this); + modelBuilder.Entity(b => + { + b.ToTable("AuditRows"); + b.HasKey(x => x.Id); + b.Property(x => x.Id).ValueGeneratedNever(); + b.Property(x => x.Group).HasMaxLength(191); + b.Property(x => x.Kind).HasMaxLength(32); + b.Property(x => x.CorrelationId).HasMaxLength(191); + }); + } +} + +/// What an auditing participant records: one row per write. +public sealed class AuditRow +{ + public Guid Id { get; set; } = Guid.NewGuid(); + public string Group { get; set; } = ""; + public string Kind { get; set; } = ""; + public string CorrelationId { get; set; } = ""; + public int ChangeCount { get; set; } +} + +/// An entity no context here maps. +public sealed class UnmappedRow +{ + public Guid Id { get; set; } +} + +[SettingGroup("ef-participant")] +public sealed class EfParticipantSettings +{ + public string? Host { get; set; } + public string? Mode { get; set; } +} + +/// +/// The Entity Framework Core store's half of the participant contract, against a real +/// database: what a participant enlists commits with the write or not at all, and a write +/// that does not commit leaves nothing tracked for the application's next save to pick up. +/// Each engine runs it through a subclass; SQLite here, PostgreSQL and SQL Server in the +/// integration tests. +/// +public abstract class EfCoreWriteParticipantTests : IAsyncLifetime +{ + private readonly List _providers = []; + + /// A new, empty database, as the options it is reached through. + protected abstract Task> NewDatabaseAsync(); + + [Fact] + public async Task Set_CommitsTheEnlistedRow_WithTheSettings() + { + var (sp, _) = await BuildAsync(); + + await WithManagerAsync(sp, m => m.For().SetAsync(new EfParticipantSettings { Host = "a" })); + + Assert.Equal("a", (await ReadSettingsAsync(sp)).Host); + var audit = Assert.Single(await ReadAuditAsync(sp)); + Assert.Equal(nameof(EfParticipantSettings), audit.Group); + Assert.Equal(nameof(SettingWriteKind.Updated), audit.Kind); + } + + [Fact] + public async Task SetProperty_CommitsTheEnlistedRow_WithTheSetting() + { + var (sp, _) = await BuildAsync(); + + await WithManagerAsync(sp, m => m.For().SetAsync(x => x.Mode, "strict")); + + Assert.Equal("strict", (await ReadSettingsAsync(sp)).Mode); + Assert.Single(await ReadAuditAsync(sp)); + } + + [Fact] + public async Task Set_ParticipantThrows_CommitsNothing_AndLeavesNothingTracked() + { + var (sp, hooks) = await BuildAsync(); + hooks.ThrowAfterEnlisting = true; + + using var scope = sp.CreateScope(); + var mgr = scope.ServiceProvider.GetRequiredService(); + + await Assert.ThrowsAsync( + () => mgr.For().SetAsync(new EfParticipantSettings { Host = "a" })); + + await AssertNothingPendingAsync(scope); + Assert.Null((await ReadSettingsAsync(sp)).Host); + Assert.Empty(await ReadAuditAsync(sp)); + } + + /// + /// The case participants can do nothing about: they all succeeded, then the conditional + /// update lost to another writer. Neither the audit row nor the modified setting may stay + /// tracked, or the application's next save would write both — an audit of a write that + /// never happened, and the losing value over the winner's. + /// + [Fact] + public async Task Set_ConcurrencyConflict_CommitsNothing_AndLeavesNothingTracked() + { + var (sp, hooks) = await BuildAsync(); + await WithManagerAsync(sp, m => m.For().SetAsync(new EfParticipantSettings { Host = "a" })); + + // Another writer changes the row after this write read it and before it saves. + hooks.BeforeEnlisting = () => ChangeHostElsewhereAsync(sp, "theirs"); + + using var scope = sp.CreateScope(); + var mgr = scope.ServiceProvider.GetRequiredService(); + + await Assert.ThrowsAsync( + () => mgr.For().SetAsync(new EfParticipantSettings { Host = "mine" })); + + await AssertNothingPendingAsync(scope); + Assert.Equal("theirs", (await ReadSettingsAsync(sp)).Host); + Assert.Single(await ReadAuditAsync(sp)); + } + + [Fact] + public async Task Clear_CommitsTheDelete_AndTheEnlistedRow_Together() + { + var (sp, _) = await BuildAsync(); + await WithManagerAsync(sp, m => m.For().SetAsync(new EfParticipantSettings { Host = "a" })); + + await WithManagerAsync(sp, m => m.For().ClearAsync()); + + Assert.Null((await ReadSettingsAsync(sp)).Host); + var audits = await ReadAuditAsync(sp); + var cleared = Assert.Single(audits, a => a.Kind == nameof(SettingWriteKind.Cleared)); + Assert.Equal(1, cleared.ChangeCount); + } + + [Fact] + public async Task Clear_ParticipantThrows_LeavesTheRows_AndNothingTracked() + { + var (sp, hooks) = await BuildAsync(); + await WithManagerAsync(sp, m => m.For().SetAsync(new EfParticipantSettings { Host = "a" })); + hooks.ThrowAfterEnlisting = true; + + using var scope = sp.CreateScope(); + var mgr = scope.ServiceProvider.GetRequiredService(); + + await Assert.ThrowsAsync(() => mgr.For().ClearAsync()); + + await AssertNothingPendingAsync(scope); + Assert.Equal("a", (await ReadSettingsAsync(sp)).Host); + Assert.Single(await ReadAuditAsync(sp)); + } + + /// + /// A clear inside the application's own transaction joins it, rather than committing on its + /// own, so the application's rollback takes back the delete and the enlisted row together. + /// Run through the execution strategy, as an application on a retrying one has to. + /// + [Fact] + public async Task Clear_InsideTheCallersTransaction_IsUndoneByItsRollback() + { + var (sp, _) = await BuildAsync(); + await WithManagerAsync(sp, m => m.For().SetAsync(new EfParticipantSettings { Host = "a" })); + + using (var scope = sp.CreateScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + var mgr = scope.ServiceProvider.GetRequiredService(); + + await db.Database.CreateExecutionStrategy().ExecuteAsync(async () => + { + await using var tx = await db.Database.BeginTransactionAsync(); + await mgr.For().ClearAsync(); + await tx.RollbackAsync(); + }); + } + + Assert.Equal("a", (await ReadSettingsAsync(sp)).Host); + Assert.Single(await ReadAuditAsync(sp)); + } + + [Fact] + public async Task Clear_InsideTheCallersTransaction_CommitsWithIt() + { + var (sp, _) = await BuildAsync(); + await WithManagerAsync(sp, m => m.For().SetAsync(new EfParticipantSettings { Host = "a" })); + + using (var scope = sp.CreateScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + var mgr = scope.ServiceProvider.GetRequiredService(); + + await db.Database.CreateExecutionStrategy().ExecuteAsync(async () => + { + await using var tx = await db.Database.BeginTransactionAsync(); + await mgr.For().ClearAsync(); + await tx.CommitAsync(); + }); + } + + Assert.Null((await ReadSettingsAsync(sp)).Host); + Assert.Equal(2, (await ReadAuditAsync(sp)).Count); + } + + /// + /// An entity the settings context does not map could never be saved with the write. It + /// fails the write on the spot rather than vanishing. + /// + [Fact] + public async Task Participant_EnlistingAnUnmappedEntity_FailsTheWrite() + { + var (sp, hooks) = await BuildAsync(); + hooks.EnlistUnmapped = true; + + await Assert.ThrowsAsync( + () => WithManagerAsync(sp, m => m.For().SetAsync(new EfParticipantSettings { Host = "a" }))); + + Assert.Null((await ReadSettingsAsync(sp)).Host); + } + + private async Task<(ServiceProvider, ParticipantHooks)> BuildAsync() + { + var configure = await NewDatabaseAsync(); + var hooks = new ParticipantHooks(); + + var services = new ServiceCollection(); + services.AddLogging(); + services.AddDbContext(configure); + services.AddSingleton(hooks); + services.AddSettings() + .UseEntityFrameworkCore() + .UseWriteParticipant(); + + var sp = services.BuildServiceProvider(new ServiceProviderOptions { ValidateScopes = true }); + _providers.Add(sp); + + using var scope = sp.CreateScope(); + await scope.ServiceProvider.GetRequiredService().Database.EnsureCreatedAsync(); + + return (sp, hooks); + } + + private static async Task WithManagerAsync(IServiceProvider sp, Func act) + { + using var scope = sp.CreateScope(); + await act(scope.ServiceProvider.GetRequiredService()); + } + + private static async Task ReadSettingsAsync(IServiceProvider sp) + { + using var scope = sp.CreateScope(); + return await scope.ServiceProvider.GetRequiredService() + .For().GetAsync(refreshCache: true); + } + + private static async Task> ReadAuditAsync(IServiceProvider sp) + { + using var scope = sp.CreateScope(); + return await scope.ServiceProvider.GetRequiredService() + .Set().AsNoTracking().ToListAsync(); + } + + private static async Task ChangeHostElsewhereAsync(IServiceProvider sp, string value) + { + using var scope = sp.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + var row = await db.Set().SingleAsync(x => x.Group == nameof(EfParticipantSettings) && x.Key == "Host"); + row.Value = value; + row.RowVersion = Setting.NewRowVersion(); + await db.SaveChangesAsync(); + } + + /// + /// Nothing left for the application's next save on the same context to write: no entry + /// added, modified or deleted, and a save that writes nothing. + /// + private static async Task AssertNothingPendingAsync(IServiceScope scope) + { + var db = scope.ServiceProvider.GetRequiredService(); + + Assert.DoesNotContain(db.ChangeTracker.Entries(), e => e.State is EntityState.Added or EntityState.Modified or EntityState.Deleted); + Assert.Equal(0, await db.SaveChangesAsync()); + } + + public Task InitializeAsync() => Task.CompletedTask; + + public virtual async Task DisposeAsync() + { + foreach (var sp in _providers) await sp.DisposeAsync(); + } + + public sealed class ParticipantHooks + { + /// Runs once, on the next write, before the participant enlists. + public Func? BeforeEnlisting { get; set; } + public bool ThrowAfterEnlisting { get; set; } + public bool EnlistUnmapped { get; set; } + } + + /// An auditing participant, written the way the docs say to write one. + public sealed class AuditParticipant(EfCoreSettingWriteScope scope, ParticipantHooks hooks) : ISettingWriteParticipant + { + public async Task OnWritingAsync(SettingWrite write, CancellationToken ct = default) + { + if (hooks.BeforeEnlisting is { } before) + { + hooks.BeforeEnlisting = null; + await before(); + } + + scope.Add(hooks.EnlistUnmapped + ? new UnmappedRow { Id = Guid.NewGuid() } + : new AuditRow + { + Group = write.GroupName, + Kind = write.Kind.ToString(), + CorrelationId = write.CorrelationId, + ChangeCount = write.Changes.Count + }); + + if (hooks.ThrowAfterEnlisting) + throw new InvalidOperationException("participant failed after enlisting"); + } + } +} + +public sealed class EfCoreSqliteWriteParticipantTests : EfCoreWriteParticipantTests +{ + private readonly List _connections = []; + + protected override async Task> NewDatabaseAsync() + { + // An in-memory database lives as long as its connection, so every context in a test + // shares this one, held open until the test ends. + var connection = new SqliteConnection("DataSource=:memory:"); + await connection.OpenAsync(); + _connections.Add(connection); + return o => o.UseSqlite(connection); + } + + public override async Task DisposeAsync() + { + await base.DisposeAsync(); + foreach (var c in _connections) await c.DisposeAsync(); + } +} + +/// Registration and wiring checks that need no database round trip. +public class EfCoreWriteParticipantRegistrationTests +{ + [Fact] + public void Store_RefusesAWriteScope_OverAnotherContext() + { + var options = new DbContextOptionsBuilder().UseSqlite("DataSource=:memory:").Options; + using var one = new ParticipantDbContext(options); + using var other = new ParticipantDbContext(options); + + Assert.Throws(() => new EfCoreSettingStore(one, new EfCoreSettingWriteScope(other))); + } + + [Fact] + public void UseEntityFrameworkCore_AcceptsAParticipant_InBuild() + { + var services = new ServiceCollection(); + services.AddDbContext(o => o.UseSqlite("DataSource=:memory:")); + + services.AddSettings() + .UseEntityFrameworkCore() + .UseWriteParticipant() + .Build(); + } + + [Fact] + public void UseDapper_RefusesAParticipant_InBuild() + { + var services = new ServiceCollection(); + + var builder = services.AddSettings() + .UseDapper(_ => new SqliteConnection("DataSource=:memory:")) + .UseWriteParticipant(); + + var ex = Assert.Throws(() => builder.Build()); + Assert.Contains("'Dapper'", ex.Message, StringComparison.Ordinal); + } +} diff --git a/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs b/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs index 64e6595..fdf514a 100644 --- a/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs +++ b/tests/DotNetBoost.Settings.UnitTests/SettingWriteObserverTests.cs @@ -179,10 +179,11 @@ public async Task Write_UnchangedSensitiveProperty_RaisesNothing_DespiteFreshCip /// /// The reason exists. Deleting every row in a group is the /// most destructive thing the library does, and under the old audit trail it was the one - /// operation that recorded nothing at all. + /// operation that recorded nothing at all. It reports what it deleted, so an audit of a + /// clear can say more than that it happened. /// [Fact] - public async Task Clear_RaisesAClearedNotification_WithNoChanges() + public async Task Clear_RaisesAClearedNotification_ListingWhatItDeleted() { var (mgr, _, observer) = Build(); @@ -193,8 +194,40 @@ public async Task Clear_RaisesAClearedNotification_WithNoChanges() var write = Assert.Single(observer.Writes); Assert.Equal(SettingWriteKind.Cleared, write.Kind); - Assert.Empty(write.Changes); Assert.Equal(nameof(ObservedSettings), write.GroupName); + Assert.Equal( + [ + new SettingChange(nameof(ObservedSettings.Host), "a", null, IsRedacted: false), + new SettingChange(nameof(ObservedSettings.Port), "25", null, IsRedacted: false) + ], + write.Changes); + } + + [Fact] + public async Task Clear_RedactsSensitiveValues_ItDeleted() + { + var (mgr, _, observer) = Build(); + + await mgr.For().SetAsync(new ObservedSettings { Secret = "s3cret" }); + observer.Writes.Clear(); + + await mgr.For().ClearAsync(); + + var change = Single(observer, nameof(ObservedSettings.Secret)); + Assert.True(change.IsRedacted); + Assert.Null(change.OldValue); + } + + [Fact] + public async Task Clear_OfAnEmptyGroup_StillNotifies_WithNoChanges() + { + var (mgr, _, observer) = Build(); + + await mgr.For().ClearAsync(); + + var write = Assert.Single(observer.Writes); + Assert.Equal(SettingWriteKind.Cleared, write.Kind); + Assert.Empty(write.Changes); } /// diff --git a/tests/DotNetBoost.Settings.UnitTests/SettingWriteParticipantTests.cs b/tests/DotNetBoost.Settings.UnitTests/SettingWriteParticipantTests.cs new file mode 100644 index 0000000..04dd03c --- /dev/null +++ b/tests/DotNetBoost.Settings.UnitTests/SettingWriteParticipantTests.cs @@ -0,0 +1,428 @@ +using DotNetBoost.Settings.Core; +using DotNetBoost.Settings.Core.Attributes; +using DotNetBoost.Settings.Core.Interfaces; +using DotNetBoost.Settings.Core.Models; +using DotNetBoost.Settings.Core.Services; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; + +namespace DotNetBoost.Settings.UnitTests; + +/// +/// The hook that runs inside a write. These use a fake store that keeps what participants +/// enlist apart until a write commits it, so "committed with the write" and "discarded with +/// it" are both observable without a database. The Entity Framework Core store's half of the +/// contract is tested against real databases in the provider and integration tests. +/// +public class SettingWriteParticipantTests +{ + [Fact] + public async Task Participant_RunsBeforeTheStore_AndWhatItEnlistsCommitsWithTheWrite() + { + var store = new EnlistingStore(); + var rowsWhenCalled = -1; + var participant = new Participant(async w => + { + rowsWhenCalled = (await store.GetGroupAsync(w.GroupName)).Count; + store.Enlist("audit"); + }); + var mgr = Build(store, participant); + + await mgr.For().SetAsync(new ParticipantSettings { Host = "a" }); + + Assert.Equal(0, rowsWhenCalled); + Assert.Equal(["audit"], store.Committed); + Assert.Empty(store.Enlisted); + } + + /// + /// One write, one : an audit entry recorded by a participant and a + /// log line written by an observer must be joinable on the correlation id. + /// + [Fact] + public async Task Participant_AndObserver_ReceiveTheSameWrite() + { + var participant = new Participant(); + var observer = new Observer(); + var mgr = Build(new EnlistingStore(), participant, observer); + + await mgr.For().SetAsync(new ParticipantSettings { Host = "a" }); + + Assert.Same(Assert.Single(participant.Writes), Assert.Single(observer.Writes)); + } + + [Fact] + public async Task Participant_ThatThrows_AbortsTheWrite_AndDiscardsWhatEarlierOnesEnlisted() + { + var store = new EnlistingStore(); + var first = new Participant(_ => { store.Enlist("audit"); return Task.CompletedTask; }); + var throwing = new Participant(_ => throw new InvalidOperationException("audit store is down")); + var observer = new Observer(); + var handler = new ChangedHandler(); + var mgr = Build(store, [first, throwing], observer, handler); + + var ex = await Assert.ThrowsAsync( + () => mgr.For().SetAsync(new ParticipantSettings { Host = "a" })); + + Assert.Equal("audit store is down", ex.Message); + Assert.Equal(0, store.Writes); + Assert.Empty(await store.GetGroupAsync(nameof(ParticipantSettings))); + Assert.Equal(1, store.Discards); + Assert.Empty(store.Enlisted); + Assert.Empty(store.Committed); + + // After-commit hooks hear only about writes that committed. + Assert.Empty(observer.Writes); + Assert.False(handler.WasCalled); + } + + /// + /// A participant's failure must leave the cache as it was. Evicting would be harmless here, + /// but evicting before the store is called is the order that, under a real failure between + /// the two, serves a value that was never written. + /// + [Fact] + public async Task Participant_ThatThrows_LeavesTheCachedEntry() + { + var store = new EnlistingStore(); + var fail = false; + var participant = new Participant(_ => fail ? throw new InvalidOperationException() : Task.CompletedTask); + var mgr = Build(store, [participant], cache: new SettingCache(new MemoryCache(new MemoryCacheOptions()))); + var settings = mgr.For(); + + await settings.SetAsync(new ParticipantSettings { Host = "a" }); + Assert.Equal("a", (await settings.GetAsync()).Host); + + fail = true; + await Assert.ThrowsAsync( + () => settings.SetAsync(new ParticipantSettings { Host = "b" })); + + var readsBefore = store.GroupReads; + Assert.Equal("a", (await settings.GetAsync()).Host); + Assert.Equal(readsBefore, store.GroupReads); + } + + /// + /// The store can fail after every participant succeeded — a concurrency conflict is the + /// usual case. What they enlisted must not survive it. + /// + [Fact] + public async Task StoreThatFails_DiscardsWhatParticipantsEnlisted() + { + var store = new EnlistingStore { FailWrites = true }; + var participant = new Participant(_ => { store.Enlist("audit"); return Task.CompletedTask; }); + var observer = new Observer(); + var mgr = Build(store, participant, observer); + + await Assert.ThrowsAsync( + () => mgr.For().SetAsync(new ParticipantSettings { Host = "a" })); + + Assert.Equal(1, store.Discards); + Assert.Empty(store.Enlisted); + Assert.Empty(store.Committed); + Assert.Empty(observer.Writes); + } + + /// + /// Running a participant against a store that cannot commit what it enlists would look + /// like an audit trail and not be one. Refused at run time too, because Build() + /// does not see registrations made outside the builder. + /// + [Fact] + public async Task Participant_WithAStoreThatCannotEnlist_RefusesTheWrite() + { + var store = new PlainStore(); + var participant = new Participant(); + var services = new ServiceCollection(); + services.AddSingleton(participant); + var mgr = new SettingManager(store, new NoCache(), services.BuildServiceProvider(), + NullLogger.Instance); + + var ex = await Assert.ThrowsAsync( + () => mgr.For().SetAsync(new ParticipantSettings { Host = "a" })); + + Assert.Contains(nameof(PlainStore), ex.Message, StringComparison.Ordinal); + Assert.Empty(participant.Writes); + Assert.Equal(0, store.Writes); + } + + [Fact] + public async Task Participant_RunsOnEveryWritePath() + { + var participant = new Participant(); + var mgr = Build(new EnlistingStore(), participant); + var settings = mgr.For(); + + await settings.SetAsync(new ParticipantSettings { Host = "a" }); + await settings.SetAsync(x => x.Port, 587); + await settings.ClearAsync(); + + Assert.Equal( + [SettingWriteKind.Updated, SettingWriteKind.Updated, SettingWriteKind.Cleared], + participant.Writes.Select(w => w.Kind)); + } + + [Fact] + public async Task Write_ThatChangesNothing_CallsNoParticipant() + { + var participant = new Participant(); + var mgr = Build(new EnlistingStore(), participant); + + await mgr.For().SetAsync(new ParticipantSettings { Host = "a" }); + await mgr.For().SetAsync(new ParticipantSettings { Host = "a" }); + + Assert.Single(participant.Writes); + } + + /// + /// A participant on a clear runs while the rows still exist and is told what they held, + /// which is what an audit of a clear records. + /// + [Fact] + public async Task Clear_Participant_SeesTheValuesAboutToBeDeleted() + { + var store = new EnlistingStore(); + var participant = new Participant(); + var mgr = Build(store, participant); + + await mgr.For().SetAsync(new ParticipantSettings { Host = "a" }); + await mgr.For().ClearAsync(); + + var cleared = participant.Writes[^1]; + Assert.Equal(SettingWriteKind.Cleared, cleared.Kind); + Assert.Equal( + [ + new SettingChange(nameof(ParticipantSettings.Host), "a", null, IsRedacted: false), + new SettingChange(nameof(ParticipantSettings.Port), "0", null, IsRedacted: false) + ], + cleared.Changes); + } + + [Fact] + public async Task Clear_Participant_ThatThrows_LeavesTheRows() + { + var store = new EnlistingStore(); + var fail = false; + var participant = new Participant(_ => fail ? throw new InvalidOperationException() : Task.CompletedTask); + var mgr = Build(store, participant); + + await mgr.For().SetAsync(new ParticipantSettings { Host = "a" }); + var stored = (await store.GetGroupAsync(nameof(ParticipantSettings))).Count; + fail = true; + + await Assert.ThrowsAsync(() => mgr.For().ClearAsync()); + + Assert.NotEqual(0, stored); + Assert.Equal(stored, (await store.GetGroupAsync(nameof(ParticipantSettings))).Count); + Assert.Equal(1, store.Discards); + } + + [Fact] + public void Validator_RefusesAParticipant_ForAProviderThatCannotEnlist() + { + var services = new ServiceCollection(); + services.AddScoped(); + + var ex = Assert.Throws( + () => SettingBuilderValidator.ValidateWriteParticipants(services, "Dapper", providerSupportsParticipants: false)); + + Assert.Contains("'Dapper'", ex.Message, StringComparison.Ordinal); + Assert.Contains(nameof(Participant), ex.Message, StringComparison.Ordinal); + } + + [Fact] + public void Validator_AcceptsAParticipant_ForAProviderThatCanEnlist() + { + var services = new ServiceCollection(); + services.AddScoped(); + + SettingBuilderValidator.ValidateWriteParticipants(services, "EntityFrameworkCore", providerSupportsParticipants: true); + } + + [Fact] + public void Validator_AcceptsAnyProvider_WhenNoParticipantIsRegistered() + => SettingBuilderValidator.ValidateWriteParticipants(new ServiceCollection(), "Dapper", providerSupportsParticipants: false); + + [Fact] + public void UseWriteParticipant_Registers_AndTheGuardRecordsSupport() + { + var builder = new ServiceCollection().AddSettings(); + SettingBuilderGuard.EnsureProviderNotConfigured(builder, "Fake", supportsWriteParticipants: true); + + builder.UseWriteParticipant(); + + Assert.Contains(builder.Services, d => + d.ServiceType == typeof(ISettingWriteParticipant) && d.ImplementationType == typeof(Participant)); + Assert.True(builder.SupportsWriteParticipants); + } + + [Fact] + public void Guard_WithoutTheFlag_RecordsNoSupport() + { + var builder = new ServiceCollection().AddSettings(); + SettingBuilderGuard.EnsureProviderNotConfigured(builder, "Fake"); + + Assert.False(builder.SupportsWriteParticipants); + } + + private static SettingManager Build( + ISettingStore store, Participant participant, Observer? observer = null) + => Build(store, [participant], observer); + + private static SettingManager Build( + ISettingStore store, Participant[] participants, Observer? observer = null, + ChangedHandler? handler = null, ISettingCache? cache = null) + { + var services = new ServiceCollection(); + foreach (var p in participants) + services.AddSingleton(p); + if (observer is not null) + services.AddSingleton(observer); + if (handler is not null) + services.AddSingleton>(handler); + + return new SettingManager(store, cache ?? new NoCache(), services.BuildServiceProvider(), + NullLogger.Instance); + } + + [SettingGroup("participant")] + public class ParticipantSettings + { + public string? Host { get; set; } + public int Port { get; set; } + } + + private sealed class Participant(Func? onWriting = null) : ISettingWriteParticipant + { + public Participant() : this(null) { } + + public List Writes { get; } = []; + + public Task OnWritingAsync(SettingWrite write, CancellationToken ct = default) + { + Writes.Add(write); + return onWriting?.Invoke(write) ?? Task.CompletedTask; + } + } + + private sealed class Observer : ISettingWriteObserver + { + public List Writes { get; } = []; + + public Task OnWrittenAsync(SettingWrite write, CancellationToken ct = default) + { + Writes.Add(write); + return Task.CompletedTask; + } + } + + private sealed class ChangedHandler : ISettingChangedHandler + { + public bool WasCalled { get; private set; } + + public Task OnChangedAsync(ParticipantSettings previous, ParticipantSettings current, CancellationToken ct = default) + { + WasCalled = true; + return Task.CompletedTask; + } + } + + /// + /// Holds what participants enlist apart from what it has stored, and commits it only when + /// one of its writes succeeds — the contract an enlisting store has to keep. + /// + private sealed class EnlistingStore : PlainStore, IEnlistingSettingStore + { + public List Enlisted { get; } = []; + public List Committed { get; } = []; + public int Discards { get; private set; } + public bool FailWrites { get; init; } + + public void Enlist(string item) => Enlisted.Add(item); + + public void DiscardEnlisted() + { + Discards++; + Enlisted.Clear(); + } + + protected override void BeforeWrite() + { + if (FailWrites) throw new SettingConcurrencyException(nameof(ParticipantSettings), "Host"); + } + + protected override void AfterWrite() + { + Committed.AddRange(Enlisted); + Enlisted.Clear(); + } + } + + /// A store whose reads reflect its writes, and which cannot enlist anything. + private class PlainStore : ISettingStore + { + private readonly Dictionary _data = new(StringComparer.Ordinal); + private static string K(string g, string k) => g + "|" + k; + + public int Writes { get; private set; } + public int GroupReads { get; private set; } + + protected virtual void BeforeWrite() { } + protected virtual void AfterWrite() { } + + public Task> GetGroupAsync(string group, CancellationToken ct = default) + { + GroupReads++; + return Task.FromResult>(_data.Values.Where(x => x.Group == group).ToList()); + } + + public Task GetAsync(string group, string key, CancellationToken ct = default) + => Task.FromResult(_data.GetValueOrDefault(K(group, key))); + + public Task UpsertAsync(Setting setting, CancellationToken ct = default) + => UpsertManyAsync([setting], ct); + + public Task UpsertManyAsync(IEnumerable settings, CancellationToken ct = default) + { + BeforeWrite(); + foreach (var s in settings) + { + _data[K(s.Group, s.Key)] = new Setting + { + Id = s.Id, Group = s.Group, Key = s.Key, Value = s.Value, Type = s.Type, + IsEncrypted = s.IsEncrypted, UpdatedAt = s.UpdatedAt + }; + } + Writes++; + AfterWrite(); + return Task.CompletedTask; + } + + public Task DeleteAsync(string group, string key, CancellationToken ct = default) + { + _data.Remove(K(group, key)); + return Task.CompletedTask; + } + + public Task DeleteGroupAsync(string group, CancellationToken ct = default) + { + BeforeWrite(); + foreach (var k in _data.Where(x => x.Value.Group == group).Select(x => x.Key).ToList()) + _data.Remove(k); + Writes++; + AfterWrite(); + return Task.CompletedTask; + } + + public Task CountAsync(string group, CancellationToken ct = default) + => Task.FromResult(_data.Values.Count(x => x.Group == group)); + } + + private sealed class NoCache : ISettingCache + { + public bool TryGetValue(string key, out T? value) { value = default; return false; } + public void Set(string key, T value, TimeSpan duration) { } + public void Remove(string key) { } + } +}