From 6b9112aaccb5459d48f37453841b6f0357271cfe Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Tue, 8 Dec 2020 10:47:26 -0800 Subject: [PATCH 1/4] InMemory: Decompose join condition which uses equals for comparison Resolves #23593 --- ...yableMethodTranslatingExpressionVisitor.cs | 14 +++ .../Query/QueryBugsInMemoryTest.cs | 92 +++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/src/EFCore.InMemory/Query/Internal/InMemoryQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.InMemory/Query/Internal/InMemoryQueryableMethodTranslatingExpressionVisitor.cs index b62f0acce1a..f4e09aabc2c 100644 --- a/src/EFCore.InMemory/Query/Internal/InMemoryQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.InMemory/Query/Internal/InMemoryQueryableMethodTranslatingExpressionVisitor.cs @@ -660,6 +660,20 @@ private static bool ProcessJoinCondition( } } + if (!(AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue23593", out var enabled) + && enabled) + && joinCondition is MethodCallExpression methodCallExpression + && methodCallExpression.Method.IsStatic + && methodCallExpression.Method.DeclaringType == typeof(object) + && methodCallExpression.Method.Name == nameof(object.Equals) + && methodCallExpression.Arguments.Count == 2) + { + leftExpressions.Add(methodCallExpression.Arguments[0]); + rightExpressions.Add(methodCallExpression.Arguments[1]); + + return true; + } + return false; } diff --git a/test/EFCore.InMemory.FunctionalTests/Query/QueryBugsInMemoryTest.cs b/test/EFCore.InMemory.FunctionalTests/Query/QueryBugsInMemoryTest.cs index b9237457dea..0d715aa85db 100644 --- a/test/EFCore.InMemory.FunctionalTests/Query/QueryBugsInMemoryTest.cs +++ b/test/EFCore.InMemory.FunctionalTests/Query/QueryBugsInMemoryTest.cs @@ -1160,6 +1160,98 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) #endregion + #region Issue23593 + + [ConditionalFact] + public virtual void Join_with_enum_as_key_selector() + { + using (CreateScratch(Seed23593, "23593")) + { + using var context = new MyContext23593(); + + var query = from sm in context.StatusMaps + join sme in context.StatusMapEvents on sm.Id equals sme.Id + select sm; + + var result = Assert.Single(query); + Assert.Equal(StatusMapCode23593.Two, result.Id); + } + } + + [ConditionalFact] + public virtual void Join_with_enum_inside_anonymous_type_as_key_selector() + { + using (CreateScratch(Seed23593, "23593")) + { + using var context = new MyContext23593(); + + var query = from sm in context.StatusMaps + join sme in context.StatusMapEvents on new { sm.Id } equals new { sme.Id } + select sm; + + var result = Assert.Single(query); + Assert.Equal(StatusMapCode23593.Two, result.Id); + } + } + + [ConditionalFact] + public virtual void Join_with_enum_inside_anonymous_type_with_other_property_as_key_selector() + { + using (CreateScratch(Seed23593, "23593")) + { + using var context = new MyContext23593(); + + var query = from sm in context.StatusMaps + join sme in context.StatusMapEvents on new { sm.Id, A = 1 } equals new { sme.Id, A = 1 } + select sm; + + var result = Assert.Single(query); + Assert.Equal(StatusMapCode23593.Two, result.Id); + } + } + + private static void Seed23593(MyContext23593 context) + { + context.Add(new StatusMap23593 { Id = StatusMapCode23593.One }); + context.Add(new StatusMap23593 { Id = StatusMapCode23593.Two }); + context.Add(new StatusMapEvent23593 { Id = StatusMapCode23593.Two }); + + context.SaveChanges(); + } + + private enum StatusMapCode23593 + { + One, + Two, + Three, + Four + } + + private class StatusMap23593 + { + public StatusMapCode23593 Id { get; set; } + } + private class StatusMapEvent23593 + { + public StatusMapCode23593 Id { get; set; } + } + + + private class MyContext23593 : DbContext + { + public DbSet StatusMaps { get; set; } + public DbSet StatusMapEvents { get; set; } + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + { + optionsBuilder + .UseInternalServiceProvider(InMemoryFixture.DefaultServiceProvider) + .UseInMemoryDatabase("23593"); + } + } + + #endregion + #region SharedHelper private static InMemoryTestStore CreateScratch(Action seed, string databaseName) From f43a1f577a5704cf181880545fd7ad7ce72fba5c Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Tue, 5 Jan 2021 07:24:59 -0800 Subject: [PATCH 2/4] [5.0.3] Set relationship snapshot correctly for skip navigations Fixes #23787 --- .../Internal/NavigationFixer.cs | 2 + .../ManyToManyTrackingTestBase.cs | 244 +++++++++++++++--- 2 files changed, 214 insertions(+), 32 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs index 2de8e7c293a..e7dd8f1ad16 100644 --- a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs +++ b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs @@ -831,6 +831,8 @@ private void InitialFixup( AddToCollection(otherEntry, skipNavigation.Inverse, entry, fromQuery); } + + entry.AddToCollectionSnapshot(skipNavigation, otherEntity); } } } diff --git a/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs b/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs index e9560a90d91..1314f9dda25 100644 --- a/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs +++ b/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs @@ -8,6 +8,8 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Query; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel; @@ -113,7 +115,7 @@ await ExecuteWithStrategyInTransactionAsync( ValidateFixup(context, leftEntities, rightEntities); }); - static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) { Assert.Equal(11, context.ChangeTracker.Entries().Count()); Assert.Equal(3, context.ChangeTracker.Entries().Count()); @@ -128,7 +130,8 @@ static void ValidateFixup(DbContext context, IList leftEntit Assert.Single(rightEntities[1].CompositeKeySkipFull); Assert.Single(rightEntities[2].CompositeKeySkipFull); - foreach (var joinEntity in context.ChangeTracker.Entries().Select(e => e.Entity).ToList()) + var joinEntities = context.ChangeTracker.Entries().Select(e => e.Entity).ToList(); + foreach (var joinEntity in joinEntities) { Assert.Equal(joinEntity.Composite.Key1, joinEntity.CompositeId1); Assert.Equal(joinEntity.Composite.Key2, joinEntity.CompositeId2); @@ -138,6 +141,10 @@ static void ValidateFixup(DbContext context, IList leftEntit Assert.Contains(joinEntity, joinEntity.Composite.JoinLeafFull); Assert.Contains(joinEntity, joinEntity.Leaf.JoinCompositeKeyFull); } + + VerifyRelationshipSnapshots(context, joinEntities); + VerifyRelationshipSnapshots(context, leftEntities); + VerifyRelationshipSnapshots(context, rightEntities); } } @@ -242,7 +249,7 @@ public virtual void Can_update_many_to_many_composite_with_navs() ValidateFixup(context, leftEntities, rightEntities, 24, 8, 39 - 4); }); - static void ValidateFixup( + void ValidateFixup( DbContext context, List leftEntities, List rightEntities, @@ -272,7 +279,8 @@ static void ValidateFixup( Assert.DoesNotContain(rightEntities[2].CompositeKeySkipFull, e => e.Key2 == "8_1"); Assert.Contains(rightEntities[2].CompositeKeySkipFull, e => e.Key2 == "7714"); - foreach (var joinEntry in context.ChangeTracker.Entries().ToList()) + var joinEntries = context.ChangeTracker.Entries().ToList(); + foreach (var joinEntry in joinEntries) { var joinEntity = joinEntry.Entity; @@ -288,6 +296,10 @@ static void ValidateFixup( var allLeft = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Key2).ToList(); var allRight = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + VerifyRelationshipSnapshots(context, joinEntries.Select(e => e.Entity)); + VerifyRelationshipSnapshots(context, allLeft); + VerifyRelationshipSnapshots(context, allRight); + var count = 0; foreach (var left in allLeft) { @@ -565,7 +577,7 @@ await ExecuteWithStrategyInTransactionAsync( ValidateFixup(context, leftEntities, rightEntities); }); - static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) { Assert.Equal(11, context.ChangeTracker.Entries().Count()); Assert.Equal(3, context.ChangeTracker.Entries().Count()); @@ -579,6 +591,9 @@ static void ValidateFixup(DbContext context, IList leftEntit Assert.Equal(3, rightEntities[0].CompositeKeySkipShared.Count); Assert.Single(rightEntities[1].CompositeKeySkipShared); Assert.Single(rightEntities[2].CompositeKeySkipShared); + + VerifyRelationshipSnapshots(context, leftEntities); + VerifyRelationshipSnapshots(context, rightEntities); } } @@ -723,6 +738,9 @@ void ValidateFixup( var allLeft = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Key2).ToList(); var allRight = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + VerifyRelationshipSnapshots(context, allLeft); + VerifyRelationshipSnapshots(context, allRight); + var count = 0; foreach (var left in allLeft) { @@ -935,7 +953,7 @@ await ExecuteWithStrategyInTransactionAsync( context.AddRange(rightEntities[0], rightEntities[1], rightEntities[2]); } - ValidateFixup(context, leftEntities, rightEntities); + ValidateFixup(context, leftEntities, rightEntities, postSave: false); if (async) { @@ -946,7 +964,7 @@ await ExecuteWithStrategyInTransactionAsync( context.SaveChanges(); } - ValidateFixup(context, leftEntities, rightEntities); + ValidateFixup(context, leftEntities, rightEntities, postSave: true); keys = leftEntities.Select(e => e.Key2).ToList(); }, @@ -962,10 +980,10 @@ await ExecuteWithStrategyInTransactionAsync( var rightEntities = context.ChangeTracker.Entries() .Select(e => e.Entity).OrderBy(e => e.Name).ToList(); - ValidateFixup(context, leftEntities, rightEntities); + ValidateFixup(context, leftEntities, rightEntities, postSave: true); }); - static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities, bool postSave) { var entries = context.ChangeTracker.Entries(); Assert.Equal(11, entries.Count()); @@ -981,7 +999,8 @@ static void ValidateFixup(DbContext context, IList leftEntit Assert.Single(rightEntities[1].CompositeKeySkipFull); Assert.Single(rightEntities[2].CompositeKeySkipFull); - foreach (var joinEntity in context.ChangeTracker.Entries().Select(e => e.Entity).ToList()) + var joinEntities = context.ChangeTracker.Entries().Select(e => e.Entity).ToList(); + foreach (var joinEntity in joinEntities) { Assert.Equal(joinEntity.Composite.Key1, joinEntity.CompositeId1); Assert.Equal(joinEntity.Composite.Key2, joinEntity.CompositeId2); @@ -991,6 +1010,16 @@ static void ValidateFixup(DbContext context, IList leftEntit Assert.Contains(joinEntity, joinEntity.Composite.JoinThreeFull); Assert.Contains(joinEntity, joinEntity.Three.JoinCompositeKeyFull); } + + // Issue #23814 + //VerifyRelationshipSnapshots(context, joinEntities); + //VerifyRelationshipSnapshots(context, leftEntities); + //VerifyRelationshipSnapshots(context, rightEntities); + + foreach (var entry in context.ChangeTracker.Entries()) + { + Assert.Equal(postSave ? EntityState.Unchanged : EntityState.Added, entry.State); + } } } @@ -1137,7 +1166,8 @@ void ValidateFixup( Assert.DoesNotContain(rightEntities[2].CompositeKeySkipFull, e => e.Key2 == "6_1"); Assert.Contains(rightEntities[2].CompositeKeySkipFull, e => e.Key2 == "Z7714"); - foreach (var joinEntry in context.ChangeTracker.Entries().ToList()) + var joinEntries = context.ChangeTracker.Entries().ToList(); + foreach (var joinEntry in joinEntries) { var joinEntity = joinEntry.Entity; @@ -1153,6 +1183,11 @@ void ValidateFixup( var allLeft = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Key2).ToList(); var allRight = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + // Issue #23814 + // VerifyRelationshipSnapshots(context, joinEntries.Select(e => e.Entity)); + // VerifyRelationshipSnapshots(context, allLeft); + // VerifyRelationshipSnapshots(context, allRight); + var count = 0; foreach (var left in allLeft) { @@ -1411,7 +1446,7 @@ await ExecuteWithStrategyInTransactionAsync( ValidateFixup(context, leftEntities, rightEntities); }); - static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) { Assert.Equal(11, context.ChangeTracker.Entries().Count()); Assert.Equal(6, context.ChangeTracker.Entries().Count()); @@ -1424,6 +1459,9 @@ static void ValidateFixup(DbContext context, IList leftEntities, ILis Assert.Equal(3, rightEntities[0].SelfSkipSharedRight.Count); Assert.Single(rightEntities[1].SelfSkipSharedRight); Assert.Single(rightEntities[2].SelfSkipSharedRight); + + VerifyRelationshipSnapshots(context, leftEntities); + VerifyRelationshipSnapshots(context, rightEntities); } } @@ -1561,6 +1599,9 @@ void ValidateFixup( var allLeft = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); var allRight = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + VerifyRelationshipSnapshots(context, allLeft); + VerifyRelationshipSnapshots(context, allRight); + var joins = 0; foreach (var left in allLeft) { @@ -1658,7 +1699,7 @@ await ExecuteWithStrategyInTransactionAsync( ValidateFixup(context, leftEntities, rightEntities); }); - static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) { Assert.Equal(11, context.ChangeTracker.Entries().Count()); Assert.Equal(3, context.ChangeTracker.Entries().Count()); @@ -1673,13 +1714,19 @@ static void ValidateFixup(DbContext context, IList leftEntities, ILis Assert.Single(rightEntities[1].TwoSkipFull); Assert.Single(rightEntities[2].TwoSkipFull); - foreach (var joinEntity in context.ChangeTracker.Entries().Select(e => e.Entity).ToList()) + var joinEntities = context.ChangeTracker.Entries().Select(e => e.Entity).ToList(); + foreach (var joinEntity in joinEntities) { 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); } + + VerifyRelationshipSnapshots(context, joinEntities); + VerifyRelationshipSnapshots(context, leftEntities); + VerifyRelationshipSnapshots(context, rightEntities); + } } @@ -1776,7 +1823,7 @@ public virtual void Can_update_many_to_many_with_navs() ValidateFixup(context, leftEntities, rightEntities, 24, 24, 60 - 4); }); - static void ValidateFixup( + void ValidateFixup( DbContext context, List leftEntities, List rightEntities, @@ -1806,7 +1853,8 @@ static void ValidateFixup( Assert.DoesNotContain(rightEntities[2].TwoSkipFull, e => e.Name == "EntityTwo 3"); Assert.Contains(rightEntities[2].TwoSkipFull, e => e.Name == "Z7714"); - foreach (var joinEntry in context.ChangeTracker.Entries().ToList()) + var joinEntries = context.ChangeTracker.Entries().ToList(); + foreach (var joinEntry in joinEntries) { var joinEntity = joinEntry.Entity; Assert.Equal(joinEntity.Two.Id, joinEntity.TwoId); @@ -1818,6 +1866,10 @@ static void ValidateFixup( var allLeft = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); var allRight = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + VerifyRelationshipSnapshots(context, joinEntries.Select(e => e.Entity)); + VerifyRelationshipSnapshots(context, allLeft); + VerifyRelationshipSnapshots(context, allRight); + var count = 0; foreach (var left in allLeft) { @@ -1915,7 +1967,7 @@ await ExecuteWithStrategyInTransactionAsync( ValidateFixup(context, leftEntities, rightEntities); }); - static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) { Assert.Equal(11, context.ChangeTracker.Entries().Count()); Assert.Equal(3, context.ChangeTracker.Entries().Count()); @@ -1929,6 +1981,9 @@ static void ValidateFixup(DbContext context, IList leftEntities, ILis Assert.Equal(3, rightEntities[0].OneSkip.Count); Assert.Single(rightEntities[1].OneSkip); Assert.Single(rightEntities[2].OneSkip); + + VerifyRelationshipSnapshots(context, leftEntities); + VerifyRelationshipSnapshots(context, rightEntities); } } @@ -2025,7 +2080,7 @@ public virtual void Can_update_many_to_many_with_inheritance() ValidateFixup(context, leftEntities, rightEntities, 24, 14, 55 - 4); }); - static void ValidateFixup( + void ValidateFixup( DbContext context, List leftEntities, List rightEntities, @@ -2058,6 +2113,9 @@ static void ValidateFixup( var allLeft = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); var allRight = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + VerifyRelationshipSnapshots(context, allLeft); + VerifyRelationshipSnapshots(context, allRight); + var count = 0; foreach (var left in allLeft) { @@ -2183,7 +2241,8 @@ void ValidateFixup(DbContext context, IList leftEntities, IList().Select(e => e.Entity).ToList()) + var joinEntities = context.ChangeTracker.Entries().Select(e => e.Entity).ToList(); + foreach (var joinEntity in joinEntities) { Assert.Equal(joinEntity.Left.Id, joinEntity.LeftId); Assert.Equal(joinEntity.Right.Id, joinEntity.RightId); @@ -2200,6 +2259,10 @@ void ValidateFixup(DbContext context, IList leftEntities, IList e.Name == "EntityOne 6"); Assert.Contains(rightEntities[4].SelfSkipPayloadLeft, e => context.Entry(e).Property(e => e.Id).CurrentValue == keys[7]); - foreach (var joinEntry in context.ChangeTracker.Entries().ToList()) + var joinEntries = context.ChangeTracker.Entries().ToList(); + foreach (var joinEntry in joinEntries) { var joinEntity = joinEntry.Entity; Assert.Equal(joinEntity.Left.Id, joinEntity.LeftId); @@ -2372,6 +2436,10 @@ void ValidateFixup( var allLeft = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); var allRight = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + VerifyRelationshipSnapshots(context, joinEntries.Select(e => e.Entity)); + VerifyRelationshipSnapshots(context, allLeft); + VerifyRelationshipSnapshots(context, allRight); + var joins = 0; foreach (var left in allLeft) { @@ -2484,6 +2552,9 @@ void ValidateFixup(DbContext context, IList leftEntities, IList context.Entry(context.EntityThrees.Local.Single(e => e.Name == name)).Property(e => e.Id).CurrentValue; - static void ValidateFixup( + void ValidateFixup( ManyToManyContext context, List leftEntities, List rightEntities, @@ -2643,7 +2714,8 @@ static void ValidateFixup( var oneId2 = GetEntityOneId(context, "EntityOne 20"); var threeId2 = GetEntityThreeId(context, "EntityThree 20"); - foreach (var joinEntry in context.ChangeTracker.Entries>().ToList()) + var joinEntries = context.ChangeTracker.Entries>().ToList(); + foreach (var joinEntry in joinEntries) { var joinEntity = joinEntry.Entity; @@ -2665,6 +2737,10 @@ static void ValidateFixup( var allLeft = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); var allRight = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + VerifyRelationshipSnapshots(context, joinEntries.Select(e => e.Entity)); + VerifyRelationshipSnapshots(context, allLeft); + VerifyRelationshipSnapshots(context, allRight); + var count = 0; foreach (var left in allLeft) { @@ -2762,7 +2838,7 @@ await ExecuteWithStrategyInTransactionAsync( ValidateFixup(context, leftEntities, rightEntities); }); - static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) { Assert.Equal(11, context.ChangeTracker.Entries().Count()); Assert.Equal(3, context.ChangeTracker.Entries().Count()); @@ -2776,6 +2852,9 @@ static void ValidateFixup(DbContext context, IList leftEntities, ILis Assert.Equal(3, rightEntities[0].OneSkipShared.Count); Assert.Single(rightEntities[1].OneSkipShared); Assert.Single(rightEntities[2].OneSkipShared); + + VerifyRelationshipSnapshots(context, leftEntities); + VerifyRelationshipSnapshots(context, rightEntities); } } @@ -2880,7 +2959,7 @@ public virtual void Can_update_many_to_many_shared() ValidateFixup(context, leftEntities, rightEntities, 24, 24, 49); }); - static void ValidateFixup( + void ValidateFixup( DbContext context, List leftEntities, List rightEntities, @@ -2913,6 +2992,9 @@ static void ValidateFixup( var allLeft = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); var allRight = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + VerifyRelationshipSnapshots(context, allLeft); + VerifyRelationshipSnapshots(context, allRight); + var count = 0; foreach (var left in allLeft) { @@ -3025,7 +3107,8 @@ void ValidateFixup(DbContext context, IList leftEntities, IList().Select(e => e.Entity).ToList()) + var joinEntities = context.ChangeTracker.Entries().Select(e => e.Entity).ToList(); + foreach (var joinEntity in joinEntities) { Assert.Equal(joinEntity.One.Id, joinEntity.OneId); Assert.Equal(joinEntity.Three.Id, joinEntity.ThreeId); @@ -3037,6 +3120,10 @@ void ValidateFixup(DbContext context, IList leftEntities, IList context.Entry(context.EntityThrees.Local.Single(e => e.Name == name)).Property(e => e.Id).CurrentValue; - static void ValidateFixup( + void ValidateFixup( ManyToManyContext context, List leftEntities, List rightEntities, @@ -3190,7 +3277,8 @@ static void ValidateFixup( var oneId2 = GetEntityOneId(context, "EntityOne 20"); var threeId2 = GetEntityThreeId(context, "EntityThree 20"); - foreach (var joinEntry in context.ChangeTracker.Entries().ToList()) + var joinEntries = context.ChangeTracker.Entries().ToList(); + foreach (var joinEntry in joinEntries) { var joinEntity = joinEntry.Entity; Assert.Equal(joinEntity.One.Id, joinEntity.OneId); @@ -3216,6 +3304,10 @@ static void ValidateFixup( var allLeft = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); var allRight = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + VerifyRelationshipSnapshots(context, joinEntries.Select(e => e.Entity)); + VerifyRelationshipSnapshots(context, allLeft); + VerifyRelationshipSnapshots(context, allRight); + var count = 0; foreach (var left in allLeft) { @@ -3438,7 +3530,7 @@ await ExecuteWithStrategyInTransactionAsync( ValidateFixup(context, leftEntities, rightEntities); }); - static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) { Assert.Equal(11, context.ChangeTracker.Entries().Count()); Assert.Equal(3, context.ChangeTracker.Entries().Count()); @@ -3452,6 +3544,9 @@ static void ValidateFixup(DbContext context, IList leftEntities, ILis Assert.Equal(3, rightEntities[0].OneSkip.Count); Assert.Single(rightEntities[1].OneSkip); Assert.Single(rightEntities[2].OneSkip); + + VerifyRelationshipSnapshots(context, leftEntities); + VerifyRelationshipSnapshots(context, rightEntities); } } @@ -3598,6 +3693,9 @@ void ValidateFixup( var allLeft = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); var allRight = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); + VerifyRelationshipSnapshots(context, allLeft); + VerifyRelationshipSnapshots(context, allRight); + var count = 0; foreach (var left in allLeft) { @@ -3796,7 +3894,7 @@ await ExecuteWithStrategyInTransactionAsync( Assert.Single(rightEntities[2].As); }); - static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) { Assert.Equal(11, context.ChangeTracker.Entries().Count()); Assert.Equal(3, context.ChangeTracker.Entries().Count()); @@ -3810,6 +3908,9 @@ static void ValidateFixup(DbContext context, IList leftEnti Assert.Equal(3, rightEntities[0].As.Count); Assert.Single(rightEntities[1].As); Assert.Single(rightEntities[2].As); + + VerifyRelationshipSnapshots(context, leftEntities); + VerifyRelationshipSnapshots(context, rightEntities); } } @@ -3891,7 +3992,7 @@ await ExecuteWithStrategyInTransactionAsync( Assert.Single(rightEntities[2].Lefts); }); - static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) { Assert.Equal(11, context.ChangeTracker.Entries().Count()); Assert.Equal(3, context.ChangeTracker.Entries().Count()); @@ -3905,6 +4006,9 @@ static void ValidateFixup(DbContext context, IList leftEntiti Assert.Equal(3, rightEntities[0].Lefts.Count); Assert.Single(rightEntities[1].Lefts); Assert.Single(rightEntities[2].Lefts); + + VerifyRelationshipSnapshots(context, leftEntities); + VerifyRelationshipSnapshots(context, rightEntities); } } @@ -4043,7 +4147,7 @@ await ExecuteWithStrategyInTransactionAsync( Assert.Single(rightEntities[2].Lefts); }); - static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) { Assert.Equal(11, context.ChangeTracker.Entries().Count()); Assert.Equal(3, context.ChangeTracker.Entries().Count()); @@ -4057,6 +4161,9 @@ static void ValidateFixup(DbContext context, IList leftEntiti Assert.Equal(3, rightEntities[0].Lefts.Count); Assert.Single(rightEntities[1].Lefts); Assert.Single(rightEntities[2].Lefts); + + VerifyRelationshipSnapshots(context, leftEntities); + VerifyRelationshipSnapshots(context, rightEntities); } } @@ -4125,7 +4232,7 @@ await ExecuteWithStrategyInTransactionAsync( ValidateFixup(context, leftEntities, rightEntities); }); - static void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) + void ValidateFixup(DbContext context, IList leftEntities, IList rightEntities) { Assert.Equal(9, context.ChangeTracker.Entries().Count()); Assert.Equal(3, context.ChangeTracker.Entries().Count()); @@ -4139,6 +4246,9 @@ static void ValidateFixup(DbContext context, IList leftEnti Assert.Single(rightEntities[0].As); Assert.Single(rightEntities[1].As); Assert.Single(rightEntities[2].As); + + VerifyRelationshipSnapshots(context, leftEntities); + VerifyRelationshipSnapshots(context, rightEntities); } } @@ -4441,6 +4551,76 @@ static void ValidateFixup(DbContext context, IList leftEntities, ILis } } + protected void VerifyRelationshipSnapshots(DbContext context, IEnumerable entities) + { + try + { + context.ChangeTracker.AutoDetectChangesEnabled = false; + + foreach (var entity in entities) + { + var entityEntry = context.Entry(entity).GetInfrastructure(); + var entityType = entityEntry.EntityType; + + if (entityEntry.HasRelationshipSnapshot) + { + foreach (var property in entityType.GetForeignKeys().SelectMany(e => e.Properties)) + { + if (property.GetRelationshipIndex() >= 0) + { + Assert.Equal(entityEntry.GetRelationshipSnapshotValue(property), entityEntry[property]); + } + } + + foreach (var navigation in entityType.GetNavigations() + .Concat((IEnumerable)entityType.GetSkipNavigations())) + { + if (navigation.GetRelationshipIndex() >= 0) + { + var snapshot = entityEntry.GetRelationshipSnapshotValue(navigation); + var current = entityEntry[navigation]; + + if (navigation.IsCollection) + { + var currentCollection = ((IEnumerable)current)?.ToList(); + var snapshotCollection = ((IEnumerable)snapshot)?.ToList(); + + if (snapshot == null) + { + Assert.True(current == null || !currentCollection.Any()); + } + else if (current == null) + { + Assert.True(snapshot == null || !snapshotCollection.Any()); + } + else + { + Assert.Equal(snapshotCollection.Count, currentCollection.Count); + + foreach (var related in snapshotCollection) + { + Assert.Contains(currentCollection, c => ReferenceEquals(c, related)); + } + } + } + else + { + Assert.Same(snapshot, current); + } + } + } + } + } + } + finally + { + if (RequiresDetectChanges) + { + context.ChangeTracker.AutoDetectChangesEnabled = true; + } + } + } + private ICollection CreateCollection() => RequiresDetectChanges ? (ICollection)new List() : new ObservableCollection(); From 26c17f7e64771b3920a990c7c947702295a76978 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Tue, 5 Jan 2021 13:53:51 -0800 Subject: [PATCH 3/4] Add quirk --- src/EFCore/ChangeTracking/Internal/NavigationFixer.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs index e7dd8f1ad16..751afbd8fa4 100644 --- a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs +++ b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs @@ -33,6 +33,9 @@ public class NavigationFixer : INavigationFixer private readonly bool _useOldBehaviorFor23659 = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue23659", out var enabled) && enabled; + private readonly bool _useOldBehaviorFor23787 + = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue23787", out var enabled) && enabled; + private readonly IChangeDetector _changeDetector; private readonly IEntityGraphAttacher _attacher; private bool _inFixup; @@ -832,7 +835,10 @@ private void InitialFixup( AddToCollection(otherEntry, skipNavigation.Inverse, entry, fromQuery); } - entry.AddToCollectionSnapshot(skipNavigation, otherEntity); + if (!_useOldBehaviorFor23787) + { + entry.AddToCollectionSnapshot(skipNavigation, otherEntity); + } } } } From 1d1b132409f669fa0a555f93b4a55397cf59ea81 Mon Sep 17 00:00:00 2001 From: Arthur Vickers Date: Thu, 7 Jan 2021 08:01:35 -0800 Subject: [PATCH 4/4] Fixed test issue with not reseting AutoDetectChanges correctly --- .../ManyToManyTrackingTestBase.cs | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs b/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs index 1314f9dda25..6ee121d5ec7 100644 --- a/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs +++ b/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs @@ -1011,10 +1011,9 @@ void ValidateFixup(DbContext context, IList leftEntities, IL Assert.Contains(joinEntity, joinEntity.Three.JoinCompositeKeyFull); } - // Issue #23814 - //VerifyRelationshipSnapshots(context, joinEntities); - //VerifyRelationshipSnapshots(context, leftEntities); - //VerifyRelationshipSnapshots(context, rightEntities); + VerifyRelationshipSnapshots(context, joinEntities); + VerifyRelationshipSnapshots(context, leftEntities); + VerifyRelationshipSnapshots(context, rightEntities); foreach (var entry in context.ChangeTracker.Entries()) { @@ -1183,10 +1182,9 @@ void ValidateFixup( var allLeft = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Key2).ToList(); var allRight = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Name).ToList(); - // Issue #23814 - // VerifyRelationshipSnapshots(context, joinEntries.Select(e => e.Entity)); - // VerifyRelationshipSnapshots(context, allLeft); - // VerifyRelationshipSnapshots(context, allRight); + VerifyRelationshipSnapshots(context, joinEntries.Select(e => e.Entity)); + VerifyRelationshipSnapshots(context, allLeft); + VerifyRelationshipSnapshots(context, allRight); var count = 0; foreach (var left in allLeft) @@ -4553,6 +4551,7 @@ static void ValidateFixup(DbContext context, IList leftEntities, ILis protected void VerifyRelationshipSnapshots(DbContext context, IEnumerable entities) { + var detectChanges = context.ChangeTracker.AutoDetectChangesEnabled; try { context.ChangeTracker.AutoDetectChangesEnabled = false; @@ -4614,10 +4613,7 @@ protected void VerifyRelationshipSnapshots(DbContext context, IEnumerable