dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.65k stars 3.15k forks source link

Allow mapping optional complex properties #31376

Open roji opened 1 year ago

roji commented 1 year ago

We currently always return a non-null instance for complex properties, including when all their columns are null. We may need to add an option to opt into optional complex types, where if all properties are null, null is returned instead.

See #9005 for the same thing with owned entity types.

marchy commented 10 months ago

Without this the use of complex types is extremely limited.

Since these are "identity-less" concepts by definition, they may often be un-set in the domain (ie: it's not a strong enough domain entity concept to warrant its own identifiable entity – high correlation to not always being set).

Please adopt nullability/optionality to both the JSON-backed and column-backed complex types in the future – until then back to good-old nullable string-column JSON mapping without any notion of complex types it is. 😔

roji commented 10 months ago

@marchy please note that you can already use owned entity modeling as a sort of complex type modeling; that supports both optionality and JSON columns. The new complex type modeling being introduced in 8.0 is still new and does not support everything, but we plan to improve that.

marchy commented 10 months ago

Thanks for the suggestion @roji, well familiar with the implications of Owned Types, as as you mention it has been around for years.

However they fail on the identity-less semantics of value objects (ie: multiple records cannot have the same Owned Entity values without conflicting) – the very reason complex types have been introduced.

Requiring value objects to be non-optional is highly arbitrary and extremely limiting.

🤞 Really looking forward to a big subsequent EF9 push on complex types to support optionality (this issue), type hierarchies and JSON mapping before they can become realistically feasible to adopt.

marchy commented 10 months ago

PS: One of the scenarios this may enable as well is complex type hierarchies – where each type in the hierarchy has to be modelled as optional, as each sub-class complex type would essentially be mutually exclusive to the other sub-classes (ie: optional, XOR-like semantics, as any instance can only be of one sub-class type or another, but never multiple)

roji commented 10 months ago

Requiring value objects to be non-optional is highly arbitrary and extremely limiting.

This wasn't a design decision or anything - we simply didn't have enough time to implement optional complex types for 8.0 (this is more complex than it looks).

One of the scenarios this may enable as well is https://github.com/dotnet/efcore/issues/31250#issuecomment-1776246495 – where each type in the hierarchy has to be modelled as optional [...]

Optionality and inheritance mapping are pretty much orthogonal, but we do plan to do both. Optional complex types will likely be prioritized much higher though.

marchy commented 10 months ago

Thank you @roji.

Having been on EF since v4 (ie: the first code-first version circa 2009)... I do appreciate that anything to do with ORM's is extremely difficult and takes time to cover the many scenarios that need to be considered. 😄

Appreciate the insight on the thinking in priority.

The optionals support could potentially enable hand-rolling associative enums however – since they are essentially the same as TPC and as long as you can null out all other sub-class complex objects except the the one the instance conforms to. In theory you could achieve this without any official support for inheritance – so long as the framework doesn't get in the way by detecting the abstract base class and preventing the mapping in some way (a scenario to consider).

This is the specific scenario riddled throughout our domain (and indeed many domains):

public abstract record Identity {
    public record Anonymous( string InstallationID, Platform Platform, Installation? Installation ) : Identity;
    public record Authenticated( long AccountID, Account? Account ) : Identity;
}

The moment you can model these as complex types, you can add all sorts of variances throughout the domain – for example when purchasing a ticket for something, you might have variances that have different fields based on the ticket type etc. Same thing when choosing a payment type, or pickup/delivery option, or login provider etc.

If we could map the above based on a manual discriminator column (IdentityType:string/enum) and two nullable complex types (AnonymousIdentity? and AuthenticatedIdentity?) that would already be a win.

Thinking this:

public partial class SomeEntity {
    // ... other state of the entity

    // NOTE: complex type hierarchy
    public Identity Identity {
        get => IdentityType switch {
            nameof(Identity.Anonymous) => AnonymousIdentity!,
            nameof(Identity.Authenticated) => AuthenticatedIdentity!,
            _ => throw new ArgumentOutOfRangeException( $"Unknown identity type '{IdentityType}' ),
        };
        init => {
            // NOTE: XOR-like logic to set all sub-class complex types in the TPC hierarchy to null except for the one the instance conforms to
            (string IdentityType, AnonymousIdentity? AnonymousIdentity, AuthenticatedIdentity? AuthenticatedIdentity) persistedValue = value switch {
                Identity.Anonymous anonymousIdentity => {
                    IdentityType: nameof(Identity.Anonymous),
                    AnonymousIdentity: anonymousIdentity,
                    AuthenticatedIdentity: null,
                },
                Identity.Authenticated authenticatedIdentity => {
                    IdentityType: nameof(Identity.Authenticated),
                    AnonymousIdentity: null,
                    AuthenticatedIdentity: authenticatedIdentity,
                }
            };
            (IdentityType, AnonymousIdentity, AuthenticatedIdentity) = persisted value
        }
    }
}

// HACK: Hide DAL fields away from the domain model
/*DAL*/partial class SomeEntity {
    internal string IdentityType { get; private set; }
    public AnonymousIdentity? AnonymousIdentity { get; private set; }
    public AuthenticatedIdentity? AuthenticatedIdentity { get; private set; }
}

Having EF automatically link the two parts together in TPC-style would definitely be the end-game (maybe EFC 10!) 🚀

End-game: (no partial class hackiness needed)

public class SomeEntity {
    // ... other state of the entity

    // complex type hierarchy
    // NOTE: this is still not itself optional – despite its constituent parts of each TPC-style sub-class getting mapped to optional complex types under the cover
    public Identity Identity { get; private set; }
}
roji commented 10 months ago

@marchy when we do get to complex type inheritance mapping (#31250), it will definitely not be composition-based as in your sample above, but rather inheritance-based (much like the current TPH for non-complex-types). You're free to implement what composition-based scheme you want (once we implement optional support), but that generally seems like quite a complicated way to go about things, pushing a lot of manual work on the query writer (e.g. needing to deal with the discriminator manually).

alexmurari commented 10 months ago

This is a must have for always valid domains.

E.g. a Phone value object can't be instantiated with null/empty Number value. The class protects its invariants. Nullability must be at the value object level (Phone?).

Here's an excelent article about when value objects should or should not be null: https://enterprisecraftsmanship.com/posts/nulls-in-value-objects/

jscarle commented 10 months ago

I completely agree with @alexmurari. Value Object validity dictates that its the Value Object itself that should be optional and nullable.

aradalvand commented 10 months ago

I second that complex types are currently basically unusable in a good 50% of scenarios precisely because of this limitation. Hopefully this makes it to v9; should be considered a high-priority item IMO.

marchy commented 10 months ago

Thanks @roji, that sounds ideal indeed – was just showing how the composition-based approach can let you model the associative enums scenario (ie: no shared state between different sub-classes) with just the support of optionals, rather than needing https://github.com/dotnet/efcore/issues/31250 (where I did drop an example of how that simplify things even further).

Hope that helps prioritize.

AndriySvyryd commented 10 months ago

We should set NRT annotations correctly to avoid warnings when configuring nullable complex types:

modelBuilder.Entity<MyEntity>().ComplexProperty(e => e.Complex).Property(c => c!.Details);
ManeeshTripathi14 commented 8 months ago

I am working on a project where we have already designed our domain entity based on DDD, where we have nullable complex property (value object) because those are NOT mandatory by business requirement. i was facing one issue with OwnsOne, in case of OwnsOne ef is not able to detect changes and entity state does not change to modified if we update the value of complex property( update means here replacing the old with new and NOT modifying the property of complex property individually ) so now due to this limitation i can't use complex property. Please allow NULL for complex property. so that we can desin domain entity without considering infrastructure concerns. thanks

oleg-varlamov commented 7 months ago

@ManeeshTripathi14 I absolutely agree with you. In all previous projects we used NHibernate, which works very well with DDD. But in new projects, due to good JSON support, we decided to switch to EFCore. We have been waiting for about a year for the appearance of new Complex Properties, since OwnedEntity is a very strange implementation that brought more problems than benefits. And in the end, when Complex Properties came out, we were so disappointed :( It is almost necessary for Complex Properties to support NULL values.

cjblomqvist commented 6 months ago

Preferably, do not limit this to require att least one non nullable property like it is for owned. Rather, add a "hidden" nullable column (or bool or whatever) if needed.

plppp2001 commented 4 months ago

I just ran into this, I also need ComplexType as this example:

builder.ComplexProperty(p => p.GpsLocationWhenCreated).IsRequired(false);

roji commented 4 months ago

@plppp2001 please up-vote the original post above rather than posting "I also need this" comments - that allows us to know how many people need the feature.

plppp2001 commented 4 months ago

@plppp2001 please up-vote the original post above rather than posting "I also need this" comments - that allows us to know how many people need the feature.

Done, thanks.

KillerBoogie commented 2 months ago

Is there any other workaround than not to use EF Core? DDD is a concept that is more than 20 years old. Value objects and always valid objects are an important concept for enterprise software.

It is my first EF core project and I'm frustrated with the constant issues and limitations of EF core that slow down our project. I'm considering now to introduce DBOs with primitive types that reflect the DB structure. EF core will then read and write those DBOs without any issues. I would need to write mappers from domain objects to DBOs and back to domain objects, but I would have full control over the mapping. Did anyone go this path?

roji commented 2 months ago

@KillerBoogie note that complex type support is currently quote limited - this is something we're hoping to fully implement for 10. But in the meantime, you can use owned entities instead, which support far more mapping possibilities (such as optional).

hazzinator1 commented 1 month ago

Is this still on the roadmap for EF Core 9? I remember seeing it mentioned somewhere but can't seem to find the reference to it now.

cjblomqvist commented 1 month ago

Is this still on the roadmap for EF Core 9? I remember seeing it mentioned somewhere but can't seem to find the reference to it now.

No - as is obvious from above comment ☺️ (disclaimer: I'm not a maintainer)

alexmurari commented 1 month ago

As of now, none of the approximately 20 most upvoted open issues are in the v9 milestone.

From the first page of the most upvoted issues, NativeAOT support is the only one in the v9 milestone.

I understand that the EF team sometimes leaves the best features for last, but this approach keeps us in the dark until almost the release candidate versions are out.

Is it just me, or does anyone else also feel that the communication about the roadmap and planned features is poor? If there's a place where this information is clearly available, please direct me, as I couldn't find anything.

matteocontrini commented 1 month ago

@alexmurari I don't think there's a public plan for EF 9 right now, there have been some requests to have something published in the docs page (which has been saying "coming soon" for 7 months now), with no feedback so far, unfortunately:

https://github.com/dotnet/EntityFramework.Docs/issues/4753

https://github.com/dotnet/EntityFramework.Docs/issues/4681

hazzinator1 commented 1 month ago

Looks like this isn't coming for awhile then which is a massive shame. Still want to use value objects so we're going to instead swap to using OwnOne/OwnMany with ToJson() for now when dealing with nullable value objects. Looking forward to the time when DDD has full support in EF!

hanzllcc commented 3 weeks ago

Is there any news about this feature?

cjblomqvist commented 3 weeks ago

Is there any news about this feature?

Well happen earliest next fall.

roji commented 3 weeks ago

Everyone, this issue is very much on our radar, and while we very much hoped we'd be able to deliver this (and other complex types improvements) for 9.0, this didn't pan out in terms of priorities vs. other work items.

I very much hope that improved complex types will end up on the top of our list for EF 10, stay tuned. In the meantime, owned types are the way to map JSON in EF.