Skip to content
Draft
Show file tree
Hide file tree
Changes from 5 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
4 changes: 3 additions & 1 deletion src/EFCore/ChangeTracking/Internal/InternalEntryBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1477,7 +1477,9 @@ public virtual void AcceptChanges()
var storeGeneratedIndex = property.GetStoreGeneratedIndex();
if (storeGeneratedIndex != -1
&& _stateData.IsPropertyFlagged(property.GetIndex(), PropertyFlag.IsStoreGenerated)
&& _storeGeneratedValues.TryGetValue(storeGeneratedIndex, out var value))
&& _storeGeneratedValues.TryGetValue(storeGeneratedIndex, out var value)
&& (property.DeclaringType is not IComplexType complexType
|| GetCurrentValue(complexType.ComplexProperty) != null))
{
this[property] = value;
}
Expand Down
12 changes: 11 additions & 1 deletion src/EFCore/Metadata/Internal/PropertyAccessorsFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,18 @@ public static Expression CreateMemberAccess(
break;
}
}
else
else if (!fromDeclaringType
|| !addNullCheck
|| property?.DeclaringType is not IRuntimeComplexType declaringComplexType
|| instanceExpression.Type.IsValueType
|| declaringComplexType.ClrType.IsValueType)
{
// Disable null check for all cases except:
// - fromDeclaringType is true AND
// - addNullCheck is true AND
// - property is declared in a complex type AND
// - instance is a reference type AND
// - declaring complex type is a reference type
addNullCheck = false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,102 @@ public class ComplexTypesTrackingSqlServerTest(
ITestOutputHelper testOutputHelper)
: ComplexTypesTrackingSqlServerTestBase<ComplexTypesTrackingSqlServerTest.SqlServerFixture>(fixture, testOutputHelper)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Microsoft.EntityFrameworkCore.Scaffolding.CompiledModelSqlServerTest.ComplexTypes and other implementations of the ComplexTypes test are failing with

System.AggregateException : Differences found in 2 files (Difference found in PrincipalBaseEntityType.cs) (Difference found in PrincipalDerivedEntityType.cs)\r\n---- System.Exception : Difference found in PrincipalBaseEntityType.cs\r\n-------- Assert.Equal() Failure: Strings differ\r\n                                  ↓ (pos 76408)\r\nExpected: ···"edType instance) => instance.Details,\r\n  "···\r\nActual:   ···"edType instance) => (instance == null ? d"···\r\n                                  ↑ (pos 76408)\r\n---- System.Exception : Difference found in PrincipalDerivedEntityType.cs\r\n-------- Assert.Equal() Failure: Strings differ\r\n                                  ↓ (pos 9682)\r\nExpected: ···"edType instance) => instance.Details,\r\n  "···\r\nActual:   ···"edType instance) => (instance == null ? d"···\r\n                                  ↑ (pos 9682)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4e16643. Added check for ComplexProperty.IsNullable so null checks are only generated for nullable complex properties, not required ones. This prevents unnecessary null checks in compiled models while still protecting against NRE for nullable complex properties in TPH scenarios.

[ConditionalFact]
public virtual async Task Can_read_original_values_with_TPH_shared_complex_property_column_null()
{
await using var context = CreateContext();

await context.Database.CreateExecutionStrategy().ExecuteAsync(
context,
async context =>
{
await using var transaction = await context.Database.BeginTransactionAsync();

context.Add(new InheritedItem1 { Name = "Item1" });
await context.SaveChangesAsync();

context.ChangeTracker.Clear();
var item = await context.Set<InheritedItemBase>().Where(i => i.Name == "Item1").FirstAsync();
var entry = context.ChangeTracker.Entries().First();
var originalItem = (InheritedItemBase)entry.OriginalValues.ToObject();

Assert.NotNull(originalItem);
Assert.Equal("Item1", originalItem.Name);
});
}

[ConditionalFact]
public virtual async Task Can_read_original_values_with_TPH_shared_complex_property_column_with_value()
{
await using var context = CreateContext();

await context.Database.CreateExecutionStrategy().ExecuteAsync(
context,
async context =>
{
await using var transaction = await context.Database.BeginTransactionAsync();

context.Add(new InheritedItem1 { Name = "Item1", SharedPrice = new SharedPrice { Amount = "10.99" } });
await context.SaveChangesAsync();

context.ChangeTracker.Clear();
var item = await context.Set<InheritedItem1>().Where(i => i.Name == "Item1").FirstAsync();
var entry = context.ChangeTracker.Entries().First();
var originalItem = (InheritedItemBase)entry.OriginalValues.ToObject();

Assert.NotNull(originalItem);
Assert.Equal("Item1", originalItem.Name);
var item1 = Assert.IsType<InheritedItem1>(originalItem);
Assert.NotNull(item1.SharedPrice);
Assert.Equal("10.99", item1.SharedPrice.Amount);
});
}

public class SqlServerFixture : SqlServerFixtureBase
{
protected override string StoreName
=> nameof(ComplexTypesTrackingSqlServerTest);

protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)
{
base.OnModelCreating(modelBuilder, context);

modelBuilder.Entity<InheritedItemBase>().HasDiscriminator<string>("Discriminator")
.HasValue<InheritedItem1>("Item1")
.HasValue<InheritedItem2>("Item2");

modelBuilder.Entity<InheritedItem1>().ComplexProperty(
x => x.SharedPrice,
p => p.Property(a => a.Amount).HasColumnName("SharedPriceAmount"));

modelBuilder.Entity<InheritedItem2>().ComplexProperty(
x => x.SharedPrice,
p => p.Property(a => a.Amount).HasColumnName("SharedPriceAmount"));
}
}
}

public abstract class InheritedItemBase
{
public int Id { get; set; }
public required string Name { get; set; }
}

public class InheritedItem1 : InheritedItemBase
{
public SharedPrice? SharedPrice { get; set; }
}

public class InheritedItem2 : InheritedItemBase
{
public SharedPrice? SharedPrice { get; set; }
}

public class SharedPrice
{
public required string Amount { get; init; }
}

public class ComplexTypesTrackingProxiesSqlServerTest(
ComplexTypesTrackingProxiesSqlServerTest.SqlServerFixture fixture,
ITestOutputHelper testOutputHelper)
Expand Down
Loading