win7user10 / Laraue.EfCoreTriggers

Library to write triggers in C# with EF.Core
MIT License
112 stars 20 forks source link

Triggers do not work for properties that are declared as enums #3

Closed koimad closed 3 years ago

koimad commented 3 years ago

Hi, When I create a project using Enums on the entities as a property, then I'm unable to generate a trigger base on that property.

For example, I added the Property Status to the User Class and declared it as a enum UserStatus.

If I create a trigger to do another action based on the status changing from draft to verified, then the Add-Migration fails with Convert is not supported.

I've done a little work to fix the issue, but I'm not sure its quite right. I include the files I updated for you to look at. I will explain the changes below.

2 changes are required to fix the issue.

In the implementations of BaseTriggerProvider GetSqlServerType need updating to include Enum : "INT" in the mapping variable, and the try get needs to be updated to try the base type as well. return mapping.TryGetValue(propertyInfo.PropertyType, out String type) || propertyInfo.PropertyType.BaseType != null && mapping.TryGetValue(propertyInfo.PropertyType.BaseType, out type) ? type : throw new NotSupportedException($"Unknown data type {propertyInfo.PropertyType}");

In BaseExpressionProvider GetBinaryExpressionSql need to be updated to test if either part is a ExpressionType.Convert, if so it needs to assign the operand property as the part. Expression[] GetBinaryExpressionParts() { Expression[] parts = new[] { binaryExpression.Left, binaryExpression.Right };

            for (int i = 0; i < parts.Length; i++)
            {
                var part = parts[i];

if (part.NodeType == ExpressionType.Convert) { UnaryExpression unaryExpression = part as UnaryExpression; if (unaryExpression != null) { parts[i] = unaryExpression.Operand; } } }

            if (binaryExpression.Method is null)
            {
                if (binaryExpression.Left is MemberExpression leftMemberExpression && leftMemberExpression.Type == typeof(bool))
                    parts[0] = Expression.IsTrue(binaryExpression.Left);
                if (binaryExpression.Right is MemberExpression rightMemberExpression && rightMemberExpression.Type == typeof(bool))
                    parts[1] = Expression.IsTrue(binaryExpression.Right);
            }
            return parts;
        };

Changes.zip

win7user10 commented 3 years ago

Hi, @koimad. If I understood the issue correctly, it should work in the new version 1.3.0. Try to check this.

koimad commented 3 years ago

Hi Ilya, My first email was not quite right, the linq expression below

//EntityTypeBuilder builder

builder.AfterInsert(trigger => trigger.Action(action => action .Condition(f => f.Status == EntityStatus.New) .Update((a, b) => a.Id == b.Id, (a, b) => new SalesCategory() {Status = EntityStatus.Draft})));

Generates the following MS SQL

migrationBuilder.Sql(" CREATE TRIGGER LC_TRIGGER_AFTER_INSERT_SALESCATEGORY ON SalesCategory AFTER Insert AS BEGIN DECLARE InsertedSalesCategoryCursor CURSOR FOR SELECT StatusId FROM Inserted DECLARE @NewStatus INT OPEN InsertedSalesCategoryCursor FETCH NEXT FROM InsertedSalesCategoryCursor INTO @NewStatus WHILE @@FETCH_STATUS = 0 BEGIN IF (@NewStatus = 0) BEGIN UPDATE SalesCategory SET StatusId = 1 WHERE @NewId = Id; END FETCH NEXT FROM InsertedSalesCategoryCursor INTO @NewStatus END CLOSE InsertedSalesCategoryCursor DEALLOCATE InsertedSalesCategoryCursor END");

@NewId as high lighted is never declared and fetched from the InsertedSalesCategoryCursor.

I only found it after running the migration and it failed.

I believe the issue is caused be table per hierarchy and Sales Category has a base class as defined below.

public abstract class DomainBase
{
    #region Properties

    public Int64 Id { get; set; }

    public DateTime CreatedDate { get; set; }

