Skip to content

Commit

Permalink
Stop special-casing the root entity on Attach
Browse files Browse the repository at this point in the history 
Issue dotnet#6990

Makes Attach/Update more useful by properly using the value-generation semantics built into the model.
  • Loading branch information
@ajcvickers
ajcvickers committed Nov 16, 2016
1 parent 3a10927 commit 0d8b36b
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 22 deletions.
12 changes: 5 additions & 7 deletions src/Microsoft.EntityFrameworkCore/ChangeTracking/Internal/EntityGraphAttacher.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,11 @@ private static bool PaintAction(EntityEntryGraphNode node)
return false;
}

if (node.InboundNavigation != null
&& !internalEntityEntry.IsKeySet)
{
node.NodeState = EntityState.Added;
}

internalEntityEntry.SetEntityState((EntityState)node.NodeState, acceptChanges: true);
internalEntityEntry.SetEntityState(
internalEntityEntry.IsKeySet
? (EntityState)node.NodeState
: EntityState.Added,
acceptChanges: true);

return true;
}
Expand Down
5 changes: 4 additions & 1 deletion src/Microsoft.EntityFrameworkCore/ChangeTracking/Internal/InternalEntityEntry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,10 @@ private bool IsTemporaryOrDefault(IProperty property)
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public virtual bool IsKeySet => !EntityType.FindPrimaryKey().Properties.Any(p => p.ClrType.IsDefaultValue(this[p]));
public virtual bool IsKeySet => !EntityType.FindPrimaryKey().Properties.Any(
p => p.ClrType.IsDefaultValue(this[p])
&& (p.ValueGenerated == ValueGenerated.OnAdd
|| p.IsForeignKey()));

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
Expand Down
4 changes: 2 additions & 2 deletions test/Microsoft.EntityFrameworkCore.Tests/ChangeTracking/AggregatesTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void Attaching_aggregate_with_no_key_set_adds_it_instead()

context.Attach(blog);

Assert.Equal(EntityState.Unchanged, context.Entry(blog).State); // See Issue #3890
Assert.Equal(EntityState.Added, context.Entry(blog).State);
Assert.Equal(EntityState.Added, context.Entry(posts[0]).State);
Assert.Equal(EntityState.Added, context.Entry(posts[1]).State);
Assert.Equal(EntityState.Added, context.Entry(comments0[0]).State);
Expand All @@ -119,7 +119,7 @@ public void Attaching_one_to_one_aggregate_with_no_key_set_adds_it_instead()

context.Attach(category);

Assert.Equal(EntityState.Unchanged, context.Entry(category).State); // See Issue #3890
Assert.Equal(EntityState.Added, context.Entry(category).State);
Assert.Equal(EntityState.Added, context.Entry(category.Statistics).State);
}
}
Expand Down
14 changes: 10 additions & 4 deletions test/Microsoft.EntityFrameworkCore.Tests/ChangeTracking/CollectionEntryTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,8 @@ public void IsModified_tracks_state_of_FK_property_principal()
using (var context = new FreezerContext())
{
var cherry = new Cherry();
var chunky1 = new Chunky { Garcia = cherry };
var chunky2 = new Chunky { Garcia = cherry };
var chunky1 = new Chunky { Id = 1, Garcia = cherry };
var chunky2 = new Chunky { Id = 2, Garcia = cherry };
cherry.Monkeys = new List<Chunky> { chunky1, chunky2 };
context.AttachRange(cherry, chunky1, chunky2);

Expand All @@ -348,8 +348,8 @@ public void IsModified_can_set_fk_to_modified_principal()
using (var context = new FreezerContext())
{
var cherry = new Cherry();
var chunky1 = new Chunky { Garcia = cherry };
var chunky2 = new Chunky { Garcia = cherry };
var chunky1 = new Chunky { Id = 1, Garcia = cherry };
var chunky2 = new Chunky { Id = 2, Garcia = cherry };
cherry.Monkeys = new List<Chunky> { chunky1, chunky2 };
context.AttachRange(cherry, chunky1, chunky2);

Expand Down Expand Up @@ -396,6 +396,12 @@ protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBu
=> optionsBuilder.UseInMemoryDatabase();

public DbSet<Chunky> Icecream { get; set; }

protected internal override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Chunky>().Property(e => e.Id).ValueGeneratedNever();
modelBuilder.Entity<Cherry>().Property(e => e.Id).ValueGeneratedNever();
}
}
}
}
6 changes: 6 additions & 0 deletions test/Microsoft.EntityFrameworkCore.Tests/ChangeTracking/EntityEntryTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,12 @@ protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBu
=> optionsBuilder.UseInMemoryDatabase();

public DbSet<Chunky> Icecream { get; set; }

