Fix persisting null optional complex property with default values

src/EFCore/ChangeTracking/Internal/ChangeDetector.cs

@@ -64,10 +64,10 @@ public virtual void PropertyChanged(IInternalEntry entry, IPropertyBase property
             case IComplexProperty { IsCollection: false } complexProperty:
                 // TODO: This requires notification change tracking for complex types
                 // Issue #36175
-                if (entry.EntityState is not EntityState.Deleted 
-                    && setModified 
+                if (entry.EntityState is not EntityState.Deleted
+                    && setModified
                     && entry is InternalEntryBase entryBase
-                    && complexProperty.IsNullable 
+                    && complexProperty.IsNullable
                     && complexProperty.GetOriginalValueIndex() >= 0)
                 {
                     DetectComplexPropertyChange(entryBase, complexProperty);
@@ -344,19 +344,16 @@ public virtual bool DetectComplexPropertyChange(InternalEntryBase entry, IComple
 
         if ((currentValue is null) != (originalValue is null))
         {
-            // If it changed from null to non-null, mark all inner properties as modified
+            // If it changed from null to non-null or from non-null to null, mark all inner properties as modified
             // to ensure the entity is detected as modified and the complex type properties are persisted
-            if (currentValue is not null)
+            foreach (var innerProperty in complexProperty.ComplexType.GetFlattenedProperties())
             {
-                foreach (var innerProperty in complexProperty.ComplexType.GetFlattenedProperties())
+                // Only mark properties that are tracked, can be modified, and are loaded
+                if (innerProperty.GetOriginalValueIndex() >= 0
+                    && innerProperty.GetAfterSaveBehavior() == PropertySaveBehavior.Save
+                    && entry.IsLoaded(innerProperty))
                 {
-                    // Only mark properties that are tracked, can be modified, and are loaded
-                    if (innerProperty.GetOriginalValueIndex() >= 0
-                        && innerProperty.GetAfterSaveBehavior() == PropertySaveBehavior.Save
-                        && entry.IsLoaded(innerProperty))
-                    {
-                        entry.SetPropertyModified(innerProperty);
-                    }
+                    entry.SetPropertyModified(innerProperty);
                 }
             }
 

test/EFCore.Cosmos.FunctionalTests/CosmosComplexTypesTrackingTest.cs

@@ -121,47 +121,62 @@ public override Task Can_save_default_values_in_optional_complex_property_with_m
             ? base.Can_save_default_values_in_optional_complex_property_with_multiple_properties(async)
             : throw SkipException.ForSkip("Cosmos does not support synchronous operations.");
 
+    public override Task Can_null_complex_property_with_default_values_and_multiple_properties(bool async)
+        => async
+            ? base.Can_null_complex_property_with_default_values_and_multiple_properties(async)
+            : throw SkipException.ForSkip("Cosmos does not support synchronous operations.");
+
     protected override async Task ExecuteWithStrategyInTransactionAsync(Func<DbContext, Task> testOperation, Func<DbContext, Task>? nestedTestOperation1 = null, Func<DbContext, Task>? nestedTestOperation2 = null, Func<DbContext, Task>? nestedTestOperation3 = null)
     {
         using var c = CreateContext();
-        await c.Database.CreateExecutionStrategy().ExecuteAsync(
-            c, async context =>
-            {
-                using (var innerContext = CreateContext())
-                {
-                    await testOperation(innerContext);
-                }
-
-                if (nestedTestOperation1 == null)
-                {
-                    return;
-                }
-
-                using (var innerContext1 = CreateContext())
-                {
-                    await nestedTestOperation1(innerContext1);
-                }
-
-                if (nestedTestOperation2 == null)
-                {
-                    return;
-                }
-
-                using (var innerContext2 = CreateContext())
-                {
-                    await nestedTestOperation2(innerContext2);
-                }
-
-                if (nestedTestOperation3 == null)
-                {
-                    return;
-                }
-
-                using (var innerContext3 = CreateContext())
+        try
+        {
+            await c.Database.CreateExecutionStrategy().ExecuteAsync(
+                c,
+                async context =>
                 {
-                    await nestedTestOperation3(innerContext3);
+                    using (var innerContext = CreateContext())
+                    {
+                        await testOperation(innerContext);
+                    }
+
+                    if (nestedTestOperation1 == null)
+                    {
+                        return;
+                    }
+
+                    using (var innerContext1 = CreateContext())
+                    {
+                        await nestedTestOperation1(innerContext1);
+                    }
+
+                    if (nestedTestOperation2 == null)
+                    {
+                        return;
+                    }
+
+                    using (var innerContext2 = CreateContext())
+                    {
+                        await nestedTestOperation2(innerContext2);
+                    }
+
+                    if (nestedTestOperation3 == null)
+                    {
+                        return;
+                    }
+
+                    using (var innerContext3 = CreateContext())
+                    {
+                        await nestedTestOperation3(innerContext3);
+                    }
                 }
-            });
+            );
+        }
+        finally
+        {
+            // Cosmos does not support rolling back transactions, so clean the database instead
+            await Fixture.TestStore.CleanAsync(c);
+        }
     }
 
     public class CosmosFixture : FixtureBase

test/EFCore.InMemory.FunctionalTests/ComplexTypesTrackingInMemoryTest.cs

@@ -27,6 +27,11 @@ public override Task Can_save_default_values_in_optional_complex_property_with_m
         // See https://github.com/dotnet/efcore/issues/31464
         => Task.CompletedTask;
 
+    public override Task Can_null_complex_property_with_default_values_and_multiple_properties(bool async)
+        // InMemory provider has issues with complex type query compilation and materialization
+        // See https://github.com/dotnet/efcore/issues/31464
+        => Task.CompletedTask;
+
     // Complex type collections are not supported in InMemory provider
     // See https://github.com/dotnet/efcore/issues/31464
     public override Task Can_change_state_from_Deleted_with_complex_collection(EntityState newState, bool async)

test/EFCore.Specification.Tests/ComplexTypesTrackingTestBase.cs

@@ -2014,6 +2014,56 @@ public virtual void Throws_when_accessing_complex_entries_using_incorrect_cardin
             Assert.Throws<InvalidOperationException>(() => entry.ComplexCollection(e => (IList<Team>)e.FeaturedTeam)).Message);
     }
 
+    [ConditionalTheory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public virtual async Task Can_null_complex_property_with_default_values_and_multiple_properties(bool async)
+    {
+        await ExecuteWithStrategyInTransactionAsync(
+            async context =>
+            {
+                var entity = Fixture.UseProxies
+                    ? context.CreateProxy<EntityWithOptionalMultiPropComplex>()
+                    : new EntityWithOptionalMultiPropComplex();
+
+                entity.Id = Guid.NewGuid();
+                // Set the complex property with default values
+                entity.ComplexProp = new MultiPropComplex
+                {
+                    IntValue = 0,
+                    BoolValue = false,
+                    DateValue = default,
+                };
+
+                _ = async ? await context.AddAsync(entity) : context.Add(entity);
+                _ = async ? await context.SaveChangesAsync() : context.SaveChanges();
+
+                Assert.NotNull(entity.ComplexProp);
+            },
+            async context =>
+            {
+                var entity = async
+                    ? await context.Set<EntityWithOptionalMultiPropComplex>().SingleAsync()
+                    : context.Set<EntityWithOptionalMultiPropComplex>().Single();
+
+                Assert.NotNull(entity.ComplexProp);
+
+                entity.ComplexProp = null;
+
+                _ = async ? await context.SaveChangesAsync() : context.SaveChanges();
+
+                Assert.Null(entity.ComplexProp);
+            },
+            async context =>
+            {
+                var entity = async
+                    ? await context.Set<EntityWithOptionalMultiPropComplex>().SingleAsync()
+                    : context.Set<EntityWithOptionalMultiPropComplex>().Single();
+
+                Assert.Null(entity.ComplexProp);
+            });
+    }
+
     protected void AssertPropertyValues(EntityEntry entry)
     {
         Assert.Equal("The FBI", entry.Property("Name").CurrentValue);

test/EFCore.SqlServer.FunctionalTests/ComplexTypesTrackingSqlServerTest.cs

@@ -441,6 +441,10 @@ public override void Can_write_original_values_for_properties_of_complex_propert
     public override Task Can_save_default_values_in_optional_complex_property_with_multiple_properties(bool async)
         => Task.CompletedTask;
 
+    // Issue #36175: Complex types with notification change tracking are not supported
+    public override Task Can_null_complex_property_with_default_values_and_multiple_properties(bool async)
+        => Task.CompletedTask;
+
     // Fields can't be proxied
     public override Task Can_change_state_from_Deleted_with_complex_field_collection(EntityState newState, bool async)
         => Task.CompletedTask;