    public String CreatedBy { get; set; }

    public String LastModifiedBy { get; set; }

    public DateTime LastModifiedDate { get; set; }

    [DatabaseGenerated(DatabaseGeneratedOption.Computed)]
    public Byte[] RowVersion { get; set; }

    #endregion
}

I believe in SqlServerProvider you need to update the internal class extensions as below to include IsAssignableFrom rather than comparison equals..

internal static class Extensions { public static IEnumerable WhereDeclaringType(this IEnumerable values) => values.Where(x => x.DeclaringType.IsAssignableFrom(typeof(T))); }

Regards Brian.

win7user10 commented 3 years ago

Hi, Brian. I tried to reproduce the latest issue as you described. I created a test case in the branch https://github.com/win7user10/Laraue.EfCoreTriggers/tree/feature/inheritance-entities/tests/Laraue.EfCoreTriggers.SqlServerTests/Issues/3, now it throws the error due to missing support for inheritance entities now. If the test case is right, I try to add support for this case the next weekend.

koimad commented 3 years ago

Hi Ilya,

       I’ll have a look this evening, I believe the fix is to update the expression at the bottom of the mssql provider.

 I also found another issue for mssql that if a table is not in the dbo schema then the trigger fails since the schema name is not included with the table name. I fixed this by overriding the get table name method from provider base. I’ll upload my changes tonight.

Regards,

         Brian.

Get Outlook for iOShttps://aka.ms/o0ukef


