Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding non-generic support #50

Closed
kjkrum opened this issue Jul 16, 2022 · 7 comments
Closed

Adding non-generic support #50

kjkrum opened this issue Jul 16, 2022 · 7 comments

Comments

@kjkrum
Copy link

kjkrum commented Jul 16, 2022

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
Copy link
Owner

win7user10 commented Jul 16, 2022

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

@kjkrum
Copy link
Author

kjkrum commented Jul 16, 2022

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
Copy link
Owner

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

@kjkrum
Copy link
Author

kjkrum commented Jul 19, 2022

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
Copy link

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
Copy link
Owner

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

public virtual void Library_ShouldHaveOpportunityToRegisterNonGenericTriggers()

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
Copy link
Author

kjkrum commented Feb 2, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants