From 8ba63f4eb7043394d91e5f250945ef905b33e056 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 05:55:57 +0000 Subject: [PATCH 1/6] Initial plan From 7fbe6ad3341cc72ba3b4024f23256a7c6b48ca8a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 06:41:38 +0000 Subject: [PATCH 2/6] Fix in-memory corruption of nested owned entities after SaveChanges with 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 dotnet/efcore#36017 Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../Internal/EntityGraphAttacher.cs | 5 +- .../ChangeTracking/Internal/StateManager.cs | 9 +++ .../Update/JsonUpdateTestBase.cs | 58 +++++++++++++++++++ 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs b/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs index ba7048a022e..2764048fd70 100644 --- a/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs +++ b/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs @@ -102,7 +102,8 @@ private bool PaintAction( SetReferenceLoaded(node); var internalEntityEntry = node.GetInfrastructure(); - if (internalEntityEntry.EntityState != EntityState.Detached + + if (internalEntityEntry.EntityState is not (EntityState.Detached or EntityState.Deleted) || (_visited != null && _visited.Contains(internalEntityEntry.Entity))) { return false; @@ -139,7 +140,7 @@ private async Task PaintActionAsync( SetReferenceLoaded(node); var internalEntityEntry = node.GetInfrastructure(); - if (internalEntityEntry.EntityState != EntityState.Detached + if (internalEntityEntry.EntityState is not (EntityState.Detached or EntityState.Deleted) || (_visited != null && _visited.Contains(internalEntityEntry.Entity))) { return false; diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs index 44b2ab405e9..2f51698eeb4 100644 --- a/src/EFCore/ChangeTracking/Internal/StateManager.cs +++ b/src/EFCore/ChangeTracking/Internal/StateManager.cs @@ -1230,6 +1230,15 @@ public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumer continue; } + // If the dependent's current principal (from identity map) is different from + // the entry being cascade-deleted, the dependent has been re-associated with + // a replacement principal (e.g., via shared identity). Skip the cascade. + var currentPrincipal = FindPrincipal(dependent, fk); + if (currentPrincipal != null && currentPrincipal != entry) + { + continue; + } + if (detectChangesEnabled) { ChangeDetector.DetectChanges(dependent); diff --git a/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs b/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs index 54c37c517d0..d8bc4c5ebd0 100644 --- a/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs @@ -3671,6 +3671,64 @@ public virtual Task Edit_single_property_with_non_ascii_characters() Assert.Equal("测试1", result.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf.SomethingSomething); }); + [ConditionalFact] + public virtual Task Replace_json_reference_root_preserves_nested_owned_entities_in_memory() + => TestHelpers.ExecuteWithStrategyInTransactionAsync( + CreateContext, + UseTransaction, + async context => + { + var query = await context.JsonEntitiesBasic.ToListAsync(); + var entity = query.Single(); + + // Save original leaf value + var originalLeaf = entity.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf; + var originalLeafValue = originalLeaf.SomethingSomething; + + // Replace the owned reference with a new instance that shares nested reference navigations + var oldRoot = entity.OwnedReferenceRoot; + entity.OwnedReferenceRoot = new JsonOwnedRoot + { + Name = "Modified", + Number = oldRoot.Number, + Names = oldRoot.Names, + Numbers = oldRoot.Numbers, + OwnedReferenceBranch = new JsonOwnedBranch + { + Id = oldRoot.OwnedReferenceBranch.Id, + Date = oldRoot.OwnedReferenceBranch.Date, + Enum = oldRoot.OwnedReferenceBranch.Enum, + Fraction = oldRoot.OwnedReferenceBranch.Fraction, + OwnedReferenceLeaf = originalLeaf, + OwnedCollectionLeaf = [], + }, + OwnedCollectionBranch = [], + }; + + // Before DetectChanges, leaf should be accessible + Assert.Same(originalLeaf, entity.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf); + + context.ChangeTracker.DetectChanges(); + + // After DetectChanges, leaf should still be accessible + Assert.NotNull(entity.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf); + + ClearLog(); + await context.SaveChangesAsync(); + + // After SaveChanges, nested owned entities should still be accessible in memory + Assert.NotNull(entity.OwnedReferenceRoot.OwnedReferenceBranch); + Assert.NotNull(entity.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf); + Assert.Equal(originalLeafValue, entity.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf.SomethingSomething); + }, + async context => + { + var result = await context.Set().SingleAsync(); + Assert.Equal("Modified", result.OwnedReferenceRoot.Name); + Assert.NotNull(result.OwnedReferenceRoot.OwnedReferenceBranch); + Assert.NotNull(result.OwnedReferenceRoot.OwnedReferenceBranch.OwnedReferenceLeaf); + }); + public void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction) => facade.UseTransaction(transaction.GetDbTransaction()); From e7a080abc7711ed386159f9467837cef61ddd0c6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 06:46:41 +0000 Subject: [PATCH 3/6] Improve comment explaining CascadeDelete skip logic Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- src/EFCore/ChangeTracking/Internal/StateManager.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs index 2f51698eeb4..dd7c64c6f1b 100644 --- a/src/EFCore/ChangeTracking/Internal/StateManager.cs +++ b/src/EFCore/ChangeTracking/Internal/StateManager.cs @@ -1230,9 +1230,11 @@ public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumer continue; } - // If the dependent's current principal (from identity map) is different from - // the entry being cascade-deleted, the dependent has been re-associated with - // a replacement principal (e.g., via shared identity). Skip the cascade. + // When an owned entity is replaced (e.g., via record 'with' expression), the old entry is + // marked Deleted and a new entry with the same key is added via SharedIdentityEntry. If a nested + // dependent was re-used by the new entity graph, FindPrincipal returns the replacement principal + // from the identity map rather than the deleted entry. In that case, skip the cascade to avoid + // incorrectly re-deleting the dependent that now belongs to the replacement principal. var currentPrincipal = FindPrincipal(dependent, fk); if (currentPrincipal != null && currentPrincipal != entry) { From 3b852721ae809ec36f397dab704abe8c3a298c6c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Feb 2026 01:14:58 +0000 Subject: [PATCH 4/6] Use SharedIdentityEntry checks instead of FindPrincipal per reviewer 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> --- .../Internal/EntityGraphAttacher.cs | 8 ++++++-- .../ChangeTracking/Internal/StateManager.cs | 19 ++++++++----------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs b/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs index 2764048fd70..1493e8a2b69 100644 --- a/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs +++ b/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs @@ -103,7 +103,9 @@ private bool PaintAction( var internalEntityEntry = node.GetInfrastructure(); - if (internalEntityEntry.EntityState is not (EntityState.Detached or EntityState.Deleted) + if ((internalEntityEntry.EntityState != EntityState.Detached + && (internalEntityEntry.EntityState != EntityState.Deleted + || node.SourceEntry?.GetInfrastructure().SharedIdentityEntry == null)) || (_visited != null && _visited.Contains(internalEntityEntry.Entity))) { return false; @@ -140,7 +142,9 @@ private async Task PaintActionAsync( SetReferenceLoaded(node); var internalEntityEntry = node.GetInfrastructure(); - if (internalEntityEntry.EntityState is not (EntityState.Detached or EntityState.Deleted) + if ((internalEntityEntry.EntityState != EntityState.Detached + && (internalEntityEntry.EntityState != EntityState.Deleted + || node.SourceEntry?.GetInfrastructure().SharedIdentityEntry == null)) || (_visited != null && _visited.Contains(internalEntityEntry.Entity))) { return false; diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs index dd7c64c6f1b..9a5a169ef1b 100644 --- a/src/EFCore/ChangeTracking/Internal/StateManager.cs +++ b/src/EFCore/ChangeTracking/Internal/StateManager.cs @@ -1208,6 +1208,14 @@ public virtual void CascadeChanges(bool force) /// public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumerable? foreignKeys = null) { + // When an owned entity is replaced (e.g., via record 'with' expression), the old entry is + // marked Deleted and a new entry with the same key is linked via SharedIdentityEntry. + // Skip cascade from the old entry since the replacement handles its own dependents. + if (entry.SharedIdentityEntry != null) + { + return; + } + var doCascadeDelete = force || CascadeDeleteTiming != CascadeTiming.Never; var principalIsDetached = entry.EntityState == EntityState.Detached; @@ -1230,17 +1238,6 @@ public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumer continue; } - // When an owned entity is replaced (e.g., via record 'with' expression), the old entry is - // marked Deleted and a new entry with the same key is added via SharedIdentityEntry. If a nested - // dependent was re-used by the new entity graph, FindPrincipal returns the replacement principal - // from the identity map rather than the deleted entry. In that case, skip the cascade to avoid - // incorrectly re-deleting the dependent that now belongs to the replacement principal. - var currentPrincipal = FindPrincipal(dependent, fk); - if (currentPrincipal != null && currentPrincipal != entry) - { - continue; - } - if (detectChangesEnabled) { ChangeDetector.DetectChanges(dependent); From 6a6718f34376dafded467f48cb5ff9291071b4e5 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Tue, 24 Feb 2026 18:54:16 -0800 Subject: [PATCH 5/6] Update test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Update/JsonUpdateTestBase.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs b/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs index d8bc4c5ebd0..2dd1a172555 100644 --- a/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Update/JsonUpdateTestBase.cs @@ -3699,6 +3699,9 @@ public virtual Task Replace_json_reference_root_preserves_nested_owned_entities_ Date = oldRoot.OwnedReferenceBranch.Date, Enum = oldRoot.OwnedReferenceBranch.Enum, Fraction = oldRoot.OwnedReferenceBranch.Fraction, + NullableEnum = oldRoot.OwnedReferenceBranch.NullableEnum, + Enums = oldRoot.OwnedReferenceBranch.Enums, + NullableEnums = oldRoot.OwnedReferenceBranch.NullableEnums, OwnedReferenceLeaf = originalLeaf, OwnedCollectionLeaf = [], }, From 5d59a9cc60334b1652e6fd7b758ecf3b7642a19b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Feb 2026 02:59:21 +0000 Subject: [PATCH 6/6] Cache node.SourceEntry?.GetInfrastructure() in local variable in EntityGraphAttacher Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com> --- .../ChangeTracking/Internal/EntityGraphAttacher.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs b/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs index 1493e8a2b69..711ab05a93a 100644 --- a/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs +++ b/src/EFCore/ChangeTracking/Internal/EntityGraphAttacher.cs @@ -102,10 +102,11 @@ private bool PaintAction( SetReferenceLoaded(node); var internalEntityEntry = node.GetInfrastructure(); + var sourceEntry = node.SourceEntry?.GetInfrastructure(); if ((internalEntityEntry.EntityState != EntityState.Detached && (internalEntityEntry.EntityState != EntityState.Deleted - || node.SourceEntry?.GetInfrastructure().SharedIdentityEntry == null)) + || sourceEntry?.SharedIdentityEntry == null)) || (_visited != null && _visited.Contains(internalEntityEntry.Entity))) { return false; @@ -117,7 +118,7 @@ private bool PaintAction( if (internalEntityEntry.StateManager.ResolveToExistingEntry( internalEntityEntry, - node.InboundNavigation, node.SourceEntry?.GetInfrastructure())) + node.InboundNavigation, sourceEntry)) { (_visited ??= new HashSet(ReferenceEqualityComparer.Instance)).Add(internalEntityEntry.Entity); } @@ -142,9 +143,10 @@ private async Task PaintActionAsync( SetReferenceLoaded(node); var internalEntityEntry = node.GetInfrastructure(); + var sourceEntry = node.SourceEntry?.GetInfrastructure(); if ((internalEntityEntry.EntityState != EntityState.Detached && (internalEntityEntry.EntityState != EntityState.Deleted - || node.SourceEntry?.GetInfrastructure().SharedIdentityEntry == null)) + || sourceEntry?.SharedIdentityEntry == null)) || (_visited != null && _visited.Contains(internalEntityEntry.Entity))) { return false; @@ -156,7 +158,7 @@ private async Task PaintActionAsync( if (internalEntityEntry.StateManager.ResolveToExistingEntry( internalEntityEntry, - node.InboundNavigation, node.SourceEntry?.GetInfrastructure())) + node.InboundNavigation, sourceEntry)) { (_visited ??= []).Add(internalEntityEntry.Entity); }