win7user10 / Laraue.EfCoreTriggers

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

Adding non-generic support #50

Closed kjkrum closed 1 year ago

kjkrum commented 2 years ago

I plan to have several entities that implement common interfaces. For example, I might have an interface that includes an Inactive flag. For ease of maintenance, I want to automatically identify entities that implement these interfaces and add triggers to them. The problem I came up against is that the types returned by methods like IMutableModel.GetEntityTypes() are not generic. And pretty much everything in Laraue.EfCoreTriggers.Common operates on generics.

So I forked the lib and started digging. I think it would be pretty simple to add support for non-generics like EntityTypeBuilder and IMutableEntityType. The only fundamental problem is that Trigger<T> assumes the trigger type (the type that actions operate on) and the entity type are one and the same. I made a few simple changes to allow the types to be distinguished while maintaining backward compatibility. The Trigger constructor takes a parameter identifying the entity type. If it's null, it falls back to the generic type. This small change was all I needed for this proof of concept (Action and Throw are extension methods in my project):

var types = builder.Model.GetEntityTypes().Where(t => t.ClrType.IsAssignableTo(typeof(RowVersionedEntity)));
foreach (var type in types)
{
    var trigger = new OnUpdateTrigger<RowVersionedEntity>(TriggerTime.After, type.ClrType);
    trigger.Action(actions => actions.Throw(50000, "You foolian!"));
    type.AddAnnotation(trigger.Name, trigger);
}

Other classes in Common could have methods or constructors to expose this. What do you think?

https://github.com/kjkrum/Laraue.EfCoreTriggers/tree/non-generic-support

win7user10 commented 2 years ago

Hi, I think it will be a good solution for support of such cases

kjkrum commented 2 years ago

Adding a parameter with a default preserves source compatibility. But if you care about binary compatibility, I think I need to change it to an overloaded constructor with the new parameter.

https://docs.microsoft.com/en-us/dotnet/core/compatibility/ https://stackoverflow.com/a/39447773/1953590

win7user10 commented 2 years ago

All nugets will be released in the same time, so I don't think that binary compatibility is necessary

kjkrum commented 2 years ago

I realized I can do the same thing without modifying the library if I configure the trigger and wrap it in this:

public class TriggerHacks<T> : ITrigger where T : class
{
    private readonly Trigger<T> _trigger;

    public TriggerHacks(Trigger<T> trigger)
    {
        _trigger = trigger;
    }

    public List<ITriggerAction> Actions => _trigger.Actions;
    public List<ITriggerAction> Conditions => _trigger.Conditions;
    public TriggerEvent TriggerEvent => _trigger.TriggerEvent;
    public TriggerTime TriggerTime => _trigger.TriggerTime;
    public Type TriggerEntityType => _entityTypeOverride ?? _trigger.TriggerEntityType;
    public string Name => _nameOverride ?? _trigger.Name;

    private Type? _entityTypeOverride;
    public Type? EntityTypeOverride
    {
        get => _entityTypeOverride;
        set
        {
            if(value != null && !value.IsAssignableTo(typeof(T)))
            {
                throw new ArgumentException($"Type must be assignable to {typeof(T).FullName}.");
            }
            _entityTypeOverride = value;
        }
    }

    private string? _nameOverride;
    public string? NameOverride
    {
        get => _nameOverride;
        set
        {
            if(value != null && !value.StartsWith(Constants.AnnotationKey))
            {
                throw new ArgumentException($"Name must start with {typeof(Constants).FullName}.{nameof(Constants.AnnotationKey)} (\"{Constants.AnnotationKey}\").");
            }
            _nameOverride = value;
        }
    }

I'm using it like this:

public static ModelBuilder FreezeInactiveRows(this ModelBuilder builder)
{
    var types = builder.Model.GetEntityTypes().Where(t => t.ClrType.IsAssignableTo(typeof(IInactivatable)));
    foreach (var type in types)
    {
        var trigger = new OnUpdateTrigger<IInactivatable>(TriggerTime.After);
        trigger.Action(action => action
        .Condition((before, after) => before.Inactive && after.Inactive)
        .Throw(CustomErrorNumbers.RowInactive, $"Cannot update inactive {type.ClrType.Name}."));
        var hacks = trigger.Hacks();
        hacks.EntityTypeOverride = type.ClrType;
        hacks.NameOverride = $"{Constants.AnnotationKey}_{type.GetTableName()!.ToUpperInvariant()}_FREEZE_INACTIVE";
        type.AddAnnotation(hacks.Name, hacks);
    }
    return builder;
}
KasperKimose commented 2 years ago

Hi @kjkrum, I tried to use your TriggerHacks implementation as we are in the same situation where we want to have the triggers automatically implemeted if they implement a specific interface. But I'm getting an error when adding migrations, saying that the DbSet with my interface needs to be in the DbContext. How did you avoid that?

win7user10 commented 1 year ago

I investigated ways to add additional parameters with the real CLR type in triggers and came to the conclusion that is too hard. All expressions that pass to triggers should replace their type from the generic to the passed. Different triggers use different numbers of generics and it also should be supported. So I tried to write code that will allow to register non-generic triggers. I think it should be acceptable in such cases. The full example is here https://github.com/win7user10/Laraue.EfCoreTriggers/blob/dde4f7d6ac764a48f05c9ed523e4fd02eed1d7e3/tests/Laraue.EfCoreTriggers.Tests/Tests/WorkaroundTests.cs#L60 In the code of the question, it will looks like

var types = builder.Model.GetEntityTypes().Where(t => t.ClrType.IsAssignableTo(typeof(RowVersionedEntity)));
foreach (var type in types)
{
    var triggerType = typeof(OnUpdateTrigger<>).MakeGenericType(type.ClrType);
    var trigger = (ITrigger) Activator.CreateInstance(triggerType, TriggerTime.After)!;

    AddTriggerAction((dynamic) trigger);

    type.AddAnnotation(trigger.Name, trigger);
}

void AddTriggerAction<T>(OnUpdateTrigger<T> trigger) where T : RowVersionedEntity
{
    trigger.Action(actions => actions.Throw(50000, "You foolian!"));
}
kjkrum commented 1 year ago

@KasperKimose Sorry I missed your question. Make sure the trigger is of the interface type, but the DbSet is of the concrete type that implements it.