From e1d0a59a90cd8f9cc883f9a3ad4ecdbe9a9c833e Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Wed, 27 Sep 2023 13:56:11 -0700 Subject: [PATCH] Don't remove entry from dependent map if it's not found in identity map. Fixes #31559 --- .../ChangeTracking/Internal/IdentityMap.cs | 8 +- .../Internal/NavigationFixer.cs | 2 + .../SqlServerEndToEndTest.cs | 94 +++++++++++++++---- 3 files changed, 83 insertions(+), 21 deletions(-) diff --git a/src/EFCore/ChangeTracking/Internal/IdentityMap.cs b/src/EFCore/ChangeTracking/Internal/IdentityMap.cs index 660a0d7d36a..830a062ac73 100644 --- a/src/EFCore/ChangeTracking/Internal/IdentityMap.cs +++ b/src/EFCore/ChangeTracking/Internal/IdentityMap.cs @@ -436,11 +436,13 @@ protected virtual void Remove(TKey key, InternalEntityEntry entry) if (otherEntry == null) { - if (_identityMap.TryGetValue(key, out var existingEntry) - && existingEntry == entry) + if (!_identityMap.TryGetValue(key, out var existingEntry) + || existingEntry != entry) { - _identityMap.Remove(key); + return; } + + _identityMap.Remove(key); } else { diff --git a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs index 0cead01a69d..ffaae66ddec 100644 --- a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs +++ b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs @@ -1098,9 +1098,11 @@ private void FindOrCreateJoinEntry( SetForeignKeyProperties( joinEntry, arguments.Entry, arguments.SkipNavigation.ForeignKey, arguments.SetModified, arguments.FromQuery); + SetNavigation(joinEntry, arguments.SkipNavigation.ForeignKey.DependentToPrincipal, arguments.Entry, arguments.FromQuery); SetForeignKeyProperties( joinEntry, arguments.OtherEntry, arguments.SkipNavigation.Inverse.ForeignKey, arguments.SetModified, arguments.FromQuery); + SetNavigation(joinEntry, arguments.SkipNavigation.Inverse.ForeignKey.DependentToPrincipal, arguments.OtherEntry, arguments.FromQuery); joinEntry.SetEntityState( arguments.SetModified diff --git a/test/EFCore.SqlServer.FunctionalTests/SqlServerEndToEndTest.cs b/test/EFCore.SqlServer.FunctionalTests/SqlServerEndToEndTest.cs index d820f27c429..41eff5ac533 100644 --- a/test/EFCore.SqlServer.FunctionalTests/SqlServerEndToEndTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/SqlServerEndToEndTest.cs @@ -856,12 +856,12 @@ public void Can_replace_identifying_FK_entity_with_many_to_many() [ConditionalTheory] [MemberData( nameof(DataGenerator.GetCombinations), - new object[] { 0, 1, 2, 3, 4 }, + new object[] { 0, 1, 2, 3, 4, 7 }, 2, MemberType = typeof(DataGenerator))] public void Can_insert_entities_with_generated_PKs(int studentCount, int courseCount) { - var students = new[] + var students = new Student[] { new() { @@ -869,43 +869,43 @@ public void Can_insert_entities_with_generated_PKs(int studentCount, int courseC LastName = "Alexander", EnrollmentDate = DateTime.Parse("2019-09-01") }, - new Student + new() { FirstMidName = "Meredith", LastName = "Alonso", EnrollmentDate = DateTime.Parse("2017-09-01") }, - new Student + new() { FirstMidName = "Arturo", LastName = "Anand", EnrollmentDate = DateTime.Parse("2018-09-01") }, - new Student + new() { FirstMidName = "Gytis", LastName = "Barzdukas", EnrollmentDate = DateTime.Parse("2017-09-01") }, - new Student + new() { FirstMidName = "Yan", LastName = "Li", EnrollmentDate = DateTime.Parse("2017-09-01") }, - new Student + new() { FirstMidName = "Peggy", LastName = "Justice", EnrollmentDate = DateTime.Parse("2016-09-01") }, - new Student + new() { FirstMidName = "Laura", LastName = "Norman", EnrollmentDate = DateTime.Parse("2018-09-01") }, - new Student + new() { FirstMidName = "Nino", LastName = "Olivetto", @@ -913,25 +913,40 @@ public void Can_insert_entities_with_generated_PKs(int studentCount, int courseC } }; - var courses = new[] + var courses = new Course[] { new() { Title = "Chemistry", Credits = 3 }, - new Course { Title = "Microeconomics", Credits = 3 }, - new Course { Title = "Macroeconomics", Credits = 3 }, - new Course { Title = "Calculus", Credits = 4 }, - new Course { Title = "Trigonometry", Credits = 4 }, - new Course { Title = "Composition", Credits = 3 }, - new Course { Title = "Literature", Credits = 4 } + new() { Title = "Microeconomics", Credits = 3 }, + new() { Title = "Macroeconomics", Credits = 3 }, + new() { Title = "Calculus", Credits = 4 }, + new() { Title = "Trigonometry", Credits = 4 }, + new() { Title = "Composition", Credits = 3 }, + new() { Title = "Literature", Credits = 4 } }; using var testDatabase = SqlServerTestStore.CreateInitialized(DatabaseName); var options = Fixture.CreateOptions(testDatabase); + var nextCourse = 0; using (var context = new UniversityContext(options)) { context.Database.EnsureCreatedResiliently(); for (var i = 0; i < studentCount; i++) { + if (courseCount > 1) + { + students[i].Courses.Add(courses[nextCourse++]); + if(nextCourse >= courseCount) + { + nextCourse = 0; + } + + students[i].Courses.Add(courses[nextCourse++]); + if (nextCourse >= courseCount) + { + nextCourse = 0; + } + } context.Students.Add(students[i]); } @@ -940,13 +955,48 @@ public void Can_insert_entities_with_generated_PKs(int studentCount, int courseC context.Courses.Add(courses[i]); } + Assert.All(context.Enrollments.Local, e => + { + var entry = context.Entry(e); + var student = e.Student; + var course = e.Course; + Assert.Equal(student.Id, e.StudentId); + Assert.Equal(course.Id, e.CourseId); + Assert.Equal(context.Entry(student).Property(e => e.Id).CurrentValue, entry.Property(e => e.StudentId).CurrentValue); + Assert.Equal(context.Entry(course).Property(e => e.Id).CurrentValue, entry.Property(e => e.CourseId).CurrentValue); + Assert.True(entry.Property(e => e.StudentId).IsTemporary); + Assert.True(entry.Property(e => e.CourseId).IsTemporary); + Assert.True(context.Entry(student).Property(e => e.Id).IsTemporary); + Assert.True(context.Entry(course).Property(e => e.Id).IsTemporary); + }); + context.SaveChanges(); + + Assert.All(context.Enrollments.Local, e => + { + var entry = context.Entry(e); + var student = e.Student; + var course = e.Course; + Assert.Equal(student.Id, e.StudentId); + Assert.Equal(course.Id, e.CourseId); + Assert.False(entry.Property(e => e.StudentId).IsTemporary); + Assert.False(entry.Property(e => e.CourseId).IsTemporary); + }); } using (var context = new UniversityContext(options)) { - Assert.Equal(studentCount, context.Students.Count()); - Assert.Equal(courseCount, context.Courses.Count()); + Assert.Equal(studentCount, context.Students.ToList().Count()); + Assert.Equal(courseCount, context.Courses.ToList().Count()); + + var enrollments = context.Enrollments.Include(e => e.Course).Include(e => e.Student).ToList(); + Assert.All(enrollments, e => + { + var student = e.Student; + var course = e.Course; + Assert.Equal(student.Id, e.StudentId); + Assert.Equal(course.Id, e.CourseId); + }); } } @@ -960,6 +1010,8 @@ public class Course public virtual ICollection Enrollments { get; set; } = new List(); + public virtual ICollection Students { get; set; } = new List(); + public byte[] RowVersion { get; set; } = Array.Empty(); } @@ -975,6 +1027,8 @@ public class Student public virtual ICollection Enrollments { get; } = new List(); + public virtual ICollection Courses { get; set; } = new List(); + public byte[] RowVersion { get; set; } = Array.Empty(); } @@ -1030,6 +1084,10 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) builder.Property(x => x.RowVersion) .IsRowVersion(); + + builder.HasMany(x => x.Students) + .WithMany(x => x.Courses) + .UsingEntity(); }); modelBuilder.Entity(