Skip to content

Commit 1b84b81

Browse files
CopilotAndriySvyryd
authored andcommitted
Fix optional complex property default values tracking
Fixes #37386
1 parent aa04e25 commit 1b84b81

File tree

3 files changed

+134
-2
lines changed

3 files changed

+134
-2
lines changed

src/EFCore/ChangeTracking/Internal/ChangeDetector.cs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,19 @@ public virtual void PropertyChanged(IInternalEntry entry, IPropertyBase property
5959

6060
DetectKeyChange(entry, property);
6161
}
62+
else if (propertyBase is IComplexProperty { IsCollection: false } complexProperty)
63+
{
64+
// TODO: This requires notification change tracking for complex types
65+
// Issue #36175
66+
if (entry.EntityState is not EntityState.Deleted
67+
&& setModified
68+
&& entry is InternalEntryBase entryBase
69+
&& complexProperty.IsNullable
70+
&& complexProperty.GetOriginalValueIndex() >= 0)
71+
{
72+
DetectComplexPropertyChange(entryBase, complexProperty);
73+
}
74+
}
6275
else if (propertyBase.GetRelationshipIndex() != -1
6376
&& propertyBase is INavigationBase navigation)
6477
{
@@ -292,11 +305,51 @@ private bool LocalDetectChanges(InternalEntryBase entry)
292305
changesFound = true;
293306
}
294307
}
308+
else if (complexProperty.IsNullable && complexProperty.GetOriginalValueIndex() >= 0)
309+
{
310+
if (DetectComplexPropertyChange(entry, complexProperty))
311+
{
312+
changesFound = true;
313+
}
314+
}
295315
}
296316

297317
return changesFound;
298318
}
299319

320+
/// <summary>
321+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
322+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
323+
/// any release. You should only use it directly in your code with extreme caution and knowing that
324+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
325+
/// </summary>
326+
public bool DetectComplexPropertyChange(InternalEntryBase entry, IComplexProperty complexProperty)
327+
{
328+
Check.DebugAssert(!complexProperty.IsCollection, $"Expected {complexProperty.Name} to not be a collection.");
329+
330+
var currentValue = entry[complexProperty];
331+
var originalValue = entry.GetOriginalValue(complexProperty);
332+
333+
if ((currentValue == null) != (originalValue == null))
334+
{
335+
// If it changed from null to non-null, mark all inner properties as modified
336+
// to ensure the entity is detected as modified and the complex property is persisted
337+
if (currentValue != null)
338+
{
339+
foreach (var innerProperty in complexProperty.ComplexType.GetFlattenedProperties())
340+
{
341+
// Mark each property as modified. This will cause the entity state to transition to Modified.
342+
// Even if the property values are defaults, marking them as modified ensures they are persisted.
343+
entry.SetPropertyModified(innerProperty, isModified: true, changeState: true, acceptChanges: false);
344+
}
345+
}
346+
347+
return true;
348+
}
349+
350+
return false;
351+
}
352+
300353
/// <summary>
301354
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
302355
/// the same compatibility standards as public APIs. It may be changed or removed without notice in

test/EFCore.Specification.Tests/ComplexTypesTrackingTestBase.cs

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.ComponentModel;
45
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
56

67
namespace Microsoft.EntityFrameworkCore;
@@ -2053,10 +2054,11 @@ protected static EntityEntry<TEntity> TrackFromQuery<TEntity>(DbContext context,
20532054
protected virtual Task ExecuteWithStrategyInTransactionAsync(
20542055
Func<DbContext, Task> testOperation,
20552056
Func<DbContext, Task>? nestedTestOperation1 = null,
2056-
Func<DbContext, Task>? nestedTestOperation2 = null)
2057+
Func<DbContext, Task>? nestedTestOperation2 = null,
2058+
Func<DbContext, Task>? nestedTestOperation3 = null)
20572059
=> TestHelpers.ExecuteWithStrategyInTransactionAsync(
20582060
CreateContext, UseTransaction,
2059-
testOperation, nestedTestOperation1, nestedTestOperation2);
2061+
testOperation, nestedTestOperation1, nestedTestOperation2, nestedTestOperation3);
20602062

