SteveDunn / Vogen

A semi-opinionated library which is a source generator and a code analyser. It Source generates Value Objects
Apache License 2.0
888 stars 46 forks source link

EF Core Support Extension #1 #512

Closed Aragas closed 1 year ago

Aragas commented 1 year ago

Create a built-in EF Core extension class for providing the correct HasConversion overload.

public static class VogenExtensions
{
    public static PropertyBuilder<VOTYPE> HasVogenConversion(this PropertyBuilder<VOTYPE> propertyBuilder) =>
        propertyBuilder.HasConversion<VOTYPE.EfCoreValueConverter>();
}
SteveDunn commented 1 year ago

Hi - thanks for the suggestions. For this, I think it would have to be a separate NuGet package as the code that Vogen generates is only additive; everything EF core related generated by Vogen is specific to a particular type. Also, Vogen doesn't directly need a dependency on EF Core, so if we were to add this to Vogen.SharedTypes, then Vogen itself would have to have that dependency.

But there certainly seems enough appetite for more EF Core support, so maybe a new Vogen.EFCore package.

Aragas commented 1 year ago

Hi - thanks for the suggestions. For this, I think it would have to be a separate NuGet package as the code that Vogen generates is only additive; everything EF core related generated by Vogen is specific to a particular type. Also, Vogen doesn't directly need a dependency on EF Core, so if we were to add this to Vogen.SharedTypes, then Vogen itself would have to have that dependency.

But there certainly seems enough appetite for more EF Core support, so maybe a new Vogen.EFCore package.

Vogen is already using EF Core if EfCoreValueConverter is set. I assumed this check could be used to add to a shared static class with the methods

SteveDunn commented 1 year ago

Unfortunately not. The source generator is only additive, so it can only extend the type decorated with the ValueObject attribute. It can declare inner classes, which is does with EFCore.

Actually, while I was writing this, I think I figured out what you meant. For each class it generates, it would also generate a static method for the extension, e.g. for a Value Object of type Age, it would generate:

public static PropertyBuilder<Age> HasVogenConversion(this PropertyBuilder<Age> propertyBuilder) =>
        propertyBuilder.HasConversion<Age.EfCoreValueConverter>();

If that's the case, then it should be trivial to add. I guess it's not a breaking change for anybody?

Aragas commented 1 year ago

@SteveDunn it should not be a breaking change, since we are adding an extension method that needs an opt-in to use

SteveDunn commented 1 year ago

I've added this. It's wrapped in a static class that has the same accessibility as the containing class. It is #ifdef out for .NET 4.6.1 because the signatures are different and I didn't feel that spending the time making it compatible is worthwhile considering the age of it.

It's building now, so I'll do a pre-release nuget package soon.

SteveDunn commented 1 year ago

This is now implemented in 3.0.23. Thanks for you support!

CheloXL commented 1 year ago

Just a comment, not sure if worth opening a new issue. The above code works for a specific property, but usually when you register a converter/comparer, you do for any properties of a specific type (at least that's what I do).

Without changing anything, you could simply add another extension method, that reads like:

public static PropertiesConfigurationBuilder<CityId> HasVogenConversion(this PropertiesConfigurationBuilder<CityId> propertyBuilder) =>
        propertyBuilder.HaveConversion<CityId.EfCoreValueConverter, CityId.EfCoreValueComparer>();

And with that you have covered both cases: registering a single property, or registering any property of the given VO type.