From 5d705b15eaca23ea7534546e6517021cde27c0d6 Mon Sep 17 00:00:00 2001 From: maliming Date: Wed, 17 Apr 2024 18:16:31 +0800 Subject: [PATCH] Improve the performance of `AbpEfCoreNavigationHelper`. --- .../Abp/EntityFrameworkCore/AbpDbContext.cs | 33 ++-- .../AbpEfCoreNavigationHelper.cs | 172 +++++++++--------- .../ChangeTrackers/AbpEntityEntry.cs | 49 +++++ .../AbpEntityEntryNavigationProperty.cs | 28 --- .../EntityHistory/EntityHistoryHelper.cs | 15 +- .../Abp/TestApp/Testing/DomainEvents_Tests.cs | 50 ++++- 6 files changed, 201 insertions(+), 146 deletions(-) create mode 100644 framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs delete mode 100644 framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntryNavigationProperty.cs diff --git a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/AbpDbContext.cs b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/AbpDbContext.cs index 09c1f0a3d7..f350d0c614 100644 --- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/AbpDbContext.cs +++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/AbpDbContext.cs @@ -162,6 +162,21 @@ public abstract class AbpDbContext : DbContext, IAbpEfCoreDbContext, { try { + if (EntityChangeOptions.Value.PublishEntityUpdatedEventWhenNavigationChanges) + { + foreach (var entityEntry in AbpEfCoreNavigationHelper.GetChangedEntityEntries()) + { + if (entityEntry.Entity is ISoftDelete && entityEntry.Entity.As().IsDeleted) + { + EntityChangeEventHelper.PublishEntityDeletedEvent(entityEntry.Entity); + } + else + { + EntityChangeEventHelper.PublishEntityUpdatedEvent(entityEntry.Entity); + } + } + } + var auditLog = AuditingManager?.Current?.Log; List? entityChangeList = null; if (auditLog != null) @@ -344,8 +359,7 @@ public abstract class AbpDbContext : DbContext, IAbpEfCoreDbContext, EntityChangeEventHelper.PublishEntityUpdatedEvent(entry.Entity); } } - else if (EntityChangeOptions.Value.PublishEntityUpdatedEventWhenNavigationChanges && - (entry.Navigations.Any(x => x.IsModified) || AbpEfCoreNavigationHelper.IsEntityEntryNavigationChanged(entry))) + else if (EntityChangeOptions.Value.PublishEntityUpdatedEventWhenNavigationChanges && AbpEfCoreNavigationHelper.IsEntityEntryModified(entry)) { if (entry.Entity is ISoftDelete && entry.Entity.As().IsDeleted) { @@ -363,21 +377,6 @@ public abstract class AbpDbContext : DbContext, IAbpEfCoreDbContext, EntityChangeEventHelper.PublishEntityDeletedEvent(entry.Entity); break; } - - if (EntityChangeOptions.Value.PublishEntityUpdatedEventWhenNavigationChanges) - { - foreach (var entityEntry in AbpEfCoreNavigationHelper.GetChangedEntityEntries()) - { - if (entityEntry.Entity is ISoftDelete && entityEntry.Entity.As().IsDeleted) - { - EntityChangeEventHelper.PublishEntityDeletedEvent(entityEntry.Entity); - } - else - { - EntityChangeEventHelper.PublishEntityUpdatedEvent(entityEntry.Entity); - } - } - } } protected virtual void HandlePropertiesBeforeSave() diff --git a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEfCoreNavigationHelper.cs b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEfCoreNavigationHelper.cs index d145ed5316..bdea524940 100644 --- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEfCoreNavigationHelper.cs +++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEfCoreNavigationHelper.cs @@ -4,6 +4,9 @@ using System.Collections.Generic; using System.Linq; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.ChangeTracking; +using Microsoft.EntityFrameworkCore.Internal; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Metadata.Internal; using Volo.Abp.DependencyInjection; using Volo.Abp.Domain.Entities; @@ -15,18 +18,18 @@ namespace Volo.Abp.EntityFrameworkCore.ChangeTrackers; /// public class AbpEfCoreNavigationHelper : ITransientDependency { - protected Dictionary> EntityEntryNavigationProperties { get; } = new Dictionary>(); + protected Dictionary EntityEntries { get; } = new(); public virtual void ChangeTracker_Tracked(ChangeTracker changeTracker, object? sender, EntityTrackedEventArgs e) { EntityEntryTrackedOrStateChanged(e.Entry); - DetectChanges(); + DetectChanges(changeTracker, e.Entry); } public virtual void ChangeTracker_StateChanged(ChangeTracker changeTracker, object? sender, EntityStateChangedEventArgs e) { EntityEntryTrackedOrStateChanged(e.Entry); - DetectChanges(); + DetectChanges(changeTracker, e.Entry); } protected virtual void EntityEntryTrackedOrStateChanged(EntityEntry entityEntry) @@ -42,83 +45,103 @@ public class AbpEfCoreNavigationHelper : ITransientDependency return; } - var navigationProperties = EntityEntryNavigationProperties.GetOrAdd(entryId, () => new List()); - var index = 0; - foreach (var navigationEntry in entityEntry.Navigations.Where(navigation => !navigation.IsModified)) + if (EntityEntries.ContainsKey(entryId)) { - var navigationProperty = navigationProperties.FirstOrDefault(x => x.Index == index); - if (navigationProperty != null) + return; + } + + EntityEntries.Add(entryId, new AbpEntityEntry(entryId, entityEntry)); + } + + protected virtual void DetectChanges(ChangeTracker changeTracker, EntityEntry entityEntry) + { + if (entityEntry.State != EntityState.Added && + entityEntry.State != EntityState.Deleted && + entityEntry.State != EntityState.Modified) + { + return; + } + +#pragma warning disable EF1001 + var stateManager = changeTracker.Context.GetDependencies().StateManager; + var internalEntityEntityEntry = stateManager.Entries.FirstOrDefault(x => x.Entity == entityEntry.Entity); + if (internalEntityEntityEntry == null) + { + return; + } + + var foreignKeys = entityEntry.Metadata.GetForeignKeys().ToList(); + foreach (var foreignKey in foreignKeys) + { + var principal = stateManager.FindPrincipal(internalEntityEntityEntry, foreignKey); + if (principal == null) { - if (navigationProperty.Value == null || IsCollectionAndEmpty(navigationProperty.Value)) - { - navigationProperty.Value = navigationEntry.CurrentValue; - } + continue; } - else + + var entryId = GetEntityId(principal.ToEntityEntry()); + if (entryId == null || !EntityEntries.TryGetValue(entryId, out var abpEntityEntry)) { - navigationProperties.Add(new AbpEntityEntryNavigationProperty(index, navigationEntry.Metadata.Name, navigationEntry.CurrentValue, false, entityEntry, navigationEntry)); + continue; } - index++; + abpEntityEntry.IsModified = true; + var navigationEntry = abpEntityEntry.NavigationEntries.FirstOrDefault(x => x.NavigationEntry.Metadata is INavigation navigationMetadata && navigationMetadata.ForeignKey == foreignKey) ?? + abpEntityEntry.NavigationEntries.FirstOrDefault(x => x.NavigationEntry.Metadata is ISkipNavigation skipNavigationMetadata && skipNavigationMetadata.ForeignKey == foreignKey); + if (navigationEntry != null) + { + navigationEntry.IsModified = true; + } } - } - protected virtual void DetectChanges() - { - foreach (var entityEntryNavigationProperties in EntityEntryNavigationProperties) + var skipNavigations = entityEntry.Metadata.GetSkipNavigations().ToList(); + foreach (var skipNavigation in skipNavigations) { - foreach (var navigationProperty in entityEntryNavigationProperties.Value.Where(x => !x.IsChanged && x.EntityEntry.State == EntityState.Unchanged)) + var joinEntityType = skipNavigation.JoinEntityType; + var foreignKey = skipNavigation.ForeignKey; + var inverseForeignKey = skipNavigation.Inverse.ForeignKey; + foreach (var joinEntry in stateManager.Entries) { - if (navigationProperty.NavigationEntry.IsModified) + if (joinEntry.EntityType != joinEntityType || stateManager.FindPrincipal(joinEntry, foreignKey) != internalEntityEntityEntry) + { + continue; + } + + var principal = stateManager.FindPrincipal(joinEntry, inverseForeignKey); + if (principal == null) { - navigationProperty.IsChanged = true; continue; } - if (navigationProperty.Value == null || IsCollectionAndEmpty(navigationProperty.Value)) + var entryId = GetEntityId(principal.ToEntityEntry()); + if (entryId == null || !EntityEntries.TryGetValue(entryId, out var abpEntityEntry)) { - if (navigationProperty.NavigationEntry.CurrentValue != null || IsCollectionAndNotEmpty(navigationProperty.NavigationEntry.CurrentValue)) - { - if (navigationProperty.NavigationEntry.CurrentValue is ICollection collection) - { - navigationProperty.Value = collection.Cast().ToList(); - } - else - { - navigationProperty.Value = navigationProperty.NavigationEntry.CurrentValue; - } - } + continue; } - if (navigationProperty.Value != null || IsCollectionAndNotEmpty(navigationProperty.Value)) + abpEntityEntry.IsModified = true; + abpEntityEntry.IsModified = true; + var navigationEntry = abpEntityEntry.NavigationEntries.FirstOrDefault(x => x.NavigationEntry.Metadata is INavigation navigationMetadata && navigationMetadata.ForeignKey == inverseForeignKey) ?? + abpEntityEntry.NavigationEntries.FirstOrDefault(x => x.NavigationEntry.Metadata is ISkipNavigation skipNavigationMetadata && skipNavigationMetadata.ForeignKey == inverseForeignKey); + if (navigationEntry != null) { - if (navigationProperty.NavigationEntry.CurrentValue == null || IsCollectionAndEmpty(navigationProperty.NavigationEntry.CurrentValue)) - { - if (IsCollectionAndEmpty(navigationProperty.Value) && IsCollectionAndEmpty(navigationProperty.NavigationEntry.CurrentValue)) - { - continue; - } - - navigationProperty.IsChanged = true; - } + navigationEntry.IsModified = true; } } } +#pragma warning restore EF1001 } public virtual List GetChangedEntityEntries() { - DetectChanges(); - return EntityEntryNavigationProperties - .SelectMany(x => x.Value) - .Where(x => x.NavigationEntry.IsModified || x.IsChanged) - .Select(x => x.EntityEntry) + return EntityEntries + .Where(x => x.Value.IsModified) + .Select(x => x.Value.EntityEntry) .ToList(); } - public virtual bool IsEntityEntryNavigationChanged(EntityEntry entityEntry) + public virtual bool IsEntityEntryModified(EntityEntry entityEntry) { - DetectChanges(); if (entityEntry.State == EntityState.Modified) { return true; @@ -130,62 +153,35 @@ public class AbpEfCoreNavigationHelper : ITransientDependency return false; } - if (EntityEntryNavigationProperties.TryGetValue(entryId, out var navigationProperties)) - { - return navigationProperties.Any(x => x.IsChanged) || - navigationProperties.Any(x => x.NavigationEntry.IsModified) || - navigationProperties.Any(x => - x.NavigationEntry is ReferenceEntry && - x.NavigationEntry.As().TargetEntry?.State == EntityState.Modified); - } - - return false; + return EntityEntries.TryGetValue(entryId, out var abpEntityEntry) && abpEntityEntry.IsModified; } - public virtual bool IsEntityEntryNavigationChanged(NavigationEntry navigationEntry, int index) + public virtual bool IsNavigationEntryModified(EntityEntry entityEntry, NavigationEntry navigationEntry, int index) { - if (navigationEntry.IsModified || (navigationEntry is ReferenceEntry && navigationEntry.As().TargetEntry?.State == EntityState.Modified)) - { - return true; - } - - var entryId = GetEntityId(navigationEntry.EntityEntry); + var entryId = GetEntityId(entityEntry); if (entryId == null) { return false; } - if (EntityEntryNavigationProperties.TryGetValue(entryId, out var navigationProperties)) + if (!EntityEntries.TryGetValue(entryId, out var abpEntityEntry)) { - var navigationProperty = navigationProperties.FirstOrDefault(x => x.Index == index); - if (navigationProperty != null && navigationProperty.IsChanged) - { - return true; - } + return false; } - return false; + var navigationEntryProperty = abpEntityEntry.NavigationEntries.ElementAtOrDefault(index); + return navigationEntryProperty != null && navigationEntryProperty.IsModified; } - public void Clear() - { - EntityEntryNavigationProperties.Clear(); - } - - private string? GetEntityId(EntityEntry entityEntry) + protected virtual string? GetEntityId(EntityEntry entityEntry) { return entityEntry.Entity is IEntity entryEntity && entryEntity.GetKeys().Length == 1 ? entryEntity.GetKeys().FirstOrDefault()?.ToString() : null; } - private bool IsCollectionAndEmpty(object? value) - { - return value is ICollection && value is ICollection collection && collection.Count == 0; - } - - private bool IsCollectionAndNotEmpty(object? value) + public void Clear() { - return value is ICollection && value is ICollection collection && collection.Count != 0; + EntityEntries.Clear(); } } diff --git a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs new file mode 100644 index 0000000000..c26ae2bbf1 --- /dev/null +++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs @@ -0,0 +1,49 @@ +using System.Collections.Generic; +using System.Linq; +using Microsoft.EntityFrameworkCore.ChangeTracking; + +namespace Volo.Abp.EntityFrameworkCore.ChangeTrackers; + +public class AbpEntityEntry +{ + public string Id { get; set; } + + public EntityEntry EntityEntry { get; set; } + + public List NavigationEntries { get; set; } + + private bool _isModified; + public bool IsModified + { + get + { + return _isModified || NavigationEntries.Any(n => n.IsModified); + } + set + { + _isModified = value; + } + } + + public AbpEntityEntry(string id, EntityEntry entityEntry) + { + Id = id; + EntityEntry = entityEntry; + NavigationEntries = EntityEntry.Navigations.Select(x => new AbpNavigationEntry(x, x.Metadata.Name)).ToList(); + } +} + +public class AbpNavigationEntry +{ + public NavigationEntry NavigationEntry { get; set; } + + public string Name { get; set; } + + public bool IsModified { get; set; } + + public AbpNavigationEntry(NavigationEntry navigationEntry, string name) + { + NavigationEntry = navigationEntry; + Name = name; + } +} diff --git a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntryNavigationProperty.cs b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntryNavigationProperty.cs deleted file mode 100644 index a3cec7cf64..0000000000 --- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntryNavigationProperty.cs +++ /dev/null @@ -1,28 +0,0 @@ -using Microsoft.EntityFrameworkCore.ChangeTracking; - -namespace Volo.Abp.EntityFrameworkCore.ChangeTrackers; - -public class AbpEntityEntryNavigationProperty -{ - public int Index { get; set; } - - public string Name { get; set; } - - public object? Value { get; set; } - - public bool IsChanged { get; set; } - - public EntityEntry EntityEntry { get; set; } - - public NavigationEntry NavigationEntry { get; set; } - - public AbpEntityEntryNavigationProperty(int index, string name, object? value, bool isChanged, EntityEntry entityEntry, NavigationEntry navigationEntry) - { - Index = index; - Name = name; - Value = value; - IsChanged = isChanged; - EntityEntry = entityEntry; - NavigationEntry = navigationEntry; - } -} diff --git a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/EntityHistory/EntityHistoryHelper.cs b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/EntityHistory/EntityHistoryHelper.cs index ef6bd230f5..49a25601a4 100644 --- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/EntityHistory/EntityHistoryHelper.cs +++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/EntityHistory/EntityHistoryHelper.cs @@ -193,17 +193,17 @@ public class EntityHistoryHelper : IEntityHistoryHelper, ITransientDependency } } - if (entityEntry.State == EntityState.Unchanged && Options.SaveEntityHistoryWhenNavigationChanges && AbpEfCoreNavigationHelper != null) + if (Options.SaveEntityHistoryWhenNavigationChanges && AbpEfCoreNavigationHelper != null) { var index = 0; - foreach (var navigation in entityEntry.Navigations) + foreach (var navigationEntry in entityEntry.Navigations) { - if (navigation.IsModified || AbpEfCoreNavigationHelper.IsEntityEntryNavigationChanged(navigation, index)) + if (AbpEfCoreNavigationHelper.IsNavigationEntryModified(entityEntry, navigationEntry, index)) { propertyChanges.Add(new EntityPropertyChangeInfo { - PropertyName = navigation.Metadata.Name, - PropertyTypeFullName = navigation.Metadata.ClrType.GetFirstGenericArgumentIfNullable().FullName! + PropertyName = navigationEntry.Metadata.Name, + PropertyTypeFullName = navigationEntry.Metadata.ClrType.GetFirstGenericArgumentIfNullable().FullName! }); } @@ -255,10 +255,9 @@ public class EntityHistoryHelper : IEntityHistoryHelper, ITransientDependency protected virtual bool HasNavigationPropertiesChanged(EntityEntry entityEntry) { - return entityEntry.State == EntityState.Unchanged && - Options.SaveEntityHistoryWhenNavigationChanges && + return Options.SaveEntityHistoryWhenNavigationChanges && AbpEfCoreNavigationHelper != null && - AbpEfCoreNavigationHelper.IsEntityEntryNavigationChanged(entityEntry); + AbpEfCoreNavigationHelper.IsEntityEntryModified(entityEntry); } protected virtual bool ShouldSavePropertyHistory(PropertyEntry propertyEntry, bool defaultValue) diff --git a/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Testing/DomainEvents_Tests.cs b/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Testing/DomainEvents_Tests.cs index 74f8198aa7..ddb91e7ba6 100644 --- a/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Testing/DomainEvents_Tests.cs +++ b/framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Testing/DomainEvents_Tests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; @@ -209,22 +209,32 @@ public abstract class DomainEvents_Tests : TestAppTestBase>(data => + // Test with value object + entityUpdatedEventTriggered = false; + await WithUnitOfWorkAsync(async () => { - throw new Exception("Should not trigger this event"); + var entity = await AppEntityWithNavigationsRepository.GetAsync(entityId); + entity.AppEntityWithValueObjectAddress = new AppEntityWithValueObjectAddress("Turkey"); + await AppEntityWithNavigationsRepository.UpdateAsync(entity); }); + entityUpdatedEventTriggered.ShouldBeTrue(); + personCreatedEventCount.ShouldBe(++entityUpdatedEventTriggerCount); - // Test with value object entityUpdatedEventTriggered = false; await WithUnitOfWorkAsync(async () => { var entity = await AppEntityWithNavigationsRepository.GetAsync(entityId); - entity.AppEntityWithValueObjectAddress = new AppEntityWithValueObjectAddress("Turkey"); + entity.AppEntityWithValueObjectAddress.Country = "USA"; await AppEntityWithNavigationsRepository.UpdateAsync(entity); }); entityUpdatedEventTriggered.ShouldBeTrue(); personCreatedEventCount.ShouldBe(++entityUpdatedEventTriggerCount); + LocalEventBus.Subscribe>(data => + { + throw new Exception("Should not trigger this event"); + }); + entityUpdatedEventTriggered = false; await WithUnitOfWorkAsync(async () => { @@ -249,6 +259,16 @@ public abstract class DomainEvents_Tests : TestAppTestBase + { + var entity = await AppEntityWithNavigationsRepository.GetAsync(entityId); + entity.OneToOne.ChildName = "ChildName2"; + await AppEntityWithNavigationsRepository.UpdateAsync(entity); + }); + entityUpdatedEventTriggered.ShouldBeTrue(); + personCreatedEventCount.ShouldBe(++entityUpdatedEventTriggerCount); + LocalEventBus.Subscribe>(data => { throw new Exception("Should not trigger this event"); @@ -282,6 +302,16 @@ public abstract class DomainEvents_Tests : TestAppTestBase + { + var entity = await AppEntityWithNavigationsRepository.GetAsync(entityId); + entity.OneToMany[0].ChildName = "ChildName2"; + await AppEntityWithNavigationsRepository.UpdateAsync(entity); + }); + entityUpdatedEventTriggered.ShouldBeTrue(); + personCreatedEventCount.ShouldBe(++entityUpdatedEventTriggerCount); + LocalEventBus.Subscribe>(data => { throw new Exception("Should not trigger this event"); @@ -314,6 +344,16 @@ public abstract class DomainEvents_Tests : TestAppTestBase + { + var entity = await AppEntityWithNavigationsRepository.GetAsync(entityId); + entity.ManyToMany[0].ChildName = "ChildName2"; + await AppEntityWithNavigationsRepository.UpdateAsync(entity); + }); + entityUpdatedEventTriggered.ShouldBeTrue(); + personCreatedEventCount.ShouldBe(++entityUpdatedEventTriggerCount); + entityUpdatedEventTriggered = false; await WithUnitOfWorkAsync(async () => {