diff --git a/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs b/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs index 5232106b24f..7f7f3f39891 100644 --- a/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs +++ b/src/EFCore/ChangeTracking/Internal/ChangeDetector.cs @@ -344,6 +344,21 @@ public virtual bool DetectComplexPropertyChange(InternalEntryBase entry, IComple if ((currentValue is null) != (originalValue is null)) { + // Set the discriminator value for the complex type when transitioning from null to non-null or vice versa. + // The discriminator is a shadow property whose value needs to be updated to reflect the new state. + var discriminatorProperty = complexProperty.ComplexType.FindDiscriminatorProperty(); + if (discriminatorProperty != null) + { + if (currentValue is not null) + { + entry[discriminatorProperty] = complexProperty.ComplexType.GetDiscriminatorValue(); + } + else if (discriminatorProperty.IsShadowProperty()) + { + entry[discriminatorProperty] = discriminatorProperty.ClrType.GetDefaultValue(); + } + } + // 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 foreach (var innerProperty in complexProperty.ComplexType.GetFlattenedProperties()) diff --git a/src/EFCore/Metadata/Internal/InternalComplexTypeBuilder.cs b/src/EFCore/Metadata/Internal/InternalComplexTypeBuilder.cs index aa03bb7c0ff..c7e467ed396 100644 --- a/src/EFCore/Metadata/Internal/InternalComplexTypeBuilder.cs +++ b/src/EFCore/Metadata/Internal/InternalComplexTypeBuilder.cs @@ -500,6 +500,24 @@ public virtual bool CanSetServiceOnlyConstructorBinding( => configurationSource.Overrides(Metadata.GetServiceOnlyConstructorBindingConfigurationSource()) || Metadata.ServiceOnlyConstructorBinding == constructorBinding; + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected override InternalPropertyBuilder? GetOrCreateDiscriminatorProperty( + Type? type, + string? name, + MemberInfo? memberInfo, + ConfigurationSource configurationSource) + { + var builder = base.GetOrCreateDiscriminatorProperty(type, name, memberInfo, configurationSource); + builder?.AfterSave(PropertySaveBehavior.Save, ConfigurationSource.Convention); + + return builder; + } + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs b/src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs index 0f4c0c5602e..f1722590db0 100644 --- a/src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs +++ b/src/EFCore/Query/Internal/ExpressionTreeFuncletizer.cs @@ -2145,7 +2145,9 @@ bool PreserveConvertNode(Expression expression) if (visited != expression) { parameterName = QueryFilterPrefix - + (RemoveConvert(expression) is MemberExpression { Member.Name: var memberName } ? ("__" + memberName) : "__p"); + + (RemoveConvert(expression) is MemberExpression { Member.Name: var memberName } + ? "__" + SanitizeCompilerGeneratedName(memberName) + : "__p"); isContextAccessor = true; // Context accessors (query filters accessing the context) never get constantized diff --git a/test/EFCore.Cosmos.FunctionalTests/Query/AdHocComplexTypeQueryCosmosTest.cs b/test/EFCore.Cosmos.FunctionalTests/Query/AdHocComplexTypeQueryCosmosTest.cs index c27c7efa56b..502e8227722 100644 --- a/test/EFCore.Cosmos.FunctionalTests/Query/AdHocComplexTypeQueryCosmosTest.cs +++ b/test/EFCore.Cosmos.FunctionalTests/Query/AdHocComplexTypeQueryCosmosTest.cs @@ -49,6 +49,11 @@ SELECT VALUE c FROM root c WHERE (c["AllOptionalsComplexType"] = null) OFFSET 0 LIMIT 2 +""", + // + """ +SELECT VALUE c +FROM root c """); } @@ -103,6 +108,115 @@ OFFSET 0 LIMIT 2 """); } + public override async Task Nullable_complex_type_with_discriminator_null_to_non_null_roundtrip() + { + await base.Nullable_complex_type_with_discriminator_null_to_non_null_roundtrip(); + + AssertSql( + """ +SELECT VALUE c +FROM root c +OFFSET 0 LIMIT 2 +""", + // + """ +SELECT VALUE c +FROM root c +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Nullable_complex_type_with_discriminator_non_null_to_null_roundtrip() + { + await base.Nullable_complex_type_with_discriminator_non_null_to_null_roundtrip(); + + AssertSql( + """ +SELECT VALUE c +FROM root c +OFFSET 0 LIMIT 2 +""", + // + """ +SELECT VALUE c +FROM root c +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Nullable_complex_type_with_discriminator_update_non_null_entity_roundtrip() + { + await base.Nullable_complex_type_with_discriminator_update_non_null_entity_roundtrip(); + + AssertSql( + """ +SELECT VALUE c +FROM root c +OFFSET 0 LIMIT 2 +""", + // + """ +SELECT VALUE c +FROM root c +OFFSET 0 LIMIT 2 +"""); + } + + public override async Task Nullable_complex_type_with_discriminator_set_to_different_value() + { + await base.Nullable_complex_type_with_discriminator_set_to_different_value(); + } + + public override async Task Nullable_complex_type_with_discriminator_set_to_null() + { + // On Cosmos, setting the discriminator shadow property to null doesn't affect materialization + // because the complex property's data is still present in the JSON document. + var contextFactory = await InitializeNonSharedTest(); + + Guid entityId; + await using (var context = contextFactory.CreateDbContext()) + { + var entity = new Context38119.EntityType + { + Id = Guid.NewGuid(), + Prop = new Context38119.OptionalComplexProperty { OptionalValue = true } + }; + context.Add(entity); + entityId = entity.Id; + + var discriminatorEntry = context.Entry(entity).ComplexProperty(e => e.Prop).Property("Discriminator"); + Assert.Equal("OptionalComplexProperty", discriminatorEntry.CurrentValue); + discriminatorEntry.CurrentValue = null; + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var entity = await context.Set().SingleAsync(e => e.Id == entityId); + Assert.NotNull(entity.Prop); + Assert.True(entity.Prop.OptionalValue); + } + + } + + public override async Task Nested_nullable_complex_type_with_discriminator_null_to_non_null_roundtrip() + { + await base.Nested_nullable_complex_type_with_discriminator_null_to_non_null_roundtrip(); + + AssertSql( + """ +SELECT VALUE c +FROM root c +OFFSET 0 LIMIT 2 +""", + // + """ +SELECT VALUE c +FROM root c +OFFSET 0 LIMIT 2 +"""); + } + protected override DbContextOptionsBuilder AddNonSharedOptions(DbContextOptionsBuilder builder) => base.AddNonSharedOptions(builder) .ConfigureWarnings(w => w.Ignore(CosmosEventId.NoPartitionKeyDefined)); diff --git a/test/EFCore.Specification.Tests/Query/AdHocComplexTypeQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/AdHocComplexTypeQueryTestBase.cs index 41ab4f94fa5..6f42292c9bc 100644 --- a/test/EFCore.Specification.Tests/Query/AdHocComplexTypeQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/AdHocComplexTypeQueryTestBase.cs @@ -154,13 +154,23 @@ public virtual async Task Optional_complex_type_with_discriminator() return context.SaveChangesAsync(); }); - await using var context = contextFactory.CreateDbContext(); + await using (var context = contextFactory.CreateDbContext()) + { + var complexTypeNull = await context.Set() + .SingleAsync(b => b.AllOptionalsComplexType == null); + Assert.Null(complexTypeNull.AllOptionalsComplexType); - var complexTypeNull = await context.Set().SingleAsync(b => b.AllOptionalsComplexType == null); - Assert.Null(complexTypeNull.AllOptionalsComplexType); + complexTypeNull.AllOptionalsComplexType = + new ContextShadowDiscriminator.AllOptionalsComplexType { OptionalProperty = "New thing" }; + await context.SaveChangesAsync(); + } - complexTypeNull.AllOptionalsComplexType = new ContextShadowDiscriminator.AllOptionalsComplexType { OptionalProperty = "New thing" }; - await context.SaveChangesAsync(); + await using (var context = contextFactory.CreateDbContext()) + { + var entities = await context.Set().ToListAsync(); + Assert.Equal(3, entities.Count); + Assert.All(entities, e => Assert.NotNull(e.AllOptionalsComplexType)); + } } private class ContextShadowDiscriminator(DbContextOptions options) : DbContext(options) @@ -379,6 +389,256 @@ public class OptionalComplexProperty #endregion Issue37337 + #region Issue38119 + + [ConditionalFact] + public virtual async Task Nullable_complex_type_with_discriminator_null_to_non_null_roundtrip() + { + var contextFactory = await InitializeNonSharedTest( + seed: context => + { + context.Add(new Context38119.EntityType { Id = Guid.NewGuid() }); + return context.SaveChangesAsync(); + }); + + await using (var context = contextFactory.CreateDbContext()) + { + var entity = await context.Set().SingleAsync(); + Assert.Null(entity.Prop); + + entity.Prop = new Context38119.OptionalComplexProperty { OptionalValue = true }; + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var entity = await context.Set().SingleAsync(); + Assert.NotNull(entity.Prop); + Assert.True(entity.Prop.OptionalValue); + } + } + + [ConditionalFact] + public virtual async Task Nullable_complex_type_with_discriminator_non_null_to_null_roundtrip() + { + var contextFactory = await InitializeNonSharedTest( + seed: context => + { + context.Add( + new Context38119.EntityType + { + Id = Guid.NewGuid(), + Prop = new Context38119.OptionalComplexProperty { OptionalValue = true } + }); + return context.SaveChangesAsync(); + }); + + await using (var context = contextFactory.CreateDbContext()) + { + var entity = await context.Set().SingleAsync(); + Assert.NotNull(entity.Prop); + + entity.Prop = null; + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var entity = await context.Set().SingleAsync(); + Assert.Null(entity.Prop); + } + } + + [ConditionalFact] + public virtual async Task Nullable_complex_type_with_discriminator_update_non_null_entity_roundtrip() + { + var contextFactory = await InitializeNonSharedTest( + seed: context => + { + context.Add( + new Context38119.EntityType + { + Id = Guid.NewGuid(), + Prop = new Context38119.OptionalComplexProperty { OptionalValue = true } + }); + return context.SaveChangesAsync(); + }); + + await using (var context = contextFactory.CreateDbContext()) + { + var entity = await context.Set().SingleAsync(); + Assert.NotNull(entity.Prop); + Assert.True(entity.Prop.OptionalValue); + + context.Update(entity); + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var entity = await context.Set().SingleAsync(); + Assert.NotNull(entity.Prop); + Assert.True(entity.Prop.OptionalValue); + } + } + + [ConditionalFact] + public virtual async Task Nullable_complex_type_with_discriminator_set_to_different_value() + { + var contextFactory = await InitializeNonSharedTest(); + + Guid entityId; + await using (var context = contextFactory.CreateDbContext()) + { + var entity = new Context38119.EntityType + { + Id = Guid.NewGuid(), + Prop = new Context38119.OptionalComplexProperty { OptionalValue = true } + }; + context.Add(entity); + entityId = entity.Id; + + // Override the discriminator value before saving + var discriminatorEntry = context.Entry(entity).ComplexProperty(e => e.Prop).Property("Discriminator"); + Assert.Equal("OptionalComplexProperty", discriminatorEntry.CurrentValue); + discriminatorEntry.CurrentValue = "SomeOtherValue"; + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + // The discriminator is non-null so the complex property is still materialized + var entity = await context.Set().SingleAsync(e => e.Id == entityId); + Assert.NotNull(entity.Prop); + Assert.True(entity.Prop.OptionalValue); + } + } + + [ConditionalFact] + public virtual async Task Nullable_complex_type_with_discriminator_set_to_null() + { + var contextFactory = await InitializeNonSharedTest(); + + Guid entityId; + await using (var context = contextFactory.CreateDbContext()) + { + var entity = new Context38119.EntityType + { + Id = Guid.NewGuid(), + Prop = new Context38119.OptionalComplexProperty { OptionalValue = true } + }; + context.Add(entity); + entityId = entity.Id; + + // Set discriminator to null before saving, which should cause the complex property to be null on reload + var discriminatorEntry = context.Entry(entity).ComplexProperty(e => e.Prop).Property("Discriminator"); + Assert.Equal("OptionalComplexProperty", discriminatorEntry.CurrentValue); + discriminatorEntry.CurrentValue = null; + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + // With null discriminator, the complex property should be materialized as null + var entity = await context.Set().SingleAsync(e => e.Id == entityId); + Assert.Null(entity.Prop); + } + } + + [ConditionalFact] + public virtual async Task Nested_nullable_complex_type_with_discriminator_null_to_non_null_roundtrip() + { + var contextFactory = await InitializeNonSharedTest( + seed: context => + { + context.Add( + new Context38119Nested.EntityType + { + Id = Guid.NewGuid(), + Outer = new Context38119Nested.OuterComplexProperty { Name = "outer" } + }); + return context.SaveChangesAsync(); + }); + + await using (var context = contextFactory.CreateDbContext()) + { + var entity = await context.Set().SingleAsync(); + Assert.NotNull(entity.Outer); + Assert.Null(entity.Outer.Inner); + + entity.Outer.Inner = new Context38119Nested.InnerComplexProperty { Value = 42 }; + await context.SaveChangesAsync(); + } + + await using (var context = contextFactory.CreateDbContext()) + { + var entity = await context.Set().SingleAsync(); + Assert.NotNull(entity.Outer); + Assert.NotNull(entity.Outer.Inner); + Assert.Equal(42, entity.Outer.Inner.Value); + } + } + + protected class Context38119(DbContextOptions options) : DbContext(options) + { + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + var entity = modelBuilder.Entity(); + entity.HasKey(p => p.Id); + entity.Property(p => p.Id).ValueGeneratedNever(); + + var compl = entity.ComplexProperty(p => p.Prop); + compl.HasDiscriminator(); + } + + public class EntityType + { + public Guid Id { get; set; } + public OptionalComplexProperty? Prop { get; set; } + } + + public class OptionalComplexProperty + { + public bool? OptionalValue { get; set; } + } + } + + private class Context38119Nested(DbContextOptions options) : DbContext(options) + { + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + var entity = modelBuilder.Entity(); + entity.HasKey(p => p.Id); + entity.Property(p => p.Id).ValueGeneratedNever(); + + entity.ComplexProperty( + p => p.Outer, outer => + { + outer.ComplexProperty( + p => p.Inner, inner => inner.HasDiscriminator()); + }); + } + + public class EntityType + { + public Guid Id { get; set; } + public OuterComplexProperty Outer { get; set; } = null!; + } + + public class OuterComplexProperty + { + public string? Name { get; set; } + public InnerComplexProperty? Inner { get; set; } + } + + public class InnerComplexProperty + { + public int? Value { get; set; } + } + } + + #endregion Issue38119 + #region Issue38105 [ConditionalFact] diff --git a/test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs index 352bc6ac97b..7d0f0f352e0 100644 --- a/test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/AdHocQueryFiltersQueryTestBase.cs @@ -815,4 +815,39 @@ public class FooBar35111 } #endregion + + #region 38132 + + [ConditionalFact] + public virtual async Task Query_filter_with_primary_constructor_parameter() + { + var contextFactory = await InitializeNonSharedTest( + addServices: s => + { + s.AddSingleton(typeof(Guid), + new Guid("00000001-0000-0000-0000-000000000001")); + return s; + }, + usePooling: false); + using var context = contextFactory.CreateDbContext(); + + var result = context.Set().ToList(); + Assert.Empty(result); + } + + protected class Context38132(DbContextOptions options, Guid tenantId) : DbContext(options) + { + protected override void OnModelCreating(ModelBuilder modelBuilder) + => modelBuilder.Entity() + .HasQueryFilter(e => e.TenantId == tenantId); + } + + public class Entity38132 + { + public int Id { get; set; } + public string Name { get; set; } + public Guid TenantId { get; set; } + } + + #endregion } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/AdHocQueryFiltersQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/AdHocQueryFiltersQuerySqlServerTest.cs index a3f7272e3fb..42cdc65a5f8 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/AdHocQueryFiltersQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/AdHocQueryFiltersQuerySqlServerTest.cs @@ -443,6 +443,20 @@ FROM [Locations] AS [l] ) AS [l0] ON [s].[LocationId] = [l0].[LocationId] WHERE [s].[IsDeleted] = 0 ORDER BY [s].[Name] +"""); + } + + public override async Task Query_filter_with_primary_constructor_parameter() + { + await base.Query_filter_with_primary_constructor_parameter(); + + AssertSql( + """ +@ef_filter__tenantId='00000001-0000-0000-0000-000000000001' + +SELECT [e].[Id], [e].[Name], [e].[TenantId] +FROM [Entity38132] AS [e] +WHERE [e].[TenantId] = @ef_filter__tenantId """); } }