Vogen icon indicating copy to clipboard operation
Vogen copied to clipboard

Document if/how Vogens ValueTypes can be used together with EntityFrameworkCore

Open Blackclaws opened this issue 1 year ago • 21 comments

Describe the feature

Currently the standard approach to using ValueTypes fails due to the inability of EntityFrameworkCore to use a value type as an id for example.

Guidance on how to do this would be appreciated. If I figure it out and have some time I might contribute a solution.

Blackclaws avatar Jul 06 '22 14:07 Blackclaws

So preliminary results:

You cannot use structs as EFCore currently only supports classes as owned entities (https://github.com/dotnet/efcore/issues/9906) which is essentially what value types are to EFCore (https://docs.microsoft.com/en-us/dotnet/architecture/microservices/microservice-ddd-cqrs-patterns/implement-value-objects).

You then need to configure the items as owned so that they don't need an id themselves:

       builder.OwnsOne(e => e.ValueObjectProperty);

That itself will however not really store the entities, as EFCore only treats properties with either a configured backing field or with a setter. To configure this configure it in the Owned statement:

        builder.OwnsOne(e => e.ValueObjectProperty, navigationBuilder =>
        {
            navigationBuilder.Property(t => t.Value).HasField("_value");
        });

An alternative solution to the owning is to create a custom conversion, which is what you have to do for objects of Id type anyway:

 builder.HasKey(t => t.Id);
 builder.Property(e => e.Id).HasConversion(v => v.Value, v => ValueObjectId.From(v));

I think there is room for improvement whereby Vogen source generates either the conversions or the owning as helper functions. I personally kind of lean towards conversion right now as that seems more concise. You do run into the issue that Vogen validation runs on data from the database then however. Which means invalid data (even purposefully invalid data) will then result in an exception. This could be solved with a custom converter that handles those specific cases.

Blackclaws avatar Jul 06 '22 15:07 Blackclaws

VoGen already handles that. I'm using it in my projects without any issues. You just need to do builder.Property(x => x.Id).HasConversion(new ValueObjectId.EfCoreValueConverter());

CheloXL avatar Jul 07 '22 12:07 CheloXL

Thanks for the report @Blackclaws , and thanks for describing the fix @CheloXL . The documentation is a lacking at the moment, but it's something I'm hoping to get around to.

I'll also make this functionality clearer in the samples.

SteveDunn avatar Jul 08 '22 05:07 SteveDunn

@Blackclaws - let us know if you're happy with suggestion by @CheloXL. If so, I'll close this Issue.

SteveDunn avatar Jul 08 '22 05:07 SteveDunn

VoGen already handles that. I'm using it in my projects without any issues. You just need to do builder.Property(x => x.Id).HasConversion(new ValueObjectId.EfCoreValueConverter());

I honestly didn't see that there was a value converter for EfCore also available, sorry for making such a big fuss about it. I think the documentation could have a general section about integration with EfCore as I simply didn't think to look under value conversion since I had to figure out what to do so EfCore would accept these entities in the first place.

May I suggest an extension method that automatically registers the OwnsOne + Conversion?

Regarding this specific conversion one issue that it brings with it is a dependency of the entity declaring assembly on EfCore. This is something we might just have to live with, but its usually frowned upon a bit because the Domain level should be as free from infrastructure concerns as possible. EfCore is definitely infrastructure. Its probably not possible to move it out of there because VoGen by design only has access to the private constructor needed here within the defining assembly.

All in all I think you can close this issue, a specific section on how EfCore works together with VoGen would be great to have as a general cookbook for this!

Blackclaws avatar Jul 08 '22 09:07 Blackclaws

I have a lot of issues getting this to work.

What I want is for the ValueObject to be the only thing I interact with from start to finish (Outside world -> Endpoint -> Service -> Repository -> Database (and vice versa)). Currently Endpoint -> Service -> Repository (and vice versa) works, but it breaks on either end. This interaction has to be transparent to the user of the API, and ideally also has no effect on the database schema

I've added the following to OnModelCreating: image

Soup looks like the following (I've tried with and without [DatabaseGenerated(DatabaseGeneratedOption.Identity)] on the Id property: image

SoupId is like this: image

These are the global settings: image

I get Use of unitialized Value object on the first instance of SaveChanges here: image

Manually assigning the Id does work, but that is far from an ideal solution.

Additionally, is there a way to use the ValueObject(of type int) as if it was an int in the context of an API Endpoint? This is a photo from Swagger (This endpoint is made using FastEndpoints, which uses NSwag for Swagger generation, if that matters) image

KennethHoff avatar Aug 15 '22 18:08 KennethHoff

@KennethHoff I never used values generated by DB as keys (I always create the Ids) so I really don't know how to answer your first question. As for the second, at least using Swashbuckle, I defined them as following:

options.MapType<CityId>(() => new() {Type = "string", Nullable = false, Format = "int64"});

Be aware that in my case, all longs are mapped as strings (js can't handle longs). Options is of type SwaggerGenOptions.

Maybe in your case it should be something like (not tested):

options.MapType<SoupId>(() => new() {Type = "integer", Nullable = false, Format = "int32"});

Hope that helps.

CheloXL avatar Aug 16 '22 11:08 CheloXL

@KennethHoff Ok tried this for EF and seems to be working...

First, you have to create a key value generator, like the following

public class SoupIdValueGenerator : ValueGenerator<SoupId>
{
        private int _id;
        public override bool GeneratesTemporaryValues => true;
        public override SoupId Next(EntityEntry entry)
        {
                _id += 1;
                return SoupId.From(_id);
        }
}

Then you can register this generator on your property by simply doing .HasValueGenerator<SoupIdValueGenerator>().ValueGeneratedOnAdd()

@SteveDunn Maybe the generator can be added to the generation options of the VO? Not sure how to define the way the generation works (What I've done, as a test, is fine, but doesn't works for a real system, as every time the system restarts it will start generating existing ids). In my projects I use either UUIDv7 for ids (as strings), or the snowflake algorithm (as longs).

CheloXL avatar Aug 16 '22 12:08 CheloXL

@CheloXL

The Swagger change was very simple with NSwag - thank you!.

However, the Entity Framework related one I did not get to work.

I swapped over to using Guids(ints did not work either, btw), and I still get the following error:

Use of uninitialized Value Object at: <....>, where <...> is the SaveChanges method.

image image image (The last one is with GeneratedTemporaryValues tested with both true and false)

Other than those three, no changes have been made.

Is the solution to manually create the Guids? I'd really like to avoid that if possible, as I feel like that shouldn't be necessary, and only coincidentally works because Guids are "guaranteed" to be unique

KennethHoff avatar Aug 16 '22 19:08 KennethHoff

I think one possible solution. That would require a change to the ValueConverter however (or alternatively that you implement your own value converter) would be to not access .Value but ._value instead. That bypasses the initialized check. Alternatively, as that is pretty much the same thing, you could introduce an instance Unintialized.

[Instance("Invalid", "Guid.Empty")]

Using this instance should have the same effect as using a default Guid as Guid.Empty is the default value.

EDIT: Just tried that. Doesn't have the same effect unfortunately.

Second Edit: I'd like to point out that client side Guid generation is basically what Efcore does by default anyway.

Blackclaws avatar Aug 23 '22 08:08 Blackclaws

: I'd like to point out that client side Guid generation is basically what Efcore does by default anyway.

I like to use UUID Version 7, or other forms of UUID so I always pass in my own ValueGenerators for EfCore -- just food for thought.

jeffward01 avatar Jan 09 '23 23:01 jeffward01

You do run into the issue that Vogen validation runs on data from the database then however. Which means invalid data (even purposefully invalid data) will then result in an exception.

@SteveDunn

Would it be possible to have some kind of escape hatch for such use cases, where validation could be disabled within certain contexts (such as reading from a database)?

One could argue for separate read/write models, but that does add a certain amount of complexity/duplication.

It's really not ideal for the validation to be running on every read.

We currently use simple converters like:

.HasConversion(
    v => (string)v,
    v => (ArticleTitle)v);

But this of course triggers validation. Maybe something like:

ArticleTitle.From("title", validate: false);

If someone has better ideas, I'm all ears.

glen-84 avatar Feb 07 '23 15:02 glen-84

@glen-84 It isn't running on every read. Check the deserialization strictness configuration paramter. You can skip validation already when using the generated EfCoreValueConverter

Blackclaws avatar Feb 07 '23 15:02 Blackclaws

@glen-84 It isn't running on every read. Check the deserialization strictness configuration paramter. You can skip validation already when using the generated EfCoreValueConverter

Oh, this isn't documented in the README.

Unfortunately, this requires you to use the generated EfCoreValueConverter, creating a dependency on EF Core in the domain layer. 😞

glen-84 avatar Feb 07 '23 15:02 glen-84

@glen-84 It isn't running on every read. Check the deserialization strictness configuration paramter. You can skip validation already when using the generated EfCoreValueConverter

Oh, this isn't documented in the README.

Unfortunately, this requires you to use the generated EfCoreValueConverter, creating a dependency on EF Core in the domain layer. disappointed

That's true. Though I've found that this dependency doesn't really change anything unless you want to share your domain layer via nuget packages you don't really have "drawbacks" from having this dependency.

If you really don't like it you can just create a Deserialize public static method that calls the private one and use that in your own value converter.

Blackclaws avatar Feb 07 '23 19:02 Blackclaws

Hi - sorry about the missing documentation. I personally don't do anything that requires EF. The times I do need to read from infrastructure and convert to domain objects, I tend to use an explicit 'anti-corruption layer'. Yes, it's more work, but it's explicit and it's clear what needs changing when business/validation rules change at a different pace than the data itself.

But as @Blackclaws says, DeserializationStrictness gives you the ability to control what happens on deserialization.

SteveDunn avatar Feb 07 '23 21:02 SteveDunn

@Blackclaws Thanks for the info. 👍

@SteveDunn

The times I do need to read from infrastructure and convert to domain objects, I tend to use an explicit 'anti-corruption layer'.

Apologies for going slightly off topic, but do you have an example of what this looks like?

glen-84 avatar Feb 08 '23 08:02 glen-84

Hi @glen-84 , the name 'anti-corruption layer' sounds complicated, but it essentially means 'map types from one layer to another'.

My flow is something like:

Infrastructure.CustomerDto[] customers = ReadCustomersFromTheDatabase();
Domain.Customer[] domainCustomers = Map(customers);

private Domain.Customer[] Map(Infrastructure.CustomerDto[] databaseCustomers) {
    return databaseCustomers.Select(Map).ToArray();
}

private Domain.Customer Map(Infastructure.CustomerDto customer) {
    return new Domain.Customer {
       Id = CustomerId.Fromat(customer.Id),
       Name = CustomerName.Fromat(customer.Name ?? CustomerName.NotSet) // *1
    }
}

*1 is an example scenario where, historically, we might've added customers without a name, and we want to represent that in our domain as a name that's 'not set' (as opposed to null). In the domain layer, if we cared, we could then use:

if(customer.Name != CustomerName.NotSet) ...

SteveDunn avatar Feb 12 '23 08:02 SteveDunn

So it's basically having a separate persistence layer?

We've decided not to use VOs for our entity properties, but still use them for parameters to entity methods. For example:

public sealed class Article : AggregateRoot
{
    public long Id { get; private init; }               // Use primitive here.
    public string Title { get; private set; } = null!;  // Use primitive here.

    private Article() { }

    public static Article Draft(ArticleId id, ArticleTitle title)
    {
        return new Article()
        {
            Id = id.Value,        // Take the underlying value here.
            Title = title.Value   // Take the underlying value here.
        };
    }
}

In this way, the arguments to Draft are still validated.

We plan to look into other architectures in the future, like persistence layers and separate read/write models.

Thanks.

glen-84 avatar Feb 12 '23 08:02 glen-84

So it's basically having a separate persistence layer?

Pretty much. It's all in the 'infrastructure' layer (in the Onion/Clean Architecture world). Similar to your code; the mapping is either in the aggregates, or the code that builds the aggregates.

I'm also a fan or read/write models. I find them useful not just in infrastructure, but also the domain. e.g. instead of a an IArticleRepository, I split these into 'role based interfaces' named IStoreArticles and IHaveArticles

SteveDunn avatar Feb 18 '23 06:02 SteveDunn

Looking to close a few issues. With the changes made to Vogen and the updated documentation, is there anything missing in the documentation, or can this be closed?

SteveDunn avatar May 01 '24 07:05 SteveDunn