From: Ilya Belyanskiy notifications@github.com Sent: Wednesday, February 24, 2021 7:05:10 AM To: win7user10/Laraue.EfCoreTriggers Laraue.EfCoreTriggers@noreply.github.com Cc: koimad koimad@blueyonder.co.uk; Mention mention@noreply.github.com Subject: Re: [win7user10/Laraue.EfCoreTriggers] Triggers do not work for properties that are declared as enums (#3)

Hi, Brian. I tried to reproduce the latest issue as you described. I created a test case in the branch https://github.com/win7user10/Laraue.EfCoreTriggers/tree/feature/inheritance-entities/tests/Laraue.EfCoreTriggers.SqlServerTests/Issues/3, now it throws the error due to missing support for inheritance entities now. If the test case is right, I try to add support for this case the next weekend.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/win7user10/Laraue.EfCoreTriggers/issues/3#issuecomment-784844198, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABLS4SWST3BDCXM6KARQ3SLTASQKNANCNFSM4YBEE2JA.

koimad commented 3 years ago

Hi Ilya, I've had a looh and believe the changes required are to SQLServerProvider below. I also added to the tests so I include them as well. I've added the model builder data setup to the TestContext this fixed the original Exception.

Regards Brian.

------------------------------------------ SQLServerProvider ------------------------------------------------ using Laraue.EfCoreTriggers.Common.Builders.Triggers.Base; using Microsoft.EntityFrameworkCore.Metadata; using System; using System.Collections.Generic; using System.Linq; using System.Linq.Expressions; using System.Reflection;

namespace Laraue.EfCoreTriggers.Common.Builders.Providers { public class SqlServerProvider : BaseTriggerProvider { public SqlServerProvider(IModel model) : base(model) { }

    protected override Dictionary<Type, string> TypeMappings { get; } = new Dictionary<Type, string>
    {
        [typeof(bool)] = "BIT",
        [typeof(byte)] = "TINYINT",
        [typeof(short)] = "SMALLINT",
        [typeof(int)] = "INT",
        [typeof(long)] = "BIGINT",
        [typeof(sbyte)] = "SMALLMONEY",
        [typeof(ushort)] = "NUMERIC(20)",
        [typeof(uint)] = "NUMERIC(28)",
        [typeof(ulong)] = "NUMERIC(29)",
        [typeof(decimal)] = "DECIMAL(38)",
        [typeof(float)] = "FLOAT(24)",
        [typeof(double)] = "FLOAT(53)",
        [typeof(Enum)] = "INT",
        [typeof(char)] = "CHAR(1)",
        [typeof(string)] = "VARCHAR(MAX)",
        [typeof(DateTime)] = "DATETIME2",
        [typeof(DateTimeOffset)] = "DATETIMEOFFSET",
        [typeof(TimeSpan)] = "TIME",
        [typeof(Guid)] = "UNIQUEIDENTIFIER",
    };

    protected override string NewEntityPrefix => "Inserted";

    protected override string OldEntityPrefix => "Deleted";

    protected override IEnumerable<TriggerTime> AvailableTriggerTimes { get; } = new[] { TriggerTime.After, TriggerTime.InsteadOf };

    public override SqlBuilder GetDropTriggerSql(string triggerName)
        => new SqlBuilder($"DROP TRIGGER {triggerName};");

    public override SqlBuilder GetTriggerSql<TTriggerEntity>(Trigger<TTriggerEntity> trigger)
    {
        var triggerTimeName = GetTriggerTimeName(trigger.TriggerTime);

        var actionsSql = trigger.Actions.Select(action => action.BuildSql(this));

        var sqlBuilder = new SqlBuilder(actionsSql);
        sqlBuilder.Append($"CREATE TRIGGER {trigger.Name} ON {GetTableName(typeof(TTriggerEntity))} ")
            .Append(triggerTimeName)
            .Append($" {trigger.TriggerEvent} AS BEGIN ");

        sqlBuilder.Append(DeclareCursorBlocksSql<TTriggerEntity>(sqlBuilder.AffectedColumns))
            .Append(" ")
            .Append(FetchCursorsSql<TTriggerEntity>(sqlBuilder.AffectedColumns))
            .Append(" WHILE @@FETCH_STATUS = 0")
            .Append(" BEGIN ")
            .AppendJoin(actionsSql.Select(x => x.StringBuilder))
            .Append(FetchCursorsSql<TTriggerEntity>(sqlBuilder.AffectedColumns))
            .Append(" END ");

        sqlBuilder.Append(CloseCursorsBlockSql<TTriggerEntity>(sqlBuilder.AffectedColumns))
            .Append(" END");

        return sqlBuilder;
    }

    protected override string GetExpressionOperandSql(Expression expression) => expression.NodeType switch
    {
        ExpressionType.IsTrue => $"= {GetBooleanSqlValue(true)}",
        ExpressionType.IsFalse => $"= {GetBooleanSqlValue(false)}",
        ExpressionType.Not => $"= {GetBooleanSqlValue(false)}",
        _ => base.GetExpressionOperandSql(expression),
    };

    public override SqlBuilder GetTriggerActionsSql<TTriggerEntity>(TriggerActions<TTriggerEntity> triggerActions)
    {
        var sqlResult = new SqlBuilder();

        if (triggerActions.ActionConditions.Count > 0)
        {
            var conditionsSql = triggerActions.ActionConditions.Select(actionCondition => actionCondition.BuildSql(this));
            sqlResult.MergeColumnsInfo(conditionsSql);
            sqlResult.Append($"IF (")
                .AppendJoin(" AND ", conditionsSql.Select(x => x.StringBuilder))
                .Append($") ");
        }

        var actionsSql = triggerActions.ActionExpressions.Select(action => action.BuildSql(this));
        sqlResult.MergeColumnsInfo(actionsSql)
            .Append("BEGIN ")
            .AppendJoin("; ", actionsSql.Select(x => x.StringBuilder))
            .Append(" END ");

        return sqlResult;
    }

    private string CursorName<TTriggerEntity>(ArgumentType argumentType)
        => $"{TemporaryTableName(argumentType)}{typeof(TTriggerEntity).Name}Cursor";

    private string TemporaryTableName(ArgumentType argumentType)
    {
        return argumentType switch
        {
            ArgumentType.New => NewEntityPrefix,
            ArgumentType.Old => OldEntityPrefix,
            _ => throw new InvalidOperationException($"Temporary table for {argumentType} is not exists")
        };
    }

    private string DeclareCursorSql(string cursorName)
        => $"DECLARE {cursorName} CURSOR FOR";

    private string FetchCursorsSql<TTriggerEntity>(Dictionary<ArgumentType, HashSet<MemberInfo>> members)
        => string.Join(" ", members.Where(x => x.Value.WhereDeclaringType<TTriggerEntity>().Any()).Select(x => FetchCursorSql<TTriggerEntity>(x.Key, x.Value)));

    private string FetchCursorSql<TTriggerEntity>(ArgumentType argumentType, IEnumerable<MemberInfo> members)
        => $"FETCH NEXT FROM {CursorName<TTriggerEntity>(argumentType)} INTO {string.Join(", ", members.WhereDeclaringType<TTriggerEntity>().Select(member => VariableNameSql(argumentType, member)))}";

    private string SelectFromCursorSql<TTriggerEntity>(ArgumentType argumentType, IEnumerable<MemberInfo> members)
        => $"SELECT {string.Join(", ", members.WhereDeclaringType<TTriggerEntity>().Select(x => GetColumnName(x)))} FROM {TemporaryTableName(argumentType)}";

    private string DeclareCursorVariablesSql<TTriggerEntity>(ArgumentType argumentType, IEnumerable<MemberInfo> members)
        => $"DECLARE {string.Join(", ", members.WhereDeclaringType<TTriggerEntity>().Select(member => DeclareVariableNameSql(argumentType, member)))}";

    private string CloseCursorSql(string cursorName)
        => $"CLOSE {cursorName}";

    private string DeallocateCursorSql(string cursorName)
        => $"DEALLOCATE {cursorName}";

    private string CloseCursorsBlockSql<TTriggerEntity>(Dictionary<ArgumentType, HashSet<MemberInfo>> members)
    {
        return string.Join(" ", members.Where(x => x.Value.Count > 0)
            .Select(x => $"{CloseCursorSql(CursorName<TTriggerEntity>(x.Key))} {DeallocateCursorSql(CursorName<TTriggerEntity>(x.Key))}"));
    }

    private string VariableNameSql(ArgumentType argumentType, MemberInfo member)
        => argumentType switch
        {
            ArgumentType.New => $"@New{member.Name}",
            ArgumentType.Old => $"@Old{member.Name}",
            _ => throw new InvalidOperationException($"Invalid attempt to generate declaring variable SQL using argument prefix {argumentType}")
        };

    private string DeclareVariableNameSql(ArgumentType argumentType, MemberInfo member)
        => $"{VariableNameSql(argumentType, member)} {GetSqlServerType((PropertyInfo)member)}";

    private string GetSqlServerType(PropertyInfo propertyInfo)
        => GetSqlType(propertyInfo.PropertyType) ?? throw new NotSupportedException($"Unknown data type {propertyInfo.PropertyType}");

    private SqlBuilder DeclareCursorBlocksSql<TTriggerEntity>(Dictionary<ArgumentType, HashSet<MemberInfo>> affectedMemberPairs)
    {
        var cursorBlocksSql = affectedMemberPairs
            .Where(x => x.Value.WhereDeclaringType<TTriggerEntity>().Any())
            .Select(x => DeclareCursorBlockSql<TTriggerEntity>(x.Key, x.Value));
        return new SqlBuilder()
            .AppendJoin(" ", cursorBlocksSql.Select(x => x.StringBuilder));
    }

    private SqlBuilder DeclareCursorBlockSql<TTriggerEntity>(ArgumentType argumentType, IEnumerable<MemberInfo> affectedMembers)
    {
        var cursorName = CursorName<TTriggerEntity>(argumentType);
        return new SqlBuilder()
            .Append(DeclareCursorSql(cursorName))
            .Append(" ")
            .Append(SelectFromCursorSql<TTriggerEntity>(argumentType, affectedMembers))
            .Append(" ")
            .Append(DeclareCursorVariablesSql<TTriggerEntity>(argumentType, affectedMembers))
            .Append($" OPEN {cursorName}");
    }

    public override SqlBuilder GetTriggerUpsertActionSql<TTriggerEntity, TUpsertEntity>(TriggerUpsertAction<TTriggerEntity, TUpsertEntity> triggerUpsertAction)
    {
        var insertStatementSql = GetInsertStatementBodySql(triggerUpsertAction.InsertExpression, triggerUpsertAction.InsertExpressionPrefixes);

        var newExpressionArguments = ((NewExpression)triggerUpsertAction.MatchExpression.Body).Arguments
            .Cast<MemberExpression>();

        var newExpressionArgumentPairs = newExpressionArguments.ToDictionary(
            argument => argument,
            argument => GetMemberExpressionSql(argument, triggerUpsertAction.MatchExpressionPrefixes));

        var sqlBuilder = new SqlBuilder(newExpressionArgumentPairs.Select(x => x.Value));

        sqlBuilder
            .Append("SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;")
            .Append($"MERGE {GetTableName(typeof(TUpsertEntity))} USING {GetTableName(typeof(TTriggerEntity))}")
            .Append($" ON ")
            .AppendJoin(" AND ", newExpressionArgumentPairs
                .Select(memberPair => $"{GetTableName(typeof(TUpsertEntity))}.{GetColumnName(memberPair.Key.Member)} = {memberPair.Value}"));

        sqlBuilder.Append(" WHEN NOT MATCHED THEN INSERT ")
            .Append(insertStatementSql.StringBuilder);

        if (triggerUpsertAction.OnMatchExpression != null)
        {
            var updateStatementSql = GetUpdateStatementBodySql(triggerUpsertAction.OnMatchExpression, triggerUpsertAction.OnMatchExpressionPrefixes);
            sqlBuilder.MergeColumnsInfo(updateStatementSql);
            sqlBuilder.Append(" WHEN MATCHED THEN UPDATE SET ")
                .Append(updateStatementSql.StringBuilder);
        }

        return sqlBuilder.Append(";");
    }

    protected override string GetMemberExpressionSql(MemberExpression memberExpression, ArgumentType argumentType)
    {
        return argumentType switch
        {
            ArgumentType.New => VariableNameSql(argumentType, memberExpression.Member),
            ArgumentType.Old => VariableNameSql(argumentType, memberExpression.Member),
            _ => GetColumnName(memberExpression.Member),
        };
    }

    protected override string GetBooleanSqlValue(bool value) => value
        ? "1"
        : "0";

    protected override string GetNewGuidExpressionSql() => "NEWID()";

    protected override string GetTableName(Type entity)
    {
        string schemaName = GetTableSchemaName(entity);

        return String.IsNullOrWhiteSpace(schemaName) ? base.GetTableName(entity) : $"{schemaName}.{base.GetTableName(entity)}";
    }
}

internal static class Extensions
{
    public static IEnumerable<MemberInfo> WhereDeclaringType<T>(this IEnumerable<MemberInfo> values)
        => values.Where(x => x.DeclaringType.IsAssignableFrom(typeof(T)));
}

}

------------------------------------ Sales Category ---------------------------------------------------------------------

namespace Laraue.EfCoreTriggers.SqlServerTests.Issues._3 { public class SalesCategory : DomainBase { public String Name { get; set; }

    public EntityStatus Status { get; set; }
}

public class SalesArea : DomainBase
{
    public String Name { get; set; }

    public EntityStatus Status { get; set; }
}

public enum EntityStatus
{
    New,
    Draft,
}

}

--------------------------------------- TestDbContext ----------------------------------------------------------

public class TestDbContext : DbContext
{
    public TestDbContext(DbContextOptions options) : base(options)
    {
    }

    public DbSet<SalesCategory> SalesCategories { get; set; }

    #region Overrides of DbContext

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<DomainBase>()
            .ToTable("DomainEntityBase", "CommonSchema")
            .HasKey(f => f.Id);

        modelBuilder.Entity<SalesCategory>()
            .ToTable("SalesCategory", "PolicySchema");

        modelBuilder.Entity<SalesArea>()
            .ToTable("SalesArea");

    }

    #endregion
}