20612063
protected virtual void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
20622064
{
@@ -2397,6 +2399,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
23972399
});
23982400
});
23992401
});
2402+
2403+
modelBuilder.Entity<EntityWithOptionalMultiPropComplex>(b =>
2404+
{
2405+
b.ComplexProperty(e => e.ComplexProp);
2406+
});
24002407
}
24012408
}
24022409

@@ -4373,4 +4380,72 @@ protected static FieldPubWithReadonlyStructCollections CreateFieldCollectionPubW
43734380
],
43744381
FeaturedTeam = new TeamReadonlyStruct("Not In This Lifetime", ["Slash", "Axl"])
43754382
};
4383+
4384+
[ConditionalTheory(), InlineData(false), InlineData(true)]
4385+
public virtual async Task Can_save_default_values_in_optional_complex_property_with_multiple_properties(bool async)
4386+
{
4387+
await ExecuteWithStrategyInTransactionAsync(
4388+
async context =>
4389+
{
4390+
var entity = Fixture.UseProxies
4391+
? context.CreateProxy<EntityWithOptionalMultiPropComplex>()
4392+
: new EntityWithOptionalMultiPropComplex();
4393+
4394+
entity.Id = Guid.NewGuid();
4395+
entity.ComplexProp = null;
4396+
4397+
_ = async ? await context.AddAsync(entity) : context.Add(entity);
4398+
_ = async ? await context.SaveChangesAsync() : context.SaveChanges();
4399+
4400+
Assert.Null(entity.ComplexProp);
4401+
},
4402+
async context =>
4403+
{
4404+
var entity = async
4405+
? await context.Set<EntityWithOptionalMultiPropComplex>().SingleAsync()
4406+
: context.Set<EntityWithOptionalMultiPropComplex>().Single();
4407+
4408+
Assert.Null(entity.ComplexProp);
4409+
4410+
// Set the complex property with default values
4411+
entity.ComplexProp = new MultiPropComplex
4412+
{
4413+
IntValue = 0,
4414+
BoolValue = false,
4415+
DateValue = default
4416+
};
4417+
4418+
_ = async ? await context.SaveChangesAsync() : context.SaveChanges();
4419+
4420+
Assert.NotNull(entity.ComplexProp);
4421+
Assert.Equal(0, entity.ComplexProp.IntValue);
4422+
Assert.False(entity.ComplexProp.BoolValue);
4423+
Assert.Equal(default, entity.ComplexProp.DateValue);
4424+
},
4425+
async context =>
4426+
{
4427+
var entity = async
4428+
? await context.Set<EntityWithOptionalMultiPropComplex>().SingleAsync()
4429+
: context.Set<EntityWithOptionalMultiPropComplex>().Single();
4430+
4431+
// Complex types with more than one property should materialize even with default values
4432+
Assert.NotNull(entity.ComplexProp);
4433+
Assert.Equal(0, entity.ComplexProp.IntValue);
4434+
Assert.False(entity.ComplexProp.BoolValue);
4435+
Assert.Equal(default, entity.ComplexProp.DateValue);
4436+
});
4437+
}
4438+
4439+
public class EntityWithOptionalMultiPropComplex
4440+
{
4441+
public virtual Guid Id { get; set; }
4442+
public virtual MultiPropComplex? ComplexProp { get; set; }
4443+
}
4444+
4445+
public class MultiPropComplex
4446+
{
4447+
public int IntValue { get; set; }
4448+
public bool BoolValue { get; set; }
4449+
public DateTimeOffset DateValue { get; set; }
4450+
}
43764451
}

test/EFCore.SqlServer.FunctionalTests/ComplexTypesTrackingSqlServerTest.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,10 @@ public override void Can_write_original_values_for_properties_of_complex_propert
338338
{
339339
}
340340

341+
// Issue #36175: Complex types with notification change tracking are not supported
342+
public override Task Can_save_default_values_in_optional_complex_property_with_multiple_properties(bool async)
343+
=> Task.CompletedTask;
344+
341345
public class SqlServerFixture : SqlServerFixtureBase
342346
{
343347
protected override string StoreName

0 commit comments

Comments
 (0)