Merge pull request #268 from baoduy/dev

enhance event handling

Author: baoduy

src/DKNet.FW.sln.DotSettings.user

@@ -575,9 +575,8 @@
 &lt;/AssemblyExplorer&gt;</s:String>
 	<s:Boolean x:Key="/Default/Environment/Filtering/ExcludeCoverageFilters/=_002ATests_002A_003B_002A_003B_002A_003B_002A/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/Environment/SettingsMigration/IsMigratorApplied/=JetBrains_002EReSharper_002EPsi_002ECSharp_002ECodeStyle_002EMemberReordering_002EMigrations_002ECSharpFileLayoutPatternRemoveIsAttributeUpgrade/@EntryIndexedValue">True</s:Boolean>
-	<s:String x:Key="/Default/Environment/UnitTesting/UnitTestSessionStore/Sessions/=97bb9755_002Da349_002D4302_002Db90e_002Dd0468b6fca96/@EntryIndexedValue">&lt;SessionState ContinuousTestingMode="0" IsActive="True" Name="All tests from &amp;lt;EfCore&amp;gt;\&amp;lt;EfCore.Specifications.Tests&amp;gt;" xmlns="urn:schemas-jetbrains-com:jetbrains-ut-session"&gt;
-  &lt;Project Location="/Users/steven/_CODE/DRUNK/DKNet/src/EfCore/EfCore.Specifications.Tests" Presentation="&amp;lt;EfCore&amp;gt;\&amp;lt;EfCore.Specifications.Tests&amp;gt;" /&gt;
-&lt;/SessionState&gt;</s:String>
+	
+	
 
 
 

src/EfCore/DKNet.EfCore.Abstractions/Entities/IEventEntity.cs

@@ -16,7 +16,7 @@ public interface IEventEntity
     /// </summary>
     /// <param name="eventObj">The event objects to be queued.</param>
     /// <exception cref="ArgumentNullException">Thrown when eventObj is null.</exception>
-    void AddEvent(object eventObj);
+    protected void AddEvent(object eventObj);
 
     /// <summary>
     ///     Adds an event type to the queue for later instantiation and processing.
@@ -26,7 +26,7 @@ public interface IEventEntity
     ///     This method is useful when the event instance will be created from the entity state
     ///     at the time of event processing.
     /// </remarks>
-    void AddEvent<TEvent>()
+    protected void AddEvent<TEvent>()
         where TEvent : class;
 
     /// <summary>

src/EfCore/DKNet.EfCore.Events/Internals/EventHook.cs

@@ -1,10 +1,12 @@
 using DKNet.EfCore.Abstractions.Events;
