Throw on saving partial changes to entities using shared tables with concurrency token

[AndriySvyryd]

If there's a concurrency token column in a table then the entity containing it should be tracked when saving changes to any entities sharing the table, otherwise an exception would be thrown.

[AndriySvyryd]

Related to #12865

[ajcvickers]

I thought about this some more and then we discussed in triage; we decided not to do this. :-)

Thought process: If multiple (non-owned) entities are mapped to the same table, and that table has a concurrency column, then:

• If the column is mapped into one or more of those entities, then those entities will participate in optimistic concurrency in the normal way.
• If the column is not mapped in the entity, and SaveChanges only sends updates for that entity (because it is the only one modified/tracked), then it will be last-one-wins, which seems correct.

@AndriySvyryd Does this make sense?

[AndriySvyryd]

@ajcvickers Sure, but we would need to do something to make this work:
A and B are tracked and sharing the table, A has a concurrency token column, but B doesn't. B is modified and saved. We need to update the token value on A to avoid a false positive next time A is saved.

[ajcvickers]

@AndriySvyryd Presumably you mean for auto-updating concurrency tokens, like rowversion? This shouldn't be an issue for normal columns marked as concurrency tokens.

[ajcvickers]

Linking to #1985 since while this in-of-itself is not about aggregates, it is closely related to table splitting with owned types.

[AndriySvyryd]

We need to make sure that this works correctly if the concurrency token is mapped to both entities.
Also for non-owned types we shouldn't to too much magic as it might be unexpected. Since not updating the token would only be an issue if the parent entity was modified and saved later we can throw a more verbose exception at that time explaining how it can be prevented.

[ajcvickers]

Decision for 3.0: We will throw (model validation) if a concurrency token that is store generated (i.e. rowversion pattern) is not mapped to all entities that share a table.

[FransBouma]

Decision for 3.0: We will throw (model validation) if a concurrency token that is store generated (i.e. rowversion pattern) is not mapped to all entities that share a table.

Why don't you just add it silently as a shadow state mapped property to all entities in the graph mapped to the target? You know which other entities there are in the graph mapped onto the same target, you know if the entities in that graph map or don't map the concurrency token field explicitly or through shadow state... (graph mapped onto the target is the set of entities related to each other and owning one another, like the order -orderdetails example in the docs)

Now the user has to explicitly do this manually while all the info to do this for the user is in the model at runtime.

[AndriySvyryd]

We felt that if the properties were created by convention then it would be less obvious how to deal with concurrency issues on those types.
But perhaps this can be mitigated with documentation, filed #18063

[FransBouma]

:)
Documentation of course doesn't make the work go away ;)
Say I have the complex types (value types in DDD) from EF6 mapped as owned types in EF Core. If the entity containing the complex typed fields has a concurrentytoken field, I now have to add the shadowmapping for that field to all complex types in that entity too, by hand. IMHO this should be done in the runtime. It avoids having the user running into an exception at runtime (because they forgot to map it) and it's a required mapping, but at the same time, it's determinable through the info already there by the runtime.

[AndriySvyryd]

@FransBouma I agree, that's why I filed #18063 to do that and add documentation.

[FransBouma]

Cool thanks! :)