From d6fc81a827187fe970cbdd5a7b005ed07d20442e Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Mon, 4 Jan 2021 15:28:50 -0800 Subject: [PATCH] [5.0.3] Set skip navigations in delayed fixup Fixes #23659 If we encounter an unmapped entity during graph discovery of Attach, etc., then that entity is put aside for delayed fixup when it later becomes tracked. In this delayed fixup we were not populating skip navigations. The fix is to do this, just like happens in non-delayed fixup. --- .../Internal/NavigationFixer.cs | 40 +++-- .../ManyToManyTrackingTestBase.cs | 148 ++++++++++++++++++ 2 files changed, 175 insertions(+), 13 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs index a2372cff602..2de8e7c293a 100644 --- a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs +++ b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections; using System.Collections.Generic; using System.Linq; @@ -29,6 +30,9 @@ namespace Microsoft.EntityFrameworkCore.ChangeTracking.Internal /// public class NavigationFixer : INavigationFixer { + private readonly bool _useOldBehaviorFor23659 + = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue23659", out var enabled) && enabled; + private readonly IChangeDetector _changeDetector; private readonly IEntityGraphAttacher _attacher; private bool _inFixup; @@ -663,19 +667,7 @@ private void InitialFixup( } } - foreach (var skipNavigation in foreignKey.GetReferencingSkipNavigations()) - { - var leftEntry = stateManager.FindPrincipal(entry, foreignKey); - if (leftEntry != null) - { - var rightEntry = stateManager.FindPrincipal(entry, skipNavigation.Inverse.ForeignKey); - if (rightEntry != null) - { - AddToCollection(leftEntry, skipNavigation, rightEntry, fromQuery); - AddToCollection(rightEntry, skipNavigation.Inverse, leftEntry, fromQuery); - } - } - } + FixupSkipNavigations(entry, foreignKey, fromQuery); } foreach (var foreignKey in entityType.GetReferencingForeignKeys()) @@ -894,6 +886,28 @@ private void DelayedFixup( else if (referencedEntry.Entity == navigationValue) { FixupToPrincipal(entry, referencedEntry, navigation.ForeignKey, setModified, fromQuery); + + if (!_useOldBehaviorFor23659) + { + FixupSkipNavigations(entry, navigation.ForeignKey, fromQuery); + } + } + } + } + } + + private void FixupSkipNavigations(InternalEntityEntry entry, IForeignKey foreignKey, bool fromQuery) + { + foreach (var skipNavigation in foreignKey.GetReferencingSkipNavigations()) + { + var leftEntry = entry.StateManager.FindPrincipal(entry, foreignKey); + if (leftEntry != null) + { + var rightEntry = entry.StateManager.FindPrincipal(entry, skipNavigation.Inverse.ForeignKey); + if (rightEntry != null) + { + AddToCollection(leftEntry, skipNavigation, rightEntry, fromQuery); + AddToCollection(rightEntry, skipNavigation.Inverse, leftEntry, fromQuery); } } } diff --git a/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs b/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs index aa810e81d30..e9560a90d91 100644 --- a/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs +++ b/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs @@ -4293,6 +4293,154 @@ public virtual void Can_insert_update_delete_proxyable_shared_type_entity_type() }); } + [ConditionalTheory] + [InlineData(false)] + [InlineData(true)] + public virtual async Task Can_insert_many_to_many_with_navs_by_join_entity(bool async) + { + await ExecuteWithStrategyInTransactionAsync( + async context => + { + var leftEntities = new[] + { + context.EntityTwos.CreateInstance( + (e, p) => + { + e.Id = Fixture.UseGeneratedKeys ? 0 : 7711; + e.Name = "Z7711"; + }), + context.EntityTwos.CreateInstance( + (e, p) => + { + e.Id = Fixture.UseGeneratedKeys ? 0 : 7712; + e.Name = "Z7712"; + }), + context.EntityTwos.CreateInstance( + (e, p) => + { + e.Id = Fixture.UseGeneratedKeys ? 0 : 7713; + e.Name = "Z7713"; + }) + }; + var rightEntities = new[] + { + context.EntityThrees.CreateInstance( + (e, p) => + { + e.Id = Fixture.UseGeneratedKeys ? 0 : 7721; + e.Name = "Z7721"; + }), + context.EntityThrees.CreateInstance( + (e, p) => + { + e.Id = Fixture.UseGeneratedKeys ? 0 : 7722; + e.Name = "Z7722"; + }), + context.EntityThrees.CreateInstance( + (e, p) => + { + e.Id = Fixture.UseGeneratedKeys ? 0 : 7723; + e.Name = "Z7723"; + }) + }; + + var joinEntities = new[] + { + context.Set().CreateInstance( + (e, p) => + { + e.Two = leftEntities[0]; + e.Three = rightEntities[0]; + }), + context.Set().CreateInstance( + (e, p) => + { + e.Two = leftEntities[0]; + e.Three = rightEntities[1]; + }), + context.Set().CreateInstance( + (e, p) => + { + e.Two = leftEntities[0]; + e.Three = rightEntities[2]; + }), + context.Set().CreateInstance( + (e, p) => + { + e.Two = leftEntities[1]; + e.Three = rightEntities[0]; + }), + context.Set().CreateInstance( + (e, p) => + { + e.Two = leftEntities[2]; + e.Three = rightEntities[0]; + }) + }; + + if (async) + { + await context.AddRangeAsync(joinEntities); + } + else + { + context.AddRange(joinEntities); + } + + ValidateFixup(context, leftEntities, rightEntities); + + if (async) + { + await context.SaveChangesAsync(); + } + else + { + context.SaveChanges(); + } + + ValidateFixup(context, leftEntities, rightEntities); + }, + async context => + { + var queryable = context.Set() + .Where(e => e.Name.StartsWith("Z")) + .OrderBy(e => e.Name) + .Include(e => e.ThreeSkipFull); + + var results = async ? await queryable.ToListAsync() : queryable.ToList(); + Assert.Equal(3, results.Count); + + var leftEntities = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + var rightEntities = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + + ValidateFixup(context, leftEntities, rightEntities); + }); + + static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + { + Assert.Equal(11, context.ChangeTracker.Entries().Count()); + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(3, context.ChangeTracker.Entries().Count()); + Assert.Equal(5, context.ChangeTracker.Entries().Count()); + + Assert.Equal(3, leftEntities[0].ThreeSkipFull.Count); + Assert.Single(leftEntities[1].ThreeSkipFull); + Assert.Single(leftEntities[2].ThreeSkipFull); + + Assert.Equal(3, rightEntities[0].TwoSkipFull.Count); + Assert.Single(rightEntities[1].TwoSkipFull); + Assert.Single(rightEntities[2].TwoSkipFull); + + foreach (var joinEntity in context.ChangeTracker.Entries().Select(e => e.Entity).ToList()) + { + Assert.Equal(joinEntity.Two.Id, joinEntity.TwoId); + Assert.Equal(joinEntity.Three.Id, joinEntity.ThreeId); + Assert.Contains(joinEntity, joinEntity.Two.JoinThreeFull); + Assert.Contains(joinEntity, joinEntity.Three.JoinTwoFull); + } + } + } + private ICollection CreateCollection() => RequiresDetectChanges ? (ICollection)new List() : new ObservableCollection();