From d51c565a1fa1e2257c72b0dd74da39e3f73d1ceb Mon Sep 17 00:00:00 2001 From: maliming Date: Sat, 19 Jul 2025 16:00:49 +0800 Subject: [PATCH 1/7] Refactor `UpdateNavigationEntries` method to improve performance. --- .../AbpEfCoreNavigationHelper.cs | 14 ++++- .../ChangeTrackers/AbpEntityEntry.cs | 59 +++++++------------ 2 files changed, 32 insertions(+), 41 deletions(-) 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 3d7b12f86e..1338e68400 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 @@ -60,7 +60,7 @@ public class AbpEfCoreNavigationHelper : ITransientDependency return; } - var foreignKeys = entityEntry.Metadata.GetForeignKeys().ToList(); + var foreignKeys = entityEntry.Metadata.GetForeignKeys(); foreach (var foreignKey in foreignKeys) { var principal = stateManager.FindPrincipal(internalEntityEntityEntry, foreignKey); @@ -75,7 +75,11 @@ public class AbpEfCoreNavigationHelper : ITransientDependency continue; } - abpEntityEntry.UpdateNavigationEntries(); + if (checkEntityEntryState && entityEntry.State == EntityState.Unchanged) + { + abpEntityEntry.UpdateNavigationEntries(entityEntry, foreignKey); + } + if (!abpEntityEntry.IsModified && (!checkEntityEntryState || IsEntityEntryChanged(entityEntry))) { abpEntityEntry.IsModified = true; @@ -115,7 +119,11 @@ public class AbpEfCoreNavigationHelper : ITransientDependency continue; } - abpEntityEntry.UpdateNavigationEntries(); + if (checkEntityEntryState && entityEntry.State == EntityState.Unchanged) + { + abpEntityEntry.UpdateNavigationEntries(entityEntry, inverseForeignKey); + } + if (!abpEntityEntry.IsModified && (!checkEntityEntryState || IsEntityEntryChanged(entityEntry))) { abpEntityEntry.IsModified = true; 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 index 8d597f85b5..00b38b289e 100644 --- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs +++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs @@ -1,8 +1,9 @@ -using System.Collections; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.ChangeTracking; +using Microsoft.EntityFrameworkCore.Metadata; namespace Volo.Abp.EntityFrameworkCore.ChangeTrackers; @@ -31,42 +32,34 @@ public class AbpEntityEntry NavigationEntries = EntityEntry.Navigations.Select(x => new AbpNavigationEntry(x, x.Metadata.Name)).ToList(); } - public void UpdateNavigationEntries() + public void UpdateNavigationEntries(EntityEntry entityEntry, IForeignKey foreignKey) { + var entityEntryNavigationEntry = NavigationEntries.FirstOrDefault(x => x.NavigationEntry.Metadata is INavigation navigationMetadata && navigationMetadata.ForeignKey == foreignKey) ?? + NavigationEntries.FirstOrDefault(x => x.NavigationEntry.Metadata is ISkipNavigation skipNavigationMetadata && skipNavigationMetadata.ForeignKey == foreignKey); foreach (var navigationEntry in NavigationEntries) { if (IsModified || EntityEntry.State == EntityState.Modified || - navigationEntry.IsModified || - navigationEntry.NavigationEntry.IsModified) + navigationEntry.IsModified) { continue; } - var currentValue = AbpNavigationEntry.GetOriginalValue(navigationEntry.NavigationEntry.CurrentValue); + var currentValue = navigationEntry.NavigationEntry.CurrentValue; if (currentValue == null) { continue; } - switch (navigationEntry.OriginalValue) - { - case null: - navigationEntry.OriginalValue = currentValue; - break; - case IEnumerable originalValueCollection when currentValue is IEnumerable currentValueCollection: - { - var existingList = originalValueCollection.Cast().ToList(); - var newList = currentValueCollection.Cast().ToList(); - if (newList.Count > existingList.Count) - { - navigationEntry.OriginalValue = currentValue; - } - break; - } - default: - navigationEntry.OriginalValue = currentValue; - break; + if (navigationEntry.NavigationEntry is CollectionEntry && navigationEntry == entityEntryNavigationEntry) + { + navigationEntry.OriginalValue ??= new List(); + var ls = navigationEntry.OriginalValue.As>(); + ls.Add(entityEntry.Entity); + } + else + { + navigationEntry.OriginalValue = currentValue; } } } @@ -80,7 +73,7 @@ public class AbpNavigationEntry public bool IsModified { get; set; } - public List? OriginalValue { get; set; } + public object? OriginalValue { get; set; } public object? CurrentValue => NavigationEntry.CurrentValue; @@ -88,21 +81,11 @@ public class AbpNavigationEntry { NavigationEntry = navigationEntry; Name = name; - OriginalValue = GetOriginalValue(navigationEntry.CurrentValue); - } - - public static List? GetOriginalValue(object? currentValue) - { - if (currentValue is null) + if (navigationEntry.CurrentValue != null) { - return null; + OriginalValue = navigationEntry is CollectionEntry collection + ? collection.CurrentValue!.Cast().ToList() + : navigationEntry.CurrentValue; } - - if (currentValue is IEnumerable enumerable) - { - return enumerable.Cast().ToList(); - } - - return new List { currentValue }; } } From c5ceb03e5851239bb5948c9a798814d23e5bf4e6 Mon Sep 17 00:00:00 2001 From: maliming Date: Sat, 19 Jul 2025 16:36:14 +0800 Subject: [PATCH 2/7] Fix collection navigation entry comparison logic --- .../EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 index 00b38b289e..7cb16ef1bb 100644 --- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs +++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs @@ -1,4 +1,5 @@ using System; +using System.Collections; using System.Collections.Generic; using System.Linq; using Microsoft.EntityFrameworkCore; @@ -51,8 +52,12 @@ public class AbpEntityEntry continue; } - if (navigationEntry.NavigationEntry is CollectionEntry && navigationEntry == entityEntryNavigationEntry) + if (navigationEntry.NavigationEntry is CollectionEntry) { + if (navigationEntry != entityEntryNavigationEntry) + { + continue; + } navigationEntry.OriginalValue ??= new List(); var ls = navigationEntry.OriginalValue.As>(); ls.Add(entityEntry.Entity); From acec1159c5caa91b74a9213c537be5af94819039 Mon Sep 17 00:00:00 2001 From: maliming <6908465+maliming@users.noreply.github.com> Date: Sat, 19 Jul 2025 18:01:46 +0800 Subject: [PATCH 3/7] Prevent duplicate entities in navigation entry list --- .../Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 7cb16ef1bb..a759ef4ea7 100644 --- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs +++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs @@ -60,7 +60,7 @@ public class AbpEntityEntry } navigationEntry.OriginalValue ??= new List(); var ls = navigationEntry.OriginalValue.As>(); - ls.Add(entityEntry.Entity); + ls.AddIfNotContains(entityEntry.Entity); } else { From 2f452931ec2e3f1da9f63e0ee7aea770d7e405fe Mon Sep 17 00:00:00 2001 From: maliming Date: Sun, 20 Jul 2025 10:14:23 +0800 Subject: [PATCH 4/7] Refactor navigation entry update logic in change tracker --- .../AbpEfCoreNavigationHelper.cs | 18 ++++--- .../ChangeTrackers/AbpEntityEntry.cs | 51 ++++++++----------- 2 files changed, 30 insertions(+), 39 deletions(-) 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 1338e68400..39835569cb 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 @@ -75,9 +75,12 @@ public class AbpEfCoreNavigationHelper : ITransientDependency continue; } - if (checkEntityEntryState && entityEntry.State == EntityState.Unchanged) + 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 && checkEntityEntryState && entityEntry.State == EntityState.Unchanged) { - abpEntityEntry.UpdateNavigationEntries(entityEntry, foreignKey); + abpEntityEntry.UpdateNavigation(entityEntry, navigationEntry); } if (!abpEntityEntry.IsModified && (!checkEntityEntryState || IsEntityEntryChanged(entityEntry))) @@ -86,8 +89,6 @@ public class AbpEfCoreNavigationHelper : ITransientDependency DetectChanges(abpEntityEntry.EntityEntry, false); } - 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 && IsEntityEntryChanged(entityEntry)) { navigationEntry.IsModified = true; @@ -119,9 +120,12 @@ public class AbpEfCoreNavigationHelper : ITransientDependency continue; } - if (checkEntityEntryState && entityEntry.State == EntityState.Unchanged) + 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 && checkEntityEntryState && entityEntry.State == EntityState.Unchanged) { - abpEntityEntry.UpdateNavigationEntries(entityEntry, inverseForeignKey); + abpEntityEntry.UpdateNavigation(entityEntry, navigationEntry); } if (!abpEntityEntry.IsModified && (!checkEntityEntryState || IsEntityEntryChanged(entityEntry))) @@ -130,8 +134,6 @@ public class AbpEfCoreNavigationHelper : ITransientDependency DetectChanges(abpEntityEntry.EntityEntry, false); } - 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 && (!checkEntityEntryState || IsEntityEntryChanged(entityEntry))) { navigationEntry.IsModified = true; 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 index a759ef4ea7..6083e7d790 100644 --- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs +++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs @@ -1,10 +1,8 @@ using System; -using System.Collections; using System.Collections.Generic; using System.Linq; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.ChangeTracking; -using Microsoft.EntityFrameworkCore.Metadata; namespace Volo.Abp.EntityFrameworkCore.ChangeTrackers; @@ -33,39 +31,30 @@ public class AbpEntityEntry NavigationEntries = EntityEntry.Navigations.Select(x => new AbpNavigationEntry(x, x.Metadata.Name)).ToList(); } - public void UpdateNavigationEntries(EntityEntry entityEntry, IForeignKey foreignKey) + public void UpdateNavigation(EntityEntry entityEntry, AbpNavigationEntry navigationEntry) { - var entityEntryNavigationEntry = NavigationEntries.FirstOrDefault(x => x.NavigationEntry.Metadata is INavigation navigationMetadata && navigationMetadata.ForeignKey == foreignKey) ?? - NavigationEntries.FirstOrDefault(x => x.NavigationEntry.Metadata is ISkipNavigation skipNavigationMetadata && skipNavigationMetadata.ForeignKey == foreignKey); - foreach (var navigationEntry in NavigationEntries) + if (IsModified || + EntityEntry.State == EntityState.Modified || + navigationEntry.IsModified) { - if (IsModified || - EntityEntry.State == EntityState.Modified || - navigationEntry.IsModified) - { - continue; - } + return; + } - var currentValue = navigationEntry.NavigationEntry.CurrentValue; - if (currentValue == null) - { - continue; - } + var currentValue = navigationEntry.NavigationEntry.CurrentValue; + if (currentValue == null) + { + return; + } - if (navigationEntry.NavigationEntry is CollectionEntry) - { - if (navigationEntry != entityEntryNavigationEntry) - { - continue; - } - navigationEntry.OriginalValue ??= new List(); - var ls = navigationEntry.OriginalValue.As>(); - ls.AddIfNotContains(entityEntry.Entity); - } - else - { - navigationEntry.OriginalValue = currentValue; - } + if (navigationEntry.NavigationEntry is CollectionEntry) + { + navigationEntry.OriginalValue ??= new List(); + var ls = navigationEntry.OriginalValue.As>(); + ls.AddIfNotContains(entityEntry.Entity); + } + else + { + navigationEntry.OriginalValue = currentValue; } } } From 9a8732d076266021ecee9f9666afd9175e534fbf Mon Sep 17 00:00:00 2001 From: maliming Date: Sun, 20 Jul 2025 10:31:04 +0800 Subject: [PATCH 5/7] Refactor handling of OriginalValue in AbpEntityEntry --- .../ChangeTrackers/AbpEntityEntry.cs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) 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 index 6083e7d790..5ffe3278c3 100644 --- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs +++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs @@ -48,9 +48,7 @@ public class AbpEntityEntry if (navigationEntry.NavigationEntry is CollectionEntry) { - navigationEntry.OriginalValue ??= new List(); - var ls = navigationEntry.OriginalValue.As>(); - ls.AddIfNotContains(entityEntry.Entity); + navigationEntry.OriginalValue!.As>().Add(entityEntry.Entity); } else { @@ -75,11 +73,9 @@ public class AbpNavigationEntry { NavigationEntry = navigationEntry; Name = name; - if (navigationEntry.CurrentValue != null) + if (navigationEntry.CurrentValue != null ) { - OriginalValue = navigationEntry is CollectionEntry collection - ? collection.CurrentValue!.Cast().ToList() - : navigationEntry.CurrentValue; + OriginalValue = navigationEntry is CollectionEntry ? new List() : navigationEntry.CurrentValue; } } } From fb09742e8c77c1ef4322a61c3b09a0596b764e71 Mon Sep 17 00:00:00 2001 From: maliming Date: Sun, 20 Jul 2025 10:31:55 +0800 Subject: [PATCH 6/7] Fix null reference in navigation entry original value --- .../Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 index 5ffe3278c3..194132d001 100644 --- a/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs +++ b/framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/ChangeTrackers/AbpEntityEntry.cs @@ -48,7 +48,8 @@ public class AbpEntityEntry if (navigationEntry.NavigationEntry is CollectionEntry) { - navigationEntry.OriginalValue!.As>().Add(entityEntry.Entity); + navigationEntry.OriginalValue ??= new List(); + navigationEntry.OriginalValue.As>().Add(entityEntry.Entity); } else { From 6fc6d0dc465368c66d729ffb44a9235795e53032 Mon Sep 17 00:00:00 2001 From: maliming Date: Mon, 21 Jul 2025 11:07:41 +0800 Subject: [PATCH 7/7] Remove unused CancellationToken from test --- .../EntityFrameworkCore/AbpEfCoreNavigationHelper_Tests.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/EntityFrameworkCore/AbpEfCoreNavigationHelper_Tests.cs b/framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/EntityFrameworkCore/AbpEfCoreNavigationHelper_Tests.cs index 578bc94417..9d10af9705 100644 --- a/framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/EntityFrameworkCore/AbpEfCoreNavigationHelper_Tests.cs +++ b/framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/EntityFrameworkCore/AbpEfCoreNavigationHelper_Tests.cs @@ -1,7 +1,6 @@ using System; using System.Diagnostics; using System.Linq; -using System.Threading; using System.Threading.Tasks; using Shouldly; using Volo.Abp.Domain.Repositories; @@ -84,11 +83,8 @@ public class AbpEfCoreNavigationHelper_Tests : EntityFrameworkCoreTestBase stopWatch.Stop(); stopWatch.Elapsed.ShouldBeLessThan(batchUpdateTime); - - var cancellationTokenSource = new CancellationTokenSource(); - cancellationTokenSource.CancelAfter(batchUpdateTime); stopWatch.Restart(); - var blog = await _blogRepository.GetAsync(blogId, cancellationToken: cancellationTokenSource.Token); + var blog = await _blogRepository.GetAsync(blogId); blog.BlogPosts.Count.ShouldBe(5 * 1000 + 1); stopWatch.Stop(); stopWatch.Elapsed.ShouldBeLessThan(queryTime);