-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add basic update pipeline support for complex properties mapped to JSON #36379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
e40075a to
a884308
Compare
roji
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty straightforward 👍
| var jsonPartialUpdateInfo = new JsonPartialUpdateInfo(); | ||
| jsonPartialUpdateInfo.Path.Insert(0, new JsonPartialUpdatePathEntry("$", null, entry, Navigation: null, ComplexProperty: complexProperty)); | ||
| jsonPartialUpdateInfo.PropertyValue = entry.GetCurrentValue(complexProperty); | ||
| jsonColumnsUpdateMap[jsonColumn] = jsonPartialUpdateInfo; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have the full context, but it seems odd that we populate this map and then iterate over it to process it - can we simply extract the code below to a local function and call that instead of having the intermediate map? I guess we'd still need a bit of tracking to ensure we don't handle the same JSON column (though I'm assuming mapping two navigations/complex properties to the same JSON column isn't supported?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, all this refactoring will come later
test/EFCore.Relational.Specification.Tests/Update/ComplexCollectionJsonUpdateTestBase.cs
Show resolved
Hide resolved
test/EFCore.Relational.Specification.Tests/Update/ComplexCollectionJsonUpdateTestBase.cs
Show resolved
Hide resolved
e9d8fc2 to
bfa6a86
Compare
a884308 to
12301c0
Compare
bfa6a86 to
af9e1dc
Compare
12301c0 to
aa2831d
Compare
af9e1dc to
1b05bed
Compare
aa2831d to
fe43fd3
Compare
Now that the update pipeline works, dotnet#36379
Now that the update pipeline works, dotnet#36379
Now that the update pipeline works, dotnet#36379
Now that the update pipeline works, dotnet#36379
Now that the update pipeline works, dotnet#36379
Now that the update pipeline works, dotnet#36379
Now that the update pipeline works, #36379
Part of #31252
It still needs to be refactored and usages of
IInternalEntryshould be replaced with a new public interface