Skip to content

Commit 2832891

Browse files
CopilotAndriySvyryd
Copilot
and
AndriySvyryd
committed
Fix optional complex property default values tracking
Fixes #37386 Co-authored-by: AndriySvyryd <6539701+AndriySvyryd@users.noreply.github.com>
1 parent fc65c2d commit 2832891

File tree

5 files changed

+178
-21
lines changed

5 files changed

+178
-21
lines changed

‎src/EFCore/ChangeTracking/Internal/ChangeDetector.cs‎

Lines changed: 73 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -46,24 +46,38 @@ public virtual void PropertyChanged(IInternalEntry entry, IPropertyBase property
4646
return;
4747
}
4848

49-
if (propertyBase is IProperty property)
49+
switch (propertyBase)
5050
{
51-
if (entry.EntityState is not EntityState.Deleted)
52-
{
53-
entry.SetPropertyModified(property, setModified);
54-
}
55-
else
56-
{
57-
ThrowIfKeyChanged(entry, property);
58-
}
51+
case IProperty property:
52+
if (entry.EntityState is not EntityState.Deleted)
53+
{
54+
entry.SetPropertyModified(property, setModified);
55+
}
56+
else
57+
{
58+
ThrowIfKeyChanged(entry, property);
59+
}
5960

60-
DetectKeyChange(entry, property);
61-
}
62-
else if (propertyBase.GetRelationshipIndex() != -1
63-
&& propertyBase is INavigationBase navigation)
64-
{
65-
DetectNavigationChange(
66-
entry as InternalEntityEntry ?? throw new UnreachableException("Complex type entry with a navigation"), navigation);
61+
DetectKeyChange(entry, property);
62+
break;
63+
64+
case IComplexProperty { IsCollection: false } complexProperty:
65+
// TODO: This requires notification change tracking for complex types
66+
// Issue #36175
67+
if (entry.EntityState is not EntityState.Deleted
68+
&& setModified
69+
&& entry is InternalEntryBase entryBase
70+
&& complexProperty.IsNullable
71+
&& complexProperty.GetOriginalValueIndex() >= 0)
72+
{
73+
DetectComplexPropertyChange(entryBase, complexProperty);
74+
}
75+
break;
76+
77+
case INavigationBase navigation when propertyBase.GetRelationshipIndex() != -1:
78+
DetectNavigationChange(
79+
entry as InternalEntityEntry ?? throw new UnreachableException("Complex type entry with a navigation"), navigation);
80+
break;
6781
}
6882
}
6983

@@ -292,11 +306,54 @@ private bool LocalDetectChanges(InternalEntryBase entry)
292306
changesFound = true;
293307
}
294308
}
309+
else if (complexProperty.IsNullable && complexProperty.GetOriginalValueIndex() >= 0)
310+
{
311+
if (DetectComplexPropertyChange(entry, complexProperty))
312+
{
313+
changesFound = true;
314+
}
315+
}
295316
}
296317

297318
return changesFound;
298319
}
299320

321+
/// <summary>
322+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
323+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
324+
/// any release. You should only use it directly in your code with extreme caution and knowing that
325+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
326+
/// </summary>
327+
public virtual bool DetectComplexPropertyChange(InternalEntryBase entry, IComplexProperty complexProperty)
328+
{
329+
Check.DebugAssert(!complexProperty.IsCollection, $"Expected {complexProperty.Name} to not be a collection.");
330+
331+
var currentValue = entry[complexProperty];
332+
var originalValue = entry.GetOriginalValue(complexProperty);
333+
334+
if ((currentValue is null) != (originalValue is null))
335+
{
336+
// If it changed from null to non-null, mark all inner properties as modified
337+
// to ensure the entity is detected as modified and the complex type properties are persisted
338+
if (currentValue is not null)
339+
{
340+
foreach (var innerProperty in complexProperty.ComplexType.GetFlattenedProperties())
341+
{
342+
// Only mark properties that are tracked and can be modified
343+
if (innerProperty.GetOriginalValueIndex() >= 0
344+
&& innerProperty.GetAfterSaveBehavior() == PropertySaveBehavior.Save)
345+
{
346+
entry.SetPropertyModified(innerProperty);
347+
}
348+
}
349+
}
350+
351+
return true;
352+
}
353+
354+
return false;
355+
}
356+
300357
/// <summary>
301358
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
302359
/// the same compatibility standards as public APIs. It may be changed or removed without notice in

‎test/EFCore.Cosmos.FunctionalTests/CosmosComplexTypesTrackingTest.cs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,12 @@ protected override Task TrackAndSaveTest<TEntity>(EntityState state, bool async,
111111
return base.TrackAndSaveTest(state, async, createPub);
112112
}
113113

