Merge pull request #25570 from abpframework/liming/fix-soft-delete-dbfunction-bool-literal

Use IsDeleted column TypeMapping in soft-delete DbFunction

framework/src/Volo.Abp.EntityFrameworkCore/Microsoft/Extensions/DependencyInjection/AbpEfCoreModelBuilderExtensions.cs

@@ -21,11 +21,11 @@ public static class AbpEfCoreModelBuilderExtensions
 
                 if (abpEfCoreCurrentDbContext.Context?.DataFilter.IsEnabled<ISoftDelete>() == true)
                 {
-                    // IsDeleted == false
+                    // IsDeleted == false. Use the column's TypeMapping so any ValueConverter is applied to the literal.
                     return new SqlBinaryExpression(
                         ExpressionType.Equal,
                         isDeleted,
-                        new SqlConstantExpression(false, typeof(bool), boolParam.TypeMapping),
+                        new SqlConstantExpression(false, typeof(bool), isDeleted.TypeMapping ?? boolParam.TypeMapping),
                         boolParam.Type,
                         boolParam.TypeMapping);
                 }

framework/test/Volo.Abp.EntityFrameworkCore.Tests/Volo/Abp/EntityFrameworkCore/DataFiltering/SoftDelete_DbFunction_Bool_Literal_Tests.cs

@@ -0,0 +1,48 @@
+using System;
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.Extensions.DependencyInjection;
+using Shouldly;
+using Volo.Abp.Data;
+using Volo.Abp.Domain.Repositories;
+using Volo.Abp.TestApp.Domain;
+using Volo.Abp.TestApp.EntityFrameworkCore;
+using Volo.Abp.TestApp.Testing;
+using Xunit;
+
+namespace Volo.Abp.EntityFrameworkCore.DataFiltering;
+
+// Regression for the soft-delete DbFunction translator hardcoding a bool false literal
+// with the bool TypeMapping instead of routing the literal through the IsDeleted
+// property's ValueConverter. EntityWithIntSoftDelete maps false->5, true->9, so the
+// buggy SQL "is_deleted = 0" matches no row (returns 0) while the correct SQL
+// "is_deleted = 5" matches the not-deleted row (returns 1).
+public class SoftDelete_DbFunction_Bool_Literal_Tests : TestAppTestBase<AbpEntityFrameworkCoreTestModule>
+{
+    private readonly IRepository<EntityWithIntSoftDelete, Guid> _repository;
+    private readonly IDataFilter<ISoftDelete> _softDeleteFilter;
+
+    public SoftDelete_DbFunction_Bool_Literal_Tests()
+    {
+        _repository = GetRequiredService<IRepository<EntityWithIntSoftDelete, Guid>>();
+        _softDeleteFilter = GetRequiredService<IDataFilter<ISoftDelete>>();
+    }
+
+    [Fact]
+    public async Task SoftDelete_Filter_Should_Route_False_Literal_Through_ValueConverter()
+    {
+        await _repository.InsertAsync(new EntityWithIntSoftDelete { Name = "kept" });
+        await _repository.InsertAsync(new EntityWithIntSoftDelete { Name = "removed", IsDeleted = true });
+
+        var visible = await _repository.GetListAsync();
+        visible.Count.ShouldBe(1);
+        visible[0].Name.ShouldBe("kept");
+
+        using (_softDeleteFilter.Disable())
+        {
+            var all = await _repository.GetListAsync();
+            all.Count.ShouldBe(2);
+        }
+    }
+}

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

@@ -27,6 +27,8 @@ public class TestMigrationsDbContext : AbpDbContext<TestMigrationsDbContext>
 
     public DbSet<Category> Categories { get; set; }
 
+    public DbSet<EntityWithIntSoftDelete> EntityWithIntSoftDeletes { get; set; }
+
     public DbSet<AppEntityWithNavigations> AppEntityWithNavigations { get; set; }
     public DbSet<AppEntityWithNavigationChildOneToMany> AppEntityWithNavigationChildOneToMany { get; set; }
 
@@ -104,6 +106,15 @@ public class TestMigrationsDbContext : AbpDbContext<TestMigrationsDbContext>
             b.HasAbpQueryFilter(e => e.Name.StartsWith("abp"));
         });
 
+        modelBuilder.Entity<EntityWithIntSoftDelete>(b =>
+        {
+            b.Property(x => x.IsDeleted)
+                .HasColumnName(EntityWithIntSoftDelete.IsDeletedColumnName)
+                .HasConversion(
+                    v => v ? EntityWithIntSoftDelete.DeletedProviderValue : EntityWithIntSoftDelete.NotDeletedProviderValue,
+                    i => i == EntityWithIntSoftDelete.DeletedProviderValue);
+        });
+
         modelBuilder.Entity<AppEntityWithNavigations>(b =>
         {
             b.ConfigureByConvention();

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

@@ -33,6 +33,8 @@ public class TestAppDbContext : AbpDbContext<TestAppDbContext>, IThirdDbContext,
 
     public DbSet<Category> Categories { get; set; }
 
+    public DbSet<EntityWithIntSoftDelete> EntityWithIntSoftDeletes { get; set; }
+
     public DbSet<AppEntityWithNavigations> AppEntityWithNavigations { get; set; }
     public DbSet<AppEntityWithNavigationChildOneToMany> AppEntityWithNavigationChildOneToMany { get; set; }
 
@@ -129,6 +131,15 @@ public class TestAppDbContext : AbpDbContext<TestAppDbContext>, IThirdDbContext,
             b.HasAbpQueryFilter(e => e.Name.StartsWith("abp"));
         });
 
+        modelBuilder.Entity<EntityWithIntSoftDelete>(b =>
+        {
+            b.Property(x => x.IsDeleted)
+                .HasColumnName(EntityWithIntSoftDelete.IsDeletedColumnName)
+                .HasConversion(
+                    v => v ? EntityWithIntSoftDelete.DeletedProviderValue : EntityWithIntSoftDelete.NotDeletedProviderValue,
+                    i => i == EntityWithIntSoftDelete.DeletedProviderValue);
+        });
+
         modelBuilder.Entity<AppEntityWithNavigations>(b =>
         {
             b.ConfigureByConvention();

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

@@ -0,0 +1,19 @@
+using System;
+using Volo.Abp.Domain.Entities;
+
+namespace Volo.Abp.TestApp.Domain;
+
+// Uses a custom bool<->int converter where false maps to a non-zero value, so the
+// soft-delete DbFunction translator (which hardcodes a bool false literal with the
+// bool TypeMapping) is observably wrong on every provider: the generated SQL
+// compares against 0/FALSE instead of the converter's actual provider value (5).
+public class EntityWithIntSoftDelete : AggregateRoot<Guid>, ISoftDelete
+{
+    public const string IsDeletedColumnName = "is_deleted";
+    public const int NotDeletedProviderValue = 5;
+    public const int DeletedProviderValue = 9;
+
+    public string Name { get; set; }
+
+    public bool IsDeleted { get; set; }
+}