protected internal override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Chunky>().Property(e => e.Id).ValueGeneratedNever();
modelBuilder.Entity<Cherry>().Property(e => e.Id).ValueGeneratedNever();
}
}
}
}
6 changes: 6 additions & 0 deletions test/Microsoft.EntityFrameworkCore.Tests/ChangeTracking/ReferenceEntryTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,12 @@ protected internal override void OnConfiguring(DbContextOptionsBuilder optionsBu
=> optionsBuilder.UseInMemoryDatabase();

public DbSet<Chunky> Icecream { get; set; }

protected internal override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Chunky>().Property(e => e.Id).ValueGeneratedNever();
modelBuilder.Entity<Cherry>().Property(e => e.Id).ValueGeneratedNever();
}
}
}
}
16 changes: 8 additions & 8 deletions test/Microsoft.EntityFrameworkCore.Tests/DbContextTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -569,13 +569,13 @@ public async Task Can_add_new_entities_with_default_value_to_context_with_graph_
[Fact]
public async Task Can_add_existing_entities_with_default_value_to_context_to_be_attached_with_graph_method()
{
await TrackEntitiesDefaultValueTest((c, e) => c.Attach(e), (c, e) => c.Attach(e), EntityState.Unchanged);
await TrackEntitiesDefaultValueTest((c, e) => c.Attach(e), (c, e) => c.Attach(e), EntityState.Added);
}

[Fact]
public async Task Can_add_existing_entities_with_default_value_to_context_to_be_updated_with_graph_method()
{
await TrackEntitiesDefaultValueTest((c, e) => c.Update(e), (c, e) => c.Update(e), EntityState.Modified);
await TrackEntitiesDefaultValueTest((c, e) => c.Update(e), (c, e) => c.Update(e), EntityState.Added);
}

private static Task TrackEntitiesDefaultValueTest(
Expand Down Expand Up @@ -628,13 +628,13 @@ public async Task Can_add_multiple_new_entities_with_default_values_to_context_a
[Fact]
public async Task Can_add_multiple_existing_entities_with_default_values_to_context_to_be_attached()
{
await TrackMultipleEntitiesDefaultValuesTest((c, e) => c.AttachRange(e[0]), (c, e) => c.AttachRange(e[0]), EntityState.Unchanged);
await TrackMultipleEntitiesDefaultValuesTest((c, e) => c.AttachRange(e[0]), (c, e) => c.AttachRange(e[0]), EntityState.Added);
}

[Fact]
public async Task Can_add_multiple_existing_entities_with_default_values_to_context_to_be_updated()
{
await TrackMultipleEntitiesDefaultValuesTest((c, e) => c.UpdateRange(e[0]), (c, e) => c.UpdateRange(e[0]), EntityState.Modified);
await TrackMultipleEntitiesDefaultValuesTest((c, e) => c.UpdateRange(e[0]), (c, e) => c.UpdateRange(e[0]), EntityState.Added);
}

[Fact]
Expand Down Expand Up @@ -902,13 +902,13 @@ public async Task Can_add_new_entities_with_default_value_to_context_non_generic
[Fact]
public async Task Can_add_existing_entities_with_default_value_to_context_to_be_attached_non_generic_graph()
{
await TrackEntitiesDefaultValuesTestNonGeneric((c, e) => c.Attach(e), (c, e) => c.Attach(e), EntityState.Unchanged);
await TrackEntitiesDefaultValuesTestNonGeneric((c, e) => c.Attach(e), (c, e) => c.Attach(e), EntityState.Added);
}

[Fact]
public async Task Can_add_existing_entities_with_default_value_to_context_to_be_updated_non_generic_graph()
{
await TrackEntitiesDefaultValuesTestNonGeneric((c, e) => c.Update(e), (c, e) => c.Update(e), EntityState.Modified);
await TrackEntitiesDefaultValuesTestNonGeneric((c, e) => c.Update(e), (c, e) => c.Update(e), EntityState.Added);
}

private static Task TrackEntitiesDefaultValuesTestNonGeneric(
Expand Down Expand Up @@ -967,13 +967,13 @@ public async Task Can_add_multiple_new_entities_with_default_values_to_context_E
[Fact]
public async Task Can_add_multiple_existing_entities_with_default_values_to_context_to_be_attached_Enumerable_graph()
{
await TrackMultipleEntitiesDefaultValueTestEnumerable((c, e) => c.AttachRange(e), (c, e) => c.AttachRange(e), EntityState.Unchanged);
await TrackMultipleEntitiesDefaultValueTestEnumerable((c, e) => c.AttachRange(e), (c, e) => c.AttachRange(e), EntityState.Added);
}

[Fact]
public async Task Can_add_multiple_existing_entities_with_default_values_to_context_to_be_updated_Enumerable_graph()
{
await TrackMultipleEntitiesDefaultValueTestEnumerable((c, e) => c.UpdateRange(e), (c, e) => c.UpdateRange(e), EntityState.Modified);
await TrackMultipleEntitiesDefaultValueTestEnumerable((c, e) => c.UpdateRange(e), (c, e) => c.UpdateRange(e), EntityState.Added);
}

private static Task TrackMultipleEntitiesDefaultValueTestEnumerable(
Expand Down

0 comments on commit 0d8b36b

Please sign in to comment.