From 0375e27603b4b00746b2b0b69b4b0c33eb74c5d2 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Tue, 30 Nov 2021 09:22:52 +0000 Subject: [PATCH] Fixup navigations to/from join entity when new many-to-many relationship is created Fixes #26779 Also cleanup of delay-fixup/suspended detection logic, and tests for #26074. --- .../ChangeTracking/Internal/ChangeDetector.cs | 25 +- .../Internal/IChangeDetector.cs | 16 - .../Internal/INavigationFixer.cs | 6 +- .../Internal/NavigationFixer.cs | 149 +-- .../ChangeTracking/Internal/StateManager.cs | 8 +- .../ManyToManyTrackingTestBase.cs | 1011 +++++++++++++++++ .../Internal/ChangeDetectorTest.cs | 25 +- .../ChangeTracking/Internal/FixupTest.cs | 486 +++++++- .../InternalEntryEntrySubscriberTest.cs | 9 +- .../Internal/StateManagerTest.cs | 9 +- test/EFCore.Tests/DbContextServicesTest.cs | 9 +- 11 files changed, 1612 insertions(+), 141 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs b/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs index 5493a292e29..eeb602ffa12 100644 --- a/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs +++ b/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs @@ -17,7 +17,6 @@ public class ChangeDetector : IChangeDetector { private readonly IDiagnosticsLogger _logger; private readonly ILoggingOptions _loggingOptions; - private bool _suspended; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -33,24 +32,6 @@ public ChangeDetector( _loggingOptions = loggingOptions; } - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public virtual void Suspend() - => _suspended = true; - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public virtual void Resume() - => _suspended = false; - /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -59,8 +40,7 @@ public virtual void Resume() /// public virtual void PropertyChanged(InternalEntityEntry entry, IPropertyBase propertyBase, bool setModified) { - if (_suspended - || entry.EntityState == EntityState.Detached + if (entry.EntityState == EntityState.Detached || propertyBase is IServiceProperty) { return; @@ -103,8 +83,7 @@ private static void ThrowIfKeyChanged(InternalEntityEntry entry, IProperty prope /// public virtual void PropertyChanging(InternalEntityEntry entry, IPropertyBase propertyBase) { - if (_suspended - || entry.EntityState == EntityState.Detached + if (entry.EntityState == EntityState.Detached || propertyBase is IServiceProperty) { return; diff --git a/src/EFCore/ChangeTracking/Internal/IChangeDetector.cs b/src/EFCore/ChangeTracking/Internal/IChangeDetector.cs index c6de03da6a7..3ecc0534702 100644 --- a/src/EFCore/ChangeTracking/Internal/IChangeDetector.cs +++ b/src/EFCore/ChangeTracking/Internal/IChangeDetector.cs @@ -48,20 +48,4 @@ public interface IChangeDetector /// doing so can result in application failures when updating to a new Entity Framework Core release. /// void DetectChanges(InternalEntityEntry entry); - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - void Suspend(); - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - void Resume(); } diff --git a/src/EFCore/ChangeTracking/Internal/INavigationFixer.cs b/src/EFCore/ChangeTracking/Internal/INavigationFixer.cs index 5494f30ee4f..7c4b1a72647 100644 --- a/src/EFCore/ChangeTracking/Internal/INavigationFixer.cs +++ b/src/EFCore/ChangeTracking/Internal/INavigationFixer.cs @@ -23,7 +23,7 @@ public interface INavigationFixer /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - void BeginAttachGraph(); + bool BeginDelayedFixup(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -31,7 +31,7 @@ public interface INavigationFixer /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - void CompleteAttachGraph(); + void CompleteDelayedFixup(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -39,7 +39,7 @@ public interface INavigationFixer /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - void AbortAttachGraph(); + void AbortDelayedFixup(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs index f392b305145..15ffc957b0d 100644 --- a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs +++ b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs @@ -15,7 +15,6 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal; public class NavigationFixer : INavigationFixer { private IList? _danglingJoinEntities; - private readonly IChangeDetector _changeDetector; private readonly IEntityGraphAttacher _attacher; private bool _inFixup; private bool _inAttachGraph; @@ -26,11 +25,8 @@ public class NavigationFixer : INavigationFixer /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public NavigationFixer( - IChangeDetector changeDetector, - IEntityGraphAttacher attacher) + public NavigationFixer(IEntityGraphAttacher attacher) { - _changeDetector = changeDetector; _attacher = attacher; } @@ -40,10 +36,17 @@ public NavigationFixer( /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual void BeginAttachGraph() + public virtual bool BeginDelayedFixup() { + if (_inAttachGraph) + { + return false; + } + _danglingJoinEntities?.Clear(); _inAttachGraph = true; + + return true; } /// @@ -52,23 +55,18 @@ public virtual void BeginAttachGraph() /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual void CompleteAttachGraph() + public virtual void CompleteDelayedFixup() { _inAttachGraph = false; - try + if (_danglingJoinEntities != null) { - if (_danglingJoinEntities != null) + var dangles = _danglingJoinEntities.ToList(); + _danglingJoinEntities.Clear(); + foreach (var synthesize in dangles) { - foreach (var synthesize in _danglingJoinEntities) - { - synthesize(); - } + synthesize(); } } - finally - { - _danglingJoinEntities?.Clear(); - } } /// @@ -77,7 +75,7 @@ public virtual void CompleteAttachGraph() /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual void AbortAttachGraph() + public virtual void AbortDelayedFixup() { _inAttachGraph = false; _danglingJoinEntities?.Clear(); @@ -120,10 +118,9 @@ public virtual void NavigationReferenceChanged( newTargetEntry = null; } + var delayingFixup = BeginDelayedFixup(); try { - _inFixup = true; - if (navigation.IsOnDependent) { if (newValue != null) @@ -226,7 +223,10 @@ public virtual void NavigationReferenceChanged( } finally { - _inFixup = false; + if (delayingFixup) + { + CompleteDelayedFixup(); + } } if (newValue != null @@ -273,13 +273,17 @@ public virtual void NavigationCollectionChanged( if (oldTargetEntry != null && oldTargetEntry.EntityState != EntityState.Detached) { + var delayingFixup = BeginDelayedFixup(); try { - _inFixup = true; - if (navigationBase is ISkipNavigation skipNavigation) { - FindJoinEntry(entry, oldTargetEntry, skipNavigation)?.SetEntityState(EntityState.Deleted); + var joinEntry = FindJoinEntry(entry, oldTargetEntry, skipNavigation); + + joinEntry?.SetEntityState( + joinEntry.EntityState != EntityState.Added + ? EntityState.Deleted + : EntityState.Detached); Check.DebugAssert( skipNavigation.Inverse.IsCollection, @@ -309,7 +313,10 @@ public virtual void NavigationCollectionChanged( } finally { - _inFixup = false; + if (delayingFixup) + { + CompleteDelayedFixup(); + } } } } @@ -319,10 +326,9 @@ public virtual void NavigationCollectionChanged( var newTargetEntry = stateManager.GetOrCreateEntry(newValue, targetEntityType); if (newTargetEntry.EntityState != EntityState.Detached) { + var delayingFixup = BeginDelayedFixup(); try { - _inFixup = true; - if (navigationBase is ISkipNavigation skipNavigation) { FindOrCreateJoinEntry( @@ -357,7 +363,10 @@ public virtual void NavigationCollectionChanged( } finally { - _inFixup = false; + if (delayingFixup) + { + CompleteDelayedFixup(); + } } } else @@ -394,10 +403,9 @@ public virtual void KeyPropertyChanged( return; } + var delayingFixup = BeginDelayedFixup(); try { - _inFixup = true; - var stateManager = entry.StateManager; foreach (var foreignKey in containingForeignKeys) @@ -538,7 +546,10 @@ var targetDependentEntry } finally { - _inFixup = false; + if (delayingFixup) + { + CompleteDelayedFixup(); + } } } @@ -558,20 +569,8 @@ public virtual void StateChanging(InternalEntityEntry entry, EntityState newStat /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual void TrackedFromQuery( - InternalEntityEntry entry) - { - try - { - _inFixup = true; - - InitialFixup(entry, fromQuery: true); - } - finally - { - _inFixup = false; - } - } + public virtual void TrackedFromQuery(InternalEntityEntry entry) + => InitialFixup(entry, fromQuery: true); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -584,20 +583,15 @@ public virtual void StateChanged( EntityState oldState, bool fromQuery) { - if (fromQuery || _inFixup) - { - return; - } - + var delayingFixup = BeginDelayedFixup(); try { - _inFixup = true; - if (oldState == EntityState.Detached) { - InitialFixup(entry, fromQuery: false); + InitialFixup(entry, fromQuery); } - else if (oldState == EntityState.Deleted + else if ((oldState == EntityState.Deleted + || oldState == EntityState.Added) && entry.EntityState == EntityState.Detached) { DeleteFixup(entry); @@ -605,7 +599,10 @@ public virtual void StateChanged( } finally { - _inFixup = false; + if (delayingFixup) + { + CompleteDelayedFixup(); + } } } @@ -1314,18 +1311,26 @@ private void ConditionallyNullForeignKeyProperties( && hasOnlyKeyProperties && dependentEntry.EntityState != EntityState.Detached) { - switch (dependentEntry.EntityState) + try + { + _inFixup = true; + switch (dependentEntry.EntityState) + { + case EntityState.Added: + dependentEntry.SetEntityState(EntityState.Detached); + DeleteFixup(dependentEntry); + break; + case EntityState.Unchanged: + case EntityState.Modified: + dependentEntry.SetEntityState( + dependentEntry.SharedIdentityEntry != null ? EntityState.Detached : EntityState.Deleted); + DeleteFixup(dependentEntry); + break; + } + } + finally { - case EntityState.Added: - dependentEntry.SetEntityState(EntityState.Detached); - DeleteFixup(dependentEntry); - break; - case EntityState.Unchanged: - case EntityState.Modified: - dependentEntry.SetEntityState( - dependentEntry.SharedIdentityEntry != null ? EntityState.Detached : EntityState.Deleted); - DeleteFixup(dependentEntry); - break; + _inFixup = false; } } } @@ -1350,7 +1355,7 @@ private void SetNavigation(InternalEntityEntry entry, INavigationBase? navigatio { if (navigation != null) { - _changeDetector.Suspend(); + _inFixup = true; var entity = value?.Entity; try { @@ -1358,7 +1363,7 @@ private void SetNavigation(InternalEntityEntry entry, INavigationBase? navigatio } finally { - _changeDetector.Resume(); + _inFixup = false; } entry.SetRelationshipSnapshotValue(navigation, entity); @@ -1369,7 +1374,7 @@ private void AddToCollection(InternalEntityEntry entry, INavigationBase? navigat { if (navigation != null) { - _changeDetector.Suspend(); + _inFixup = true; try { if (entry.AddToCollection(navigation, value, fromQuery)) @@ -1379,14 +1384,14 @@ private void AddToCollection(InternalEntityEntry entry, INavigationBase? navigat } finally { - _changeDetector.Resume(); + _inFixup = false; } } } private void RemoveFromCollection(InternalEntityEntry entry, INavigationBase navigation, InternalEntityEntry value) { - _changeDetector.Suspend(); + _inFixup = true; try { if (entry.RemoveFromCollection(navigation, value)) @@ -1396,7 +1401,7 @@ private void RemoveFromCollection(InternalEntityEntry entry, INavigationBase nav } finally { - _changeDetector.Resume(); + _inFixup = false; } } diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs index 724c9d8359d..b7915bbbd29 100644 --- a/src/EFCore/ChangeTracking/Internal/StateManager.cs +++ b/src/EFCore/ChangeTracking/Internal/StateManager.cs @@ -638,7 +638,7 @@ public virtual void Unsubscribe() public virtual void ResetState() { Clear(); - Dependencies.NavigationFixer.AbortAttachGraph(); + Dependencies.NavigationFixer.AbortDelayedFixup(); Tracked = null; StateChanged = null; @@ -688,7 +688,7 @@ public virtual Task ResetStateAsync(CancellationToken cancellationToken = defaul /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual void BeginAttachGraph() - => Dependencies.NavigationFixer.BeginAttachGraph(); + => Dependencies.NavigationFixer.BeginDelayedFixup(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -697,7 +697,7 @@ public virtual void BeginAttachGraph() /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual void CompleteAttachGraph() - => Dependencies.NavigationFixer.CompleteAttachGraph(); + => Dependencies.NavigationFixer.CompleteDelayedFixup(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -706,7 +706,7 @@ public virtual void CompleteAttachGraph() /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public virtual void AbortAttachGraph() - => Dependencies.NavigationFixer.AbortAttachGraph(); + => Dependencies.NavigationFixer.AbortDelayedFixup(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs b/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs index 5ca2ddeea69..8b8bd5d12b0 100644 --- a/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs +++ b/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs @@ -4856,6 +4856,1017 @@ static void ValidateFixup(DbContext context, IList leftEntities, ILis } } + [ConditionalTheory] + [InlineData(false, false, false, false)] + [InlineData(false, true, false, false)] + [InlineData(true, false, false, false)] + [InlineData(true, true, false, false)] + [InlineData(false, false, true, false)] + [InlineData(false, true, true, false)] + [InlineData(true, false, true, false)] + [InlineData(true, true, true, false)] + [InlineData(false, false, true, true)] + [InlineData(false, true, true, true)] + [InlineData(true, false, true, true)] + [InlineData(true, true, true, true)] + public virtual void Can_add_and_remove_a_new_relationship(bool modifyLeft, bool modifyRight, bool useJoin, bool useNavs) + { + var leftId = -1; + var rightId = -1; + + ExecuteWithStrategyInTransaction( + context => + { + var left = context.Set().Where(e => !e.TwoSkip.Any()).OrderBy(e => e.Id).First(); + var right = context.Set().OrderBy(e => e.Id).First(); + + if (modifyLeft) + { + context.Entry(left).State = EntityState.Modified; + } + + if (modifyRight) + { + context.Entry(right).State = EntityState.Modified; + } + + leftId = left.Id; + rightId = right.Id; + + if (useJoin) + { + context.Add( + RequiresDetectChanges + ? useNavs + ? new JoinOneToTwo { One = left, Two = right } + : new JoinOneToTwo { OneId = leftId, TwoId = rightId } + : useNavs + ? context.CreateProxy( + e => + { + e.One = left; + e.Two = right; + }) + : context.CreateProxy( + e => + { + e.OneId = leftId; + e.TwoId = rightId; + })); + } + else + { + left.TwoSkip ??= CreateCollection(); + left.TwoSkip.Add(right); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Same(right, left.TwoSkip.Single()); + Assert.Same(left, right.OneSkip.Single()); + + var joinEntry = context.ChangeTracker.Entries().Single(); + Assert.Equal(EntityState.Added, joinEntry.State); + Assert.Equal(left.Id, joinEntry.Entity.OneId); + Assert.Equal(right.Id, joinEntry.Entity.TwoId); + Assert.Same(left, joinEntry.Entity.One); + Assert.Same(right, joinEntry.Entity.Two); + + if (useJoin) + { + joinEntry.State = EntityState.Detached; + } + else + { + right.OneSkip.Remove(left); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Empty(left.TwoSkip); + Assert.Empty(right.OneSkip); + + Assert.Equal(EntityState.Detached, joinEntry.State); + Assert.Equal(leftId, joinEntry.Entity.OneId); + Assert.Equal(rightId, joinEntry.Entity.TwoId); + Assert.Same(left, joinEntry.Entity.One); + Assert.Same(right, joinEntry.Entity.Two); + + context.SaveChanges(); + }, + context => + { + var left = context.Set().Where(e => !e.TwoSkip.Any()).OrderBy(e => e.Id).First(); + var right = context.Set().OrderBy(e => e.Id).First(); + + Assert.Equal(leftId, left.Id); + Assert.Equal(rightId, right.Id); + }); + } + + [ConditionalTheory] + [InlineData(false, false, false)] + [InlineData(false, true, false)] + [InlineData(true, false, false)] + [InlineData(true, true, false)] + [InlineData(false, false, true)] + [InlineData(false, true, true)] + [InlineData(true, false, true)] + [InlineData(true, true, true)] + public virtual void Can_add_and_remove_a_new_relationship_self(bool modifyLeft, bool modifyRight, bool useJoin) + { + var leftId = -1; + var rightId = -1; + + ExecuteWithStrategyInTransaction( + context => + { + var left = context.Set().Where(e => !e.SelfSkipSharedRight.Any()).OrderBy(e => e.Id).First(); + var right = context.Set().Where(e => !e.SelfSkipSharedLeft.Any()).OrderBy(e => e.Id).First(); + + if (modifyLeft) + { + context.Entry(left).State = EntityState.Modified; + } + + if (modifyRight) + { + context.Entry(right).State = EntityState.Modified; + } + + leftId = left.Id; + rightId = right.Id; + + if (useJoin && RequiresDetectChanges) + { + context.Set>("EntityTwoEntityTwo") + .Add(new Dictionary { ["SelfSkipSharedLeftId"] = leftId, ["SelfSkipSharedRightId"] = rightId }); + } + else + { + left.SelfSkipSharedRight ??= CreateCollection(); + left.SelfSkipSharedRight.Add(right); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Same(right, left.SelfSkipSharedRight.Single()); + Assert.Same(left, right.SelfSkipSharedLeft.Single()); + + var joinEntry = context.ChangeTracker.Entries>().Single(); + Assert.Equal(EntityState.Added, joinEntry.State); + Assert.Equal(left.Id, joinEntry.Entity["SelfSkipSharedLeftId"]); + Assert.Equal(right.Id, joinEntry.Entity["SelfSkipSharedRightId"]); + + if (useJoin) + { + joinEntry.State = EntityState.Detached; + } + else + { + left.SelfSkipSharedRight.Remove(right); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Empty(left.SelfSkipSharedRight); + Assert.Empty(right.SelfSkipSharedLeft); + + Assert.Equal(EntityState.Detached, joinEntry.State); + Assert.Equal(left.Id, joinEntry.Entity["SelfSkipSharedLeftId"]); + Assert.Equal(right.Id, joinEntry.Entity["SelfSkipSharedRightId"]); + + context.SaveChanges(); + }, + context => + { + var left = context.Set().Where(e => !e.SelfSkipSharedRight.Any()).OrderBy(e => e.Id).First(); + var right = context.Set().Where(e => !e.SelfSkipSharedLeft.Any()).OrderBy(e => e.Id).First(); + + Assert.Equal(leftId, left.Id); + Assert.Equal(rightId, right.Id); + }); + } + + [ConditionalTheory] + [InlineData(false, false, false, false)] + [InlineData(false, true, false, false)] + [InlineData(true, false, false, false)] + [InlineData(true, true, false, false)] + [InlineData(false, false, true, false)] + [InlineData(false, true, true, false)] + [InlineData(true, false, true, false)] + [InlineData(true, true, true, false)] + [InlineData(false, false, true, true)] + [InlineData(false, true, true, true)] + [InlineData(true, false, true, true)] + [InlineData(true, true, true, true)] + public virtual void Can_add_and_remove_a_new_relationship_composite_with_navs( + bool modifyLeft, + bool modifyRight, + bool useJoin, + bool useNavs) + { + var leftKey1 = -1; + var leftKey2 = "-1"; + var leftKey3 = new DateTime(); + var rightId = -1; + + ExecuteWithStrategyInTransaction( + context => + { + var left = context.Set() + .Where(e => !e.LeafSkipFull.Any()) + .OrderBy(e => e.Key1) + .ThenBy(e => e.Key2) + .ThenBy(e => e.Key3) + .First(); + + var right = context.Set().OrderBy(e => e.Id).First(); + + if (modifyLeft) + { + context.Entry(left).State = EntityState.Modified; + } + + if (modifyRight) + { + context.Entry(right).State = EntityState.Modified; + } + + leftKey1 = left.Key1; + leftKey2 = left.Key2; + leftKey3 = left.Key3; + rightId = right.Id; + + if (useJoin) + { + context.Add( + RequiresDetectChanges + ? useNavs + ? new JoinCompositeKeyToLeaf { Composite = left, Leaf = right } + : new JoinCompositeKeyToLeaf + { + CompositeId1 = leftKey1, + CompositeId2 = leftKey2, + CompositeId3 = leftKey3, + LeafId = rightId + } + : useNavs + ? context.CreateProxy( + e => + { + e.Composite = left; + e.Leaf = right; + }) + : context.CreateProxy( + e => + { + e.CompositeId1 = leftKey1; + e.CompositeId2 = leftKey2; + e.CompositeId3 = leftKey3; + e.LeafId = rightId; + })); + } + else + { + left.LeafSkipFull ??= CreateCollection(); + left.LeafSkipFull.Add(right); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Same(right, left.LeafSkipFull.Single()); + Assert.Same(left, right.CompositeKeySkipFull.Single()); + + var joinEntry = context.ChangeTracker.Entries().Single(); + Assert.Equal(EntityState.Added, joinEntry.State); + Assert.Equal(left.Key1, joinEntry.Entity.CompositeId1); + Assert.Equal(left.Key2, joinEntry.Entity.CompositeId2); + Assert.Equal(left.Key3, joinEntry.Entity.CompositeId3); + Assert.Equal(right.Id, joinEntry.Entity.LeafId); + Assert.Same(left, joinEntry.Entity.Composite); + Assert.Same(right, joinEntry.Entity.Leaf); + Assert.Contains(joinEntry.Entity, left.JoinLeafFull); + Assert.Contains(joinEntry.Entity, right.JoinCompositeKeyFull); + + if (useJoin) + { + joinEntry.State = EntityState.Detached; + } + else + { + right.CompositeKeySkipFull.Remove(left); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Empty(left.LeafSkipFull); + Assert.Empty(right.CompositeKeySkipFull); + + Assert.Equal(EntityState.Detached, joinEntry.State); + Assert.Equal(left.Key1, joinEntry.Entity.CompositeId1); + Assert.Equal(left.Key2, joinEntry.Entity.CompositeId2); + Assert.Equal(left.Key3, joinEntry.Entity.CompositeId3); + Assert.Equal(right.Id, joinEntry.Entity.LeafId); + Assert.Same(left, joinEntry.Entity.Composite); + Assert.Same(right, joinEntry.Entity.Leaf); + Assert.DoesNotContain(joinEntry.Entity, left.JoinLeafFull); + Assert.DoesNotContain(joinEntry.Entity, right.JoinCompositeKeyFull); + + context.SaveChanges(); + }, + context => + { + var left = context.Set() + .Where(e => !e.LeafSkipFull.Any()) + .OrderBy(e => e.Key1) + .ThenBy(e => e.Key2) + .ThenBy(e => e.Key3) + .First(); + + var right = context.Set().OrderBy(e => e.Id).First(); + + Assert.Equal(leftKey1, left.Key1); + Assert.Equal(leftKey2, left.Key2); + Assert.Equal(leftKey3, left.Key3); + Assert.Equal(rightId, right.Id); + }); + } + + [ConditionalTheory] + [InlineData(false, false, false, false)] + [InlineData(false, true, false, false)] + [InlineData(true, false, false, false)] + [InlineData(true, true, false, false)] + [InlineData(false, false, true, false)] + [InlineData(false, true, true, false)] + [InlineData(true, false, true, false)] + [InlineData(true, true, true, false)] + [InlineData(false, false, true, true)] + [InlineData(false, true, true, true)] + [InlineData(true, false, true, true)] + [InlineData(true, true, true, true)] + public virtual void Can_add_and_remove_a_new_relationship_composite_additional_pk_with_navs( + bool modifyLeft, + bool modifyRight, + bool useJoin, + bool useNavs) + { + var leftKey1 = -1; + var leftKey2 = "-1"; + var leftKey3 = new DateTime(); + var rightId = -1; + + ExecuteWithStrategyInTransaction( + context => + { + var left = context.Set() + .Where(e => !e.ThreeSkipFull.Any()) + .OrderBy(e => e.Key1) + .ThenBy(e => e.Key2) + .ThenBy(e => e.Key3) + .First(); + + var right = context.Set().OrderBy(e => e.Id).First(); + + if (modifyLeft) + { + context.Entry(left).State = EntityState.Modified; + } + + if (modifyRight) + { + context.Entry(right).State = EntityState.Modified; + } + + leftKey1 = left.Key1; + leftKey2 = left.Key2; + leftKey3 = left.Key3; + rightId = right.Id; + + if (useJoin) + { + context.Add( + RequiresDetectChanges + ? useNavs + ? new JoinThreeToCompositeKeyFull { Composite = left, Three = right } + : new JoinThreeToCompositeKeyFull + { + CompositeId1 = leftKey1, + CompositeId2 = leftKey2, + CompositeId3 = leftKey3, + ThreeId = rightId + } + : useNavs + ? context.CreateProxy( + e => + { + e.Composite = left; + e.Three = right; + }) + : context.CreateProxy( + e => + { + e.CompositeId1 = leftKey1; + e.CompositeId2 = leftKey2; + e.CompositeId3 = leftKey3; + e.ThreeId = rightId; + })); + } + else + { + left.ThreeSkipFull ??= CreateCollection(); + left.ThreeSkipFull.Add(right); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Same(right, left.ThreeSkipFull.Single()); + Assert.Same(left, right.CompositeKeySkipFull.Single()); + + var joinEntry = context.ChangeTracker.Entries().Single(); + Assert.Equal(EntityState.Added, joinEntry.State); + Assert.Equal(left.Key1, joinEntry.Entity.CompositeId1); + Assert.Equal(left.Key2, joinEntry.Entity.CompositeId2); + Assert.Equal(left.Key3, joinEntry.Entity.CompositeId3); + Assert.Equal(right.Id, joinEntry.Entity.ThreeId); + Assert.Same(left, joinEntry.Entity.Composite); + Assert.Same(right, joinEntry.Entity.Three); + Assert.Contains(joinEntry.Entity, left.JoinThreeFull); + Assert.Contains(joinEntry.Entity, right.JoinCompositeKeyFull); + + if (useJoin) + { + joinEntry.State = EntityState.Detached; + } + else + { + right.CompositeKeySkipFull.Remove(left); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Empty(left.ThreeSkipFull); + Assert.Empty(right.CompositeKeySkipFull); + + Assert.Equal(EntityState.Detached, joinEntry.State); + Assert.Equal(left.Key1, joinEntry.Entity.CompositeId1); + Assert.Equal(left.Key2, joinEntry.Entity.CompositeId2); + Assert.Equal(left.Key3, joinEntry.Entity.CompositeId3); + Assert.Equal(right.Id, joinEntry.Entity.ThreeId); + Assert.Same(left, joinEntry.Entity.Composite); + Assert.Same(right, joinEntry.Entity.Three); + Assert.DoesNotContain(joinEntry.Entity, left.JoinThreeFull); + Assert.DoesNotContain(joinEntry.Entity, right.JoinCompositeKeyFull); + + context.SaveChanges(); + }, + context => + { + var left = context.Set() + .Where(e => !e.ThreeSkipFull.Any()) + .OrderBy(e => e.Key1) + .ThenBy(e => e.Key2) + .ThenBy(e => e.Key3) + .First(); + + var right = context.Set().OrderBy(e => e.Id).First(); + + Assert.Equal(leftKey1, left.Key1); + Assert.Equal(leftKey2, left.Key2); + Assert.Equal(leftKey3, left.Key3); + Assert.Equal(rightId, right.Id); + }); + } + + [ConditionalTheory] + [InlineData(false, false, false)] + [InlineData(false, true, false)] + [InlineData(true, false, false)] + [InlineData(true, true, false)] + [InlineData(false, false, true)] + [InlineData(false, true, true)] + [InlineData(true, false, true)] + [InlineData(true, true, true)] + public virtual void Can_add_and_remove_a_new_relationship_with_inheritance(bool modifyLeft, bool modifyRight, bool useJoin) + { + var leftId = -1; + var rightId = -1; + + ExecuteWithStrategyInTransaction( + context => + { + var left = context.Set().Where(e => !e.BranchSkip.Any()).OrderBy(e => e.Id).First(); + var right = context.Set().OrderBy(e => e.Id).First(); + + if (modifyLeft) + { + context.Entry(left).State = EntityState.Modified; + } + + if (modifyRight) + { + context.Entry(right).State = EntityState.Modified; + } + + leftId = left.Id; + rightId = right.Id; + + if (useJoin) + { + context.Add( + RequiresDetectChanges + ? new JoinOneToBranch { EntityOneId = leftId, EntityBranchId = rightId } + : context.CreateProxy( + e => + { + e.EntityOneId = leftId; + e.EntityBranchId = rightId; + })); + } + else + { + left.BranchSkip ??= CreateCollection(); + left.BranchSkip.Add(right); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Same(right, left.BranchSkip.Single()); + Assert.Same(left, right.OneSkip.Single()); + + var joinEntry = context.ChangeTracker.Entries().Single(); + Assert.Equal(EntityState.Added, joinEntry.State); + Assert.Equal(left.Id, joinEntry.Entity.EntityOneId); + Assert.Equal(right.Id, joinEntry.Entity.EntityBranchId); + + if (useJoin) + { + joinEntry.State = EntityState.Detached; + } + else + { + right.OneSkip.Remove(left); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Empty(left.BranchSkip); + Assert.Empty(right.OneSkip); + + Assert.Equal(EntityState.Detached, joinEntry.State); + Assert.Equal(leftId, joinEntry.Entity.EntityOneId); + Assert.Equal(rightId, joinEntry.Entity.EntityBranchId); + + context.SaveChanges(); + }, + context => + { + var left = context.Set().Where(e => !e.BranchSkip.Any()).OrderBy(e => e.Id).First(); + var right = context.Set().OrderBy(e => e.Id).First(); + + Assert.Equal(leftId, left.Id); + Assert.Equal(rightId, right.Id); + }); + } + + [ConditionalTheory] + [InlineData(false, false, false)] + [InlineData(false, true, false)] + [InlineData(true, false, false)] + [InlineData(true, true, false)] + [InlineData(false, false, true)] + [InlineData(false, true, true)] + [InlineData(true, false, true)] + [InlineData(true, true, true)] + public virtual void Can_add_and_remove_a_new_relationship_shared_with_payload(bool modifyLeft, bool modifyRight, bool useJoin) + { + var leftId = -1; + var rightId = -1; + + ExecuteWithStrategyInTransaction( + context => + { + var left = context.Set().Where(e => !e.ThreeSkipPayloadFullShared.Any()).OrderBy(e => e.Id).First(); + var right = context.Set().OrderBy(e => e.Id).First(); + + if (modifyLeft) + { + context.Entry(left).State = EntityState.Modified; + } + + if (modifyRight) + { + context.Entry(right).State = EntityState.Modified; + } + + leftId = left.Id; + rightId = right.Id; + + if (useJoin && RequiresDetectChanges) + { + context.Set>("JoinOneToThreePayloadFullShared") + .Add(new Dictionary { ["OneId"] = leftId, ["ThreeId"] = rightId }); + } + else + { + left.ThreeSkipPayloadFullShared ??= CreateCollection(); + left.ThreeSkipPayloadFullShared.Add(right); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Same(right, left.ThreeSkipPayloadFullShared.Single()); + Assert.Same(left, right.OneSkipPayloadFullShared.Single()); + + var joinEntry = context.ChangeTracker.Entries>().Single(); + Assert.Equal(EntityState.Added, joinEntry.State); + Assert.Equal(left.Id, (int)joinEntry.Entity["OneId"]); + Assert.Equal(right.Id, (int)joinEntry.Entity["ThreeId"]); + Assert.Contains(joinEntry.Entity, left.JoinThreePayloadFullShared); + Assert.Contains(joinEntry.Entity, right.JoinOnePayloadFullShared); + + if (useJoin) + { + joinEntry.State = EntityState.Detached; + } + else + { + right.OneSkipPayloadFullShared.Remove(left); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Empty(left.ThreeSkipPayloadFullShared); + Assert.Empty(right.OneSkipPayloadFullShared); + Assert.Equal(left.Id, (int)joinEntry.Entity["OneId"]); + Assert.Equal(right.Id, (int)joinEntry.Entity["ThreeId"]); + Assert.DoesNotContain(joinEntry.Entity, left.JoinThreePayloadFullShared); + Assert.DoesNotContain(joinEntry.Entity, right.JoinOnePayloadFullShared); + + Assert.Equal(EntityState.Detached, joinEntry.State); + + context.SaveChanges(); + }, + context => + { + var left = context.Set().Where(e => !e.ThreeSkipPayloadFullShared.Any()).OrderBy(e => e.Id).First(); + var right = context.Set().OrderBy(e => e.Id).First(); + + Assert.Equal(leftId, left.Id); + Assert.Equal(rightId, right.Id); + }); + } + + [ConditionalTheory] + [InlineData(false, false, false)] + [InlineData(false, true, false)] + [InlineData(true, false, false)] + [InlineData(true, true, false)] + [InlineData(false, false, true)] + [InlineData(false, true, true)] + [InlineData(true, false, true)] + [InlineData(true, true, true)] + public virtual void Can_add_and_remove_a_new_relationship_shared(bool modifyLeft, bool modifyRight, bool useJoin) + { + var leftId = -1; + var rightId = -1; + + ExecuteWithStrategyInTransaction( + context => + { + var left = context.Set().Where(e => !e.TwoSkipShared.Any()).OrderBy(e => e.Id).First(); + var right = context.Set().OrderBy(e => e.Id).First(); + + if (modifyLeft) + { + context.Entry(left).State = EntityState.Modified; + } + + if (modifyRight) + { + context.Entry(right).State = EntityState.Modified; + } + + leftId = left.Id; + rightId = right.Id; + + if (useJoin && RequiresDetectChanges) + { + context.Set>("EntityOneEntityTwo") + .Add(new Dictionary { ["OneSkipSharedId"] = leftId, ["TwoSkipSharedId"] = rightId }); + } + else + { + left.TwoSkipShared ??= CreateCollection(); + left.TwoSkipShared.Add(right); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Same(right, left.TwoSkipShared.Single()); + Assert.Same(left, right.OneSkipShared.Single()); + + var joinEntry = context.ChangeTracker.Entries>().Single(); + Assert.Equal(EntityState.Added, joinEntry.State); + Assert.Equal(left.Id, (int)joinEntry.Entity["OneSkipSharedId"]); + Assert.Equal(right.Id, (int)joinEntry.Entity["TwoSkipSharedId"]); + + if (useJoin) + { + joinEntry.State = EntityState.Detached; + } + else + { + right.OneSkipShared.Remove(left); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Empty(left.TwoSkipShared); + Assert.Empty(right.OneSkipShared); + + Assert.Equal(EntityState.Detached, joinEntry.State); + Assert.Equal(left.Id, (int)joinEntry.Entity["OneSkipSharedId"]); + Assert.Equal(right.Id, (int)joinEntry.Entity["TwoSkipSharedId"]); + + context.SaveChanges(); + }, + context => + { + var left = context.Set().Where(e => !e.TwoSkipShared.Any()).OrderBy(e => e.Id).First(); + var right = context.Set().OrderBy(e => e.Id).First(); + + Assert.Equal(leftId, left.Id); + Assert.Equal(rightId, right.Id); + }); + } + + [ConditionalTheory] + [InlineData(false, false, false, false)] + [InlineData(false, true, false, false)] + [InlineData(true, false, false, false)] + [InlineData(true, true, false, false)] + [InlineData(false, false, true, false)] + [InlineData(false, true, true, false)] + [InlineData(true, false, true, false)] + [InlineData(true, true, true, false)] + [InlineData(false, false, true, true)] + [InlineData(false, true, true, true)] + [InlineData(true, false, true, true)] + [InlineData(true, true, true, true)] + public virtual void Can_add_and_remove_a_new_relationship_with_payload( + bool modifyLeft, + bool modifyRight, + bool useJoin, + bool useNavs) + { + var leftId = -1; + var rightId = -1; + + ExecuteWithStrategyInTransaction( + context => + { + var left = context.Set().Where(e => !e.ThreeSkipPayloadFull.Any()).OrderBy(e => e.Id).First(); + var right = context.Set().OrderBy(e => e.Id).First(); + + if (modifyLeft) + { + context.Entry(left).State = EntityState.Modified; + } + + if (modifyRight) + { + context.Entry(right).State = EntityState.Modified; + } + + leftId = left.Id; + rightId = right.Id; + + if (useJoin) + { + context.Add( + RequiresDetectChanges + ? useNavs + ? new JoinOneToThreePayloadFull { One = left, Three = right } + : new JoinOneToThreePayloadFull { OneId = leftId, ThreeId = rightId } + : useNavs + ? context.CreateProxy( + e => + { + e.One = left; + e.Three = right; + }) + : context.CreateProxy( + e => + { + e.OneId = leftId; + e.ThreeId = rightId; + })); + } + else + { + left.ThreeSkipPayloadFull ??= CreateCollection(); + left.ThreeSkipPayloadFull.Add(right); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Same(right, left.ThreeSkipPayloadFull.Single()); + Assert.Same(left, right.OneSkipPayloadFull.Single()); + + var joinEntry = context.ChangeTracker.Entries().Single(); + Assert.Equal(EntityState.Added, joinEntry.State); + Assert.Equal(left.Id, joinEntry.Entity.OneId); + Assert.Equal(right.Id, joinEntry.Entity.ThreeId); + Assert.Same(left, joinEntry.Entity.One); + Assert.Same(right, joinEntry.Entity.Three); + Assert.Contains(joinEntry.Entity, left.JoinThreePayloadFull); + Assert.Contains(joinEntry.Entity, right.JoinOnePayloadFull); + + if (useJoin) + { + joinEntry.State = EntityState.Detached; + } + else + { + right.OneSkipPayloadFull.Remove(left); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + } + + Assert.Empty(left.ThreeSkipPayloadFull); + Assert.Empty(right.OneSkipPayloadFull); + + Assert.Equal(EntityState.Detached, joinEntry.State); + Assert.Equal(leftId, joinEntry.Entity.OneId); + Assert.Equal(rightId, joinEntry.Entity.ThreeId); + Assert.Same(left, joinEntry.Entity.One); + Assert.Same(right, joinEntry.Entity.Three); + Assert.DoesNotContain(joinEntry.Entity, left.JoinThreePayloadFull); + Assert.DoesNotContain(joinEntry.Entity, right.JoinOnePayloadFull); + + context.SaveChanges(); + }, + context => + { + var left = context.Set().Where(e => !e.ThreeSkipPayloadFull.Any()).OrderBy(e => e.Id).First(); + var right = context.Set().OrderBy(e => e.Id).First(); + + Assert.Equal(leftId, left.Id); + Assert.Equal(rightId, right.Id); + }); + } + + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public virtual async Task Can_replace_dependent_with_many_to_many(bool async) + { + var principalKey = 0; + var newRightKey = 0; + + await ExecuteWithStrategyInTransactionAsync( + async context => + { + var principal = context.EntityOnes.CreateInstance((e, p) => e.Id = Fixture.UseGeneratedKeys ? 0 : 7711); + var leftEntity = context.EntityTwos.CreateInstance((e, p) => e.Id = Fixture.UseGeneratedKeys ? 0 : 7721); + var rightEntity = context.EntityThrees.CreateInstance((e, p) => e.Id = Fixture.UseGeneratedKeys ? 0 : 7731); + + principal.Reference = leftEntity; + + leftEntity.ThreeSkipFull ??= CreateCollection(); + leftEntity.ThreeSkipFull.Add(rightEntity); + + _ = async + ? await context.AddAsync(principal) + : context.Add(principal); + + ValidateFixup(context, principal, leftEntity, rightEntity); + + _ = async + ? await context.SaveChangesAsync() + : context.SaveChanges(); + + ValidateFixup(context, principal, leftEntity, rightEntity); + + principalKey = principal.Id; + }, + async context => + { + var queryable = context.Set().Where(e => principalKey == e.Id).Include(e => e.Reference.ThreeSkipFull); + var principal = async ? await queryable.FirstAsync() : queryable.First(); + + var leftEntity = context.ChangeTracker.Entries().Select(e => e.Entity).Single(); + var rightEntity = context.ChangeTracker.Entries().Select(e => e.Entity).Single(); + + ValidateFixup(context, principal, leftEntity, rightEntity); + + var newLeftEntity = context.EntityTwos.CreateInstance((e, p) => e.Id = Fixture.UseGeneratedKeys ? 0 : 7722); + var newRightEntity = context.EntityThrees.CreateInstance((e, p) => e.Id = Fixture.UseGeneratedKeys ? 0 : 7732); + + principal.Reference = newLeftEntity; + + newLeftEntity.ThreeSkipFull ??= CreateCollection(); + newLeftEntity.ThreeSkipFull.Add(newRightEntity); + + if (RequiresDetectChanges) + { + context.ChangeTracker.DetectChanges(); + } + + context.Remove(leftEntity); + context.Remove(rightEntity); + + _ = async + ? await context.SaveChangesAsync() + : context.SaveChanges(); + + ValidateFixup(context, principal, newLeftEntity, newRightEntity); + newRightKey = newRightEntity.Id; + }, + async context => + { + var queryable = context.Set().Where(e => principalKey == e.Id).Include(e => e.Reference.ThreeSkipFull); + var principal = async ? await queryable.FirstAsync() : queryable.First(); + + var leftEntity = context.ChangeTracker.Entries().Select(e => e.Entity).Single(); + var rightEntity = context.ChangeTracker.Entries().Select(e => e.Entity).Single(); + + ValidateFixup(context, principal, leftEntity, rightEntity); + + Assert.Equal(newRightKey, principal.Reference.ThreeSkipFull.Single().Id); + }); + + static void ValidateFixup(DbContext context, EntityOne principal, EntityTwo leftEntity, EntityThree rightEntity) + { + Assert.Equal(4, context.ChangeTracker.Entries().Count()); + Assert.Single(context.ChangeTracker.Entries()); + Assert.Single(context.ChangeTracker.Entries()); + Assert.Single(context.ChangeTracker.Entries()); + Assert.Single(context.ChangeTracker.Entries()); + + Assert.Same(leftEntity, principal.Reference); + Assert.Same(principal, leftEntity.ReferenceInverse); + + Assert.Same(rightEntity, leftEntity.ThreeSkipFull.Single()); + Assert.Same(leftEntity, rightEntity.TwoSkipFull.Single()); + + VerifyRelationshipSnapshots(context, new[] { principal }); + VerifyRelationshipSnapshots(context, new[] { leftEntity }); + VerifyRelationshipSnapshots(context, new[] { rightEntity }); + } + } + protected static void VerifyRelationshipSnapshots(DbContext context, IEnumerable entities) { var detectChanges = context.ChangeTracker.AutoDetectChangesEnabled; diff --git a/test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs index 78346fddaf9..e6a393325dc 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/ChangeDetectorTest.cs @@ -909,8 +909,13 @@ public void Detects_adding_to_collection_navigation() Assert.Equal(new[] { product3 }, testListener.CollectionChange.Item3); Assert.Empty(testListener.CollectionChange.Item4); + var productEntry = stateManager.GetOrCreateEntry(product3); + Assert.Same(productEntry, testListener.ReferenceChange.Item1); + Assert.Same(productEntry.EntityType.FindNavigation("Category"), testListener.ReferenceChange.Item2); + Assert.Null(testListener.ReferenceChange.Item3); + Assert.Equal(category, testListener.ReferenceChange.Item4); + Assert.Null(testListener.KeyChange); - Assert.Null(testListener.ReferenceChange); } [ConditionalFact] @@ -1076,8 +1081,13 @@ public void Skips_detecting_changes_to_notifying_collections() Assert.Equal(new[] { product3 }, testListener.CollectionChange.Item3); Assert.Empty(testListener.CollectionChange.Item4); + var productEntry = stateManager.GetOrCreateEntry(product3); + Assert.Same(productEntry, testListener.ReferenceChange.Item1); + Assert.Same(productEntry.EntityType.FindNavigation("Category"), testListener.ReferenceChange.Item2); + Assert.Null(testListener.ReferenceChange.Item3); + Assert.Equal(category, testListener.ReferenceChange.Item4); + Assert.Null(testListener.KeyChange); - Assert.Null(testListener.ReferenceChange); } [ConditionalFact] @@ -1548,7 +1558,12 @@ public void Handles_notification_of_adding_to_collection_navigation() Assert.Equal(new[] { product3 }, testListener.CollectionChange.Item3); Assert.Empty(testListener.CollectionChange.Item4); - Assert.Null(testListener.ReferenceChange); + var productEntry = stateManager.GetOrCreateEntry(product3); + Assert.Same(productEntry, testListener.ReferenceChange.Item1); + Assert.Same(productEntry.EntityType.FindNavigation("Category"), testListener.ReferenceChange.Item2); + Assert.Null(testListener.ReferenceChange.Item3); + Assert.Equal(category, testListener.ReferenceChange.Item4); + Assert.Null(testListener.KeyChange); } @@ -2112,8 +2127,8 @@ public override void AttachGraph( private class TestRelationshipListener : NavigationFixer { - public TestRelationshipListener(IChangeDetector changeDetector, IEntityGraphAttacher attacher) - : base(changeDetector, attacher) + public TestRelationshipListener(IEntityGraphAttacher attacher) + : base(attacher) { } diff --git a/test/EFCore.Tests/ChangeTracking/Internal/FixupTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/FixupTest.cs index ab7a657bdee..ff2649a25bd 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/FixupTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/FixupTest.cs @@ -2883,8 +2883,11 @@ public void SetProduct(Product product) private sealed class FixupContext : DbContext { - public FixupContext() + private readonly string _databaseName; + + public FixupContext(string databaseName = null) { + _databaseName = databaseName; ChangeTracker.AutoDetectChangesEnabled = false; } @@ -2959,12 +2962,14 @@ protected internal override void OnModelCreating(ModelBuilder modelBuilder) .HasOne() .WithOne() .HasForeignKey(e => e.ParentId); + + modelBuilder.Entity(); } protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) => optionsBuilder .UseInternalServiceProvider(InMemoryFixture.DefaultServiceProvider) - .UseInMemoryDatabase(nameof(FixupContext)); + .UseInMemoryDatabase(_databaseName ?? nameof(FixupContext)); } private void AssertFixup(DbContext context, Action asserts) @@ -3067,7 +3072,7 @@ protected internal override void OnConfiguring(DbContextOptionsBuilder options) } [ConditionalFact] // Issue #21949 - public void Detatching_principal_tracks_unreferenced_foreign_keys() + public void Detaching_principal_tracks_unreferenced_foreign_keys() { using var context = new DetachingContext(); @@ -3089,6 +3094,7 @@ public void Detatching_principal_tracks_unreferenced_foreign_keys() Assert.Equal(EntityState.Added, context.Entry(dependent).State); principal.Id = 0; // So it re-adds + dependent.Principal = principal; context.Add(principal); Assert.NotEqual(principalId, principal.Id); @@ -3099,4 +3105,478 @@ public void Detatching_principal_tracks_unreferenced_foreign_keys() Assert.Equal(EntityState.Added, context.Entry(principal).State); Assert.Equal(EntityState.Added, context.Entry(dependent).State); } + + public class FixupBlog + { + public int Id { get; set; } + public string Name { get; set; } + + public FixupSite FixupSite { get; set; } + public List Posts { get; } = new(); + } + + public class FixupSite + { + public int Id { get; set; } + + public int? FixupBlogId { get; set; } + public FixupBlog FixupBlog { get; set; } + } + + public class FixupPost + { + public int Id { get; set; } + public string Title { get; set; } + public string Content { get; set; } + + public int? FixupBlogId { get; set; } + public FixupBlog FixupBlog { get; set; } + public List Tags { get; } = new(); + } + + public class FixupTag + { + public int Id { get; set; } + public string Content { get; set; } + + public List Posts { get; } = new(); + } + + [ConditionalFact] + public void Detaching_required_one_to_many_dependent_does_not_clear_navigation_to_deleted_principal() + { + var databaseName = Guid.NewGuid().ToString(); + using (var context = new FixupContext(databaseName)) + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + var principal = context.Add(new Category()).Entity; + principal.AddProduct(new Product()); + context.ChangeTracker.DetectChanges(); + context.SaveChanges(); + } + + using (var context = new FixupContext(databaseName)) + { + var principal = context.Set().Include(e => e.Products).Single(); + var dependent = principal.Products.First(); + + context.Remove(principal); + + Assert.Same(dependent, principal.Products.FirstOrDefault()); + Assert.Same(principal, dependent.Category); + Assert.Equal(principal.Id, dependent.CategoryId); + Assert.Equal(EntityState.Deleted, context.Entry(principal).State); + Assert.Equal(EntityState.Deleted, context.Entry(dependent).State); + + context.Entry(dependent).State = EntityState.Detached; + + Assert.Same(dependent, principal.Products.FirstOrDefault()); + Assert.Same(principal, dependent.Category); + Assert.Equal(principal.Id, dependent.CategoryId); + Assert.Equal(EntityState.Deleted, context.Entry(principal).State); + Assert.Equal(EntityState.Detached, context.Entry(dependent).State); + } + } + + [ConditionalFact] + public void Detaching_required_one_to_many_principal_does_not_clear_navigation_to_deleted_dependent() + { + var databaseName = Guid.NewGuid().ToString(); + using (var context = new FixupContext(databaseName)) + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + var principal = context.Add(new Category()).Entity; + principal.AddProduct(new Product()); + context.ChangeTracker.DetectChanges(); + context.SaveChanges(); + } + + using (var context = new FixupContext(databaseName)) + { + var principal = context.Set().Include(e => e.Products).Single(); + var dependent = principal.Products.First(); + + context.Remove(dependent); + + Assert.Same(dependent, principal.Products.FirstOrDefault()); + Assert.Same(principal, dependent.Category); + Assert.Equal(principal.Id, dependent.CategoryId); + Assert.Equal(EntityState.Unchanged, context.Entry(principal).State); + Assert.Equal(EntityState.Deleted, context.Entry(dependent).State); + + context.Entry(principal).State = EntityState.Detached; + + Assert.Same(dependent, principal.Products.FirstOrDefault()); + Assert.Same(principal, dependent.Category); + Assert.Equal(principal.Id, dependent.CategoryId); + Assert.Equal(EntityState.Detached, context.Entry(principal).State); + Assert.Equal(EntityState.Deleted, context.Entry(dependent).State); } + } + + [ConditionalFact] + public void Detaching_required_one_to_one_dependent_does_not_clear_navigation_to_deleted_principal() + { + var databaseName = Guid.NewGuid().ToString(); + using (var context = new FixupContext(databaseName)) + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + var principal = context.Add(new Parent(1)).Entity; + principal.SetChild(new Child(2, 1)); + context.ChangeTracker.DetectChanges(); + context.SaveChanges(); + } + + using (var context = new FixupContext(databaseName)) + { + var principal = context.Set().Include(e => e.Child).Single(); + var dependent = principal.Child; + + context.Remove(principal); + + Assert.Same(dependent, principal.Child); + Assert.Same(principal, dependent.Parent); + Assert.Equal(principal.Id, dependent.ParentId); + Assert.Equal(EntityState.Deleted, context.Entry(principal).State); + Assert.Equal(EntityState.Deleted, context.Entry(dependent).State); + + context.Entry(dependent).State = EntityState.Detached; + + Assert.Same(dependent, principal.Child); + Assert.Same(principal, dependent.Parent); + Assert.Equal(principal.Id, dependent.ParentId); + Assert.Equal(EntityState.Deleted, context.Entry(principal).State); + Assert.Equal(EntityState.Detached, context.Entry(dependent).State); + } + } + + [ConditionalFact] + public void Detaching_required_one_to_one_principal_does_not_clear_navigation_to_deleted_dependent() + { + var databaseName = Guid.NewGuid().ToString(); + using (var context = new FixupContext(databaseName)) + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + var principal = context.Add(new Parent(1)).Entity; + principal.SetChild(new Child(2, 1)); + context.ChangeTracker.DetectChanges(); + context.SaveChanges(); + } + + using (var context = new FixupContext(databaseName)) + { + var principal = context.Set().Include(e => e.Child).Single(); + var dependent = principal.Child; + + context.Remove(dependent); + + Assert.Same(dependent, principal.Child); + Assert.Same(principal, dependent.Parent); + Assert.Equal(principal.Id, dependent.ParentId); + Assert.Equal(EntityState.Unchanged, context.Entry(principal).State); + Assert.Equal(EntityState.Deleted, context.Entry(dependent).State); + + context.Entry(principal).State = EntityState.Detached; + + Assert.Same(dependent, principal.Child); + Assert.Same(principal, dependent.Parent); + Assert.Equal(principal.Id, dependent.ParentId); + Assert.Equal(EntityState.Detached, context.Entry(principal).State); + Assert.Equal(EntityState.Deleted, context.Entry(dependent).State); + } + } + + [ConditionalFact] + public void Detaching_optional_one_to_many_dependent_does_not_clear_navigation_to_deleted_principal() + { + var databaseName = Guid.NewGuid().ToString(); + using (var context = new FixupContext(databaseName)) + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + context.Add( + new FixupBlog + { + FixupSite = new(), + Posts = {new() { Tags = { new() }}} + }); + context.SaveChanges(); + } + + using (var context = new FixupContext(databaseName)) + { + var principal = context.Set().Include(e => e.Posts).Single(); + var dependent = principal.Posts.First(); + + context.ChangeTracker.CascadeDeleteTiming = CascadeTiming.OnSaveChanges; + context.Remove(principal); + + Assert.Same(dependent, principal.Posts.FirstOrDefault()); + Assert.Same(principal, dependent.FixupBlog); + Assert.Equal(principal.Id, dependent.FixupBlogId); + Assert.Equal(EntityState.Deleted, context.Entry(principal).State); + Assert.Equal(EntityState.Unchanged, context.Entry(dependent).State); + + context.Entry(dependent).State = EntityState.Detached; + + Assert.Same(dependent, principal.Posts.FirstOrDefault()); + Assert.Same(principal, dependent.FixupBlog); + Assert.Equal(principal.Id, dependent.FixupBlogId); + Assert.Equal(EntityState.Deleted, context.Entry(principal).State); + Assert.Equal(EntityState.Detached, context.Entry(dependent).State); + } + } + + [ConditionalFact] + public void Detaching_optional_one_to_many_dependent_does_not_unclear_navigation_fixup_to_deleted_principal() + { + var databaseName = Guid.NewGuid().ToString(); + using (var context = new FixupContext(databaseName)) + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + context.Add( + new FixupBlog + { + FixupSite = new(), + Posts = {new() { Tags = { new() }}} + }); + context.SaveChanges(); + } + + using (var context = new FixupContext(databaseName)) + { + var principal = context.Set().Include(e => e.Posts).Single(); + var dependent = principal.Posts.First(); + + context.Remove(principal); + + Assert.Same(dependent, principal.Posts.FirstOrDefault()); + Assert.Null(dependent.FixupBlog); + Assert.Null(dependent.FixupBlogId); + Assert.Equal(EntityState.Deleted, context.Entry(principal).State); + Assert.Equal(EntityState.Modified, context.Entry(dependent).State); + + context.Entry(dependent).State = EntityState.Detached; + + Assert.Same(dependent, principal.Posts.FirstOrDefault()); + Assert.Null(dependent.FixupBlog); + Assert.Null(dependent.FixupBlogId); + Assert.Equal(EntityState.Deleted, context.Entry(principal).State); + Assert.Equal(EntityState.Detached, context.Entry(dependent).State); + } + } + + [ConditionalFact] + public void Detaching_optional_one_to_many_principal_does_not_clear_navigation_to_deleted_dependent() + { + var databaseName = Guid.NewGuid().ToString(); + using (var context = new FixupContext(databaseName)) + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + context.Add( + new FixupBlog + { + FixupSite = new(), + Posts = {new() { Tags = { new() }}} + }); + context.SaveChanges(); + } + + using (var context = new FixupContext(databaseName)) + { + var principal = context.Set().Include(e => e.Posts).Single(); + var dependent = principal.Posts.First(); + + context.Remove(dependent); + + Assert.Same(dependent, principal.Posts.FirstOrDefault()); + Assert.Same(principal, dependent.FixupBlog); + Assert.Equal(principal.Id, dependent.FixupBlogId); + Assert.Equal(EntityState.Unchanged, context.Entry(principal).State); + Assert.Equal(EntityState.Deleted, context.Entry(dependent).State); + + context.Entry(principal).State = EntityState.Detached; + + Assert.Same(dependent, principal.Posts.FirstOrDefault()); + Assert.Same(principal, dependent.FixupBlog); + Assert.Equal(principal.Id, dependent.FixupBlogId); + Assert.Equal(EntityState.Detached, context.Entry(principal).State); + Assert.Equal(EntityState.Deleted, context.Entry(dependent).State); + } + } + + [ConditionalFact] + public void Detaching_optional_one_to_one_dependent_does_not_clear_navigation_to_deleted_principal() + { + var databaseName = Guid.NewGuid().ToString(); + using (var context = new FixupContext(databaseName)) + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + context.Add( + new FixupBlog + { + FixupSite = new(), + Posts = {new() { Tags = { new() }}} + }); + context.SaveChanges(); + } + + using (var context = new FixupContext(databaseName)) + { + var principal = context.Set().Include(e => e.FixupSite).Single(); + var dependent = principal.FixupSite; + + context.ChangeTracker.CascadeDeleteTiming = CascadeTiming.OnSaveChanges; + context.Remove(principal); + + Assert.Same(dependent, principal.FixupSite); + Assert.Same(principal, dependent.FixupBlog); + Assert.Equal(principal.Id, dependent.FixupBlogId); + Assert.Equal(EntityState.Deleted, context.Entry(principal).State); + Assert.Equal(EntityState.Unchanged, context.Entry(dependent).State); + + context.Entry(dependent).State = EntityState.Detached; + + Assert.Same(dependent, principal.FixupSite); + Assert.Same(principal, dependent.FixupBlog); + Assert.Equal(principal.Id, dependent.FixupBlogId); + Assert.Equal(EntityState.Deleted, context.Entry(principal).State); + Assert.Equal(EntityState.Detached, context.Entry(dependent).State); + } + } + + [ConditionalFact] + public void Detaching_optional_one_to_one_dependent_does_not_unclear_navigation_fixup_to_deleted_principal() + { + var databaseName = Guid.NewGuid().ToString(); + using (var context = new FixupContext(databaseName)) + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + context.Add( + new FixupBlog + { + FixupSite = new(), + Posts = {new() { Tags = { new() }}} + }); + context.SaveChanges(); + } + + using (var context = new FixupContext(databaseName)) + { + var principal = context.Set().Include(e => e.FixupSite).Single(); + var dependent = principal.FixupSite; + + context.Remove(principal); + + Assert.Same(dependent, principal.FixupSite); + Assert.Null(dependent.FixupBlog); + Assert.Null(dependent.FixupBlogId); + Assert.Equal(EntityState.Deleted, context.Entry(principal).State); + Assert.Equal(EntityState.Modified, context.Entry(dependent).State); + + context.Entry(dependent).State = EntityState.Detached; + + Assert.Same(dependent, principal.FixupSite); + Assert.Null(dependent.FixupBlog); + Assert.Null(dependent.FixupBlogId); + Assert.Equal(EntityState.Deleted, context.Entry(principal).State); + Assert.Equal(EntityState.Detached, context.Entry(dependent).State); + } + } + + [ConditionalFact] + public void Detaching_optional_one_to_one_principal_does_not_clear_navigation_to_deleted_dependent() + { + var databaseName = Guid.NewGuid().ToString(); + using (var context = new FixupContext(databaseName)) + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + context.Add( + new FixupBlog + { + FixupSite = new(), + Posts = {new() { Tags = { new() }}} + }); + context.SaveChanges(); + } + + using (var context = new FixupContext(databaseName)) + { + var principal = context.Set().Include(e => e.FixupSite).Single(); + var dependent = principal.FixupSite; + + context.Remove(dependent); + + Assert.Same(dependent, principal.FixupSite); + Assert.Same(principal, dependent.FixupBlog); + Assert.Equal(principal.Id, dependent.FixupBlogId); + Assert.Equal(EntityState.Unchanged, context.Entry(principal).State); + Assert.Equal(EntityState.Deleted, context.Entry(dependent).State); + + context.Entry(principal).State = EntityState.Detached; + + Assert.Same(dependent, principal.FixupSite); + Assert.Same(principal, dependent.FixupBlog); + Assert.Equal(principal.Id, dependent.FixupBlogId); + Assert.Equal(EntityState.Detached, context.Entry(principal).State); + Assert.Equal(EntityState.Deleted, context.Entry(dependent).State); + } + } + + [ConditionalFact] + public void Detaching_other_side_of_deleted_many_to_many_does_not_clear_navigation() + { + var databaseName = Guid.NewGuid().ToString(); + using (var context = new FixupContext(databaseName)) + { + context.Database.EnsureDeleted(); + context.Database.EnsureCreated(); + + context.Add( + new FixupBlog + { + FixupSite = new(), + Posts = {new() { Tags = { new() }}} + }); + context.SaveChanges(); + } + + using (var context = new FixupContext(databaseName)) + { + var post = context.Set().Include(e => e.Tags).Single(); + var tag = post.Tags.Single(); + + context.Remove(tag); + + Assert.Same(tag, post.Tags.FirstOrDefault()); + Assert.Same(post, tag.Posts.FirstOrDefault()); + Assert.Equal(EntityState.Deleted, context.Entry(tag).State); + Assert.Equal(EntityState.Unchanged, context.Entry(post).State); + + context.Entry(post).State = EntityState.Detached; + + Assert.Same(tag, post.Tags.FirstOrDefault()); + Assert.Same(post, tag.Posts.FirstOrDefault()); + Assert.Equal(EntityState.Deleted, context.Entry(tag).State); + Assert.Equal(EntityState.Detached, context.Entry(post).State); + } + } } diff --git a/test/EFCore.Tests/ChangeTracking/Internal/InternalEntryEntrySubscriberTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/InternalEntryEntrySubscriberTest.cs index 52b4646f100..c367c74e118 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/InternalEntryEntrySubscriberTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/InternalEntryEntrySubscriberTest.cs @@ -504,15 +504,14 @@ private class TestNavigationListener : INavigationFixer public List, IEnumerable>> CollectionChanged { get; } = new(); - public void BeginAttachGraph() - { - } + public bool BeginDelayedFixup() + => false; - public void CompleteAttachGraph() + public void CompleteDelayedFixup() { } - public void AbortAttachGraph() + public void AbortDelayedFixup() { } diff --git a/test/EFCore.Tests/ChangeTracking/Internal/StateManagerTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/StateManagerTest.cs index e991125a4f5..c98fbc02c3b 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/StateManagerTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/StateManagerTest.cs @@ -627,15 +627,14 @@ private class TestListener : INavigationFixer public EntityState ChangingState; public EntityState ChangedState; - public void BeginAttachGraph() - { - } + public bool BeginDelayedFixup() + => false; - public void CompleteAttachGraph() + public void CompleteDelayedFixup() { } - public void AbortAttachGraph() + public void AbortDelayedFixup() { } diff --git a/test/EFCore.Tests/DbContextServicesTest.cs b/test/EFCore.Tests/DbContextServicesTest.cs index 63760516649..aee6e22e5c8 100644 --- a/test/EFCore.Tests/DbContextServicesTest.cs +++ b/test/EFCore.Tests/DbContextServicesTest.cs @@ -386,15 +386,14 @@ public void StateChanging(InternalEntityEntry entry, EntityState newState) public void StateChanged(InternalEntityEntry entry, EntityState oldState, bool fromQuery) => throw new NotImplementedException(); - public void BeginAttachGraph() - { - } + public bool BeginDelayedFixup() + => false; - public void CompleteAttachGraph() + public void CompleteDelayedFixup() { } - public void AbortAttachGraph() + public void AbortDelayedFixup() { }