From c74c0e617b17aa5268c0ef01c3eec19b7e03de52 Mon Sep 17 00:00:00 2001 From: Koen <2912652+koenbeuk@users.noreply.github.com> Date: Sun, 4 Oct 2026 00:59:15 +0100 Subject: [PATCH 1/3] release trigger session on every SaveChanges exit --- .../Internal/TriggersOptionExtension.cs | 4 +- .../TriggerSessionSaveChangesInterceptor.cs | 95 ++++++-- ...ssionSaveChangesInterceptorReleaseTests.cs | 228 ++++++++++++++++++ 3 files changed, 311 insertions(+), 16 deletions(-) create mode 100644 test/EntityFrameworkCore.Triggered.Tests/Internal/TriggerSessionSaveChangesInterceptorReleaseTests.cs diff --git a/src/EntityFrameworkCore.Triggered/Infrastructure/Internal/TriggersOptionExtension.cs b/src/EntityFrameworkCore.Triggered/Infrastructure/Internal/TriggersOptionExtension.cs index 3c24b89..4351d1e 100644 --- a/src/EntityFrameworkCore.Triggered/Infrastructure/Internal/TriggersOptionExtension.cs +++ b/src/EntityFrameworkCore.Triggered/Infrastructure/Internal/TriggersOptionExtension.cs @@ -159,7 +159,9 @@ public void ApplyServices(IServiceCollection services) services.AddScoped(); - services.TryAddScoped(); + services.AddScoped(); + services.AddScoped(serviceProvider => serviceProvider.GetRequiredService()); + services.TryAddScoped(serviceProvider => serviceProvider.GetRequiredService()); services.Configure(triggerServiceOptions => { triggerServiceOptions.MaxCascadeCycles = _maxCascadeCycles; diff --git a/src/EntityFrameworkCore.Triggered/Internal/TriggerSessionSaveChangesInterceptor.cs b/src/EntityFrameworkCore.Triggered/Internal/TriggerSessionSaveChangesInterceptor.cs index 3b2e446..2a4de65 100644 --- a/src/EntityFrameworkCore.Triggered/Internal/TriggerSessionSaveChangesInterceptor.cs +++ b/src/EntityFrameworkCore.Triggered/Internal/TriggerSessionSaveChangesInterceptor.cs @@ -9,7 +9,7 @@ namespace EntityFrameworkCore.Triggered.Internal { #pragma warning disable CS0618 // Type or member is obsolete (TriggeredDbContext with EFCore5) - public class TriggerSessionSaveChangesInterceptor : ISaveChangesInterceptor + public class TriggerSessionSaveChangesInterceptor : ISaveChangesInterceptor, IResettableService { #if DEBUG DbContext? _capturedDbContext; @@ -170,29 +170,94 @@ public async ValueTask SavedChangesAsync(SaveChangesCompletedEventData even public void SaveChangesFailed(DbContextErrorEventData eventData) { - Debug.Assert(_triggerSession != null); + try + { + RaiseAfterSaveFailedTriggers(eventData.Exception); + } + finally + { + DelistTriggerSession(eventData); + } + } + + public async Task SaveChangesFailedAsync(DbContextErrorEventData eventData, CancellationToken cancellationToken = default) + { + try + { + await RaiseAfterSaveFailedAsyncTriggers(eventData.Exception, cancellationToken).ConfigureAwait(false); + } + finally + { + DelistTriggerSession(eventData); + } + } - _triggerSession.RaiseAfterSaveFailedStartingTriggers(eventData.Exception); - _triggerSession.RaiseAfterSaveFailedTriggers(eventData.Exception); - _triggerSession.RaiseAfterSaveFailedCompletedTriggers(eventData.Exception); + public InterceptionResult ThrowingConcurrencyException(ConcurrencyExceptionEventData eventData, InterceptionResult result) + { + if (!result.IsSuppressed) + { + SaveChangesFailed(eventData); + } + + return result; + } + public async ValueTask ThrowingConcurrencyExceptionAsync(ConcurrencyExceptionEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + { + if (!result.IsSuppressed) + { + await SaveChangesFailedAsync(eventData, cancellationToken).ConfigureAwait(false); + } + + return result; + } + + public void SaveChangesCanceled(DbContextEventData eventData) + => DelistTriggerSession(eventData); + + public Task SaveChangesCanceledAsync(DbContextEventData eventData, CancellationToken cancellationToken = default) + { DelistTriggerSession(eventData); + return Task.CompletedTask; } - public async Task SaveChangesFailedAsync(DbContextErrorEventData eventData, CancellationToken cancellationToken = default) + public void ResetState() + { + _triggerSession?.Dispose(); + _triggerSession = null; + _parallelSaveChangesCount = 0; +#if DEBUG + _capturedDbContext = null; +#endif + } + + public Task ResetStateAsync(CancellationToken cancellationToken = default) + { + ResetState(); + return Task.CompletedTask; + } + + private void RaiseAfterSaveFailedTriggers(Exception exception) + { + Debug.Assert(_triggerSession != null); + + _triggerSession.RaiseAfterSaveFailedStartingTriggers(exception); + _triggerSession.RaiseAfterSaveFailedTriggers(exception); + _triggerSession.RaiseAfterSaveFailedCompletedTriggers(exception); + } + + private async Task RaiseAfterSaveFailedAsyncTriggers(Exception exception, CancellationToken cancellationToken) { Debug.Assert(_triggerSession != null); - _triggerSession.RaiseAfterSaveFailedStartingTriggers(eventData.Exception); - await _triggerSession.RaiseAfterSaveFailedStartingAsyncTriggers(eventData.Exception, cancellationToken).ConfigureAwait(false); - - _triggerSession.RaiseAfterSaveFailedTriggers(eventData.Exception); - await _triggerSession.RaiseAfterSaveFailedAsyncTriggers(eventData.Exception, cancellationToken).ConfigureAwait(false); - - _triggerSession.RaiseAfterSaveFailedCompletedTriggers(eventData.Exception); - await _triggerSession.RaiseAfterSaveFailedCompletedAsyncTriggers(eventData.Exception, cancellationToken).ConfigureAwait(false); + _triggerSession.RaiseAfterSaveFailedStartingTriggers(exception); + await _triggerSession.RaiseAfterSaveFailedStartingAsyncTriggers(exception, cancellationToken).ConfigureAwait(false); - DelistTriggerSession(eventData); + _triggerSession.RaiseAfterSaveFailedTriggers(exception); + await _triggerSession.RaiseAfterSaveFailedAsyncTriggers(exception, cancellationToken).ConfigureAwait(false); + + _triggerSession.RaiseAfterSaveFailedCompletedTriggers(exception); + await _triggerSession.RaiseAfterSaveFailedCompletedAsyncTriggers(exception, cancellationToken).ConfigureAwait(false); } } #pragma warning restore CS0618 // Type or member is obsolete diff --git a/test/EntityFrameworkCore.Triggered.Tests/Internal/TriggerSessionSaveChangesInterceptorReleaseTests.cs b/test/EntityFrameworkCore.Triggered.Tests/Internal/TriggerSessionSaveChangesInterceptorReleaseTests.cs new file mode 100644 index 0000000..0fc49c7 --- /dev/null +++ b/test/EntityFrameworkCore.Triggered.Tests/Internal/TriggerSessionSaveChangesInterceptorReleaseTests.cs @@ -0,0 +1,228 @@ +using System; +using System.Data.Common; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using EntityFrameworkCore.Triggered.Tests.Stubs; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace EntityFrameworkCore.Triggered.Tests.Internal +{ + /// + /// Every way a SaveChanges call can end has to release the trigger session, or the next save on the same context reuses a stale one (#219). + /// + public class TriggerSessionSaveChangesInterceptorReleaseTests : IDisposable + { + public class Item + { + public Guid Id { get; set; } + public string Name { get; set; } + public int Version { get; set; } + } + + public class ItemDbContext : DbContext + { + public ItemDbContext(DbContextOptions options) : base(options) { } + + public DbSet Items { get; set; } + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity().Property(x => x.Version).IsConcurrencyToken(); + } + } + + class CancelingCommandInterceptor : DbCommandInterceptor + { + public bool Enabled { get; set; } + + public override InterceptionResult ReaderExecuting(DbCommand command, CommandEventData eventData, InterceptionResult result) + => Enabled ? throw new OperationCanceledException() : result; + + public override ValueTask> ReaderExecutingAsync(DbCommand command, CommandEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + => Enabled ? throw new OperationCanceledException() : new ValueTask>(result); + } + + readonly SqliteConnection _connection = new("DataSource=:memory:"); + readonly CancelingCommandInterceptor _cancelingInterceptor = new(); + readonly TriggerStub _trigger = new(); + + public TriggerSessionSaveChangesInterceptorReleaseTests() + { + _connection.Open(); + } + + public void Dispose() => _connection.Dispose(); + + void ConfigureOptions(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder + .UseSqlite(_connection) + .AddInterceptors(_cancelingInterceptor) + .ConfigureWarnings(warnings => warnings.Ignore(CoreEventId.ManyServiceProvidersCreatedWarning)) + .UseTriggers(triggerOptions => triggerOptions.AddTrigger(_trigger)); + + ItemDbContext CreateContext() + { + var optionsBuilder = new DbContextOptionsBuilder(); + ConfigureOptions(optionsBuilder); + + var context = new ItemDbContext(optionsBuilder.Options); + context.Database.EnsureCreated(); + return context; + } + + static Task Save(DbContext context, bool async) + { + if (async) + { + return context.SaveChangesAsync(); + } + + context.SaveChanges(); + return Task.CompletedTask; + } + + static Item SeedItem(ItemDbContext context) + { + var item = new Item { Id = Guid.NewGuid(), Name = "seed" }; + context.Items.Add(item); + context.SaveChanges(); + return item; + } + + static void CauseConcurrencyConflict(ItemDbContext context, Item item) + { + context.Database.ExecuteSqlRaw("UPDATE Items SET Version = Version + 1"); + item.Name = "conflicting"; + } + + async Task AssertNextSaveRaisesBeforeSaveTriggers(ItemDbContext context, bool async) + { + Assert.Null(context.GetService().Current); + + foreach (var entry in context.ChangeTracker.Entries().ToList()) + { + entry.State = EntityState.Detached; + } + + _trigger.BeforeSaveInvocations.Clear(); + _trigger.BeforeSaveAsyncInvocations.Clear(); + + var item = new Item { Id = Guid.NewGuid(), Name = "next" }; + context.Items.Add(item); + await Save(context, async); + + Assert.Contains(_trigger.BeforeSaveInvocations, invocation => invocation.Entity == item); + if (async) + { + Assert.Contains(_trigger.BeforeSaveAsyncInvocations, invocation => invocation.Entity == item); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ConcurrencyException_ReleasesTriggerSession(bool async) + { + using var context = CreateContext(); + CauseConcurrencyConflict(context, SeedItem(context)); + + await Assert.ThrowsAsync(() => Save(context, async)); + + await AssertNextSaveRaisesBeforeSaveTriggers(context, async); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ConcurrencyException_RaisesAfterSaveFailedTriggers(bool async) + { + using var context = CreateContext(); + var item = SeedItem(context); + CauseConcurrencyConflict(context, item); + + await Assert.ThrowsAsync(() => Save(context, async)); + + var invocation = Assert.Single(_trigger.AfterSaveFailedInvocations); + Assert.Same(item, invocation.context.Entity); + Assert.IsType(invocation.exception); + Assert.Equal(async ? 1 : 0, _trigger.AfterSaveFailedAsyncInvocations.Count); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Cancellation_ReleasesTriggerSession(bool async) + { + using var context = CreateContext(); + SeedItem(context).Name = "changed"; + _cancelingInterceptor.Enabled = true; + + await Assert.ThrowsAnyAsync(() => Save(context, async)); + + _cancelingInterceptor.Enabled = false; + await AssertNextSaveRaisesBeforeSaveTriggers(context, async); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ThrowingAfterSaveFailedTrigger_ReleasesTriggerSession(bool async) + { + using var context = CreateContext(); + var item = SeedItem(context); + context.Entry(item).State = EntityState.Detached; + context.Items.Add(new Item { Id = item.Id, Name = "duplicate" }); + _trigger.AfterSaveFailedHandler = (_, _) => throw new InvalidOperationException("trigger failed"); + + var exception = await Assert.ThrowsAsync(() => Save(context, async)); + Assert.Equal("trigger failed", exception.Message); + + _trigger.AfterSaveFailedHandler = null; + await AssertNextSaveRaisesBeforeSaveTriggers(context, async); + } + + [Fact] + public async Task PooledContext_AfterConcurrencyException_NextLeaseRaisesAsyncBeforeSaveTriggers() + { + using var serviceProvider = new ServiceCollection() + .AddTriggeredDbContextPool(ConfigureOptions, poolSize: 1) + .BuildServiceProvider(); + + Guid seededId; + using (var scope = serviceProvider.CreateScope()) + { + var context = scope.ServiceProvider.GetRequiredService(); + context.Database.EnsureCreated(); + seededId = SeedItem(context).Id; + } + + ItemDbContext pooledInstance; + using (var scope = serviceProvider.CreateScope()) + { + pooledInstance = scope.ServiceProvider.GetRequiredService(); + CauseConcurrencyConflict(pooledInstance, await pooledInstance.Items.SingleAsync(x => x.Id == seededId)); + + await Assert.ThrowsAsync(() => pooledInstance.SaveChangesAsync()); + } + + using (var scope = serviceProvider.CreateScope()) + { + var context = scope.ServiceProvider.GetRequiredService(); + Assert.Same(pooledInstance, context); + _trigger.BeforeSaveAsyncInvocations.Clear(); + + var item = new Item { Id = Guid.NewGuid(), Name = "next lease" }; + context.Items.Add(item); + await context.SaveChangesAsync(); + + Assert.Contains(_trigger.BeforeSaveAsyncInvocations, invocation => invocation.Entity == item); + } + } + } +} From 946bb269980baa89ffa4d31d0e031450a99357e4 Mon Sep 17 00:00:00 2001 From: Koen <2912652+koenbeuk@users.noreply.github.com> Date: Sun, 4 Oct 2026 00:59:25 +0100 Subject: [PATCH 2/3] keep registered trigger instances across pooled context leases --- .../Internal/TriggerInstanceFactory.cs | 4 ++- .../ServiceCollectionExtensionsTests.cs | 26 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/EntityFrameworkCore.Triggered/Internal/TriggerInstanceFactory.cs b/src/EntityFrameworkCore.Triggered/Internal/TriggerInstanceFactory.cs index 53c4c36..ba27c43 100644 --- a/src/EntityFrameworkCore.Triggered/Internal/TriggerInstanceFactory.cs +++ b/src/EntityFrameworkCore.Triggered/Internal/TriggerInstanceFactory.cs @@ -21,10 +21,12 @@ public sealed class TriggerInstanceFactory : ITriggerInstanceFacto { static ObjectFactory? _internalFactory; + readonly object? _capturedInstance; object? _instance; public TriggerInstanceFactory(object? instance) { + _capturedInstance = instance; _instance = instance; } @@ -46,7 +48,7 @@ public object Create(IServiceProvider serviceProvider) public void ResetState() { - _instance = null; + _instance = _capturedInstance; } public Task ResetStateAsync(CancellationToken cancellationToken = default) diff --git a/test/EntityFrameworkCore.Triggered.Tests/Infrastructure/ServiceCollectionExtensionsTests.cs b/test/EntityFrameworkCore.Triggered.Tests/Infrastructure/ServiceCollectionExtensionsTests.cs index ca6b729..0847080 100644 --- a/test/EntityFrameworkCore.Triggered.Tests/Infrastructure/ServiceCollectionExtensionsTests.cs +++ b/test/EntityFrameworkCore.Triggered.Tests/Infrastructure/ServiceCollectionExtensionsTests.cs @@ -122,6 +122,32 @@ public void AddTriggeredDbContextPool_ReusesScopedServiceProvider() Assert.Single(triggerStub.BeforeSaveInvocations); } + [Fact] + public void AddTriggeredDbContextPool_KeepsRegisteredTriggerInstanceAcrossLeases() + { + var triggerStub = new TriggerStub(); + var subject = new ServiceCollection(); + subject.AddTriggeredDbContextPool(options => { + options.UseInMemoryDatabase(nameof(AddTriggeredDbContextPool_KeepsRegisteredTriggerInstanceAcrossLeases)); + options.ConfigureWarnings(warningOptions => { + warningOptions.Ignore(CoreEventId.ManyServiceProvidersCreatedWarning); + }); + options.UseTriggers(triggerOptions => triggerOptions.AddTrigger(triggerStub)); + }, poolSize: 1); + + using var serviceProvider = subject.BuildServiceProvider(); + + for (var lease = 0; lease < 3; lease++) + { + using var scope = serviceProvider.CreateScope(); + var context = scope.ServiceProvider.GetRequiredService(); + context.TestModels.Add(new TestModel()); + context.SaveChanges(); + } + + Assert.Equal(3, triggerStub.BeforeSaveInvocations.Count); + } + [Fact] public void AddTriggeredDbContextPool_SupportsAScopedLifetime() { From acfe86472740343ce73c046e12dcf1166e9c5764 Mon Sep 17 00:00:00 2001 From: Koen <2912652+koenbeuk@users.noreply.github.com> Date: Sun, 4 Oct 2026 01:22:14 +0100 Subject: [PATCH 3/3] handle recursive failures and retries --- .github/workflows/release.yml | 12 +- .../TriggerSessionSaveChangesInterceptor.cs | 91 +++++++++++--- ...ssionSaveChangesInterceptorReleaseTests.cs | 112 +++++++++++++++++- 3 files changed, 196 insertions(+), 19 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 19e558a..b7bf813 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -14,11 +14,14 @@ env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Official NuGet Feed settings NUGET_FEED: https://api.nuget.org/v3/index.json - NUGET_KEY: ${{ secrets.NUGET_KEY }} jobs: deploy: runs-on: ubuntu-latest + permissions: + contents: read + packages: write + id-token: write steps: - uses: actions/checkout@v4 - name: Setup .NET Core @@ -39,9 +42,14 @@ jobs: do dotnet nuget push $f --source $GITHUB_FEED --api-key ${{github.token}} --skip-duplicate --no-symbols done + - name: NuGet login + uses: NuGet/login@v1 + id: nuget-login + with: + user: Koenbeuk - name: Push to NuGet Feed run: | for f in ./nupkg/*.nupkg do - dotnet nuget push $f --source $NUGET_FEED --skip-duplicate --api-key $NUGET_KEY + dotnet nuget push $f --source $NUGET_FEED --skip-duplicate --api-key ${{ steps.nuget-login.outputs.NUGET_API_KEY }} done \ No newline at end of file diff --git a/src/EntityFrameworkCore.Triggered/Internal/TriggerSessionSaveChangesInterceptor.cs b/src/EntityFrameworkCore.Triggered/Internal/TriggerSessionSaveChangesInterceptor.cs index 2a4de65..0372ff5 100644 --- a/src/EntityFrameworkCore.Triggered/Internal/TriggerSessionSaveChangesInterceptor.cs +++ b/src/EntityFrameworkCore.Triggered/Internal/TriggerSessionSaveChangesInterceptor.cs @@ -17,6 +17,11 @@ public class TriggerSessionSaveChangesInterceptor : ISaveChangesInterceptor, IRe ITriggerSession? _triggerSession; int _parallelSaveChangesCount; + int _afterSaveFailedTriggersRaisedDepth; + bool _subscribedToSaveChangesFailed; + + // Triggers can call SaveChanges on the same context, so this tracks the nesting depth whose AfterSaveFailed triggers already ran + private bool AfterSaveFailedTriggersRaised => _parallelSaveChangesCount > 0 && _afterSaveFailedTriggersRaisedDepth == _parallelSaveChangesCount; private void EnlistTriggerSession(DbContextEventData eventData) { @@ -50,17 +55,28 @@ private void EnlistTriggerSession(DbContextEventData eventData) } } + if (!_subscribedToSaveChangesFailed) + { + eventData.Context!.SaveChangesFailed += OnSaveChangesFailed; + _subscribedToSaveChangesFailed = true; + } + _parallelSaveChangesCount += 1; } - private void DelistTriggerSession(DbContextEventData eventData) + private void DelistTriggerSession(DbContext? context) { Debug.Assert(_triggerSession != null); #if DEBUG - Debug.Assert(_capturedDbContext == eventData.Context); + Debug.Assert(_capturedDbContext == context); #endif + if (AfterSaveFailedTriggersRaised) + { + _afterSaveFailedTriggersRaisedDepth = 0; + } + _parallelSaveChangesCount -= 1; if (_parallelSaveChangesCount == 0) @@ -90,7 +106,7 @@ public InterceptionResult SavingChanges(DbContextEventData eventData, Inter catch { // We're aborting the SaveChanges call, delist the trigger session now - DelistTriggerSession(eventData); + DelistTriggerSession(eventData.Context); throw; } finally @@ -126,7 +142,7 @@ public async ValueTask> SavingChangesAsync(DbContextEven catch { // We're aborting the SaveChanges call, delist the trigger session now - DelistTriggerSession(eventData); + DelistTriggerSession(eventData.Context); throw; } finally @@ -145,7 +161,7 @@ public int SavedChanges(SaveChangesCompletedEventData eventData, int result) _triggerSession.RaiseAfterSaveTriggers(); _triggerSession.RaiseAfterSaveCompletedTriggers(); - DelistTriggerSession(eventData); + DelistTriggerSession(eventData.Context); return result; } @@ -163,7 +179,7 @@ public async ValueTask SavedChangesAsync(SaveChangesCompletedEventData even _triggerSession.RaiseAfterSaveCompletedTriggers(); await _triggerSession.RaiseAfterSaveCompletedAsyncTriggers(cancellationToken).ConfigureAwait(false); - DelistTriggerSession(eventData); + DelistTriggerSession(eventData.Context); return result; } @@ -172,11 +188,14 @@ public void SaveChangesFailed(DbContextErrorEventData eventData) { try { - RaiseAfterSaveFailedTriggers(eventData.Exception); + if (!AfterSaveFailedTriggersRaised) + { + RaiseAfterSaveFailedTriggers(eventData.Exception); + } } finally { - DelistTriggerSession(eventData); + DelistTriggerSession(eventData.Context); } } @@ -184,19 +203,31 @@ public async Task SaveChangesFailedAsync(DbContextErrorEventData eventData, Canc { try { - await RaiseAfterSaveFailedAsyncTriggers(eventData.Exception, cancellationToken).ConfigureAwait(false); + if (!AfterSaveFailedTriggersRaised) + { + await RaiseAfterSaveFailedAsyncTriggers(eventData.Exception, cancellationToken).ConfigureAwait(false); + } } finally { - DelistTriggerSession(eventData); + DelistTriggerSession(eventData.Context); } } + // An interceptor registered after us can still suppress the exception, so the session is only released once SaveChanges + // reports its final outcome: SavedChanges, SaveChangesFailed, SaveChangesCanceled or the DbContext.SaveChangesFailed event. public InterceptionResult ThrowingConcurrencyException(ConcurrencyExceptionEventData eventData, InterceptionResult result) { - if (!result.IsSuppressed) + if (!result.IsSuppressed && !AfterSaveFailedTriggersRaised) { - SaveChangesFailed(eventData); + try + { + RaiseAfterSaveFailedTriggers(eventData.Exception); + } + finally + { + _afterSaveFailedTriggersRaisedDepth = _parallelSaveChangesCount; + } } return result; @@ -204,20 +235,47 @@ public InterceptionResult ThrowingConcurrencyException(ConcurrencyExceptionEvent public async ValueTask ThrowingConcurrencyExceptionAsync(ConcurrencyExceptionEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) { - if (!result.IsSuppressed) + if (!result.IsSuppressed && !AfterSaveFailedTriggersRaised) { - await SaveChangesFailedAsync(eventData, cancellationToken).ConfigureAwait(false); + try + { + await RaiseAfterSaveFailedAsyncTriggers(eventData.Exception, cancellationToken).ConfigureAwait(false); + } + finally + { + _afterSaveFailedTriggersRaisedDepth = _parallelSaveChangesCount; + } } return result; } + private void OnSaveChangesFailed(object? sender, SaveChangesFailedEventArgs eventArgs) + { + if (eventArgs.Exception is not DbUpdateConcurrencyException || _triggerSession is null) + { + return; + } + + try + { + if (!AfterSaveFailedTriggersRaised) + { + RaiseAfterSaveFailedTriggers(eventArgs.Exception); + } + } + finally + { + DelistTriggerSession(sender as DbContext); + } + } + public void SaveChangesCanceled(DbContextEventData eventData) - => DelistTriggerSession(eventData); + => DelistTriggerSession(eventData.Context); public Task SaveChangesCanceledAsync(DbContextEventData eventData, CancellationToken cancellationToken = default) { - DelistTriggerSession(eventData); + DelistTriggerSession(eventData.Context); return Task.CompletedTask; } @@ -226,6 +284,7 @@ public void ResetState() _triggerSession?.Dispose(); _triggerSession = null; _parallelSaveChangesCount = 0; + _afterSaveFailedTriggersRaisedDepth = 0; #if DEBUG _capturedDbContext = null; #endif diff --git a/test/EntityFrameworkCore.Triggered.Tests/Internal/TriggerSessionSaveChangesInterceptorReleaseTests.cs b/test/EntityFrameworkCore.Triggered.Tests/Internal/TriggerSessionSaveChangesInterceptorReleaseTests.cs index 0fc49c7..a65acb8 100644 --- a/test/EntityFrameworkCore.Triggered.Tests/Internal/TriggerSessionSaveChangesInterceptorReleaseTests.cs +++ b/test/EntityFrameworkCore.Triggered.Tests/Internal/TriggerSessionSaveChangesInterceptorReleaseTests.cs @@ -48,6 +48,15 @@ public override ValueTask> ReaderExecutingAsync => Enabled ? throw new OperationCanceledException() : new ValueTask>(result); } + class SuppressingConcurrencyInterceptor : ISaveChangesInterceptor + { + public InterceptionResult ThrowingConcurrencyException(ConcurrencyExceptionEventData eventData, InterceptionResult result) + => InterceptionResult.Suppress(); + + public ValueTask ThrowingConcurrencyExceptionAsync(ConcurrencyExceptionEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default) + => new(InterceptionResult.Suppress()); + } + readonly SqliteConnection _connection = new("DataSource=:memory:"); readonly CancelingCommandInterceptor _cancelingInterceptor = new(); readonly TriggerStub _trigger = new(); @@ -59,10 +68,14 @@ public TriggerSessionSaveChangesInterceptorReleaseTests() public void Dispose() => _connection.Dispose(); + bool _suppressConcurrencyExceptions; + void ConfigureOptions(DbContextOptionsBuilder optionsBuilder) => optionsBuilder .UseSqlite(_connection) - .AddInterceptors(_cancelingInterceptor) + .AddInterceptors(_suppressConcurrencyExceptions + ? new IInterceptor[] { _cancelingInterceptor, new SuppressingConcurrencyInterceptor() } + : new IInterceptor[] { _cancelingInterceptor }) .ConfigureWarnings(warnings => warnings.Ignore(CoreEventId.ManyServiceProvidersCreatedWarning)) .UseTriggers(triggerOptions => triggerOptions.AddTrigger(_trigger)); @@ -187,6 +200,103 @@ public async Task ThrowingAfterSaveFailedTrigger_ReleasesTriggerSession(bool asy await AssertNextSaveRaisesBeforeSaveTriggers(context, async); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ConcurrencyException_WithThrowingAfterSaveFailedTrigger_ReleasesTriggerSession(bool async) + { + using var context = CreateContext(); + CauseConcurrencyConflict(context, SeedItem(context)); + _trigger.AfterSaveFailedHandler = (_, _) => throw new InvalidOperationException("trigger failed"); + + var exception = await Assert.ThrowsAsync(() => Save(context, async)); + Assert.Equal("trigger failed", Assert.IsType(exception.InnerException).Message); + Assert.Single(_trigger.AfterSaveFailedInvocations); + + _trigger.AfterSaveFailedHandler = null; + await AssertNextSaveRaisesBeforeSaveTriggers(context, async); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ConcurrencyException_SuppressedByLaterInterceptor_ReleasesTriggerSession(bool async) + { + _suppressConcurrencyExceptions = true; + using var context = CreateContext(); + CauseConcurrencyConflict(context, SeedItem(context)); + + await Save(context, async); + + await AssertNextSaveRaisesBeforeSaveTriggers(context, async); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ConcurrencyException_SuppressedForMultipleCommands_RaisesAfterSaveFailedTriggersOnce(bool async) + { + _suppressConcurrencyExceptions = true; + using var context = CreateContext(); + var first = SeedItem(context); + var second = SeedItem(context); + CauseConcurrencyConflict(context, first); + second.Name = "conflicting"; + + await Save(context, async); + + Assert.Equal(2, _trigger.AfterSaveFailedInvocations.Count); + Assert.Contains(_trigger.AfterSaveFailedInvocations, invocation => invocation.context.Entity == first); + Assert.Contains(_trigger.AfterSaveFailedInvocations, invocation => invocation.context.Entity == second); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ConcurrencyException_Suppressed_NestedSaveFailureRaisesItsOwnAfterSaveFailedTriggers(bool async) + { + _suppressConcurrencyExceptions = true; + using var context = CreateContext(); + var seeded = SeedItem(context); + CauseConcurrencyConflict(context, seeded); + + var duplicate = new Item { Id = seeded.Id, Name = "duplicate" }; + var nestedSaveAttempted = false; + _trigger.AfterSaveHandler = _ => { + if (nestedSaveAttempted) + { + return; + } + + nestedSaveAttempted = true; + context.Entry(seeded).State = EntityState.Detached; + context.Items.Add(duplicate); + Assert.Throws(() => context.SaveChanges()); + }; + + await Save(context, async); + + Assert.True(nestedSaveAttempted); + Assert.Contains(_trigger.AfterSaveFailedInvocations, invocation => invocation.context.Entity == duplicate); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task ConcurrencyExceptionThrownByAfterSaveTrigger_ReleasesTriggerSession(bool async) + { + using var context = CreateContext(); + var item = SeedItem(context); + item.Name = "changed"; + _trigger.AfterSaveHandler = _ => throw new DbUpdateConcurrencyException("from trigger"); + + await Assert.ThrowsAsync(() => Save(context, async)); + Assert.Contains(_trigger.AfterSaveFailedInvocations, invocation => invocation.context.Entity == item); + + _trigger.AfterSaveHandler = null; + await AssertNextSaveRaisesBeforeSaveTriggers(context, async); + } + [Fact] public async Task PooledContext_AfterConcurrencyException_NextLeaseRaisesAsyncBeforeSaveTriggers() {