Efcore: Store temporary key values in state entry rather than setting them on entity instances

[ajcvickers]

Similar to the way store-generated values are stored during SaveChanges. This would mean that temporary values would not be stored in or visible in the key property, which would have a few advantages:

• Entities that have are moved from one context instance to another will not incorrectly treat temporary key values as real--see #10167
• When using data binding, the temporary value will not show up in the data grid--see #12218

The reasons that we didn't do this before were:

• The key value in the entity may not be the key value that EF is using. This can be confusing.
• Fixup could be more complex, but I think our indirections for shadow values, store-generated values, etc. are pretty good now, so I am hopeful this won't be a major problem.

[avinash-phaniraj-readify]

@ajcvickers @optiks
On our migration project (from L2S), we find heavy reliance on the primary key properties being 0 throughout the app; including business logic etc.

It will be great if EFCore can get away from setting the properties to negative numbers.

While you think through the possibility, can you please advise if there are any workarounds/ideas that may help us ?

Thanks,
\A

[ajcvickers]

@avinash-phaniraj-readify I don't have any great ideas for a workaround other than do to what I already saw in some other code from you guys where you instead check for negative values instead of zero--maybe you can move this into a common helper that you then call whenever this logic exists.

[ajcvickers]

Re-opening to ensure breaking change is announced.

[ajcvickers]

@divega Draft breaking change announcement for review

Entity Framework Core breaking change: Temporary values are no longer stored in entity instances

See issue https://github.com/aspnet/EntityFrameworkCore/issues/12378

Summary

When a new entity instance is tracked by the context it needs a primary key value. If the primary key value is specified or can be generated immediately, then this permanent key value is used immediately. However, if the entity will get a store-generated key value from the database, then the entity needs a temporary key value until SaveChanges is called.

Prior to EF Core 3.0, this temporary key value was set into the primary key property of the entity. Starting with EF Core 3.0, the temporary key value is now stored with EF's tracking information while the value of primary key property itself will not be changed.

Where this helps

The primary motivation for this change is to allow entities that have been previously tracked by some context instance to be moved to being tracked on another instance without the temporary key values erroneously becoming permanent.

Where this may break applications

Applications may be using the temporary key values to form associations between entities. For example, the temporary primary key value may have been used to set an FK value. If this is happening in your application, then there are two approaches to avoid this while still using store-generated keys:

• Don't use primary key/foreign key values to associate entities, but instead use navigation properties. This is a best practice anyway since it uses only the object model without dependency on the underlying keys.
• Obtain the temporary values from EF's tracking information. For example, context.Entry(blog).Property(e => e.Id).CurrentValue will return the temporary value even though blog.Id itself has not been set.

[sjb-sjb]

@ajcvickers concerning the idea of using navigation properties exclusively … will EF set the foreign key of an Added dependent entity if the principal entity is not attached to the context? Correspondingly for a non-attached principal entity will EF add the dependent entity to the principle entity's navigation collection? Would be interested to know if there is any detail document talking about this kind of thing.

[ajcvickers]

@sjb-sjb In general, EF will not take a value from an un-tracked entity and propagate to one that is tracked.

[sjb-sjb]

@ajcvickers So … if I have two entities and I want to make one of them refer to the other, then I should (a) set the navigation property _and_ (b) check the primary key of the principle entity; if it is > 0 then set the foreign key into the dependent entity.

[ajcvickers]

@sjb-sjb You should track them both.

[ilmax]

I think this change was a really bad one for few reasons:

• The prev behavior was intuitive and useful
• You cannot simply use object references especially if you're trying to build a DDD-ish model, you cannot reference another aggregate root but you can have a FK reference to it
• The "workaround" involves using the context to get the temporary value, but again it _may_ not be possible (due to architecture constraints) to have the context everywhere e.g. inside an entity method.
• The motivation for changing this behavior and breaking a lot of customer code is to support something "uncommon" like sharing an entity with two different contexts and all the documentation around EF tells you to have a single context per request (in the web scenario) and I cannot really come up with a proper usage of "sharing the same entity between multiple dbcontext".
  The other one being data-binding is a bit odd to me as well, you can solve it in so many ways without changing the underlying behavior
• The most important thing to me is that, like other people have already called out, this was a breaking change without any knob to turn on the previous behavior

This is a rant because it's taking me forever to migrate a project to aspnet core 3.1 due to EF's 56 breaking changes. and because of this I'm still running on top of an unsupported dotnet core version in production!
I really hope this will be handled better with EF vNext (5?) because despite being an EF fan and enthusiast since v4.1, I've to admin this EF 3 version migrations is the most painful ever.

[chrisevans9629]

What if there was setting to set the primary key properties on add to a negative number? This would at least give people an option to choose

[Zero3]

@ajcvickers I am facing a similar issue like @ilmax above, where I am trying to avoid having navigation properties between entities of different modules of my codebase (aka between different "aggregate roots"). I also use plain foreign keys instead.

The reason is to prevent issues like these:

1) Navigation properties on the linked entity will not be eager loaded correctly with the necessary .Include() statements, since the code that did the querying doesn't know what to include inside the linked entity, as it belongs to another part of the code. So as the linked entity is passed around, there is a significant risk of NullReferenceException or empty collections that are not supposed to be empty.

2) Concurrency assumptions by the code that owns the linked entity are broken. For example, the owning code might be using locking to guard access to its entities and thereby ensure thread safety, but that safety goes out the window when other parts of the code sneaks their way into the entities through navigation properties.

The behaviour prior to this change, where EF assigned temporary primary key values, was thus very appreciated. This made it possible to have module A create an instance of entity A, module B create an instance of entity B, and link them together using their (temporary) primary keys. But now, their primary keys are all left at their default values, making this impossible without exposing the unwanted navigation properties between the entities.

As @chrisevans9629 mentions, an option for re-enabling this behaviour would be appreciated.

You do mention a workaround with using context.Entry(entity).Property(entity => entity.Id).CurrentValue. Would it be safe to set entity.Id to this value manually to revert to the old behaviour?

[ajcvickers]

@Zero3 This should still work because the temporary values still exist. But you'll need to use the EF Core APIs (e.g. context.Entry(foo).Property(e => e.Bar).CurrentValue) to read them.

[Zero3]

To answer my own question: No, it is not safe to set entity.Id to the temporary value. EF will then store the temporary value in the database as-is, instead of storing the database generated ID.

So it appears like there is no way to restore the old behaviour. It would be really nice if the new behaviour was opt-in/opt-out, for the reasons already described above.