Merge pull request #24104 from abpframework/onlyForeignKeyModifiedEntity

Improve foreign key change detection in domain events

.github/workflows/build-and-test.yml

@@ -51,11 +51,14 @@ jobs:
     timeout-minutes: 50
     if: ${{ !github.event.pull_request.draft }}
     steps:
+    - uses: jlumbroso/free-disk-space@main
+    - uses: PSModule/install-powershell@v1
+      with:
+        Version: latest
     - uses: actions/checkout@v2
     - uses: actions/setup-dotnet@master
       with:
         dotnet-version: 9.0.100
-
     - name: Build All
       run: ./build-all.ps1
       working-directory: ./build

framework/src/Volo.Abp.EntityFrameworkCore/Volo/Abp/EntityFrameworkCore/AbpDbContext.cs

@@ -268,7 +268,7 @@ public abstract class AbpDbContext<TDbContext> : DbContext, IAbpEfCoreDbContext,
             if (EntityChangeOptions.Value.PublishEntityUpdatedEventWhenNavigationChanges)
             {
                 var ignoredEntity = EntityChangeOptions.Value.IgnoredNavigationEntitySelectors.Any(selector => selector.Predicate(entityEntry.Entity.GetType()));
-                var onlyForeignKeyModifiedEntity = entityEntry.State == EntityState.Modified && entityEntry.Properties.Where(x => x.IsModified).All(x => x.Metadata.IsForeignKey());
+                var onlyForeignKeyModifiedEntity = entityEntry.State == EntityState.Modified && IsOnlyForeignKeysModified(entityEntry);
                 if ((entityEntry.State == EntityState.Unchanged && ignoredEntity) || onlyForeignKeyModifiedEntity && ignoredEntity)
                 {
                     continue;
@@ -292,7 +292,7 @@ public abstract class AbpDbContext<TDbContext> : DbContext, IAbpEfCoreDbContext,
             }
             else if (entityEntry.Properties.Any(x => x.IsModified && (x.Metadata.ValueGenerated == ValueGenerated.Never || x.Metadata.ValueGenerated == ValueGenerated.OnAdd)))
             {
-                if (entityEntry.Properties.Where(x => x.IsModified).All(x => x.Metadata.IsForeignKey()))
+                if (IsOnlyForeignKeysModified(entityEntry))
                 {
                     // Skip `PublishEntityDeletedEvent/PublishEntityUpdatedEvent` if only foreign keys have changed.
                     break;
@@ -428,7 +428,7 @@ public abstract class AbpDbContext<TDbContext> : DbContext, IAbpEfCoreDbContext,
             case EntityState.Modified:
                 if (entry.Properties.Any(x => x.IsModified && (x.Metadata.ValueGenerated == ValueGenerated.Never || x.Metadata.ValueGenerated == ValueGenerated.OnAdd)))
                 {
-                    if (entry.Properties.Where(x => x.IsModified).All(x => x.Metadata.IsForeignKey()))
+                    if (IsOnlyForeignKeysModified(entry))
                     {
                         // Skip `PublishEntityDeletedEvent/PublishEntityUpdatedEvent` if only foreign keys have changed.
                         break;
@@ -472,6 +472,12 @@ public abstract class AbpDbContext<TDbContext> : DbContext, IAbpEfCoreDbContext,
         }
     }
 
+    protected virtual bool IsOnlyForeignKeysModified(EntityEntry entry)
+    {
+        return entry.Properties.Where(x => x.IsModified).All(x => x.Metadata.IsForeignKey() &&
+                                         (x.CurrentValue == null || x.OriginalValue?.ToString() == x.CurrentValue?.ToString()));
+    }
+
     protected virtual void HandlePropertiesBeforeSave()
     {
         var entries = ChangeTracker.Entries().ToList();

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

@@ -28,7 +28,7 @@ public class AbpAuditingTestDbContext : AbpDbContext<AbpAuditingTestDbContext>
     public DbSet<AppEntityWithValueObject> AppEntityWithValueObject { get; set; }
 
     public DbSet<AppEntityWithNavigations> AppEntityWithNavigations { get; set; }
-
+    public DbSet<AppEntityWithNavigationChildOneToMany> AppEntityWithNavigationChildOneToMany { get; set; }
     public AbpAuditingTestDbContext(DbContextOptions<AbpAuditingTestDbContext> options)
         : base(options)
     {

framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/EntityFrameworkCore/AbpEntityFrameworkCoreTestModule.cs

@@ -58,6 +58,11 @@ public class AbpEntityFrameworkCoreTestModule : AbpModule
             {
                 opt.DefaultWithDetailsFunc = q => q.Include(p => p.BlogPosts);
             });
+
+            options.Entity<AppEntityWithNavigationsForeign>(opt =>
+            {
+                opt.DefaultWithDetailsFunc = q => q.Include(p => p.OneToMany);
+            });
         });
 
         context.Services.AddAbpDbContext<HostTestAppDbContext>(options =>

framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/EntityFrameworkCore/DomainEvents/DomainEvents_Tests.cs

@@ -48,6 +48,7 @@ public class AbpEntityChangeOptions_DomainEvents_IgnoreEntityChangeSelectorList_
 public class AbpEfCoreDomainEvents_Tests : EntityFrameworkCoreTestBase
 {
     protected readonly IRepository<AppEntityWithNavigations, Guid> AppEntityWithNavigationsRepository;
+    protected readonly IRepository<AppEntityWithNavigationChildOneToMany,  Guid> AppEntityWithNavigationChildOneToManyRepository;
     protected readonly ILocalEventBus LocalEventBus;
     protected readonly IRepository<Person, Guid> PersonRepository;
     protected bool _loadEntityWithoutDetails = false;
@@ -55,6 +56,7 @@ public class AbpEfCoreDomainEvents_Tests : EntityFrameworkCoreTestBase
     public AbpEfCoreDomainEvents_Tests()
     {
         AppEntityWithNavigationsRepository = GetRequiredService<IRepository<AppEntityWithNavigations, Guid>>();
+        AppEntityWithNavigationChildOneToManyRepository = GetRequiredService<IRepository<AppEntityWithNavigationChildOneToMany, Guid>>();
         LocalEventBus = GetRequiredService<ILocalEventBus>();
         PersonRepository = GetRequiredService<IRepository<Person, Guid>>();
     }
@@ -357,6 +359,22 @@ public class AbpEfCoreDomainEvents_Tests : EntityFrameworkCoreTestBase
         var entityId = Guid.NewGuid();
 
         await AppEntityWithNavigationsRepository.InsertAsync(new AppEntityWithNavigations(entityId, "TestEntity")
+        {
+            OneToMany = new List<AppEntityWithNavigationChildOneToMany>()
+            {
+                new AppEntityWithNavigationChildOneToMany(Guid.NewGuid())
+                {
+                    ChildName = "ChildName1"
+                },
+                new AppEntityWithNavigationChildOneToMany(Guid.NewGuid())
+                {
+                    ChildName = "ChildName2"
+                }
+            }
+        });
+
+        var entityId2 = Guid.NewGuid();
+        await AppEntityWithNavigationsRepository.InsertAsync(new AppEntityWithNavigations(entityId2, "TestEntity")
         {
             OneToMany = new List<AppEntityWithNavigationChildOneToMany>()
             {
@@ -367,6 +385,33 @@ public class AbpEfCoreDomainEvents_Tests : EntityFrameworkCoreTestBase
             }
         });
 
+        var oneToManyEntity = Guid.NewGuid();
+        await AppEntityWithNavigationChildOneToManyRepository.InsertAsync(
+            new AppEntityWithNavigationChildOneToMany(oneToManyEntity)
+            {
+                AppEntityWithNavigationId = entityId,
+            });
+
+        LocalEventBus.Subscribe<EntityUpdatedEventData<AppEntityWithNavigationChildOneToMany>>(data =>
+        {
+            data.Entity.AppEntityWithNavigationId.ShouldBe(entityId2);
+            return Task.CompletedTask;
+        });
+
+        using (var scope = ServiceProvider.CreateScope())
+        {
+            var uowManager = scope.ServiceProvider.GetRequiredService<IUnitOfWorkManager>();
+            using (var uow = uowManager.Begin())
+            {
+                var entity = await AppEntityWithNavigationChildOneToManyRepository.GetAsync(oneToManyEntity);
+
+                entity.AppEntityWithNavigationId = entityId2;
+                await AppEntityWithNavigationChildOneToManyRepository.UpdateAsync(entity);
+
+                await uow.CompleteAsync();
+            }
+        }
+
         var entityUpdatedEventTriggered = false;
 
         LocalEventBus.Subscribe<EntityUpdatedEventData<AppEntityWithNavigations>>(data =>
@@ -375,6 +420,11 @@ public class AbpEfCoreDomainEvents_Tests : EntityFrameworkCoreTestBase
             return Task.CompletedTask;
         });
 
+        LocalEventBus.Subscribe<EntityUpdatedEventData<AppEntityWithNavigationChildOneToMany>>(data =>
+        {
+            throw new Exception("Should not trigger this event");
+        });
+
         using (var scope = ServiceProvider.CreateScope())
         {
             var uowManager = scope.ServiceProvider.GetRequiredService<IUnitOfWorkManager>();

framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/EntityFrameworkCore/TestMigrationsDbContext.cs

@@ -27,6 +27,7 @@ public class TestMigrationsDbContext : AbpDbContext<TestMigrationsDbContext>
     public DbSet<Category> Categories { get; set; }
 
     public DbSet<AppEntityWithNavigations> AppEntityWithNavigations { get; set; }
+    public DbSet<AppEntityWithNavigationChildOneToMany> AppEntityWithNavigationChildOneToMany { get; set; }
 
     public DbSet<AppEntityWithNavigationsForeign> AppEntityWithNavigationsForeign { get; set; }
 
@@ -81,7 +82,12 @@ public class TestMigrationsDbContext : AbpDbContext<TestMigrationsDbContext>
             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(x => x.ManyToMany).UsingEntity<AppEntityWithNavigationsAndAppEntityWithNavigationChildManyToMany>();
-            b.HasOne<AppEntityWithNavigationsForeign>().WithMany().HasForeignKey(x => x.AppEntityWithNavigationForeignId).IsRequired(false);
+        });
+
+        modelBuilder.Entity<AppEntityWithNavigationsForeign>(b =>
+        {
+            b.ConfigureByConvention();
+            b.HasMany(x => x.OneToMany).WithOne().HasForeignKey(x => x.AppEntityWithNavigationForeignId);
         });
 
         modelBuilder.Entity<AppEntityWithNavigationChildOneToOne>(b =>

framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/TestApp/EntityFrameworkCore/TestAppDbContext.cs

@@ -34,6 +34,7 @@ public class TestAppDbContext : AbpDbContext<TestAppDbContext>, IThirdDbContext,
     public DbSet<Category> Categories { get; set; }
 
     public DbSet<AppEntityWithNavigations> AppEntityWithNavigations { get; set; }
+    public DbSet<AppEntityWithNavigationChildOneToMany> AppEntityWithNavigationChildOneToMany { get; set; }
 
     public DbSet<AppEntityWithNavigationsForeign> AppEntityWithNavigationsForeign { get; set; }
 
@@ -107,7 +108,12 @@ public class TestAppDbContext : AbpDbContext<TestAppDbContext>, IThirdDbContext,
             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(x => x.ManyToMany).UsingEntity<AppEntityWithNavigationsAndAppEntityWithNavigationChildManyToMany>();
-            b.HasOne<AppEntityWithNavigationsForeign>().WithMany().HasForeignKey(x => x.AppEntityWithNavigationForeignId).IsRequired(false);
+        });
+
+        modelBuilder.Entity<AppEntityWithNavigationsForeign>(b =>
+        {
+            b.ConfigureByConvention();
+            b.HasMany(x => x.OneToMany).WithOne().HasForeignKey(x => x.AppEntityWithNavigationForeignId);
         });
 
         modelBuilder.Entity<AppEntityWithNavigationChildOneToOne>(b =>

framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Domain/AppEntityWithNavigations.cs

@@ -65,6 +65,17 @@ public class AppEntityWithNavigationChildOneToOneAndOneToOne : Entity<Guid>
 
 public class AppEntityWithNavigationChildOneToMany : Entity<Guid>
 {
+    public AppEntityWithNavigationChildOneToMany()
+    {
+
+    }
+
+    public AppEntityWithNavigationChildOneToMany(Guid id)
+        : base(id)
+    {
+
+    }
+
     public Guid AppEntityWithNavigationId { get; set; }
 
     public string ChildName { get; set; }
@@ -107,4 +118,6 @@ public class AppEntityWithNavigationsForeign : AggregateRoot<Guid>
     }
 
     public string Name { get; set; }
+
+    public virtual List<AppEntityWithNavigations> OneToMany { get; set; }
 }

framework/test/Volo.Abp.TestApp/Volo/Abp/TestApp/Testing/DomainEvents_Tests.cs

@@ -347,10 +347,20 @@ public abstract class AbpEntityChangeOptions_DomainEvents_Tests<TStartupModule>
 
         var entityWithNavigationForeignId = Guid.NewGuid();
         var entityWithNavigationForeignId2 = Guid.NewGuid();
+        var entityWithNavigationForeignId3 = Guid.NewGuid();
         await AppEntityWithNavigationForeignRepository.InsertAsync(new AppEntityWithNavigationsForeign(entityWithNavigationForeignId, "TestEntityWithNavigationForeign"));
         await AppEntityWithNavigationForeignRepository.InsertAsync(new AppEntityWithNavigationsForeign(entityWithNavigationForeignId2, "TestEntityWithNavigationForeign2"));
+        await AppEntityWithNavigationForeignRepository.InsertAsync(new AppEntityWithNavigationsForeign(entityWithNavigationForeignId3, "TestEntityWithNavigationForeign3")
+        {
+            OneToMany = new List<AppEntityWithNavigations>()
+            {
+                new AppEntityWithNavigations(Guid.NewGuid(), "TestEntity2"),
+                new AppEntityWithNavigations(Guid.NewGuid(), "TestEntity3")
+            }
+        });
 
         var entityUpdatedEventTriggered = false;
+        var entityWithNavigationsForeignUpdatedEventTriggered = false;
 
         LocalEventBus.Subscribe<EntityUpdatedEventData<AppEntityWithNavigations>>(data =>
         {
@@ -358,6 +368,12 @@ public abstract class AbpEntityChangeOptions_DomainEvents_Tests<TStartupModule>
             return Task.CompletedTask;
         });
 
+        LocalEventBus.Subscribe<EntityUpdatedEventData<AppEntityWithNavigationsForeign>>(data =>
+        {
+            entityWithNavigationsForeignUpdatedEventTriggered = !entityWithNavigationsForeignUpdatedEventTriggered;
+            return Task.CompletedTask;
+        });
+
         // Test with simple property with foreign key
         await WithUnitOfWorkAsync(async () =>
         {
@@ -368,17 +384,39 @@ public abstract class AbpEntityChangeOptions_DomainEvents_Tests<TStartupModule>
         });
         entityUpdatedEventTriggered.ShouldBeTrue();
 
-        // Test only foreign key changed
+        // Test only foreign key change to null
         entityUpdatedEventTriggered = false;
         await WithUnitOfWorkAsync(async () =>
         {
             var entity = await AppEntityWithNavigationsRepository.GetAsync(entityId);
-            entity.AppEntityWithNavigationForeignId = entityWithNavigationForeignId2;
+            entity.AppEntityWithNavigationForeignId = null;
             await AppEntityWithNavigationsRepository.UpdateAsync(entity);
         });
         entityUpdatedEventTriggered.ShouldBeFalse();
 
+        // Test only foreign key change to new id
+        entityUpdatedEventTriggered = false;
+        await WithUnitOfWorkAsync(async () =>
+        {
+            var entity = await AppEntityWithNavigationsRepository.GetAsync(entityId);
+            entity.AppEntityWithNavigationForeignId = entityWithNavigationForeignId;
+            await AppEntityWithNavigationsRepository.UpdateAsync(entity);
+        });
+        entityUpdatedEventTriggered.ShouldBeTrue();
+
+        // Test only foreign key changed
+        entityWithNavigationsForeignUpdatedEventTriggered = false;
+        await WithUnitOfWorkAsync(async () =>
+        {
+            var entity = await AppEntityWithNavigationForeignRepository.GetAsync(entityWithNavigationForeignId3);
+            entity.OneToMany.ShouldNotBeEmpty();
+            entity.OneToMany.Clear();
+            await AppEntityWithNavigationForeignRepository.UpdateAsync(entity);
+        });
+        entityWithNavigationsForeignUpdatedEventTriggered.ShouldBeFalse();
+
         // Test with simple property with value object
+        entityUpdatedEventTriggered = false;
         await WithUnitOfWorkAsync(async () =>
         {
             var entity = await AppEntityWithNavigationsRepository.GetAsync(entityId);