diff --git a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs index 1a7342cd017..e2213cc5ba2 100644 --- a/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs +++ b/src/EFCore/ChangeTracking/Internal/NavigationFixer.cs @@ -247,6 +247,39 @@ public virtual void NavigationReferenceChanged( if (newValue != null && newTargetEntry == null) { + // The owned entity is not yet tracked under the target entity type. Check if it's tracked + // under a different entity type (shared CLR type) and its current owner still references it. + if (foreignKey.IsOwnership && !navigation.IsOnDependent) + { + var existingEntry = stateManager.TryGetEntry(newValue, throwOnNonUniqueness: false); + if (existingEntry != null && existingEntry.EntityState != EntityState.Detached) + { + var existingOwnership = existingEntry.EntityType.FindOwnership(); + if (existingOwnership != null) + { + var existingNavigation = existingOwnership.PrincipalToDependent; + if (existingNavigation != null) + { + var existingOwner = stateManager.FindPrincipal(existingEntry, existingOwnership); + // Only throw if the existing owner's navigation still points to this entity. + // If the navigation has already been cleared on the CLR object, this is a + // legitimate move/swap rather than a duplicate reference. + if (existingOwner != null + && ReferenceEquals(existingOwner[existingNavigation], newValue)) + { + throw new InvalidOperationException( + CoreStrings.DuplicateOwnedEntityInstance( + newValue.GetType().ShortDisplayName(), + existingNavigation.Name, + existingOwner.EntityType.DisplayName(), + navigation.Name, + entry.EntityType.DisplayName())); + } + } + } + } + } + stateManager.RecordReferencedUntrackedEntity(newValue, navigation, entry); entry.SetRelationshipSnapshotValue(navigation, newValue); @@ -868,6 +901,25 @@ private void InitialFixup( } else { + // Check for duplicate owned entity instance + // This handles the case where an already-tracked owned entity is being assigned + // to a different owner during the initial tracking phase (not during DetectChanges) + if (foreignKey.IsOwnership) + { + var existingOwner = stateManager.FindPrincipal(dependentEntry, foreignKey); + if (existingOwner != null + && !ReferenceEquals(existingOwner, entry)) + { + throw new InvalidOperationException( + CoreStrings.DuplicateOwnedEntityInstance( + navigationValue.GetType().ShortDisplayName(), + principalToDependent.Name, + existingOwner.EntityType.DisplayName(), + principalToDependent.Name, + entry.EntityType.DisplayName())); + } + } + FixupToDependent(entry, dependentEntry, foreignKey, setModified, fromQuery); } } @@ -982,9 +1034,42 @@ private void InitialFixup( // If the entity was previously referenced while it was still untracked, go back and do the fixup // that we would have done then now that the entity is tracked. - foreach (var (navigationBase, internalEntityEntry) in stateManager.GetRecordedReferrers(entry.Entity, clear: true)) + var referrers = stateManager.GetRecordedReferrers(entry.Entity, clear: true); + + // Check for duplicate owned entity instances + // Track the first owned navigation referrer to compare against subsequent ones + INavigation? firstOwnedNavigation = null; + InternalEntityEntry? firstOwnerEntry = null; + + foreach (var (navigationBase, ownerEntry) in referrers) { - DelayedFixup(internalEntityEntry, navigationBase, entry, fromQuery); + if (navigationBase is INavigation { IsCollection: false, ForeignKey.IsOwnership: true } ownedNavigation) + { + if (firstOwnedNavigation == null) + { + firstOwnedNavigation = ownedNavigation; + firstOwnerEntry = ownerEntry; + } + else + { + // Throw if it's the same owner with a different navigation, or a different owner (with same or different navigation) + // Both scenarios are invalid: an owned entity can only have one owner with one navigation + if (!ReferenceEquals(firstOwnerEntry, ownerEntry) || firstOwnedNavigation.Name != ownedNavigation.Name) + { + Check.DebugAssert(firstOwnerEntry != null, "firstOwnerEntry should not be null when firstOwnedNavigation is set"); + + throw new InvalidOperationException( + CoreStrings.DuplicateOwnedEntityInstance( + entry.Entity.GetType().ShortDisplayName(), + firstOwnedNavigation.Name, + firstOwnerEntry.EntityType.DisplayName(), + ownedNavigation.Name, + ownerEntry.EntityType.DisplayName())); + } + } + } + + DelayedFixup(ownerEntry, navigationBase, entry, fromQuery); } } } diff --git a/src/EFCore/EFCore.baseline.json b/src/EFCore/EFCore.baseline.json index 7c9812b71e2..4fa5cd2b2a6 100644 --- a/src/EFCore/EFCore.baseline.json +++ b/src/EFCore/EFCore.baseline.json @@ -3973,6 +3973,9 @@ { "Member": "static string DuplicateNamedIndex(object? indexName, object? indexProperties, object? entityType, object? duplicateEntityType);" }, + { + "Member": "static string DuplicateOwnedEntityInstance(object? entityType, object? firstNavigation, object? firstOwnerType, object? secondNavigation, object? secondOwnerType);" + }, { "Member": "static string DuplicatePropertiesOnBase(object? entityType, object? baseType, object? derivedPropertyType, object? derivedProperty, object? basePropertyType, object? baseProperty);" }, diff --git a/src/EFCore/Properties/CoreStrings.Designer.cs b/src/EFCore/Properties/CoreStrings.Designer.cs index 186af2df532..480d7b33b6e 100644 --- a/src/EFCore/Properties/CoreStrings.Designer.cs +++ b/src/EFCore/Properties/CoreStrings.Designer.cs @@ -1186,6 +1186,14 @@ public static string DuplicateEntityType(object? entityType) GetString("DuplicateEntityType", nameof(entityType)), entityType); + /// + /// The same object instance of type '{entityType}' is being referenced from the navigation '{firstNavigation}' on '{firstOwnerType}' and from the navigation '{secondNavigation}' on '{secondOwnerType}'. Owned entity types cannot have multiple owners pointing to the same instance. Consider creating a copy of the object to use for each navigation. + /// + public static string DuplicateOwnedEntityInstance(object? entityType, object? firstNavigation, object? firstOwnerType, object? secondNavigation, object? secondOwnerType) + => string.Format( + GetString("DuplicateOwnedEntityInstance", nameof(entityType), nameof(firstNavigation), nameof(firstOwnerType), nameof(secondNavigation), nameof(secondOwnerType)), + entityType, firstNavigation, firstOwnerType, secondNavigation, secondOwnerType); + /// /// The foreign key {foreignKeyProperties} cannot be added to the entity type '{entityType}' because a foreign key on the same properties already exists on entity type '{duplicateEntityType}' and also targets the key {keyProperties} on '{principalType}'. /// diff --git a/src/EFCore/Properties/CoreStrings.resx b/src/EFCore/Properties/CoreStrings.resx index 09ee563d2d1..48c5deb8608 100644 --- a/src/EFCore/Properties/CoreStrings.resx +++ b/src/EFCore/Properties/CoreStrings.resx @@ -558,6 +558,9 @@ The entity type '{entityType}' cannot be added to the model because an entity type with the same name already exists. + + The same object instance of type '{entityType}' is being referenced from the navigation '{firstNavigation}' on '{firstOwnerType}' and from the navigation '{secondNavigation}' on '{secondOwnerType}'. Owned entity types cannot have multiple owners pointing to the same instance. Consider creating a copy of the object to use for each navigation. + The foreign key {foreignKeyProperties} cannot be added to the entity type '{entityType}' because a foreign key on the same properties already exists on entity type '{duplicateEntityType}' and also targets the key {keyProperties} on '{principalType}'. diff --git a/test/EFCore.Tests/ChangeTracking/Internal/OwnedFixupTest.cs b/test/EFCore.Tests/ChangeTracking/Internal/OwnedFixupTest.cs index fae6c7fa5d8..a2421d9ccd6 100644 --- a/test/EFCore.Tests/ChangeTracking/Internal/OwnedFixupTest.cs +++ b/test/EFCore.Tests/ChangeTracking/Internal/OwnedFixupTest.cs @@ -5019,6 +5019,121 @@ public async Task SaveChanges_when_owner_has_PK_with_default_values(bool async) } } + [Fact] + public void Throws_when_same_instance_is_used_for_multiple_owned_navigations() + { + using var context = new FixupContext(); + var dependent = new Child { Name = "1" }; + var principal = new Parent + { + Id = 77, + Child1 = dependent, + Child2 = dependent + }; + + Assert.Equal( + CoreStrings.DuplicateOwnedEntityInstance( + nameof(Child), nameof(Parent.Child1), nameof(Parent), nameof(Parent.Child2), nameof(Parent)), + Assert.Throws(() => context.Add(principal)).Message); + } + + [Fact] + public void Throws_when_already_tracked_owned_instance_is_assigned_to_another_owner() + { + using var context = new FixupContext(); + var dependent = new Child { Name = "1" }; + var principal1 = new Parent + { + Id = 77, + Child1 = dependent + }; + + context.Add(principal1); + + var principal2 = new Parent + { + Id = 78, + Child1 = dependent // Same instance, already tracked + }; + + var ex = Assert.Throws(() => context.Add(principal2)); + Assert.Equal( + CoreStrings.DuplicateOwnedEntityInstance( + nameof(Child), nameof(Parent.Child1), nameof(Parent), nameof(Parent.Child1), nameof(Parent)), + ex.Message); + } + + [Fact] + public void Throws_when_already_tracked_owned_instance_is_later_assigned_to_different_navigation_on_different_owner() + { + using var context = new FixupContext(); + var dependent = new Child { Name = "1" }; + var principal1 = new Parent + { + Id = 77, + Child1 = dependent + }; + var principal2 = new Parent { Id = 78 }; + + context.Add(principal1); + context.Add(principal2); + + principal2.Child2 = dependent; // Same instance, different navigation, different owner + + Assert.Equal( + CoreStrings.DuplicateOwnedEntityInstance( + nameof(Child), nameof(Parent.Child1), nameof(Parent), nameof(Parent.Child2), nameof(Parent)), + Assert.Throws(() => context.ChangeTracker.DetectChanges()).Message); + } + + [Fact] + public void Throws_when_already_tracked_owned_instance_is_later_assigned_to_different_navigation_on_same_owner() + { + using var context = new FixupContext(); + var dependent = new Child { Name = "1" }; + var principal = new Parent + { + Id = 79, + Child1 = dependent + }; + + context.Add(principal); + + principal.Child2 = dependent; + + Assert.Equal( + CoreStrings.DuplicateOwnedEntityInstance( + nameof(Child), nameof(Parent.Child1), nameof(Parent), nameof(Parent.Child2), nameof(Parent)), + Assert.Throws(() => context.ChangeTracker.DetectChanges()).Message); + } + + [Fact] + public void Throws_when_owned_instance_in_unchanged_state_is_assigned_to_another_owner() + { + using var context = new FixupContext(); + var dependent = new Child { Name = "1" }; + var principal1 = new Parent + { + Id = 80, + Child1 = dependent + }; + + context.Add(principal1); + context.Entry(principal1).State = EntityState.Unchanged; + context.Entry(dependent).State = EntityState.Unchanged; + + var principal2 = new Parent + { + Id = 81, + Child1 = dependent + }; + + Assert.Equal( + CoreStrings.DuplicateOwnedEntityInstance( + nameof(Child), nameof(Parent.Child1), nameof(Parent), nameof(Parent.Child1), nameof(Parent)), + Assert.Throws(() => context.Add(principal2)).Message); + } + private class OneRowContext(bool async) : DbContext { private readonly bool _async = async;