Add() behavior with graphs

[divega]

Unlike in previous versions of EF, currently calling Add() on an object using EF7 won't mark any of its related objects as added.

We know this is not the only behavior that customers expect from Add() (see #2434 and #2387 as examples), however this is certainly a behavior we wanted to expose because in past versions of EF we had a lot of feedback from customers not expecting certain related objects to be marked as added.

On one hand this kind of feedback has been very common among customers trying to work with disconnected graphs: Since EF does not provide a high level API for disconnected graphs, in those situations it is quite usual to use an algorithm that walks the graph and explicitly marks each individual object with a specific state, making it desirable to use an API that won't have any side effects on the state of other objects.

On the other hand even customers that used Add() in a standard unit of work pattern (and for whom it is very useful that the method automatically adds related objects at once) were often surprised that previous versions of EF would mark the whole graph as added.

From the perspective of those customers we seem to fundamentally lack the ability to differentiate between objects which should be added from those that shouldn't, however there doesn't seem to be a single rationale that informs their expectations.

As a general approach, we have talked about having two different APIs for the two different behaviors, e.g.:

• Add() for single objects would be for customers writing lower level logic and needing the finer control
• Add() for graphs would be for customers using the context on a unit of work pattern and wanting the API to smartly mark additional objects as added

We could simply call them AddSingleObject() and AddGraph(), but another option is to just make the Add() method the one that tries to automatically infer what should be added and have a separate lower level/fine control API that allows to set the state to added on a single object, e.g. what we already have in the EntityEntry.State.

Regardless of the shape of the API, we need to figure out what behavior to use for Add() for graphs.
Here are some options:

1. Leverage containment/aggregates:
   Aggregate-oriented databases (e.g. document databases) offer a good frame of reference. Adding an aggregate root object inherently adds the whole aggregate but doesn't add anything external to the agregate.

   From an O/RM perspective the key to emulate this behavior would be to have knowledge of the shape of the aggregate. As mentioned in the comments below, GraphDiff is an example of an extension for EF6 that layers this type of knowledge on top of the EF model to enable smarter decisions on state transitions, e.g. if a navigation property is "painted" with a call to AssociatedCollection() or AssociatedEntity(), then adds won't spread through the relationship in the direction of the navigation property. On the other hand if the navigation property is touched by a call to OwnedCollection() or OwnedEntity() then adds will spread through that relationship in the direction of the navigation property.

   We currently don't have the ability to model aggregates on EF (it something that we have talked about adding in the future, see Take advantage of ownership to enable aggregate behaviors in model #1985), but we actually do have at least a rudimentary understanding of child vs. non-child objects, which we use (or are currently planning to use) to drive cascading/orphan deletes for Remove(). In that sense @tonysneed offered interesting feedback some time ago saying that he expected the behavior of Add() for graphs to be more consistent with Remove():

   with the vCurrent behavior ... if I add a territory to an employee, then the territory will be marked as Added and EF will attempt to add it to the Territories table. If I want to just add the relationship to an existing territory, then I need to explicitly mark it as Unchanged, which makes sense but isn't all that intuitive. It's also inconsistent with the behavior for removing entities from a relationship, where removing a territory from an employee will not mark it as Deleted, without the need to explicitly mark it as Unchanged.

   We could indeed use this kind of rudimentary "aggregates by convention" based on FK relationships as the basis of the behavior of Add() for graphs. If we later improve EF to be more aware of aggregates, Add() for graphs would automatically pick up the new information.

2. Leverage key values:
   Another approach @rowanmiller suggested to explore is to use key values to determine which objects need to be added.

   Note that basing the rules just on aggregates, the following code would insert a new Post, but would assume the associated Blog is existing:

   var blog = new Blog { Title = "Blogging from the leaf" };
   context.Posts.Add(new Post { Contents = x, Blog = blog});

   In order to get the Blog object also added, you would need to indicate it explicitly:

   var blog = context.Blogs.Add(new Blog { Title = "Blogging from the leaf" });
   context.Posts.Add(new Post { Contents = x, Blog = blog});

   Note that if the key of Blog is generated, we would actually have enough information to know whether it is an existing entity by just looking at its key. Indeed, if we leverage key values we can infer what objects need to be Added irrespective of the shape of the aggregate and the direction of relationships: assuming generated keys, the first snippet would add both Blog and Post, regardless of the fact that Blog is the principal and Post is being added.

   Note that this would have equivalent semantics to the aggregate approach for a number of cases, e.g. for identifying relationships any child object of an added principal should have temporary keys too and therefore would become added automatically.

   The approach per se won't work well for entities that don't have generated keys as those will look to EF as existing entities. We need to understand if that is ok or whether we would fallback to a different strategy.

3. Hybrid:
   The idea is to figure out a way in which can leverage both aggregates and keys, e.g. leverage keys but fallback to aggregates for objects that don't have generated keys.

Modified objects on the edge
It seems that for all cases any dependent entity that we won't add but that points to a principal that is added should be updated to be able to update the FK value.

[rowanmiller]

We should also consider whether we should mark entities without a key set as added during Attach

[biqas]

Hi,

maybe this article is also giving an alternative view on the topic

http://blog.brentmckendrick.com/introducing-graphdiff-for-entity-framework-code-first-allowing-automated-updates-of-a-graph-of-detached-entities/

[divega]

@biqas indeed, I think it is related, even if Add() might not be a general purpose method for disconnected graphs.

The OwnedCollection(), AssociatedCollection(), AssociatedEntity(), etc. methods in GraphDiff outline the shape of the aggregate on top of the model. Once you have that information you can make better decisions about how states should spread through which relationships as I mentiined earlier.

It is possible that we won't have all the expressiveness necessary to define an aggregate in EF 7.0.0 RTM but just a convention that should figure out the expected behavior in the majority of cases.

[divega]

Updated write up to reflect some more thinking and feedback from @rowanmiller.

[ilmax]

Working with disconnected graph in previous version of EF was quite challenging, so for vNext I would like to see 2 different api, one for graphs, and another one for signle entities. Please try to avoid "magic" and instead give us two levels of api.

Regards, Max

[divega]

Please try to avoid "magic" and instead give us two levels of api.

From this statement I assume you definitively want one of the two levels of APIs to add single object without side effects (which is covered in our thinking). Do you have any feedback on what the second level of API should do?

[ilmax]

You're right! For the second level api (the graph one) It's fine for me that Add keep marking all objects as added, the unfortunate part (at lest IMHO) is with update and delete, in those cases it's quite difficult to make the right choice without knowing the aggregate boundaries, so maybe we could just avoid this api (update/delete) for graphs.

Regards, Max.

[divega]

Thanks for the feedback.

FWIW, "delete for graphs", e.g. cascade delete and deleted orphans behaviors after a call to Remove() are a quite desirable feature we have all the intention to support for EF7 RTM. See #333.

[rowanmiller]

We've implemented everything we concluded from design meeting etc.

[bonsmik]

Should graph behavior browse through all related objects and also their relations? I tried to add an object and result was only related objects were saved not their relations.

using "EntityFramework.Sqlite": "7.0.0-rc1-15945"

[julielerman]

With Beta8 (testing with both SQLServer & InMemory api), a child in a graph is getting added now. Not what I expected. I'm having a hard time working out if the above discussion says that you went back to this behavior.
My scenario is
create "parent"
parent.add(new child)
context.parents.Add(parent)

child state is added.

Entry.State seems to work as expected (addressing only the root but I've only tested Add so far)

[rowanmiller]

@julielerman that is expected now, dependents get added as well. There is a new GraphBehavior enum which can be passed to the API to change this too.

[julielerman]

Thanks. I will look harder for discussion of this so I'm totally clear on behaviors. I looked but apparently not thoroughly enough. :)

[julielerman]

P.s. Cool enum!

[Yanal-Yves]

@julielerman I have successfully tested your scenario (Parent has an ICollection of Child, parent.Add(child)) with Beta8 (with both SQLServer & InMemory api)

Unfortunatelly it does not work for all use cases, here is my scenario:
I have created a Parent class and a Child class.
If Parent is having a reference to Child (so not an ICollection of Child like in your scenario), the Child is not saved to the database:
create "parent"
parent.child = new Child(); // so not parent.add(new child) like in your scenario
context.parents.Add(parent);
context.SaveChanges()
Parent is saved to SQL server but not Child.

I tried on EF6 and I don't reproduce this issue on EF 6, I was expecting GraphBehavior.IncludeDependents to behave like EF 6.

@rowanmiller Am I missing something?

Yanal

[julielerman]

my experience with beta 8 is that the default behavior of Add will add everything in the graph. There is a new enum parameter that allows you to specify root only.But that is not the default. I may have only tested with collection navigations but not reference navigations. Also, I have have only tested with with inmemory provider lately. I will go have a look.

[julielerman]

Verified that doing this with collection items passes where I add the "parent" to the context. However, my first test of doing this with a reference property fails ... but I wonder if it is because the default graph behavior on Add is includedependents and my test is starting with a dependent and setting its principal. It is not how I would really code this in my domain as the principal would be responsible for its dependents. I will go modify my model to include a one:one so I can test adding a principal and see if the related object gets included..

[julielerman]

Ahh even easier was to check the design meeting notes! (https://github.com/aspnet/EntityFramework/wiki/Design-Meeting-Notes-(August-27,-2015)) Yes, the Add will only add dependents, not principals as is the case I was testing above. So now the question is: in your test, Yanal-Yves , are you using add method on the principal or dependent? I will finish my test of the reverse just to be confident. :)

[Yanal-Yves]

Thank you for your answer Julie!
I have just pushed my test here: https://github.com/Yanal-Yves/ConsoleApp2, I will now read deeper your answer to see if I am doing something wrong.

[Yanal-Yves]

So if my understanding is correct I am using the add method on the principal and it should save the children whatever relation the children have with their parent (1:1, 1:many). I am wondering if there is a bug here on beta8?

[divega]

@Yanal-Yves I am not aware of any bugs in this area. Could you create an issue including a repro?

[julielerman]

FWIW I tested 4 scenarios:

1. Parent with collection. Add(newParent) that has one or more new children attached. All get added.
2. Parent with collection. Add(newchild) that has a newparent attached. Only child gets added, not the principal. As expected.
   Note: One:one EF7 doesn't have "principal/dependent" definition in fluent but is inferring it by assiging FK on one of the ends. I can't figure out what is driving this. Pro is that we can get one:one without having to configure. Con is I have to go look for more details about what's going on. :) So given I can tell which is principal & dependent based on the db schema,:
3. when I use ADD(principal), the attached new dependent instance is getting added.
4. when I use ADD(dependent), with an attached new principal, I'm so far getting a failure because EF is inserting the dependent first before there is a key available for the "parent". It's passing in -1 for the key value.

Is this what we should expect? @divega can you tell me what I'm missing wrt how EF is inferring principal and dependent besides reading my mind (cuz it got it right) :)?

[divega]

Yes, all those cases sound like what we have built in EF7: adds propagate only from principal to dependent.

And the entity that owns the FK is the dependent.

[rowanmiller]

@julielerman if you look at the overload of Add there is one that takes an enum for the two behaviors we support. The options are IncludeDependents and SingleObject, the former being the default.

BTW with a one:one, we do infer from the FK, but if we get it wrong you can use the generic overload of HasForeignKey to tell EF which entity the FK is on.

[julielerman]

Thanks Rowan and Diego. I was familiar with the enums, but hadn't thought clearly enough on the fact that you explicitly use the term dependents! Makes sense. It's not "all related objects". Big difference. :)
Still confused re inferring from the FK for one to one. In two classes, they each have an ID property and they each have a navigation to the other. But I'm doing that using the old rules. I'm going to try again by doing it the way I've always wanted to .. rather than a shared pk which in the dependent becomes pk and FK, I will define an explicit FK. Fwiw when I did not do that, EF did the old behavior of inferring that the pk on the dependent is a pk+FK column. I'll go experiment more and not presume old conventions here! Happily!!

[Yanal-Yves]

@divega, I have created an issue as requested #3724 and provided a code sample in a personal github repository. I hope it's understandable.

[julielerman]

Final note:
Using the new formulation for one to one that allows FKs! Nice. And adding a graph from the prinicipal works great. Adding a graph from the dependent (without changing the default) throws which IS GREAT! Why? Because otherwise it would just do what it wants without letting me know ...silent failure and unexpected behavior. Having it throw is so nice. I wonder if someone clever with Roslyn could detect code that was attempting to use Add (with default graph behavior) on a dependent and highlighting the problem at design time?

[Yanal-Yves]

@julielerman, I am not sure to understand, could you share some code that shows what you are explaining in your last comment?
What is wrong with what I am doing in my code sample https://github.com/Yanal-Yves/ConsoleApp2 and why is it working as I expect on EF 6 but not on EF7?

[julielerman]

First thing to remember is that ef7 is not ef6 so don't expect behavior to be the same.

Code looks ok but I may be missing a subtlety. Note that with ef7, one to one relationships now support FKs. Right now your childa ID property is both an FK back to parent and its own pk. It's possible that as Rowan suggested, EF is guessing wrong and doesn't see that childa is the dependent.

Can you isolate the problem and take the addition of the childbs in program.cs and see what happens?

On Nov 11, 2015, at 3:40 PM, Yanal-Yves [email protected] wrote:

@julielerman, I am not sure to understand, could you share some code that shows what you are explaining in your last comment?
What is wrong with what I am doing in my code sample https://github.com/Yanal-Yves/ConsoleApp2 and why is it working as I expect on EF 6 but not on EF7?

—
Reply to this email directly or view it on GitHub.

[julielerman]

yep, EF needs some help configuring your model. It is assuming that childA is the "parent" and that the Parent type is the "child" aka dependent. Look at the schema of the parent table created by your model:

I think EF6 would infer a 1:0..1 with this. (1:1/0..1 mappings are too hard for me off the top of my head). But it looks like EF7 isn't doing that. Which makes sense since much of the relationship magic has changed. (Am I getting this right, @divega ?)

i don't know how you can do this without at least a navigation property back to the parent. You can't add a fluentapi configuration without it either. I know you are trying to make a "clean" type with ChildA. If we had complex types, I would recommend just using a complex type/value object instead of the 1:1 relationship.

I tried to do a PR to just show you the new navigation & FK props in childA which are enough for convention to figure out. But that maybe I don't have correct permissions..

[Yanal-Yves]

Thank you @julielerman, I have just added you as a contributor to my repo, you should be able to do a PR now. Could you try again to do a PR?

[Yanal-Yves]

Thank you @julielerman for your commit, with this "tweak" EF 7 is now saving the data correctly on the database. I am just wondering if adding the Parent and ParentId in the Child entity is what you guys expect from developpers to do?

The real use case I am having is that I have a "User" entity and a "PostalAddress" entity. In the application I am playing with, we don't need/want to have more than one postal address per user and adding the address field in the user entity is not relevant either. The User is the Parent entity while the PostalAdress a child entity I might want to load/update depending on the use cases.

[Eneuman]

@Yanal-Yves I'm having the exact same problem as you at the moment and I'm using this "workaround" so I can have a "clean" address object.

 var customer = new Customer();
 customer.Addresses = new List<Adress>();
 customer.Addresses.Add(new Address);
_dbContext.Customers.Add(customer); 

  foreach (var item in customer.Addresses)
  {
    _dbContext.Entry(item).State = EntityState.Added;
  }
  await _dbContext.SaveChangesAsync();