Change default graph behavior of Add/Attach/etc

[rowanmiller]

In EF6.x the default behavior of these methods was to start tracking every reachable entity in the state being requested for the root entity. This caused a lot of issues because:

1. Attaching a root would often end in exceptions because there were new entities in the graph (with no PK set) which would attempt to be tracked as unchanged, resulting in duplicate PK values.
2. There was no easy mechanism to traverse over a graph and chose the state of each entity as you go.
3. There was no ability to leave parts of the graph un-tracked (i.e. I want to add an order and I don't care about telling you the state of the Customer).

In EF Core we tried to alleviate some of these issues by initially making Add/Attach/etc. just work on a single object, but this caused a lot of confusion since it seems the natural expectation (based on the feedback we saw) is that you could add an Order and the OrderLines would be automatically added too.

We then tried an "Include Dependents" approach, where if you add a parent, we also include the children as added. This works well for some cases... but it gets pretty confusing. This is compounded by the fact that it's not always clear from your model which is the principal/dependent of a relationship.

After discussion, we realized that with some small variations on the rules from EF6.x, we already have APIs in EF Core to enable these scenarios:

There was no easy mechanism to traverse over a graph and chose the state of each entity as you go.

We have the TrackGraph API for exactly this purpose.

There was no ability to leave parts of the graph un-tracked (i.e. I want to add an order and I don't care about telling you the state of the Customer).

This is now supported in EF Core, our stack is capable of handling entities in the graph that it is not tracking. We have the DbContext.Entry(...).State API that allows you to put a single entity into the context without any graph behavior (i.e. no related entities are brought in).

Attaching a root would often end in exceptions because there were new entities in the graph (with no PK set) which would attempt to be tracked as unchanged, resulting in duplicate PK values.

We will adjust the graph behavior of these APIs as follows:

• Add: Adds every reachable entity that is not already tracked
• Attach: Attaches every reachable entity, except where a reachable entity has a store generated key and no key value is assigned, these will be marked as added.
• Update: Same as Attach, but entities are marked as modified
• Remove: Same as Attach, and then mark the root as deleted. Since cascade delete now happens on SaveChanges, this allows cascade rules to flow to entities later on.

Some important results of this behavior:

• It works as expected for attached scenarios (i.e. non N-Tier scenarios). I.e. if I create a graph of objects and then call Add, they all get added.
• For N-Tier scenarios, it leaves you with the ability to cycle over each entity and tell EF the state (which was blocked in EF6.x because you would get duplicate key exceptions). Of course, TrackGraph may also be a better API for these scenarios too.

[ajcvickers]

Nice write up. :-)

[julielerman]

:(

Thought I should clarify that ... :) .... sorry you weren't able to work it out. I liked that idea but also know that people are going to have a hard time with changed behavior no matter how many times you tell them "it's not EF6". I'm glad we still have ENTRY to add just the root. It was always a hard problem not being able to tear apart graphs for this purpose so that's still a big win. Agree re great detailed writeup. Thanks. Even Frans said "Tho, reading about the defaults, what they've picked is the best choice for POCOs I think." But he also adds "Root cause is lack of self tracking." (Don't ever bring that back! :) )

[tonysneed]

So just to clarify, we'll still be able to set an entity state to Added via TrackGraph and EF will only set the state of that single entity to Added?

[rowanmiller]

@tonysneed if you want to set the state of a single entity then context.Entry(myEntity).State = EntityState.Added is probably the easiest. TrackGraph will work... but that is really designed for iterating over a graph of entities and setting the state for each one.

[tonysneed]

Thanks @rowanmiller, so context.Entry(myEntity).State = EntityState.Added will not set child entity states to Added, as it did in EF6. Just wanted to confirm, thanks!

[rowanmiller]

@tonysneed correct, EF Core is now ok with having tracked entities reference entities that are not tracked.

[tonysneed]

Excellent. In case anyone is interested, Here is a test which shows the difference between EF 6 and EF Core when it comes to setting entity state to Added on a disconnected graph.

[Fact]
public void Set_state_should_mark_single_entity()
{
    // Arrange
    var detail = new OrderDetail();
    var order = new Order
    {
        OrderDetails = new List<OrderDetail> { detail }
    };

    // Act: Set parent state to Added
    _context.Entry(order).State = EntityState.Added;

    // Assert for EF 6: Child state set to Added
    Assert.Equal(EntityState.Added, _context.Entry(detail).State);

    // Assert for EF Core: Child state remains Detached
    Assert.Equal(EntityState.Detached, _context.Entry(detail).State);
}

Here is the full source code for the tests.

[julielerman]

