From 3b67c3d9198cc540767b4d4eb6f5543b89b4dcb8 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Wed, 22 Sep 2021 09:45:18 -0700 Subject: [PATCH] Use the correct value when the discriminator is part of the key. Optimize the discriminator value in the runtime model Fixes #18126 --- .../CSharpRuntimeModelCodeGenerator.cs | 10 ++++++++++ .../Internal/ValueGenerationManager.cs | 5 +---- .../CSharpRuntimeAnnotationCodeGenerator.cs | 1 - .../Conventions/RuntimeModelConvention.cs | 4 ++-- src/EFCore/Metadata/RuntimeEntityType.cs | 10 +++++++++- src/EFCore/Metadata/RuntimeModel.cs | 7 +++++-- .../DiscriminatorValueGeneratorFactory.cs | 2 +- .../Internal/DiscriminatorValueGenerator.cs | 17 +++-------------- .../CSharpRuntimeModelCodeGeneratorTest.cs | 16 ++++++++-------- .../CompositeKeyEndToEndTestBase.cs | 19 +++++++++++++++---- 10 files changed, 54 insertions(+), 37 deletions(-) diff --git a/src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs b/src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs index 667564d3f5e..9a40b8923e0 100644 --- a/src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs +++ b/src/EFCore.Design/Scaffolding/Internal/CSharpRuntimeModelCodeGenerator.cs @@ -658,6 +658,16 @@ private void Create(IEntityType entityType, CSharpRuntimeAnnotationCodeGenerator .Append(_code.Literal(true)); } + var discriminatorValue = entityType.GetDiscriminatorValue(); + if (discriminatorValue != null) + { + AddNamespace(discriminatorValue.GetType(), parameters.Namespaces); + + mainBuilder.AppendLine(",") + .Append("discriminatorValue: ") + .Append(_code.UnknownLiteral(discriminatorValue)); + } + mainBuilder .AppendLine(");") .AppendLine() diff --git a/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs b/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs index 84d07c99394..8a608bae0ee 100644 --- a/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs +++ b/src/EFCore/ChangeTracking/Internal/ValueGenerationManager.cs @@ -160,10 +160,7 @@ public virtual async Task GenerateAsync( } private ValueGenerator GetValueGenerator(InternalEntityEntry entry, IProperty property) - => _valueGeneratorSelector.Select( - property, property.IsKey() - ? property.DeclaringEntityType - : entry.EntityType); + => _valueGeneratorSelector.Select(property, property.DeclaringEntityType); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/src/EFCore/Design/Internal/CSharpRuntimeAnnotationCodeGenerator.cs b/src/EFCore/Design/Internal/CSharpRuntimeAnnotationCodeGenerator.cs index b33c6138732..287ac6b30c9 100644 --- a/src/EFCore/Design/Internal/CSharpRuntimeAnnotationCodeGenerator.cs +++ b/src/EFCore/Design/Internal/CSharpRuntimeAnnotationCodeGenerator.cs @@ -67,7 +67,6 @@ public virtual void Generate(IEntityType entityType, CSharpRuntimeAnnotationCode foreach (var annotation in annotations) { if (CoreAnnotationNames.AllNames.Contains(annotation.Key) - && annotation.Key != CoreAnnotationNames.DiscriminatorValue && annotation.Key != CoreAnnotationNames.DiscriminatorMappingComplete) { annotations.Remove(annotation.Key); diff --git a/src/EFCore/Metadata/Conventions/RuntimeModelConvention.cs b/src/EFCore/Metadata/Conventions/RuntimeModelConvention.cs index 1ce792486c9..03355f9bfd3 100644 --- a/src/EFCore/Metadata/Conventions/RuntimeModelConvention.cs +++ b/src/EFCore/Metadata/Conventions/RuntimeModelConvention.cs @@ -223,7 +223,8 @@ private RuntimeEntityType Create(IEntityType entityType, RuntimeModel model) entityType.GetDiscriminatorPropertyName(), entityType.GetChangeTrackingStrategy(), entityType.FindIndexerPropertyInfo(), - entityType.IsPropertyBag); + entityType.IsPropertyBag, + entityType.GetDiscriminatorValue()); private ParameterBinding Create(ParameterBinding parameterBinding, RuntimeEntityType entityType) => parameterBinding.With(parameterBinding.ConsumedProperties.Select(property => @@ -256,7 +257,6 @@ protected virtual void ProcessEntityTypeAnnotations( if (CoreAnnotationNames.AllNames.Contains(annotation.Key) && annotation.Key != CoreAnnotationNames.QueryFilter && annotation.Key != CoreAnnotationNames.DefiningQuery - && annotation.Key != CoreAnnotationNames.DiscriminatorValue && annotation.Key != CoreAnnotationNames.DiscriminatorMappingComplete) { annotations.Remove(annotation.Key); diff --git a/src/EFCore/Metadata/RuntimeEntityType.cs b/src/EFCore/Metadata/RuntimeEntityType.cs index 638df57a49b..47fba81093a 100644 --- a/src/EFCore/Metadata/RuntimeEntityType.cs +++ b/src/EFCore/Metadata/RuntimeEntityType.cs @@ -59,6 +59,7 @@ private readonly SortedDictionary _serviceProper private InstantiationBinding? _serviceOnlyConstructorBinding; private readonly PropertyInfo? _indexerPropertyInfo; private readonly bool _isPropertyBag; + private readonly object? _discriminatorValue; // Warning: Never access these fields directly as access needs to be thread-safe private PropertyCounts? _counts; @@ -89,7 +90,8 @@ public RuntimeEntityType( string? discriminatorProperty, ChangeTrackingStrategy changeTrackingStrategy, PropertyInfo? indexerPropertyInfo, - bool propertyBag) + bool propertyBag, + object? discriminatorValue) { Name = name; _clrType = type; @@ -104,6 +106,7 @@ public RuntimeEntityType( _indexerPropertyInfo = indexerPropertyInfo; _isPropertyBag = propertyBag; SetAnnotation(CoreAnnotationNames.DiscriminatorProperty, discriminatorProperty); + _discriminatorValue = discriminatorValue; _properties = new SortedDictionary(new PropertyNameComparer(this)); } @@ -860,6 +863,11 @@ ChangeTrackingStrategy IReadOnlyEntityType.GetChangeTrackingStrategy() return (string?)this[CoreAnnotationNames.DiscriminatorProperty]; } + /// + [DebuggerStepThrough] + object? IReadOnlyEntityType.GetDiscriminatorValue() + => _discriminatorValue; + /// bool IReadOnlyTypeBase.HasSharedClrType { diff --git a/src/EFCore/Metadata/RuntimeModel.cs b/src/EFCore/Metadata/RuntimeModel.cs index d018b88ee30..9e021887431 100644 --- a/src/EFCore/Metadata/RuntimeModel.cs +++ b/src/EFCore/Metadata/RuntimeModel.cs @@ -69,6 +69,7 @@ public virtual void SetSkipDetectChanges(bool skipDetectChanges) /// A value indicating whether this entity type has an indexer which is able to contain arbitrary properties /// and a method that can be used to determine whether a given indexer property contains a value. /// + /// the discriminator value for this entity type. /// The new entity type. public virtual RuntimeEntityType AddEntityType( string name, @@ -78,7 +79,8 @@ public virtual RuntimeEntityType AddEntityType( string? discriminatorProperty = null, ChangeTrackingStrategy changeTrackingStrategy = ChangeTrackingStrategy.Snapshot, PropertyInfo? indexerPropertyInfo = null, - bool propertyBag = false) + bool propertyBag = false, + object? discriminatorValue = null) { var entityType = new RuntimeEntityType( name, @@ -89,7 +91,8 @@ public virtual RuntimeEntityType AddEntityType( discriminatorProperty, changeTrackingStrategy, indexerPropertyInfo, - propertyBag); + propertyBag, + discriminatorValue); if (sharedClrType) { diff --git a/src/EFCore/ValueGeneration/DiscriminatorValueGeneratorFactory.cs b/src/EFCore/ValueGeneration/DiscriminatorValueGeneratorFactory.cs index 5a28fe25502..a040064025a 100644 --- a/src/EFCore/ValueGeneration/DiscriminatorValueGeneratorFactory.cs +++ b/src/EFCore/ValueGeneration/DiscriminatorValueGeneratorFactory.cs @@ -17,6 +17,6 @@ public class DiscriminatorValueGeneratorFactory : ValueGeneratorFactory { /// public override ValueGenerator Create(IProperty property, IEntityType entityType) - => new DiscriminatorValueGenerator(entityType.GetDiscriminatorValue()!); + => new DiscriminatorValueGenerator(); } } diff --git a/src/EFCore/ValueGeneration/Internal/DiscriminatorValueGenerator.cs b/src/EFCore/ValueGeneration/Internal/DiscriminatorValueGenerator.cs index 041c74ca20d..05a99ab71dc 100644 --- a/src/EFCore/ValueGeneration/Internal/DiscriminatorValueGenerator.cs +++ b/src/EFCore/ValueGeneration/Internal/DiscriminatorValueGenerator.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.EntityFrameworkCore.ChangeTracking; +using Microsoft.EntityFrameworkCore.ChangeTracking.Internal; +using Microsoft.EntityFrameworkCore.Infrastructure; namespace Microsoft.EntityFrameworkCore.ValueGeneration.Internal { @@ -13,19 +15,6 @@ namespace Microsoft.EntityFrameworkCore.ValueGeneration.Internal /// public class DiscriminatorValueGenerator : ValueGenerator { - private readonly object _discriminator; - - /// - /// 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. - /// - public DiscriminatorValueGenerator(object discriminator) - { - _discriminator = discriminator; - } - /// /// 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 @@ -33,7 +22,7 @@ public DiscriminatorValueGenerator(object discriminator) /// doing so can result in application failures when updating to a new Entity Framework Core release. /// protected override object NextValue(EntityEntry entry) - => _discriminator; + => entry.GetInfrastructure().EntityType.GetDiscriminatorValue()!; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs index c93b47dedc1..d0fb98f0198 100644 --- a/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Scaffolding/Internal/CSharpRuntimeModelCodeGeneratorTest.cs @@ -652,7 +652,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas ""Microsoft.EntityFrameworkCore.TestModels.AspNetIdentity.IdentityUser"", typeof(IdentityUser), baseEntityType, - discriminatorProperty: ""Discriminator""); + discriminatorProperty: ""Discriminator"", + discriminatorValue: ""IdentityUser""); var id = runtimeEntityType.AddProperty( ""Id"", @@ -769,7 +770,6 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) { - runtimeEntityType.AddAnnotation(""DiscriminatorValue"", ""IdentityUser""); Customize(runtimeEntityType); } @@ -801,14 +801,14 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType bas ""Microsoft.EntityFrameworkCore.Scaffolding.Internal.IdentityUser"", typeof(IdentityUser), baseEntityType, - discriminatorProperty: ""Discriminator""); + discriminatorProperty: ""Discriminator"", + discriminatorValue: ""DerivedIdentityUser""); return runtimeEntityType; } public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) { - runtimeEntityType.AddAnnotation(""DiscriminatorValue"", ""DerivedIdentityUser""); Customize(runtimeEntityType); } @@ -958,7 +958,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType? ba ""Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGeneratorTest+DependentBase"", typeof(CSharpRuntimeModelCodeGeneratorTest.DependentBase), baseEntityType, - discriminatorProperty: ""EnumDiscriminator""); + discriminatorProperty: ""EnumDiscriminator"", + discriminatorValue: CSharpMigrationsGeneratorTest.Enum1.One); var principalId = runtimeEntityType.AddProperty( ""PrincipalId"", @@ -1040,7 +1041,6 @@ public static RuntimeForeignKey CreateForeignKey2(RuntimeEntityType declaringEnt public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) { - runtimeEntityType.AddAnnotation(""DiscriminatorValue"", CSharpMigrationsGeneratorTest.Enum1.One); runtimeEntityType.AddAnnotation(""Relational:FunctionName"", null); runtimeEntityType.AddAnnotation(""Relational:Schema"", null); runtimeEntityType.AddAnnotation(""Relational:SqlQuery"", null); @@ -1480,7 +1480,8 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType? ba ""Microsoft.EntityFrameworkCore.Scaffolding.Internal.CSharpRuntimeModelCodeGeneratorTest+DependentDerived"", typeof(CSharpRuntimeModelCodeGeneratorTest.DependentDerived), baseEntityType, - discriminatorProperty: ""EnumDiscriminator""); + discriminatorProperty: ""EnumDiscriminator"", + discriminatorValue: CSharpMigrationsGeneratorTest.Enum1.Two); var data = runtimeEntityType.AddProperty( ""Data"", @@ -1505,7 +1506,6 @@ public static RuntimeEntityType Create(RuntimeModel model, RuntimeEntityType? ba public static void CreateAnnotations(RuntimeEntityType runtimeEntityType) { - runtimeEntityType.AddAnnotation(""DiscriminatorValue"", CSharpMigrationsGeneratorTest.Enum1.Two); runtimeEntityType.AddAnnotation(""Relational:FunctionName"", null); runtimeEntityType.AddAnnotation(""Relational:Schema"", null); runtimeEntityType.AddAnnotation(""Relational:SqlQuery"", null); diff --git a/test/EFCore.Specification.Tests/CompositeKeyEndToEndTestBase.cs b/test/EFCore.Specification.Tests/CompositeKeyEndToEndTestBase.cs index 207f26e61ea..3df0fadb08a 100644 --- a/test/EFCore.Specification.Tests/CompositeKeyEndToEndTestBase.cs +++ b/test/EFCore.Specification.Tests/CompositeKeyEndToEndTestBase.cs @@ -26,13 +26,16 @@ public virtual async Task Can_use_two_non_generated_integers_as_composite_key_en using (var context = CreateContext()) { - context.Add( + var pegasus = context.Add( new Pegasus { Id1 = ticks, Id2 = ticks + 1, Name = "Rainbow Dash" }); + + Assert.Equal("Pegasus", pegasus.Entity.Discriminator); + await context.SaveChangesAsync(); } @@ -40,6 +43,7 @@ public virtual async Task Can_use_two_non_generated_integers_as_composite_key_en { var pegasus = context.Pegasuses.Single(e => e.Id1 == ticks && e.Id2 == ticks + 1); + Assert.Equal("Pegasus", pegasus.Discriminator); pegasus.Name = "Rainbow Crash"; await context.SaveChangesAsync(); @@ -212,13 +216,15 @@ public BronieContext(DbContextOptions options) protected override void OnModelCreating(ModelBuilder modelBuilder) { - modelBuilder.Entity( + modelBuilder.Entity( b => { b.HasKey( - e => new { e.Id1, e.Id2 }); + e => new { e.Id1, e.Id2, e.Discriminator }); }); + modelBuilder.Entity(); + modelBuilder.Entity( b => { @@ -243,10 +249,15 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) } } - protected class Pegasus + protected abstract class Flyer { + public string Discriminator { get; set; } public long Id1 { get; set; } public long Id2 { get; set; } + } + + protected class Pegasus : Flyer + { public string Name { get; set; } }