Skip to content

[release/3.1] Cherry-picking commits for for closely related detach/delete issues #19911

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 5 commits into from
Feb 15, 2020
Merged
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
26 changes: 21 additions & 5 deletions src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1149,12 +1149,28 @@ private void SetProperty(
{
WritePropertyValue(propertyBase, value, isMaterialization);

if (currentValueType != CurrentValueType.Normal
&& !_temporaryValues.IsEmpty
&& equals(value, asProperty.ClrType.GetDefaultValue()))
var useNewBehavior
= !AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue19137", out var isEnabled) || !isEnabled;

if (useNewBehavior)
{
var storeGeneratedIndex = asProperty.GetStoreGeneratedIndex();
_temporaryValues.SetValue(asProperty, value, storeGeneratedIndex);
if (currentValueType != CurrentValueType.Normal
&& !_temporaryValues.IsEmpty)
{
var defaultValue = asProperty.ClrType.GetDefaultValue();
var storeGeneratedIndex = asProperty.GetStoreGeneratedIndex();
_temporaryValues.SetValue(asProperty, defaultValue, storeGeneratedIndex);
}
}
else
{
if (currentValueType != CurrentValueType.Normal
&& !_temporaryValues.IsEmpty
&& equals(value, asProperty.ClrType.GetDefaultValue()))
{
var storeGeneratedIndex = asProperty.GetStoreGeneratedIndex();
_temporaryValues.SetValue(asProperty, value, storeGeneratedIndex);
}
}
}
else
Expand Down
16 changes: 12 additions & 4 deletions src/EFCore/ChangeTracking/Internal/StateManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,8 @@ public virtual void CascadeChanges(bool force)
public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumerable<IForeignKey> foreignKeys = null)
{
var doCascadeDelete = force || CascadeDeleteTiming != CascadeTiming.Never;
var principalIsDetached = entry.EntityState == EntityState.Detached;
var useNewBehavior = !AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue18982", out var isEnabled) || !isEnabled;

foreignKeys ??= entry.EntityType.GetReferencingForeignKeys();
foreach (var fk in foreignKeys)
Expand All @@ -980,9 +982,14 @@ public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumer
|| fk.DeleteBehavior == DeleteBehavior.ClientCascade)
&& doCascadeDelete)
{
var cascadeState = dependent.EntityState == EntityState.Added
? EntityState.Detached
: EntityState.Deleted;
var cascadeState = useNewBehavior
? (principalIsDetached
|| dependent.EntityState == EntityState.Added
? EntityState.Detached
: EntityState.Deleted)
: (dependent.EntityState == EntityState.Added
? EntityState.Detached
: EntityState.Deleted);

if (SensitiveLoggingEnabled)
{
Expand All @@ -997,7 +1004,8 @@ public virtual void CascadeDelete(InternalEntityEntry entry, bool force, IEnumer

CascadeDelete(dependent, force);
}
else
else if (!useNewBehavior
|| !principalIsDetached)
{
foreach (var dependentProperty in fk.Properties)
{
Expand Down
74 changes: 37 additions & 37 deletions test/EFCore.Specification.Tests/PropertyValuesTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1262,53 +1262,53 @@ public async Task Values_can_be_reloaded_from_database_for_entity_in_any_state(E
[InlineData(EntityState.Detached, false)]
public async Task Reload_when_entity_deleted_in_store_can_happen_for_any_state(EntityState state, bool async)
{
using (var context = CreateContext())
{
var office = new Office { Number = "35" };
var mailRoom = new MailRoom { id = 36 };
var building = Building.Create(Guid.NewGuid(), "Bag End", 77);
using var context = CreateContext();

building.Offices.Add(office);
building.PrincipalMailRoom = mailRoom;
office.Building = building;
mailRoom.Building = building;
var office = new Office { Number = "35" };
var mailRoom = new MailRoom { id = 36 };
var building = Building.Create(Guid.NewGuid(), "Bag End", 77);

var entry = context.Entry(building);
building.Offices.Add(office);
building.PrincipalMailRoom = mailRoom;
office.Building = building;
mailRoom.Building = building;

context.Attach(building);
entry.State = state;
var entry = context.Entry(building);

if (async)
{
await entry.ReloadAsync();
}
else
{
entry.Reload();
}
context.Attach(building);
entry.State = state;

Assert.Equal("Bag End", entry.Property(e => e.Name).OriginalValue);
Assert.Equal("Bag End", entry.Property(e => e.Name).CurrentValue);
Assert.Equal("Bag End", building.Name);

if (state == EntityState.Added)
{
Assert.Equal(EntityState.Added, entry.State);
Assert.Same(mailRoom, building.PrincipalMailRoom);
Assert.Contains(office, building.Offices);
}
else
{
Assert.Equal(EntityState.Detached, entry.State);
Assert.Null(mailRoom.Building);
if (async)
{
await entry.ReloadAsync();
}
else
{
entry.Reload();
}

Assert.Equal(EntityState.Detached, context.Entry(office.Building).State);
Assert.Same(building, office.Building);
}
Assert.Equal("Bag End", entry.Property(e => e.Name).OriginalValue);
Assert.Equal("Bag End", entry.Property(e => e.Name).CurrentValue);
Assert.Equal("Bag End", building.Name);

if (state == EntityState.Added)
{
Assert.Equal(EntityState.Added, entry.State);
Assert.Same(mailRoom, building.PrincipalMailRoom);
Assert.Contains(office, building.Offices);
}
else
{
Assert.Equal(EntityState.Detached, entry.State);
Assert.Same(mailRoom, building.PrincipalMailRoom);
Assert.Contains(office, building.Offices);

Assert.Equal(EntityState.Detached, context.Entry(office.Building).State);
Assert.Same(building, office.Building);
}

Assert.Same(mailRoom, building.PrincipalMailRoom);
Assert.Contains(office, building.Offices);
}

[ConditionalFact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ public virtual void Save_required_one_to_one_changed_by_reference(ChangeMechanis
Assert.Same(new1, new2.Back);

Assert.NotNull(old1.Root);
Assert.Null(old2.Back);
Assert.Same(old1, old2.Back);
Assert.Equal(old1.Id, old2.Id);
});
}
Expand Down
125 changes: 125 additions & 0 deletions test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,129 @@ public virtual void Identity_property_on_Added_entity_with_temporary_value_gets_
context => Assert.Equal("Banana Joe", context.Set<Gumball>().Single(e => e.Id == id).Identity));
}

[ConditionalFact] // Issue #19137
public void Clearing_optional_FK_does_not_leave_temporary_value()
{
ExecuteWithStrategyInTransaction(
context =>
{
var product = new OptionalProduct();
context.Add(product);

var productEntry = context.Entry(product);
Assert.Equal(EntityState.Added, productEntry.State);

Assert.Equal(0, product.Id);
Assert.True(productEntry.Property(e => e.Id).CurrentValue < 0);
Assert.True(productEntry.Property(e => e.Id).IsTemporary);

Assert.Null(product.CategoryId);
Assert.Null(productEntry.Property(e => e.CategoryId).CurrentValue);
Assert.False(productEntry.Property(e => e.CategoryId).IsTemporary);

context.SaveChanges();

productEntry = context.Entry(product);
Assert.Equal(EntityState.Unchanged, productEntry.State);

Assert.Equal(1, product.Id);
Assert.Equal(1, productEntry.Property(e => e.Id).CurrentValue);
Assert.False(productEntry.Property(e => e.Id).IsTemporary);

Assert.Null(product.CategoryId);
Assert.Null(productEntry.Property(e => e.CategoryId).CurrentValue);
Assert.False(productEntry.Property(e => e.CategoryId).IsTemporary);

var category = new OptionalCategory();
product.Category = category;

productEntry = context.Entry(product);
Assert.Equal(EntityState.Modified, productEntry.State);

Assert.Equal(1, product.Id);
Assert.Equal(1, productEntry.Property(e => e.Id).CurrentValue);
Assert.False(productEntry.Property(e => e.Id).IsTemporary);

Assert.Null(product.CategoryId);
Assert.True(productEntry.Property(e => e.CategoryId).CurrentValue < 0);
Assert.True(productEntry.Property(e => e.CategoryId).IsTemporary);

var categoryEntry = context.Entry(category);
Assert.Equal(EntityState.Added, categoryEntry.State);
Assert.Equal(0, category.Id);
Assert.True(categoryEntry.Property(e => e.Id).CurrentValue < 0);
Assert.True(categoryEntry.Property(e => e.Id).IsTemporary);

context.SaveChanges();

productEntry = context.Entry(product);
Assert.Equal(EntityState.Unchanged, productEntry.State);

Assert.Equal(1, product.Id);
Assert.Equal(1, productEntry.Property(e => e.Id).CurrentValue);
Assert.False(productEntry.Property(e => e.Id).IsTemporary);

Assert.Equal(1, product.CategoryId);
Assert.Equal(1, productEntry.Property(e => e.CategoryId).CurrentValue);
Assert.False(productEntry.Property(e => e.CategoryId).IsTemporary);

categoryEntry = context.Entry(category);
Assert.Equal(EntityState.Unchanged, categoryEntry.State);
Assert.Equal(1, category.Id);
Assert.Equal(1, categoryEntry.Property(e => e.Id).CurrentValue);
Assert.False(categoryEntry.Property(e => e.Id).IsTemporary);

product.Category = null;

productEntry = context.Entry(product);
Assert.Equal(EntityState.Modified, productEntry.State);

Assert.Equal(1, product.Id);
Assert.Equal(1, productEntry.Property(e => e.Id).CurrentValue);
Assert.False(productEntry.Property(e => e.Id).IsTemporary);

Assert.Null(product.CategoryId);
Assert.Null(productEntry.Property(e => e.CategoryId).CurrentValue);
Assert.False(productEntry.Property(e => e.CategoryId).IsTemporary);

categoryEntry = context.Entry(category);
Assert.Equal(EntityState.Unchanged, categoryEntry.State);
Assert.Equal(1, category.Id);
Assert.Equal(1, categoryEntry.Property(e => e.Id).CurrentValue);

context.SaveChanges();

productEntry = context.Entry(product);
Assert.Equal(EntityState.Unchanged, productEntry.State);

Assert.Equal(1, product.Id);
Assert.Null(product.CategoryId);
Assert.False(productEntry.Property(e => e.Id).IsTemporary);

Assert.Equal(1, productEntry.Property(e => e.Id).CurrentValue);
Assert.Null(productEntry.Property(e => e.CategoryId).CurrentValue);
Assert.False(productEntry.Property(e => e.CategoryId).IsTemporary);

categoryEntry = context.Entry(category);
Assert.Equal(EntityState.Unchanged, categoryEntry.State);
Assert.Equal(1, category.Id);
Assert.Equal(1, categoryEntry.Property(e => e.Id).CurrentValue);
Assert.False(categoryEntry.Property(e => e.Id).IsTemporary);
});
}

protected class OptionalProduct
{
public int Id { get; set; }
public int? CategoryId { get; set; }
public OptionalCategory Category { get; set; }
}

protected class OptionalCategory
{
public int Id { get; set; }
}

[ConditionalFact]
public virtual void Identity_property_on_Added_entity_with_temporary_value_gets_value_from_store_even_if_same()
{
Expand Down Expand Up @@ -1535,6 +1658,8 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
b.Property(e => e.NullableAsNonNullable).HasField("_nullableAsNonNullable").ValueGeneratedOnAddOrUpdate();
b.Property(e => e.NonNullableAsNullable).HasField("_nonNullableAsNullable").ValueGeneratedOnAddOrUpdate();
});

modelBuilder.Entity<OptionalProduct>();
}
}
}
Expand Down
Loading