diff --git a/src/EFCore.Specification.Tests/GraphUpdatesFixtureBase.cs b/src/EFCore.Specification.Tests/GraphUpdatesFixtureBase.cs index a167ebea3a7..24ba8929873 100644 --- a/src/EFCore.Specification.Tests/GraphUpdatesFixtureBase.cs +++ b/src/EFCore.Specification.Tests/GraphUpdatesFixtureBase.cs @@ -328,6 +328,20 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con modelBuilder.Entity(); modelBuilder.Entity(); + + modelBuilder.Entity(); + + modelBuilder.Entity() + .HasMany(qt => qt.Choices) + .WithOne() + .HasForeignKey(tc => tc.QuestTaskId); + + modelBuilder.Entity() + .HasMany(hat => hat.Choices) + .WithOne() + .HasForeignKey(tc => tc.QuestTaskId); + + modelBuilder.Entity(); } protected virtual object CreateFullGraph() @@ -2548,6 +2562,54 @@ public BadCustomer BadCustomer } } + protected class HiddenAreaTask : TaskWithChoices + { + } + + protected abstract class QuestTask : NotifyingEntity + { + private int _id; + + public int Id + { + get => _id; + set => SetWithNotify(value, ref _id); + } + } + + protected class QuizTask : TaskWithChoices + { + } + + protected class TaskChoice : NotifyingEntity + { + private int _id; + private int _questTaskId; + + public int Id + { + get => _id; + set => SetWithNotify(value, ref _id); + } + + public int QuestTaskId + { + get => _questTaskId; + set => SetWithNotify(value, ref _questTaskId); + } + } + + protected abstract class TaskWithChoices : QuestTask + { + private ICollection _choices = new ObservableHashSet(ReferenceEqualityComparer.Instance); + + public ICollection Choices + { + get => _choices; + set => SetWithNotify(value, ref _choices); + } + } + protected class NotifyingEntity : INotifyPropertyChanging, INotifyPropertyChanged { protected void SetWithNotify(T value, ref T field, [CallerMemberName] string propertyName = "") diff --git a/src/EFCore.Specification.Tests/GraphUpdatesTestBase.cs b/src/EFCore.Specification.Tests/GraphUpdatesTestBase.cs index 0a7e47052a0..ddeff66bff8 100644 --- a/src/EFCore.Specification.Tests/GraphUpdatesTestBase.cs +++ b/src/EFCore.Specification.Tests/GraphUpdatesTestBase.cs @@ -5733,5 +5733,102 @@ public virtual void Sometimes_not_calling_DetectChanges_when_required_does_not_t Assert.Empty(principal.BadOrders); }); } + + [ConditionalFact] + public virtual void Can_add_valid_first_depedent_when_multiple_possible_principal_sides() + { + ExecuteWithStrategyInTransaction( + context => + { + var quizTask = new QuizTask(); + quizTask.Choices.Add(new TaskChoice()); + + context.Add(quizTask); + + context.SaveChanges(); + }, + context => + { + var quizTask = context.Set().Include(e => e.Choices).Single(); + + Assert.Equal(quizTask.Id, quizTask.Choices.Single().QuestTaskId); + + Assert.Same(quizTask.Choices.Single(), context.Set().Single()); + + Assert.Empty(context.Set().Include(e => e.Choices)); + }); + } + + [ConditionalFact] + public virtual void Can_add_valid_second_depedent_when_multiple_possible_principal_sides() + { + ExecuteWithStrategyInTransaction( + context => + { + var hiddenAreaTask = new HiddenAreaTask(); + hiddenAreaTask.Choices.Add(new TaskChoice()); + + context.Add(hiddenAreaTask); + + context.SaveChanges(); + }, + context => + { + var hiddenAreaTask = context.Set().Include(e => e.Choices).Single(); + + Assert.Equal(hiddenAreaTask.Id, hiddenAreaTask.Choices.Single().QuestTaskId); + + Assert.Same(hiddenAreaTask.Choices.Single(), context.Set().Single()); + + Assert.Empty(context.Set().Include(e => e.Choices)); + }); + } + + [ConditionalFact] + public virtual void Can_add_multiple_depedents_when_multiple_possible_principal_sides() + { + ExecuteWithStrategyInTransaction( + context => + { + var quizTask = new QuizTask(); + quizTask.Choices.Add(new TaskChoice()); + quizTask.Choices.Add(new TaskChoice()); + + context.Add(quizTask); + + var hiddenAreaTask = new HiddenAreaTask(); + hiddenAreaTask.Choices.Add(new TaskChoice()); + hiddenAreaTask.Choices.Add(new TaskChoice()); + + context.Add(hiddenAreaTask); + + context.SaveChanges(); + }, + context => + { + var quizTask = context.Set().Include(e => e.Choices).Single(); + var hiddenAreaTask = context.Set().Include(e => e.Choices).Single(); + + Assert.Equal(2, quizTask.Choices.Count); + foreach (var quizTaskChoice in quizTask.Choices) + { + Assert.Equal(quizTask.Id, quizTaskChoice.QuestTaskId); + } + + Assert.Equal(2, hiddenAreaTask.Choices.Count); + foreach (var hiddenAreaTaskChoice in hiddenAreaTask.Choices) + { + Assert.Equal(hiddenAreaTask.Id, hiddenAreaTaskChoice.QuestTaskId); + } + + foreach (var taskChoice in context.Set()) + { + Assert.Equal( + 1, + quizTask.Choices.Count(e => e.Id == taskChoice.Id) + + hiddenAreaTask.Choices.Count(e => e.Id == taskChoice.Id)); + } + }); + } } } diff --git a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs index ebebbaf5c0d..796d04fa897 100644 --- a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs +++ b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs @@ -530,6 +530,8 @@ private void InitialFixup( { var entityType = (EntityType)entry.EntityType; var stateManager = entry.StateManager; + IForeignKey conflictingPrincipalForeignKey = null; + var matchingPrincipal = false; foreach (var foreignKey in entityType.GetForeignKeys()) { @@ -541,32 +543,47 @@ private void InitialFixup( { if (!foreignKey.PrincipalEntityType.IsAssignableFrom(principalEntry.EntityType)) { - if (_sensitiveLoggingEnabled) - { - throw new InvalidOperationException(CoreStrings.IncompatiblePrincipalEntrySensitive( - entry.BuildCurrentValuesString(foreignKey.Properties), - entityType.DisplayName(), - entry.BuildOriginalValuesString(entityType.FindPrimaryKey().Properties), - principalEntry.EntityType.DisplayName(), - foreignKey.PrincipalEntityType.DisplayName())); - } - - throw new InvalidOperationException(CoreStrings.IncompatiblePrincipalEntry( - Property.Format(foreignKey.Properties), - entityType.DisplayName(), - principalEntry.EntityType.DisplayName(), - foreignKey.PrincipalEntityType.DisplayName())); + conflictingPrincipalForeignKey = foreignKey; } + else + { + matchingPrincipal = true; - // Set navigation to principal based on FK properties - SetNavigation(entry, foreignKey.DependentToPrincipal, principalEntry); + // Set navigation to principal based on FK properties + SetNavigation(entry, foreignKey.DependentToPrincipal, principalEntry); - // Add this entity to principal's collection, or set inverse for 1:1 - ToDependentFixup(entry, principalEntry, foreignKey); + // Add this entity to principal's collection, or set inverse for 1:1 + ToDependentFixup(entry, principalEntry, foreignKey); + } } } } + if (!matchingPrincipal + && conflictingPrincipalForeignKey != null) + { + var principalEntry = stateManager.GetPrincipal(entry, conflictingPrincipalForeignKey); + + if (_sensitiveLoggingEnabled) + { + throw new InvalidOperationException( + CoreStrings.IncompatiblePrincipalEntrySensitive( + entry.BuildCurrentValuesString(conflictingPrincipalForeignKey.Properties), + entityType.DisplayName(), + entry.BuildOriginalValuesString(entityType.FindPrimaryKey().Properties), + principalEntry.EntityType.DisplayName(), + conflictingPrincipalForeignKey.PrincipalEntityType.DisplayName())); + } + + throw new InvalidOperationException( + CoreStrings.IncompatiblePrincipalEntry( + Property.Format(conflictingPrincipalForeignKey.Properties), + entityType.DisplayName(), + principalEntry.EntityType.DisplayName(), + conflictingPrincipalForeignKey.PrincipalEntityType.DisplayName())); + } + + foreach (var foreignKey in entityType.GetReferencingForeignKeys()) { if (handledForeignKeys == null @@ -829,7 +846,7 @@ private void ConditionallyNullForeignKeyProperties( for (var i = 0; i < foreignKey.Properties.Count; i++) { if (!PrincipalValueEqualsDependentValue( - principalProperties[i], + principalProperties[i], dependentEntry[dependentProperties[i]], principalEntry[principalProperties[i]])) {