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

EF Core Support Extension #1 #512

Closed
Aragas opened this issue Oct 28, 2023 · 7 comments
Closed

EF Core Support Extension #1 #512

Aragas opened this issue Oct 28, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@Aragas
Copy link

Aragas commented Oct 28, 2023

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>();
}
@Aragas Aragas added the enhancement New feature or request label Oct 28, 2023
@Aragas Aragas changed the title EF Core Support Extension EF Core Support Extension #1 Oct 28, 2023
@SteveDunn
Copy link
Owner

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

Aragas commented Oct 30, 2023

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

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

Aragas commented Oct 31, 2023

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

@SteveDunn
Copy link
Owner

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 added a commit that referenced this issue Oct 31, 2023
feat: implement #513 EF Core add `EfCoreValueComparer'
@SteveDunn
Copy link
Owner

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

@CheloXL
Copy link

CheloXL commented Nov 3, 2023

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.

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

No branches or pull requests

3 participants