Fix in-memory corruption of nested owned entities after SaveChanges when navigation is replaced#37787
Conversation
…ith record with expression When an owned entity stored as JSON is replaced with a new instance that shares nested owned entity references (e.g., via C# record 'with' expression), the change tracker now properly handles the shared child entities: 1. EntityGraphAttacher.PaintAction: Allow entities in Deleted state to be re-attached when encountered during graph traversal of a new parent entity. 2. StateManager.CascadeDelete: Skip cascade deletion for dependents whose current principal (from identity map) has been replaced by a new entity via shared identity. Fixes #36017 Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
…feedback - StateManager.CascadeDelete: Early-return when entry.SharedIdentityEntry is set, skipping cascade entirely since the replacement handles its own dependents. - EntityGraphAttacher.PaintAction/PaintActionAsync: Allow Deleted entries only when the source entry has SharedIdentityEntry (indicating traversal from a replacement entity), rather than broadly allowing all Deleted entries. Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a change-tracking bug where replacing a JSON-mapped owned reference with a new instance (while reusing nested owned instances) could leave nested owned navigations null in-memory after SaveChanges, despite correct persistence.
Changes:
- Allow
EntityGraphAttacherto re-attachDeletedentries during traversal when coming from a replacement graph (SharedIdentityEntrypath). - Skip cascading deletes from entries participating in shared-identity replacement in
StateManager.CascadeDelete. - Add a regression test covering replace-with-shared-nested-owned behavior for JSON-owned graphs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs | Adds regression test for replacing a JSON-owned root while reusing a nested owned leaf instance. |
| src/EFCore/ChangeTracking/Internal/StateManager.cs | Avoids cascading delete from entries involved in shared-identity replacement scenarios. |
| src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs | Expands graph attachment to include certain Deleted entries when traversing from a replacement graph. |
…stBase.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…tyGraphAttacher Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
|
@copilot Update the PR description with a readable summary |
Updated the PR description with a readable summary covering the reproduction scenario, root cause breakdown, and the fix applied to each file. |
|
@copilot public override async Task Replace_json_reference_root_preserves_nested_owned_entities_in_memory(bool async) } |
…nested_owned_entities_in_memory Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Head branch was pushed to by a user without write access
|
@copilot You need to actually specify the correct baseline for in the Replace_json_reference_root_preserves_nested_owned_entities_in_memory override assert: |
…sted_owned_entities_in_memory in JsonUpdateJsonTypeSqlServerTest Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
Head branch was pushed to by a user without write access
... Fixed in 0232ce2 — added the correct SQL baseline with the expected UPDATE and SELECT statements for |
Fixes #38095
When a JSON-mapped owned entity navigation is replaced with a new instance that shares nested child references (e.g., via C# record
withexpression), nested owned entities becomenullin memory afterSaveChanges, despite being correctly persisted to the database.Root cause
During the replace-and-save cycle, three things go wrong:
DeletedEntityGraphAttacher.PaintActiononly processesDetachedentities, so shared children stuck inDeletedstate are skipped when the new parent graph is attachedStateManager.CascadeDeletere-cascades the old parent's deletion to the shared children, andDeleteFixupresolves the replacement principal via the identity map and nulls its navigation — corrupting the in-memory graphFix
EntityGraphAttacher.PaintAction/PaintActionAsync: AllowDeletedentities to be re-attached during graph traversal when the source entry has aSharedIdentityEntry(indicating the graph is being traversed from a replacement parent)StateManager.CascadeDelete: Early-return when the entry has aSharedIdentityEntry, skipping cascade from old entries that have already been replacedJsonUpdateTestBase: Add regression testReplace_json_reference_root_preserves_nested_owned_entities_in_memoryJsonUpdateJsonTypeSqlServerTestandJsonUpdateSqliteTestOriginal prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.