114-
protected override async Task ExecuteWithStrategyInTransactionAsync(Func<DbContext, Task> testOperation, Func<DbContext, Task>? nestedTestOperation1 = null, Func<DbContext, Task>? nestedTestOperation2 = null)
114+
public override Task Can_save_default_values_in_optional_complex_property_with_multiple_properties(bool async)
115+
// Optional complex properties are not supported on Cosmos
116+
// See https://github.com/dotnet/efcore/issues/31253
117+
=> Task.CompletedTask;
118+
119+
protected override async Task ExecuteWithStrategyInTransactionAsync(Func<DbContext, Task> testOperation, Func<DbContext, Task>? nestedTestOperation1 = null, Func<DbContext, Task>? nestedTestOperation2 = null, Func<DbContext, Task>? nestedTestOperation3 = null)
115120
{
116121
using var c = CreateContext();
117122
await c.Database.CreateExecutionStrategy().ExecuteAsync(
@@ -141,6 +146,16 @@ await c.Database.CreateExecutionStrategy().ExecuteAsync(
141146
{
142147
await nestedTestOperation2(innerContext2);
143148
}
149+
150+
if (nestedTestOperation3 == null)
151+
{
152+
return;
153+
}
154+
155+
using (var innerContext3 = CreateContext())
156+
{
157+
await nestedTestOperation3(innerContext3);
158+
}
144159
});
145160
}
146161

@@ -161,6 +176,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
161176
modelBuilder.Entity<PubWithArrayCollections>().HasPartitionKey(x => x.Id);
162177
modelBuilder.Entity<PubWithRecordArrayCollections>().HasPartitionKey(x => x.Id);
163178
modelBuilder.Entity<PubWithPropertyBagCollections>().HasPartitionKey(x => x.Id);
179+
modelBuilder.Entity<EntityWithOptionalMultiPropComplex>().HasPartitionKey(x => x.Id);
164180
if (!UseProxies)
165181
{
166182
modelBuilder.Entity<FieldPub>().HasPartitionKey(x => x.Id);

‎test/EFCore.InMemory.FunctionalTests/ComplexTypesTrackingInMemoryTest.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,24 @@ public class ComplexTypesTrackingInMemoryTest(ComplexTypesTrackingInMemoryTest.I
99
protected override async Task ExecuteWithStrategyInTransactionAsync(
1010
Func<DbContext, Task> testOperation,
1111
Func<DbContext, Task> nestedTestOperation1 = null,
12-
Func<DbContext, Task> nestedTestOperation2 = null)
12+
Func<DbContext, Task> nestedTestOperation2 = null,
13+
Func<DbContext, Task> nestedTestOperation3 = null)
1314
{
1415
try
1516
{
16-
await base.ExecuteWithStrategyInTransactionAsync(testOperation, nestedTestOperation1, nestedTestOperation2);
17+
await base.ExecuteWithStrategyInTransactionAsync(testOperation, nestedTestOperation1, nestedTestOperation2, nestedTestOperation3);
1718
}
1819
finally
1920
{
2021
await Fixture.ReseedAsync();
2122
}
2223
}
2324

25+
public override Task Can_save_default_values_in_optional_complex_property_with_multiple_properties(bool async)
26+
// InMemory provider has issues with complex type query compilation
27+
// See https://github.com/dotnet/efcore/issues/31464
28+
=> Task.CompletedTask;
29+
2430
public class InMemoryFixture : FixtureBase
2531
{
2632
protected override ITestStoreFactory TestStoreFactory

‎test/EFCore.Specification.Tests/ComplexTypesTrackingTestBase.cs‎

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2053,10 +2053,11 @@ protected static EntityEntry<TEntity> TrackFromQuery<TEntity>(DbContext context,
20532053
protected virtual Task ExecuteWithStrategyInTransactionAsync(
20542054
Func<DbContext, Task> testOperation,
20552055
Func<DbContext, Task>? nestedTestOperation1 = null,
2056-
Func<DbContext, Task>? nestedTestOperation2 = null)
2056+
Func<DbContext, Task>? nestedTestOperation2 = null,
2057+
Func<DbContext, Task>? nestedTestOperation3 = null)
20572058
=> TestHelpers.ExecuteWithStrategyInTransactionAsync(
20582059
CreateContext, UseTransaction,
2059-
testOperation, nestedTestOperation1, nestedTestOperation2);
2060+
testOperation, nestedTestOperation1, nestedTestOperation2, nestedTestOperation3);
20602061

20612062
protected virtual void UseTransaction(DatabaseFacade facade, IDbContextTransaction transaction)
20622063
{
@@ -2397,6 +2398,11 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
23972398
});
23982399
});
23992400
});
2401+
2402+
modelBuilder.Entity<EntityWithOptionalMultiPropComplex>(b =>
2403+
{
2404+
b.ComplexProperty(e => e.ComplexProp);
2405+
});
24002406
}
24012407
}
24022408

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

‎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)