Efcore: Take advantage of ownership to enable aggregate behaviors in model

Created on 3 Apr 2015 · 18Comments · Source: dotnet/efcore

This an "epic" issue for the theme of automatic aggregate behaviors in the model based on ownership. Specific pieces of work will be tracked by linked issues.

Proposed for 6.0

Backlog

[ ] A delete orphans/cascade delete policy an can be applied by convention to owned entities (#10179)
[ ] For example, we could implement client-only cascade delete for aggregates, which should be fully loaded and hence won't have issues with some entities needing to be deleted in the store. However, also consider that an aggregate cannot have true cycles, and hence the SQL Server limitation might not be as bad. (#12168)
[ ] A cascade update could also be applied (#10551)
[ ] Smarter logic can be applied to infer the intended state of objects while merging a detached graph into a context: non root objects within the aggregate that are no longer reachable can be marked as deleted, while non root objects that appear for the first time within the aggregate being merged can be marked as added. The same assumptions can't be made with the same confidence across aggregate boundaries. This could be handled by a new method: 'Merge()' (related to #5536)
[x] Having this logic we could create shadow keys for owned collections, since we'll be able to figure out the appropriate changes to the store to reflect the desired state of the aggregate in disconnected and seeding scenarios without having the key values. (#13295)
[ ] Concurrency control could be delegated to the aggregate root when it's not mapped to the same store object (#18529)
[ ] Ability to pass owned entities in the constructor of the aggregate root (#12078)
[ ] Decide whether it should be possible to query only a part of the aggregate with an implicit AsNoTracking (#17836)
[ ] Make EntityEntry methods (GetDatabaseValues, Reload, etc) aggregate friendly (#13890)
[ ] Allow to configure an owned navigation as lazy (#10787)

Original issue

Define included properties

When you get an entity from the database and you also want to get a "child entity", you need to pass that "child object" as an expression in order to also get it from the database (when not using lazy loading).

public class Parent
{
    public Child Child { get; set;}
}
public class Child
{   
}

var dataContext = new DataContext();
var parents = dataContext.Parents.Include(p => p.Child).ToList();

Would it be possible to add an new attribute that will be read by the datacontext and add the include statements automatic?

public class Parent
{
   [Include]
    public Child Child { get; set;}
}
public class Child
{   
}

var dataContext = new DataContext();
var parents = dataContext.Parents.ToList();  // Child entity is also read from the database

area-aggregates area-model-building needs-design type-enhancement

Source

kennytordeur

👍20 😕1 😄1

Most helpful comment

Just because an entity's property is an aggregate part of the entity, doesn't mean you _always_ want to include it. But _sometimes_ you do. Instead of [Include], consider introducing [Part] to indicate a part-whole relationship. After appropriately dispersing [Part] throughout your model, you query like this:

var parents = dataContext.Parents.IncludeParts().ToList();

This recursively loads parts and parts of parts, etc..

Also consider adding a RequiresFilter property to [Part] like this:

public class PartAttribute : Attribute
{
    public bool RequiresFilter { get; set; }
}

Setting this property to true indicates that a filter must be specified for the property as part of a query with IncludeParts, or else an exception is thrown upon loading.

HappyNomad on 3 Nov 2016

👍3 👎1

All 18 comments

@kennytordeur this is a very dangerous feature. In a large application someone would "think" they always want to load B & C together with A. However in other parts of the code this creates issues due to the fact that it creates overly complex queries (perf) or loads more than necessary (attach/detach scenarios over the wire/web services) or breaks some logic IE: someone checks if B is not loaded (== null) then loads B and B1+B2 etc (an entire graph) after this change application is broken because B is loaded but not B1+B2.

Also there are alternative solutions to this, like:
A repository pattern:
List<Parent> GetParents(){return datacontext.Parents.Include(p => p.Child);}

Which is also more flexible because it allows you to customize the query with more than just includes.

popcatalin81 on 3 Apr 2015

👍2 👎1

@kennytordeur Your request maps to some of our thinking around being able to declaratively define the shapes of aggregates in an EF model. We don't know exactly how this feature would look like or the timeframe in which we will try to tackle it. In the meantime I will move your suggestion to our backlog, since it seems to provide one way to define an aggregate that we could consider.

While I agree with @popcatalin81 that in many applications it is dangerous to assume that aggregates are fixed and always need to be loaded together, I also believe that many applications can take advantage of this behavior and be constrained to it. After all, loading full aggregates only is the primary behavior you get when you use a document database. Of course if the database is relational, the overhead of loading from multiple tables is higher, but at least in EF Core the queries that we would generate are much simpler than in EF6.

As @popcatalin81 mentioned, writing repository methods that execute queries with calls to Include can address the requirement of loading fixed shapes of graphs without sacrificing the fine grained control. However a more declarative way of specifying the shape of the aggregate would allow for other automatic behavior, e.g.:

A delete orphans policy can be automatically applied to non root entities in the aggregate.
Smarter logic can be applied to infer the intended state of objects while merging a detached graph into a context: non root objects within the aggregate that are no longer reachable can be marked as deleted, while non root objects that appear for the first time within the aggregate being merged can be marked as added. The same assumptions can't be made with the same confidence across aggregate boundaries.

divega on 18 Apr 2015

cc @rowanmiller in case he knows if we already have something covering aggregates in the backlog (I couldn't find it) and to suggest whether we should have it as a separate item to this.

divega on 18 Apr 2015

I thought we had something... but I can't find it... so we should just keep this one open to track it :smile:

rowanmiller on 20 Apr 2015

Thanks. I moved it to backlog and changed the title to make it more general about aggregates.

divega on 20 Apr 2015

Thanks. It would be nice to define fetch strategies for aggregates.

kennytordeur on 24 Apr 2015

I have worked around this issue using extension methods that include a defined subset of the children I want to load.

public static IQueryable<Adult> IncludeAll (this IQueryable<Adult> query)
{
    return query.Include(f => f.Relationships).ThenInclude(f => f.Child)
        .Include(f => f.CreatedBy)
        .Include(f => f.ModifiedBy);
}

GrantErickson on 14 Apr 2016

I really, really liked the idea implemented in this package for EF6. Basically it's using expressions to define what is part of the aggregate and what is referencing other aggregates of the model, e.g.:

context.UpdateGraph(order, map => map
    .OwnedCollection(ord => ord.LineItems)
    .AssociatedItem(ord => ord.Customer));

Unfortunately the project is a) abandoned and b) not ported to EF Core. A package inspired by this exists, but it seems to be in an early unstable phase.

marcwittke on 14 Oct 2016

var parents = dataContext.Parents.IncludeParts().ToList();

This recursively loads parts and parts of parts, etc..

Also consider adding a RequiresFilter property to [Part] like this:

public class PartAttribute : Attribute
{
    public bool RequiresFilter { get; set; }
}

Setting this property to true indicates that a filter must be specified for the property as part of a query with IncludeParts, or else an exception is thrown upon loading.

HappyNomad on 3 Nov 2016

👍3 👎1

Note for triage: @bricelam called out that most of what is described in this issue is either implemented already (owned entities, [Owned] attributes) or captured elsewhere:

support for collections of owned entities #8172
support for collections of scalars (using value converters) #4179

I would like to propose that we ~~close this and~~ make sure additional issues exist for other aggregate goodness, for example:

Delete orphans/cascade delete policy can be applied by convention to owned entities (if it isn't the case already).
Smarter logic can be applied to infer the intended state of objects while merging a detached graph into a context: non root objects within the aggregate that are no longer reachable can be marked as deleted, while non root objects that appear for the first time within the aggregate being merged can be marked as added. The same assumptions can't be made with the same confidence across aggregate boundaries.
Concurrency control could be delegated to the aggregate root.

divega on 17 May 2018

👎1

Consider key configuration--see #12078

ajcvickers on 24 May 2018

Note: consider client-only cascade delete for aggregates, which should be fully loaded and hence won't have issues with some entities needing to be deleted in the store. However, also consider that an aggregate cannot have true cycles, and hence the SQL Server limitation might not be as bad. See #12168

ajcvickers on 4 Jun 2018