Repository navigation
Close the composition gaps found by the sample: write participants, GroupType, failure counters - #10
Merged
Merged
Conversation
SettingWrite named the group by GroupName and Route but not by type, so an observer that needed the class's attributes had to rebuild the map itself: scan an assembly, call SettingGroupAttribute.ResolveName on every [SettingGroup] class, and throw for a group outside the scanned assembly. Auditing needs the type to apply [AuditIgnore] and [AuditRedact], and the sample's audit observer did exactly that scan. GroupType is an init property rather than a new positional parameter. A positional one changes the constructor and Deconstruct, which breaks compiled callers even with a default value. As a result it is nullable: the library sets it on every write it raises, and it is null only on a SettingWrite built elsewhere, such as in a test. The manager's TypeMap now carries the Type, so NotifyWrittenAsync sets it once and all three write paths (group set, single-property set, clear) get it. The docs say to key a group on GroupName, not on the type's name. Rows are found by the [SettingGroup] Name, or the class's simple name, so moving a class to another namespace changes its FullName but not what it reads; a record keyed on the type would split one group's history in two. Verified: the solution builds with 0 warnings, and the unit tests pass (189 x2, net8.0 and net10.0), including a new one that checks GroupType on all three write paths. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Write observers and change handlers run after the write commits, so their exceptions are logged and swallowed (events 1007 and 1002). That stays. But the log line was the only trace: nothing counted them, so a steadily failing observer showed up only for someone reading logs. This adds the library's first meter, DotNetBoost.Settings, with two counters beside those log lines: dotnetboost.settings.write_observer.failures and dotnetboost.settings.change_handler.failures. Each is tagged with the implementation's class name (as in the log line), the group, and error.type. Every tag is bounded by the application's own types, never by a value. The meter comes from the container's IMeterFactory when there is one, which every generic host registers, so each host, a test host included, gets its own meter, disposed with it. Without a factory (a bare ServiceCollection, or a manager constructed by hand) the counters fall back to one process-wide meter. SettingMetrics is internal and is resolved from the manager's service provider only when something fails, the same way observers and handlers are resolved, so SettingManager's public constructor is unchanged. No OnObserverFailure(Throw) option, though the gap notes first proposed one. ChangeSignalPublisher is itself an observer, so it would surface change-signal failures the read path deliberately tolerates; it would skip the change handlers that run after observers; and reporting a committed write as failed invites a retry. Tests can assert the counter instead. The sample exports the meter, and the write-notifications doc lists both instruments and their tags. Verified: the solution builds with 0 warnings; unit (193 x2), provider (54 x2) and API (51 x2) tests pass. Both counter tests fail with the two counting calls removed. Integration tests were not run; nothing they cover changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Observers run after the store has committed, and their exceptions are logged and swallowed. That is right for notifications, and wrong for an audit entry: a failure to record leaves the change standing with no entry, and the observer has to open a scope and save a second time, outside the write's transaction. That is the flaw the built-in audit trail was removed for in preview.2, and the sample's audit observer had it again. ISettingWriteParticipant runs inside every write, before the store is called. It enlists entities through EfCoreSettingWriteScope.Add, which wraps the very context instance the EF Core store saves, so the store's own SaveChangesAsync commits them with the rows. An entity on any other context would never be saved; one the settings context does not map throws on Add and aborts the write. A participant's exception aborts the write: the store is not called, the cache keeps its entry, observers and change handlers do not run, and what earlier participants enlisted is discarded. If the store fails after them (a concurrency conflict), the enlisted work is discarded too. The manager builds one SettingWrite per write, before the store call, and hands the same instance to participants and observers, so an audit entry and a log line share a correlation id. Only a store that implements IEnlistingSettingStore accepts participants, and a provider declares it through a new EnsureProviderNotConfigured overload. EF Core does. Dapper opens its own transaction per write and MongoDB uses none, so Build() refuses a participant for either, and a write refuses one at run time for a store that cannot enlist. Running them anyway would look like an audit trail and not be one, as the old Dapper SettingAudits table did. A clear now reports what it deleted: one SettingChange per row with the stored value as OldValue, redacted for [Sensitive] properties and any encrypted row. The group is read first only when a participant or observer is registered. In the EF store, a clear with something enlisted runs ExecuteDelete and SaveChanges in one transaction: the caller's when there is one, otherwise its own inside the execution strategy, since a transaction opened outside a retrying strategy throws. Also fixed in passing: a failed EF write left its modified and added Setting entities tracked on the scoped context, so the application's next unrelated SaveChangesAsync wrote the losing value over the winner's. They are now detached when the save fails. Verified: the solution builds with 0 warnings; unit (210 x2), provider (66 x2), API (51 x2) and integration (158, Testcontainers Postgres, SQL Server and MongoDB) tests pass. The EF participant suite runs on SQLite, Postgres, and SQL Server under EnableRetryOnFailure. Removing the store's detach, the manager's discard, or the save in the clear's transaction each fails the tests that cover it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Composing Settings with DotNetBoost.Auditing, MultiLingual and Media in one
DbContextin dotnetboost-sample showed three gaps. One commit per gap.Write participants (
ec59bd5)Observers run after the commit and swallow their exceptions, which is right for notifications and wrong for an audit entry. A failed record left the change with no entry, and the sample's observer had to open a scope and save a second time, outside the write's transaction. That is the flaw the built-in audit trail was removed for in preview.2.
ISettingWriteParticipant(.UseWriteParticipant<T>()) runs inside every write, before the store saves. It enlists entities throughEfCoreSettingWriteScope.Add, which wraps the same context instance the EF Core store saves, so they commit with the rows or not at all. If a participant throws, the write is aborted: the store isn't called, the cache keeps its entry, observers and change handlers don't run, and anything already enlisted is discarded. Enlisted work is also discarded when the store itself fails, for example on a concurrency conflict.SettingWrite, so they share a correlation id.IEnlistingSettingStoreand declaring it through a newSettingBuilderGuard.EnsureProviderNotConfiguredoverload. Only EF Core does.Build()refuses a participant registered with Dapper or MongoDB, and a write checks again at run time.Changesgets one entry per deleted row, with the old value redacted for[Sensitive]or encrypted rows. Before,Changeswas empty. The group is read first only when a participant or observer is registered.SaveChangesAsyncrun in one transaction. That's the caller's transaction if there is one; otherwise the store opens its own inside the execution strategy, so it works underEnableRetryOnFailure.Settingentities tracked on the shared context, so the app's next unrelated save wrote the losing value. They're now detached.SettingWrite.GroupType(3ee8749)GroupTypegives observers the settings class without scanning assemblies. It's an init property, so existing code that constructsSettingWritekeeps compiling. The docs say to key onGroupName, because moving the class to another namespace changes the type's full name but not the rows it reads.Failure counters (
72e6b95)The library's first meter,
DotNetBoost.Settings, comes from the container'sIMeterFactorywhen there is one. It has two counters:dotnetboost.settings.write_observer.failuresdotnetboost.settings.change_handler.failuresEach is tagged with the implementation class, the group and
error.type. Failures are still logged and swallowed; the counters are new. I considered anOnObserverFailure(Throw)option and left it out: the change-signal publisher is itself an observer, throwing would skip the change handlers, and reporting a committed write as failed invites the caller to retry it.For review
The new public API is what 1.0 will freeze, so the names matter most:
ISettingWriteParticipantIEnlistingSettingStoreEfCoreSettingWriteScopeUseWriteParticipant<T>()SettingBuilderValidator.ValidateWriteParticipantsEnsureProviderNotConfiguredoverloadAn app with no participants is unaffected, apart from the clear change above and the EF Core detach fix.
Verification
EnableRetryOnFailure.🤖 Generated with Claude Code