yeah I better go do a RC2 branch and update all of my disconnected data/graph tests in my ef7 repo (https://github.com/julielerman/EF7WhoAreYou/tree/master/Familiar%20and%20Enhancements%20Console%20Full%20NET%20After/Tests.RC)

[rowanmiller]

@julielerman and rename your repo to EFCore 😉

BTW our nightly builds are pretty unstable at the moment due to moving over to .NET CLI... so you may want to hold off a couple of weeks.

[johnkwaters]

So right now I am adding a Project entity (a new one) that has a list of Folder entities (also new). Each folder points to a FolderType (existing). For it to work, I have to attach all the existing folder types to the context before saving, or it tries to insert new folder types. What is the correct approach when part of the graph (root and some bits) are new, and part references existing objects? I was hoping for one method that would look at the IDs <= 0 to infer whether that object was new or existing..

[divega]

I was hoping for one method that would look at the IDs <= 0 to infer whether that object was new or existing

It doesn't do that automatically because that would only work if you have setup database generated keys, which is only a subset of the scenarios EF supports. If you know you did that in your entities you should be able to use TrackGraph() passing a lambda that will pick the right state for each of the entities based on the value of its key.

[aflx]

I'm a little confused now. Until today (I upgraded to the latest build), I just used Add and everything worked like a charm ;)

Can someone give an example for the following scenario? How to realize this?

public class A {
  public int Id { get; set;} 
  public virtual B ObjectB { get; set;}
  public virtual C ObjectC { get; set;}
}

public class B {
   public int Id { get; set;} 
}

public class C {
   public int Id { get; set;} 
}

A a = new A();
a.ObjectB = b; //b has been fetched from the database in a using statement => no global context
a.ObjectC = new ObjectC();

using (MyContext context = new MyContext())
{
 //What to use, if I don't want to add every new child (C) manually and there are references to an existing object (B) too
  context.A.Add(a); 
  context.SaveChanges();
}

[julielerman]

I know you asked Diego, but if I may jump in as this is a topic near & dear to my heart...

First of all, the problem would go away if you would just use FKs .

public class A {
  public int Id { get; set;} 
  public virtual B ObjectB { get; set;}
  public int BId {get;set}
  public virtual C ObjectC { get; set;}
  public int CId {get;set}
}

Then you could set the relationships like:

A a = new A();
a.BId=b.Id; <existing object ..this is the one that is causing you trouble :)
a.ObjectC = new ObjectC();

And context.Add(a) would work perfectly! :)

Because it's a new context and unaware of the existing objects, Add (update delete and attach) will affect the entire graph with whatever state you imply with the method.

As of EFCore, you can use the DbContext.Entry(). State method e.g.

context.Entry(a).State=EntityState.Added

to apply that state only to the entity that you specified and it will ignore anything else that is attached. That is new behavior for Entry and gives us a great opportunity to have a consistent pattern. This is good in your example when you know for 100% sure that you only want to affect "a" and that you 100% surely kow that you don't want b & c to be updated. But that is not your scenario, so Entry doesn't 'work either because you want to skip B but you DO want to add C as well.

If your method does not have that confidence then you need to choose a pattern to employ. There is no magic. If you know that in your system a pre-existing key value ("Id" in your case) indicates that an entity should not get dragged into the update, then you can iterate through and check for Id =0 vs Id has a value >0. (or whatever your rule is). The new TrackGraph method gives you a chance to do this.
context.ChangeTracker.TrackGraph(a, a=>[somefunction to perform])

TrackGraph iterates through every item in the graph and performs the specified function on it.

