Merge pull request #19911 from dotnet/DetachmentSyndrome0213

[release/3.1] Cherry-picking commits for for closely related detach/delete issues
dotnet · Feb 15, 2020 · 79a546f · 79a546f
2 parents d463ed4 + 859588d
commit 79a546f
Show file tree

Hide file tree

Showing 8 changed files with 825 additions and 128 deletions.
diff --git a/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs b/src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
@@ -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

diff --git a/src/EFCore/ChangeTracking/Internal/StateManager.cs b/src/EFCore/ChangeTracking/Internal/StateManager.cs
@@ -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)
@@ -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)
                             {
@@ -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)
                             {

diff --git a/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs b/test/EFCore.Specification.Tests/PropertyValuesTestBase.cs
@@ -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]

diff --git a/test/EFCore.Specification.Tests/ProxyGraphUpdatesTestBase.cs b/test/EFCore.Specification.Tests/ProxyGraphUpdatesTestBase.cs
@@ -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);
                 });
         }

diff --git a/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs b/test/EFCore.Specification.Tests/StoreGeneratedTestBase.cs
@@ -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()
         {
@@ -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>();
             }
         }
     }