Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 87 additions & 2 deletions src/EFCore/ChangeTracking/Internal/NavigationFixer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/EFCore/EFCore.baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -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);"
},
Expand Down
8 changes: 8 additions & 0 deletions src/EFCore/Properties/CoreStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,9 @@
<data name="DuplicateEntityType" xml:space="preserve">
<value>The entity type '{entityType}' cannot be added to the model because an entity type with the same name already exists.</value>
</data>
<data name="DuplicateOwnedEntityInstance" xml:space="preserve">
<value>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.</value>
</data>
<data name="DuplicateForeignKey" xml:space="preserve">
<value>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}'.</value>
</data>
Expand Down
115 changes: 115 additions & 0 deletions test/EFCore.Tests/ChangeTracking/Internal/OwnedFixupTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<InvalidOperationException>(() => 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" };
Comment thread
AndriySvyryd marked this conversation as resolved.
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<InvalidOperationException>(() => 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<InvalidOperationException>(() => 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<InvalidOperationException>(() => 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<InvalidOperationException>(() => context.Add(principal2)).Message);
}
Comment thread
AndriySvyryd marked this conversation as resolved.

private class OneRowContext(bool async) : DbContext
{
private readonly bool _async = async;
Expand Down
Loading