Merge pull request #17998 from htve/performance_multiple_attach_on_Update_with_EfCore

performance: multiple attach on Update with EfCore

framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/Domain/Repositories/EntityFrameworkCore/EfCoreRepository.cs

@@ -161,16 +161,18 @@ public class EfCoreRepository<TDbContext, TEntity> : RepositoryBase<TEntity>, IE
     {
         var dbContext = await GetDbContextAsync();
 
-        dbContext.Attach(entity);
-
-        var updatedEntity = dbContext.Update(entity).Entity;
+        if (dbContext.Set<TEntity>().Local.All(e => e != entity))
+        {
+            dbContext.Set<TEntity>().Attach(entity);
+            dbContext.Update(entity);
+        }
 
         if (autoSave)
         {
             await dbContext.SaveChangesAsync(GetCancellationToken(cancellationToken));
         }
 
-        return updatedEntity;
+        return entity;
     }
 
     public async override Task UpdateManyAsync(IEnumerable<TEntity> entities, bool autoSave = false, CancellationToken cancellationToken = default)

framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/EntityHistory/EntityHistoryHelper.cs

@@ -85,8 +85,10 @@ public class EntityHistoryHelper : IEntityHistoryHelper, ITransientDependency
             case EntityState.Modified:
                 changeType = IsDeleted(entityEntry) ? EntityChangeType.Deleted : EntityChangeType.Updated;
                 break;
-            case EntityState.Detached:
             case EntityState.Unchanged:
+                changeType = EntityChangeType.Updated; // Navigation property changes.
+                break;
+            case EntityState.Detached:
             default:
                 return null;
         }
@@ -184,6 +186,21 @@ public class EntityHistoryHelper : IEntityHistoryHelper, ITransientDependency
             }
         }
 
+        if (entityEntry.State == EntityState.Unchanged)
+        {
+            foreach (var navigation in entityEntry.Navigations)
+            {
+                if (navigation.IsModified || (navigation is ReferenceEntry && navigation.As<ReferenceEntry>().TargetEntry?.State == EntityState.Modified))
+                {
+                    propertyChanges.Add(new EntityPropertyChangeInfo
+                    {
+                        PropertyName = navigation.Metadata.Name,
+                        PropertyTypeFullName = navigation.Metadata.ClrType.GetFirstGenericArgumentIfNullable().FullName!
+                    });
+                }
+            }
+        }
+
         return propertyChanges;
     }
 
