Skip to content
Closed
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
12 changes: 12 additions & 0 deletions src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,18 @@ protected override void OnStateChanging(EntityState newState)

if (EntityState == EntityState.Detached)
{
// Check if this entity is already being tracked by a different entry.
// This can happen when the entity was detached and then re-tracked through
// navigation fixup (via DetectChanges) before the user manually sets its state.
var existingEntry = StateManager.TryGetEntry(Entity, EntityType);
if (existingEntry != null && existingEntry != this)
{
// The entity is already tracked by a different entry.
// Update the state of the existing entry instead of trying to track this stale entry.
existingEntry.SetEntityState(newState);
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The early return at line 216 bypasses the rest of the OnStateChanging flow, which means the subsequent call to StateManager.StartTracking(this) at line 219 is skipped. However, this also means that OnStateChanged (line 229) will never be called for this stale entry. This could result in state change events not being fired, change counters not being updated, and potentially other side effects. Consider whether OnStateChanged or other cleanup should be called before the early return, or if the current entry needs different handling (e.g., should this entry be marked as detached or disposed?).

Suggested change
existingEntry.SetEntityState(newState);
existingEntry.SetEntityState(newState);
OnStateChanged(newState);

Copilot uses AI. Check for mistakes.
return;
}

StateManager.StartTracking(this);
}
}
Expand Down
64 changes: 64 additions & 0 deletions test/EFCore.Tests/ChangeTracking/GraphTrackingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,70 @@ public void Can_add_two_aggregates_linked_down_the_tree()
Assert.Equal(EntityState.Added, context.Entry(reminders[1]).State);
}

[ConditionalFact]
public void Can_reattach_graph_with_client_generated_keys()
{
using var context = new GuidKeyContext();
var foo = new FooGuid { Name = "Foo1" };
var bar1 = new BarGuid { Name = "Bar1", Foo = foo };
var bar2 = new BarGuid { Name = "Bar2", Foo = foo, RelatedBar = bar1 };

var fooEntry = context.Foos.Add(foo);
var bar1Entry = context.Bars.Add(bar1);
var bar2Entry = context.Bars.Add(bar2);

fooEntry.State = EntityState.Detached;
bar1Entry.State = EntityState.Detached;
bar2Entry.State = EntityState.Detached;

// When detaching entities in Added state, EF Core automatically nulls out navigations
// to maintain consistency (since Added entities don't exist in the database yet).
// This means bar2.RelatedBar is now null and needs to be manually restored.

fooEntry.State = EntityState.Added;
fooEntry.DetectChanges();

bar2Entry.State = EntityState.Added;
bar2.RelatedBar = bar1; // Manually restore the navigation
bar2Entry.DetectChanges();

// This should not throw
bar1Entry.State = EntityState.Added;
bar1Entry.DetectChanges();

Assert.Equal(EntityState.Added, context.Entry(foo).State);
Assert.Equal(EntityState.Added, context.Entry(bar1).State);
Assert.Equal(EntityState.Added, context.Entry(bar2).State);
}

private class GuidKeyContext : DbContext
{
protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
=> optionsBuilder
.UseInMemoryDatabase(nameof(GuidKeyContext))
.UseInternalServiceProvider(InMemoryFixture.DefaultServiceProvider);

public DbSet<FooGuid> Foos => Set<FooGuid>();
public DbSet<BarGuid> Bars => Set<BarGuid>();
}

private class FooGuid
{
public Guid Id { get; set; }
public string Name { get; set; }
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nullable reference type annotations are missing for reference type properties. According to the coding guidelines, nullable reference types should be used. These properties should either be marked as nullable (e.g., public string? Name { get; set; }) or initialized to prevent potential null reference warnings.

Suggested change
public string Name { get; set; }
public string Name { get; set; } = string.Empty;

Copilot uses AI. Check for mistakes.
public ICollection<BarGuid> Bars { get; set; } = new HashSet<BarGuid>();
}

private class BarGuid
{
public Guid Id { get; set; }
public string Name { get; set; }
public FooGuid Foo { get; set; }
public Guid FooId { get; set; }
public BarGuid RelatedBar { get; set; }
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nullable reference type annotations are missing for reference type properties. According to the coding guidelines, nullable reference types should be used. These properties should either be marked as nullable (e.g., public string? Name { get; set; }) or initialized to prevent potential null reference warnings.

Suggested change
public BarGuid RelatedBar { get; set; }
public BarGuid? RelatedBar { get; set; }

Copilot uses AI. Check for mistakes.
public Guid? RelatedBarId { get; set; }
}

private class AggregateContext : DbContext
{
protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
Expand Down
Loading