-------------------------------------------- Tests ---------------------------------------------------------

public class Tests { private const String _expectedSaleCategoryTriggerSQL = "CREATE TRIGGER LC_TRIGGER_AFTER_INSERT_SALESCATEGORY ON PolicySchema.SalesCategory AFTER Insert AS BEGIN DECLARE InsertedSalesCategoryCursor CURSOR FOR SELECT Status, Id FROM Inserted DECLARE @NewStatus INT, @NewId BIGINT OPEN InsertedSalesCategoryCursor FETCH NEXT FROM InsertedSalesCategoryCursor INTO @NewStatus, @NewId WHILE @@FETCH_STATUS = 0 BEGIN IF (@NewStatus = 0) BEGIN UPDATE PolicySchema.SalesCategory SET Status = 1 WHERE @NewId = Id; END FETCH NEXT FROM InsertedSalesCategoryCursor INTO @NewStatus, @NewId END CLOSE InsertedSalesCategoryCursor DEALLOCATE InsertedSalesCategoryCursor END";

    private const String _expectedSalesAreaTriggerSQL = "CREATE TRIGGER LC_TRIGGER_AFTER_INSERT_SALESAREA ON SalesArea AFTER Insert AS BEGIN DECLARE InsertedSalesAreaCursor CURSOR FOR SELECT Status, Id FROM Inserted DECLARE @NewStatus INT, @NewId BIGINT OPEN InsertedSalesAreaCursor FETCH NEXT FROM InsertedSalesAreaCursor INTO @NewStatus, @NewId WHILE @@FETCH_STATUS = 0 BEGIN IF (@NewStatus = 0) BEGIN UPDATE SalesArea SET Status = 1 WHERE @NewId = Id; END FETCH NEXT FROM InsertedSalesAreaCursor INTO @NewStatus, @NewId END CLOSE InsertedSalesAreaCursor DEALLOCATE InsertedSalesAreaCursor END";