@@ -205,12 +222,26 @@ public class EntityHistoryHelper : IEntityHistoryHelper, ITransientDependency
 
     protected virtual bool ShouldSaveEntityHistory(EntityEntry entityEntry, bool defaultValue = false)
     {
-        if (entityEntry.State == EntityState.Detached ||
-            entityEntry.State == EntityState.Unchanged)
+        if (entityEntry.State == EntityState.Detached)
         {
             return false;
         }
 
+        if (entityEntry.State == EntityState.Unchanged)
+        {
+            if (entityEntry.Navigations.Any(navigationEntry => navigationEntry.IsModified))
+            {
+                return true;
+            }
+
+            if (entityEntry.Navigations.Where(x => x is ReferenceEntry).Cast<ReferenceEntry>().Any(x => x.TargetEntry != null && x.TargetEntry.State == EntityState.Modified))
+            {
+                return true;
+            }
+
+            return false;
+        }
+
         var entityType = entityEntry.Metadata.ClrType;
 
         if (!EntityHelper.IsEntity(entityType) && !EntityHelper.IsValueObject(entityType))

framework/test/Volo.Abp.Auditing.Tests/Volo/Abp/Auditing/App/Entities/AppEntityWithNavigations.cs

@@ -0,0 +1,51 @@
+using System;
+using System.Collections.Generic;
+using Volo.Abp.Domain.Entities;
+
+namespace Volo.Abp.Auditing.App.Entities;
+
+[Audited]
+public class AppEntityWithNavigations : AggregateRoot<Guid>
+{
+    protected AppEntityWithNavigations()
+    {
+
+    }
+
+    public AppEntityWithNavigations(Guid id, string name)
+        : base(id)
+    {
+        Name = name;
+        FullName = name;
+    }
+
+    public string Name { get; set; }
+
+    public string FullName { get; set; }
+
+    public virtual AppEntityWithNavigationChildOneToOne OneToOne { get; set; }
+
+    public virtual List<AppEntityWithNavigationChildOneToMany> OneToMany { get; set; }
+
+    public virtual List<AppEntityWithNavigationChildManyToMany> ManyToMany { get; set; }
+}
+
+[Audited]
+public class AppEntityWithNavigationChildOneToOne : Entity<Guid>
+{
+    public string ChildName { get; set; }
+}
+
+[Audited]
+public class AppEntityWithNavigationChildOneToMany : Entity<Guid>
+{
+    public Guid AppEntityWithNavigationId { get; set; }
+
+    public string ChildName { get; set; }
+}
+
+[Audited]
+public class AppEntityWithNavigationChildManyToMany : Entity<Guid>
+{
+    public string ChildName { get; set; }
+}

framework/test/Volo.Abp.Auditing.Tests/Volo/Abp/Auditing/App/EntityFrameworkCore/AbpAuditingTestDbContext.cs

@@ -27,6 +27,8 @@ public class AbpAuditingTestDbContext : AbpDbContext<AbpAuditingTestDbContext>
 
     public DbSet<AppEntityWithValueObject> AppEntityWithValueObject { get; set; }
 
+    public DbSet<AppEntityWithNavigations> AppEntityWithNavigations { get; set; }
+
     public AbpAuditingTestDbContext(DbContextOptions<AbpAuditingTestDbContext> options)
         : base(options)
     {
@@ -42,5 +44,14 @@ public class AbpAuditingTestDbContext : AbpDbContext<AbpAuditingTestDbContext>
             b.ConfigureByConvention();
             b.OwnsOne(v => v.AppEntityWithValueObjectAddress);
         });
+
+        modelBuilder.Entity<AppEntityWithNavigations>(b =>
+        {
+            b.ConfigureByConvention();
+            b.HasOne(x => x.OneToOne).WithOne().HasForeignKey<AppEntityWithNavigationChildOneToOne>(x => x.Id);
+            b.HasMany(x => x.OneToMany).WithOne().HasForeignKey(x => x.AppEntityWithNavigationId);
+            b.HasMany(x => x.ManyToMany).WithMany();
+        });
+
     }
 }

framework/test/Volo.Abp.Auditing.Tests/Volo/Abp/Auditing/Auditing_Tests.cs