I have an example of using TrackGraph in my RC2 sample on Github. (https://github.com/julielerman/EFCore-RC2-Demo). But I'm not just using a "if id=0, added, if not unchanged" rule. I'm using trackgraph in combination with an old pattern that I wrote about yet again in a recent MSDN Mag article: https://msdn.microsoft.com/magazine/mt694083 (Rowan & I wrote about it in our DbContext book, too and that was back with EF 4.1 :) )

Look at the weathercontroller FixSeasonInReactions method. It uses trackgraph to update state based on a known state flag applied to each of my classes.

Hope this helps and sorry if you wanted to get an answer straight from Diego because that would represent the guidance directly from the team.

[ajcvickers]

@aflx I am having a little trouble understanding everything Julie said, but given the code you posted I think you just need two steps:

1. Call context.Attach for all objects that already existed in the database -- so for b in your code.
2. Call context.Add for all new objects - for a and c in your code.

Attach tells EF that the entity already exists in the database, and sets the state of the entity to Unchanged. Add tells EF that the entity is new and should be inserted into the database, and so sets the state to Added.

Since Attach and Add both operate on the graph of reachable objects you may not need to call them for every single object in your graph. For example, in the code you wrote you can Attach b and then Add a, and since c is reachable from a it will also get added.

If you want to automatically decide whether the entity should be Added or Attached based on key value, then you can use TrackGraph as Diego suggested. But this may be overkill.

[aflx]

@julielerman Thank you! (I removed Diego 10 seconds after publishing the comment...so don't worry ;)

I already tried it with just setting the Id...but no chance...

public int Add<T>(T entity) where T : class
{
    using (MyContext context = new MyContext())
    {
        DbSet<T> dbSet = context.Set<T>();
        dbSet.Add(entity); // Here the object reference gets loaded by Id
        context.SaveChanges(); // And here I get the exception (unique constraint...)
    }

    return (entity as BaseObject).Id;
}

I tried the easy example with TrackGraph (checking for Id), but it seems that I'm doing something wrong, because the lambda function is just called once for the parent (A)

public int Add<T>(T entity) where T : class
{
    using (TraxContext context = new TraxContext())
    {
        DbSet<T> dbSet = context.Set<T>();

        context.ChangeTracker.TrackGraph(entity, e => {
            if ((e.Entry.Entity as BaseObject).Id > 0) e.Entry.State = EntityState.Unchanged;
        });

        dbSet.Add(entity);
        context.SaveChanges();
    }

    return (entity as BaseObject).Id;
}

@ajcvickers Thx. But I don't like to use attach for b. I think, this only makes sense if you have something like a global context. Otherwise you always have to keep in mind storing and attaching existing objects. For a larger solution with complex objects and relationships this will be hard.
Everything was fine with the "old" behavior for Add for me (I liked the magic) :D

And maybe I have a total wrong concept of MVVM in mind.
I have a ViewModel for every View and every ViewModel uses a service which wraps every db-action in using statements... That means, that I don't even a local context for a view. Maybe I should change that...

[johnkwaters]

There are certainly some interesting changes here! I found another one today... I was adding a new Project object and it has a Client property, that may be an existing Client or a new One. For new ones, we had set the Id to -1. Client and Project have IDENTITY columns. I believe this used to cause it to be Inserted, but omitting the ID. But NOW, it tries to insert the -1! This of cause fails. To further complicate matters, 0 is a valid ID values. So now I need the client to specify a negative ID to mean add, then I need to CHANGE that Id to 0 before saving it!
I think these changes are geared toward making things more explicit and less automagic, and the TrackChanges is a nice place to put your 'business logic' for IDs, but a lot of people will be confused, so some careful documentation will be needed!

[julielerman]

oh no! If what I suggested was so convoluted that @ajcvickers didn't grok it, that makes me nervous. Maybe we should chat offline about this Arthur because I want to make sure I'm not inventing anti-patterns.

@aflx Add works the same as it did in EF6 and earlier. I wouldn't combine TrackGraph and dbset.add. Use one of the other. The result of what you are doing is not much different than Arthur's suggestion. You are having the changetracker essentially call Attach on the object that has a value in id. Then you are calling Add on the dbset. The trackgraph is itereating through all of the objects. That's its job. Your code is telling it to ignore A & C. Make it say (this is not real syntax) e.entry.state= if (id>0) Unchanged else Added. Get rid of dbset.add.

[julielerman]

@johnkwaters

think these changes are geared toward making things more explicit and less automagic,
Totally agreed! That's how I've been explaining it.

TrackChanges is a nice place to put your 'business logic' for IDs,
also agreed

but a lot of people will be confused, so some careful documentation will be needed!

This is where "this is not EF6" comes in. People are going to see familiar methods and presume they will act the same as they always did.

[aflx]

@julielerman Ok, with TrackGraph and without Add => works ;) Thank you for this hint.

using (TraxContext context = new TraxContext())
{
    DbSet<T> dbSet = context.Set<T>();

    context.ChangeTracker.TrackGraph(entity, e => {
        if ((e.Entry.Entity as BaseObject).Id > 0)
        {
            e.Entry.State = EntityState.Unchanged;
        }
        else
        {
            e.Entry.State = EntityState.Added;
        }
    });

    context.SaveChanges();
}

[johnkwaters]

Funny, I came up with the same code.

[divega]

You shouldn't need a base entity class. Try e.Entry.IsKeySet.

[julielerman]

IsKeySet? Oh what a great property!! I need to start iterating through the APIs to see what gems I've missed!! Which reminds me there was a graphnode property who's purpose I have to figure out..

[johnkwaters]

But don't you still need to examine the value of the key? How does IsKeySet help with that?

[johnkwaters]

Wouldnt it be cool if when specifying a key, you could give a lambda specifying what constitutes a 'new' value, for instances SetKey( e => e.Id, () => e.Id <=0 ). Then the fwk could go back to doing the heavy lifting in a generic way... and with some good defaults, you could even not need to do this for traditional patterns (Guid.Empty, Int<0 etc).

[julielerman]

@johnkwaters did you check the code base? It checks to make sure that the key is not at the default value. That's pretty good coverage. And it's virtual so perhaps you can override it ... though I haven't tried it out yet.

public virtual bool IsKeySet => !EntityType.FindPrimaryKey().Properties.Any(p => p.ClrType.IsDefaultValue(this[p]));

[johnkwaters]

Awesome. No, I didnt check the codebase... next time I will! Right now I am working on a DB that was ported over from Access to SQL server, and sadly they use 0 as a valid key value for all kinds of tables, so having it be negative is the lambda I would need, or the override.

[johnkwaters]

@julielerman @divega @ajcvickers And I must say it is such a treat to be able to discuss and possibly influence the evolution of EF7 as it is crafted! I am using it in several production projects, due to ship in the August time frame (!!), so having this dialog definately makes me feel good about my bet on .net core.

[johnkwaters]

So I am passing this root object to track changes. It is NOT iterating over any of the referenced objects or any of the child object lists.
The relationships ARE spelled out in the fluent config.
Do I need to use virtual for this to work?

[divega]

and sadly they use 0 as a valid key value for all kinds of tables

If zero is a valid key you can change the type of the key to be nullable, e.g. int? instead of int. Then IsKeySet will return true for zero and false for null.

So I am passing this root object to track changes. It is NOT iterating over any of the referenced objects or any of the child object lists.
The relationships ARE spelled out in the fluent config.
Do I need to use virtual for this to work?

Not sure what the problem could be. You don't need to make the navigation properties virtual, in fact virtual doesn't make any difference with EF Core. Could you please create a new issue with a self-contained repro of this issue?

[johnkwaters]

But then wouldn't I need a nullable pk in the db?

Sent from my iPhone

On Jun 2, 2016, at 2:23 PM, Diego Vega [email protected] wrote:

and sadly they use 0 as a valid key value for all kinds of tables

If zero is a valid key you can change the type of the key to be nullable, e.g. int? instead of int. Then IsKeySet will return true for zero and false for null.

So I am passing this root object to track changes. It is NOT iterating over any of the referenced objects or any of the child object lists.
The relationships ARE spelled out in the fluent config.
Do I need to use virtual for this to work?

Not sure what the problem could be. You don't need to make the navigation properties virtual, in fact virtual doesn't make any difference with EF Core. Could you please create a new issue with a self-contained repro of this issue?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[divega]

No. The column will still be non-nullable as null is not a valid key value in a persisted object. It only means that the key can have a null value temporarily while it is in memory and before it is added to a DbContext.

[julielerman]

@ajcvickers when Attaching a graph (no previously tracked entities) and child does NOT have it's key value filled, it becomes Added. When there is a a value in key, it becomes Unchanged. I did not expect this. Is Attach now reading key values to make state decisions? Did I miss that explanation somewhere?

edit: yeah okay I see that re-reading the original post in this thread....

[sjb-sjb]

It's great to follow this discussion, especially given how long these challenges have been kicking around.

I believe the TrackGraph pattern described by @julielerman may be superceded by the IsKeySet approach, i.e. simply use store-generated keys and then do one of the following: either
(a) Attach copies of the entities that already exist in the store ("b" in @aflx's example) with their primary keys set, and then Add the (root of the) new entities; or
(b) Refer to existing entities by putting their store-generated keys in the new entities, and then Add the (root of the) new entities.

@julielerman 's comment of June 21, and the description of Attach in @rowanmiller 's original posting, seem to say that in case (a) you could use Attach instead of Add for the new entities. The new entities' store-generated keys will not have been set and therefore the Attach will actually set the state to Added. In other words you can just construct the complete graph, including store-generated Id's in the case where the entity is a copy of an object already in the data store, and just Attach the whole thing. Or you can refer to the existing objects by Id instead of by reference, in which case you can either Add or Attach the graph.

Have I gotten that right? It would be great to have an update to @julielerman 's April MSDN article that takes into account the June discussion above.

[ErikEJ]

@sjb-sjb did you see this https://msdn.microsoft.com/magazine/mt767693 ?

[sjb-sjb]

Cool, thanks!

Sent from my iPhone

On Aug 13, 2016, at 2:07 AM, Erik Ejlskov Jensen [email protected] wrote:

@sjb-sjb did you see this https://msdn.microsoft.com/magazine/mt767693 ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.