Graph update tests for change tracking proxies #19706
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
|
@orionstudt Would appreciate a quick review if you can! |
|
@ajcvickers Oh damn! Looks like some intricacies in the value comparison that I missed not being as knowledgeable in EF as you, but it makes sense looking at it. I think the resulting interceptor code is a bit cleaner too. Looks good to me. The CreateProxy overload is a nice QOL improvement. Edit: I now see what you meant when you mentioned the ProxyGraph tests during the PR. For some reason I hadn't realized that that would need to be updated as well - and I'm not sure I would've known how to go about that lol so I'm glad you got to it. |
|
@orionstudt Thanks. These are things that are easy to miss--I reviewed the code and missed them! |
AndriySvyryd
approved these changes
Jan 26, 2020
|
@AndriySvyryd Covered by #16454 |
Issue #10949 Found and fixed a couple of bugs: * Proxies were reading directly from the property. This can cause recursive loops in some cases. Fixed by using EF's compiled delegates for writing to the field directly. * Comparisons were being done by the default for the property/navigation. Instead force use of configured value comparer for regular properties or reference equality for navigations. Also added overloads to `CreateProxy` to make it easier to do creation of proxy graphs inline.
ajcvickers
force-pushed
the
ChangeTrackingProxies0119
branch
from
6c89aa5
to
6f9a116
Compare
|
Touché |
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.
Issue #10949
Found and fixed a couple of bugs:
Also added overloads to
CreateProxyto make it easier to do creation of proxy graphs inline.