@@ -411,14 +411,172 @@ public class Auditing_Tests : AbpAuditingTestBase
 #pragma warning disable 4014
         AuditingStore.Received().SaveAsync(Arg.Is<AuditLogInfo>(x => x.EntityChanges.Count == 2 &&
                                                                      x.EntityChanges[0].ChangeType == EntityChangeType.Updated &&
-                                                                     x.EntityChanges[0].EntityTypeFullName == typeof(AppEntityWithValueObject).FullName &&
+                                                                     x.EntityChanges[0].EntityTypeFullName == typeof(AppEntityWithValueObjectAddress).FullName &&
+                                                                     x.EntityChanges[0].PropertyChanges.Count == 1 &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].PropertyName == nameof(AppEntityWithValueObjectAddress.Country) &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].OriginalValue == "\"England\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].NewValue == "\"Germany\"" &&
+
+                                                                     x.EntityChanges[1].ChangeType == EntityChangeType.Updated &&
+                                                                     x.EntityChanges[1].EntityTypeFullName == typeof(AppEntityWithValueObject).FullName &&
+                                                                     x.EntityChanges[1].PropertyChanges.Count == 1 &&
+                                                                     x.EntityChanges[1].PropertyChanges[0].PropertyName == nameof(AppEntityWithValueObject.AppEntityWithValueObjectAddress)));
+
+#pragma warning restore 4014
+
+        using (var scope = _auditingManager.BeginScope())
+        {
+            using (var uow = _unitOfWorkManager.Begin())
+            {
+                var entity = await repository.GetAsync(entityId);
+
+                entity.AppEntityWithValueObjectAddress = null;
+
+                await repository.UpdateAsync(entity);
+                await uow.CompleteAsync();
+                await scope.SaveAsync();
+            }
+        }
+
+#pragma warning disable 4014
+        AuditingStore.Received().SaveAsync(Arg.Is<AuditLogInfo>(x => x.EntityChanges.Count == 2 &&
+                                                                     x.EntityChanges[0].ChangeType == EntityChangeType.Updated &&
+                                                                     x.EntityChanges[0].EntityTypeFullName == typeof(AppEntityWithValueObjectAddress).FullName &&
+                                                                     x.EntityChanges[0].PropertyChanges.Count == 1 &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].PropertyName == nameof(AppEntityWithValueObjectAddress.Country) &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].OriginalValue == "\"England\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].NewValue == "\"Germany\"" &&
+
+                                                                     x.EntityChanges[1].ChangeType == EntityChangeType.Updated &&
+                                                                     x.EntityChanges[1].EntityTypeFullName == typeof(AppEntityWithValueObject).FullName));
+
+#pragma warning restore 4014
+    }
+
+    [Fact]
+    public virtual async Task Should_Write_AuditLog_For_Navigations_Changes()
+    {
+        var entityId = Guid.NewGuid();
+        var repository = ServiceProvider.GetRequiredService<IBasicRepository<AppEntityWithNavigations, Guid>>();
+        await repository.InsertAsync(new AppEntityWithNavigations(entityId, "test name"));
+
+        using (var scope = _auditingManager.BeginScope())
+        {
+            using (var uow = _unitOfWorkManager.Begin())
+            {
+                var entity = await repository.GetAsync(entityId);
+
+                entity.FullName = "test full name";
+
+                await repository.UpdateAsync(entity);
+
+                await uow.CompleteAsync();
+                await scope.SaveAsync();
+            }
+        }
+
+#pragma warning disable 4014
+        AuditingStore.Received().SaveAsync(Arg.Is<AuditLogInfo>(x => x.EntityChanges.Count == 1 &&
+                                                                     x.EntityChanges[0].ChangeType == EntityChangeType.Updated &&
+                                                                     x.EntityChanges[0].EntityTypeFullName == typeof(AppEntityWithNavigations).FullName &&
+                                                                     x.EntityChanges[0].PropertyChanges.Count == 1 &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].OriginalValue == "\"test name\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].NewValue == "\"test full name\"" &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].PropertyName == nameof(AppEntityWithNavigations.FullName) &&
+                                                                     x.EntityChanges[0].PropertyChanges[0].PropertyTypeFullName == typeof(string).FullName));
+#pragma warning restore 4014
 
