Merge pull request #19 from koenbeuk/feature/before-save-changes-multiple-calls

Feature/before save changes multiple calls

Author: koenbeuk

src/EntityFrameworkCore.Triggered.Abstractions/ITriggerSession.cs

@@ -22,6 +22,11 @@ public interface ITriggerSession
         /// </summary>
         Task RaiseBeforeSaveTriggers(CancellationToken cancellationToken = default);
         /// <summary>
+        /// Makes a snapshot of all changes in the DbContext and invokes BeforeSaveTriggers recursively based on the recursive settings until all changes have been processed
+        /// </summary>
+        /// <param name="skipDetectedChanges">Allows BeforeSaveTriggers not to include previously detected changes. Only new changes will be detected and fired upon. This is useful in case of multiple calls to RaiseBeforeSaveTriggers</param>
+        Task RaiseBeforeSaveTriggers(bool skipDetectedChanges, CancellationToken cancellationToken = default);
+        /// <summary>
         /// Invokes AfterSaveTriggers non-recursively. Calling this method expects that either RaiseBeforeSaveTriggers() or DiscoverChanges() has called
         /// </summary>
         /// <returns></returns>

src/EntityFrameworkCore.Triggered/Internal/RecursiveTriggerContextDiscoveryStrategy.cs

@@ -23,10 +23,12 @@ public class RecursiveTriggerContextDiscoveryStrategy : ITriggerContextDiscovery
             "Discovered changes: {changes} for {name}. Iteration ({iteration}/{maxRecursion})");
 
         readonly string _name;
+        readonly bool _skipDetectedChanges;
 
-        public RecursiveTriggerContextDiscoveryStrategy(string name)
+        public RecursiveTriggerContextDiscoveryStrategy(string name, bool skipDetectedChanges)
         {
             _name = name ?? throw new ArgumentNullException(nameof(name));
+            _skipDetectedChanges = skipDetectedChanges;
         }
 
         public IEnumerable<ITriggerContextDescriptor> Discover(TriggerOptions options, TriggerContextTracker tracker, ILogger logger)
