Laraue.EfCoreTriggers icon indicating copy to clipboard operation
Laraue.EfCoreTriggers copied to clipboard

Adding non-generic support

Open kjkrum opened this issue 2 years ago • 5 comments

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

kjkrum avatar Jul 16 '22 19:07 kjkrum

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

win7user10 avatar Jul 16 '22 21:07 win7user10

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

kjkrum avatar Jul 16 '22 22:07 kjkrum

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

win7user10 avatar Jul 17 '22 14:07 win7user10

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;
}

kjkrum avatar Jul 19 '22 06:07 kjkrum

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?

KasperKimose avatar Sep 01 '22 14:09 KasperKimose

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!"));
}

win7user10 avatar Jan 29 '23 11:01 win7user10

@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.

kjkrum avatar Feb 02 '23 16:02 kjkrum