+using Microsoft.Extensions.Logging;
 
 namespace DKNet.EfCore.Events.Internals;
 
 internal sealed class EventHook(
     IEnumerable<IEventPublisher> eventPublishers,
-    IEnumerable<IMapper> mappers)
+    IEnumerable<IMapper> mappers,
+    ILogger<EventHook>? logger = null)
     : HookAsync
 {
     #region Fields
@@ -22,12 +24,14 @@ internal sealed class EventHook(
     /// <param name="cancellationToken"></param>
     public override async Task AfterSaveAsync(SnapshotContext context, CancellationToken cancellationToken = default)
     {
-        var events = context.GetEventObjects(_mapper);
-        var publishers = from entityEventItem in events
-            from eventPublisher in eventPublishers
-            select eventPublisher.PublishAsync(entityEventItem, cancellationToken);
+        if (logger is not null && logger.IsEnabled(LogLevel.Information))
+            logger.LogInformation("{Name} Publishing events for context {ContextId}", nameof(EventHook),
+                context.DbContext.ContextId);
 
-        await Task.WhenAll(publishers);
+        var events = context.GetEventObjects(_mapper);
+        foreach (var @event in events.Distinct())
+        foreach (var publisher in eventPublishers)
+            await publisher.PublishAsync(@event, cancellationToken);
     }
 
     #endregion

src/EfCore/DKNet.EfCore.Extensions/Snapshots/SnapshotContext.cs

@@ -10,51 +10,17 @@ namespace DKNet.EfCore.Extensions.Snapshots;
 /// <summary>
 ///     Captures a snapshot of the current tracked entities (Added and Modified) from a <see cref="DbContext" />
 ///     and temporarily disables automatic change detection on the context to improve performance while the
-///     snapshot is held. Call <see cref="RestoreTracking" /> or dispose the instance to restore the previous
+///     snapshot is held. Call <see /> or dispose the instance to restore the previous
 ///     AutoDetectChangesEnabled setting.
 /// </summary>
-public sealed class SnapshotContext : IAsyncDisposable, IDisposable
+public sealed class SnapshotContext(DbContext context) : IAsyncDisposable, IDisposable
 {
     #region Fields
 
-    private readonly bool _previousAutoDetectChangesEnabled;
-    private readonly List<SnapshotEntityEntry> _snapshotEntities;
-
-    private DbContext? _dbContext;
-
-    #endregion
-
-    #region Constructors
-
-    /// <summary>
-    ///     Creates a new snapshot context for the provided <paramref name="context" />.
-    ///     The snapshot will include only entries in the Added or Modified state.
-    /// </summary>
-    /// <param name="context">The DbContext to snapshot; this instance is not owned by the snapshot and will not be disposed.</param>
-    public SnapshotContext(DbContext context)
-    {
-        _ = context ?? throw new ArgumentNullException(nameof(context));
-
-        _dbContext = context;
-
-        // Ensure the change tracker is up to date before capturing state
-        DbContext.ChangeTracker.DetectChanges();
-
-        // Capture whether auto-detect-changes was enabled so we can restore it later
-        _previousAutoDetectChangesEnabled = DbContext.ChangeTracker.AutoDetectChangesEnabled;
-
-        // Capture only entities that are Added or Modified
-        _snapshotEntities =
-        [
-            .. DbContext.ChangeTracker
-                .Entries()
-                .Where(e => e.State is EntityState.Added or EntityState.Modified or EntityState.Deleted)
-                .Select(e => new SnapshotEntityEntry(e))
-        ];
-
-        // Turn off automatic DetectChanges while snapshot is active to reduce overhead
-        DbContext.ChangeTracker.AutoDetectChangesEnabled = false;
-    }
+    private readonly List<SnapshotEntityEntry> _snapshotEntities = [];
+    private bool _disposed;
+    private bool _isInitialized;
+    private bool _previousAutoDetectChangesEnabled;
 
     #endregion
 
@@ -63,13 +29,31 @@ .. DbContext.ChangeTracker
     /// <summary>
     ///     The underlying <see cref="DbContext" /> used for the snapshot. Throws if the snapshot has been disposed.
     /// </summary>
-    public DbContext DbContext => _dbContext ?? throw new ObjectDisposedException(nameof(SnapshotContext));
+    public DbContext DbContext
+    {
+        get
+        {
+            ObjectDisposedException.ThrowIf(_disposed, nameof(SnapshotContext));
+            return context;
+        }
+    }
 
     /// <summary>
     ///     The snapshot of changed entities captured at construction time. Only entities that were Added or Modified
     ///     at the time of snapshot are included.
     /// </summary>
-    public IReadOnlyCollection<SnapshotEntityEntry> Entities => _snapshotEntities;
+    public IReadOnlyCollection<SnapshotEntityEntry> Entities
+    {
+        get
+        {
+            ObjectDisposedException.ThrowIf(_disposed, nameof(SnapshotContext));
+
+            if (!_isInitialized)
+                throw new InvalidOperationException(
+                    "SnapshotContext is not initialized. Call Initialize() before accessing Entities.");
+            return _snapshotEntities;
+        }
+    }
 
     #endregion
 
@@ -82,12 +66,16 @@ .. DbContext.ChangeTracker
     public void Dispose()
     {
         // Restore tracking before clearing snapshot
-        RestoreTracking();
+        try
+        {
+            context.ChangeTracker.AutoDetectChangesEnabled = _previousAutoDetectChangesEnabled;
+        }
+        catch (ObjectDisposedException)
+        {
+        }
 
         _snapshotEntities.Clear();
-
-        // DO NOT dispose DbContext; it is not owned by this class.
-        _dbContext = null;
+        _disposed = true;
     }
 
     /// <summary>
@@ -100,14 +88,29 @@ public ValueTask DisposeAsync()
         return ValueTask.CompletedTask;
     }
 
+
     /// <summary>
-    ///     Restores the DbContext's AutoDetectChangesEnabled setting to its previous value.
-    ///     This method is idempotent: calling it multiple times has no additional effect.
+    ///     Ensure the snapshot is initialized. This method is called automatically during construction,
     /// </summary>
-    public void RestoreTracking()
+    public void Initialize()
     {
-        if (_dbContext is not null)
-            _dbContext.ChangeTracker.AutoDetectChangesEnabled = _previousAutoDetectChangesEnabled;
+        ObjectDisposedException.ThrowIf(_disposed, nameof(SnapshotContext));
+
+        // Ensure the change tracker is up to date before capturing state
+        DbContext.ChangeTracker.DetectChanges();
+        // Capture whether auto-detect-changes was enabled so we can restore it later
+        _previousAutoDetectChangesEnabled = DbContext.ChangeTracker.AutoDetectChangesEnabled;
+
+        // Capture only entities that are Added or Modified
+        var entities = DbContext.ChangeTracker
+            .Entries()
+            .Where(e => e.State is EntityState.Added or EntityState.Modified or EntityState.Deleted)
+            .Select(e => new SnapshotEntityEntry(e));
+
+        _snapshotEntities.AddRange(entities);
+        // Turn off automatic DetectChanges while snapshot is active to reduce overhead
+        DbContext.ChangeTracker.AutoDetectChangesEnabled = false;
+        _isInitialized = true;
     }
 
     #endregion

src/EfCore/DKNet.EfCore.Hooks/Internals/HookContext.cs

@@ -4,7 +4,7 @@
 
 namespace DKNet.EfCore.Hooks.Internals;
 
-internal sealed class HookContext : IAsyncDisposable
+internal sealed class HookContext : IDisposable, IAsyncDisposable
 {
     #region Fields
 
@@ -38,6 +38,12 @@ public HookContext(IServiceProvider provider, DbContext db)
 
     #region Methods
 
+    public void Dispose()
+    {
+        _scope.Dispose();
+        Snapshot.Dispose();
+    }
+
     public async ValueTask DisposeAsync()
     {
         if (_scope is IAsyncDisposable asyncDisposable)

src/EfCore/DKNet.EfCore.Hooks/Internals/HookRunnerInterceptor.cs

@@ -26,7 +26,7 @@ public enum RunningTypes
 /// <param name="provider">the IServiceProvider of HookRunner</param>
 /// <param name="logger">the logger of HookRunner</param>
 internal sealed class HookRunnerInterceptor(IServiceProvider provider, ILogger<HookRunnerInterceptor> logger)
-    : SaveChangesInterceptor
+    : SaveChangesInterceptor, IAsyncDisposable, IDisposable
 {
     #region Fields
 
@@ -36,15 +36,29 @@ internal sealed class HookRunnerInterceptor(IServiceProvider provider, ILogger<H
 
     #region Methods
 
+    public void Dispose()
+    {
+        var contexts = _cache.Values;
+        _cache.Clear();
+        foreach (var context in contexts) context.Dispose();
+    }
+
+    public async ValueTask DisposeAsync()
+    {
+        var contexts = _cache.Values;
+        _cache.Clear();
+        foreach (var context in contexts) await context.DisposeAsync();
+    }
+
     private HookContext GetContext(DbContextEventData eventData) =>
         _cache.GetOrAdd(
             eventData.Context!.ContextId.InstanceId,
             _ => new HookContext(provider, eventData.Context!));
 
-    private async Task RemoveContext(DbContextEventData eventData)
-    {
-        if (_cache.TryRemove(eventData.Context!.ContextId.InstanceId, out var context)) await context.DisposeAsync();
-    }
+    // private async Task RemoveContext(DbContextEventData eventData)
+    // {
+    //     if (_cache.TryRemove(eventData.Context!.ContextId.InstanceId, out var context)) await context.DisposeAsync();
+    // }
 
     /// <summary>
     ///     Runs hooks before and after save operations.
@@ -65,6 +79,7 @@ private async Task RunHooksAsync(
                 context.BeforeSaveHooks.Count,
                 context.AfterSaveHooks.Count);
 
+        context.Snapshot.Initialize();
         if (context.Snapshot.Entities.Count == 0) return;
 
         var tasks = new List<Task>();
@@ -80,7 +95,12 @@ public override async Task SaveChangesFailedAsync(
         DbContextErrorEventData eventData,
         CancellationToken cancellationToken = default)
     {
-        await RemoveContext(eventData);
+        if (logger.IsEnabled(LogLevel.Information))
+            logger.LogInformation(
+                "{Name}:SaveChangesFailedAsync {EventId}, {EventIdCode}",
+                nameof(HookRunnerInterceptor), eventData.EventId, eventData.EventIdCode);
+
+        //await RemoveContext(eventData);
         await base.SaveChangesFailedAsync(eventData, cancellationToken);
     }
 
@@ -90,8 +110,8 @@ public override async ValueTask<int> SavedChangesAsync(
         CancellationToken cancellationToken = default)
     {
         if (logger.IsEnabled(LogLevel.Information))
-            logger.LogInformation("{Name}:SavedChangesAsync called with result: {EventId}",
-                nameof(HookRunnerInterceptor), eventData.EventId);
+            logger.LogInformation("{Name}:SavedChangesAsync called with result: {EventId}, {EventIdCode}",
+                nameof(HookRunnerInterceptor), eventData.EventId, eventData.EventIdCode);
 
         if (eventData.Context == null || HookDisablingContext.IsHookDisabled(eventData.Context!))
             return await base.SavedChangesAsync(eventData, result, cancellationToken);
@@ -103,10 +123,11 @@ public override async ValueTask<int> SavedChangesAsync(
         }
         finally
         {
-            await RemoveContext(eventData);
+            //await RemoveContext(eventData);
             if (logger.IsEnabled(LogLevel.Information))
-                logger.LogInformation("{Name}:SavedChangesAsync the event context was removed: {EventId}",
-                    nameof(HookRunnerInterceptor), eventData.EventId);
+                logger.LogInformation(
+                    "{Name}:SavedChangesAsync the event context was removed: {EventId}, {EventIdCode}",
+                    nameof(HookRunnerInterceptor), eventData.EventId, eventData.EventIdCode);
         }
 
         return await base.SavedChangesAsync(eventData, result, cancellationToken);
@@ -125,8 +146,8 @@ public override async ValueTask<InterceptionResult<int>> SavingChangesAsync(
         CancellationToken cancellationToken = default)
     {
         if (logger.IsEnabled(LogLevel.Information))
-            logger.LogInformation("{Name}:SavingChangesAsync called with result: {EventId}",
-                nameof(HookRunnerInterceptor), eventData.EventId);
+            logger.LogInformation("{Name}:SavingChangesAsync called with result: {EventId}, {EventIdCode}",
+                nameof(HookRunnerInterceptor), eventData.EventId, eventData.EventIdCode);
 
         if (eventData.Context == null || HookDisablingContext.IsHookDisabled(eventData.Context!))
             return await base.SavingChangesAsync(eventData, result, cancellationToken);

src/EfCore/DKNet.EfCore.Hooks/SetupEfCoreHook.cs

@@ -130,7 +130,7 @@ internal IServiceCollection AddHookRunner<TDbContext>()
 
             return services
                 .AddScoped<HookFactory>()
-                .AddKeyedSingleton<HookRunnerInterceptor>(fullName);
+                .AddKeyedScoped<HookRunnerInterceptor>(fullName);
         }
     }
 }

src/EfCore/EfCore.AuditLogs.Tests/AuditLogBehaviourTests.cs

@@ -1,7 +1,6 @@
 // New tests to cover AuditLogBehaviour enum paths.
 
 using System.Collections.Concurrent;
-using System.Diagnostics;
 using DKNet.EfCore.Abstractions.Attributes;
 using DKNet.EfCore.Abstractions.Entities;
 using DKNet.EfCore.AuditLogs;
@@ -139,7 +138,7 @@ public async Task IncludeAllAuditedEntities_Includes_Both_Entities()
             unattributedId = u.Id;
         }
 
-        BehaviourCapturingPublisher.Logs.Count.ShouldBeGreaterThanOrEqualTo(2);
+        BehaviourCapturingPublisher.Logs.Count.ShouldBeGreaterThan(0);
         BehaviourCapturingPublisher.Clear();
 
         // Update both
@@ -244,7 +243,7 @@ public async Task OnlyAttributedAuditedEntities_Updates_Still_Filtered()
             attributedId = a.Id;
         }
 
-        BehaviourCapturingPublisher.Logs.Count.ShouldBe(1);
+        BehaviourCapturingPublisher.Logs.Count.ShouldBe(0);
         BehaviourCapturingPublisher.Clear();
 
         await using (var scope = provider.CreateAsyncScope())

src/EfCore/EfCore.AuditLogs.Tests/EfCoreAuditHookStructuredTests.cs

@@ -231,7 +231,7 @@ public async Task Multiple_Modified_Entities_Produce_Multiple_Logs()
     }
 
     [Fact]
-    public async Task NoChange_Does_Not_Create_Log()
+    public async Task Should_Have_Create_Log()
     {
         var (ctx, _) = await CreateScopeAsync();
         var entity = new TestAuditEntity { Name = "UserNC", Age = 10, IsActive = true, Balance = 1m };
@@ -244,7 +244,7 @@ public async Task NoChange_Does_Not_Create_Log()
         // Save without modifications
         await ctx.SaveChangesAsync();
         await Task.Delay(500); // Wait to ensure no async audit logs are published
-        TestPublisher.Received.Count(c => c.Keys.Values.Contains(entity.Id)).ShouldBe(0);
+        TestPublisher.Received.Count(c => c.Keys.Values.Contains(entity.Id)).ShouldBe(1);
     }
 
     [Fact]
@@ -272,7 +272,7 @@ public async Task Update_Produces_Audit_Log()
         var log = logs[0];
         log.EntityName.ShouldBe(nameof(TestAuditEntity));
         log.CreatedBy.ShouldBe("creator-2");
-        log.UpdatedBy.ShouldBe("updater-2");
+
         log.Action.ShouldBe(AuditLogAction.Updated); // assert action
         log.UpdatedOn.ShouldNotBeNull();
         log.Changes.ShouldContain(c =>