Merge pull request #25568 from abpframework/liming/fix-soft-delete-filter-property-name

Use CLR property name in soft-delete and multi-tenant filters

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

@@ -957,8 +957,8 @@ public abstract class AbpDbContext<TDbContext> : DbContext, IAbpEfCoreDbContext,
 
         if (typeof(ISoftDelete).IsAssignableFrom(typeof(TEntity)))
         {
-            var softDeleteColumnName = entityTypeBuilder.Metadata.FindProperty(nameof(ISoftDelete.IsDeleted))?.GetColumnName() ?? "IsDeleted";
-            expression = e => !IsSoftDeleteFilterEnabled || !EF.Property<bool>(e, softDeleteColumnName);
+            var softDeletePropertyName = entityTypeBuilder.Metadata.FindProperty(nameof(ISoftDelete.IsDeleted))?.Name ?? nameof(ISoftDelete.IsDeleted);
+            expression = e => !IsSoftDeleteFilterEnabled || !EF.Property<bool>(e, softDeletePropertyName);
             if (UseDbFunction())
             {
                 expression = e => AbpEfCoreDataFilterDbFunctionMethods.SoftDeleteFilter(((ISoftDelete)e).IsDeleted, true);
@@ -971,8 +971,8 @@ public abstract class AbpDbContext<TDbContext> : DbContext, IAbpEfCoreDbContext,
 
         if (typeof(IMultiTenant).IsAssignableFrom(typeof(TEntity)))
         {
-            var multiTenantColumnName = entityTypeBuilder.Metadata.FindProperty(nameof(IMultiTenant.TenantId))?.GetColumnName() ?? "TenantId";
-            Expression<Func<TEntity, bool>> multiTenantFilter = e => !IsMultiTenantFilterEnabled || EF.Property<Guid>(e, multiTenantColumnName) == CurrentTenantId;
+            var multiTenantPropertyName = entityTypeBuilder.Metadata.FindProperty(nameof(IMultiTenant.TenantId))?.Name ?? nameof(IMultiTenant.TenantId);
+            Expression<Func<TEntity, bool>> multiTenantFilter = e => !IsMultiTenantFilterEnabled || EF.Property<Guid>(e, multiTenantPropertyName) == CurrentTenantId;
             if (UseDbFunction())
             {
                 multiTenantFilter = e => AbpEfCoreDataFilterDbFunctionMethods.MultiTenantFilter(((IMultiTenant)e).TenantId, CurrentTenantId, true);

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

@@ -0,0 +1,18 @@
+using Volo.Abp.EntityFrameworkCore.GlobalFilters;
+using Volo.Abp.Modularity;
+
+namespace Volo.Abp.EntityFrameworkCore.DataFiltering;
+
+// Disables UseDbFunction so the EF.Property global filter path is exercised
+// (covers both ISoftDelete and IMultiTenant filters).
+[DependsOn(typeof(AbpEntityFrameworkCoreTestModule))]
+public class AbpEntityFrameworkCoreTestModuleWithoutDbFunction : AbpModule
+{
+    public override void ConfigureServices(ServiceConfigurationContext context)
+    {
+        Configure<AbpEfCoreGlobalFilterOptions>(options =>
+        {
+            options.UseDbFunction = false;
+        });
+    }
+}

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

@@ -0,0 +1,83 @@
+using System.Collections;
+using System.Collections.Generic;
+using System.Linq.Expressions;
+using System.Text;
+using Microsoft.EntityFrameworkCore;
+
+namespace Volo.Abp.EntityFrameworkCore.DataFiltering;
+
+internal static class FilterExpressionPropertyNameInspector
+{
+    // EF Core stores the registered query filter under the CoreAnnotationNames.QueryFilter annotation.
+    // Since EF Core 9 the value is a QueryFilterCollection (internal) whose elements expose an
+    // Expression property. Older versions stored a bare LambdaExpression. We handle both via duck typing
+    // so the test stays compatible across EF Core versions.
+    public static List<string> GetEfPropertyStringArgs(object annotationValue)
+    {
+        return Inspect(annotationValue).Args;
+    }
+
+    public static InspectionResult Inspect(object annotationValue)
+    {
+        var collector = new EfPropertyArgCollector();
+        var dump = new StringBuilder();
+        dump.Append("annotationType=").Append(annotationValue?.GetType().FullName ?? "<null>").AppendLine();
+
+        switch (annotationValue)
+        {
+            case LambdaExpression lambda:
+                dump.Append("lambda: ").AppendLine(lambda.ToString());
+                collector.Visit(lambda);
+                break;
+            case IEnumerable enumerable:
+                var index = 0;
+                foreach (var item in enumerable)
+                {
+                    if (item == null)
+                    {
+                        continue;
+                    }
+
+                    dump.Append("[").Append(index++).Append("] itemType=").Append(item.GetType().FullName).AppendLine();
+                    var expr = item.GetType().GetProperty("Expression")?.GetValue(item) as Expression
+                               ?? item as Expression;
+                    if (expr != null)
+                    {
+                        dump.Append("    expr: ").AppendLine(expr.ToString());
+                        collector.Visit(expr);
+                    }
+                    else
+                    {
+                        dump.AppendLine("    (no Expression property)");
+                    }
+                }
+                break;
+        }
+
+        return new InspectionResult(collector.Args, dump.ToString());
+    }
+
+    public sealed record InspectionResult(List<string> Args, string Dump);
+
+    private sealed class EfPropertyArgCollector : ExpressionVisitor
+    {
+        public List<string> Args { get; } = new();
+
+        protected override Expression VisitMethodCall(MethodCallExpression node)
+        {
+            if (node.Method.DeclaringType == typeof(EF)
+                && node.Method.Name == nameof(EF.Property)
+                && node.Arguments.Count == 2)
+            {
+                // The 2nd arg comes from a closure-captured local at filter registration, so it
+                // appears as a MemberExpression rather than a ConstantExpression. Evaluate it.
+                if (Expression.Lambda(node.Arguments[1]).Compile().DynamicInvoke() is string name)
+                {
+                    Args.Add(name);
+                }
+            }
+
+            return base.VisitMethodCall(node);
+        }
+    }
+}

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

@@ -0,0 +1,69 @@
+using System;
+using System.Threading.Tasks;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Metadata.Internal;
+using Microsoft.Extensions.DependencyInjection;
+using Shouldly;
+using Volo.Abp.Data;
+using Volo.Abp.Domain.Repositories;
+using Volo.Abp.MultiTenancy;
+using Volo.Abp.TestApp.Domain;
+using Volo.Abp.TestApp.EntityFrameworkCore;
+using Volo.Abp.TestApp.Testing;
+using Xunit;
+
+namespace Volo.Abp.EntityFrameworkCore.DataFiltering;
+
+public class MultiTenant_With_Custom_Column_Name_Tests : TestAppTestBase<AbpEntityFrameworkCoreTestModuleWithoutDbFunction>
+{
+    private readonly IRepository<EntityWithCustomTenantIdColumn, Guid> _repository;
+    private readonly IDataFilter<IMultiTenant> _multiTenantFilter;
+
+    public MultiTenant_With_Custom_Column_Name_Tests()
+    {
+        _repository = GetRequiredService<IRepository<EntityWithCustomTenantIdColumn, Guid>>();
+        _multiTenantFilter = GetRequiredService<IDataFilter<IMultiTenant>>();
+    }
+
+    [Fact]
+    public async Task MultiTenant_Filter_Should_Work_When_TenantId_Has_Custom_Column_Name()
+    {
+        var ctx = ServiceProvider.GetRequiredService<TestAppDbContext>();
+        var tenantIdProperty = ctx.Model.FindEntityType(typeof(EntityWithCustomTenantIdColumn))!
+            .FindProperty(nameof(IMultiTenant.TenantId))!;
+        tenantIdProperty.GetColumnName().ShouldBe(EntityWithCustomTenantIdColumn.TenantIdColumnName);
+        tenantIdProperty.Name.ShouldBe(nameof(IMultiTenant.TenantId));
+
+        using (_multiTenantFilter.Disable())
+        {
+            await _repository.InsertAsync(new EntityWithCustomTenantIdColumn { Name = "host", TenantId = null });
+            await _repository.InsertAsync(new EntityWithCustomTenantIdColumn { Name = "tenant", TenantId = Guid.NewGuid() });
+
+            var all = await _repository.GetListAsync();
+            all.Count.ShouldBe(2);
+        }
+
+        var hostScoped = await _repository.GetListAsync();
+        hostScoped.Count.ShouldBe(1);
+        hostScoped[0].Name.ShouldBe("host");
+    }
+
+    [Fact]
+    public void MultiTenant_Filter_Should_Reference_TenantId_By_Clr_Property_Name()
+    {
+        var ctx = ServiceProvider.GetRequiredService<TestAppDbContext>();
+        var entityType = ctx.Model.FindEntityType(typeof(EntityWithCustomTenantIdColumn))!;
+
+#pragma warning disable EF1001
+        var annotation = entityType.FindAnnotation(CoreAnnotationNames.QueryFilter);
+#pragma warning restore EF1001
+        annotation.ShouldNotBeNull();
+
+        var args = FilterExpressionPropertyNameInspector.GetEfPropertyStringArgs(annotation.Value!);
+        args.ShouldNotBeEmpty();
+        foreach (var arg in args)
+        {
+            arg.ShouldBe(nameof(IMultiTenant.TenantId));
+        }
+    }
+}

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

@@ -0,0 +1,69 @@
+using System;
+using System.Threading.Tasks;
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Metadata.Internal;
+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;
+
+public class SoftDelete_With_Custom_Column_Name_Tests : TestAppTestBase<AbpEntityFrameworkCoreTestModuleWithoutDbFunction>
+{
+    private readonly IRepository<EntityWithCustomSoftDeleteColumn, Guid> _repository;
+    private readonly IDataFilter<ISoftDelete> _softDeleteFilter;
+
+    public SoftDelete_With_Custom_Column_Name_Tests()
+    {
+        _repository = GetRequiredService<IRepository<EntityWithCustomSoftDeleteColumn, Guid>>();
+        _softDeleteFilter = GetRequiredService<IDataFilter<ISoftDelete>>();
+    }
+
+    [Fact]
+    public async Task SoftDelete_Filter_Should_Work_When_IsDeleted_Has_Custom_Column_Name()
+    {
+        var ctx = ServiceProvider.GetRequiredService<TestAppDbContext>();
+        var isDeletedProperty = ctx.Model.FindEntityType(typeof(EntityWithCustomSoftDeleteColumn))!
+            .FindProperty(nameof(ISoftDelete.IsDeleted))!;
+        isDeletedProperty.GetColumnName().ShouldBe(EntityWithCustomSoftDeleteColumn.IsDeletedColumnName);
+        isDeletedProperty.Name.ShouldBe(nameof(ISoftDelete.IsDeleted));
+
+        await _repository.InsertAsync(new EntityWithCustomSoftDeleteColumn { Name = "kept" });
+        await _repository.InsertAsync(new EntityWithCustomSoftDeleteColumn { 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);
+            all.ShouldContain(x => x.Name == "removed" && x.IsDeleted);
+        }
+    }
+
+    [Fact]
+    public void SoftDelete_Filter_Should_Reference_IsDeleted_By_Clr_Property_Name()
+    {
+        var ctx = ServiceProvider.GetRequiredService<TestAppDbContext>();
+        var entityType = ctx.Model.FindEntityType(typeof(EntityWithCustomSoftDeleteColumn))!;
+
+#pragma warning disable EF1001
+        var annotation = entityType.FindAnnotation(CoreAnnotationNames.QueryFilter);
+#pragma warning restore EF1001
+        annotation.ShouldNotBeNull();
+
+        var result = FilterExpressionPropertyNameInspector.Inspect(annotation.Value!);
+        result.Args.ShouldNotBeEmpty(customMessage: result.Dump);
+        foreach (var arg in result.Args)
+        {
+            arg.ShouldBe(nameof(ISoftDelete.IsDeleted), customMessage: result.Dump);
+        }
+    }
+}

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

@@ -27,6 +27,10 @@ public class TestMigrationsDbContext : AbpDbContext<TestMigrationsDbContext>
 
     public DbSet<Category> Categories { get; set; }
 
+    public DbSet<EntityWithCustomSoftDeleteColumn> EntityWithCustomSoftDeleteColumns { get; set; }
+
+    public DbSet<EntityWithCustomTenantIdColumn> EntityWithCustomTenantIdColumns { get; set; }
+
     public DbSet<EntityWithIntSoftDelete> EntityWithIntSoftDeletes { get; set; }
 
     public DbSet<AppEntityWithNavigations> AppEntityWithNavigations { get; set; }
@@ -69,6 +73,17 @@ public class TestMigrationsDbContext : AbpDbContext<TestMigrationsDbContext>
 
         base.OnModelCreating(modelBuilder);
 
+        // Mirror the column renames in TestAppDbContext so the generated SQLite schema matches.
+        modelBuilder.Entity<EntityWithCustomSoftDeleteColumn>(b =>
+        {
+            b.Property(x => x.IsDeleted).HasColumnName(EntityWithCustomSoftDeleteColumn.IsDeletedColumnName);
+        });
+
+        modelBuilder.Entity<EntityWithCustomTenantIdColumn>(b =>
+        {
+            b.Property(x => x.TenantId).HasColumnName(EntityWithCustomTenantIdColumn.TenantIdColumnName);
+        });
+
         modelBuilder.Entity<Phone>(b =>
         {
             b.HasKey(p => new { p.PersonId, p.Number });

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

@@ -1,12 +1,14 @@
 using System;
 using Microsoft.EntityFrameworkCore;
 using Microsoft.EntityFrameworkCore.Infrastructure;
+using Microsoft.EntityFrameworkCore.Metadata;
 using Microsoft.EntityFrameworkCore.Metadata.Builders;
 using Volo.Abp.DependencyInjection;
 using Volo.Abp.EntityFrameworkCore;
 using Volo.Abp.EntityFrameworkCore.Modeling;
 using Volo.Abp.EntityFrameworkCore.TestApp.FourthContext;
 using Volo.Abp.EntityFrameworkCore.TestApp.ThirdDbContext;
+using Volo.Abp.MultiTenancy;
 using Volo.Abp.TestApp.Domain;
 using Volo.Abp.TestApp.Testing;
 
@@ -33,6 +35,10 @@ public class TestAppDbContext : AbpDbContext<TestAppDbContext>, IThirdDbContext,
 
     public DbSet<Category> Categories { get; set; }
 
+    public DbSet<EntityWithCustomSoftDeleteColumn> EntityWithCustomSoftDeleteColumns { get; set; }
+
+    public DbSet<EntityWithCustomTenantIdColumn> EntityWithCustomTenantIdColumns { get; set; }
+
     public DbSet<EntityWithIntSoftDelete> EntityWithIntSoftDeletes { get; set; }
 
     public DbSet<AppEntityWithNavigations> AppEntityWithNavigations { get; set; }
@@ -187,4 +193,32 @@ public class TestAppDbContext : AbpDbContext<TestAppDbContext>, IThirdDbContext,
 
         modelBuilder.TryConfigureObjectExtensions<TestAppDbContext>();
     }
+
+    // Renames IsDeleted / TenantId to a non-default column name and re-registers the global filter
+    // afterwards. CreateFilterExpression then captures the renamed column name; before the fix it
+    // would feed that string to EF.Property<T>(...), which expects a CLR property name and breaks
+    // translation. Covered by SoftDelete_With_Custom_Column_Name_Tests and
+    // MultiTenant_With_Custom_Column_Name_Tests.
+    protected override void ConfigureBaseProperties<TEntity>(ModelBuilder modelBuilder, IMutableEntityType mutableEntityType)
+    {
+        base.ConfigureBaseProperties<TEntity>(modelBuilder, mutableEntityType);
+
+        if (typeof(EntityWithCustomSoftDeleteColumn).IsAssignableFrom(typeof(TEntity)))
+        {
+            modelBuilder.Entity<TEntity>()
+                .Property(nameof(ISoftDelete.IsDeleted))
+                .HasColumnName(EntityWithCustomSoftDeleteColumn.IsDeletedColumnName);
+
+            ConfigureGlobalFilters<TEntity>(modelBuilder, mutableEntityType, modelBuilder.Entity<TEntity>());
+        }
+
+        if (typeof(EntityWithCustomTenantIdColumn).IsAssignableFrom(typeof(TEntity)))
+        {
+            modelBuilder.Entity<TEntity>()
+                .Property(nameof(IMultiTenant.TenantId))
+                .HasColumnName(EntityWithCustomTenantIdColumn.TenantIdColumnName);
+
+            ConfigureGlobalFilters<TEntity>(modelBuilder, mutableEntityType, modelBuilder.Entity<TEntity>());
+        }
+    }
 }

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

@@ -0,0 +1,13 @@
+using System;
+using Volo.Abp.Domain.Entities;
+
+namespace Volo.Abp.TestApp.Domain;
+
+public class EntityWithCustomSoftDeleteColumn : AggregateRoot<Guid>, ISoftDelete
+{
+    public const string IsDeletedColumnName = "custom_is_deleted_column";
+
+    public string Name { get; set; }
+
+    public bool IsDeleted { get; set; }
+}

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

@@ -0,0 +1,14 @@
+using System;
+using Volo.Abp.Domain.Entities;
+using Volo.Abp.MultiTenancy;
+
+namespace Volo.Abp.TestApp.Domain;
+
+public class EntityWithCustomTenantIdColumn : AggregateRoot<Guid>, IMultiTenant
+{
+    public const string TenantIdColumnName = "custom_tenant_id_column";
+
+    public string Name { get; set; }
+
+    public Guid? TenantId { get; set; }
+}