+        using (var scope = _auditingManager.BeginScope())
+        {
+            using (var uow = _unitOfWorkManager.Begin())
+            {
+                var entity = await repository.GetAsync(entityId);
+
+                entity.OneToOne = new AppEntityWithNavigationChildOneToOne
+                {
+                    ChildName = "ChildName"
+                };
+
+                await repository.UpdateAsync(entity);
+
+                await uow.CompleteAsync();
+                await scope.SaveAsync();
+            }
+        }
+
+#pragma warning disable 4014
+        AuditingStore.Received().SaveAsync(Arg.Is<AuditLogInfo>(x => x.EntityChanges.Count == 2 &&
+                                                                     x.EntityChanges[0].ChangeType == EntityChangeType.Created &&
+                                                                     x.EntityChanges[0].EntityTypeFullName == typeof(AppEntityWithNavigationChildOneToOne).FullName &&
+                                                                     x.EntityChanges[1].ChangeType == EntityChangeType.Updated &&
+                                                                     x.EntityChanges[1].EntityTypeFullName == typeof(AppEntityWithNavigations).FullName &&
+                                                                     x.EntityChanges[1].PropertyChanges.Count == 1 &&
+                                                                     x.EntityChanges[1].PropertyChanges[0].PropertyName == nameof(AppEntityWithNavigations.OneToOne) &&
+                                                                     x.EntityChanges[1].PropertyChanges[0].PropertyTypeFullName == typeof(AppEntityWithNavigationChildOneToOne).FullName));
+#pragma warning restore 4014
+
+        using (var scope = _auditingManager.BeginScope())
+        {
+            using (var uow = _unitOfWorkManager.Begin())
+            {
+                var entity = await repository.GetAsync(entityId);
+
+                entity.OneToMany = new List<AppEntityWithNavigationChildOneToMany>()
+                {
+                    new AppEntityWithNavigationChildOneToMany
+                    {
+                        AppEntityWithNavigationId = entity.Id,
+                        ChildName = "ChildName1"
+                    }
+                };
+
+                await repository.UpdateAsync(entity);
+                await uow.CompleteAsync();
+                await scope.SaveAsync();
+            }
+        }
+
+#pragma warning disable 4014
+        AuditingStore.Received().SaveAsync(Arg.Is<AuditLogInfo>(x => x.EntityChanges.Count == 2 &&
+                                                                     x.EntityChanges[0].ChangeType == EntityChangeType.Created &&
+                                                                     x.EntityChanges[0].EntityTypeFullName == typeof(AppEntityWithNavigationChildOneToMany).FullName &&
+                                                                     x.EntityChanges[1].ChangeType == EntityChangeType.Updated &&
+                                                                     x.EntityChanges[1].EntityTypeFullName == typeof(AppEntityWithNavigations).FullName &&
+                                                                     x.EntityChanges[1].PropertyChanges.Count == 1 &&
+                                                                     x.EntityChanges[1].PropertyChanges[0].PropertyName == nameof(AppEntityWithNavigations.OneToMany) &&
+                                                                     x.EntityChanges[1].PropertyChanges[0].PropertyTypeFullName == typeof(List<AppEntityWithNavigationChildOneToMany>).FullName));
+
+#pragma warning restore 4014
+
+        using (var scope = _auditingManager.BeginScope())
+        {
+            using (var uow = _unitOfWorkManager.Begin())
+            {
+                var entity = await repository.GetAsync(entityId);
+
+                entity.ManyToMany = new List<AppEntityWithNavigationChildManyToMany>()
+                {
+                    new AppEntityWithNavigationChildManyToMany
+                    {
+                        ChildName = "ChildName1"
+                    }
+                };
+
+                await repository.UpdateAsync(entity);
+                await uow.CompleteAsync();
+                await scope.SaveAsync();
+            }
+        }
+
+#pragma warning disable 4014
+        AuditingStore.Received().SaveAsync(Arg.Is<AuditLogInfo>(x => x.EntityChanges.Count == 2 &&
+                                                                     x.EntityChanges[0].ChangeType == EntityChangeType.Created &&
+                                                                     x.EntityChanges[0].EntityTypeFullName == typeof(AppEntityWithNavigationChildManyToMany).FullName &&
                                                                      x.EntityChanges[1].ChangeType == EntityChangeType.Updated &&
-                                                                     x.EntityChanges[1].EntityTypeFullName == typeof(AppEntityWithValueObjectAddress).FullName &&
+                                                                     x.EntityChanges[1].EntityTypeFullName == typeof(AppEntityWithNavigations).FullName &&
                                                                      x.EntityChanges[1].PropertyChanges.Count == 1 &&
-                                                                     x.EntityChanges[1].PropertyChanges[0].PropertyName == nameof(AppEntityWithValueObjectAddress.Country) &&
-                                                                     x.EntityChanges[1].PropertyChanges[0].OriginalValue == "\"England\"" &&
-                                                                     x.EntityChanges[1].PropertyChanges[0].NewValue == "\"Germany\""));
+                                                                     x.EntityChanges[1].PropertyChanges[0].PropertyName == nameof(AppEntityWithNavigations.ManyToMany) &&
+                                                                     x.EntityChanges[1].PropertyChanges[0].PropertyTypeFullName == typeof(List<AppEntityWithNavigationChildManyToMany>).FullName));
 
 #pragma warning restore 4014
     }