    private readonly ITriggerProvider _provider;

    public Tests()
    {
        var context = new TestDbContext(new DbContextOptionsBuilder<TestDbContext>()
            .UseSqlServer()
            .UseTriggers()
            .Options);

        _provider = new SqlServerProvider(context.Model); 
    }

    [Fact]
    public virtual void SalesCategoryShouldBeGeneratedCorrectSql()
    {
        var trigger = new OnInsertTrigger<SalesCategory>(TriggerTime.After)
            .Action(action => action
                .Condition(f => f.Status == EntityStatus.New)
                .Update<SalesCategory>((a, b) => a.Id == b.Id, (a, b) => new SalesCategory() { Status = EntityStatus.Draft }));

        var sql = trigger.BuildSql(_provider);

        Assert.Equal(_expectedSaleCategoryTriggerSQL,sql);
    }

    [Fact]
    public virtual void SalesAreaShouldBeGeneratedCorrectSql()
    {
        var trigger = new OnInsertTrigger<SalesArea>(TriggerTime.After)
            .Action(action => action
                .Condition(f => f.Status == EntityStatus.New)
                .Update<SalesArea>((a, b) => a.Id == b.Id, (a, b) => new SalesArea() { Status = EntityStatus.Draft }));

        var sql = trigger.BuildSql(_provider);

        Assert.Equal(_expectedSalesAreaTriggerSQL,sql);
    }

}
win7user10 commented 3 years ago

Thank you for the code, Brian. I added your fixes, now specified triggers work in SqlServer.

koimad commented 3 years ago

Thank you for all your effort. If you would like to do a further update you could consider updating it to handle enums that are longs.

Brian.