diff --git a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs index c0747ca0d38..a2372cff602 100644 --- a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs +++ b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs @@ -292,21 +292,7 @@ public virtual void NavigationCollectionChanged( if (navigationBase is ISkipNavigation skipNavigation) { - var joinEntry = FindOrCreateJoinEntry( - entry, newTargetEntry, skipNavigation, fromQuery: false, setModified: false); - - if (joinEntry.EntityState == EntityState.Detached) - { - try - { - _inFixup = false; - joinEntry.SetEntityState(EntityState.Added); - } - finally - { - _inFixup = true; - } - } + FindOrCreateJoinEntry(entry, newTargetEntry, skipNavigation, fromQuery: false, setModified: true); Check.DebugAssert( skipNavigation.Inverse.IsCollection, @@ -845,21 +831,7 @@ private void InitialFixup( } else { - var joinEntry = FindOrCreateJoinEntry( - entry, otherEntry, skipNavigation, fromQuery, setModified); - - if (joinEntry.EntityState == EntityState.Detached) - { - try - { - _inFixup = false; - joinEntry.SetEntityState(setModified ? EntityState.Added : EntityState.Unchanged); - } - finally - { - _inFixup = true; - } - } + FindOrCreateJoinEntry(entry, otherEntry, skipNavigation, fromQuery, setModified); Check.DebugAssert( skipNavigation.Inverse.IsCollection, @@ -893,21 +865,11 @@ private void DelayedFixup( var setModified = referencedEntry.EntityState != EntityState.Unchanged; if (navigationBase is ISkipNavigation skipNavigation) { - var joinEntry = FindOrCreateJoinEntry( - entry, referencedEntry, skipNavigation, fromQuery, setModified); + FindOrCreateJoinEntry(entry, referencedEntry, skipNavigation, fromQuery, setModified); - if (joinEntry.EntityState == EntityState.Detached) - { - try - { - _inFixup = false; - joinEntry.SetEntityState(setModified ? EntityState.Added : EntityState.Unchanged); - } - finally - { - _inFixup = true; - } - } + Check.DebugAssert( + skipNavigation.Inverse.IsCollection, + "Issue #21673. Non-collection skip navigations not supported."); AddToCollection(referencedEntry, skipNavigation.Inverse, entry, fromQuery); } @@ -937,7 +899,7 @@ private void DelayedFixup( } } - private static InternalEntityEntry FindOrCreateJoinEntry( + private InternalEntityEntry FindOrCreateJoinEntry( InternalEntityEntry entry, InternalEntityEntry otherEntry, ISkipNavigation skipNavigation, @@ -958,6 +920,25 @@ private static InternalEntityEntry FindOrCreateJoinEntry( SetForeignKeyProperties(joinEntry, entry, skipNavigation.ForeignKey, setModified, fromQuery); SetForeignKeyProperties(joinEntry, otherEntry, skipNavigation.Inverse.ForeignKey, setModified, fromQuery); + if (joinEntry.EntityState == EntityState.Detached) + { + try + { + _inFixup = false; + + joinEntry.SetEntityState( + setModified + || entry.EntityState == EntityState.Added + || otherEntry.EntityState == EntityState.Added + ? EntityState.Added + : EntityState.Unchanged); + } + finally + { + _inFixup = true; + } + } + return joinEntry; } diff --git a/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs b/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs index c4a9f7222be..7610d00545a 100644 --- a/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs +++ b/test/EFCore.Specification.Tests/ManyToManyTrackingTestBase.cs @@ -2920,6 +2920,216 @@ static void ValidateFixup(DbContext context, IList leftEnti } } + [ConditionalFact] + public virtual void Can_insert_many_to_many_fully_by_convention_generated_keys() + { + ExecuteWithStrategyInTransaction( + context => + { + var leftEntities = new[] + { + context.Set().CreateInstance(), + context.Set().CreateInstance(), + context.Set().CreateInstance() + }; + var rightEntities = new[] + { + context.Set().CreateInstance(), + context.Set().CreateInstance(), + context.Set().CreateInstance() + }; + + leftEntities[0].Rights.Add(rightEntities[0]); // 11 - 21 + leftEntities[0].Rights.Add(rightEntities[1]); // 11 - 22 + leftEntities[0].Rights.Add(rightEntities[2]); // 11 - 23 + + rightEntities[0].Lefts.Add(leftEntities[0]); // 21 - 11 (Dupe) + rightEntities[0].Lefts.Add(leftEntities[1]); // 21 - 12 + rightEntities[0].Lefts.Add(leftEntities[2]); // 21 - 13 + + context.AddRange(leftEntities[0], leftEntities[1], leftEntities[2]); + + ValidateFixup(context, leftEntities, rightEntities); + + context.SaveChanges(); + + ValidateFixup(context, leftEntities, rightEntities); + }, + context => + { + var results = context.Set().Include(e => e.Rights).ToList(); + Assert.Equal(3, results.Count); + + 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()); + + var leftEntities = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Id) + .ToList(); + var rightEntities = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Id) + .ToList(); + + Assert.Equal(3, leftEntities[0].Rights.Count); + Assert.Single(leftEntities[1].Rights); + Assert.Single(leftEntities[2].Rights); + + Assert.Equal(3, rightEntities[0].Lefts.Count); + Assert.Single(rightEntities[1].Lefts); + Assert.Single(rightEntities[2].Lefts); + }); + + 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].Rights.Count); + Assert.Single(leftEntities[1].Rights); + Assert.Single(leftEntities[2].Rights); + + Assert.Equal(3, rightEntities[0].Lefts.Count); + Assert.Single(rightEntities[1].Lefts); + Assert.Single(rightEntities[2].Lefts); + } + } + + [ConditionalTheory] + [InlineData(true)] + [InlineData(false)] + public virtual void Can_Attach_or_Update_a_many_to_many_with_mixed_set_and_unset_keys(bool useUpdate) + { + var existingLeftId = -1; + var existingRightId = -1; + ExecuteWithStrategyInTransaction( + context => + { + var left = context.Set().CreateInstance(); + var right = context.Set().CreateInstance(); + + if (!useUpdate) + { + left.Rights.Add(right); + } + + context.AddRange(left, right); + context.SaveChanges(); + + existingLeftId = left.Id; + existingRightId = right.Id; + }, + context => + { + var leftEntities = new[] + { + context.Set().CreateInstance((e, p) => e.Id = existingLeftId), + context.Set().CreateInstance(), + context.Set().CreateInstance() + }; + var rightEntities = new[] + { + context.Set().CreateInstance((e, p) => e.Id = existingRightId), + context.Set().CreateInstance(), + context.Set().CreateInstance() + }; + + leftEntities[0].Rights.Add(rightEntities[0]); // 11 - 21 + leftEntities[0].Rights.Add(rightEntities[1]); // 11 - 22 + leftEntities[0].Rights.Add(rightEntities[2]); // 11 - 23 + + rightEntities[0].Lefts.Add(leftEntities[0]); // 21 - 11 (Dupe) + rightEntities[0].Lefts.Add(leftEntities[1]); // 21 - 12 + rightEntities[0].Lefts.Add(leftEntities[2]); // 21 - 13 + + if (useUpdate) + { + context.Update(leftEntities[0]); + } + else + { + context.Attach(leftEntities[0]); + } + + ValidateFixup(context, leftEntities, rightEntities); + + var entityEntries = context.ChangeTracker.Entries>().ToList(); + foreach (var joinEntry in entityEntries) + { + Assert.Equal( + !useUpdate + && joinEntry.Property("RightsId").CurrentValue == existingRightId + && joinEntry.Property("LeftsId").CurrentValue == existingLeftId + ? EntityState.Unchanged + : EntityState.Added, joinEntry.State); + } + + foreach (var leftEntry in context.ChangeTracker.Entries()) + { + Assert.Equal( + leftEntry.Entity.Id == existingLeftId + ? useUpdate + ? EntityState.Modified + : EntityState.Unchanged + : EntityState.Added, leftEntry.State); + } + + foreach (var rightEntry in context.ChangeTracker.Entries()) + { + Assert.Equal( + rightEntry.Entity.Id == existingRightId + ? useUpdate + ? EntityState.Modified + : EntityState.Unchanged + : EntityState.Added, rightEntry.State); + } + + context.SaveChanges(); + + ValidateFixup(context, leftEntities, rightEntities); + }, + context => + { + var results = context.Set().Include(e => e.Rights).ToList(); + Assert.Equal(3, results.Count); + + 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()); + + var leftEntities = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Id) + .ToList(); + var rightEntities = context.ChangeTracker.Entries().Select(e => e.Entity).OrderBy(e => e.Id) + .ToList(); + + Assert.Equal(3, leftEntities[0].Rights.Count); + Assert.Single(leftEntities[1].Rights); + Assert.Single(leftEntities[2].Rights); + + Assert.Equal(3, rightEntities[0].Lefts.Count); + Assert.Single(rightEntities[1].Lefts); + Assert.Single(rightEntities[2].Lefts); + }); + + 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].Rights.Count); + Assert.Single(leftEntities[1].Rights); + Assert.Single(leftEntities[2].Rights); + + Assert.Equal(3, rightEntities[0].Lefts.Count); + Assert.Single(rightEntities[1].Lefts); + Assert.Single(rightEntities[2].Lefts); + } + } + [ConditionalFact] public virtual void Initial_tracking_uses_skip_navigations() { diff --git a/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/GeneratedKeysLeft.cs b/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/GeneratedKeysLeft.cs new file mode 100644 index 00000000000..5b5e75bf251 --- /dev/null +++ b/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/GeneratedKeysLeft.cs @@ -0,0 +1,16 @@ +// 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.Collections.Generic; +using System.Collections.ObjectModel; + +namespace Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel +{ + public class GeneratedKeysLeft + { + public virtual int Id { get; set; } + public virtual string Name { get; set; } + + public virtual ICollection Rights { get; } = new ObservableCollection(); + } +} diff --git a/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/GeneratedKeysRight.cs b/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/GeneratedKeysRight.cs new file mode 100644 index 00000000000..9bbabab8d41 --- /dev/null +++ b/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/GeneratedKeysRight.cs @@ -0,0 +1,16 @@ +// 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.Collections.Generic; +using System.Collections.ObjectModel; + +namespace Microsoft.EntityFrameworkCore.TestModels.ManyToManyModel +{ + public class GeneratedKeysRight + { + public virtual int Id { get; set; } + public virtual string Name { get; set; } + + public virtual ICollection Lefts { get; } = new ObservableCollection(); + } +} diff --git a/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/ManyToManyContext.cs b/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/ManyToManyContext.cs index c3d3a9fe37b..30a213d73d5 100644 --- a/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/ManyToManyContext.cs +++ b/test/EFCore.Specification.Tests/TestModels/ManyToManyModel/ManyToManyContext.cs @@ -22,6 +22,8 @@ public ManyToManyContext(DbContextOptions options) public DbSet EntityRoots { get; set; } public DbSet ImplicitManyToManyAs { get; set; } public DbSet ImplicitManyToManyBs { get; set; } + public DbSet GeneratedKeysLefts { get; set; } + public DbSet GeneratedKeysRights { get; set; } public static void Seed(ManyToManyContext context) => ManyToManyData.Seed(context); @@ -29,14 +31,14 @@ public static void Seed(ManyToManyContext context) public static class ManyToManyContextExtensions { - public static TEntity CreateInstance(this DbSet set, Action configureEntity) + public static TEntity CreateInstance(this DbSet set, Action configureEntity = null) where TEntity : class, new() { var isProxy = set.GetService().FindExtension()?.UseChangeTrackingProxies == true; var entity = isProxy ? set.CreateProxy() : new TEntity(); - configureEntity(entity, isProxy); + configureEntity?.Invoke(entity, isProxy); return entity; }