Release trigger session on concurrency conflicts and cancellation

.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

src/EntityFrameworkCore.Triggered/Infrastructure/Internal/TriggersOptionExtension.cs

@@ -159,7 +159,9 @@ public void ApplyServices(IServiceCollection services)
 
             services.AddScoped<TriggerFactory>();
 
-            services.TryAddScoped<IInterceptor, TriggerSessionSaveChangesInterceptor>();
+            services.AddScoped<TriggerSessionSaveChangesInterceptor>();
+            services.AddScoped<IResettableService>(serviceProvider => serviceProvider.GetRequiredService<TriggerSessionSaveChangesInterceptor>());
+            services.TryAddScoped<IInterceptor>(serviceProvider => serviceProvider.GetRequiredService<TriggerSessionSaveChangesInterceptor>());
 
             services.Configure<TriggerOptions>(triggerServiceOptions => {
                 triggerServiceOptions.MaxCascadeCycles = _maxCascadeCycles;

src/EntityFrameworkCore.Triggered/Internal/TriggerInstanceFactory.cs

@@ -21,10 +21,12 @@ public sealed class TriggerInstanceFactory<TTriggerType> : 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)

src/EntityFrameworkCore.Triggered/Internal/TriggerSessionSaveChangesInterceptor.cs

@@ -9,14 +9,19 @@
 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;
 #endif
 
         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<int> 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<InterceptionResult<int>> 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,36 +179,144 @@ public async ValueTask<int> SavedChangesAsync(SaveChangesCompletedEventData even
             _triggerSession.RaiseAfterSaveCompletedTriggers();
             await _triggerSession.RaiseAfterSaveCompletedAsyncTriggers(cancellationToken).ConfigureAwait(false);
 
-            DelistTriggerSession(eventData);
+            DelistTriggerSession(eventData.Context);
 
             return result;
         }
 
         public void SaveChangesFailed(DbContextErrorEventData eventData)
         {
-            Debug.Assert(_triggerSession != null);
+            try
+            {
+                if (!AfterSaveFailedTriggersRaised)
+                {
+                    RaiseAfterSaveFailedTriggers(eventData.Exception);
+                }
+            }
+            finally
+            {
+                DelistTriggerSession(eventData.Context);
+            }
+        }
 
-            _triggerSession.RaiseAfterSaveFailedStartingTriggers(eventData.Exception);
-            _triggerSession.RaiseAfterSaveFailedTriggers(eventData.Exception);
-            _triggerSession.RaiseAfterSaveFailedCompletedTriggers(eventData.Exception);
+        public async Task SaveChangesFailedAsync(DbContextErrorEventData eventData, CancellationToken cancellationToken = default)
+        {
+            try
+            {
+                if (!AfterSaveFailedTriggersRaised)
+                {
+                    await RaiseAfterSaveFailedAsyncTriggers(eventData.Exception, cancellationToken).ConfigureAwait(false);
+                }
+            }
+            finally
+            {
+                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 && !AfterSaveFailedTriggersRaised)
+            {
+                try
+                {
+                    RaiseAfterSaveFailedTriggers(eventData.Exception);
+                }
+                finally
+                {
+                    _afterSaveFailedTriggersRaisedDepth = _parallelSaveChangesCount;
+                }
+            }
 
-            DelistTriggerSession(eventData);
+            return result;
         }
 
-        public async Task SaveChangesFailedAsync(DbContextErrorEventData eventData, CancellationToken cancellationToken = default)
+        public async ValueTask<InterceptionResult> ThrowingConcurrencyExceptionAsync(ConcurrencyExceptionEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default)
+        {
+            if (!result.IsSuppressed && !AfterSaveFailedTriggersRaised)
+            {
+                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.Context);
+
+        public Task SaveChangesCanceledAsync(DbContextEventData eventData, CancellationToken cancellationToken = default)
+        {
+            DelistTriggerSession(eventData.Context);
+            return Task.CompletedTask;
+        }
+
+        public void ResetState()
+        {
+            _triggerSession?.Dispose();
+            _triggerSession = null;
+            _parallelSaveChangesCount = 0;
+            _afterSaveFailedTriggersRaisedDepth = 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);
+
+            _triggerSession.RaiseAfterSaveFailedTriggers(exception);
+            await _triggerSession.RaiseAfterSaveFailedAsyncTriggers(exception, cancellationToken).ConfigureAwait(false);
 
-            DelistTriggerSession(eventData);
+            _triggerSession.RaiseAfterSaveFailedCompletedTriggers(exception);
+            await _triggerSession.RaiseAfterSaveFailedCompletedAsyncTriggers(exception, cancellationToken).ConfigureAwait(false);
         }
     }
 #pragma warning restore CS0618 // Type or member is obsolete

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<TestModel>();
+            var subject = new ServiceCollection();
+            subject.AddTriggeredDbContextPool<TestDbContext>(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<TestDbContext>();
+                context.TestModels.Add(new TestModel());
+                context.SaveChanges();
+            }
+
+            Assert.Equal(3, triggerStub.BeforeSaveInvocations.Count);
+        }
+
         [Fact]
         public void AddTriggeredDbContextPool_SupportsAScopedLifetime()
         {