@@ -45,7 +47,7 @@ public IEnumerable<ITriggerContextDescriptor> Discover(TriggerOptions options, T
                 IEnumerable<ITriggerContextDescriptor> changes = tracker.DiscoverChanges().ToList();
 
                 // In case someone made a call to TriggerSession.DetectChanges, prior to calling RaiseBeforeSaveTriggers, we want to make sure that we include that discovery result in the first iteration
-                if (iteration == 0)
+                if (iteration == 0 && !_skipDetectedChanges)
                 {
                     changes = tracker.DiscoveredChanges.ToList();
                 }

src/EntityFrameworkCore.Triggered/TriggerSession.cs

@@ -15,13 +15,16 @@ namespace EntityFrameworkCore.Triggered
     public class TriggerSession : ITriggerSession
     {
         static ITriggerContextDiscoveryStrategy? _beforeSaveTriggerContextDiscoveryStrategy;
+        static ITriggerContextDiscoveryStrategy? _beforeSaveTriggerContextDiscoveryStrategyWithSkipDetectedChanges; // To satisfy RaiseBeforeSaveTrigger's overload
         static ITriggerContextDiscoveryStrategy? _afterSaveTriggerContextDiscoveryStrategy;
 
         readonly TriggerOptions _options;
         readonly ITriggerDiscoveryService _triggerDiscoveryService;
         readonly TriggerContextTracker _tracker;
         readonly ILogger<TriggerSession> _logger;
 
+        bool _raiseBeforeSaveTriggersCalled;
+
         public TriggerSession(TriggerOptions options, ITriggerDiscoveryService triggerDiscoveryService, TriggerContextTracker tracker, ILogger<TriggerSession> logger)
         {
             _options = options ?? throw new ArgumentNullException(nameof(options));
@@ -62,15 +65,36 @@ public async Task RaiseTriggers(Type openTriggerType, ITriggerContextDiscoverySt
             }
         }
 
-
         public Task RaiseBeforeSaveTriggers(CancellationToken cancellationToken)
+            => RaiseBeforeSaveTriggers(_raiseBeforeSaveTriggersCalled, cancellationToken);
+
+        public Task RaiseBeforeSaveTriggers(bool skipDetectedChanges, CancellationToken cancellationToken)
         {
-            if (_beforeSaveTriggerContextDiscoveryStrategy == null)
+            _raiseBeforeSaveTriggersCalled = true;
+
+            ITriggerContextDiscoveryStrategy? strategy;
+
+            if (skipDetectedChanges)
             {
-                _beforeSaveTriggerContextDiscoveryStrategy = new RecursiveTriggerContextDiscoveryStrategy("BeforeSave");
+                if (_beforeSaveTriggerContextDiscoveryStrategyWithSkipDetectedChanges == null)
+                {
+                    _beforeSaveTriggerContextDiscoveryStrategyWithSkipDetectedChanges = new RecursiveTriggerContextDiscoveryStrategy("BeforeSave", true);
+                }
+
+                strategy = _beforeSaveTriggerContextDiscoveryStrategyWithSkipDetectedChanges;
+            }
+            else
+            {
+                if (_beforeSaveTriggerContextDiscoveryStrategy == null)
+                {
+                    _beforeSaveTriggerContextDiscoveryStrategy = new RecursiveTriggerContextDiscoveryStrategy("BeforeSave", false);
+                }
+
+                strategy = _beforeSaveTriggerContextDiscoveryStrategy;
             }
 
-            return RaiseTriggers(typeof(IBeforeSaveTrigger<>), _beforeSaveTriggerContextDiscoveryStrategy, entityType => new BeforeSaveTriggerDescriptor(entityType), cancellationToken);  
+            _raiseBeforeSaveTriggersCalled = true;
+            return RaiseTriggers(typeof(IBeforeSaveTrigger<>), strategy, entityType => new BeforeSaveTriggerDescriptor(entityType), cancellationToken);  
         }
 
         public Task RaiseAfterSaveTriggers(CancellationToken cancellationToken = default)

src/EntityFrameworkCore.Triggered/TriggeredDbContext.cs

@@ -10,6 +10,8 @@ namespace EntityFrameworkCore.Triggered
 {
     public abstract class TriggeredDbContext : DbContext
     {
+        private ITriggerSession? _triggerSession;
+
         protected TriggeredDbContext()
             : this(new DbContextOptions<DbContext>())
         {
@@ -32,28 +34,53 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
 
         public override int SaveChanges(bool acceptAllChangesOnSuccess)
         {
-            var triggerService = this.GetService<ITriggerService>() ?? throw new InvalidOperationException("Triggers are not configured");
+            if (_triggerSession == null)
+            {
+                var triggerService = this.GetService<ITriggerService>() ?? throw new InvalidOperationException("Triggers are not configured");
+                _triggerSession = triggerService.CreateSession(this);
+            }
 
-            var triggerSession = triggerService.CreateSession(this);
+            try
+            {
 
-            triggerSession.RaiseBeforeSaveTriggers(default).GetAwaiter().GetResult();
-            var result = base.SaveChanges(acceptAllChangesOnSuccess);
-            triggerSession.RaiseAfterSaveTriggers(default).GetAwaiter().GetResult();
+                _triggerSession.RaiseBeforeSaveTriggers(default).GetAwaiter().GetResult();
+                var result = base.SaveChanges(acceptAllChangesOnSuccess);
+                _triggerSession.RaiseAfterSaveTriggers(default).GetAwaiter().GetResult();
 
-            return result;
+                return result;
+            }
+            finally
+            {
+                _triggerSession = null;
+            }
         }
 
         public override async Task<int> SaveChangesAsync(bool acceptAllChangesOnSuccess, CancellationToken cancellationToken = default)
         {
-            var triggerService = this.GetService<ITriggerService>() ?? throw new InvalidOperationException("Triggers are not configured");
+            bool createdTriggerSession = false;
 
-            var triggerSession = triggerService.CreateSession(this);
+            if (_triggerSession == null)
+            {
+                var triggerService = this.GetService<ITriggerService>() ?? throw new InvalidOperationException("Triggers are not configured");
+                _triggerSession = triggerService.CreateSession(this);
+                createdTriggerSession = true;
+            }
+            try
+            {
 
-            await triggerSession.RaiseBeforeSaveTriggers(cancellationToken).ConfigureAwait(false);
-            var result = base.SaveChanges(acceptAllChangesOnSuccess);
-            await triggerSession.RaiseAfterSaveTriggers(cancellationToken).ConfigureAwait(false);
+                await _triggerSession.RaiseBeforeSaveTriggers(cancellationToken).ConfigureAwait(false);
+                var result = base.SaveChanges(acceptAllChangesOnSuccess);
+                await _triggerSession.RaiseAfterSaveTriggers(cancellationToken).ConfigureAwait(false);
 
-            return result;
+                return result;
+            }
+            finally
+            {
+                if (createdTriggerSession)
+                {
+                    _triggerSession = null;
+                }
+            }
         }
     }
 }

test/EntityFrameworkCore.Triggered.Tests/Internal/RecursiveTriggerContextDiscoveryStrategyTests.cs

@@ -39,7 +39,7 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
         public void DiscoverChanges_MultipleCalls_ReturnsDeltaOfChanges()
         {
             using var dbContext = new TestDbContext();
-            var subject = new RecursiveTriggerContextDiscoveryStrategy("test");
+            var subject = new RecursiveTriggerContextDiscoveryStrategy("test", false);
             var triggerContextTracker = new TriggerContextTracker(dbContext.ChangeTracker, new EntityAndTypeRecursionStrategy());
             triggerContextTracker.DiscoverChanges().Count();
             var initialContextDescriptors = subject.Discover(new TriggerOptions { }, triggerContextTracker, new NullLogger<object>()).ToList();

test/EntityFrameworkCore.Triggered.Tests/Stubs/TriggerSessionStub.cs

@@ -30,5 +30,11 @@ public Task RaiseBeforeSaveTriggers(CancellationToken cancellationToken = defaul
             RaiseBeforeSaveTriggersCalls += 1;
             return Task.CompletedTask;
         }
+
+        public Task RaiseBeforeSaveTriggers(bool skipDetectedChanges = false, CancellationToken cancellationToken = default)
+        {
+            RaiseBeforeSaveTriggersCalls += 1;
+            return Task.CompletedTask;
+        }
     }
 }

test/EntityFrameworkCore.Triggered.Tests/Stubs/TriggerStub.cs

@@ -15,19 +15,26 @@ public class TriggerStub<TEntity> : IBeforeSaveTrigger<TEntity>, IAfterSaveTrigg
         public ICollection<ITriggerContext<TEntity>> AfterSaveInvocations { get; } = new List<ITriggerContext<TEntity>>();
         public int Priority { get; set; }
 
+        public Func<ITriggerContext<TEntity>, CancellationToken, Task> BeforeSaveHandler { get; set; }
+        public Func<ITriggerContext<TEntity>, CancellationToken, Task> AfterSaveHandler { get; set; }
+
         public TriggerStub()
         {
 
         }
 
         public Task BeforeSave(ITriggerContext<TEntity> context, CancellationToken cancellationToken)
         {
+            BeforeSaveHandler?.Invoke(context, cancellationToken);
+
             BeforeSaveInvocations.Add(context);
             return Task.CompletedTask;
         }
 
         public Task AfterSave(ITriggerContext<TEntity> context, CancellationToken cancellationToken)
         {
+            AfterSaveHandler?.Invoke(context, cancellationToken);
+
             AfterSaveInvocations.Add(context);
             return Task.CompletedTask;
         }

test/EntityFrameworkCore.Triggered.Tests/TriggerSessionTests.cs

@@ -7,6 +7,7 @@
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Infrastructure;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 using Xunit;
 
 namespace EntityFrameworkCore.Triggered.Tests
@@ -29,6 +30,7 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
             {
                 base.OnConfiguring(optionsBuilder);
 
+                optionsBuilder.EnableServiceProviderCaching(false);
                 optionsBuilder.UseInMemoryDatabase("test");
                 optionsBuilder.UseTriggers(triggerOptions =>
                 {
@@ -100,8 +102,7 @@ public async Task RaiseAfterSaveTriggers_RaisesOnceOnSimpleAddition()
             using var context = new TestDbContext();
             var subject = CreateSubject(context);
 
-            context.TestModels.Add(new TestModel
-            {
+            context.TestModels.Add(new TestModel {
                 Id = Guid.NewGuid(),
                 Name = "test1"
             });
@@ -146,5 +147,64 @@ public async Task RaiseAfterSaveTriggers_ThrowsOnCancelledException()
 
             await Assert.ThrowsAsync<OperationCanceledException>(async () => await subject.RaiseAfterSaveTriggers(cancellationTokenSource.Token));
         }
+
+        [Fact]
+        public async Task RaiseBeforeSaveTriggers_RecursiveCall_SkipsDiscoveredChanges()
+        {
+            using var context = new TestDbContext();
+            var subject = CreateSubject(context);
+
+            context.TriggerStub.BeforeSaveHandler = (_1, _2) => {
+                if (context.TriggerStub.BeforeSaveInvocations.Count > 1)
+                {
+                    return Task.CompletedTask; 
+                }
+                return subject.RaiseBeforeSaveTriggers(default);
+            };
+            
+            context.TestModels.Add(new TestModel {
+                Id = Guid.NewGuid(),
+                Name = "test1"
+            });
+
+            subject.DiscoverChanges();
+            await subject.RaiseBeforeSaveTriggers();
+
+            Assert.NotEmpty(context.TriggerStub.BeforeSaveInvocations);
+        }
+
+        [Fact]
+        public async Task RaiseBeforeSaveTriggers_SkipDetectedChangesAsTrue_ExcludesDetectedChanges()
+        {
+            using var context = new TestDbContext();
+            var subject = CreateSubject(context);
+
+            context.TestModels.Add(new TestModel {
+                Id = Guid.NewGuid(),
+                Name = "test1"
+            });
+
+            subject.DiscoverChanges();
+            await subject.RaiseBeforeSaveTriggers(true);
+
+            Assert.Empty(context.TriggerStub.BeforeSaveInvocations);
+        }
+
+        [Fact]
+        public async Task RaiseBeforeSaveTriggers_SkipDetectedChangesAsFalse_IncludesDetectedChanges()
+        {
+            using var context = new TestDbContext();
+            var subject = CreateSubject(context);
+
+            context.TestModels.Add(new TestModel {
+                Id = Guid.NewGuid(),
+                Name = "test1"
+            });
+
+            subject.DiscoverChanges();
+            await subject.RaiseBeforeSaveTriggers();
+
+            Assert.NotEmpty(context.TriggerStub.BeforeSaveInvocations);
+        }
     }
 }

test/EntityFrameworkCore.Triggered.Tests/TriggeredDbContextTests.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 using EntityFrameworkCore.Triggered.Tests.Stubs;
 using Microsoft.EntityFrameworkCore;
@@ -20,6 +21,13 @@ class TestModel
 
         class TestDbContext : TriggeredDbContext
         {
+            readonly bool _stubService;
+
+            public TestDbContext(bool stubService)
+            {
+                _stubService = stubService;
+            }
+
             public TriggerStub<TestModel> TriggerStub { get; } = new TriggerStub<TestModel>();
 
             public DbSet<TestModel> TestModels { get; set; }
@@ -32,12 +40,16 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
                 optionsBuilder.UseTriggers(triggerOptions => {
                     triggerOptions.AddTrigger(TriggerStub);
                 });
-                optionsBuilder.ReplaceService<ITriggerService, TriggerServiceStub>();
+
+                if (_stubService)
+                {
+                    optionsBuilder.ReplaceService<ITriggerService, TriggerServiceStub>();
+                }
             }
         }
 
-        TestDbContext CreateSubject()
-            => new TestDbContext();
+        TestDbContext CreateSubject(bool stubService = true)
+            => new TestDbContext(stubService);
 
         [Fact]
         public void SaveChanges_CreatesChangeHandlerSession()
@@ -73,10 +85,34 @@ public async Task SaveChangesAsync_CreatesChangeHandlerSession()
         public async Task SaveChangesAsyncWithAccept_CreatesChangeHandlerSession()
         {
             var subject = CreateSubject();
-            var TriggersessionStub = (TriggerServiceStub)subject.GetService<ITriggerService>();
+            var triggerSessionStub = (TriggerServiceStub)subject.GetService<ITriggerService>();
 
             await subject.SaveChangesAsync(true);
-            Assert.Equal(1, TriggersessionStub.CreateSessionCalls);
+            Assert.Equal(1, triggerSessionStub.CreateSessionCalls);
+        }
+
+        [Fact]
+        public async Task SaveChangesAsync_RecursiveCall_ReturnsActiveTriggerSession()
+        {
+            var subject = CreateSubject(false);
+
+            subject.TriggerStub.BeforeSaveHandler = (_1, _2) => {
+                if (subject.TriggerStub.BeforeSaveInvocations.Count > 1)
+                {
+                    return Task.CompletedTask;
+                }
+
+                return subject.SaveChangesAsync();
+            };
+
+            subject.TestModels.Add(new TestModel {
+                Id = Guid.NewGuid(),
+                Name = "test1"
+            });
+
+            await subject.SaveChangesAsync();
+
+            Assert.Equal(1, subject.TriggerStub.BeforeSaveInvocations.Count);
         }
     }
 }