Store temporary values in the state entry #14256
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ajcvickers
force-pushed
the
ReturnOfTheRucksack1226
branch
from
5cc1655
to
27896a6
Compare
ajcvickers
force-pushed
the
ReturnOfTheRucksack1226
branch
2 times, most recently
from
96bf182
to
ce13254
Compare
|
@AndriySvyryd New version up and passed checks. |
| foreach (var foreignKey in currentProperty | ||
| .DeclaringEntityType | ||
| .GetDerivedTypesInclusive() | ||
| .SelectMany(e => e.GetForeignKeys())) |
There was a problem hiding this comment.
Call GetDeclaredForeignKeys() to avoid duplicates
There was a problem hiding this comment.
Or even better currentProperty.GetContainingForeignKeys()
Fixes #12378, #10167 The main breaking change here is that application logic that relies on the temporary value being set into the actual entity instance will be broken. Possibly the only place this is used is to trigger fixup between Added entities (before they are saved) using FK values only. This is pretty uncommon. The case where both navigation properties and FKs are excessively set will still work because the navigations will cause fixup to happen. Anyone hitting this can still make it work by grabbing temporary values from the state manager--tests that were hitting this have been updated to work this way. Functionally, the change works like this: * If a property is set to a non-default value, then it cannot be temporary or store-generated * This is kind of a new restriction, but functionally shouldn't change anything that worked before * If a property has a default value, then it can have a temporary value set, which is stored in a new "sidecar". * When the update pipeline sets a store-generated value, it uses the same mechanism. * So, real values hide store generated values, which hide temporary values. * This means that if SaveChanges fails and the store-generated values are discarded, then the temporary values take over again. Setting a store-generated or temporary value has been made explicit. Transparently setting store-generated values wasn't actually very useful.
ajcvickers
force-pushed
the
ReturnOfTheRucksack1226
branch
from
ce13254
to
b846dc3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #12378, #10167
The main breaking change here is that application logic that relies on the temporary value being set into the actual entity instance will be broken. Possibly the only place this is used is to trigger fixup between Added entities (before they are saved) using FK values only. This is pretty uncommon. The case where both navigation properties and FKs are excessively set will still work because the navigations will cause fixup to happen. Anyone hitting this can still make it work by grabbing temporary values from the state manager--tests that were hitting this have been updated to work this way.
Functionally, the change works like this:
Setting a store-generated or temporary value has been made explicit. Transparently setting store-generated values wasn't actually very useful.