diff --git a/CHANGELOG.md b/CHANGELOG.md index 28acb94..57fbe65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,21 @@ 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. +- 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/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/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 c268810..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,14 +137,23 @@ 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 +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. +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 @@ -86,6 +193,27 @@ 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; 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. + +| 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: @@ -100,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/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/Interfaces/ISettingWriteObserver.cs b/src/DotNetBoost.Settings.Core/Interfaces/ISettingWriteObserver.cs index 05efceb..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, @@ -67,11 +75,27 @@ 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 -/// 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. /// /// /// @@ -88,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 cea2ff9..9613a4f 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 @@ -252,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); @@ -325,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); @@ -395,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); } @@ -438,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); - if (observers.Count == 0) return; + 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(); + } - var write = new SettingWrite( + /// + /// 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()]); + + /// + /// 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, @@ -464,9 +507,84 @@ 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 + }; + + /// + /// 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; + } + } - foreach (var observer in observers) + /// + /// 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 { @@ -475,6 +593,7 @@ private async Task NotifyWrittenAsync( catch (Exception ex) { LogWriteObserverFailed(_logger, observer.GetType().Name, map.GroupName, ex); + Metrics.WriteObserverFailed(observer.GetType(), map.GroupName, ex); } } } @@ -538,6 +657,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); } } } @@ -607,7 +727,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) @@ -714,10 +834,17 @@ 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); - 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/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/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 cab21cf..541c731 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); } } @@ -125,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 658f63c..fdf514a 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; @@ -178,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(); @@ -192,8 +194,61 @@ 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); + } + + /// + /// 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)); } /// @@ -225,6 +280,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() { @@ -287,6 +422,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 { @@ -317,6 +459,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; 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) { } + } +}