Make the EF Core capture safe beside other SaveChanges interceptors (1.0.0-preview.3)

CHANGELOG.md

@@ -5,6 +5,67 @@ This project follows [Semantic Versioning](https://semver.org/).
 
 ## [Unreleased]
 
+Makes the EF Core capture safe beside another library's `SaveChanges` interceptor — such as
+DotNetBoost.MultiLingual's — registered before it or after it, with or without a retrying strategy. 543
+tests on net8.0 and net10.0 needing nothing installed, plus 280 integration tests against
+PostgreSQL, SQL Server and MongoDB in containers.
+
+### Added
+- **`OnLostAtomicity(LostAtomicity.Report | Throw)`** on the builder, **`IAuditObserver.AtomicityLost`**
+  (a default member, so existing observers still compile) and the
+  **`dotnetboost.auditing.atomicity.lost`** counter. When an interceptor registered earlier commits
+  the transaction in its own `SavedChanges`, the deferred `Created` entries can no longer be atomic
+  with the change. That used to happen silently; it is now reported (the default — the
+  `LoggingAuditObserver` logs it as an error), or fails the save under `Throw`.
+
+### Changed
+- **With another `SaveChanges` interceptor registered after it, the capture commits its own
+  transaction from EF's `DbContext.SavedChanges` event,** once every interceptor has finished,
+  instead of in its own `SavedChanges`. The later interceptor's post-save work is then inside the
+  transaction, and a failure in it rolls the entity and its entry back. The event is synchronous, so
+  that commit is one blocking round trip, even under `SaveChangesAsync`; with no interceptor after
+  it, the commit stays where it was.
+- **The EF Core capture adds its entries to the context being saved, instead of through
+  `IAuditStore.AppendAsync`.** The store is bound to the scope's registered context, which is not
+  always the one saving (see the second-context fix below). Explicit `IAuditManager` calls still go
+  through the store. So a store wrapped or replaced to observe entries as they are recorded no
+  longer sees the capture's: count them with the `dotnetboost.auditing.entries.recorded` counter,
+  which still counts every one, or read them from the table.
+
+### Fixed
+- **A save another interceptor issues from inside this one is nested, not a new save.** It joins
+  the save in progress; only the outermost save writes the deferred entries, commits or rolls back.
+  Before, the nested save's completion did all three and the outer save's found nothing left.
+- **A save replayed by another interceptor is recorded once.** DotNetBoost.MultiLingual replays a
+  save under a retrying strategy; with the capture registered first, every change was recorded twice,
+  and again after each retry. Either registration order is now correct.
+- **A rollback that fails while a save is already failing no longer hides why the save failed.**
+  The rollback is best-effort on those paths: disposing the transaction rolls back what is left, and
+  the caller sees the save's own exception.
+- **A save abandoned when a later interceptor's `SavingChanges` throws is cleaned up by the next
+  save** on that context, which rolls back the transaction left open and records each change once.
+  EF reports nothing for such a save, so until the next save the transaction stays open.
+- **A concurrency conflict now rolls back the capture's transaction.** EF reports it through
+  `ThrowingConcurrencyException`, not `SaveChangesFailed`, so the transaction stayed open. The
+  rollback waits for EF's `DbContext.SaveChangesFailed` event: EF raises the hook while it is still
+  reading the batch, and PostgreSQL and SQL Server refuse a `ROLLBACK` on a busy connection — whose
+  error would then replace `DbUpdateConcurrencyException`.
+- **A cancelled save no longer leaves the interceptor's transaction open.** EF reports a
+  cancellation through `SaveChangesCanceled`, which the interceptor did not handle, so a save with
+  a deferred creation kept its transaction and its state on the context, and retrying the same
+  save recorded the creation twice. It now rolls back exactly as it does for `SaveChangesFailed`.
+- **A creation inside a `TransactionScope` no longer throws.** The interceptor tried to open a
+  transaction of its own inside the ambient one, which EF refuses. It now opens none, and both
+  saves enlist in the scope.
+- **A save another interceptor suppressed is no longer recorded.** With `SuppressWithResult`, the
+  interceptor still captured: an update left an entry tracked for the context's next save, and a
+  deferred creation threw because its key never arrived.
+- **A second audited context in a scope now writes its own entries.** The capture recorded through
+  the store, which holds the scope's registered context, so entries for another instance (or
+  another type) were tracked on the wrong context and never written by the save they described.
+  Entries now go to the context being saved, and a context with `UseAuditing(sp)` but no audit
+  tables is refused with the fix in the message.
+
 ### Security
 - **Aspire moved from 13.6.4 to 13.6.5**, for
   [GHSA-f7vh-h5g3-7jrh](https://github.com/advisories/GHSA-f7vh-h5g3-7jrh) (CVE-2026-120006), an

README.md

@@ -284,7 +284,8 @@ change and its trail land together or neither does. A creation with a store-gene
 exception — its id does not exist until the INSERT has run, so that entry needs a second save, and
 the interceptor opens a transaction around both when the caller has none. Assign keys in code and
 creations join the single-save path too; [limitations.md](docs/limitations.md) has the two cases
-that cannot be made atomic.
+that cannot be made atomic, and what changes when
+[another library's interceptor shares the context](docs/limitations.md#sharing-a-dbcontext-with-other-savechanges-interceptors).
 
 **Dapper and MongoDB write when they are called** rather than joining a unit of work, so atomicity
 is yours to arrange: hand the first your transaction, or the second a session, and the guarantee is

docs/configuration-reference.md

@@ -28,6 +28,7 @@ reads the declared entities and refuses a configuration with no store.
 | `.RequireOptIn()` | Core | Record only entities marked `[Audited]` |
 | `.WithoutCreatedAndDeletedValues()` | Core | Record that a row appeared or went, without its values |
 | `.DisableRecording()` | Core | The kill switch. Reads still work; every record call returns `null` |
+| `.OnLostAtomicity(LostAtomicity)` | Core | When another `SaveChanges` interceptor commits the transaction before the EF Core capture writes its deferred `Created` entries: `Report` (default — write them and report it) or `Throw` (fail the save; use it in tests) |
 | `.UseActorAccessor<TAccessor>()` | Core | Replace how the actor is read. Registered **scoped** — an accessor worth having reads something per request |
 | `.UseEntityKeyAccessor<TAccessor>()` | Core | Replace how entity keys are read. Both EF Core providers register one that reads the model instead |
 | `.UseObserver<TObserver>()` | Core | Be told what the engine decides **not** to record. Registered **singleton**; the default does nothing |

docs/limitations.md

@@ -45,6 +45,100 @@ Both still record, rather than refusing. They are the holes in the headline guar
 
 Neither applies when nothing is deferred — see above.
 
+An ambient `TransactionScope` is not one of them. Inside a scope the interceptor opens nothing:
+both saves enlist in the scope and commit or roll back with it. (The provider has to support
+ambient transactions; EF's SQLite provider does not.)
+
+## Sharing a DbContext with other SaveChanges interceptors
+
+Another library's interceptor on the same context (DotNetBoost.MultiLingual, say) changes one
+thing: **who owns the transaction**. What this interceptor does at each step:
+
+| Step | What it does |
+|---|---|
+| `SavingChanges` | Captures. When there are deferred creations and no transaction is open (neither `Database.CurrentTransaction` nor an ambient `TransactionScope`), opens one. When one is open, remembers it |
+| `SavedChanges` | Writes the deferred entries through the same context. That is a nested `SaveChanges`, which other interceptors see too. Then commits the transaction it opened, unless another `SaveChanges` interceptor is registered after it (next row) |
+| EF's `DbContext.SavedChanges` event | Commits the transaction it opened, when another `SaveChanges` interceptor follows it. EF raises the event once every interceptor's `SavedChanges` has returned, so their post-save work is inside the transaction too |
+| `SaveChangesFailed`, `SaveChangesCanceled`, `ThrowingConcurrencyException` | Rolls back the transaction it opened |
+
+A `SaveChanges` another interceptor issues from inside a save — from its `SavedChanges`, after EF
+has accepted the changes — is recognised as **nested**: its work joins the save in progress, which
+alone writes, commits or rolls back. An interceptor sharing the context should count nested saves
+the same way, because the deferred write above is one.
+
+**Without a retrying strategy the order does not matter**: registered before or after
+`UseAuditing(sp)`, another interceptor's work is inside the same transaction. With Auditing first,
+the commit waits for EF's event — synchronous even under `SaveChangesAsync`, because the event is:
+one blocking round trip, and only on a context where another `SaveChanges` interceptor follows this
+one.
+
+**With a retrying strategy the order does not matter either.** DotNetBoost.MultiLingual, with a
+retrying strategy and no caller transaction, suppresses the save and replays it inside the strategy
+and a transaction of its own. This interceptor records the replay as the save, once per change,
+inside that transaction, and again once per change if the strategy retries it. Registered before
+MultiLingual, it captures the original save first and then throws that capture away when the replay
+starts; registering MultiLingual first saves that work, and nothing else.
+
+**When another interceptor commits first.** An interceptor registered before this one that opens a
+transaction in `SavingChanges` and commits it in its own `SavedChanges` leaves the deferred entries
+nothing to be atomic with: by the time they are written, the transaction is gone. This interceptor
+notices, and by default writes them anyway and reports it — to `IAuditObserver.AtomicityLost` and the
+`dotnetboost.auditing.atomicity.lost` counter. `OnLostAtomicity(LostAtomicity.Throw)` fails the save
+instead, which is what tests should use. Neither undoes the change: it is already committed.
+
+What to do when you share a context:
+
+| Your setup | Do this |
+|---|---|
+| Keys assigned in code (a `Guid`, HiLo, a sequence) | Nothing: no creation is deferred, so there is no second save and no transaction to share |
+| No retrying strategy | Nothing, in either order. To make every save atomic regardless of what other interceptors do, open the transaction yourself (below) |
+| A retrying strategy (`EnableRetryOnFailure`) | Nothing, in either order. Registering an interceptor that replays the save (MultiLingual) before `UseAuditing(sp)` saves a capture that would be thrown away |
+| Another library's table with a composite key | Exclude it with `IgnoreEntity<T>()`: composite keys are refused, so it would otherwise fail every save that writes it. MultiLingual's `Translation` is the case |
+
+The application-owned transaction, in the context. Override the synchronous `SaveChanges(bool)`
+the same way if you call it:
+
+```csharp
+public override async Task<int> SaveChangesAsync(bool accept, CancellationToken ct = default)
+{
+    if (Database.CurrentTransaction is not null || Transaction.Current is not null)
+        return await base.SaveChangesAsync(accept, ct);
+
+    await using var tx = await Database.BeginTransactionAsync(ct);
+    var result = await base.SaveChangesAsync(accept, ct);
+    await tx.CommitAsync(ct);
+    return result;
+}
+```
+
+Under `EnableRetryOnFailure`, wrap the body in `Database.CreateExecutionStrategy().ExecuteAsync(...)`.
+**Do not** use EF's usual `SaveChangesAsync(acceptAllChangesOnSuccess: false)` … `AcceptAllChanges()`
+retry pattern here: this interceptor saves again from `SavedChanges`, before the changes are
+accepted, so the second save would write them again — an added row inserted twice, a deleted one
+failing as a concurrency conflict. It also hides the one signal this interceptor has for telling a
+save nested inside one of its own from a save that never completed — whether EF has accepted the
+outer save's changes — so another interceptor's nested save is taken for a new one, and the
+transaction this interceptor opened is rolled back under it.
+
+## A throw from a later interceptor's `SavingChanges` is cleaned up by the next save
+
+When a creation is deferred, the interceptor opens its transaction in `SavingChanges`. EF calls the
+`SavingChanges` hooks before the part of `SaveChanges` that reports failures, so if an interceptor
+registered **after** `UseAuditing(sp)` throws from its own `SavingChanges`, no hook of this one runs
+again for that save. The transaction stays open on the context **until the context saves again or
+is disposed**. The next save recognises the abandoned one, rolls its transaction back, drops the
+entries it captured, and starts over — so a retry records each change once. Measured, not inferred.
+
+A failure or a cancellation anywhere later in the save — the database refusing a row, a concurrency
+conflict, the request being aborted — is reported, and rolled back at once.
+
+## A suppressed save is not recorded
+
+An interceptor that returns `InterceptionResult<int>.SuppressWithResult(...)` from `SavingChanges`
+skips EF's write and takes responsibility for the save. This one then records nothing for it, and
+opens no transaction. An interceptor that suppresses a save and replays it through `SaveChanges`
+— as DotNetBoost.MultiLingual does under a retrying strategy — is recorded through the replay.
+
 ## `db.Update(entity)` has no before-image
 
 `Update(entity)` and `Entry(entity).State = Modified` on an entity the context never loaded give EF
@@ -251,6 +345,14 @@ A history of changes is not a history of states. Rebuilding "the product as it w
 a change list is only correct if the trail is complete from creation, which it never is for a table
 that existed before the library was installed. SQL Server temporal tables do this properly.
 
+## Every audited context maps the audit tables
+
+Entries are added to the context being saved, so a scope can hold several audited contexts —
+another instance of the registered type, or another type — and each writes its own trail in its
+own save. A context with `UseAuditing(sp)` whose model lacks the tables
+(`modelBuilder.ApplyAuditing()`) is refused on its first save, before anything is written. Explicit
+`IAuditManager` calls still go to the context registered with `UseEntityFrameworkCore<TContext>()`.
+
 ## Context pooling is not supported
 
 `AddDbContextPool` builds its options once, from the root provider, so they cannot carry the

docs/observability.md

@@ -55,8 +55,16 @@ are no attributes to read, so it reports what it knows. This is the only failure
 that cannot be undone, and one line at `Build()` prevents it:
 `DeclareEntitiesFrom(typeof(Product).Assembly)`.
 
+One report is an **error**: **atomicity lost**. The EF Core interceptor wrote deferred `Created`
+entries outside the transaction of the change they describe, because another `SaveChanges`
+interceptor committed it first (see
+[limitations.md](limitations.md#sharing-a-dbcontext-with-other-savechanges-interceptors)). It names
+the save's correlation id, which those entries carry. Under `OnLostAtomicity(LostAtomicity.Throw)`
+the save fails instead and nothing is reported.
+
 For anything other than logging — a counter of your own, a health check, an alert — implement
-`IAuditObserver` and register it with `.UseObserver<T>()`. Two methods, and the interface says what
+`IAuditObserver` and register it with `.UseObserver<T>()`. Three methods — `AtomicityLost` has a
+default that does nothing, so an observer written before it existed still compiles — and the interface says what
 an implementation may not do: **throw, or block.** Both are called on the recording path,
 synchronously, including from inside an EF Core `SaveChanges`, so an exception there fails the
 business write that was being audited.
@@ -73,6 +81,7 @@ builder.Services.AddOpenTelemetry()
 | `dotnetboost.auditing.entries.recorded` | `{entry}` | `audit.entity`, `audit.action` |
 | `dotnetboost.auditing.writes.suppressed` | `{write}` | `audit.entity`, `audit.reason`, `audit.action` |
 | `dotnetboost.auditing.entries.deleted` | `{entry}` | `audit.operation` — `prune` or `erase` |
+| `dotnetboost.auditing.atomicity.lost` | `{save}` | none. Should stay at zero; anything else is an interceptor composition to fix |
 
 **The first one is what you alert on, and it is the absence that matters.** "This entity recorded
 nothing for an hour" is the shape of every way a trail stops covering a table, and it is a question

src/DotNetBoost.Auditing.API/LoggingAuditObserver.cs

@@ -78,6 +78,17 @@ public void WriteSuppressed(AuditSuppression reason, string entityName, string e
     public void RecordedWithoutModelRules(string entityName, string entityId, AuditAction action) =>
         LogNoModelRules(_logger, action, entityName, entityId);
 
+    /// <inheritdoc/>
+    public void AtomicityLost(string correlationId, int entries) =>
+        LogAtomicityLost(_logger, entries, correlationId);
+
+    [LoggerMessage(EventId = 7107, Level = LogLevel.Error,
+        Message = "Wrote {Entries} Created entries outside the transaction of the change they describe " +
+                  "(correlation {CorrelationId}): another SaveChanges interceptor committed it first. Open the " +
+                  "transaction yourself around SaveChanges, or assign these entities' keys in code; see " +
+                  "docs/limitations.md.")]
+    private static partial void LogAtomicityLost(ILogger logger, int entries, string correlationId);
+
     [LoggerMessage(EventId = 7100, Level = LogLevel.Debug,
         Message = "Not recording a {Action} of '{EntityName}' ({EntityId}): recording is disabled.")]
     private static partial void LogRecordingDisabled(ILogger logger, AuditAction action, string entityName, string entityId);

src/DotNetBoost.Auditing.API/PublicApi.txt

@@ -159,6 +159,7 @@ namespace DotNetBoost.Auditing.API
     public sealed class LoggingAuditObserver : DotNetBoost.Auditing.Core.Interfaces.IAuditObserver
     {
         public LoggingAuditObserver(Microsoft.Extensions.Logging.ILogger<DotNetBoost.Auditing.API.LoggingAuditObserver> logger);
+        public void AtomicityLost(string correlationId, int entries);
         public void RecordedWithoutModelRules(string entityName, string entityId, DotNetBoost.Auditing.Core.Models.AuditAction action);
         public void WriteSuppressed(DotNetBoost.Auditing.Core.Models.AuditSuppression reason, string entityName, string entityId, DotNetBoost.Auditing.Core.Models.AuditAction action);
     }

src/DotNetBoost.Auditing.Core/AuditingOptions.cs

@@ -1,3 +1,4 @@
+using DotNetBoost.Auditing.Core.Models;
 using DotNetBoost.Auditing.Core.Metadata;
 
 namespace DotNetBoost.Auditing.Core;
@@ -75,6 +76,13 @@ public sealed class AuditingOptions
     /// </summary>
     public bool RecordUpdatesWithoutBeforeImage { get; set; } = true;
 
+    /// <summary>
+    /// What the EF Core capture does when another interceptor commits the transaction a save ran in
+    /// before the capture has written its deferred <c>Created</c> entries. <see cref="LostAtomicity.Report"/>
+    /// by default.
+    /// </summary>
+    public LostAtomicity OnLostAtomicity { get; set; } = LostAtomicity.Report;
+
     /// <summary>
     /// How long a recorded value may be before it is cut short and
     /// <see cref="Models.AuditPropertyChange.IsTruncated"/> is set. Default 4000.